[PATCH] D124369: [InstCombine] C0 >>{ashr, exact} (X - C1) --> (C0 << C1) >>{ashr} X

Tue Apr 26 13:51:59 PDT 2022

spatel added a comment.

In D124369#3472995 <https://reviews.llvm.org/D124369#3472995>, @nico-abram wrote:

> I collapsed all three into `InstCombinerImpl::commonShiftTransforms`, right below the pre-existing pre-shift combine.

Nice!

> Had to add setHasNoSignedWrap(false); for the Shl case. Otherwise the @shl_nsw_add_negative test case was generating a `shl nsw i32 1` instead of `shl i32 1`. Not entirely sure why, possibly because I'm genering the Op with `BinaryOperator::Create(I.getOpcode()`?

Hmm - I'm not sure what that is about. Let's pre-commit the tests with baseline CHECK lines to confirm that we are testing the expected patterns. First, we need to add at least 2 more tests though that do **not** have "exact" - we want to verify that this transform does not fire without "exact".
If you do not have commit access yet, let me know, and I'll push the tests for you.

================
Comment at: llvm/test/Transforms/InstCombine/shift-add.ll:202
+  %a = add i32 %x, -1
+  %r = ashr exact i32 1073741824, %a
+  ret i32 %r
----------------
If the constant is not negative, the ashr will get converted to lshr before we hit the new transform, so this test is not providing the coverage that we want. How about something like this:
  %a = add i8 %x, -4
  %r = ashr exact i8 -112, %a ; 0b1001_0000

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

https://reviews.llvm.org/D124369