Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[InstCombine] Fold (icmp eq/ne (add nuw x, y), 0) -> (icmp eq/ne (or x, y), 0) #88088

Closed
wants to merge 2 commits into from

Conversation

goldsteinn
Copy link
Contributor

@goldsteinn goldsteinn commented Apr 9, 2024

  • [InstCombine] Add tests for folding (icmp eq/ne (add nuw x, y), 0); NFC
  • [InstCombine] Fold (icmp eq/ne (add nuw x, y), 0) -> (icmp eq/ne (or x, y), 0)

(icmp eq/ne (or x, y), 0) is probably easier to analyze than (icmp eq/ne x, -y)

Proof: https://alive2.llvm.org/ce/z/2-VTb6

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 9, 2024

@llvm/pr-subscribers-llvm-transforms

Author: None (goldsteinn)

Changes
  • [InstCombine] Add tests for folding (icmp eq/ne (add nuw x, y), 0); NFC
  • [InstCombine] Fold (icmp eq/ne (add nuw x, y), 0) -> (icmp eq/ne (or x, y), 0)

(icmp eq/ne (or x, y), 0) is probably easier to analyze than (icmp eq/ne x, -y)

Proof: https://alive2.llvm.org/ce/z/2-VTb6


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp (+5)
  • (modified) llvm/test/Transforms/InstCombine/icmp-add.ll (+39-24)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index db302d7e526844..53aa84d53f3085 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -3453,6 +3453,11 @@ Instruction *InstCombinerImpl::foldICmpBinOpEqualityWithConstant(
       if (Value *NegVal = dyn_castNegVal(BOp0))
         return new ICmpInst(Pred, NegVal, BOp1);
       if (BO->hasOneUse()) {
+        // (add nuw A, B) != 0 -> (or A, B) != 0
+        if (match(BO, m_NUWAdd(m_Value(), m_Value()))) {
+          Value *Or = Builder.CreateOr(BOp0, BOp1);
+          return new ICmpInst(Pred, Or, Constant::getNullValue(BO->getType()));
+        }
         Value *Neg = Builder.CreateNeg(BOp1);
         Neg->takeName(BO);
         return new ICmpInst(Pred, BOp0, Neg);
diff --git a/llvm/test/Transforms/InstCombine/icmp-add.ll b/llvm/test/Transforms/InstCombine/icmp-add.ll
index b99ed20d7d431c..e71a3af4b5be4f 100644
--- a/llvm/test/Transforms/InstCombine/icmp-add.ll
+++ b/llvm/test/Transforms/InstCombine/icmp-add.ll
@@ -9,10 +9,8 @@ declare void @use(i32)
 define i1 @cvt_icmp_0_zext_plus_zext_eq_i16(i16 %arg, i16 %arg1) {
 ; CHECK-LABEL: @cvt_icmp_0_zext_plus_zext_eq_i16(
 ; CHECK-NEXT:  bb:
-; CHECK-NEXT:    [[I:%.*]] = zext i16 [[ARG:%.*]] to i32
-; CHECK-NEXT:    [[I2:%.*]] = zext i16 [[ARG1:%.*]] to i32
-; CHECK-NEXT:    [[I3:%.*]] = sub nsw i32 0, [[I]]
-; CHECK-NEXT:    [[I4:%.*]] = icmp eq i32 [[I2]], [[I3]]
+; CHECK-NEXT:    [[TMP0:%.*]] = or i16 [[ARG1:%.*]], [[ARG:%.*]]
+; CHECK-NEXT:    [[I4:%.*]] = icmp eq i16 [[TMP0]], 0
 ; CHECK-NEXT:    ret i1 [[I4]]
 ;
 bb:
@@ -27,10 +25,8 @@ bb:
 define i1 @cvt_icmp_0_zext_plus_zext_eq_i8(i8 %arg, i8 %arg1) {
 ; CHECK-LABEL: @cvt_icmp_0_zext_plus_zext_eq_i8(
 ; CHECK-NEXT:  bb:
-; CHECK-NEXT:    [[I:%.*]] = zext i8 [[ARG:%.*]] to i32
-; CHECK-NEXT:    [[I2:%.*]] = zext i8 [[ARG1:%.*]] to i32
-; CHECK-NEXT:    [[I3:%.*]] = sub nsw i32 0, [[I]]
-; CHECK-NEXT:    [[I4:%.*]] = icmp eq i32 [[I2]], [[I3]]
+; CHECK-NEXT:    [[TMP0:%.*]] = or i8 [[ARG1:%.*]], [[ARG:%.*]]
+; CHECK-NEXT:    [[I4:%.*]] = icmp eq i8 [[TMP0]], 0
 ; CHECK-NEXT:    ret i1 [[I4]]
 ;
 bb:
@@ -1802,22 +1798,17 @@ define i1 @test4(i32 %a) {
   ret i1 %c
 }
 
-define { i32, i1 } @test4multiuse(i32 %a) {
-; CHECK-LABEL: @test4multiuse(
-; CHECK-NEXT:    [[B:%.*]] = add nsw i32 [[A:%.*]], -2147483644
-; CHECK-NEXT:    [[C:%.*]] = icmp slt i32 [[A]], 2147483640
-; CHECK-NEXT:    [[TMP:%.*]] = insertvalue { i32, i1 } undef, i32 [[B]], 0
-; CHECK-NEXT:    [[RES:%.*]] = insertvalue { i32, i1 } [[TMP]], i1 [[C]], 1
-; CHECK-NEXT:    ret { i32, i1 } [[RES]]
+define {
+i32, i1 } @test4multiuse(i32 %a) {
 ;
 
-  %b = add nsw i32 %a, -2147483644
-  %c = icmp slt i32 %b, -4
+%b = add nsw i32 %a, -2147483644
+%c = icmp slt i32 %b, -4
 
-  %tmp = insertvalue { i32, i1 } undef, i32 %b, 0
-  %res = insertvalue { i32, i1 } %tmp, i1 %c, 1
+%tmp = insertvalue { i32, i1 } undef, i32 %b, 0
+%res = insertvalue { i32, i1 } %tmp, i1 %c, 1
 
-  ret { i32, i1 } %res
+ret { i32, i1 } %res
 }
 
 define <2 x i1> @test4vec(<2 x i32> %a) {
@@ -2857,7 +2848,7 @@ define i1 @icmp_add_add_C_comm2(i32 %X, i32 %b) {
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i32 [[A]], [[TMP1]]
 ; CHECK-NEXT:    ret i1 [[CMP]]
 ;
-  %a = udiv i32 42, %X ; thwart complexity-based canonicalization
+  %a = udiv i32 42, %X  ; thwart complexity-based canonicalization
   %add1 = add i32 %a, %b
   %add2 = add i32 %add1, -1
   %cmp = icmp ugt i32 %a, %add2
@@ -2871,7 +2862,7 @@ define i1 @icmp_add_add_C_comm2_pred(i32 %X, i32 %b) {
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ule i32 [[A]], [[TMP1]]
 ; CHECK-NEXT:    ret i1 [[CMP]]
 ;
-  %a = udiv i32 42, %X ; thwart complexity-based canonicalization
+  %a = udiv i32 42, %X  ; thwart complexity-based canonicalization
   %add1 = add i32 %a, %b
   %add2 = add i32 %add1, -1
   %cmp = icmp ule i32 %a, %add2
@@ -2886,7 +2877,7 @@ define i1 @icmp_add_add_C_comm2_wrong_pred(i32 %X, i32 %b) {
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ult i32 [[A]], [[ADD2]]
 ; CHECK-NEXT:    ret i1 [[CMP]]
 ;
-  %a = udiv i32 42, %X ; thwart complexity-based canonicalization
+  %a = udiv i32 42, %X  ; thwart complexity-based canonicalization
   %add1 = add i32 %a, %b
   %add2 = add i32 %add1, -1
   %cmp = icmp ult i32 %a, %add2
@@ -2900,7 +2891,7 @@ define i1 @icmp_add_add_C_comm3(i32 %X, i32 %b) {
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i32 [[A]], [[TMP1]]
 ; CHECK-NEXT:    ret i1 [[CMP]]
 ;
-  %a = udiv i32 42, %X ; thwart complexity-based canonicalization
+  %a = udiv i32 42, %X  ; thwart complexity-based canonicalization
   %add1 = add i32 %b, %a
   %add2 = add i32 %add1, -1
   %cmp = icmp ugt i32 %a, %add2
@@ -3003,4 +2994,28 @@ define i1 @icmp_dec_notnonzero(i8 %x) {
   ret i1 %c
 }
 
+define i1 @icmp_addnuw_nonzero(i8 %x, i8 %y) {
+; CHECK-LABEL: @icmp_addnuw_nonzero(
+; CHECK-NEXT:    [[TMP1:%.*]] = or i8 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    [[C:%.*]] = icmp eq i8 [[TMP1]], 0
+; CHECK-NEXT:    ret i1 [[C]]
+;
+  %i = add nuw i8 %x, %y
+  %c = icmp eq i8 %i, 0
+  ret i1 %c
+}
+
+define i1 @icmp_addnuw_nonzero_fail_multiuse(i32 %x, i32 %y) {
+; CHECK-LABEL: @icmp_addnuw_nonzero_fail_multiuse(
+; CHECK-NEXT:    [[I:%.*]] = add nuw i32 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    [[C:%.*]] = icmp eq i32 [[I]], 0
+; CHECK-NEXT:    call void @use(i32 [[I]])
+; CHECK-NEXT:    ret i1 [[C]]
+;
+  %i = add nuw i32 %x, %y
+  %c = icmp eq i32 %i, 0
+  call void @use(i32 %i)
+  ret i1 %c
+}
+
 declare void @llvm.assume(i1)

@goldsteinn goldsteinn changed the title goldsteinn/add nuw to or [InstCombine] Fold (icmp eq/ne (add nuw x, y), 0) -> (icmp eq/ne (or x, y), 0) Apr 9, 2024
@goldsteinn goldsteinn requested a review from dtcxzyw April 9, 2024 04:43
; CHECK-NEXT: [[RES:%.*]] = insertvalue { i32, i1 } [[TMP]], i1 [[C]], 1
; CHECK-NEXT: ret { i32, i1 } [[RES]]
define {
i32, i1 } @test4multiuse(i32 %a) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something went wrong here -- did UTC break this function?

Copy link
Contributor Author

@goldsteinn goldsteinn Apr 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, I have a crappy regex to try to format my IR that failed miserably here. Ill fix

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM with the test fixed. I agree that converting this to or is better.

dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request Apr 9, 2024
@dtcxzyw
Copy link
Member

dtcxzyw commented Apr 9, 2024

Missing fold: https://alive2.llvm.org/ce/z/DShc49

define i1 @src(i1 %x, i32 %z) {
  %22 = select i1 %x, i32 0, i32 255
  %23 = or i32 %z, %22
  %eq = icmp eq i32 %23, 0
  ret i1 %eq
}

define i1 @tgt(i1 %x, i32 %z) {
  %eq = icmp eq i32 %z, 0
  %and = and i1 %x, %eq
  ret i1 %and
}

See also https://github.com/dtcxzyw/llvm-opt-benchmark/pull/485/files#r1557018422

Copy link
Member

@dtcxzyw dtcxzyw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@goldsteinn goldsteinn closed this in 7599d47 Apr 9, 2024
goldsteinn added a commit to goldsteinn/llvm-project that referenced this pull request Apr 9, 2024
Four cases:
`(icmp eq (or (select cond, 0, NonZero), Other))`
 -> `(and cond, (icmp eq Other, 0))`
`(icmp ne (or (select cond, NonZero, 0), Other))`
 -> `(or cond, (icmp ne Other, 0))`
`(icmp ne (or (select cond, 0, NonZero), Other))`
 -> `(or (not cond), (icmp ne Other, 0))`
`(icmp eq (or (select cond, NonZero, 0), Other))`
 -> `(and (not cond), (icmp eq Other, 0))`

These cases came up in tests on: llvm#88088

Proofs: https://alive2.llvm.org/ce/z/ojGo_J
@goldsteinn
Copy link
Contributor Author

Missing fold: https://alive2.llvm.org/ce/z/DShc49

define i1 @src(i1 %x, i32 %z) {
  %22 = select i1 %x, i32 0, i32 255
  %23 = or i32 %z, %22
  %eq = icmp eq i32 %23, 0
  ret i1 %eq
}

define i1 @tgt(i1 %x, i32 %z) {
  %eq = icmp eq i32 %z, 0
  %and = and i1 %x, %eq
  ret i1 %and
}

See also https://github.com/dtcxzyw/llvm-opt-benchmark/pull/485/files#r1557018422

See: #88183

goldsteinn added a commit to goldsteinn/llvm-project that referenced this pull request Apr 9, 2024
Four cases:
`(icmp eq (or (select cond, 0, NonZero), Other))`
 -> `(and cond, (icmp eq Other, 0))`
`(icmp ne (or (select cond, NonZero, 0), Other))`
 -> `(or cond, (icmp ne Other, 0))`
`(icmp ne (or (select cond, 0, NonZero), Other))`
 -> `(or (not cond), (icmp ne Other, 0))`
`(icmp eq (or (select cond, NonZero, 0), Other))`
 -> `(and (not cond), (icmp eq Other, 0))`

These cases came up in tests on: llvm#88088

Proofs: https://alive2.llvm.org/ce/z/ojGo_J
goldsteinn added a commit to goldsteinn/llvm-project that referenced this pull request Apr 10, 2024
Four cases:
`(icmp eq (or (select cond, 0, NonZero), Other))`
 -> `(and cond, (icmp eq Other, 0))`
`(icmp ne (or (select cond, NonZero, 0), Other))`
 -> `(or cond, (icmp ne Other, 0))`
`(icmp ne (or (select cond, 0, NonZero), Other))`
 -> `(or (not cond), (icmp ne Other, 0))`
`(icmp eq (or (select cond, NonZero, 0), Other))`
 -> `(and (not cond), (icmp eq Other, 0))`

These cases came up in tests on: llvm#88088

Proofs: https://alive2.llvm.org/ce/z/ojGo_J
goldsteinn added a commit to goldsteinn/llvm-project that referenced this pull request May 5, 2024
Four cases:
`(icmp eq (or (select cond, 0, NonZero), Other))`
 -> `(and cond, (icmp eq Other, 0))`
`(icmp ne (or (select cond, NonZero, 0), Other))`
 -> `(or cond, (icmp ne Other, 0))`
`(icmp ne (or (select cond, 0, NonZero), Other))`
 -> `(or (not cond), (icmp ne Other, 0))`
`(icmp eq (or (select cond, NonZero, 0), Other))`
 -> `(and (not cond), (icmp eq Other, 0))`

These cases came up in tests on: llvm#88088

Proofs: https://alive2.llvm.org/ce/z/ojGo_J
goldsteinn added a commit to goldsteinn/llvm-project that referenced this pull request May 11, 2024
Four cases:
`(icmp eq (or (select cond, 0, NonZero), Other))`
 -> `(and cond, (icmp eq Other, 0))`
`(icmp ne (or (select cond, NonZero, 0), Other))`
 -> `(or cond, (icmp ne Other, 0))`
`(icmp ne (or (select cond, 0, NonZero), Other))`
 -> `(or (not cond), (icmp ne Other, 0))`
`(icmp eq (or (select cond, NonZero, 0), Other))`
 -> `(and (not cond), (icmp eq Other, 0))`

These cases came up in tests on: llvm#88088

Proofs: https://alive2.llvm.org/ce/z/ojGo_J
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants