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.
Thanks,
Prathamesh
>
> Thanks,
> Richard.
