Re: [RFA] Reimplement canonicalization of comparison arguments in match.pd

[Jeff Law <law@redhat.com>]

On 05/30/2015 03:57 AM, Marc Glisse wrote:
> On Fri, 29 May 2015, Jeff Law wrote:
>
>> c-common.c::shorten_compare has code to canonicalize the arguments of
>> a comparison so that the constant is the second argument.  This patch
>> removes the implementation from c-common.c and instead implements it
>> in match.pd.
>>
>> Note the match.pd tries to match the prior behavior of
>> shorten_compare, hence the strange handling of zero.  No justification
>> exists AFAIK for that strange handling in shorten_compare.
>>
>> The match.pd pattern is primarily Kai's -- I just took the 4 patterns
>> he wrote and squashed them into a single pattern to avoid the test
>> duplication.
>>
>> The xfailed testcase is only case I saw across my comparison tests
>> where this change regressed.  Basically shorten-compare had something
>> non-canonical when called.  It was able to canonicalize, then optimize
>> the result.  I just wanted a record of that test in the testsuite.
>> Obviously if we hit our goal of implementing everything from
>> shorten_compare, that test will no longer need to be xfailed :-)
>>
>> Bootstrapped and regression tested on x86-linux-gnu.  OK for the trunk?
>
> I understand doing it in 2 commits to better see what regresses, but I
> don't think we should keep the weirdness in match.pd.
By the weirdness, are you referring to the handling of zero?  I don't 
mind losing that at all.    I'd speculate that Kai was just trying to 
mirror what shorten_compare did to make testing easier.

In fact, that seems like something we ought to test.  If that special 
handling gets ripped out, does anything change and if it does we can 
evaluate the pros/cons of whatever we see.


>
> Does it regress anything if we instead add inside the for loop that
> follows /* -A CMP -B -> B CMP A.  */
>
> (simplify
>   (cmp CONSTANT_CLASS_P@0 @1)
>   (scmp @1 @0))
I'll give it a try.  If we can cleanly integrate this into an existing 
pattern, that works for me.

Jeff