[InstCombine] Propagate exact flags in shift-combine transforms

[AtariDreams]

There were a couple of places where we could propagate exact flags but did not.
This patch addresses some of those places.

Alive 2 Proofs:
https://alive2.llvm.org/ce/z/vmoZrX
https://alive2.llvm.org/ce/z/9zxKKA
https://alive2.llvm.org/ce/z/HJebVu
https://alive2.llvm.org/ce/z/96ez9n

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: AtariDreams (AtariDreams)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/88340.diff

1 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp (+12-5)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
index 95aa2119e2d88b..4c3f8b474745fd 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
@@ -1332,7 +1332,7 @@ Instruction *InstCombinerImpl::visitLShr(BinaryOperator &I) {
     if (match(Op0,
               m_OneUse(m_c_Add(m_OneUse(m_Shl(m_Value(X), m_Specific(Op1))),
                                m_Value(Y))))) {
-      Value *NewLshr = Builder.CreateLShr(Y, Op1);
+      Value *NewLshr = Builder.CreateLShr(Y, Op1, "", I.isExact());
       Value *NewAdd = Builder.CreateAdd(NewLshr, X);
       unsigned Op1Val = C->getLimitedValue(BitWidth);
       APInt Bits = APInt::getLowBitsSet(BitWidth, BitWidth - Op1Val);
@@ -1395,11 +1395,17 @@ Instruction *InstCombinerImpl::visitLShr(BinaryOperator &I) {
     }
 
     // (X >>u C1) >>u C --> X >>u (C1 + C)
-    if (match(Op0, m_LShr(m_Value(X), m_APInt(C1)))) {
+    Instruction *Inst;
+    if (match(Op0, m_Instruction(Inst)) &&
+        match(Inst, m_LShr(m_Value(X), m_APInt(C1)))) {
       // Oversized shifts are simplified to zero in InstSimplify.
       unsigned AmtSum = ShAmtC + C1->getZExtValue();
-      if (AmtSum < BitWidth)
-        return BinaryOperator::CreateLShr(X, ConstantInt::get(Ty, AmtSum));
+      if (AmtSum < BitWidth) {
+        auto *NewLShr =
+            BinaryOperator::CreateLShr(X, ConstantInt::get(Ty, AmtSum));
+        NewLShr->setIsExact(I.isExact() && Inst->isExact());
+        return NewLShr;
+      }
     }
 
     Instruction *TruncSrc;
@@ -1415,7 +1421,8 @@ Instruction *InstCombinerImpl::visitLShr(BinaryOperator &I) {
       // mask instruction is eliminated (and so the use check is relaxed).
       if (AmtSum < SrcWidth &&
           (TruncSrc->hasOneUse() || C1->uge(SrcWidth - BitWidth))) {
-        Value *SumShift = Builder.CreateLShr(X, AmtSum, "sum.shift");
+        Value *SumShift = Builder.CreateLShr(
+            X, AmtSum, "sum.shift", TruncSrc->isExact() && I.isExact());
         Value *Trunc = Builder.CreateTrunc(SumShift, Ty, I.getName());
 
         // If the first shift does not cover the number of bits truncated, then

[XChy]

Could you please please add alive2 proof for it?

[nikic]

@AtariDreams Your contribution behavior continues to be unacceptable, to the point that I am this close to requesting a ban from the LLVM organization for you.

Please, do not submit pull requests until you both have adequate test coverage and proofs. See https://llvm.org/docs/InstCombineContributorGuide.html for our contribution guidelines. If you want to submit a PR without these things, submit it as a draft pull request, and only mark it as ready for review when it is actually ready.

[AtariDreams]

Could you please please add alive2 proof for it?

https://alive2.llvm.org/ce/z/kspgi5

@AtariDreams Your contribution behavior continues to be unacceptable, to the point that I am this close to requesting a ban from the LLVM organization for you.

Please, do not submit pull requests until you both have adequate test coverage and proofs. See https://llvm.org/docs/InstCombineContributorGuide.html for our contribution guidelines. If you want to submit a PR without these things, submit it as a draft pull request, and only mark it as ready for review when it is actually ready.

I apologize. I will ensure moving forward to ensure all PRs have proofs and tests.

[goldsteinn]

NB: you duplicate your message in commit 2

[goldsteinn]

LGTM. Please wait for one more approval before pushing.

[dtcxzyw]

LGTM.

[dtcxzyw]

Failed Tests (1):
LLVM :: Transforms/InstCombine/shift.ll

Please update the test.

[AtariDreams]

Failed Tests (1):
LLVM :: Transforms/InstCombine/shift.ll

Please update the test.

Done!

[nikic]

This test (and the next two) are useless in that the pattern only works if both shift amounts are constants. This does not actually trigger the fold.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[AtariDreams]

@nikic I found the cause. turns out this particular fold is entirely redundant

[AtariDreams]

I will just remove the test for this particular case because for that particular transform there needs to be some more things done.

[dtcxzyw]

Failed Tests (2):
LLVM :: Transforms/InstCombine/lshr.ll
LLVM :: Transforms/InstCombine/shift.ll