This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [match-and-simplify] fix incorrect code-gen in 'for' pattern


On 18 May 2015 at 14:12, Richard Biener <rguenther@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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]