[PATCH] D76500: GlobalISel: Lower funnel shifts

[Matt Arsenault via Phabricator via llvm-commits]

arsenm marked 2 inline comments as done.
arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:5133-5134
+  } else {
+    // fshl X, Y, Z -> fshr (srl X, 1), (fshr X, Y, 1), ~Z
+    // fshr X, Y, Z -> fshl (fshl X, Y, 1), (shl Y, 1), ~Z
+    auto One = MIRBuilder.buildConstant(ShTy, 1);
----------------
foad wrote:
> Using `~Z` here only works if BitWidth is a power of two. Otherwise you need something like `BitWidth - 1 - (Z % BitWidth)`.
Isn't that what the isNonZeroModBitWidthOrUndef check is for? I just directly copied this from the DAG, so this should match the behavior


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:4975
+        const ConstantInt *C = getConstantIntVRegVal(R, MRI);
+        return !C || C->getValue().urem(BW) != 0;
+      },
----------------
foad wrote:
> foad wrote:
> > Should be `C && C->getValue().urem(BW) != 0`.
> I still think this check for `!C` is either wrong or redundant, depending on what the behaviour of matchUnaryPredicate is supposed to be.
This is is correct because in the variable case, you have to assume it's a non-zero mod


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

https://reviews.llvm.org/D76500