On Fri, Mar 16, 2018 at 4:30 PM, Dylan Baker <dy...@pnwbakers.com> wrote:
> Quoting Rob Clark (2018-03-16 12:20:10)
>> On Fri, Mar 16, 2018 at 3:13 PM, Jason Ekstrand <ja...@jlekstrand.net>
wrote:
>> > On Fri, Mar 16, 2018 at 11:53 AM, Dylan Baker <dy...@pnwbakers.com> wrote:
>> >>
>> >> Quoting Jason Ekstrand (2018-03-16 11:38:47)
>> >> > On Fri, Mar 16, 2018 at 11:28 AM, Dylan Baker <dy...@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
