[DAGCombiner] isAllOnesConstantOrAllOnesSplatConstant(): look through bitcasts

[lebedev.ri]

Summary

As pointed out in D46528, we errneously transform cases like xor X, -1,
even though we use said function.
It's because the -1 is actually a bitcast there.
So i think we can just look through it in the function.

[lebedev.ri]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

916 ↗

(On Diff #147841)

The comment next to the ISD::BITCAST declaration notes:

/// This operator is subtly different from the bitcast instruction from
/// LLVM-IR since this node may change the bits in the register. For
/// example, this occurs on big-endian NEON and big-endian MSA where the
/// layout of the bits in the register depends on the vector type and this
/// operator acts as a shuffle operation for some vector type combinations.
BITCAST,

so i'm not too sure this is that simple,
but currently also i don't see an alternative solution.

[RKSimon]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

917 ↗

(On Diff #147841)

Don't make it recursive:

while (N.getOpcode() == ISD::BITCAST)
  N = N->getOperand(0);

[lebedev.ri]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

917 ↗

(On Diff #147841)

Oh, i can do that?
I was under impression that would change the the N itself (in IR/DAG), not just the variable N here.

[lebedev.ri]

No recursion.

[RKSimon]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

917 ↗

(On Diff #147841)

No, it;s fine - see ISD::isBuildVectorAllZeros - it might have been a problem if you weren't just interested in ALL bits being set.

[spatel]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

916 ↗

(On Diff #147847)

Use peekThroughBitcast()

Should we do the same for isOneConstantOrOneSplatConstant and isNullConstantOrNullSplatConstant?

[RKSimon]

LGTM

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

916 ↗

(On Diff #147847)

Let's leave it until there is a test case - you can add TODO comments if you wish.

llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

 /// it is profitable to do so.
 bool DAGCombiner::isOneUseSetCC(SDValue N) const {
   SDValue N0, N1, N2;
   if (isSetCCEquivalent(N, N0, N1, N2) && N.getNode()->hasOneUse())
     return true;
   return false;
 }
 
+static SDValue peekThroughBitcast(SDValue V) {
+  while (V.getOpcode() == ISD::BITCAST)
+    V = V.getOperand(0);
+  return V;
+}
+
 // Returns the SDNode if it is a constant float BuildVector
 // or constant float.
 static SDNode *isConstantFPBuildVectorOrConstantFP(SDValue N) {
   if (isa<ConstantFPSDNode>(N))
     return N.getNode();
   if (ISD::isBuildVectorOfConstantFPSDNodes(N.getNode()))
     return N.getNode();
   return nullptr;
static bool isConstantOrConstantVector(SDValue N, bool NoOpaques = false) {
   }
   return true;
 }
 
 // Determines if it is a constant null integer or a splatted vector of a
 // constant null integer (with no undefs).
 // Build vector implicit truncation is not an issue for null values.
 static bool isNullConstantOrNullSplatConstant(SDValue N) {
+  // TODO: may want to use peekThroughBitcast() here.
   if (ConstantSDNode *Splat = isConstOrConstSplat(N))
     return Splat->isNullValue();
   return false;
 }
 
 // Determines if it is a constant integer of one or a splatted vector of a
 // constant integer of one (with no undefs).
 // Do not permit build vector implicit truncation.
 static bool isOneConstantOrOneSplatConstant(SDValue N) {
+  // TODO: may want to use peekThroughBitcast() here.
   unsigned BitWidth = N.getScalarValueSizeInBits();
   if (ConstantSDNode *Splat = isConstOrConstSplat(N))
     return Splat->isOne() && Splat->getAPIntValue().getBitWidth() == BitWidth;
   return false;
 }
 
 // Determines if it is a constant integer of all ones or a splatted vector of a
 // constant integer of all ones (with no undefs).
 // Do not permit build vector implicit truncation.
 static bool isAllOnesConstantOrAllOnesSplatConstant(SDValue N) {
+  N = peekThroughBitcast(N);
   unsigned BitWidth = N.getScalarValueSizeInBits();
   if (ConstantSDNode *Splat = isConstOrConstSplat(N))
     return Splat->isAllOnesValue() &&
            Splat->getAPIntValue().getBitWidth() == BitWidth;
   return false;
 }
 
 // Determines if a BUILD_VECTOR is composed of all-constants possibly mixed with
if (Use->getOpcode() == ISD::MUL) { // We have another multiply use.
         return true;
     }
   }
 
   // Didn't find a case where this would be profitable.
   return false;
 }
 
-static SDValue peekThroughBitcast(SDValue V) {
-  while (V.getOpcode() == ISD::BITCAST)
-    V = V.getOperand(0);
-  return V;
-}
-
 SDValue DAGCombiner::getMergeStoreChains(SmallVectorImpl<MemOpLink> &StoreNodes,
                                          unsigned NumStores) {
   SmallVector<SDValue, 8> Chains;
   SmallPtrSet<const SDNode *, 8> Visited;
   SDLoc StoreDL(StoreNodes[0].MemNode);
 
   for (unsigned i = 0; i < NumStores; ++i) {
     Visited.insert(StoreNodes[i].MemNode);

llvm/trunk/test/CodeGen/X86/unfold-masked-merge-vector-variablemask-const.ll

 ; CHECK-SSE1-NEXT:    andnps (%rcx), %xmm0
 ; CHECK-SSE1-NEXT:    xorps {{.*}}(%rip), %xmm0
 ; CHECK-SSE1-NEXT:    movaps %xmm0, (%rdi)
 ; CHECK-SSE1-NEXT:    movq %rdi, %rax
 ; CHECK-SSE1-NEXT:    retq
 ;
 ; CHECK-SSE2-LABEL: in_constant_varx_mone:
 ; CHECK-SSE2:       # %bb.0:
-; CHECK-SSE2-NEXT:    movdqa (%rdx), %xmm0
+; CHECK-SSE2-NEXT:    movdqa (%rdi), %xmm0
 ; CHECK-SSE2-NEXT:    pcmpeqd %xmm1, %xmm1
-; CHECK-SSE2-NEXT:    pxor %xmm0, %xmm1
-; CHECK-SSE2-NEXT:    pand (%rdi), %xmm0
-; CHECK-SSE2-NEXT:    por %xmm1, %xmm0
+; CHECK-SSE2-NEXT:    pandn (%rdx), %xmm0
+; CHECK-SSE2-NEXT:    pxor %xmm1, %xmm0
 ; CHECK-SSE2-NEXT:    retq
 ;
 ; CHECK-XOP-LABEL: in_constant_varx_mone:
 ; CHECK-XOP:       # %bb.0:
-; CHECK-XOP-NEXT:    vmovdqa (%rdx), %xmm0
+; CHECK-XOP-NEXT:    vmovdqa (%rdi), %xmm0
 ; CHECK-XOP-NEXT:    vpcmpeqd %xmm1, %xmm1, %xmm1
-; CHECK-XOP-NEXT:    vpxor %xmm1, %xmm0, %xmm1
-; CHECK-XOP-NEXT:    vpand (%rdi), %xmm0, %xmm0
-; CHECK-XOP-NEXT:    vpor %xmm1, %xmm0, %xmm0
+; CHECK-XOP-NEXT:    vpandn (%rdx), %xmm0, %xmm0
+; CHECK-XOP-NEXT:    vpxor %xmm1, %xmm0, %xmm0
 ; CHECK-XOP-NEXT:    retq
   %x = load <4 x i32>, <4 x i32> *%px, align 16
   %y = load <4 x i32>, <4 x i32> *%py, align 16
   %mask = load <4 x i32>, <4 x i32> *%pmask, align 16
   %n0 = xor <4 x i32> %x, <i32 -1, i32 -1, i32 -1, i32 -1> ; %x
   %n1 = and <4 x i32> %n0, %mask
   %r = xor <4 x i32> %n1, <i32 -1, i32 -1, i32 -1, i32 -1>
   ret <4 x i32> %r

 ; CHECK-SSE1-NEXT:    andnps %xmm2, %xmm0
 ; CHECK-SSE1-NEXT:    xorps %xmm1, %xmm0
 ; CHECK-SSE1-NEXT:    movaps %xmm0, (%rdi)
 ; CHECK-SSE1-NEXT:    movq %rdi, %rax
 ; CHECK-SSE1-NEXT:    retq
 ;
 ; CHECK-SSE2-LABEL: in_constant_varx_mone_invmask:
 ; CHECK-SSE2:       # %bb.0:
-; CHECK-SSE2-NEXT:    movdqa (%rdx), %xmm1
-; CHECK-SSE2-NEXT:    pcmpeqd %xmm0, %xmm0
-; CHECK-SSE2-NEXT:    pxor %xmm0, %xmm0
+; CHECK-SSE2-NEXT:    movdqa (%rdi), %xmm0
+; CHECK-SSE2-NEXT:    pcmpeqd %xmm1, %xmm1
+; CHECK-SSE2-NEXT:    movdqa (%rdx), %xmm2
+; CHECK-SSE2-NEXT:    pxor %xmm1, %xmm2
+; CHECK-SSE2-NEXT:    pandn %xmm2, %xmm0
 ; CHECK-SSE2-NEXT:    pxor %xmm1, %xmm0
-; CHECK-SSE2-NEXT:    pandn (%rdi), %xmm1
-; CHECK-SSE2-NEXT:    por %xmm1, %xmm0
 ; CHECK-SSE2-NEXT:    retq
 ;
 ; CHECK-XOP-LABEL: in_constant_varx_mone_invmask:
 ; CHECK-XOP:       # %bb.0:
-; CHECK-XOP-NEXT:    vmovdqa (%rdx), %xmm0
+; CHECK-XOP-NEXT:    vmovdqa (%rdi), %xmm0
 ; CHECK-XOP-NEXT:    vpcmpeqd %xmm1, %xmm1, %xmm1
-; CHECK-XOP-NEXT:    vpandn (%rdi), %xmm0, %xmm2
-; CHECK-XOP-NEXT:    vpxor %xmm1, %xmm1, %xmm1
+; CHECK-XOP-NEXT:    vpxor (%rdx), %xmm1, %xmm2
+; CHECK-XOP-NEXT:    vpandn %xmm2, %xmm0, %xmm0
 ; CHECK-XOP-NEXT:    vpxor %xmm1, %xmm0, %xmm0
-; CHECK-XOP-NEXT:    vpor %xmm0, %xmm2, %xmm0
 ; CHECK-XOP-NEXT:    retq
   %x = load <4 x i32>, <4 x i32> *%px, align 16
   %y = load <4 x i32>, <4 x i32> *%py, align 16
   %mask = load <4 x i32>, <4 x i32> *%pmask, align 16
   %notmask = xor <4 x i32> %mask, <i32 -1, i32 -1, i32 -1, i32 -1>
   %n0 = xor <4 x i32> %x, <i32 -1, i32 -1, i32 -1, i32 -1> ; %x
   %n1 = and <4 x i32> %n0, %notmask
   %r = xor <4 x i32> %n1, <i32 -1, i32 -1, i32 -1, i32 -1>