[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 a comment.

1. Please pre-commit the tests with baseline CHECKs. Add one more positive test with vector type, so we know that works correctly -- at least for splat (uniform) constants. A negative test with 'nsw' would also be good. What happens if we have both 'nsw' and 'nuw'?

2. There's an existing fold for: `(X * C2) << C1 --> X * (C2 << C1)`

...and it does not check for one-use. For consistency, we probably don't want the one-use restriction here either. If there's already a multiply in the pattern before this transform, another one is probably fine? The backend could theoretically decompose it back to shift (but I don't think we have that transform currently).



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp:1178-1179
+        APInt NewMulC = MulC->lshr(ShAmtC);
+        // if c is divisible by (1 << ShAmtC):
+        // lshr (mul nuw x, c), ShAmtC -> mul nuw x, (c >> ShAmtC)
+        if (MulC->eq(NewMulC.shl(ShAmtC)))
----------------
Replace 'c' with 'MulC'  in this comment to make it clearer.


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

https://reviews.llvm.org/D123453