Instcombine (A|B) ^(A^B) to A&B

[karthikthecool]

Summary

Hi All,
This patch implements transform for pattern (A|B) ^(A^B) to A&B.

Please find the correctness proof of the transform using CVC3 -

$ cat t.cvc
A, B : BITVECTOR(32);
QUERY BVXOR(A | B, BVXOR(A,B) ) = A & B;

$ cvc3 t.cvc
Valid.

Please let me know if this is good to commit.

Thanks
Karthik Bhat

[majnemer]

I believe the following is a simpler, but equivalent, wording of this transform.

--- a/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -2508,6 +2508,18 @@ Instruction *InstCombiner::visitXor(BinaryOperator &I) {
       if ((A == C && B == D) || (A == D && B == C))
         return BinaryOperator::CreateXor(A, B);
     }
+    // (A ^ B)^(A | B) -> A & B
+    if (match(Op0I, m_Xor(m_Value(A), m_Value(B))) &&
+        match(Op1I, m_Or(m_Value(C), m_Value(D)))) {
+      if ((A == C && B == D) || (A == D && B == C))
+        return BinaryOperator::CreateAnd(A, B);
+    }
+    // (A | B)^(A ^ B) -> A & B
+    if (match(Op0I, m_Or(m_Value(A), m_Value(B))) &&
+        match(Op1I, m_Xor(m_Value(C), m_Value(D)))) {
+      if ((A == C && B == D) || (A == D && B == C))
+        return BinaryOperator::CreateAnd(A, B);
+    }
     // (A & B) ^ (A ^ B) -> (A | B)
     if (match(Op0I, m_And(m_Value(A), m_Value(B))) &&
         match(Op1I, m_Xor(m_Specific(A), m_Specific(B))))

lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

2451–2459

