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