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] Loosen one-use restriction if common operand is constant #79767

Closed
wants to merge 2 commits into from

Conversation

AtariDreams
Copy link
Contributor

No description provided.

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 28, 2024

@llvm/pr-subscribers-llvm-transforms

Author: AtariDreams (AtariDreams)

Changes

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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp (+7-11)
  • (modified) llvm/test/Transforms/InstCombine/xor-and-or.ll (+15)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index 5fd944a859ef098..c72f3dfd4c6139e 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -4756,21 +4756,17 @@ Instruction *InstCombinerImpl::visitXor(BinaryOperator &I) {
     return BinaryOperator::CreateNot(Builder.CreateAnd(Op0, B));
 
   // (A | B) ^ (A | C) --> (B ^ C) & ~A -- There are 4 commuted variants.
-  // TODO: Loosen one-use restriction if common operand is a constant.
-  Value *D;
-  if (match(Op0, m_OneUse(m_Or(m_Value(A), m_Value(B)))) &&
-      match(Op1, m_OneUse(m_Or(m_Value(C), m_Value(D))))) {
-    if (B == C || B == D)
-      std::swap(A, B);
-    if (A == C)
-      std::swap(C, D);
-    if (A == D) {
-      Value *NotA = Builder.CreateNot(A);
-      return BinaryOperator::CreateAnd(Builder.CreateXor(B, C), NotA);
+  if (match(Op0, m_c_Or(m_Value(A), m_Value(B))) &&
+      match(Op1, m_c_Or(m_Specific(A), m_Value(C)))) {  
+    // Allow the tranformation to happen if A is one use or a constant.
+    if (A->hasOneUse() || match(A, m_ImmConstant())) {
+      return BinaryOperator::CreateAnd(Builder.CreateXor(B, C),
+                                       Builder.CreateNot(A));
     }
   }
 
   // (A & B) ^ (A | C) --> A ? ~B : C -- There are 4 commuted variants.
+  Value *D;
   if (I.getType()->isIntOrIntVectorTy(1) &&
       match(Op0, m_OneUse(m_LogicalAnd(m_Value(A), m_Value(B)))) &&
       match(Op1, m_OneUse(m_LogicalOr(m_Value(C), m_Value(D))))) {
diff --git a/llvm/test/Transforms/InstCombine/xor-and-or.ll b/llvm/test/Transforms/InstCombine/xor-and-or.ll
index feba47912b1dac5..d2f7ec2b4b924e2 100644
--- a/llvm/test/Transforms/InstCombine/xor-and-or.ll
+++ b/llvm/test/Transforms/InstCombine/xor-and-or.ll
@@ -241,4 +241,19 @@ define i1 @xor_and_or_negative_oneuse(i1 %c, i1 %x, i1 %y) {
   ret i1 %r
 }
 
+define i32 @xor_and_or_constant(i32 %X, i32 %Y, i32 %Z) {
+; CHECK-LABEL: @xor_and_or_constant(
+; CHECK-NEXT:    [[TMP1:%.*]] = or i32 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    [[TMP2:%.*]] = or i32 [[X]], [[Z:%.*]]
+; CHECK-NEXT:    [[TMP3:%.*]] = xor i32 [[TMP1]], [[TMP2]]
+; CHECK-NEXT:    ret i32 [[TMP3]]
+;
+  %1 = or i32 %X, %Y
+  %2 = or i32 %X, %Z
+  %3 = xor i32 %1, %2
+  %4 = xor i32 %X, -1
+  %5 = and i32 %3, %4
+  ret i32 %5
+}
+
 declare void @use(i1)

Copy link

github-actions bot commented Jan 28, 2024

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

dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request Jan 28, 2024
@AtariDreams
Copy link
Contributor Author

@nikic Ready!

%xor = xor i32 %or1, %or2
%add = add i32 %A, %D
ret i32 %add
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Result is the same without your change: https://llvm.godbolt.org/z/debfW4WqY

match(Op1, m_c_Or(m_Specific(A), m_Value(C)))) {
if (match(A, m_ImmConstant())) {
Value *NotA = Builder.CreateNot(A);
return BinaryOperator::CreateAnd(Builder.CreateXor(B, C), NotA);
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not possible to remove one-use entirely. This may replace one instruction with two instructions.

@nikic
Copy link
Contributor

nikic commented Feb 13, 2024

Loosening this one-use restriction does not seem particularly valuable, and is not worth babysitting this PR.

@nikic nikic closed this Feb 13, 2024
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