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

[zjaffal]

following #94915 (comment) we can move the check from there inside isKnownInversion and use the code for foldOrOfInversions to do the simplification

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-llvm-analysis

Author: Zain Jaffal (zjaffal)

Changes

following #94915 (comment) we can move the check from there inside isKnownInversion and use the code for foldOrOfInversions to do the simplification

---

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

3 Files Affected:

• (modified) llvm/lib/Analysis/ValueTracking.cpp (+5)
• (modified) llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp (+7-15)
• (modified) llvm/test/Transforms/InstCombine/and-or-not.ll (+2-2)

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 0df061923f625..603ba5dbfa8ad 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -8180,6 +8180,11 @@ bool llvm::isKnownNegation(const Value *X, const Value *Y, bool NeedNSW,
 }
 
 bool llvm::isKnownInversion(const Value *X, const Value *Y) {
+
+  // Handle X = xor Y, -1 or Y = xor X, -1
+  if (match(X, m_Not(m_Specific(Y))) || match(Y, m_Not(m_Specific(X))))
+    return true;
+
   // Handle X = icmp pred A, B, Y = icmp pred A, C.
   Value *A, *B, *C;
   ICmpInst::Predicate Pred1, Pred2;
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index 19a12343748df..41f32fe4e8335 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -1840,14 +1840,6 @@ static Instruction *foldOrToXor(BinaryOperator &I,
         match(Op1, m_Not(m_c_Or(m_Specific(A), m_Specific(B)))))
       return BinaryOperator::CreateNot(Builder.CreateAnd(A, B));
 
-  // (A & ~B) | (~A & B) --> A ^ B
-  // (A & ~B) | (B & ~A) --> A ^ B
-  // (~B & A) | (~A & B) --> A ^ B
-  // (~B & A) | (B & ~A) --> A ^ B
-  if (match(Op0, m_c_And(m_Value(A), m_Not(m_Value(B)))) &&
-      match(Op1, m_c_And(m_Not(m_Specific(A)), m_Specific(B))))
-    return BinaryOperator::CreateXor(A, B);
-
   return nullptr;
 }
 
@@ -3426,16 +3418,16 @@ static Value *foldOrOfInversions(BinaryOperator &I,
   assert(I.getOpcode() == Instruction::Or &&
          "Simplification only supports or at the moment.");
 
-  Value *Cmp1, *Cmp2, *Cmp3, *Cmp4;
-  if (!match(I.getOperand(0), m_And(m_Value(Cmp1), m_Value(Cmp2))) ||
-      !match(I.getOperand(1), m_And(m_Value(Cmp3), m_Value(Cmp4))))
+  Value *A, *B, *C, *D;
+  if (!match(I.getOperand(0), m_And(m_Value(A), m_Value(B))) ||
+      !match(I.getOperand(1), m_And(m_Value(C), m_Value(D))))
     return nullptr;
 
   // Check if any two pairs of the and operations are inversions of each other.
-  if (isKnownInversion(Cmp1, Cmp3) && isKnownInversion(Cmp2, Cmp4))
-    return Builder.CreateXor(Cmp1, Cmp4);
-  if (isKnownInversion(Cmp1, Cmp4) && isKnownInversion(Cmp2, Cmp3))
-    return Builder.CreateXor(Cmp1, Cmp3);
+  if (isKnownInversion(A, C) && isKnownInversion(B, D))
+    return Builder.CreateXor(A, D);
+  if (isKnownInversion(A, D) && isKnownInversion(B, C))
+    return Builder.CreateXor(A, C);
 
   return nullptr;
 }
diff --git a/llvm/test/Transforms/InstCombine/and-or-not.ll b/llvm/test/Transforms/InstCombine/and-or-not.ll
index 2e351c30ea1f7..ba5a8c7a78439 100644
--- a/llvm/test/Transforms/InstCombine/and-or-not.ll
+++ b/llvm/test/Transforms/InstCombine/and-or-not.ll
@@ -200,7 +200,7 @@ define i32 @or_to_xor3(float %fa, float %fb) {
 ; CHECK-LABEL: @or_to_xor3(
 ; CHECK-NEXT:    [[A:%.*]] = fptosi float [[FA:%.*]] to i32
 ; CHECK-NEXT:    [[B:%.*]] = fptosi float [[FB:%.*]] to i32
-; CHECK-NEXT:    [[OR:%.*]] = xor i32 [[B]], [[A]]
+; CHECK-NEXT:    [[OR:%.*]] = xor i32 [[A]], [[B]]
 ; CHECK-NEXT:    ret i32 [[OR]]
 ;
   %a = fptosi float %fa to i32
@@ -219,7 +219,7 @@ define i32 @or_to_xor4(float %fa, float %fb) {
 ; CHECK-LABEL: @or_to_xor4(
 ; CHECK-NEXT:    [[A:%.*]] = fptosi float [[FA:%.*]] to i32
 ; CHECK-NEXT:    [[B:%.*]] = fptosi float [[FB:%.*]] to i32
-; CHECK-NEXT:    [[OR:%.*]] = xor i32 [[B]], [[A]]
+; CHECK-NEXT:    [[OR:%.*]] = xor i32 [[A]], [[B]]
 ; CHECK-NEXT:    ret i32 [[OR]]
 ;
   %a = fptosi float %fa to i32

[dtcxzyw]

Can you pre-commit this test case?

#93591 (comment)

[nikic]

Can you pre-commit this test case?

#93591 (comment)

We should also have that test case with vectors containing poison elements. I think we need to match m_Not without poison here to preserve the currently existing property that both arguments are equally poisonous.

[zjaffal]

Can you pre-commit this test case?

#93591 (comment)

Do you want me to add the test case to this pr?

[dtcxzyw]

Can you pre-commit this test case?
#93591 (comment)

Do you want me to add the test case to this pr?

See https://llvm.org/docs/InstCombineContributorGuide.html#precommit-tests