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] Use existing (icmp pred (add x, y), C) folds with or disjoint #87937

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

goldsteinn
Copy link
Contributor

  • [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)
  • [InstCombine] Use existing (icmp pred (add x, y), C) folds with or disjoint

…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
… disjoint`

No significant changes, just some missed plumbing.
@goldsteinn goldsteinn requested a review from nikic as a code owner April 7, 2024 19:49
@goldsteinn goldsteinn requested a review from dtcxzyw April 7, 2024 19:49
@goldsteinn goldsteinn requested a review from XChy April 7, 2024 19:49
@goldsteinn goldsteinn changed the title goldsteinn/icmp add or disjoint or [InstCombine] Use existing (icmp pred (add x, y), C) folds with or disjoint Apr 7, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 7, 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)
  • [InstCombine] Use existing (icmp pred (add x, y), C) folds with or disjoint

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

3 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp (+32-23)
  • (modified) llvm/test/Transforms/InstCombine/icmp-add.ll (+39-24)
  • (modified) llvm/test/Transforms/InstCombine/known-bits.ll (+1-2)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index db302d7e526844..f805fc21f97de6 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -2986,10 +2986,10 @@ Instruction *InstCombinerImpl::foldICmpAddConstant(ICmpInst &Cmp,
   Value *Op0, *Op1;
   Instruction *Ext0, *Ext1;
   const CmpInst::Predicate Pred = Cmp.getPredicate();
-  if (match(Add,
-            m_Add(m_CombineAnd(m_Instruction(Ext0), m_ZExtOrSExt(m_Value(Op0))),
-                  m_CombineAnd(m_Instruction(Ext1),
-                               m_ZExtOrSExt(m_Value(Op1))))) &&
+  if (match(Add, m_AddLike(m_CombineAnd(m_Instruction(Ext0),
+                                        m_ZExtOrSExt(m_Value(Op0))),
+                           m_CombineAnd(m_Instruction(Ext1),
+                                        m_ZExtOrSExt(m_Value(Op1))))) &&
       Op0->getType()->isIntOrIntVectorTy(1) &&
       Op1->getType()->isIntOrIntVectorTy(1)) {
     unsigned BW = C.getBitWidth();
@@ -3021,9 +3021,9 @@ Instruction *InstCombinerImpl::foldICmpAddConstant(ICmpInst &Cmp,
   // If the add does not wrap, we can always adjust the compare by subtracting
   // the constants. Equality comparisons are handled elsewhere. SGE/SLE/UGE/ULE
   // are canonicalized to SGT/SLT/UGT/ULT.
-  if ((Add->hasNoSignedWrap() &&
+  if ((match(Add, m_NSWAddLike(m_Value(), m_Value())) &&
        (Pred == ICmpInst::ICMP_SGT || Pred == ICmpInst::ICMP_SLT)) ||
-      (Add->hasNoUnsignedWrap() &&
+      (match(Add, m_NUWAddLike(m_Value(), m_Value())) &&
        (Pred == ICmpInst::ICMP_UGT || Pred == ICmpInst::ICMP_ULT))) {
     bool Overflow;
     APInt NewC =
@@ -3438,6 +3438,20 @@ Instruction *InstCombinerImpl::foldICmpBinOpEqualityWithConstant(
       }
     }
     break;
+  case Instruction::Or: {
+    const APInt *BOC;
+    if (match(BOp1, m_APInt(BOC)) && BO->hasOneUse() && RHS->isAllOnesValue()) {
+      // Comparing if all bits outside of a constant mask are set?
+      // Replace (X | C) == -1 with (X & ~C) == ~C.
+      // This removes the -1 constant.
+      Constant *NotBOC = ConstantExpr::getNot(cast<Constant>(BOp1));
+      Value *And = Builder.CreateAnd(BOp0, NotBOC);
+      return new ICmpInst(Pred, And, NotBOC);
+    }
+    if (!cast<PossiblyDisjointInst>(BO)->isDisjoint())
+      break;
+    [[fallthrough]];
+  }
   case Instruction::Add: {
     // (A + C2) == C --> A == (C - C2)
     // (A + C2) != C --> A != (C - C2)
@@ -3452,7 +3466,12 @@ Instruction *InstCombinerImpl::foldICmpBinOpEqualityWithConstant(
         return new ICmpInst(Pred, BOp0, NegVal);
       if (Value *NegVal = dyn_castNegVal(BOp0))
         return new ICmpInst(Pred, NegVal, BOp1);
-      if (BO->hasOneUse()) {
+      if (BO->hasOneUse() && BO->getOpcode() != Instruction::Or) {
+        // (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);
@@ -3472,18 +3491,6 @@ Instruction *InstCombinerImpl::foldICmpBinOpEqualityWithConstant(
       }
     }
     break;
-  case Instruction::Or: {
-    const APInt *BOC;
-    if (match(BOp1, m_APInt(BOC)) && BO->hasOneUse() && RHS->isAllOnesValue()) {
-      // Comparing if all bits outside of a constant mask are set?
-      // Replace (X | C) == -1 with (X & ~C) == ~C.
-      // This removes the -1 constant.
-      Constant *NotBOC = ConstantExpr::getNot(cast<Constant>(BOp1));
-      Value *And = Builder.CreateAnd(BOp0, NotBOC);
-      return new ICmpInst(Pred, And, NotBOC);
-    }
-    break;
-  }
   case Instruction::UDiv:
   case Instruction::SDiv:
     if (BO->isExact()) {
@@ -3756,10 +3763,6 @@ Instruction *InstCombinerImpl::foldICmpBinOpWithConstant(ICmpInst &Cmp,
     if (Instruction *I = foldICmpAndConstant(Cmp, BO, C))
       return I;
     break;
-  case Instruction::Or:
-    if (Instruction *I = foldICmpOrConstant(Cmp, BO, C))
-      return I;
-    break;
   case Instruction::Mul:
     if (Instruction *I = foldICmpMulConstant(Cmp, BO, C))
       return I;
@@ -3789,6 +3792,12 @@ Instruction *InstCombinerImpl::foldICmpBinOpWithConstant(ICmpInst &Cmp,
     if (Instruction *I = foldICmpSubConstant(Cmp, BO, C))
       return I;
     break;
+  case Instruction::Or:
+    if (Instruction *I = foldICmpOrConstant(Cmp, BO, C))
+      return I;
+    if (!cast<PossiblyDisjointInst>(BO)->isDisjoint())
+      break;
+    [[fallthrough]];
   case Instruction::Add:
     if (Instruction *I = foldICmpAddConstant(Cmp, BO, C))
       return I;
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)
diff --git a/llvm/test/Transforms/InstCombine/known-bits.ll b/llvm/test/Transforms/InstCombine/known-bits.ll
index 769f7661fc8dc0..ba5619a25a49df 100644
--- a/llvm/test/Transforms/InstCombine/known-bits.ll
+++ b/llvm/test/Transforms/InstCombine/known-bits.ll
@@ -483,8 +483,7 @@ if.else:
 define i1 @test_icmp_or_distjoint(i8 %n, i1 %other) {
 ; CHECK-LABEL: @test_icmp_or_distjoint(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[N_OR:%.*]] = or disjoint i8 [[N:%.*]], 16
-; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i8 [[N_OR]], -111
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i8 [[N:%.*]], -127
 ; CHECK-NEXT:    br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
 ; CHECK:       if.then:
 ; CHECK-NEXT:    ret i1 true

m_Add(m_CombineAnd(m_Instruction(Ext0), m_ZExtOrSExt(m_Value(Op0))),
m_CombineAnd(m_Instruction(Ext1),
m_ZExtOrSExt(m_Value(Op1))))) &&
if (match(Add, m_AddLike(m_CombineAnd(m_Instruction(Ext0),
Copy link
Member

Choose a reason for hiding this comment

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

Duplicate of #75899.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, ill sit on this for a bit, if he gets back in the next month or so ill close. Ill pull out the add nuw patch.

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

3 participants