[Mesa-dev] [PATCH 02/11] nir: Add a pass-running infastructure

[Ian Romanick]

On 10/28/2015 04:43 PM, Connor Abbott wrote:
> On Wed, Oct 28, 2015 at 7:06 PM, Ian Romanick <idr at freedesktop.org> wrote:
>> On 10/28/2015 02:32 PM, Jason Ekstrand wrote:
>>> ---
>>>  src/glsl/nir/nir.h      | 19 +++++++++++++++
>>>  src/glsl/nir/nir_pass.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++-
>>>  2 files changed, 82 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
>>> index e3777f9..069c7c1 100644
>>> --- a/src/glsl/nir/nir.h
>>> +++ b/src/glsl/nir/nir.h
>>> @@ -1582,6 +1582,25 @@ typedef struct nir_shader {
>>>        foreach_list_typed(nir_function_overload, overload, node, \
>>>                           &(func)->overload_list)
>>>
>>> +typedef struct nir_pass {
>>
>> A couple years ago Kristian, with the support of everyone, waged a war
>> on 'typedef struct foo { ... } foo;'  Has this awful idiom really risen
>> from the dead?  Can we please send it back to the grave?
> 
> (flamewar incoming!)
> 
> Yes, it has, and no.
> 
> In case you haven't read any NIR code, NIR uses this all over the
> place -- all of the core datastructures are typedef'd. The only
> argument I've heard against this practice, from Linus, is that it
> makes telling if a value is a lightweight thing, like an integer, or
> not. But this isn't the kernel; we don't use typedefs for integers at
> all, and the only place where we use nir_* and it *isn't* a structure
> is the enums. Pretty much the entire time, it's fairly obvious these
> are enums, either because they have "type" in the name (e.g.
> nir_instr_type), indicating C-style subclassing, or they're some kind
> of operation code (nir_op and nir_intrinsic_op). This is fairly
> subjective, but if you're not able to look at a nir_* type and know
> whether it's a lightweight thing or not, then you probably don't know
> the code well enough to understand it anyways. The amount of typing
> they save is just way too large compared to the potential
> (theoretical) inconvenience of not knowing how the type is declared.
> But if this bikeshed seriously disturbs you, then I'd much rather
> change the enums to not be typedef'd, since you have to type out their
> types a lot less often.

As I've been going through Meta today, I was reminded the other (much
more important IMO) reason for not doing the typedef business.  I
believe this is part of the reason we did the giant s/GLcontext/struct
gl_context/g a couple years ago.

Meta has a few structures, like meta_clear_state, that nothing outside
meta ever needs to know anything about.  However, instances of these
structures need to be tracked in the gl_context so that they can be used
the next time we enter meta, freed when the context is destroyed, etc.

You really don't want to put the definition of meta_clear_state in
mtypes.h both for encapsulation and avoiding rebuilding the world
whenever you change meta_clear_state.  You also don't want to stick a
'void *meta_clear;' in gl_context.  That sounds awful.

C language lets you use 'struct meta_clear_state *meta_clear;' without
knowing what a 'struct meta_clear_state' is.  If you want to use a
'meta_clear_state_t *' or similar, you at have to sprinkle the typedefs
in multiple places.  Then you have the same information in two places to
maintain.  That also sounds awful.

Or you mix uses of 'meta_clear_state_t *' and 'struct meta_clear_state
*'.  That also sounds awful.

Or you decide that "because reasons" some structures get a typedef and
some don't. That also sounds awful.

Once you eliminate the awful, whatever remains, no matter how much extra
typing, must be the truth.  Or something along those lines.