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 (select (icmp eq (and x, y), 0), (add/xor x, y), F) #121841

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

Conversation

goldsteinn
Copy link
Contributor

@goldsteinn goldsteinn commented Jan 6, 2025

  • [InstCombine] Add tests for folding (select (icmp eq (and x, y), 0), (add/xor x, y), F); NFC
  • [InstCombine] Fold (select (icmp eq (and x, y), 0), (add/xor x, y), F)

In the context of (icmp eq (and x, y), 0) we can replace add/xor
with or:
https://alive2.llvm.org/ce/z/R7CQp3

@llvmbot
Copy link
Member

llvmbot commented Jan 6, 2025

@llvm/pr-subscribers-llvm-transforms

Author: None (goldsteinn)

Changes
  • [InstCombine] Add tests for folding (select (icmp eq (and x, y), 0), (add/xor x, y), F); NFC
  • [InstCombine] Fold (select (icmp eq (and x, y), 0), (add/xor x, y), F)

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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp (+26)
  • (modified) llvm/test/Transforms/InstCombine/select.ll (+39-6)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index 7fd91c72a2fb0e..5ec98d937327d3 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -4395,5 +4395,31 @@ Instruction *InstCombinerImpl::visitSelectInst(SelectInst &SI) {
       return replaceOperand(SI, 2, ConstantInt::get(FalseVal->getType(), 0));
   }
 
+  {
+    // (select (icmp eq (and X, Y), 0), (add/xor X, Y), F)
+    //	-> (select (icmp eq (and X, Y), 0), (or X, Y), F)
+    // And vice versa for `ne` pred.
+    CmpPredicate Pred;
+    Value *X, *Y;
+    if (match(CondVal, m_ICmp(Pred, m_And(m_Value(X), m_Value(Y)), m_Zero())) &&
+        ICmpInst::isEquality(Pred)) {
+      unsigned RepIdx;
+      Value *RepArm;
+      if (Pred == CmpInst::ICMP_EQ) {
+        RepIdx = 1;
+        RepArm = TrueVal;
+      } else {
+        RepIdx = 2;
+        RepArm = FalseVal;
+      }
+      auto ReduceToOrNoCommonBits =
+          m_CombineOr(m_c_Xor(m_Specific(X), m_Specific(Y)),
+                      m_c_Add(m_Specific(X), m_Specific(Y)));
+
+      if (RepArm->hasOneUse() && match(RepArm, ReduceToOrNoCommonBits))
+        return replaceOperand(SI, RepIdx, Builder.CreateOr(X, Y));
+    }
+  }
+
   return nullptr;
 }
diff --git a/llvm/test/Transforms/InstCombine/select.ll b/llvm/test/Transforms/InstCombine/select.ll
index 9de3c2483ba49c..267306de1f791c 100644
--- a/llvm/test/Transforms/InstCombine/select.ll
+++ b/llvm/test/Transforms/InstCombine/select.ll
@@ -5,6 +5,9 @@
 
 target datalayout = "e-p:64:64-p1:16:16-p2:32:32:32-p3:64:64:64"
 
