[PATCH] D88834: [InstCombine] matchRotate - add support for matching general funnel shifts with constant shift amounts (PR46896)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2075
 /// Transform UB-safe variants of bitwise rotate to the funnel shift intrinsic.
 static Instruction *matchRotate(Instruction &Or) {
   // TODO: Can we reduce the code duplication between this and the related
----------------
This is expanded now - change name/code comment to matchFunnelShift()?


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2080-2081
 
   // First, find an or'd pair of opposite shifts with the same shifted operand:
   // or (lshr ShVal, ShAmt0), (shl ShVal, ShAmt1)
   BinaryOperator *Or0, *Or1;
----------------
Update comment to acknowledge different ShVals.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2112
 
+    // Only match rotation patterns with non-constant shift amounts.
+    if (ShVal0 != ShVal1)
----------------
The comment doesn't read clearly to me. Is the restriction temporary/artificial, or do we not want to match general funnels here?

  // For variable shift amounts, we only match rotates, not the more general funnel shift
  // because... 


================
Comment at: llvm/test/Transforms/InstCombine/funnel.ll:33
 
 ; TODO: Vector types are allowed.
 
----------------
Remove stale comment.


================
Comment at: llvm/test/Transforms/InstCombine/funnel.ll:68
 
 ; TODO: Non-power-of-2 vector types are allowed.
 
----------------
Remove stale comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88834/new/

https://reviews.llvm.org/D88834