[InstCombine] Fold `(add X, (sext/zext (icmp eq X, C)))` #93840

goldsteinn · 2024-05-30T16:01:30Z

[InstCombine] Add tests for folding (add X, (sext/zext (icmp eq X, C))); NFC
[InstCombine] Fold (add X, (sext/zext (icmp eq X, C)))

We can convert this to a select based on the (icmp eq X, C), then
constant fold the addition the true arm begin (add C, (sext/zext 1))
and the false arm being (add X, 0) e.g

- `(select (icmp eq X, C), (add C, (sext/zext 1)), (add X, 0))`.

This is essentially a specialization of the only case that sees to
actually show up from #89020

Proofs: https://alive2.llvm.org/ce/z/dMbHbM

llvmbot · 2024-05-30T16:02:02Z

@llvm/pr-subscribers-llvm-transforms

Author: None (goldsteinn)

Changes

[InstCombine] Add tests for folding (add X, (sext/zext (icmp eq X, C))); NFC
[InstCombine] Fold (add X, (sext/zext (icmp eq X, C)))

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

3 Files Affected:

(modified) llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp (+20)
(modified) llvm/test/Transforms/InstCombine/apint-shift.ll (+5-12)
(added) llvm/test/Transforms/InstCombine/fold-ext-eq-c-with-op.ll (+56)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index bff09f5676680..b861c8e9cc19f 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -1694,6 +1694,26 @@ Instruction *InstCombinerImpl::visitAdd(BinaryOperator &I) {
     return BinaryOperator::CreateOr(LHS, Zext);
   }
 
+  {
+    Value *Cond, *Ext;
+    Constant *C;
+    // (add X, (sext/zext (icmp eq X, C)))
+    //    -> (select (icmp eq X, C), (add C, (sext/zext 1)), X)
+    auto CondMatcher = m_CombineAnd(
+        m_Value(Cond), m_ICmp(Pred, m_Deferred(A), m_ImmConstant(C)));
+
+    if (match(&I,
+              m_c_Add(m_Value(A),
+                      m_CombineAnd(m_Value(Ext), m_ZExtOrSExt(CondMatcher)))) &&
+        Pred == ICmpInst::ICMP_EQ && Ext->hasOneUse()) {
+      Constant *ExtC = match(Ext, m_ZExt(m_Value()))
+                           ? ConstantInt::get(C->getType(), 1)
+                           : ConstantInt::getAllOnesValue(C->getType());
+      return replaceInstUsesWith(
+          I, Builder.CreateSelect(Cond, Builder.CreateAdd(C, ExtC), A));
+    }
+  }
+
   if (Instruction *Ashr = foldAddToAshr(I))
     return Ashr;
 
