[InstCombine] Support (X | C1) & C2 --> (X & C2^(C1&C2)) | (C1&C2) for vector splats

[craig.topper]

Summary

Note the original code I deleted incorrectly listed this as (X | C1) & C2 --> (X & C2^(C1&C2)) | C1 Which is only valid if C1 is a subset of C2. This relied on SimplifyDemandedBits to remove any extra bits from C1 before we got to that code.

My new implementation avoids relying on that behavior so that it can be naively verified with alive.

[craig.topper]

test/Transforms/InstCombine/or.ll

284 ↗

(On Diff #109944)

There's obviously still something missing here that keeps this from matching the non vector case.

[davide]

test/Transforms/InstCombine/or.ll

271–281 ↗

(On Diff #109944)

Why removing this test? cheers.

[craig.topper]

Fixed diff of tests. Somehow it was relative to an unsquashed commit that added test30vec with the wrong type or something.

[spatel]

LGTM.

lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

1222 ↗

(On Diff #109945)

It's not unique to this transform, but I've wondered: are these calls completely eliminated in release builds?

[craig.topper]

lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

1222 ↗

(On Diff #109945)

I highly doubt it because release builds should still support tracking names. It's only disabled because the clang driver passes a flag to the cc1 command that disables the name tracking.

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

Instruction *InstCombiner::OptAndOp(BinaryOperator *Op,
                                     BinaryOperator &TheAnd) {
   Value *X = Op->getOperand(0);
   Constant *Together = nullptr;
   if (!Op->isShift())
     Together = ConstantExpr::getAnd(AndRHS, OpRHS);
 
   switch (Op->getOpcode()) {
   default: break;
-  case Instruction::Or:
-    if (Op->hasOneUse()){
-      ConstantInt *TogetherCI = dyn_cast<ConstantInt>(Together);
-      if (TogetherCI && !TogetherCI->isZero()){
-        // (X | C1) & C2 --> (X & (C2^(C1&C2))) | C1
-        // NOTE: This reduces the number of bits set in the & mask, which
-        // can expose opportunities for store narrowing.
-        Together = ConstantExpr::getXor(AndRHS, Together);
-        Value *And = Builder.CreateAnd(X, Together);
-        And->takeName(Op);
-        return BinaryOperator::CreateOr(And, OpRHS);
-      }
-    }
-
-    break;
   case Instruction::Add:
     if (Op->hasOneUse()) {
       // Adding a one to a single bit bit-field should be turned into an XOR
       // of the bit.  First thing to check is to see if this AND is with a
       // single bit constant.
       const APInt &AndRHSV = AndRHS->getValue();
 
       // If there is only one bit set.
if (match(Op1, m_APInt(C))) {
     if (match(Op0, m_OneUse(m_Xor(m_Value(X), m_APInt(XorC))))) {
       // (X ^ C1) & C2 --> (X & C2) ^ (C1&C2)
       Constant *NewC = ConstantInt::get(I.getType(), *C & *XorC);
       Value *And = Builder.CreateAnd(X, Op1);
       And->takeName(Op0);
       return BinaryOperator::CreateXor(And, NewC);
     }
 
+    const APInt *OrC;
+    if (match(Op0, m_OneUse(m_Or(m_Value(X), m_APInt(OrC))))) {
+      // (X | C1) & C2 --> (X & C2^(C1&C2)) | (C1&C2)
+      // NOTE: This reduces the number of bits set in the & mask, which
+      // can expose opportunities for store narrowing for scalars.
+      // NOTE: SimplifyDemandedBits should have already removed bits from C1
+      // that aren't set in C2. Meaning we can replace (C1&C2) with C1 in
+      // above, but this feels safer.
+      APInt Together = *C & *OrC;
+      Value *And = Builder.CreateAnd(X, ConstantInt::get(I.getType(),
+                                                         Together ^ *C));
+      And->takeName(Op0);
+      return BinaryOperator::CreateOr(And, ConstantInt::get(I.getType(),
+                                                            Together));
+    }
+
     // If the mask is only needed on one incoming arm, push the 'and' op up.
     if (match(Op0, m_OneUse(m_Xor(m_Value(X), m_Value(Y)))) ||
         match(Op0, m_OneUse(m_Or(m_Value(X), m_Value(Y))))) {
       APInt NotAndMask(~(*C));
       BinaryOperator::BinaryOps BinOp = cast<BinaryOperator>(Op0)->getOpcode();
       if (MaskedValueIsZero(X, NotAndMask, 0, &I)) {
         // Not masking anything out for the LHS, move mask to RHS.
         // and ({x}or X, Y), C --> {x}or X, (and Y, C)

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

 ;
   %B = or i32 %A, 32962
   %C = and i32 %A, -65536
   %D = and i32 %B, 40186
   %E = or i32 %D, %C
   ret i32 %E
 }
 
+define <2 x i32> @test30vec(<2 x i32> %A) {
+; CHECK-LABEL: @test30vec(
+; CHECK-NEXT:    [[C:%.*]] = and <2 x i32> [[A:%.*]], <i32 -65536, i32 -65536>
+; CHECK-NEXT:    [[B:%.*]] = and <2 x i32> [[A]], <i32 7224, i32 7224>
+; CHECK-NEXT:    [[D:%.*]] = or <2 x i32> [[B]], <i32 32962, i32 32962>
+; CHECK-NEXT:    [[E:%.*]] = or <2 x i32> [[D]], [[C]]
+; CHECK-NEXT:    ret <2 x i32> [[E]]
+;
+  %B = or <2 x i32> %A, <i32 32962, i32 32962>
+  %C = and <2 x i32> %A, <i32 -65536, i32 -65536>
+  %D = and <2 x i32> %B, <i32 40186, i32 40186>
+  %E = or <2 x i32> %D, %C
+  ret <2 x i32> %E
+}
+
 ; PR4216
 define i64 @test31(i64 %A) {
 ; CHECK-LABEL: @test31(
 ; CHECK-NEXT:    [[E:%.*]] = and i64 %A, 4294908984
 ; CHECK-NEXT:    [[F:%.*]] = or i64 [[E]], 32962
 ; CHECK-NEXT:    ret i64 [[F]]
 ;
   %B = or i64 %A, 194
   %D = and i64 %B, 250
 
   %C = or i64 %A, 32768
   %E = and i64 %C, 4294941696
 
   %F = or i64 %D, %E
   ret i64 %F
 }
 
+define <2 x i64> @test31vec(<2 x i64> %A) {
+; CHECK-LABEL: @test31vec(
+; CHECK-NEXT:    [[E:%.*]] = and <2 x i64> [[A:%.*]], <i64 4294908984, i64 4294908984>
+; CHECK-NEXT:    [[F:%.*]] = or <2 x i64> [[E]], <i64 32962, i64 32962>
+; CHECK-NEXT:    ret <2 x i64> [[F]]
+;
+  %B = or <2 x i64> %A, <i64 194, i64 194>
+  %D = and <2 x i64> %B, <i64 250, i64 250>
+
+  %C = or <2 x i64> %A, <i64 32768, i64 32768>
+  %E = and <2 x i64> %C, <i64 4294941696, i64 4294941696>
+
+  %F = or <2 x i64> %D, %E
+  ret <2 x i64> %F
+}
+
 ; codegen is mature enough to handle vector selects.
 define <4 x i32> @test32(<4 x i1> %and.i1352, <4 x i32> %vecinit6.i176, <4 x i32> %vecinit6.i191) {
 ; CHECK-LABEL: @test32(
 ; CHECK-NEXT:    [[OR_I:%.*]] = select <4 x i1> %and.i1352, <4 x i32> %vecinit6.i176, <4 x i32> %vecinit6.i191
 ; CHECK-NEXT:    ret <4 x i32> [[OR_I]]
 ;
   %and.i135 = sext <4 x i1> %and.i1352 to <4 x i32>
   %and.i129 = and <4 x i32> %vecinit6.i176, %and.i135