[PATCH] D123453: [InstCombine] Fold mul nuw+lshr to a single multiplication when the latter is a factor

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp:1172
+      // TODO: Generalize to allow more than just half-width shifts?
+      if (ShAmtC * 2 == BitWidth && (*MulC - 1).isPowerOf2() &&
+          MulC->logBase2() == ShAmtC)
----------------
craig.topper wrote:
> Not related to this patch, but this existing transform is incorrect if MulC happens to be 2 and BitWidth is 2 and ShAmtC is 1. MulC-1 would be 1 which is a power 2 but it doesn't have 2 bits set like the transform expects. Hopefully we would always canonicalize the Mul to a `shl` first so this transform won't fire for that case, but I'm not sure. @spatel @lebedev.ri 
Good catch. I'm not sure how to write a test for it either, but I added a "BitWidth > 2" clause:
1206a18d417a


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

https://reviews.llvm.org/D123453