[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:4196-4197
+  auto CCTy = Ty.changeElementSize(1);
+  // If (Z % BW == 0), then the opposite direction shift is shift-by-bitwidth,
+  // and that is undefined. We must compare and select to avoid UB.
+
----------------
arsenm wrote:
> arsenm wrote:
> > foad wrote:
> > > It would be slicker to implement `(X << (BW - (Z % BW)))` as `(X << 1 << ((BW - 1) - (Z % BW)))`.
> > I copied this from the DAG version. Maybe that should be updated first? (or maybe it happens to turn into this anyway from other combines)
> I don't actually see how this is better? It doesn't eliminate the urem, and adds an extra instruction?
Adding one instruction here (the `<< 1`) avoids the need for the compare and select at the end to handle the shift-by-zero case.


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

https://reviews.llvm.org/D76500