This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] Pull shifts through a select plus binop with constant
ClosedPublic

Authored by craig.topper on Oct 23 2017, 8:18 PM.

Details

Summary

This pulls shifts through a select+binop with a constant where the select conditionally executes the binop. We already do this for just the binop, but not with the select.

This can allow us to get the select closer to other selects to enable removing one.

Diff Detail

Repository
rL LLVM

Event Timeline

craig.topper created this revision.Oct 23 2017, 8:18 PM
spatel added inline comments.Nov 6 2017, 9:29 AM
lib/Transforms/InstCombine/InstCombineShifts.cpp
318–320 ↗(On Diff #119996)

Since we're moving this, we might as well fix the formatting - capitalize variables.

539–544 ↗(On Diff #119996)

Use matchers to reduce the number of if-checks and indenting?

CmpInst::Predicate P;
BinaryOperator *TVI;
Value *FalseVal;
if (match(Op0, m_Select(P, m_OneUse(m_BinOp(TVI)), m_Value(FalseVal))) ...

Address Sanjay's comments.

spatel accepted this revision.Nov 6 2017, 11:25 AM

LGTM. See inline for a couple of other minor bits.

lib/Transforms/InstCombine/InstCombineShifts.cpp
537–538 ↗(On Diff #121758)
// If we have a select that conditionally executes some binary operator,
// see if we can pull the shift and binop ahead of the select.

This would be clearer if we copied one of the test patterns in the comment as an example of this transform.

545–547 ↗(On Diff #121758)

More && clauses? Could combine these ifs.

This revision is now accepted and ready to land.Nov 6 2017, 11:25 AM
This revision was automatically updated to reflect the committed changes.