Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

InstCombine: simplify signed range checks

Authored by eeckstein on Nov 26 2014, 7:27 AM.



This is a patch for InstCombine. It tries to simplify range checks.
(icmp sge x, 0) & (icmp slt x, n) --> icmp ult x, n
(icmp slt x, 0) | (icmp sgt x, n) --> icmp ugt x, n

Diff Detail

Event Timeline

eeckstein updated this revision to Diff 16651.Nov 26 2014, 7:27 AM
eeckstein retitled this revision from to InstCombine: simplify signed range checks.
eeckstein updated this object.
eeckstein edited the test plan for this revision. (Show Details)
eeckstein added a reviewer: reames.
eeckstein added a subscriber: Unknown Object (MLST).

This is already the second version of the patch. The first version was discussed by email.

The main difference to the first version is that I added support for the inverted case: icmp | icmp (in addition to icmp & icmp).

My comments to the original email from Philip:

Is there a reason you're only handling the upper bound phrased as a sgt/sge? Do we canonicalize away the alternate slt/sle formation?

Yes, but as I'm now also handling the inverted case, I have to deal with all the possibilities anyway.

This doesn't need to be a member function does it? A static function would be preferred.

It's a member function because it uses the DL, AT and DT members.

reames accepted this revision.Dec 2 2014, 5:54 PM
reames edited edge metadata.



Minor style: I might hide the boolean parameter behind a helper function. It would help readability at the call site.

simplifyRangeCheck(A,B) { return simplifyRangeCheckImpl(A,B,false); }
simplifyInvertedRangeCheck(A,B) { return simplifyRangeCheckImpl(A,B, true); }


This would be clearer as:
if (!(a || b))

Minor, feel free to ignore if you disagree.

This revision is now accepted and ready to land.Dec 2 2014, 5:54 PM
eeckstein closed this revision.Dec 3 2014, 2:41 AM

commited in r223224


I added a /*Inverted=*/ comment at the call sites. This should improve readability as well.