This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] Simplify funnel shift with one zero/undef operands to simple shift
ClosedPublic

Authored by nikic on Nov 20 2018, 3:44 PM.

Details

Summary

The following simplifications are implemented:

  • fshl(X, 0, C) -> shl X, C%BW
  • fshl(X, undef, C) -> shl X, C%BW (assuming undef = 0)
  • fshl(0, X, C) -> lshr X, BW-C%BW
  • fshl(undef, X, C) -> lshr X, BW-C%BW (assuming undef = 0)
  • fshr(X, 0, C) -> shl X, (BW-C%BW)
  • fshr(X, undef, C) -> shl X, BW-C%BW (assuming undef = 0)
  • fshr(0, X, C) -> lshr X, C%BW
  • fshr(undef, X, C) -> lshr, X, C%BW (assuming undef = 0)

The simplification is only performed if the shift amount C is constant, because we can explicitly compute C%BW and BW-C%BW in this case.


This change has been split off from D54666 and handling for the zero case has been added. Unfortunately that also means new tests :(

Diff Detail

Event Timeline

nikic created this revision.Nov 20 2018, 3:44 PM
nikic updated this revision to Diff 174978.Nov 21 2018, 1:49 PM

Rebase over baseline tests.

spatel added inline comments.Nov 22 2018, 1:16 PM
lib/Transforms/InstCombine/InstCombineCalls.cpp
2000

This can be made an assert. We are guaranteed to call InstSimplify (line 1844) for this instruction before we reach this point in InstCombine.

2009

The convention is to use something like:

return BinaryOperator::CreateShl(Op0, ShiftAmt);

...then InstCombine handles the replacement in the calling function.

2014–2015

Similar to above:

return BinaryOperator::CreateLShr(Op1, BitWidth - ShiftAmt);
nikic updated this revision to Diff 175070.Nov 22 2018, 1:39 PM

Convert check to assert, don't use builder.

spatel accepted this revision.Nov 23 2018, 12:11 PM

LGTM - can you add at least one vector test when before committing, so we know that the constant creation is producing the correct types.

lib/Transforms/InstCombine/InstCombineCalls.cpp
2008–2009

Here and below - did that clang-format like that? Doesn't look like the normal indent.

This revision is now accepted and ready to land.Nov 23 2018, 12:11 PM
This revision was automatically updated to reflect the committed changes.
nikic marked 3 inline comments as done.
nikic marked 2 inline comments as done.Nov 23 2018, 2:53 PM

I've added some vector tests before committing. Thanks for the reviews!

lib/Transforms/InstCombine/InstCombineCalls.cpp
2008–2009

You're right, I fixed this before commit. TIL about git clang-format.