[InstCombine] Generalize (A + 1) + ~B fold to any constant

[pfusik]

Example:

int foo(int a, int b) { return a - 1 + ~b; }

Before, on AArch64:

mvn w8, w1
add w8, w0, w8
sub w0, w8, #1

After (matches gcc):

sub w0, w0, w1
sub w0, w0, #2

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

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Piotr Fusik (pfusik)

Changes

Example:

int foo(int a, int b) { return a - 1 + ~b; }

Before, on AArch64:

mvn w8, w1
add w8, w0, w8
sub w0, w8, #<!-- -->1

After (matches gcc):

sub w0, w0, w1
sub w0, w0, #<!-- -->2

---

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

2 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp (+14-7)
• (modified) llvm/test/Transforms/InstCombine/add.ll (+61)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index c781c6978b275..ae9b66dd5a6f8 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -1604,13 +1604,20 @@ Instruction *InstCombinerImpl::visitAdd(BinaryOperator &I) {
   if (Value *V = checkForNegativeOperand(I, Builder))
     return replaceInstUsesWith(I, V);
 
-  // (A + 1) + ~B --> A - B
-  // ~B + (A + 1) --> A - B
-  // (~B + A) + 1 --> A - B
-  // (A + ~B) + 1 --> A - B
-  if (match(&I, m_c_BinOp(m_Add(m_Value(A), m_One()), m_Not(m_Value(B)))) ||
-      match(&I, m_BinOp(m_c_Add(m_Not(m_Value(B)), m_Value(A)), m_One())))
-    return BinaryOperator::CreateSub(A, B);
+  {
+    // (A + C) + ~B --> A - B + (C-1)
+    // ~B + (A + C) --> A - B + (C-1)
+    // (~B + A) + C --> A - B + (C-1)
+    // (A + ~B) + C --> A - B + (C-1)
+    const APInt *C;
+    if (match(&I,
+              m_c_BinOp(m_Add(m_Value(A), m_APInt(C)), m_Not(m_Value(B)))) ||
+        match(&I,
+              m_BinOp(m_c_Add(m_Not(m_Value(B)), m_Value(A)), m_APInt(C)))) {
+      Value *Sub = Builder.CreateSub(A, B);
+      return BinaryOperator::CreateAdd(Sub, ConstantInt::get(Ty, *C - 1));
+    }
+  }
 
   // (A + RHS) + RHS --> A + (RHS << 1)
   if (match(LHS, m_OneUse(m_c_Add(m_Value(A), m_Specific(RHS)))))
diff --git a/llvm/test/Transforms/InstCombine/add.ll b/llvm/test/Transforms/InstCombine/add.ll
index aa68dfb540064..1006831fc1e0f 100644
--- a/llvm/test/Transforms/InstCombine/add.ll
+++ b/llvm/test/Transforms/InstCombine/add.ll
@@ -1194,6 +1194,67 @@ define i32 @add_to_sub2(i32 %A, i32 %M) {
   ret i32 %E
 }
 
+define i32 @add_not_add_m1(i32 %a, i32 %b) {
+; CHECK-LABEL: @add_not_add_m1(
+; CHECK-NEXT:    [[TMP1:%.*]] = sub i32 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[R:%.*]] = add i32 [[TMP1]], -2
+; CHECK-NEXT:    ret i32 [[R]]
+;
+  %not = xor i32 %b, -1
+  %add = add i32 %a, -1
+  %r = add i32 %add, %not
+  ret i32 %r
+}
+
+define <2 x i32> @add_not_add_m1_vec(<2 x i32> %a, <2 x i32> %b) {
+; CHECK-LABEL: @add_not_add_m1_vec(
+; CHECK-NEXT:    [[TMP1:%.*]] = sub <2 x i32> [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[R:%.*]] = add <2 x i32> [[TMP1]], splat (i32 -2)
+; CHECK-NEXT:    ret <2 x i32> [[R]]
+;
+  %not = xor <2 x i32> %b, <i32 -1, i32 -1>
+  %add = add <2 x i32> %a, <i32 -1, i32 -1>
+  %r = add <2 x i32> %add, %not
+  ret <2 x i32> %r
+}
+
+define i32 @add_not_add_m1_commuted(i32 %a, i32 %b) {
+; CHECK-LABEL: @add_not_add_m1_commuted(
+; CHECK-NEXT:    [[TMP1:%.*]] = sub i32 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[R:%.*]] = add i32 [[TMP1]], -2
+; CHECK-NEXT:    ret i32 [[R]]
+;
+  %not = xor i32 %b, -1
+  %add = add i32 %a, -1
+  %r = add i32 %not, %add
+  ret i32 %r
+}
+
+define i32 @add_not_add_m1_assoc(i32 %a, i32 %b) {
+; CHECK-LABEL: @add_not_add_m1_assoc(
+; CHECK-NEXT:    [[TMP1:%.*]] = sub i32 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[R:%.*]] = add i32 [[TMP1]], -2
+; CHECK-NEXT:    ret i32 [[R]]
+;
+  %not = xor i32 %b, -1
+  %add = add i32 %not, %a
+  %r = add i32 %add, -1
+  ret i32 %r
+}
+
+define i32 @add_not_add_intmin(i32 %a, i32 %b) {
+; CHECK-LABEL: @add_not_add_intmin(
+; CHECK-NEXT:    [[NOT:%.*]] = xor i32 [[B:%.*]], -1
+; CHECK-NEXT:    [[ADD:%.*]] = xor i32 [[A:%.*]], -2147483648
+; CHECK-NEXT:    [[R:%.*]] = add i32 [[ADD]], [[NOT]]
+; CHECK-NEXT:    ret i32 [[R]]
+;
+  %not = xor i32 %b, -1
+  %add = add i32 %a, -2147483648
+  %r = add i32 %add, %not
+  ret i32 %r
+}
+
 ; (X | C1) + C2 --> (X | C1) ^ C1 iff (C1 == -C2)
 define i32 @test44(i32 %A) {
 ; CHECK-LABEL: @test44(

[github-actions]

🐧 Linux x64 Test Results

• 193400 tests passed
• 5022 tests skipped

✅ The build succeeded and all tests passed.

[github-actions]

🪟 Windows x64 Test Results

• 133326 tests passed
• 3080 tests skipped

✅ The build succeeded and all tests passed.

[artagnon]

Would be good to include an Alive2 proof?

[artagnon]

LGTM, with suggestion for improving clarity, thanks! Kindly wait 24h before landing, to give Yingwei a chance to take a look.

[rj-jesus]

Thanks for changes, I've left a few more comments!

[rj-jesus]

LGTM, thanks!

There may be cases where C-1 isn't actually free, please keep an eye out for potential regressions.

[pfusik]

There may be cases where C-1 isn't actually free

Do you mean add C-1 more costly than add C for certain values of C on certain targets?

[rj-jesus]

There may be cases where C-1 isn't actually free

Do you mean add C-1 more costly than add C for certain values of C on certain targets?

Yes, you may not be able to encode or materialise C-1 as easily as C (https://godbolt.org/z/f8bP64xzn). But the opposite is also true, so I don't think this should be much of a concern.

It might also happen that C still needs to be materialised for something else, but I don't think that can be avoided easily.