[InstCombine] Make (X|C1)^C2 -> X^(C1^C2) iff X&~C1 == 0 work for splat vectors

[craig.topper]

Summary

This also corrects the description to match what was actually implemented. The old comment said X^(C1|C2), but it implemented X^((C1|C2)&~(C1&C2)). I believe ((C1|C2)&~(C1&C2)) is equivalent to (C1^C2).

[spatel]

LGTM.

lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

2393–2394 ↗

(On Diff #110326)

Nit / potential clean-up: I like to make the variable names match the comment names for easier reading when possible, so I'd pick either 'X' or 'V' and stick with it throughout this block.

test/Transforms/InstCombine/select-with-bitwise-ops.ll

271 ↗

(On Diff #110326)

Are we just changing the mask constant in these 3 tests to prove that MaskedValueIsZero() is working as expected? Might be more interesting to use a shift or zext to clear some bits instead?

[craig.topper]

test/Transforms/InstCombine/select-with-bitwise-ops.ll

271 ↗

(On Diff #110326)

These are all tests for select combines.

Test65 tests an and of a larger type with a power of 2 constant that fits in the smaller type. Until ver recently we tried to truncate first then shift. So it only worked for the smaller constant.
Test66 tests an and of a larger type with a power of 2 constant that does not fit in the smaller type. Until recently we punted this transform because we wanted to truncate first. But that would have cut off the constant.
Test67 tests an and of a smaller type with a power of 2 constant.

I don't think these tests were intentionally trying to test the xor combine we have here. It just happens that they trigger it. But the fact that they trigger it and the vector versions didn't is why I even noticed in the first place.

llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

Instruction *InstCombiner::visitXor(BinaryOperator &I) {
   if (match(&I, m_Not(m_OneUse(m_Cmp(Pred, m_Value(), m_Value()))))) {
     cast<CmpInst>(Op0)->setPredicate(CmpInst::getInversePredicate(Pred));
     return replaceInstUsesWith(I, Op0);
   }
 
   {
     const APInt *RHSC;
     if (match(Op1, m_APInt(RHSC))) {
-      Value *V;
+      Value *X;
       const APInt *C;
-      if (match(Op0, m_Sub(m_APInt(C), m_Value(V)))) {
+      if (match(Op0, m_Sub(m_APInt(C), m_Value(X)))) {
         // ~(c-X) == X-c-1 == X+(-c-1)
         if (RHSC->isAllOnesValue()) {
           Constant *NewC = ConstantInt::get(I.getType(), -(*C) - 1);
-          return BinaryOperator::CreateAdd(V, NewC);
+          return BinaryOperator::CreateAdd(X, NewC);
         }
         if (RHSC->isSignMask()) {
           // (C - X) ^ signmask -> (C + signmask - X)
           Constant *NewC = ConstantInt::get(I.getType(), *C + *RHSC);
-          return BinaryOperator::CreateSub(NewC, V);
+          return BinaryOperator::CreateSub(NewC, X);
         }
-      } else if (match(Op0, m_Add(m_Value(V), m_APInt(C)))) {
+      } else if (match(Op0, m_Add(m_Value(X), m_APInt(C)))) {
         // ~(X-c) --> (-c-1)-X
         if (RHSC->isAllOnesValue()) {
           Constant *NewC = ConstantInt::get(I.getType(), -(*C) - 1);
-          return BinaryOperator::CreateSub(NewC, V);
+          return BinaryOperator::CreateSub(NewC, X);
         }
         if (RHSC->isSignMask()) {
           // (X + C) ^ signmask -> (X + C + signmask)
           Constant *NewC = ConstantInt::get(I.getType(), *C + *RHSC);
-          return BinaryOperator::CreateAdd(V, NewC);
+          return BinaryOperator::CreateAdd(X, NewC);
         }
       }
+
+      // (X|C1)^C2 -> X^(C1^C2) iff X&~C1 == 0
+      if (match(Op0, m_Or(m_Value(X), m_APInt(C))) &&
+          MaskedValueIsZero(X, *C, 0, &I)) {
+        Constant *NewC = ConstantInt::get(I.getType(), *C ^ *RHSC);
+        Worklist.Add(cast<Instruction>(Op0));
+        I.setOperand(0, X);
+        I.setOperand(1, NewC);
+        return &I;
+      }
     }
   }
 
   if (ConstantInt *RHSC = dyn_cast<ConstantInt>(Op1)) {
     if (BinaryOperator *Op0I = dyn_cast<BinaryOperator>(Op0)) {
       if (ConstantInt *Op0CI = dyn_cast<ConstantInt>(Op0I->getOperand(1))) {
-        if (Op0I->getOpcode() == Instruction::Or) {
-          // (X|C1)^C2 -> X^(C1|C2) iff X&~C1 == 0
-          if (MaskedValueIsZero(Op0I->getOperand(0), Op0CI->getValue(),
-                                0, &I)) {
-            Constant *NewRHS = ConstantExpr::getOr(Op0CI, RHSC);
-            // Anything in both C1 and C2 is known to be zero, remove it from
-            // NewRHS.
-            Constant *CommonBits = ConstantExpr::getAnd(Op0CI, RHSC);
-            NewRHS = ConstantExpr::getAnd(NewRHS,
-                                       ConstantExpr::getNot(CommonBits));
-            Worklist.Add(Op0I);
-            I.setOperand(0, Op0I->getOperand(0));
-            I.setOperand(1, NewRHS);
-            return &I;
-          }
-        } else if (Op0I->getOpcode() == Instruction::LShr) {
+        if (Op0I->getOpcode() == Instruction::LShr) {
           // ((X^C1) >> C2) ^ C3 -> (X>>C2) ^ ((C1>>C2)^C3)
           // E1 = "X ^ C1"
           BinaryOperator *E1;
           ConstantInt *C1;
           if (Op0I->hasOneUse() &&
               (E1 = dyn_cast<BinaryOperator>(Op0I->getOperand(0))) &&
               E1->getOpcode() == Instruction::Xor &&
               (C1 = dyn_cast<ConstantInt>(E1->getOperand(1)))) {

llvm/trunk/test/Transforms/InstCombine/or.ll

 ;
   %B = lshr i16 %A, 1
   ;; fold or into xor
   %C = or i16 %B, -32768
   %D = xor i16 %C, 8193
   ret i16 %D
 }
 
+define <2 x i16> @test23vec(<2 x i16> %A) {
+; CHECK-LABEL: @test23vec(
+; CHECK-NEXT:    [[B:%.*]] = lshr <2 x i16> [[A:%.*]], <i16 1, i16 1>
+; CHECK-NEXT:    [[D:%.*]] = xor <2 x i16> [[B]], <i16 -24575, i16 -24575>
+; CHECK-NEXT:    ret <2 x i16> [[D]]
+;
+  %B = lshr <2 x i16> %A, <i16 1, i16 1>
+  ;; fold or into xor
+  %C = or <2 x i16> %B, <i16 -32768, i16 -32768>
+  %D = xor <2 x i16> %C, <i16 8193, i16 8193>
+  ret <2 x i16> %D
+}
+
 ; PR1738
 define i1 @test24(double %X, double %Y) {
 ; CHECK-LABEL: @test24(
 ; CHECK-NEXT:    [[TMP1:%.*]] = fcmp uno double %Y, %X
 ; CHECK-NEXT:    ret i1 [[TMP1]]
 ;
   %tmp9 = fcmp uno double %X, 0.000000e+00
   %tmp13 = fcmp uno double %Y, 0.000000e+00

llvm/trunk/test/Transforms/InstCombine/select-with-bitwise-ops.ll

;
   ret i32 %3
 }
 
 define <2 x i32> @test65vec(<2 x i64> %x) {
 ; CHECK-LABEL: @test65vec(
 ; CHECK-NEXT:    [[TMP1:%.*]] = lshr <2 x i64> [[X:%.*]], <i64 3, i64 3>
 ; CHECK-NEXT:    [[TMP2:%.*]] = trunc <2 x i64> [[TMP1]] to <2 x i32>
 ; CHECK-NEXT:    [[TMP3:%.*]] = and <2 x i32> [[TMP2]], <i32 2, i32 2>
-; CHECK-NEXT:    [[TMP4:%.*]] = or <2 x i32> [[TMP3]], <i32 40, i32 40>
-; CHECK-NEXT:    [[TMP5:%.*]] = xor <2 x i32> [[TMP4]], <i32 2, i32 2>
-; CHECK-NEXT:    ret <2 x i32> [[TMP5]]
+; CHECK-NEXT:    [[TMP4:%.*]] = xor <2 x i32> [[TMP3]], <i32 42, i32 42>
+; CHECK-NEXT:    ret <2 x i32> [[TMP4]]
 ;
   %1 = and <2 x i64> %x, <i64 16, i64 16>
   %2 = icmp ne <2 x i64> %1, zeroinitializer
   %3 = select <2 x i1> %2, <2 x i32> <i32 40, i32 40>, <2 x i32> <i32 42, i32 42>
   ret <2 x i32> %3
 }
 
 define i32 @test66(i64 %x) {
;
   ret i32 %3
 }
 
 define <2 x i32> @test66vec(<2 x i64> %x) {
 ; CHECK-LABEL: @test66vec(
 ; CHECK-NEXT:    [[TMP1:%.*]] = lshr <2 x i64> [[X:%.*]], <i64 31, i64 31>
 ; CHECK-NEXT:    [[TMP2:%.*]] = trunc <2 x i64> [[TMP1]] to <2 x i32>
 ; CHECK-NEXT:    [[TMP3:%.*]] = and <2 x i32> [[TMP2]], <i32 2, i32 2>
-; CHECK-NEXT:    [[TMP4:%.*]] = or <2 x i32> [[TMP3]], <i32 40, i32 40>
-; CHECK-NEXT:    [[TMP5:%.*]] = xor <2 x i32> [[TMP4]], <i32 2, i32 2>
-; CHECK-NEXT:    ret <2 x i32> [[TMP5]]
+; CHECK-NEXT:    [[TMP4:%.*]] = xor <2 x i32> [[TMP3]], <i32 42, i32 42>
+; CHECK-NEXT:    ret <2 x i32> [[TMP4]]
 ;
   %1 = and <2 x i64> %x, <i64 4294967296, i64 4294967296>
   %2 = icmp ne <2 x i64> %1, zeroinitializer
   %3 = select <2 x i1> %2, <2 x i32> <i32 40, i32 40>, <2 x i32> <i32 42, i32 42>
   ret <2 x i32> %3
 }
 
 ; Make sure we don't try to optimize a scalar 'and' with a vector select.
;
   %3 = select i1 %2, i32 40, i32 42
   ret i32 %3
 }
 
 define <2 x i32> @test67vec(<2 x i16> %x) {
 ; CHECK-LABEL: @test67vec(
 ; CHECK-NEXT:    [[TMP1:%.*]] = lshr <2 x i16> [[X:%.*]], <i16 1, i16 1>
 ; CHECK-NEXT:    [[TMP2:%.*]] = and <2 x i16> [[TMP1]], <i16 2, i16 2>
-; CHECK-NEXT:    [[TMP3:%.*]] = or <2 x i16> [[TMP2]], <i16 40, i16 40>
-; CHECK-NEXT:    [[TMP4:%.*]] = xor <2 x i16> [[TMP3]], <i16 2, i16 2>
-; CHECK-NEXT:    [[TMP5:%.*]] = zext <2 x i16> [[TMP4]] to <2 x i32>
-; CHECK-NEXT:    ret <2 x i32> [[TMP5]]
+; CHECK-NEXT:    [[TMP3:%.*]] = xor <2 x i16> [[TMP2]], <i16 42, i16 42>
+; CHECK-NEXT:    [[TMP4:%.*]] = zext <2 x i16> [[TMP3]] to <2 x i32>
+; CHECK-NEXT:    ret <2 x i32> [[TMP4]]
 ;
   %1 = and <2 x i16> %x, <i16 4, i16 4>
   %2 = icmp ne <2 x i16> %1, zeroinitializer
   %3 = select <2 x i1> %2, <2 x i32> <i32 40, i32 40>, <2 x i32> <i32 42, i32 42>
   ret <2 x i32> %3
 }
 
 define i32 @test68(i32 %x, i32 %y) {