[InstCombine] Add optimization to combine adds through zext nneg, and add testcase

[Aethezz]

This adds the optimizations to combine adds through zext nneg when adds cancel.

Updated proof: https://alive2.llvm.org/ce/z/T2BmZj
This fixes #157111

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: None (Aethezz)

Changes

This adds the optimizations to combine adds through zext nneg when adds cancel.

Proof: https://alive2.llvm.org/ce/z/BhksTu

This fixes #157111

---

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

2 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp (+17)
• (modified) llvm/test/Transforms/InstCombine/zext.ll (+11)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index d934638c15e75..1896bc3d577f3 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -1910,6 +1910,23 @@ Instruction *InstCombinerImpl::visitAdd(BinaryOperator &I) {
   if (Instruction *Res = foldBinOpOfSelectAndCastOfSelectCondition(I))
     return Res;
 
+  // Combine adds through zext nneg:
+  // (zext nneg (X + C1)) + C2 --> zext X if C1 + C2 == 0
+  {
+    Value *X;
+    const APInt *C1, *C2;
+    if (match(&I, m_c_Add(m_NNegZExt(m_Add(m_Value(X), m_APInt(C1))), 
+                          m_APInt(C2)))) {
+      // Check if the constants cancel out (C1 + C2 == 0)
+      APInt Sum = C1->sext(C2->getBitWidth()) + *C2;
+      if (Sum.isZero()) {
+        // The add inside the zext and the outer add cancel out
+        Value *NewZExt = Builder.CreateZExt(X, I.getType());
+        return replaceInstUsesWith(I, NewZExt);
+      }
+    }
+  }
+
   // Re-enqueue users of the induction variable of add recurrence if we infer
   // new nuw/nsw flags.
   if (Changed) {
diff --git a/llvm/test/Transforms/InstCombine/zext.ll b/llvm/test/Transforms/InstCombine/zext.ll
index e4d18e9395219..15aa2aaec430a 100644
--- a/llvm/test/Transforms/InstCombine/zext.ll
+++ b/llvm/test/Transforms/InstCombine/zext.ll
@@ -976,3 +976,14 @@ entry:
   %res = zext nneg i2 %x to i32
   ret i32 %res
 }
+
+define i32 @zext_nneg_add_cancel(i8 %arg) {
+; CHECK-LABEL: @zext_nneg_add_cancel(
+; CHECK-NEXT:    [[ADD2:%.*]] = zext i8 [[ARG:%.*]] to i32
+; CHECK-NEXT:    ret i32 [[ADD2]]
+;
+  %add = add i8 %arg, -2
+  %zext = zext nneg i8 %add to i32
+  %add2 = add i32 %zext, 2
+  ret i32 %add2
+}
\ No newline at end of file

[github-actions]

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

[artagnon]

This is incorrect? https://alive2.llvm.org/ce/z/Rn5mX0

[nikic]

This is missing the precondition that the inner constant must be negative.

Can you please add a generalized proof? See https://llvm.org/docs/InstCombineContributorGuide.html#use-generic-values-in-proofs.

Also we don't need the constants to exactly cancel, something like this is also profitable: https://alive2.llvm.org/ce/z/pT89U9 (Though this one would need a one-use check.)

[nikic]

Updated proof: https://alive2.llvm.org/ce/z/T2BmZj

This proof is not correct? src and tgt are the same here.

I believe this is a translation of your preconditions: https://alive2.llvm.org/ce/z/nrK53A It shows that they are not correct. Even if the addition result is a signed i8, the fact that the original constant is larger is a problem.

This variant works: https://alive2.llvm.org/ce/z/5geVhA

[Aethezz]

Updated proof: https://alive2.llvm.org/ce/z/T2BmZj

This proof is not correct? src and tgt are the same here.

I believe this is a translation of your preconditions: https://alive2.llvm.org/ce/z/nrK53A It shows that they are not correct. Even if the addition result is a signed i8, the fact that the original constant is larger is a problem.

This variant works: https://alive2.llvm.org/ce/z/5geVhA

Oh thanks, I didn't export the updated link after editing the proof.

[dtcxzyw]

@zyw-bot mfuzz

[nikic]

I believe this is a translation of your preconditions: https://alive2.llvm.org/ce/z/nrK53A It shows that they are not correct. Even if the addition result is a signed i8, the fact that the original constant is larger is a problem.

This variant works: https://alive2.llvm.org/ce/z/5geVhA

This issue is still open. To give a specific example, you are currently miscompiling this test case: https://alive2.llvm.org/ce/z/66TJWX

[nikic]

This increases the number of instructions. The transform should not be performed here.

[Aethezz]

Also we don't need the constants to exactly cancel, something like this is also profitable: https://alive2.llvm.org/ce/z/pT89U9 (Though this one would need a one-use check.)

In this case, the multiuse shows that number of instructions increases, so are we supporting the constants not exactly cancelling?

[nikic]

We should do this fold, but guarded by a hasOneUse check.

[nikic]

This issue is still open. To give a specific example, you are currently miscompiling this test case: https://alive2.llvm.org/ce/z/66TJWX

Can you please also add a test for this?

[nikic]

If you use early exits like this, the transform should be moved into a separate function. Otherwise there is a high risk of accidentally skipping transforms that get added after this point.

[nikic]

Per the alive proof in https://alive2.llvm.org/ce/z/pY63fh, you need to perform this check on C2, not on Sum. And if you do, then you don't need the additional check below.