+declare void @use.i1(i1)
+declare void @use.i32(i32)
+
 define i1 @test5(i1 %C) {
 ; CHECK-LABEL: @test5(
 ; CHECK-NEXT:    [[NOT_C:%.*]] = xor i1 [[C:%.*]], true
@@ -3784,12 +3787,8 @@ entry:
 define i32 @src_and_eq_0_xor_or(i32 %x, i32 %y) {
 ; CHECK-LABEL: @src_and_eq_0_xor_or(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[AND:%.*]] = and i32 [[Y:%.*]], [[X:%.*]]
-; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[AND]], 0
-; CHECK-NEXT:    [[XOR:%.*]] = xor i32 [[Y]], [[X]]
-; CHECK-NEXT:    [[OR:%.*]] = or i32 [[Y]], [[X]]
-; CHECK-NEXT:    [[COND:%.*]] = select i1 [[CMP]], i32 [[XOR]], i32 [[OR]]
-; CHECK-NEXT:    ret i32 [[COND]]
+; CHECK-NEXT:    [[OR:%.*]] = or i32 [[Y:%.*]], [[X:%.*]]
+; CHECK-NEXT:    ret i32 [[OR]]
 ;
 entry:
   %and = and i32 %y, %x
@@ -4425,17 +4424,51 @@ define i32 @src_no_trans_select_and_eq0_xor_and(i32 %x, i32 %y) {
 ; CHECK-LABEL: @src_no_trans_select_and_eq0_xor_and(
 ; CHECK-NEXT:    [[AND:%.*]] = and i32 [[X:%.*]], [[Y:%.*]]
 ; CHECK-NEXT:    [[AND0:%.*]] = icmp eq i32 [[AND]], 0
+; CHECK-NEXT:    [[XOR:%.*]] = or i32 [[X]], [[Y]]
+; CHECK-NEXT:    [[COND:%.*]] = select i1 [[AND0]], i32 [[XOR]], i32 [[AND]]
+; CHECK-NEXT:    ret i32 [[COND]]
+;
+  %and = and i32 %x, %y
+  %and0 = icmp eq i32 %and, 0
+  %xor = xor i32 %x, %y
+  %cond = select i1 %and0, i32 %xor, i32 %and
+  ret i32 %cond
+}
+
+define i32 @src_no_trans_select_and_eq0_xor_and_fail_multiuse(i32 %x, i32 %y) {
+; CHECK-LABEL: @src_no_trans_select_and_eq0_xor_and_fail_multiuse(
+; CHECK-NEXT:    [[AND:%.*]] = and i32 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    [[AND0:%.*]] = icmp eq i32 [[AND]], 0
 ; CHECK-NEXT:    [[XOR:%.*]] = xor i32 [[X]], [[Y]]
+; CHECK-NEXT:    call void @use.i32(i32 [[XOR]])
 ; CHECK-NEXT:    [[COND:%.*]] = select i1 [[AND0]], i32 [[XOR]], i32 [[AND]]
 ; CHECK-NEXT:    ret i32 [[COND]]
 ;
   %and = and i32 %x, %y
   %and0 = icmp eq i32 %and, 0
   %xor = xor i32 %x, %y
+  call void @use.i32(i32 %xor)
   %cond = select i1 %and0, i32 %xor, i32 %and
   ret i32 %cond
 }
 
+define i32 @src_no_trans_select_and_eq0_add(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: @src_no_trans_select_and_eq0_add(
+; CHECK-NEXT:    [[AND:%.*]] = and i32 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    [[AND0:%.*]] = icmp ne i32 [[AND]], 0
+; CHECK-NEXT:    call void @use.i1(i1 [[AND0]])
+; CHECK-NEXT:    [[ADD:%.*]] = or i32 [[X]], [[Y]]
+; CHECK-NEXT:    [[COND:%.*]] = select i1 [[AND0]], i32 [[Z:%.*]], i32 [[ADD]]
+; CHECK-NEXT:    ret i32 [[COND]]
+;
+  %and = and i32 %x, %y
+  %and0 = icmp ne i32 %and, 0
+  call void @use.i1(i1 %and0)
+  %add = add i32 %x, %y
+  %cond = select i1 %and0, i32 %z, i32 %add
+  ret i32 %cond
+}
+
 define i32 @src_no_trans_select_or_eq0_or_and(i32 %x, i32 %y) {
 ; CHECK-LABEL: @src_no_trans_select_or_eq0_or_and(
 ; CHECK-NEXT:    [[OR:%.*]] = or i32 [[X:%.*]], [[Y:%.*]]

@goldsteinn goldsteinn changed the title goldsteinn/select and eq 0 [InstCombine] Fold (select (icmp eq (and x, y), 0), (add/xor x, y), F) Jan 6, 2025
@goldsteinn goldsteinn requested a review from dtcxzyw January 6, 2025 21:31
@dtcxzyw
Copy link
Member

dtcxzyw commented Jan 7, 2025

What is the motivation of this patch?
Is it a replacement of #73362?

@goldsteinn
Copy link
Contributor Author

What is the motivation of this patch? Is it a replacement of #73362?

Yeah, partially handles some of the cases from that

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.

3 participants