I think this would be simpler as:

} else if (match(Op0, m_Xor(m_Specific(A), m_Specific(B)))) {
  I.swapOperands();    // (A^B)^(A|B) == (A|B)^(A^B)
  std::swap(Op0, Op1); // Simplified below.
} else if (match(Op0, m_Xor(m_Specific(B), m_Specific(A)))) {
  Op1I->swapOperands(); // (A^B)^(B|A) == (A|B)^(A^B)
  I.swapOperands();     // Simplied below.
  std::swap(Op0, Op1);

You would avoid needing A1 and B1 *and* you could avoid hoisting Op0I.

2483–2484

This code shouldn't have been changed. Op0I is already checked for null.

test/Transforms/InstCombine/or-xor.ll

140

Kill the reference to #0.

[karthikthecool]

Hi David,
Thanks for the review. Yes the newer version is definetly more simple and correct place to do this transform.
Updated the patch as per review comments.

Thanks
Karthik Bhat

[majnemer]

LGTM with comments addressed.

test/Transforms/InstCombine/or-xor.ll

141–147

All the other tests use names for their instructions, can you please do the same?

[karthikthecool]

Thanks David. Comitted as r215524 after test case modifications.

lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

if (Op1I) {
     if (match(Op1I, m_Or(m_Value(A), m_Value(B)))) {
       if (A == Op0) {              // B^(B|A) == (A|B)^B
         Op1I->swapOperands();
         I.swapOperands();
         std::swap(Op0, Op1);
       } else if (B == Op0) {       // B^(A|B) == (A|B)^B
         I.swapOperands();     // Simplified below.
         std::swap(Op0, Op1);
       }
     } else if (match(Op1I, m_And(m_Value(A), m_Value(B))) &&
                Op1I->hasOneUse()){
       if (A == Op0) {                                      // A^(A&B) -> A^(B&A)
         Op1I->swapOperands();
         std::swap(A, B);
       }
       if (B == Op0) {                                      // A^(B&A) -> (B&A)^A
         I.swapOperands();     // Simplified below.
         std::swap(Op0, Op1);
       }
     }
   }
 
   BinaryOperator *Op0I = dyn_cast<BinaryOperator>(Op0);
   if (Op0I) {
     Value *A, *B;
     if (match(Op0I, m_Or(m_Value(A), m_Value(B))) &&
         Op0I->hasOneUse()) {
       if (A == Op1)                                  // (B|A)^B == (A|B)^B
         std::swap(A, B);
       if (B == Op1)                                  // (A|B)^B == A & ~B
         return BinaryOperator::CreateAnd(A, Builder->CreateNot(Op1));
     } else if (match(Op0I, m_And(m_Value(A), m_Value(B))) &&
                Op0I->hasOneUse()){
       if (A == Op1)                                        // (A&B)^A -> (B&A)^A
         std::swap(A, B);
       if (B == Op1 &&                                      // (B&A)^A == ~B & A
           !isa<ConstantInt>(Op1)) {  // Canonical form is (B&C)^C
         return BinaryOperator::CreateAnd(Builder->CreateNot(A), Op1);
       }
     }
   }
 
   // (X >> Z) ^ (Y >> Z)  -> (X^Y) >> Z  for all shifts.
   if (Op0I && Op1I && Op0I->isShift() &&
       Op0I->getOpcode() == Op1I->getOpcode() &&
       Op0I->getOperand(1) == Op1I->getOperand(1) &&
       (Op0I->hasOneUse() || Op1I->hasOneUse())) {
     Value *NewOp =
       Builder->CreateXor(Op0I->getOperand(0), Op1I->getOperand(0),
                          Op0I->getName());
if (match(Op0I, m_And(m_Value(A), m_Value(B))) &&
         return BinaryOperator::CreateXor(A, B);
     }
     // (A | B)^(A & B) -> A ^ B
     if (match(Op0I, m_Or(m_Value(A), m_Value(B))) &&
         match(Op1I, m_And(m_Value(C), m_Value(D)))) {
       if ((A == C && B == D) || (A == D && B == C))
         return BinaryOperator::CreateXor(A, B);
     }
+    // (A ^ B)^(A | B) -> A & B
+    if (match(Op0I, m_Xor(m_Value(A), m_Value(B))) &&
+        match(Op1I, m_Or(m_Value(C), m_Value(D)))) {
+      if ((A == C && B == D) || (A == D && B == C))
+        return BinaryOperator::CreateAnd(A, B);
+    }
+    // (A | B)^(A ^ B) -> A & B
+    if (match(Op0I, m_Or(m_Value(A), m_Value(B))) &&
+        match(Op1I, m_Xor(m_Value(C), m_Value(D)))) {
+      if ((A == C && B == D) || (A == D && B == C))
+        return BinaryOperator::CreateAnd(A, B);
+    }
     // (A & B) ^ (A ^ B) -> (A | B)
     if (match(Op0I, m_And(m_Value(A), m_Value(B))) &&
         match(Op1I, m_Xor(m_Specific(A), m_Specific(B))))
       return BinaryOperator::CreateOr(A, B);
     // (A ^ B) ^ (A & B) -> (A | B)
     if (match(Op0I, m_Xor(m_Value(A), m_Value(B))) &&
         match(Op1I, m_And(m_Specific(A), m_Specific(B))))
       return BinaryOperator::CreateOr(A, B);

test/Transforms/InstCombine/or-xor.ll

  %xor = xor i32 %neg, %y
  %or = or i32 %x, %y
  %and = and i32 %xor, %or
  ret i32 %and
 ; CHECK-LABEL: @test13(
 ; CHECK-NEXT: %and = and i32 %x, %y
 ; CHECK-NEXT: ret i32 %and
 }
+
+define i32 @test15(i32 %x, i32 %y) {
+  %1 = xor i32 %y, %x
+  %2 = or i32 %y, %x
+  %3 = xor i32 %2, %1
+  ret i32 %3
+; CHECK-LABEL: @test15(
+; CHECK-NEXT: %1 = and i32 %y, %x
+; CHECK-NEXT: ret i32 %1
+}