Page MenuHomePhabricator

[InstCombine] canonicalize min/max constant to select's false value

Authored by spatel on Nov 10 2016, 2:24 PM.



This is a first step towards canonicalization and improved folding/codegen for integer min/max as discussed here:

Here, we're just matching the simplest min/max patterns and adjusting the icmp predicate while swapping the select operands.

I've included FIXME tests in test/Transforms/InstCombine/select_meta.ll so it's easier to see how this might be extended (corresponds to the TODO comment in the code). That's also why I'm using matchSelectPattern() rather than a simpler check; once the backend is patched, we can just remove some of the restrictions to allow the obfuscated min/max patterns in the FIXME tests to be matched.

Diff Detail


Event Timeline

spatel updated this revision to Diff 77554.Nov 10 2016, 2:24 PM
spatel retitled this revision from to [InstCombine] canonicalize min/max constant to select's false value .
spatel updated this object.
spatel added a subscriber: llvm-commits.
Ayal added a subscriber: Ayal.Nov 14 2016, 8:19 AM
Ayal added inline comments.
515–516 ↗(On Diff #77554)

When the constant value of the select differs from that of the cmp, is it still considered min/max?

spatel added inline comments.Nov 14 2016, 8:30 AM
515–516 ↗(On Diff #77554)

Yes - if you believe ValueTracking's (matchSelectPattern) definition of min/max. Eg:

// Look through 'not' ops to find disguised signed min/max.
// (X >s C) ? ~X : ~C ==> (~X <s ~C) ? ~X : ~C ==> SMIN(~X, ~C)

I recently (rL286772) moved obfuscated patterns like that to "matchMinMax()" if you want to see the full list of patterns that we currently recognize as min/max.

jmolloy accepted this revision.Nov 18 2016, 12:52 AM
jmolloy edited edge metadata.

LGTM. Thanks in particular for the heavy testing!

This revision is now accepted and ready to land.Nov 18 2016, 12:52 AM
This revision was automatically updated to reflect the committed changes.