diff --git a/llvm/test/Transforms/InstCombine/apint-shift.ll b/llvm/test/Transforms/InstCombine/apint-shift.ll
index 05c3db70ce1ca..ecf9c4e9c4e69 100644
--- a/llvm/test/Transforms/InstCombine/apint-shift.ll
+++ b/llvm/test/Transforms/InstCombine/apint-shift.ll
@@ -240,8 +240,8 @@ define i23 @test11(i23 %x) {
 
 define i47 @test12(i47 %X) {
 ; CHECK-LABEL: @test12(
-; CHECK-NEXT:    [[SH2:%.*]] = and i47 [[X:%.*]], -256
-; CHECK-NEXT:    ret i47 [[SH2]]
+; CHECK-NEXT:    [[SH1:%.*]] = and i47 [[X:%.*]], -256
+; CHECK-NEXT:    ret i47 [[SH1]]
 ;
   %sh1 = ashr i47 %X, 8
   %sh2 = shl i47 %sh1, 8
@@ -250,8 +250,8 @@ define i47 @test12(i47 %X) {
 
 define <2 x i47> @test12_splat_vec(<2 x i47> %X) {
 ; CHECK-LABEL: @test12_splat_vec(
-; CHECK-NEXT:    [[SH2:%.*]] = and <2 x i47> [[X:%.*]], <i47 -256, i47 -256>
-; CHECK-NEXT:    ret <2 x i47> [[SH2]]
+; CHECK-NEXT:    [[SH1:%.*]] = and <2 x i47> [[X:%.*]], <i47 -256, i47 -256>
+; CHECK-NEXT:    ret <2 x i47> [[SH1]]
 ;
   %sh1 = ashr <2 x i47> %X, <i47 8, i47 8>
   %sh2 = shl <2 x i47> %sh1, <i47 8, i47 8>
@@ -564,14 +564,7 @@ define i40 @test26(i40 %A) {
 ; https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9880
 define i177 @ossfuzz_9880(i177 %X) {
 ; CHECK-LABEL: @ossfuzz_9880(
-; CHECK-NEXT:    [[A:%.*]] = alloca i177, align 8
-; CHECK-NEXT:    [[L1:%.*]] = load i177, ptr [[A]], align 4
-; CHECK-NEXT:    [[TMP1:%.*]] = icmp eq i177 [[L1]], -1
-; CHECK-NEXT:    [[B5_NEG:%.*]] = sext i1 [[TMP1]] to i177
-; CHECK-NEXT:    [[B14:%.*]] = add i177 [[L1]], [[B5_NEG]]
-; CHECK-NEXT:    [[TMP2:%.*]] = icmp eq i177 [[B14]], -1
-; CHECK-NEXT:    [[B1:%.*]] = zext i1 [[TMP2]] to i177
-; CHECK-NEXT:    ret i177 [[B1]]
+; CHECK-NEXT:    ret i177 0
 ;
   %A = alloca i177
   %L1 = load i177, ptr %A
diff --git a/llvm/test/Transforms/InstCombine/fold-ext-eq-c-with-op.ll b/llvm/test/Transforms/InstCombine/fold-ext-eq-c-with-op.ll
new file mode 100644
index 0000000000000..960efc524910d
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/fold-ext-eq-c-with-op.ll
@@ -0,0 +1,56 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+
+declare void @use.i8(i8)
+define i8 @fold_add_zext_eq_0(i8 %x) {
+; CHECK-LABEL: @fold_add_zext_eq_0(
+; CHECK-NEXT:    [[R:%.*]] = call i8 @llvm.umax.i8(i8 [[X:%.*]], i8 1)
+; CHECK-NEXT:    ret i8 [[R]]
+;
+  %x_eq = icmp eq i8 %x, 0
+  %x_eq_ext = zext i1 %x_eq to i8
+  %r = add i8 %x, %x_eq_ext
+  ret i8 %r
+}
+
+define i8 @fold_add_sext_eq_4(i8 %x) {
+; CHECK-LABEL: @fold_add_sext_eq_4(
+; CHECK-NEXT:    [[X_EQ:%.*]] = icmp eq i8 [[X:%.*]], 4
+; CHECK-NEXT:    [[R:%.*]] = select i1 [[X_EQ]], i8 3, i8 [[X]]
+; CHECK-NEXT:    ret i8 [[R]]
+;
+  %x_eq = icmp eq i8 %x, 4
+  %x_eq_ext = sext i1 %x_eq to i8
+  %r = add i8 %x, %x_eq_ext
+  ret i8 %r
+}
+
+define i8 @fold_add_zext_eq_0_fail_multiuse_exp(i8 %x) {
+; CHECK-LABEL: @fold_add_zext_eq_0_fail_multiuse_exp(
+; CHECK-NEXT:    [[X_EQ:%.*]] = icmp eq i8 [[X:%.*]], 0
+; CHECK-NEXT:    [[X_EQ_EXT:%.*]] = zext i1 [[X_EQ]] to i8
+; CHECK-NEXT:    [[R:%.*]] = add i8 [[X_EQ_EXT]], [[X]]
+; CHECK-NEXT:    call void @use.i8(i8 [[X_EQ_EXT]])
+; CHECK-NEXT:    ret i8 [[R]]
+;
+  %x_eq = icmp eq i8 %x, 0
+  %x_eq_ext = zext i1 %x_eq to i8
+  %r = add i8 %x, %x_eq_ext
+  call void @use.i8(i8 %x_eq_ext)
+  ret i8 %r
+}
+
+define i8 @fold_add_sext_eq_4_fail_wrong_cond(i8 %x, i8 %y) {
+; CHECK-LABEL: @fold_add_sext_eq_4_fail_wrong_cond(
+; CHECK-NEXT:    [[X_EQ:%.*]] = icmp eq i8 [[Y:%.*]], 4
+; CHECK-NEXT:    [[X_EQ_EXT:%.*]] = sext i1 [[X_EQ]] to i8
+; CHECK-NEXT:    [[R:%.*]] = add i8 [[X_EQ_EXT]], [[X:%.*]]
+; CHECK-NEXT:    call void @use.i8(i8 [[X_EQ_EXT]])
+; CHECK-NEXT:    ret i8 [[R]]
+;
+  %x_eq = icmp eq i8 %y, 4
+  %x_eq_ext = sext i1 %x_eq to i8
+  %r = add i8 %x, %x_eq_ext
+  call void @use.i8(i8 %x_eq_ext)
+  ret i8 %r
+}

goldsteinn · 2024-05-30T16:02:26Z

@dtcxzyw Can you run this?

PR Link: llvm/llvm-project#93840

dtcxzyw · 2024-05-30T17:33:04Z

llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp

+                           ? ConstantInt::get(C->getType(), 1)
+                           : ConstantInt::getAllOnesValue(C->getType());
+      return replaceInstUsesWith(
+          I, Builder.CreateSelect(Cond, Builder.CreateAdd(C, ExtC), A));


Use InstCombiner::AddOne/SubOne?

llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp

dtcxzyw · 2024-05-30T17:46:56Z

The IR diff LGTM. And there is no extra space for us to generalize this pattern (e.g., add->sub, eq -> ne) :)

goldsteinn · 2024-05-30T18:12:33Z

The IR diff LGTM. And there is no extra space for us to generalize this pattern (e.g., add->sub, eq -> ne) :)

What do you mean there is no extra space for us the generalize?

dtcxzyw · 2024-05-30T18:19:24Z

The IR diff LGTM. And there is no extra space for us to generalize this pattern (e.g., add->sub, eq -> ne) :)

What do you mean there is no extra space for us the generalize?

Both (sub X, (sext/zext (icmp eq X, C))) and (add X, (sext/zext (icmp ne X, C))) are not canonical forms.

goldsteinn · 2024-05-30T19:18:49Z

The IR diff LGTM.

Yeah you guys where right, exact same diff.

nikic · 2024-06-01T10:04:31Z

llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp

+              m_c_Add(m_Value(A),
+                      m_CombineAnd(m_Value(Ext), m_ZExtOrSExt(CondMatcher)))) &&
+        Pred == ICmpInst::ICMP_EQ && Ext->hasOneUse()) {
+      Constant *ExtC = match(Ext, m_ZExt(m_Value()))


Suggested change

Constant *ExtC = match(Ext, m_ZExt(m_Value()))

Constant *ExtC = isa<ZExtInst>(Ext)

nikic · 2024-06-01T10:05:17Z

llvm/test/Transforms/InstCombine/fold-ext-eq-c-with-op.ll

+  %x_eq_ext = sext i1 %x_eq to i8
+  %r = add i8 %x, %x_eq_ext
+  ret i8 %r
+}


Commuted tests?

nikic · 2024-06-01T10:06:57Z

llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp

+    // (add X, (sext/zext (icmp eq X, C)))
+    //    -> (select (icmp eq X, C), (add C, (sext/zext 1)), X)
+    auto CondMatcher = m_CombineAnd(
+        m_Value(Cond), m_ICmp(Pred, m_Deferred(A), m_ImmConstant(C)));


I think it might be a bit more elegant if you match m_Value(Cond) first and then do a second match call on Cond. Especially with this in a separate variable (so that m_Deferred occurs before the corresponding m_Value) I found this confusing.

That will be flakey if X itself is a sext/zext of another value though no?

…)))`; NFC

We can convert this to a select based on the `(icmp eq X, C)`, then constant fold the addition the true arm begin `(add C, (sext/zext 1))` and the false arm being `(add X, 0)` e.g - `(select (icmp eq X, C), (add C, (sext/zext 1)), (add X, 0))`. This is essentially a specialization of the only case that sees to actually show up from llvm#89020

nikic

LGTM

goldsteinn requested a review from nikic as a code owner May 30, 2024 16:01

llvmbot added the llvm:transforms label May 30, 2024

goldsteinn changed the title ~~goldsteinn/add of ext~~ [InstCombine] Fold (add X, (sext/zext (icmp eq X, C))) May 30, 2024

goldsteinn requested a review from dtcxzyw May 30, 2024 16:02

goldsteinn mentioned this pull request May 30, 2024

[InstCombine] Fold (op x, ({z,s}ext (icmp eq x, C))) to select #89020

Closed

dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request May 30, 2024

pre-commit: test PR93840

9297ddc

PR Link: llvm/llvm-project#93840

dtcxzyw mentioned this pull request May 30, 2024

pre-commit: test PR93840 dtcxzyw/llvm-opt-benchmark#649

Closed

dtcxzyw reviewed May 30, 2024

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp Show resolved Hide resolved

nikic reviewed Jun 1, 2024

View reviewed changes

goldsteinn added 2 commits June 1, 2024 12:40

[InstCombine] Add tests for folding `(add X, (sext/zext (icmp eq X, C…

e51e643

…)))`; NFC

goldsteinn force-pushed the goldsteinn/add-of-ext branch from 8da0428 to bc2ae38 Compare June 1, 2024 17:40

nikic approved these changes Jun 1, 2024

View reviewed changes

goldsteinn closed this in 0310f7f Jun 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[InstCombine] Fold `(add X, (sext/zext (icmp eq X, C)))` #93840

[InstCombine] Fold `(add X, (sext/zext (icmp eq X, C)))` #93840

Uh oh!

goldsteinn commented May 30, 2024 •

edited

Loading

Uh oh!

llvmbot commented May 30, 2024

Uh oh!

goldsteinn commented May 30, 2024

Uh oh!

dtcxzyw May 30, 2024

Uh oh!

Uh oh!

dtcxzyw commented May 30, 2024

Uh oh!

goldsteinn commented May 30, 2024

Uh oh!

dtcxzyw commented May 30, 2024

Uh oh!

goldsteinn commented May 30, 2024

Uh oh!

nikic Jun 1, 2024

Uh oh!

nikic Jun 1, 2024

Uh oh!

nikic Jun 1, 2024

Uh oh!

goldsteinn Jun 1, 2024

Uh oh!

nikic left a comment

Uh oh!

Uh oh!

	Constant *ExtC = match(Ext, m_ZExt(m_Value()))
	Constant *ExtC = isa<ZExtInst>(Ext)

[InstCombine] Fold (add X, (sext/zext (icmp eq X, C))) #93840

[InstCombine] Fold (add X, (sext/zext (icmp eq X, C))) #93840

Uh oh!

Conversation

goldsteinn commented May 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented May 30, 2024

Uh oh!

goldsteinn commented May 30, 2024

Uh oh!

dtcxzyw May 30, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dtcxzyw commented May 30, 2024

Uh oh!

goldsteinn commented May 30, 2024

Uh oh!

dtcxzyw commented May 30, 2024

Uh oh!

goldsteinn commented May 30, 2024

Uh oh!

nikic Jun 1, 2024

Choose a reason for hiding this comment

Uh oh!

nikic Jun 1, 2024

Choose a reason for hiding this comment

Uh oh!

nikic Jun 1, 2024

Choose a reason for hiding this comment

Uh oh!

goldsteinn Jun 1, 2024

Choose a reason for hiding this comment

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

[InstCombine] Fold `(add X, (sext/zext (icmp eq X, C)))` #93840

[InstCombine] Fold `(add X, (sext/zext (icmp eq X, C)))` #93840

goldsteinn commented May 30, 2024 •

edited

Loading