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] Optimize 'xor-and/or-select' sequence to 'or' for bool #66394

Closed

Conversation

Qi-Hu
Copy link
Contributor

@Qi-Hu Qi-Hu commented Sep 14, 2023

Optimize xor-and/or-select sequence to or : https://alive2.llvm.org/ce/z/52ZT62, https://alive2.llvm.org/ce/z/CHCJQd.

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 14, 2023

@llvm/pr-subscribers-llvm-transforms

Changes Optimize `xor`-`and`-`select` sequence to `or` : https://alive2.llvm.org/ce/z/52ZT62 -- Full diff: https://github.com//pull/66394.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp (+8)
  • (added) llvm/test/Transforms/InstCombine/fold-xor-and-select-i1.ll (+14)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index e2965e7a5703976..8a0ba6f766b4ad1 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -3202,6 +3202,14 @@ Instruction *InstCombinerImpl::foldSelectOfBools(SelectInst &SI) {
     }
   }
 
+  // select (~b & a), a, b -> or a, b
+  // only for scalar types
+  if (match(CondVal, m_And(m_Not(m_Specific(FalseVal)), m_Specific(TrueVal))) &&
+      TrueVal->getType()->isIntegerTy(1) &&
+      FalseVal->getType()->isIntegerTy(1)) {
+    return BinaryOperator::CreateOr(TrueVal, FalseVal);
+  }
+
   return nullptr;
 }
 
diff --git a/llvm/test/Transforms/InstCombine/fold-xor-and-select-i1.ll b/llvm/test/Transforms/InstCombine/fold-xor-and-select-i1.ll
new file mode 100644
index 000000000000000..a5870a3ce9d973f
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/fold-xor-and-select-i1.ll
@@ -0,0 +1,14 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+
+define i1 @max_if(i1 %a, i1 %b) {
+; CHECK-LABEL: define i1 @max_if
+; CHECK-SAME: (i1 [[A:%.*]], i1 [[B:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = or i1 [[A]], [[B]]
+; CHECK-NEXT:    ret i1 [[TMP1]]
+;
+  %1 = xor i1 %b, true
+  %cmp = and i1 %1, %a
+  %2 = select i1 %cmp, i1 %a, i1 %b
+  ret i1 %2
+}

@Qi-Hu Qi-Hu force-pushed the instcombine-optimize-xor-and-select-to-or branch from ab6002f to 09947d0 Compare September 14, 2023 17:59
@Qi-Hu Qi-Hu changed the title [InstCombine] Optimize 'xor-and-select' sequence to 'or' for bool [InstCombine] Optimize 'xor-and/or-select' sequence to 'or' for bool Sep 14, 2023
@@ -3112,6 +3112,26 @@ Instruction *InstCombinerImpl::foldSelectOfBools(SelectInst &SI) {
return SelectInst::Create(FalseVal, One, AndV);
}

// select (~b & a), a, b -> or a, b
// only for scalar types
if (match(CondVal, m_c_And(m_Not(m_Specific(FalseVal)), m_Specific(TrueVal))) &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

This pattern is similar to, but more specific than, the one on line 3109. You should probably move this transform above that one.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Correction: Both your patterns are more specific than the ones between lines 3089 and 3109. Move your code above those.

if (match(CondVal, m_c_And(m_Not(m_Specific(FalseVal)), m_Specific(TrueVal))) &&
TrueVal->getType()->isIntegerTy(1) &&
FalseVal->getType()->isIntegerTy(1) &&
CondVal->getType()->isIntegerTy(1) &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

These scalar type checks are not necessary. When given vector operands, the select instruction will perform the selection element by element.

@@ -0,0 +1,14 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
Copy link
Collaborator

@bryanpkc bryanpkc Sep 15, 2023

Choose a reason for hiding this comment

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

Add your tests into llvm/test/Transforms/InstCombine/select-and-or.ll, instead of creating new files.

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.

Hasn't this already been implemented in https://reviews.llvm.org/D158983?

Please split test additions into a separate commit so the actual impact of the change can be seen. If the only benefit of the additional transform is replacing a logical or with a bitwise or, then I don't think this is needed.

@Qi-Hu
Copy link
Contributor Author

Qi-Hu commented Sep 18, 2023

Hasn't this already been implemented in https://reviews.llvm.org/D158983?

Please split test additions into a separate commit so the actual impact of the change can be seen. If the only benefit of the additional transform is replacing a logical or with a bitwise or, then I don't think this is needed.

Indeed, this is already implemented in D158983. Thanks for pointing it out. I'll close this PR.

@Qi-Hu Qi-Hu closed this Sep 18, 2023
@Qi-Hu Qi-Hu deleted the instcombine-optimize-xor-and-select-to-or branch September 18, 2023 18:09
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

4 participants