[InstCombineSimplifyDemanded] Fold xor into or based on a condition

[karouzakisp]

This patch folds a XOR operation into an OR one, if the conditions are valid.

(X & C1) ^ C2 --> (X & C1) | C2 iff(C1 & C2) == 0

This is always legal.

Alive proof: https://alive2.llvm.org/ce/z/3P2jdP

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Panagiotis Karouzakis (karouzakisp)

Changes

This patch folds a XOR operation into an OR one, if the conditions are valid.

(X & C1) ^ C2 --> (X & C1) | C2 iff(C1 & C2) == 0

This is always legal.

Alive proof: https://alive2.llvm.org/ce/z/3P2jdP

---

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

2 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp (+11)
• (added) llvm/test/Transforms/InstCombine/xor-into-or.ll (+15)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
index 0e3436d12702d..c24e831722a54 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
@@ -335,6 +335,17 @@ Value *InstCombinerImpl::SimplifyDemandedUseBits(Instruction *I,
       return InsertNewInstWith(And, I->getIterator());
     }
 
+    // If all of the demanded bits on one side are known, and all of the set
+    // bits on that side are also known to not be set on the other side, turn
+    // this into an OR, as we know the bits will not affect the result. e.g (X &
+    // C1) ^ C2 --> (X & C1) | C2 iff(C1 & C2) == 0
+    if (DemandedMask.isSubsetOf(RHSKnown.Zero | RHSKnown.One) &&
+        (RHSKnown.One.isSubsetOf(LHSKnown.Zero))) {
+      Instruction *Or =
+          BinaryOperator::CreateOr(I->getOperand(0), I->getOperand(1));
+      return InsertNewInstWith(Or, I->getIterator());
+    }
+
     // If the RHS is a constant, see if we can change it. Don't alter a -1
     // constant because that's a canonical 'not' op, and that is better for
     // combining, SCEV, and codegen.
diff --git a/llvm/test/Transforms/InstCombine/xor-into-or.ll b/llvm/test/Transforms/InstCombine/xor-into-or.ll
new file mode 100644
index 0000000000000..b647941b9645c
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/xor-into-or.ll
@@ -0,0 +1,15 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+
+define i8 @a(i8 %x) nounwind {
+; CHECK-LABEL: @a(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[REM:%.*]] = and i8 [[X:%.*]], -120
+; CHECK-NEXT:    [[XOR:%.*]] = or disjoint i8 [[REM]], 36
+; CHECK-NEXT:    ret i8 [[XOR]]
+;
+entry:
+  %rem = and i8 %x, 136
+  %xor = xor i8 %rem, 36
+  ret i8 %xor
+}

[karouzakisp]

@topperc @jayfoad @dtcxzyw Could you please review? Thanks

[dtcxzyw]

It has been covered by an existing fold in instcombine: https://godbolt.org/z/aoenYGsEG

[dtcxzyw]

I don't mean to reject this patch. It may be valuable as it also takes the demanded bits into account.

Please pre-commit your regression tests. Then we can see the IR diff made by your change. See also https://llvm.org/docs/InstCombineContributorGuide.html#precommit-tests.

[karouzakisp]

I don't mean to reject this patch. It may be valuable as it also takes the demanded bits into account.

Please pre-commit your regression tests. Then we can see the IR diff made by your change. See also https://llvm.org/docs/InstCombineContributorGuide.html#precommit-tests.

This patch was part of a bigger change. When I split the changes into multiple patches, I didn't verify if any transformation occurred on every patch. Seems that the current implementation takes into account every case. I am closing this PR.