<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Mar 19, 2018 at 10:21 AM, Dylan Baker <<a href="mailto:dylan@pnwbakers.com" target="_blank">dylan@pnwbakers.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Quoting Jason Ekstrand (2018-03-17 23:37:51) <div><div class="h5">> On Sat, Mar 17, 2018 at 10:32 PM, Dylan Baker <<a href="mailto:dylan@pnwbakers.com">dylan@pnwbakers.com</a>> wrote: > > Quoting Jason Ekstrand (2018-03-17 09:53:13) > > On March 16, 2018 23:36:50 Dylan Baker <<a href="mailto:dylan@pnwbakers.com">dylan@pnwbakers.com</a>> wrote: > > > > > Quoting Rob Clark (2018-03-16 14:25:09) > > >> On Fri, Mar 16, 2018 at 4:30 PM, Dylan Baker <<a href="mailto:dylan@pnwbakers.com">dylan@pnwbakers.com</a>> > wrote: > > >> > Quoting Rob Clark (2018-03-16 12:20:10) > > >> >> On Fri, Mar 16, 2018 at 3:13 PM, Jason Ekstrand < > <a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> > > >> wrote: > > >> >> > On Fri, Mar 16, 2018 at 11:53 AM, Dylan Baker < > <a href="mailto:dylan@pnwbakers.com">dylan@pnwbakers.com</a>> wrote: > > >> >> >> > > >> >> >> Quoting Jason Ekstrand (2018-03-16 11:38:47) > > >> >> >> > On Fri, Mar 16, 2018 at 11:28 AM, Dylan Baker < > <a href="mailto:dylan@pnwbakers.com">dylan@pnwbakers.com</a>> > > >> >> >> > 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 = {<a href="http://i.name" rel="noreferrer" target="_blank">i.name</a>: 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. > > > That is likely true. :-) > > > 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. > > > Yes they do and I whole-heatedly agree with your desire for cleanliness. The > reason why this particular script gets sticky is that there are two pieces > whose cleanliness is in conflict: the list of intrinsic definitions, and the > python code which puts them into a data structure. You are arguing for the > cleanliness of the python code over the list of declarations. What is really > important in this file, however, is the list of declarations. The python code > that turns those declarations into a data structure is just an implementation > detail. > > Unfortunately, all of the mechanisms that have so-far been suggested to clean > up the python have one or more of the following side-effects: > > 1) Duplicating names of opcodes so that they can get out-of-sync > 2) Replacing immutable objects with mutable ones (so we can add names later) > 3) Expose the underlying data structure used to store the opcodes. > > The third one is interesting because, in nir_opcodes.py, I believe we did > actually change the underlying data structure at one point. If it's exposed, > then doing so requires altering every single line of the file. > > The more I think about this, the more I think that Rob's code is more-or-less > the right direction. I'm not sure what to think about system values and agree > that adding a side-effecting wrapper for those seems wrong. However, I think > that having declaration functions is, in this case, actually the cleanest way > to do it when you consider all of the different pieces in play. > > --Jason </div></div>The side effects of side effects is *exactly* why the GLX generators are *HORRIBLE*, you can't change anything because every function has a return value and at least one side-effect. Want to alter a return value? oh, sorry that causes a function three levels down in the call chain's side effect to change and now the generated code is completely broken. At the very least I will put my foot down hard on return values and side effects.</blockquote><div> </div><div>I think I agree with you on return values. If we're going to use a "register an intrinsic" model, we shouldn't have magic create+register+other functions. We need to pick and I'd like to pick side-effects and can return values. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I really don't like side effects, but pick one. And you're right, I'm asking that the python code that has to be maintained should be maintainable. I don't understand why that's such a big ask. </blockquote><div> </div><div>I also want it to be maintainable. I just don't think that </div><div>INTRINSICS = { </div><div> 'foo' = intrinsic('foo', ...) ... } </div><div>is maintainable. </div><div>--Jason </div></div></div></div>