This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] Add new rule for MIN(MAX(~A, ~B), ~C) et. al.
ClosedPublic

Authored by sanjoy on Apr 29 2015, 3:12 PM.

Details

Summary

Optimizing these well are especially interesting for IRCE since it
"clamps" values by generating this sort of pattern through SCEV
expressions.

Depends on D9352.

Diff Detail

Repository
rL LLVM

Event Timeline

sanjoy updated this revision to Diff 24660.Apr 29 2015, 3:12 PM
sanjoy retitled this revision from to [InstCombine] Add new rule for MIN(MAX(~A, ~B), ~C) et. al..
sanjoy updated this object.
sanjoy edited the test plan for this revision. (Show Details)
sanjoy added a reviewer: majnemer.
sanjoy added a subscriber: Unknown Object (MLST).
majnemer added inline comments.Apr 29 2015, 8:55 PM
lib/Transforms/InstCombine/InstCombineSelect.cpp
24 ↗(On Diff #24660)

getInverseMinMaxSelectPattern would be more conforming to the coding standards.

40 ↗(On Diff #24660)

Let's call this generateMinMaxSelectPattern instead.

43–57 ↗(On Diff #24660)

It might be late but why the lambda? I guess it makes the control flow a little nicer. How about we just split it out to another function?

902 ↗(On Diff #24660)

Can we kill this newline?

903 ↗(On Diff #24660)

The other transforms are a little more concrete in that they are described with a specific SelectPatternFlavor and an actual value for the constant. I think the abstract pattern would still be good to keep around though.

Speaking of which, I'm going to have to admit that I am not quite sure what ~! means exactly.

sanjoy added inline comments.Apr 29 2015, 9:21 PM
lib/Transforms/InstCombine/InstCombineSelect.cpp
24 ↗(On Diff #24660)

done

40 ↗(On Diff #24660)

done

43–57 ↗(On Diff #24660)

How about we just split it out to another function?

done

902 ↗(On Diff #24660)

done

903 ↗(On Diff #24660)

The abstract pattern does not abstract over very much. :) I'll just document the four possibilities explicitly.

sanjoy updated this revision to Diff 24678.Apr 29 2015, 9:21 PM
  • address review comments
majnemer accepted this revision.Apr 29 2015, 9:24 PM
majnemer edited edge metadata.

LGTM

This revision is now accepted and ready to land.Apr 29 2015, 9:24 PM
This revision was automatically updated to reflect the committed changes.