Skip to content

Conversation

goldsteinn
Copy link
Contributor

@goldsteinn goldsteinn commented Apr 9, 2024

  • [InstCombine] Add tests for folding (icmp eq/ne (or (select cond, 0/NZ, 0/NZ), X), 0); NFC
  • [InstCombine] Fold (icmp eq/ne (or (select cond, 0/NZ, 0/NZ), X), 0)

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: #88088

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

@llvmbot
Copy link
Member

llvmbot commented Apr 9, 2024

@llvm/pr-subscribers-llvm-ir
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-llvm-transforms

Author: None (goldsteinn)

Changes
  • [InstCombine] Add tests for folding (icmp eq/ne (or (select cond, 0/NZ, 0/NZ), X), 0); NFC
  • [InstCombine] Fold (icmp eq/ne (or (select cond, 0/NZ, 0/NZ), X), 0)

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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp (+60)
  • (added) llvm/test/Transforms/InstCombine/icmp-or-of-select-with-zero.ll (+225)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index 53aa84d53f3085..c6949ff7441889 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -3487,6 +3487,66 @@ Instruction *InstCombinerImpl::foldICmpBinOpEqualityWithConstant(
       Value *And = Builder.CreateAnd(BOp0, NotBOC);
       return new ICmpInst(Pred, And, NotBOC);
     }
+    // (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))
+    Value *Cond, *TV, *FV, *Other;
+    if (C.isZero() &&
+        match(BO, m_c_Or(m_Select(m_Value(Cond), m_Value(TV), m_Value(FV)),
+                         m_Value(Other)))) {
+      auto IsNonZero = [&](Value *V) {
+        return isKnownNonZero(V, SQ.DL, /*Depth=*/0, SQ.AC, SQ.CxtI, SQ.DT);
+      };
+      // Easy case is if eq/ne matches whether 0 is trueval/falseval.
+      if (Pred == ICmpInst::ICMP_EQ
+              ? (match(TV, m_SpecificInt(C)) && IsNonZero(FV))
+              : (match(FV, m_SpecificInt(C)) && IsNonZero(TV))) {
+        Value *Cmp = Builder.CreateICmp(
+            Pred, Other, Constant::getNullValue(Other->getType()));
+        return BinaryOperator::Create(
+            Pred == ICmpInst::ICMP_EQ ? Instruction::And : Instruction::Or, Cmp,
+            Cond);
+      }
+      // Harder case is if eq/ne matches whether 0 is falseval/trueval. In this
+      // case we need to invert the select condition so we need to be careful to
+      // avoid creating extra instructions.
+      if (Pred == ICmpInst::ICMP_EQ
+              ? (match(FV, m_SpecificInt(C)) && IsNonZero(TV))
+              : (match(TV, m_SpecificInt(C)) && IsNonZero(FV))) {
+        Value *NotCond = nullptr;
+        // If the select is one use, we are essentially replacing select with
+        // `(not Cond)`.
+        if (match(BO, m_c_Or(m_OneUse(m_Select(m_Specific(Cond), m_Specific(TV),
+                                               m_Specific(FV))),
+                             m_Value()))) {
+          NotCond = Builder.CreateNot(Cond);
+        } else {
+          // Otherwise, see if we can get NotCond for free.
+          Instruction *Ins = dyn_cast<Instruction>(Cond);
+          bool InvertAll = Ins && InstCombiner::canFreelyInvertAllUsersOf(
+                                      Ins, /*IgnoredUser=*/nullptr);
+          if (Ins)
+            Builder.SetInsertPoint(Ins);
+          NotCond = getFreelyInverted(Cond, InvertAll, &Builder);
+          if (NotCond && InvertAll) {
+            freelyInvertAllUsersOf(Ins, /*IgnoredUser=*/nullptr);
+            replaceInstUsesWith(*Ins, NotCond);            
+          }
+        }
+        if (NotCond) {
+          Value *Cmp = Builder.CreateICmp(
+              Pred, Other, Constant::getNullValue(Other->getType()));
+          return BinaryOperator::Create(
+              Pred == ICmpInst::ICMP_EQ ? Instruction::And : Instruction::Or,
+              Cmp, NotCond);
+        }
+      }
+    }
     break;
   }
   case Instruction::UDiv:
