This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] push constant operand down/outside in sequence of min/max intrinsics
ClosedPublic

Authored by spatel on Feb 16 2022, 9:15 AM.

Details

Summary

A generalization like this was suggested in D119754. This is the inverse direction of D119851, and we get all of the folds there plus the one that was missed.

There is precedence for this kind of transform in instcombine with "or" instructions (but strangely only with that one opcode AFAICT).

Similar justification as in the other patch:
The line between instcombine and reassociate for these kinds of folds is blurry. This doesn't appear to have much cost and gives us the expected wins from repeated folds as seen in the last set of test diffs.

Diff Detail

Event Timeline

spatel created this revision.Feb 16 2022, 9:15 AM
spatel requested review of this revision.Feb 16 2022, 9:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 16 2022, 9:15 AM
lebedev.ri added inline comments.Feb 16 2022, 9:23 AM
llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
924

Constant or ImmConstant?

spatel marked an inline comment as done.Feb 16 2022, 9:30 AM
spatel added inline comments.
llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
924

I figured any constant would be safer. I'm not sure what happens if we put constant expressions in these patterns, so just give up?

lebedev.ri added inline comments.Feb 16 2022, 9:33 AM
llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
924

This fold only hoists an immconstant, so there indeed is a chance of looping iff x or y is a immconstant,
but i don't see why non-imm constant would be dangerous.

spatel updated this revision to Diff 409337.Feb 16 2022, 11:03 AM
spatel marked 2 inline comments as done.

Patch updated:
Allow constant expression transform (test added).

lebedev.ri accepted this revision.Feb 16 2022, 11:04 AM

LGTM, thank you!

This revision is now accepted and ready to land.Feb 16 2022, 11:04 AM
This revision was landed with ongoing or failed builds.Feb 17 2022, 7:38 AM
This revision was automatically updated to reflect the committed changes.