[SDAG] try harder to fold casts into vector compare

[spatel]

Summary

sext (vsetcc X, Y) --> vsetcc (zext X), (zext Y) -- (when the zexts are free and a bunch of other conditions)
We have a couple of similar folds to this already for vector selects, but this pattern slips through because it is only a setcc.

The tests are based on the motivating case from:
https://llvm.org/PR50055
...but we need extra logic to get that example, so I've left that as a TODO for now.

[lebedev.ri]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

10908

You already have CC

10946–10947

SVT is the result type of comparison of values with N00VT type.
Shouldn't this be !TLI.isOperationLegalOrCustom(ISD::SETCC, N00VT) ?

10950

You already have N00 and N01

10958

I think we can also accept ZEXTLOAD, since we'd just increase the desired number of leading zeros?

[pengfei]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

10946–10947

I think this is used for preventing the legal cases of the result type of comparison, e.g. cmp_slt_load_const?

[spatel]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

10946–10947

This is a bit tricky. I missed adding a test for this predicate, but this is what I intended.
That check is there to prevent transforming a legal vector op into a wider op because that's not guaranteed to be a win. Let me add a test and explain further.

And I think we do want to check the SVT type because that is how target handling for setcc is specified. For example, with AVX512, we have:

for (auto VT : { MVT::v2i1, MVT::v4i1, MVT::v8i1, MVT::v16i1 }) {
  setOperationAction(ISD::SETCC,            VT, Custom);

Let me know if I'm misunderstanding it. I'm not finding an example where I can show a difference because other transforms always seem to change the type before we get here.

[spatel]

Patch updated:

1. Removed redundant local variables.
2. Added (currently negative) test for widening of a legal compare.
3. Added TODO comment for allowing widening of an existing zext-load.

[spatel]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

10958

Probably, but I don't have a test example yet for that pattern. So ok to add one more TODO?

llvm/test/CodeGen/X86/sext-vsetcc.ll

419–424

If we allow widening of a legal type, this test will show it. We would convert to:

vpmovzxwd	(%rdi), %ymm0   
vpmovzxwd	(%rsi), %ymm1
vpcmpeqd	%ymm1, %ymm0, %ymm0
vpcmpeqd	%ymm1, %ymm1, %ymm1
vpxor	%ymm1, %ymm0, %ymm0

I don't think we can say with certainty that's a win for all targets (although I don't have any non-x86 examples to compare). It's probably ok, but if we want to allow it, then it can be a one-line follow-up patch?

[lebedev.ri]

One more random thought: should this be guarded with a check that getBooleanContents(N00VT) == ZeroOrNegativeOneBooleanContent ?

[spatel]

In D103280#2786791, @lebedev.ri wrote:

One more random thought: should this be guarded with a check that getBooleanContents(N00VT) == ZeroOrNegativeOneBooleanContent ?

Yes, we need that check. It's already there with some others at line 10913:

if (VT.isVector() && !LegalOperations &&
    TLI.getBooleanContents(N00VT) ==
        TargetLowering::ZeroOrNegativeOneBooleanContent) {

...this comment suggests that I should move this all into a helper because it's too big to read. That's why I had the redundant local variables in the earlier draft - I started by pulling it out, but got lazy. :)

[lebedev.ri]

TODOs sound fine to me.
LGTM assuming there are no other comments.
Thanks.

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

10950

I strongly suspect you might want /*NoOpaques=*/true, and the default is wrong.

[spatel]

Patch updated:

1. Restrict to non-opaque constants.
2. Fix lambda name to be clang-tidy compliant.

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

10950

Good catch! Not sure how to expose it in a test, but I'll add that.

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

SDValue DAGCombiner::visitSIGN_EXTEND(SDNode *N) {
   }
 
   if (SDValue V = foldExtendedSignBitTest(N, DAG, LegalOperations))
     return V;
 
   if (N0.getOpcode() == ISD::SETCC) {
     SDValue N00 = N0.getOperand(0);
     SDValue N01 = N0.getOperand(1);
     ISD::CondCode CC = cast<CondCodeSDNode>(N0.getOperand(2))->get();
     EVT N00VT = N00.getValueType();
 
     // sext(setcc) -> sext_in_reg(vsetcc) for vectors.
     // Only do this before legalize for now.
     if (VT.isVector() && !LegalOperations &&
         TLI.getBooleanContents(N00VT) ==
             TargetLowering::ZeroOrNegativeOneBooleanContent) {
       // On some architectures (such as SSE/NEON/etc) the SETCC result type is
if (VT.isVector() && !LegalOperations &&
         // elements, we can use a matching integer vector type and then
         // truncate/sign extend.
         EVT MatchingVecType = N00VT.changeVectorElementTypeToInteger();
         if (SVT == MatchingVecType) {
           SDValue VsetCC = DAG.getSetCC(DL, MatchingVecType, N00, N01, CC);
           return DAG.getSExtOrTrunc(VsetCC, DL, VT);
         }
       }
+
+      // Try to eliminate the sext of a setcc by zexting the compare operands.
+      // TODO: Handle signed compare by sexting the ops.
+      if (!ISD::isSignedIntSetCC(CC) && N0.hasOneUse() &&
+          TLI.isOperationLegalOrCustom(ISD::SETCC, VT) /*&&
+          !TLI.isOperationLegalOrCustom(ISD::SETCC, SVT)*/) {
+        // We have an unsupported narrow vector compare op that would be legal
+        // if extended to the destination type. See if the compare operands
+        // can be freely extended to the destination type.
+        auto IsFreeToZext = [&](SDValue V) {
+          if (isConstantOrConstantVector(V, /*NoOpaques*/ true))
+            return true;
+
+          // Match a simple, non-extended load that can be converted to a
+          // legal zext-load.
+          // TODO: Handle more than one use if the other uses are free to zext.
+          // TODO: Allow widening of an existing zext-load?
+          return ISD::isNON_EXTLoad(V.getNode()) &&
+                 ISD::isUNINDEXEDLoad(V.getNode()) &&
+                 cast<LoadSDNode>(V)->isSimple() &&
+                 TLI.isLoadExtLegal(ISD::ZEXTLOAD, VT, V.getValueType()) &&
+                 V.hasOneUse();
+        };
+
+        if (IsFreeToZext(N00) && IsFreeToZext(N01)) {
+          SDValue Ext0 = DAG.getZExtOrTrunc(N00, DL, VT);
+          SDValue Ext1 = DAG.getZExtOrTrunc(N01, DL, VT);
+          return DAG.getSetCC(DL, VT, Ext0, Ext1, CC);
+        }
+      }
     }
 
     // sext(setcc x, y, cc) -> (select (setcc x, y, cc), T, 0)
     // Here, T can be 1 or -1, depending on the type of the setcc and
     // getBooleanContents().
     unsigned SetCCWidth = N0.getScalarValueSizeInBits();
 
     // To determine the "true" side of the select, we need to know the high bit

llvm/test/CodeGen/X86/sext-vsetcc.ll

 ; SSE-NEXT:    pxor %xmm1, %xmm1
 ; SSE-NEXT:    pcmpeqb %xmm0, %xmm1
 ; SSE-NEXT:    pcmpeqd %xmm0, %xmm0
 ; SSE-NEXT:    pxor %xmm1, %xmm0
 ; SSE-NEXT:    punpcklbw {{.*#+}} xmm0 = xmm0[0,0,1,1,2,2,3,3,4,4,5,5,6,6,7,7]
 ; SSE-NEXT:    psraw $8, %xmm0
 ; SSE-NEXT:    retq
 ;
-; AVX2-LABEL: cmp_ne_load_const:
-; AVX2:       # %bb.0:
-; AVX2-NEXT:    vmovq {{.*#+}} xmm0 = mem[0],zero
-; AVX2-NEXT:    vpxor %xmm1, %xmm1, %xmm1
-; AVX2-NEXT:    vpcmpeqb %xmm1, %xmm0, %xmm0
-; AVX2-NEXT:    vpcmpeqd %xmm1, %xmm1, %xmm1
-; AVX2-NEXT:    vpxor %xmm1, %xmm0, %xmm0
-; AVX2-NEXT:    vpmovsxbw %xmm0, %xmm0
-; AVX2-NEXT:    retq
-;
-; AVX512-LABEL: cmp_ne_load_const:
-; AVX512:       # %bb.0:
-; AVX512-NEXT:    vmovq {{.*#+}} xmm0 = mem[0],zero
-; AVX512-NEXT:    vpxor %xmm1, %xmm1, %xmm1
-; AVX512-NEXT:    vpcmpeqb %xmm1, %xmm0, %xmm0
-; AVX512-NEXT:    vpternlogq $15, %zmm0, %zmm0, %zmm0
-; AVX512-NEXT:    vpmovsxbw %xmm0, %xmm0
-; AVX512-NEXT:    vzeroupper
-; AVX512-NEXT:    retq
+; AVX-LABEL: cmp_ne_load_const:
+; AVX:       # %bb.0:
+; AVX-NEXT:    vpmovzxbw {{.*#+}} xmm0 = mem[0],zero,mem[1],zero,mem[2],zero,mem[3],zero,mem[4],zero,mem[5],zero,mem[6],zero,mem[7],zero
+; AVX-NEXT:    vpxor %xmm1, %xmm1, %xmm1
+; AVX-NEXT:    vpcmpgtw %xmm1, %xmm0, %xmm0
+; AVX-NEXT:    retq
   %loadx = load <8 x i8>, <8 x i8>* %x
   %icmp = icmp ne <8 x i8> %loadx, zeroinitializer
   %sext = sext <8 x i1> %icmp to <8 x i16>
   ret <8 x i16> %sext
 }
 
+; negative test - simple loads only
+
 define <8 x i16> @cmp_ne_load_const_volatile(<8 x i8>* %x) nounwind {
 ; SSE-LABEL: cmp_ne_load_const_volatile:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    movq {{.*#+}} xmm0 = mem[0],zero
 ; SSE-NEXT:    pxor %xmm1, %xmm1
 ; SSE-NEXT:    pcmpeqb %xmm0, %xmm1
 ; SSE-NEXT:    pcmpeqd %xmm0, %xmm0
 ; SSE-NEXT:    pxor %xmm1, %xmm0

 ; AVX512-NEXT:    vzeroupper
 ; AVX512-NEXT:    retq
   %loadx = load volatile <8 x i8>, <8 x i8>* %x
   %icmp = icmp ne <8 x i8> %loadx, zeroinitializer
   %sext = sext <8 x i1> %icmp to <8 x i16>
   ret <8 x i16> %sext
 }
 
+; negative test - don't create extra load
+
 define <8 x i16> @cmp_ne_load_const_extra_use1(<8 x i8>* %x) nounwind {
 ; SSE-LABEL: cmp_ne_load_const_extra_use1:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    subq $24, %rsp
 ; SSE-NEXT:    movq {{.*#+}} xmm0 = mem[0],zero
 ; SSE-NEXT:    movdqa %xmm0, (%rsp) # 16-byte Spill
 ; SSE-NEXT:    callq use_v8i8@PLT
 ; SSE-NEXT:    pxor %xmm0, %xmm0

 ; AVX512-NEXT:    retq
   %loadx = load <8 x i8>, <8 x i8>* %x
   call void @use_v8i8(<8 x i8> %loadx)
   %icmp = icmp ne <8 x i8> %loadx, zeroinitializer
   %sext = sext <8 x i1> %icmp to <8 x i16>
   ret <8 x i16> %sext
 }
 
+; negative test - don't create extra compare
+
 define <8 x i16> @cmp_ne_load_const_extra_use2(<8 x i8>* %x) nounwind {
 ; SSE-LABEL: cmp_ne_load_const_extra_use2:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    subq $24, %rsp
 ; SSE-NEXT:    movq {{.*#+}} xmm0 = mem[0],zero
 ; SSE-NEXT:    pxor %xmm1, %xmm1
 ; SSE-NEXT:    pcmpeqb %xmm0, %xmm1
 ; SSE-NEXT:    pcmpeqd %xmm0, %xmm0

 ; AVX512-NEXT:    retq
   %loadx = load <8 x i8>, <8 x i8>* %x
   %icmp = icmp ne <8 x i8> %loadx, zeroinitializer
   call void @use_v8i1(<8 x i1> %icmp)
   %sext = sext <8 x i1> %icmp to <8 x i16>
   ret <8 x i16> %sext
 }
 
+; negative test - not free extend
+
 define <8 x i16> @cmp_ne_no_load_const(i64 %x) nounwind {
 ; SSE-LABEL: cmp_ne_no_load_const:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    movq %rdi, %xmm0
 ; SSE-NEXT:    pxor %xmm1, %xmm1
 ; SSE-NEXT:    pcmpeqb %xmm0, %xmm1
 ; SSE-NEXT:    pcmpeqd %xmm0, %xmm0
 ; SSE-NEXT:    pxor %xmm1, %xmm0

 ; SSE-NEXT:    pcmpeqb %xmm0, %xmm1
 ; SSE-NEXT:    pcmpeqd %xmm0, %xmm0
 ; SSE-NEXT:    pxor %xmm1, %xmm0
 ; SSE-NEXT:    punpcklbw {{.*#+}} xmm0 = xmm0[0,0,1,1,2,2,3,3,4,4,5,5,6,6,7,7]
 ; SSE-NEXT:    punpcklwd {{.*#+}} xmm0 = xmm0[0,0,1,1,2,2,3,3]
 ; SSE-NEXT:    psrad $24, %xmm0
 ; SSE-NEXT:    retq
 ;
-; AVX2-LABEL: cmp_ult_load_const:
-; AVX2:       # %bb.0:
-; AVX2-NEXT:    vmovd {{.*#+}} xmm0 = mem[0],zero,zero,zero
-; AVX2-NEXT:    vpmaxub {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0, %xmm1
-; AVX2-NEXT:    vpcmpeqb %xmm1, %xmm0, %xmm0
-; AVX2-NEXT:    vpcmpeqd %xmm1, %xmm1, %xmm1
-; AVX2-NEXT:    vpxor %xmm1, %xmm0, %xmm0
-; AVX2-NEXT:    vpmovsxbd %xmm0, %xmm0
-; AVX2-NEXT:    retq
-;
-; AVX512-LABEL: cmp_ult_load_const:
-; AVX512:       # %bb.0:
-; AVX512-NEXT:    vmovd {{.*#+}} xmm0 = mem[0],zero,zero,zero
-; AVX512-NEXT:    vpmaxub {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0, %xmm1
-; AVX512-NEXT:    vpcmpeqb %xmm1, %xmm0, %xmm0
-; AVX512-NEXT:    vpternlogq $15, %zmm0, %zmm0, %zmm0
-; AVX512-NEXT:    vpmovsxbd %xmm0, %xmm0
-; AVX512-NEXT:    vzeroupper
-; AVX512-NEXT:    retq
+; AVX-LABEL: cmp_ult_load_const:
+; AVX:       # %bb.0:
+; AVX-NEXT:    vpmovzxbd {{.*#+}} xmm0 = mem[0],zero,zero,zero,mem[1],zero,zero,zero,mem[2],zero,zero,zero,mem[3],zero,zero,zero
+; AVX-NEXT:    vmovdqa {{.*#+}} xmm1 = [42,214,0,255]
+; AVX-NEXT:    vpcmpgtd %xmm0, %xmm1, %xmm0
+; AVX-NEXT:    retq
   %loadx = load <4 x i8>, <4 x i8>* %x
   %icmp = icmp ult <4 x i8> %loadx, <i8 42, i8 -42, i8 0, i8 -1>
   %sext = sext <4 x i1> %icmp to <4 x i32>
   ret <4 x i32> %sext
 }
 
+; negative test - type must be legal
+
 define <3 x i32> @cmp_ult_load_const_bad_type(<3 x i8>* %x) nounwind {
 ; SSE-LABEL: cmp_ult_load_const_bad_type:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    movd {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; SSE-NEXT:    movdqa {{.*#+}} xmm1 = <42,214,0,u,u,u,u,u,u,u,u,u,u,u,u,u>
 ; SSE-NEXT:    pmaxub %xmm0, %xmm1
 ; SSE-NEXT:    pcmpeqb %xmm0, %xmm1
 ; SSE-NEXT:    pcmpeqd %xmm0, %xmm0

 ; AVX512-NEXT:    vzeroupper
 ; AVX512-NEXT:    retq
   %loadx = load <3 x i8>, <3 x i8>* %x
   %icmp = icmp ult <3 x i8> %loadx, <i8 42, i8 -42, i8 0>
   %sext = sext <3 x i1> %icmp to <3 x i32>
   ret <3 x i32> %sext
 }
 
+; negative test - signed cmp (TODO)
+
 define <4 x i32> @cmp_slt_load_const(<4 x i8>* %x) nounwind {
 ; SSE-LABEL: cmp_slt_load_const:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    movd {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; SSE-NEXT:    movdqa {{.*#+}} xmm1 = <42,214,0,255,u,u,u,u,u,u,u,u,u,u,u,u>
 ; SSE-NEXT:    pcmpgtb %xmm0, %xmm1
 ; SSE-NEXT:    punpcklbw {{.*#+}} xmm0 = xmm0[0],xmm1[0],xmm0[1],xmm1[1],xmm0[2],xmm1[2],xmm0[3],xmm1[3],xmm0[4],xmm1[4],xmm0[5],xmm1[5],xmm0[6],xmm1[6],xmm0[7],xmm1[7]
 ; SSE-NEXT:    punpcklwd {{.*#+}} xmm0 = xmm0[0,0,1,1,2,2,3,3]

 ; SSE-NEXT:    pxor %xmm1, %xmm0
 ; SSE-NEXT:    pxor %xmm1, %xmm1
 ; SSE-NEXT:    pcmpgtd %xmm0, %xmm1
 ; SSE-NEXT:    punpckldq {{.*#+}} xmm0 = xmm0[0],xmm1[0],xmm0[1],xmm1[1]
 ; SSE-NEXT:    retq
 ;
 ; AVX2-LABEL: cmp_ne_zextload:
 ; AVX2:       # %bb.0:
-; AVX2-NEXT:    vmovq {{.*#+}} xmm0 = mem[0],zero
-; AVX2-NEXT:    vmovq {{.*#+}} xmm1 = mem[0],zero
-; AVX2-NEXT:    vpcmpeqd %xmm1, %xmm0, %xmm0
+; AVX2-NEXT:    vpmovzxdq {{.*#+}} xmm0 = mem[0],zero,mem[1],zero
+; AVX2-NEXT:    vpmovzxdq {{.*#+}} xmm1 = mem[0],zero,mem[1],zero
+; AVX2-NEXT:    vpcmpeqq %xmm1, %xmm0, %xmm0
 ; AVX2-NEXT:    vpcmpeqd %xmm1, %xmm1, %xmm1
 ; AVX2-NEXT:    vpxor %xmm1, %xmm0, %xmm0
-; AVX2-NEXT:    vpmovsxdq %xmm0, %xmm0
 ; AVX2-NEXT:    retq
 ;
 ; AVX512-LABEL: cmp_ne_zextload:
 ; AVX512:       # %bb.0:
-; AVX512-NEXT:    vmovq {{.*#+}} xmm0 = mem[0],zero
-; AVX512-NEXT:    vmovq {{.*#+}} xmm1 = mem[0],zero
-; AVX512-NEXT:    vpcmpeqd %xmm1, %xmm0, %xmm0
+; AVX512-NEXT:    vpmovzxdq {{.*#+}} xmm0 = mem[0],zero,mem[1],zero
+; AVX512-NEXT:    vpmovzxdq {{.*#+}} xmm1 = mem[0],zero,mem[1],zero
+; AVX512-NEXT:    vpcmpeqq %xmm1, %xmm0, %xmm0
 ; AVX512-NEXT:    vpternlogq $15, %zmm0, %zmm0, %zmm0
-; AVX512-NEXT:    vpmovsxdq %xmm0, %xmm0
+; AVX512-NEXT:    # kill: def $xmm0 killed $xmm0 killed $zmm0
 ; AVX512-NEXT:    vzeroupper
 ; AVX512-NEXT:    retq
   %loadx = load <2 x i32>, <2 x i32>* %x
   %loady = load <2 x i32>, <2 x i32>* %y
   %icmp = icmp ne <2 x i32> %loadx, %loady
   %sext = sext <2 x i1> %icmp to <2 x i64>
   ret <2 x i64> %sext
 }
 
 define <8 x i16> @cmp_ugt_zextload(<8 x i8>* %x, <8 x i8>* %y) nounwind {
 ; SSE-LABEL: cmp_ugt_zextload:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    movq {{.*#+}} xmm0 = mem[0],zero
 ; SSE-NEXT:    movq {{.*#+}} xmm1 = mem[0],zero
 ; SSE-NEXT:    pminub %xmm0, %xmm1
 ; SSE-NEXT:    pcmpeqb %xmm0, %xmm1
 ; SSE-NEXT:    pcmpeqd %xmm0, %xmm0
 ; SSE-NEXT:    pxor %xmm1, %xmm0
 ; SSE-NEXT:    punpcklbw {{.*#+}} xmm0 = xmm0[0,0,1,1,2,2,3,3,4,4,5,5,6,6,7,7]
 ; SSE-NEXT:    psraw $8, %xmm0
 ; SSE-NEXT:    retq
 ;
-; AVX2-LABEL: cmp_ugt_zextload:
-; AVX2:       # %bb.0:
-; AVX2-NEXT:    vmovq {{.*#+}} xmm0 = mem[0],zero
-; AVX2-NEXT:    vmovq {{.*#+}} xmm1 = mem[0],zero
-; AVX2-NEXT:    vpminub %xmm1, %xmm0, %xmm1
-; AVX2-NEXT:    vpcmpeqb %xmm1, %xmm0, %xmm0
-; AVX2-NEXT:    vpcmpeqd %xmm1, %xmm1, %xmm1
-; AVX2-NEXT:    vpxor %xmm1, %xmm0, %xmm0
-; AVX2-NEXT:    vpmovsxbw %xmm0, %xmm0
-; AVX2-NEXT:    retq
-;
-; AVX512-LABEL: cmp_ugt_zextload:
-; AVX512:       # %bb.0:
-; AVX512-NEXT:    vmovq {{.*#+}} xmm0 = mem[0],zero
-; AVX512-NEXT:    vmovq {{.*#+}} xmm1 = mem[0],zero
-; AVX512-NEXT:    vpminub %xmm1, %xmm0, %xmm1
-; AVX512-NEXT:    vpcmpeqb %xmm1, %xmm0, %xmm0
-; AVX512-NEXT:    vpternlogq $15, %zmm0, %zmm0, %zmm0
-; AVX512-NEXT:    vpmovsxbw %xmm0, %xmm0
-; AVX512-NEXT:    vzeroupper
-; AVX512-NEXT:    retq
+; AVX-LABEL: cmp_ugt_zextload:
+; AVX:       # %bb.0:
+; AVX-NEXT:    vpmovzxbw {{.*#+}} xmm0 = mem[0],zero,mem[1],zero,mem[2],zero,mem[3],zero,mem[4],zero,mem[5],zero,mem[6],zero,mem[7],zero
+; AVX-NEXT:    vpmovzxbw {{.*#+}} xmm1 = mem[0],zero,mem[1],zero,mem[2],zero,mem[3],zero,mem[4],zero,mem[5],zero,mem[6],zero,mem[7],zero
+; AVX-NEXT:    vpcmpgtw %xmm1, %xmm0, %xmm0
+; AVX-NEXT:    retq
   %loadx = load <8 x i8>, <8 x i8>* %x
   %loady = load <8 x i8>, <8 x i8>* %y
   %icmp = icmp ugt <8 x i8> %loadx, %loady
   %sext = sext <8 x i1> %icmp to <8 x i16>
   ret <8 x i16> %sext
 }
 
+; negative test - signed cmp (TODO)
+
 define <8 x i16> @cmp_sgt_zextload(<8 x i8>* %x, <8 x i8>* %y) nounwind {
 ; SSE-LABEL: cmp_sgt_zextload:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    movq {{.*#+}} xmm0 = mem[0],zero
 ; SSE-NEXT:    movq {{.*#+}} xmm1 = mem[0],zero
 ; SSE-NEXT:    pcmpgtb %xmm1, %xmm0
 ; SSE-NEXT:    punpcklbw {{.*#+}} xmm0 = xmm0[0,0,1,1,2,2,3,3,4,4,5,5,6,6,7,7]
 ; SSE-NEXT:    psraw $8, %xmm0
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: cmp_sgt_zextload:
 ; AVX:       # %bb.0:
 ; AVX-NEXT:    vmovq {{.*#+}} xmm0 = mem[0],zero
 ; AVX-NEXT:    vmovq {{.*#+}} xmm1 = mem[0],zero
 ; AVX-NEXT:    vpcmpgtb %xmm1, %xmm0, %xmm0
 ; AVX-NEXT:    vpmovsxbw %xmm0, %xmm0
 ; AVX-NEXT:    retq
   %loadx = load <8 x i8>, <8 x i8>* %x
   %loady = load <8 x i8>, <8 x i8>* %y
   %icmp = icmp sgt <8 x i8> %loadx, %loady
   %sext = sext <8 x i1> %icmp to <8 x i16>
   ret <8 x i16> %sext
 }
 
+; negative test - don't change a legal op
+; TODO: Or should we? We can eliminate the vpmovsxwd at the cost of a 256-bit ymm vpcmpeqw.
+
 define <8 x i32> @cmp_ne_zextload_from_legal_op(<8 x i16>* %x, <8 x i16>* %y) {
 ; SSE-LABEL: cmp_ne_zextload_from_legal_op:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    movdqa (%rdi), %xmm0
 ; SSE-NEXT:    pcmpeqw (%rsi), %xmm0
 ; SSE-NEXT:    pcmpeqd %xmm1, %xmm1
 ; SSE-NEXT:    pxor %xmm0, %xmm1
 ; SSE-NEXT:    punpcklwd {{.*#+}} xmm0 = xmm0[0],xmm1[0],xmm0[1],xmm1[1],xmm0[2],xmm1[2],xmm0[3],xmm1[3]
 ; SSE-NEXT:    psrad $16, %xmm0
 ; SSE-NEXT:    punpckhwd {{.*#+}} xmm1 = xmm1[4,4,5,5,6,6,7,7]
 ; SSE-NEXT:    psrad $16, %xmm1
 ; SSE-NEXT:    retq
 ;
 ; AVX2-LABEL: cmp_ne_zextload_from_legal_op:
 ; AVX2:       # %bb.0:
-; AVX2-NEXT:    vmovdqa (%rdi), %xmm0
-; AVX2-NEXT:    vpcmpeqw (%rsi), %xmm0, %xmm0
-; AVX2-NEXT:    vpcmpeqd %xmm1, %xmm1, %xmm1
-; AVX2-NEXT:    vpxor %xmm1, %xmm0, %xmm0
-; AVX2-NEXT:    vpmovsxwd %xmm0, %ymm0
+; AVX2-NEXT:    vpmovzxwd {{.*#+}} ymm0 = mem[0],zero,mem[1],zero,mem[2],zero,mem[3],zero,mem[4],zero,mem[5],zero,mem[6],zero,mem[7],zero
+; AVX2-NEXT:    vpmovzxwd {{.*#+}} ymm1 = mem[0],zero,mem[1],zero,mem[2],zero,mem[3],zero,mem[4],zero,mem[5],zero,mem[6],zero,mem[7],zero
+; AVX2-NEXT:    vpcmpeqd %ymm1, %ymm0, %ymm0
+; AVX2-NEXT:    vpcmpeqd %ymm1, %ymm1, %ymm1
+; AVX2-NEXT:    vpxor %ymm1, %ymm0, %ymm0
 ; AVX2-NEXT:    retq
 ;
 ; AVX512-LABEL: cmp_ne_zextload_from_legal_op:
 ; AVX512:       # %bb.0:
-; AVX512-NEXT:    vmovdqa (%rdi), %xmm0
-; AVX512-NEXT:    vpcmpeqw (%rsi), %xmm0, %xmm0
+; AVX512-NEXT:    vpmovzxwd {{.*#+}} ymm0 = mem[0],zero,mem[1],zero,mem[2],zero,mem[3],zero,mem[4],zero,mem[5],zero,mem[6],zero,mem[7],zero
+; AVX512-NEXT:    vpmovzxwd {{.*#+}} ymm1 = mem[0],zero,mem[1],zero,mem[2],zero,mem[3],zero,mem[4],zero,mem[5],zero,mem[6],zero,mem[7],zero
+; AVX512-NEXT:    vpcmpeqd %ymm1, %ymm0, %ymm0
 ; AVX512-NEXT:    vpternlogq $15, %zmm0, %zmm0, %zmm0
-; AVX512-NEXT:    vpmovsxwd %xmm0, %ymm0
+; AVX512-NEXT:    # kill: def $ymm0 killed $ymm0 killed $zmm0
 ; AVX512-NEXT:    retq
   %loadx = load <8 x i16>, <8 x i16>* %x
   %loady = load <8 x i16>, <8 x i16>* %y
   %icmp = icmp ne <8 x i16> %loadx, %loady
   %sext = sext <8 x i1> %icmp to <8 x i32>
   ret <8 x i32> %sext
 }
 
+; negative test - extra use (TODO)
+
 define <8 x i32> @PR50055(<8 x i8>* %src, <8 x i32>* %dst) nounwind {
 ; SSE-LABEL: PR50055:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    movq {{.*#+}} xmm2 = mem[0],zero
 ; SSE-NEXT:    pxor %xmm3, %xmm3
 ; SSE-NEXT:    movdqa %xmm2, %xmm1
 ; SSE-NEXT:    punpcklbw {{.*#+}} xmm1 = xmm1[0],xmm3[0],xmm1[1],xmm3[1],xmm1[2],xmm3[2],xmm1[3],xmm3[3],xmm1[4],xmm3[4],xmm1[5],xmm3[5],xmm1[6],xmm3[6],xmm1[7],xmm3[7]
 ; SSE-NEXT:    movdqa %xmm1, %xmm0