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
