On Mon, 18 May 2015, Prathamesh Kulkarni wrote: > On 18 May 2015 at 14:12, Richard Biener <rguent...@suse.de> wrote: > > On Sat, 16 May 2015, Prathamesh Kulkarni wrote: > > > >> Hi, > >> genmatch generates incorrect code for following (artificial) pattern: > >> > >> (for op (plus) > >> op2 (op) > >> (simplify > >> (op @x @y) > >> (op2 @x @y) > >> > >> generated gimple code: http://pastebin.com/h1uau9qB > >> 'op' is not replaced in the generated code on line 33: > >> *res_code = op; > >> > >> I think it would be a better idea to make op2 iterate over same set > >> of operators (op2->substitutes = op->substitutes). > >> I have attached patch for the same. > >> Bootstrap + testing in progress on x86_64-unknown-linux-gnu. > >> OK for trunk after bootstrap+testing completes ? > > > > Hmm, but then the example could as well just use 'op'. I think we > > should instead reject this. > > > > Consider > > > > (for op (plus minus) > > (for op2 (op) > > (simplify ... > > > > where it is not clear what would be desired. Simple replacement > > of 'op's value would again just mean you could have used 'op' in > > the first place. Doing what you propose would get you > > > > (for op (plus minus) > > (for op2 (plus minus) > > (simplify ... > > > > thus a different iteration. > > > >> I wonder if we really need is_oper_list flag in user_id ? > >> We can determine if user_id is an operator list > >> if user_id::substitutes is not empty ? > > > > After your change yes. > > > >> That will lose the ability to distinguish between user-defined operator > >> list and list-iterator in for like op/op2, but I suppose we (so far) don't > >> need to distinguish between them ? > > > > Well, your change would simply make each list-iterator a (temporary) > > user-defined operator list as well as the current iterator element > > (dependent on context - see the nested for example). I think that > > adds to confusion. > > > > So - can you instead reject this use? > Well my intention was to have support for walking operator list in reverse. > For eg: > (for bitop (bit_and bit_ior) > rbitop (bit_ior bit_and) > ...) > Could be replaced by sth like: > (for bitop (bit_and bit_ior) > rbitop (~bitop)) > ...) > > where "~bitop" would indicate walking (bit_and bit_ior) in reverse. > Would that be a good idea ? For symmetry, I thought > (for op (list) > op2 (op)) > should be supported too.
Hmm, but is this really a useful extension? To me it just complicates the syntax for the occasional reader. Richard. > > Thanks, > Prathamesh > > > > > > Thanks, > > Richard. > > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nuernberg)
