[PATCH] D54778: [InstCombine] Simplify funnel shift with one zero/undef operands to simple shift

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:2000
+      // Zero shift is already handled in simplification.
+      if (ShiftAmt != 0) {
+        Value *Op0 = II->getArgOperand(0), *Op1 = II->getArgOperand(1);
----------------
This can be made an assert. We are guaranteed to call InstSimplify (line 1844) for this instruction before we reach this point in InstCombine.


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:2009
+        if (match(Op1, m_Zero()) || match(Op1, m_Undef()))
+          return replaceInstUsesWith(*II, Builder.CreateShl(Op0, ShiftAmt));
+
----------------
The convention is to use something like:
  return BinaryOperator::CreateShl(Op0, ShiftAmt);
...then InstCombine handles the replacement in the calling function.


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:2014-2015
+        if (match(Op0, m_Zero()) || match(Op0, m_Undef()))
+          return replaceInstUsesWith(*II,
+            Builder.CreateLShr(Op1, BitWidth - ShiftAmt));
+      }
----------------
Similar to above:
  return BinaryOperator::CreateLShr(Op1, BitWidth - ShiftAmt);


https://reviews.llvm.org/D54778