[Mesa-dev] [PATCH v0] nir: mako all the intrinsics

[Jason Ekstrand]

On March 16, 2018 23:36:50 Dylan Baker <dylan at pnwbakers.com> wrote:

> Quoting Rob Clark (2018-03-16 14:25:09)
>> On Fri, Mar 16, 2018 at 4:30 PM, Dylan Baker <dylan at pnwbakers.com> wrote:
>> > Quoting Rob Clark (2018-03-16 12:20:10)
>> >> On Fri, Mar 16, 2018 at 3:13 PM, Jason Ekstrand <jason at jlekstrand.net> 
>> wrote:
>> >> > On Fri, Mar 16, 2018 at 11:53 AM, Dylan Baker <dylan at pnwbakers.com> wrote:
>> >> >>
>> >> >> Quoting Jason Ekstrand (2018-03-16 11:38:47)
>> >> >> > On Fri, Mar 16, 2018 at 11:28 AM, Dylan Baker <dylan at pnwbakers.com>
>> >> >> > wrote:
>> >> >> >
>> >> >> >     intr_opcodes = {
>> >> >> >         'nop': Intrinsic('nop', flags=[CAN_ELIMINATE]),
>> >> >> >         ...
>> >> >> >     }
>> >> >> >
>> >> >> >     I prefer this since each dictionary is clearly created without a
>> >> >> > function
>> >> >> >     obscuring what's actually going on. If you dislike having to repeat
>> >> >> > the
>> >> >> >     name you
>> >> >> >     could even do something like:
>> >> >> >     intr_opcodes = [
>> >> >> >         'nop': Intrinsic('nop', flags=[CAN_ELIMINATE]),
>> >> >> >         ...
>> >> >> >     ]
>> >> >> >     intr_opcodes = {i.name: i for i in intr_opcodes}
>> >> >> >
>> >> >> >
>> >> >> > I'm not sure what I think about this.  On the one hand, having the
>> >> >> > dictionary
>> >> >> > explicitly declared is nice.  On the other hand, in nir_opcodes.py we
>> >> >> > have a
>> >> >> > bunch of other helper functions we declare along the way to help with
>> >> >> > specific
>> >> >> > kinds of opcodes.  It's not as practical to do this if everything is
>> >> >> > inside of
>> >> >> > a dictionary declaration.
>> >> >>
>> >> >> Why not?
>> >> >>
>> >> >> def make_op(name, *args):
>> >> >>     return Intrinsic(name, foo='bar', *args)
>> >> >>
>> >> >> intr_opcodes = [
>> >> >>     make_op('nop', ...),
>> >> >> ]
>> >> >
>> >> >
>> >> > Because it's nice to keep the definition of the wrapper close to where 
>> it's
>> >> > used.
>> >> >
>> >>
>> >> also, fwiw, I end up needing two sets (or possibly lists), a second
>> >> one for the builders that are generated for sysval intrinsics.. I'm
>> >> not entirely sure how that would work if everything was declared
>> >> inline instead of via helper fxns
>> >
>> > I'm missing where a helper function adds an intrinsic to more than one list.
>>
>> system_value() adds to system_values table, after calling intrinsic()
>> which adds to the global table (this is the reason for the return
>> statement in intrinsic()
>>
>> BR,
>> -R
>>
>
> I don't know, maybe it's just that I really don't like side effects, but
> functions with side-effects that call functions with side-effects make me
> nervous.

I think side effects are ok here.  This is one of those cases where we're 
not so much writing python code as writing a tiny DSL in python for 
declaring NIR intrinsics.  Alternatively, you can think of the "intrinsic" 
function as a sort of "add this to a database" function which is something 
which has fairly natural and easy to understand side effects.  In either 
case, the focus needs to be on the NIR intrinsics being declared and the 
details of how the python works should fade into the background.

--Jason