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.

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))
Nothing regresses compared to the version I posted with the distinct patterns for canonicalization in my tests. This seems the cleanest, so I'm going to spin it as a patch and repost.

Thanks for the suggestion,
jeff

Reply via email to