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

[Dylan Baker]

Quoting Jason Ekstrand (2018-03-17 09:53:13)
> 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.
> 

That's the sort of logic that made the mess in mapi/glapi/gen. These generators
are like the shell script thats "just a one off script, it's just a few lines",
and suddenly ten people have modified it and no one understands how it works or
can modify it without everything falling apart. Code generators need to be clean
and maintainable just like the C and C++ code that makes up mesa. Seriously,
this is why I'm rewriting all of the generators in mapi/glapi/gen instead of
trying to fix them to work with Khronos XML.

Dylan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180317/7fda4629/attachment.sig>