[PATCH] D58009: [DAGCombine] Simplify funnel shifts with undef/zero args to bitshifts

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7133
+      return true;
+    if (ConstantSDNode *Cst = isConstOrConstSplat(V))
+      return Cst->getAPIntValue() == 0;
----------------
Maybe pass AllowUndefs=true, as we want to deal with both undef/zero anyway?


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7155
+    // fold fshl(N0, undef_or_zero, C) -> lshr(N0, C)
+    // fold fshr(N0, undef_or_zero, C) -> lshr(N0, BW-C)
+    if (IsUndefOrZero(N0))
----------------
Should be s/lshr/shl in the two latter comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7171
+  if (isPowerOf2_32(BitWidth)) {
+    APInt UpperBits = APInt::getHighBitsSet(BitWidth, Log2_32(BitWidth));
+    if (IsUndefOrZero(N0) && !IsFSHL && DAG.MaskedValueIsZero(N2, UpperBits))
----------------
Shouldn't this be either setting the `BitWidth - Log2_32(BitWidth)` high bits, or maybe use `getBitsSetFrom()` instead? I think right now this is checking too few bits.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58009