On Thu, Nov 15, 2012 at 9:48 AM, Michael Matz <m...@suse.de> wrote: > Hi Lawrence, > > On Wed, 14 Nov 2012, Lawrence Crowl wrote: > >> Diego and I seek your comments on the following (loose) proposal. > > In principle I agree with the goal, I'm not sure I like the specific way > yet, and even if I do I have some suggestions:
Sure. We do not have very firm notions yet. We have only started exploring this recently. We wanted to discuss our ideas early on to make sure we are going in the right direction. >> We will add a set of helper classes to be used as local variables >> to manage the details of handling the existing types. > > I think one goal should be to minimize the number of those helper classes > if we can. And use clear names, for the statement builder e.g. > stmt_builder, or the like, not just ssa_seq. Sure. > >> We propose a simplified form using new build helper classes ssa_seq >> and ssa_stmt that would allow the above code to be written as >> follows. >> >> ssa_seq q; >> ssa_stmt t = q.stmt (NE_EXPR, shadow, 0); >> ssa_stmt a = q.stmt (BIT_AND_EXPR, base_addr, 7); >> ssa_stmt b = q.stmt (shadow_type, a); > > I think consistency should trump brevity here, so also add a tree code for > the converter, i.e. > ssa_stmt b = q.stmt (NOP_EXPR, shadow_type, a); Ah, yes. This one was amusing. When we were drafting the proposal, Lawrence kept wondering what this NOP_EXPR thing is. I've been suffering this name for so long, that it no longer irritates me. Had it been named CAST_EXPR, or even NOP_CAST_EXPR, he would have probably kept it in the example code :) > The method name should imply the action, e.g. 'add_stmt' or append_stmt > or the like. I'm not sure if we need the ssa_stmt class. We could use > overloading to accept 'gimple' as operands, with the understanding that > those will be implicitely converted to 'tree' by accessing the LHS: Hm, maybe you are right. The main goal was reduce all the ssa name and temporary creation needed to glue the statements together. > gimple append_stmt (gimple g, tree_code code, gimple op1, tree op2) > { > return append_stmt (g, code, gimple_lhs (op1), op2); > } > > (where gimple_lhs would ICE when the stmt in question doesn't have > one). As gimple statements are their own iterators meanwhile I'm not even > sure if you need the ssa_seq (or ssa_stmt_builder) class. The above > append_stmt would simply add the newly created statement to after 'g'. Yes, I think that could work. More details will surface as we start the implementation, of course. > All in all I think we can severely improve on building gimple statements > without introduction of any helper class. Basically, whenever a helper > class merely contains one member of a different type, then I think that > other type should be improved so that the wrapper class isn't necessary > anymore. Fewer types -> good :) Sure. If the helper classes do not contain any significant state that cannot be gleaned from the operands to the builder, then we won't introduce them. > >> Generating a Type > > Apart from naming of some methods to imply the action done, I don't have > any opinion about this at this point. Though I agree that again the > boilerplate sequencing (_CONTEXT and _CHAIN) should be improved. Right. I think it's mostly record types that will benefit from this type of shortening. Building other types is usually more straightforward. Diego.