diff --git a/llvm/test/Transforms/InstCombine/icmp-or-of-select-with-zero.ll b/llvm/test/Transforms/InstCombine/icmp-or-of-select-with-zero.ll
new file mode 100644
index 00000000000000..f7da7a50973a50
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/icmp-or-of-select-with-zero.ll
@@ -0,0 +1,225 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -passes='instcombine<no-verify-fixpoint>' -S | FileCheck %s
+
+declare void @use.i8(i8)
+declare void @use.i1(i1)
+define i1 @src_tv_eq(i1 %c0, i8 %x, i8 %yy) {
+; CHECK-LABEL: @src_tv_eq(
+; CHECK-NEXT:    [[R:%.*]] = icmp eq i8 [[SELX:%.*]], 0
+; CHECK-NEXT:    [[R1:%.*]] = and i1 [[R]], [[C0:%.*]]
+; CHECK-NEXT:    ret i1 [[R1]]
+;
+  %y = add nuw i8 %yy, 1
+  %sel = select i1 %c0, i8 0, i8 %y
+  %selx = or i8 %sel, %x
+  %r = icmp eq i8 %selx, 0
+  ret i1 %r
+}
+
+define i1 @src_tv_eq_fail_tv_nonzero(i1 %c0, i8 %x, i8 %yy) {
+; CHECK-LABEL: @src_tv_eq_fail_tv_nonzero(
+; CHECK-NEXT:    [[Y:%.*]] = add nsw i8 [[YY:%.*]], 1
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[C0:%.*]], i8 1, i8 [[Y]]
+; CHECK-NEXT:    [[SELX:%.*]] = or i8 [[SEL]], [[X:%.*]]
+; CHECK-NEXT:    [[R:%.*]] = icmp eq i8 [[SELX]], 0
+; CHECK-NEXT:    ret i1 [[R]]
+;
+  %y = add nsw i8 %yy, 1
+  %sel = select i1 %c0, i8 1, i8 %y
+  %selx = or i8 %sel, %x
+  %r = icmp eq i8 %selx, 0
+  ret i1 %r
+}
+
+define i1 @src_fv_ne(i1 %c0, i8 %x, i8 %yy) {
+; CHECK-LABEL: @src_fv_ne(
+; CHECK-NEXT:    [[R:%.*]] = icmp ne i8 [[SELX:%.*]], 0
+; CHECK-NEXT:    [[R1:%.*]] = or i1 [[R]], [[C0:%.*]]
+; CHECK-NEXT:    ret i1 [[R1]]
+;
+  %y = add nuw i8 %yy, 1
+  %sel = select i1 %c0, i8 %y, i8 0
+  %selx = or i8 %sel, %x
+  %r = icmp ne i8 %selx, 0
+  ret i1 %r
+}
+
+define i1 @src_fv_ne_fail_maybe_zero(i1 %c0, i8 %x, i8 %yy) {
+; CHECK-LABEL: @src_fv_ne_fail_maybe_zero(
+; CHECK-NEXT:    [[Y:%.*]] = add nsw i8 [[YY:%.*]], 1
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[C0:%.*]], i8 [[Y]], i8 0
+; CHECK-NEXT:    [[SELX:%.*]] = or i8 [[SEL]], [[X:%.*]]
+; CHECK-NEXT:    [[R:%.*]] = icmp ne i8 [[SELX]], 0
+; CHECK-NEXT:    ret i1 [[R]]
+;
+  %y = add nsw i8 %yy, 1
+  %sel = select i1 %c0, i8 %y, i8 0
+  %selx = or i8 %sel, %x
+  %r = icmp ne i8 %selx, 0
+  ret i1 %r
+}
+
+define i1 @src_tv_ne(i1 %c0, i8 %x, i8 %yy) {
+; CHECK-LABEL: @src_tv_ne(
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i1 [[C0:%.*]], true
+; CHECK-NEXT:    [[R:%.*]] = icmp ne i8 [[SELX:%.*]], 0
+; CHECK-NEXT:    [[R1:%.*]] = or i1 [[R]], [[TMP1]]
+; CHECK-NEXT:    ret i1 [[R1]]
+;
+  %y = add nuw i8 %yy, 1
+  %sel = select i1 %c0, i8 0, i8 %y
+  %selx = or i8 %sel, %x
+  %r = icmp ne i8 %selx, 0
+  ret i1 %r
+}
+
+define i1 @src_tv_ne_fail_cmp_nonzero(i1 %c0, i8 %x, i8 %yy) {
+; CHECK-LABEL: @src_tv_ne_fail_cmp_nonzero(
+; CHECK-NEXT:    [[Y:%.*]] = add nuw i8 [[YY:%.*]], 1
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[C0:%.*]], i8 0, i8 [[Y]]
+; CHECK-NEXT:    [[SELX:%.*]] = or i8 [[SEL]], [[X:%.*]]
+; CHECK-NEXT:    [[R:%.*]] = icmp ne i8 [[SELX]], 1
+; CHECK-NEXT:    ret i1 [[R]]
+;
+  %y = add nuw i8 %yy, 1
+  %sel = select i1 %c0, i8 0, i8 %y
+  %selx = or i8 %sel, %x
+  %r = icmp ne i8 %selx, 1
+  ret i1 %r
+}
+
+define i1 @src_fv_eq(i1 %c0, i8 %x, i8 %yy) {
+; CHECK-LABEL: @src_fv_eq(
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i1 [[C0:%.*]], true
+; CHECK-NEXT:    [[R:%.*]] = icmp eq i8 [[SELX:%.*]], 0
+; CHECK-NEXT:    [[R1:%.*]] = and i1 [[R]], [[TMP1]]
+; CHECK-NEXT:    ret i1 [[R1]]
+;
+  %y = add nuw i8 %yy, 1
+  %sel = select i1 %c0, i8 %y, i8 0
+  %selx = or i8 %sel, %x
+  %r = icmp eq i8 %selx, 0
+  ret i1 %r
+}
+
+define i1 @src_fv_eq_fail_cant_invert(i1 %c0, i8 %x, i8 %yy) {
+; CHECK-LABEL: @src_fv_eq_fail_cant_invert(
+; CHECK-NEXT:    [[Y:%.*]] = add nuw i8 [[YY:%.*]], 1
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[C0:%.*]], i8 [[Y]], i8 0
+; CHECK-NEXT:    [[SELX:%.*]] = or i8 [[SEL]], [[X:%.*]]
+; CHECK-NEXT:    [[R:%.*]] = icmp eq i8 [[SELX]], 0
+; CHECK-NEXT:    call void @use.i8(i8 [[SEL]])
+; CHECK-NEXT:    ret i1 [[R]]
+;
+  %y = add nuw i8 %yy, 1
+  %sel = select i1 %c0, i8 %y, i8 0
+  %selx = or i8 %sel, %x
+  %r = icmp eq i8 %selx, 0
+  call void @use.i8(i8 %sel)
+  ret i1 %r
+}
+
+define i1 @src_fv_eq_fail_cant_invert2(i1 %c1, i8 %a, i8 %b, i8 %x, i8 %yy) {
+; CHECK-LABEL: @src_fv_eq_fail_cant_invert2(
+; CHECK-NEXT:    [[C0:%.*]] = icmp ugt i8 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[Y:%.*]] = add nuw i8 [[YY:%.*]], 1
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[C0]], i8 [[Y]], i8 0
+; CHECK-NEXT:    [[CC:%.*]] = or i1 [[C0]], [[C1:%.*]]
+; CHECK-NEXT:    [[SEL_OTHER:%.*]] = select i1 [[CC]], i8 [[Y]], i8 [[B]]
+; CHECK-NEXT:    [[SELX:%.*]] = or i8 [[SEL]], [[X:%.*]]
+; CHECK-NEXT:    [[R:%.*]] = icmp eq i8 [[SELX]], 0
+; CHECK-NEXT:    call void @use.i8(i8 [[SEL]])
+; CHECK-NEXT:    call void @use.i8(i8 [[SEL_OTHER]])
+; CHECK-NEXT:    ret i1 [[R]]
+;
+  %c0 = icmp ugt i8 %a, %b
+  %y = add nuw i8 %yy, 1
+  %sel = select i1 %c0, i8 %y, i8 0
+  %cc = or i1 %c0, %c1
+  %sel_other = select i1 %cc, i8 %y, i8 %b
+
+  %selx = or i8 %sel, %x
+  %r = icmp eq i8 %selx, 0
+  call void @use.i8(i8 %sel)
+  call void @use.i8(i8 %sel_other)
+  ret i1 %r
+}
+
+define i1 @src_fv_eq_invert2(i1 %c1, i8 %a, i8 %b, i8 %x, i8 %yy) {
+; CHECK-LABEL: @src_fv_eq_invert2(
+; CHECK-NEXT:    [[C0:%.*]] = icmp ugt i8 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[Y:%.*]] = add nuw i8 [[YY:%.*]], 1
+; CHECK-NEXT:    [[CC:%.*]] = or i1 [[C0]], [[C1:%.*]]
+; CHECK-NEXT:    [[SEL_OTHER:%.*]] = select i1 [[CC]], i8 [[Y]], i8 [[B]]
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i1 [[C0]], true
+; CHECK-NEXT:    [[R:%.*]] = icmp eq i8 [[SELX:%.*]], 0
+; CHECK-NEXT:    [[R1:%.*]] = and i1 [[R]], [[TMP1]]
+; CHECK-NEXT:    call void @use.i8(i8 [[SEL_OTHER]])
+; CHECK-NEXT:    ret i1 [[R1]]
+;
+  %c0 = icmp ugt i8 %a, %b
+  %y = add nuw i8 %yy, 1
+  %sel = select i1 %c0, i8 %y, i8 0
+  %cc = or i1 %c0, %c1
+  %sel_other = select i1 %cc, i8 %y, i8 %b
+
+  %selx = or i8 %sel, %x
+  %r = icmp eq i8 %selx, 0
+  call void @use.i8(i8 %sel_other)
+  ret i1 %r
+}
+
+define i1 @src_fv_eq_invert3(i8 %a, i8 %b, i8 %x, i8 %yy) {
+; CHECK-LABEL: @src_fv_eq_invert3(
+; CHECK-NEXT:    [[C1:%.*]] = icmp ule i8 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp eq i8 [[X:%.*]], 0
+; CHECK-NEXT:    [[C0:%.*]] = icmp ugt i8 [[A]], [[B]]
+; CHECK-NEXT:    [[Y:%.*]] = add nuw i8 [[YY:%.*]], 1
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[C1]], i8 0, i8 [[Y]]
+; CHECK-NEXT:    [[SEL_OTHER:%.*]] = select i1 [[C1]], i8 [[B]], i8 [[Y]]
+; CHECK-NEXT:    [[R:%.*]] = and i1 [[TMP1]], [[C1]]
+; CHECK-NEXT:    call void @use.i8(i8 [[SEL_OTHER]])
+; CHECK-NEXT:    call void @use.i8(i8 [[SEL]])
+; CHECK-NEXT:    ret i1 [[R]]
+;
+  %c0 = icmp ugt i8 %a, %b
+  %y = add nuw i8 %yy, 1
+  %sel = select i1 %c0, i8 %y, i8 0
+  %sel_other = select i1 %c0, i8 %y, i8 %b
+
+  %selx = or i8 %sel, %x
+  %r = icmp eq i8 %selx, 0
+  call void @use.i8(i8 %sel_other)
+  call void @use.i8(i8 %sel)
+  ret i1 %r
+}
+
+define i1 @src_tv_ne_invert(i1 %c1, i8 %a, i8 %b, i8 %x, i8 %yy) {
+; CHECK-LABEL: @src_tv_ne_invert(
+; CHECK-NEXT:    [[NOT_C0:%.*]] = icmp ugt i8 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    call void @use.i1(i1 [[NOT_C0]])
+; CHECK-NEXT:    [[C0:%.*]] = xor i1 [[NOT_C0]], true
+; CHECK-NEXT:    [[Y:%.*]] = add nuw i8 [[YY:%.*]], 1
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[NOT_C0]], i8 [[Y]], i8 0
+; CHECK-NEXT:    [[CC:%.*]] = or i1 [[C0]], [[C1:%.*]]
+; CHECK-NEXT:    [[SEL_OTHER:%.*]] = select i1 [[CC]], i8 [[Y]], i8 [[B]]
+; CHECK-NEXT:    [[R:%.*]] = icmp ne i8 [[SELX:%.*]], 0
+; CHECK-NEXT:    [[R1:%.*]] = or i1 [[R]], [[NOT_C0]]
+; CHECK-NEXT:    call void @use.i8(i8 [[SEL]])
+; CHECK-NEXT:    call void @use.i8(i8 [[SEL_OTHER]])
+; CHECK-NEXT:    ret i1 [[R1]]
+;
+  %not_c0 = icmp ugt i8 %a, %b
+  call void @use.i1(i1 %not_c0)
+  %c0 = xor i1 %not_c0, true
+  %y = add nuw i8 %yy, 1
+  %sel = select i1 %c0, i8 0, i8 %y
+  %cc = or i1 %c0, %c1
+  %sel_other = select i1 %cc, i8 %y, i8 %b
+
+  %selx = or i8 %sel, %x
+  %r = icmp ne i8 %selx, 0
+  call void @use.i8(i8 %sel)
+  call void @use.i8(i8 %sel_other)
+  ret i1 %r
+}

@goldsteinn goldsteinn changed the title perf/goldsteinn/or of select 0 non zero [InstCombine] Fold (icmp eq/ne (or (select cond, 0/NZ, 0/NZ), X), 0) Apr 9, 2024
Copy link

github-actions bot commented Apr 9, 2024

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

@goldsteinn goldsteinn force-pushed the perf/goldsteinn/or-of-select-0-non-zero branch from a18df9a to 7f9c836 Compare April 9, 2024 20:01
dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request Apr 9, 2024
@goldsteinn goldsteinn force-pushed the perf/goldsteinn/or-of-select-0-non-zero branch from 7f9c836 to 29ae6b9 Compare April 10, 2024 19:11
@dtcxzyw dtcxzyw linked an issue Apr 11, 2024 that may be closed by this pull request
@goldsteinn
Copy link
Contributor Author

ping

@goldsteinn goldsteinn force-pushed the perf/goldsteinn/or-of-select-0-non-zero branch from 29ae6b9 to 6c1c790 Compare May 5, 2024 02:22
@llvmbot llvmbot added clang Clang issues not falling into any other category llvm:ir clang:openmp OpenMP related changes to Clang labels May 5, 2024
@goldsteinn goldsteinn force-pushed the perf/goldsteinn/or-of-select-0-non-zero branch from 6c1c790 to f43e49a Compare May 5, 2024 02:24
@goldsteinn
Copy link
Contributor Author

ping

@goldsteinn goldsteinn force-pushed the perf/goldsteinn/or-of-select-0-non-zero branch from f43e49a to 430db44 Compare May 11, 2024 05:52
@goldsteinn
Copy link
Contributor Author

ping

1 similar comment
@goldsteinn
Copy link
Contributor Author

ping

@dtcxzyw
Copy link
Member

dtcxzyw commented Jun 4, 2024

@goldsteinn Can you have a look at the ir diff? dtcxzyw/llvm-opt-benchmark#491
There are some unnecessary and/or insts before branches.

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

@goldsteinn Can you have a look at the ir diff? dtcxzyw/llvm-opt-benchmark#491 There are some unnecessary and/or insts before branches.

Sorry for taking so long to get to this... can you re-run, I went through a few of them but they seem to have disappeared...

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

dtcxzyw commented Aug 3, 2024

@goldsteinn Can you have a look at the ir diff? dtcxzyw/llvm-opt-benchmark#491 There are some unnecessary and/or insts before branches.

Sorry for taking so long to get to this... can you re-run, I went through a few of them but they seem to have disappeared...

Yeah, IR diff looks good now :)

@goldsteinn goldsteinn force-pushed the perf/goldsteinn/or-of-select-0-non-zero branch from 430db44 to 489e75b Compare August 3, 2024 16:30
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. Please wait for approval from @nikic.

Comment on lines +189 to +191



Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

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.

LGTM

@goldsteinn goldsteinn closed this in b4ac7f4 Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:openmp OpenMP related changes to Clang clang Clang issues not falling into any other category llvm:ir llvm:transforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update diff April 9th 2024, 7:28:23 pm
4 participants