[InstCombine] fold `(a == c && b != c) || (a != c && b == c))` to `(a == c) == (b != c)` #94915

zjaffal · 2024-06-09T20:58:15Z

resolves #92966

alive proof
https://alive2.llvm.org/ce/z/bLAQBS

llvmbot · 2024-06-09T20:58:48Z

@llvm/pr-subscribers-llvm-transforms

Author: Zain Jaffal (zjaffal)

Changes

resolves #92966

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

2 Files Affected:

(modified) llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp (+27)
(added) llvm/test/Transforms/InstCombine/fold-a-or-b-zero.ll (+104)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index 8695e9e69df20..e873a86f3332f 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -3421,6 +3421,29 @@ Value *InstCombinerImpl::foldAndOrOfICmps(ICmpInst *LHS, ICmpInst *RHS,
   return foldAndOrOfICmpsUsingRanges(LHS, RHS, IsAnd);
 }
 
+Value *foldAorBZero(BinaryOperator &I, InstCombiner::BuilderTy &Builder) {
+  Value *Op0 = I.getOperand(0);
+  Value *Op1 = I.getOperand(1);
+  if (!Op0->hasOneUse() || !Op1->hasOneUse())
+    return nullptr;
+
+  // match each operand of I with and
+  Value *A, *B;
+  CmpInst::Predicate Pred = CmpInst::ICMP_EQ;
+  CmpInst::Predicate InPred = CmpInst::ICMP_EQ;
+  bool IsOp0 = match(Op0, m_c_And(m_Cmp(Pred, m_Value(A), m_ZeroInt()),
+                                  m_Cmp(InPred, m_Value(B), m_ZeroInt())));
+  bool IsOp1 = match(Op1, m_c_And(m_Cmp(InPred, m_Specific(A), m_ZeroInt()),
+                                  m_Cmp(Pred, m_Specific(B), m_ZeroInt())));
+  if (!IsOp0 || !IsOp1)
+    return nullptr;
+
+  Constant *Zero = ConstantInt::getNullValue(A->getType());
+  auto *LHS = Builder.CreateICmpEQ(A, Zero);
+  auto *RHS = Builder.CreateICmpEQ(B, Zero);
+  return Builder.CreateICmpNE(LHS, RHS);
+}
+
 // FIXME: We use commutative matchers (m_c_*) for some, but not all, matches
 // here. We should standardize that construct where it is needed or choose some
 // other way to ensure that commutated variants of patterns are not missed.
