[PATCH] D76500: GlobalISel: Lower funnel shifts

[Jay Foad via Phabricator via llvm-commits]

foad 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:
> arsenm wrote:
> > 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
> DAG only tries to use a shift in the other direction if `isPowerOf2_32(BW)` (TargetLowering.cpp line 6456).
I realise I approved the patch but I still think this needs fixing. The ~Z thing only works if BW is a power of two.


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

https://reviews.llvm.org/D76500