@@ -3450,6 +3473,10 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) {
   if (Instruction *X = foldComplexAndOrPatterns(I, Builder))
     return X;
 
+  // (A == 0 & B != 0) | (A != 0 & B == 0)) -> (A == 0) != (B == 0)
+  if (Value *V = foldAorBZero(I, Builder))
+    return replaceInstUsesWith(I, V);
+
   // (A&B)|(A&C) -> A&(B|C) etc
   if (Value *V = foldUsingDistributiveLaws(I))
     return replaceInstUsesWith(I, V);
diff --git a/llvm/test/Transforms/InstCombine/fold-a-or-b-zero.ll b/llvm/test/Transforms/InstCombine/fold-a-or-b-zero.ll
new file mode 100644
index 0000000000000..5a68927e879a7
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/fold-a-or-b-zero.ll
@@ -0,0 +1,104 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt < %s -S -passes=instcombine | FileCheck %s
+
+declare void @use(i1)
+
+define void @a_or_b(i32 %a, i32 %b)  {
+; CHECK-LABEL: define void @src(
+; CHECK-SAME: i32 [[A:%.*]], i32 [[B:%.*]]) {
+; CHECK-NEXT:    [[A_EQ_ZERO:%.*]] = icmp eq i32 [[A]], 0
+; CHECK-NEXT:    [[B_NE_ZERO:%.*]] = icmp ne i32 [[B]], 0
+; CHECK-NEXT:    [[AND_1:%.*]] = and i1 [[A_EQ_ZERO]], [[B_NE_ZERO]]
+; CHECK-NEXT:    [[A_NE_ZERO:%.*]] = icmp ne i32 [[A]], 0
+; CHECK-NEXT:    [[B_EQ_ZERO:%.*]] = icmp eq i32 [[B]], 0
+; CHECK-NEXT:    [[AND_2:%.*]] = and i1 [[A_NE_ZERO]], [[B_EQ_ZERO]]
+; CHECK-NEXT:    [[OR:%.*]] = or i1 [[AND_1]], [[AND_2]]
+; CHECK-NEXT:    call void @use(i1 [[OR]])
+; CHECK-NEXT:    ret void
+;
+  %a_eq_zero = icmp eq i32 %a, 0
+  %b_ne_zero = icmp ne i32 %b, 0
+  %and.1 = and i1 %a_eq_zero, %b_ne_zero
+  %a_ne_zero = icmp ne i32 %a, 0
+  %b_eq_zero = icmp eq i32 %b, 0
+  %and.2 = and i1 %a_ne_zero, %b_eq_zero
+  %or = or i1 %and.1, %and.2
+  call void @use(i1 %or)
+  ret void
+}
+
+
+define void @a_or_b_zero(i32 %a, i32 %b)  {
+; CHECK-LABEL: define void @src(
+; CHECK-SAME: i32 [[A:%.*]], i32 [[B:%.*]]) {
+; CHECK-NEXT:    [[A_EQ_ZERO:%.*]] = icmp eq i32 [[A]], 0
+; CHECK-NEXT:    [[B_NE_ZERO:%.*]] = icmp ne i32 [[B]], 0
+; CHECK-NEXT:    [[AND_1:%.*]] = and i1 [[A_EQ_ZERO]], [[B_NE_ZERO]]
+; CHECK-NEXT:    [[A_NE_ZERO:%.*]] = icmp ne i32 [[A]], 0
+; CHECK-NEXT:    [[B_EQ_ZERO:%.*]] = icmp eq i32 [[B]], 0
+; CHECK-NEXT:    [[AND_2:%.*]] = and i1 [[A_NE_ZERO]], [[B_EQ_ZERO]]
+; CHECK-NEXT:    [[OR:%.*]] = or i1 [[AND_1]], [[AND_2]]
+; CHECK-NEXT:    call void @use(i1 [[OR]])
+; CHECK-NEXT:    ret void
+;
+  %a_eq_zero = icmp eq i32 %a, 0
+  %b_ne_zero = icmp ne i32 %b, 0
+  %and.1 = and i1 %a_eq_zero, %b_ne_zero
+  %a_ne_zero = icmp ne i32 %a, 0
+  %b_eq_zero = icmp eq i32 %b, 0
+  %and.2 = and i1 %a_ne_zero, %b_eq_zero
+  %or = or i1 %and.1, %and.2
+  call void @use(i1 %or)
+  ret void
+}
+
+define void @a_or_b_multiple_uses(i32 %a, i32 %b)  {
+; CHECK-LABEL: define void @src(
+; CHECK-SAME: i32 [[A:%.*]], i32 [[B:%.*]]) {
+; CHECK-NEXT:    [[A_EQ_ZERO:%.*]] = icmp eq i32 [[A]], 0
+; CHECK-NEXT:    [[B_NE_ZERO:%.*]] = icmp ne i32 [[B]], 0
+; CHECK-NEXT:    [[AND_1:%.*]] = and i1 [[A_EQ_ZERO]], [[B_NE_ZERO]]
+; CHECK-NEXT:    [[A_NE_ZERO:%.*]] = icmp ne i32 [[A]], 0
+; CHECK-NEXT:    [[B_EQ_ZERO:%.*]] = icmp eq i32 [[B]], 0
+; CHECK-NEXT:    [[AND_2:%.*]] = and i1 [[A_NE_ZERO]], [[B_EQ_ZERO]]
+; CHECK-NEXT:    [[OR:%.*]] = or i1 [[AND_1]], [[AND_2]]
+; CHECK-NEXT:    call void @use(i1 [[OR]])
+; CHECK-NEXT:    ret void
+;
+  %a_eq_zero = icmp eq i32 %a, 0
+  %b_ne_zero = icmp ne i32 %b, 0
+  %and.1 = and i1 %a_eq_zero, %b_ne_zero
+  %a_ne_zero = icmp ne i32 %a, 0
+  %b_eq_zero = icmp eq i32 %b, 0
+  %and.2 = and i1 %a_ne_zero, %b_eq_zero
+  call void @use(i1 %and.2)
+  %or = or i1 %and.1, %and.2
+  ret void
+}
+
+define void @a_or_b_multiple_uses_2(i32 %a, i32 %b)  {
+; CHECK-LABEL: define void @src(
+; CHECK-SAME: i32 [[A:%.*]], i32 [[B:%.*]]) {
+; CHECK-NEXT:    [[A_EQ_ZERO:%.*]] = icmp eq i32 [[A]], 0
+; CHECK-NEXT:    [[B_NE_ZERO:%.*]] = icmp ne i32 [[B]], 0
+; CHECK-NEXT:    [[AND_1:%.*]] = and i1 [[A_EQ_ZERO]], [[B_NE_ZERO]]
+; CHECK-NEXT:    [[A_NE_ZERO:%.*]] = icmp ne i32 [[A]], 0
+; CHECK-NEXT:    [[B_EQ_ZERO:%.*]] = icmp eq i32 [[B]], 0
+; CHECK-NEXT:    [[AND_2:%.*]] = and i1 [[A_NE_ZERO]], [[B_EQ_ZERO]]
+; CHECK-NEXT:    [[OR:%.*]] = or i1 [[AND_1]], [[AND_2]]
+; CHECK-NEXT:    call void @use(i1 [[OR]])
+; CHECK-NEXT:    ret void
+;
+  %a_eq_zero = icmp eq i32 %a, 0
+  %b_ne_zero = icmp ne i32 %b, 0
+  call void @use(i1 %b_ne_zero)
+  %and.1 = and i1 %a_eq_zero, %b_ne_zero
+  %a_ne_zero = icmp ne i32 %a, 0
+  %b_eq_zero = icmp eq i32 %b, 0
+  %and.2 = and i1 %a_ne_zero, %b_eq_zero
+  call void @use(i1 %and.1)
+  %or = or i1 %and.1, %and.2
+  ret void
+}
+
+

dtcxzyw · 2024-06-09T21:31:39Z

@zjaffal @pskrgag
Does this pattern exist in some real-world applications? Can we generalize it into (A & B) | (!A & !B) -> A == B?

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

pskrgag · 2024-06-09T21:43:45Z

Hello, @dtcxzyw!

I noticed that pattern during core review on my job and was curios if it can be optimized. IIRC the check was about buffer == NULL and size == 0.

I believe, it can be generalized.

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

I am using `isKnownInversion` in the following pr #94915 it is useful to have the method in a shared class so I can reuse it. I am not sure if `ValueTracking` is the correct place but it looks like most of the methods with the pattern `isKnownX` belong there.

0) != (b == 0)`

zjaffal · 2024-06-19T07:49:49Z

I used isKnownInversion to check for both operands and added tests to check against a constant and to check for pointer compare

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

llvm/test/Transforms/InstCombine/fold-a-or-b-zero.ll

pskrgag · 2024-06-19T08:38:42Z

Also there is a small typo in pr title: a == 1 && b != 0 -> a == 0 && b != 0.

nikic · 2024-06-19T08:40:41Z

Please also add an alive2 proof to the PR description.

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

nikic

Looks mostly good, a few more nits.

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

PR Link: llvm/llvm-project#94915

dtcxzyw · 2024-06-22T06:17:30Z

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

@@ -3421,6 +3421,25 @@ Value *InstCombinerImpl::foldAndOrOfICmps(ICmpInst *LHS, ICmpInst *RHS,
  return foldAndOrOfICmpsUsingRanges(LHS, RHS, IsAnd);
 }

+static Value *foldAorBConst(BinaryOperator &I,


What does Const mean here?

Suggested change

static Value *foldAorBConst(BinaryOperator &I,

static Value *foldOrOfInversions(BinaryOperator &I,

Maybe?

dtcxzyw

LGTM. Thank you!
Please wait for additional approval from other reviewers :)

nikic

LGTM

Note that there is a conjugate pattern with or and and swapped: https://alive2.llvm.org/ce/z/mRhkK2 Would be good to handle it in a followup as well.

nikic · 2024-06-22T07:02:36Z

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

+      !match(I.getOperand(1), m_And(m_Value(Cmp3), m_Value(Cmp4))))
+    return nullptr;
+
+  // Check if any two pairs of the and operations are invertions of each other.


Suggested change

// Check if any two pairs of the and operations are invertions of each other.

// Check if any two pairs of the and operations are inversions of each other.

nikic · 2024-06-22T07:02:50Z

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

@@ -3421,6 +3421,25 @@ Value *InstCombinerImpl::foldAndOrOfICmps(ICmpInst *LHS, ICmpInst *RHS,
  return foldAndOrOfICmpsUsingRanges(LHS, RHS, IsAnd);
 }

+static Value *foldAorBConst(BinaryOperator &I,


Suggested change

static Value *foldAorBConst(BinaryOperator &I,

static Value *foldOrOfInversions(BinaryOperator &I,

Maybe?

… == c) == (b != c)` (llvm#94915) resolves llvm#92966 alive proof https://alive2.llvm.org/ce/z/bLAQBS

zjaffal requested review from fhahn, rampitec and EugeneZelenko June 9, 2024 20:58

zjaffal requested a review from nikic as a code owner June 9, 2024 20:58

llvmbot added the llvm:transforms label Jun 9, 2024

EugeneZelenko removed their request for review June 9, 2024 21:33

dtcxzyw requested changes Jun 9, 2024

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp Outdated Show resolved Hide resolved

goldsteinn reviewed Jun 9, 2024

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp Outdated Show resolved Hide resolved

pskrgag reviewed Jun 9, 2024

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp Outdated Show resolved Hide resolved

zjaffal mentioned this pull request Jun 12, 2024

[ValueTracking][NFC] move isKnownInversion to ValueTracking #95321

Merged

[InstCombine] fold (a == 1 && b != 0) || (a != 0 && b == 0)) to `(a ==

3ec3b32

0) != (b == 0)`

zjaffal added 2 commits June 19, 2024 08:46

generalise compare

c2b0197

update test cases

371770c

zjaffal force-pushed the instcombine/eq/merge_compare branch from 8ade0f6 to 371770c Compare June 19, 2024 07:48

nikic reviewed Jun 19, 2024

View reviewed changes

simplify the compare

1c0318d

zjaffal changed the title ~~[InstCombine] fold (a == 1 && b != 0) || (a != 0 && b == 0)) to (a ==0) != (b == 0)~~ [InstCombine] fold (a == c && b != c) || (a != c && b == c)) to (a == c) == (b != c) Jun 19, 2024

dtcxzyw requested changes Jun 19, 2024

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp Outdated Show resolved Hide resolved

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp Outdated Show resolved Hide resolved

use correct inversions

17ad22a

goldsteinn reviewed Jun 20, 2024

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp Outdated Show resolved Hide resolved

goldsteinn reviewed Jun 20, 2024

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp Outdated Show resolved Hide resolved

goldsteinn reviewed Jun 20, 2024

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp Outdated Show resolved Hide resolved

nikic reviewed Jun 20, 2024

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp Outdated Show resolved Hide resolved

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp Outdated Show resolved Hide resolved

use inv(cmp2) and update test cases

6b5a689

nikic reviewed Jun 21, 2024

View reviewed changes

address nit comments

9dd61c6

goldsteinn reviewed Jun 21, 2024

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp Outdated Show resolved Hide resolved

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

pre-commit: test PR94915

e901fb1

PR Link: llvm/llvm-project#94915

dtcxzyw mentioned this pull request Jun 22, 2024

pre-commit: test PR94915 dtcxzyw/llvm-opt-benchmark#729

Closed

dtcxzyw reviewed Jun 22, 2024

View reviewed changes

dtcxzyw approved these changes Jun 22, 2024

View reviewed changes

nikic approved these changes Jun 22, 2024

View reviewed changes

address nit

ff02d06

zjaffal merged commit f7fc72e into llvm:main Jun 23, 2024
4 of 5 checks passed

zjaffal deleted the instcombine/eq/merge_compare branch June 23, 2024 09:44

This was referenced Jun 23, 2024

Add check inversion for x and xor x, -1 zjaffal/llvm-project#1

Closed

[ValueTracking] add check inversion for x and xor x, -1 #96425

Open

AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024

[InstCombine] fold (a == c && b != c) || (a != c && b == c)) to `(a…

f8dc56b

… == c) == (b != c)` (llvm#94915) resolves llvm#92966 alive proof https://alive2.llvm.org/ce/z/bLAQBS

dtcxzyw mentioned this pull request Jul 13, 2024

Failure to optimize some related bool comparisons to xor+and #46405

Closed

	static Value *foldAorBConst(BinaryOperator &I,
	static Value *foldOrOfInversions(BinaryOperator &I,

	// Check if any two pairs of the and operations are invertions of each other.
	// Check if any two pairs of the and operations are inversions of each other.

[InstCombine] fold (a == c && b != c) || (a != c && b == c)) to (a == c) == (b != c) #94915

[InstCombine] fold (a == c && b != c) || (a != c && b == c)) to (a == c) == (b != c) #94915

Uh oh!

Conversation

zjaffal commented Jun 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jun 9, 2024

Uh oh!

dtcxzyw commented Jun 9, 2024

Uh oh!

Uh oh!

Uh oh!

pskrgag commented Jun 9, 2024

Uh oh!

Uh oh!

zjaffal commented Jun 19, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pskrgag commented Jun 19, 2024

Uh oh!

nikic commented Jun 19, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dtcxzyw Jun 22, 2024

Choose a reason for hiding this comment

Uh oh!

nikic Jun 22, 2024

Choose a reason for hiding this comment

Uh oh!

dtcxzyw left a comment

Choose a reason for hiding this comment

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

nikic Jun 22, 2024

Choose a reason for hiding this comment

Uh oh!

nikic Jun 22, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

[InstCombine] fold `(a == c && b != c) || (a != c && b == c))` to `(a == c) == (b != c)` #94915

[InstCombine] fold `(a == c && b != c) || (a != c && b == c))` to `(a == c) == (b != c)` #94915

zjaffal commented Jun 9, 2024 •

edited

Loading