[x86] bug fix for https://reviews.llvm.org/D64551

[yubing]

Summary

bug fix for https://reviews.llvm.org/D64551
Prevent combining X86ISD::VZEXT_MOVL into wrong DAGs

https://reviews.llvm.org/D64551 has produced a bug in llvm:

t.ll1 KBDownload

the attached t.ll can reproduce this bug:
For t.ll, llvm without this patch produces the correct asm while llvm with this patch produces bad asm:
llvm with this patch:

vmovd   xmm0, dword ptr [rdi + 260] # xmm0 = mem[0],zero,zero,zero
vmovd   xmm1, dword ptr [rdi + 252] # xmm1 = mem[0],zero,zero,zero
vpunpcklqdq     xmm0, xmm1, xmm0 # xmm0 = xmm1[0],xmm0[0]
vxorps  xmm1, xmm1, xmm1
vinserti128     ymm0, ymm1, xmm0, 1
vmovdqa ymmword ptr [rsi + 672], ymm0

llvm without this patch:

vmovq   xmm0, qword ptr [rdi + 252] # xmm0 = mem[0],zero
movabs  rax, offset .LCPI0_0
vmovdqa xmm1, xmmword ptr [rax]
vpshufb xmm0, xmm0, xmm1
vmovd   xmm1, dword ptr [rdi + 260] # xmm1 = mem[0],zero,zero,zero
vpunpcklqdq     xmm0, xmm0, xmm1 # xmm0 = xmm0[0],xmm1[0]
vxorps  xmm1, xmm1, xmm1
vinserti128     ymm0, ymm1, xmm0, 1
vmovdqa ymmword ptr [rsi + 672], ymm0

When I compared the ‘llc -debug’ output, I found the procedure of Combining: t53: v4i32 = X86ISD::VZEXT_MOVL t43 are different:
llvm without this patch:

Legalizing: t53: v4i32 = X86ISD::VZEXT_MOVL t43
Legal node: nothing to do

Combining: t53: v4i32 = X86ISD::VZEXT_MOVL t43
Creating new node: t55: v2i64 = undef
Creating constant: t56: i8 = Constant<4>
Creating constant: t57: i8 = Constant<5>
Creating constant: t58: i8 = Constant<6>
Creating constant: t59: i8 = Constant<7>
Creating constant: t60: i8 = Constant<-1>

llvm with this patch:

Legalizing: t53: v4i32 = X86ISD::VZEXT_MOVL t43
Legal node: nothing to do

Combining: t53: v4i32 = X86ISD::VZEXT_MOVL t43

Then I used ‘dbg llc’ and found that EltsFromConsecutiveLoads will return SDValue() while llvm with this patch won’t.

The return SDValue() was deleted by your patch:

if (!ISD::isNON_EXTLoad(Elt.getNode()))
   return SDValue();

So, I added these two lines back to llvm with this patch, then output asm is correct and the same with llvm without this patch:

[RKSimon]

Please abandon this, it isn't a valid solution to the issue (raised at PR43227). I have a WIP fix that will address this correctly.

llvm/lib/Target/X86/X86ISelLowering.cpp

7681

This misses the entire point of D64551 which adds support for drilling down through other ops to find a NON_EXTLoad with a byte offset.

[LuoYuanke]

In D67210#1659230, @RKSimon wrote:

Please abandon this, it isn't a valid solution to the issue (raised at PR43227). I have a WIP fix that will address this correctly.

Hi Simon,
We meet some deadline for fixing the issue. We are eager to see your patch. :)

[yubing]

In D67210#1659230, @RKSimon wrote:

Please abandon this, it isn't a valid solution to the issue (raised at PR43227). I have a WIP fix that will address this correctly.

Thanks @RKSimon

[RKSimon]

rL371078 should fix this

[LuoYuanke]

In D67210#1659439, @RKSimon wrote:

rL371078 should fix this

Thanks Simon! We'll try it.

llvm/lib/Target/X86/X86ISelLowering.cpp

for (unsigned i = 0; i < NumElems; ++i) {
     if (Elt.isUndef()) {
       UndefMask.setBit(i);
       continue;
     }
     if (X86::isZeroNode(Elt) || ISD::isBuildVectorAllZeros(Elt.getNode())) {
       ZeroMask.setBit(i);
       continue;
     }
-
+    if (!ISD::isNON_EXTLoad(Elt.getNode()))
+      return SDValue();
     // Each loaded element must be the correct fractional portion of the
     // requested vector load.
     unsigned EltSizeInBits = Elt.getValueSizeInBits();
     if ((NumElems * EltSizeInBits) != VT.getSizeInBits())
       return SDValue();
 
     if (!findEltLoadSrc(Elt, Loads[i], ByteOffsets[i]) || ByteOffsets[i] < 0)
       return SDValue();

llvm/test/CodeGen/X86/load-partial.ll

; AVX-NEXT:    retq
   %r0 = insertelement <8 x float> undef, float %ld0, i32 0
   %r1 = insertelement <8 x float> %r0,   float %ld1, i32 1
   %r2 = insertelement <8 x float> %r1,   float %ld2, i32 2
   %r3 = insertelement <8 x float> %r2,   float %ld2, i32 3
   ret <8 x float> %r3
 }
 
 define <4 x float> @load_float4_float3_as_float2_float(<4 x float>* nocapture readonly dereferenceable(16)) {
-; SSE-LABEL: load_float4_float3_as_float2_float:
-; SSE:       # %bb.0:
-; SSE-NEXT:    movups (%rdi), %xmm0
-; SSE-NEXT:    retq
+; SSE2-LABEL: load_float4_float3_as_float2_float:
+; SSE2:       # %bb.0:
+; SSE2-NEXT:    movsd   (%rdi), %xmm0           # xmm0 = mem[0],zero
+; SSE2-NEXT:    movss   8(%rdi), %xmm1          # xmm1 = mem[0],zero,zero,zero
+; SSE2-NEXT:    shufps  $48, %xmm0, %xmm1       # xmm1 = xmm1[0,0],xmm0[3,0]
+; SSE2-NEXT:    shufps  $132, %xmm1, %xmm0      # xmm0 = xmm0[0,1],xmm1[0,2]
+; SSE2-NEXT:    retq
+;
+; SSSE3-LABEL: load_float4_float3_as_float2_float:
+; SSSE3:       # %bb.0:
+; SSSE3-NEXT:    movsd   (%rdi), %xmm0           # xmm0 = mem[0],zero
+; SSSE3-NEXT:    movss   8(%rdi), %xmm1          # xmm1 = mem[0],zero,zero,zero
+; SSSE3-NEXT:    shufps  $48, %xmm0, %xmm1       # xmm1 = xmm1[0,0],xmm0[3,0]
+; SSSE3-NEXT:    shufps  $132, %xmm1, %xmm0      # xmm0 = xmm0[0,1],xmm1[0,2]
+; SSSE3-NEXT:    retq
+;
+; SSE41-LABEL: load_float4_float3_as_float2_float:
+; SSE41:       # %bb.0:
+; SSE41-NEXT:    movsd   (%rdi), %xmm0           # xmm0 = mem[0],zero
+; SSE41-NEXT:    insertps        $32, 8(%rdi), %xmm0 # xmm0 = xmm0[0,1],mem[0],xmm0[3]
+; SSE41-NEXT:    retq
+
 ;
 ; AVX-LABEL: load_float4_float3_as_float2_float:
 ; AVX:       # %bb.0:
-; AVX-NEXT:    vmovups (%rdi), %xmm0
+; AVX-NEXT:    vmovsd  (%rdi), %xmm0           # xmm0 = mem[0],zero
+; AVX-NEXT:    vinsertps       $32, 8(%rdi), %xmm0, %xmm0 # xmm0 = xmm0[0,1],mem[0],xmm0[3]
 ; AVX-NEXT:    retq
   %2 = bitcast <4 x float>* %0 to <2 x float>*
   %3 = load <2 x float>, <2 x float>* %2, align 4
   %4 = extractelement <2 x float> %3, i32 0
   %5 = insertelement <4 x float> undef, float %4, i32 0
   %6 = extractelement <2 x float> %3, i32 1
   %7 = insertelement <4 x float> %5, float %6, i32 1
   %8 = getelementptr inbounds <4 x float>, <4 x float>* %0, i64 0, i64 2
; AVX-NEXT:    retq
   %8 = getelementptr inbounds <4 x float>, <4 x float>* %0, i64 0, i64 2
   %9 = load float, float* %8, align 4
   %10 = insertelement <4 x float> %7, float %9, i32 2
   %11 = insertelement <4 x float> %10, float %9, i32 3
   ret <4 x float> %11
 }
 
 define <4 x float> @load_float4_float3_trunc(<4 x float>* nocapture readonly dereferenceable(16)) {
-; SSE-LABEL: load_float4_float3_trunc:
-; SSE:       # %bb.0:
-; SSE-NEXT:    movaps (%rdi), %xmm0
-; SSE-NEXT:    retq
+; SSE2-LABEL: load_float4_float3_trunc:
+; SSE2:       # %bb.0:
+; SSE2-NEXT:    movss   (%rdi), %xmm0           # xmm0 = mem[0],zero,zero,zero
+; SSE2-NEXT:    movss   4(%rdi), %xmm1          # xmm1 = mem[0],zero,zero,zero
+; SSE2-NEXT:    unpcklps        %xmm1, %xmm0    # xmm0 = xmm0[0],xmm1[0],xmm0[1],xmm1[1]
+; SSE2-NEXT:    movss   8(%rdi), %xmm1          # xmm1 = mem[0],zero,zero,zero
+; SSE2-NEXT:    movlhps %xmm1, %xmm0            # xmm0 = xmm0[0],xmm1[0]
+; SSE2-NEXT:    retq
+;
+; SSSE3-LABEL: load_float4_float3_trunc:
+; SSSE3:       # %bb.0:
+; SSSE3-NEXT:    movss   (%rdi), %xmm0           # xmm0 = mem[0],zero,zero,zero
+; SSSE3-NEXT:    movss   4(%rdi), %xmm1          # xmm1 = mem[0],zero,zero,zero
+; SSSE3-NEXT:    unpcklps        %xmm1, %xmm0    # xmm0 = xmm0[0],xmm1[0],xmm0[1],xmm1[1]
+; SSSE3-NEXT:    movss   8(%rdi), %xmm1          # xmm1 = mem[0],zero,zero,zero
+; SSSE3-NEXT:    movlhps %xmm1, %xmm0            # xmm0 = xmm0[0],xmm1[0]
+; SSSE3-NEXT:    retq
+;
+; SSE41-LABEL: load_float4_float3_trunc:
+; SSE41:       # %bb.0:
+; SSE41-NEXT:    movss   (%rdi), %xmm0           # xmm0 = mem[0],zero,zero,zero
+; SSE41-NEXT:    insertps        $16, 4(%rdi), %xmm0 # xmm0 = xmm0[0],mem[0],xmm0[2,3]
+; SSE41-NEXT:    insertps        $32, 8(%rdi), %xmm0 # xmm0 = xmm0[0,1],mem[0],xmm0[3]
+; SSE41-NEXT:    retq
+
 ;
 ; AVX-LABEL: load_float4_float3_trunc:
 ; AVX:       # %bb.0:
-; AVX-NEXT:    vmovaps (%rdi), %xmm0
+; AVX-NEXT:    vmovss  (%rdi), %xmm0           # xmm0 = mem[0],zero,zero,zero
+; AVX-NEXT:    vinsertps       $16, 4(%rdi), %xmm0, %xmm0 # xmm0 = xmm0[0],mem[0],xmm0[2,3]
+; AVX-NEXT:    vinsertps       $32, 8(%rdi), %xmm0, %xmm0 # xmm0 = xmm0[0,1],mem[0],xmm0[3]
 ; AVX-NEXT:    retq
   %2 = bitcast <4 x float>* %0 to i64*
   %3 = load i64, i64* %2, align 16
   %4 = getelementptr inbounds <4 x float>, <4 x float>* %0, i64 0, i64 2
   %5 = bitcast float* %4 to i64*
   %6 = load i64, i64* %5, align 8
   %7 = trunc i64 %3 to i32
   %8 = bitcast i32 %7 to float
   %9 = insertelement <4 x float> undef, float %8, i32 0
   %10 = lshr i64 %3, 32
   %11 = trunc i64 %10 to i32
   %12 = bitcast i32 %11 to float
   %13 = insertelement <4 x float> %9, float %12, i32 1
   %14 = trunc i64 %6 to i32
   %15 = bitcast i32 %14 to float
   %16 = insertelement <4 x float> %13, float %15, i32 2
   ret <4 x float> %16
 }
 
 define <4 x float> @load_float4_float3_trunc_0122(<4 x float>* nocapture readonly dereferenceable(16)) {
-; SSE-LABEL: load_float4_float3_trunc_0122:
-; SSE:       # %bb.0:
-; SSE-NEXT:    movss {{.*#+}} xmm1 = mem[0],zero,zero,zero
-; SSE-NEXT:    movaps (%rdi), %xmm0
-; SSE-NEXT:    shufps {{.*#+}} xmm0 = xmm0[0,1],xmm1[0,0]
-; SSE-NEXT:    retq
+; SSE2-LABEL: load_float4_float3_trunc_0122:
+; SSE2:       # %bb.0:
+; SSE2-NEXT:    movss   (%rdi), %xmm0           # xmm0 = mem[0],zero,zero,zero
+; SSE2-NEXT:    movss   4(%rdi), %xmm1          # xmm1 = mem[0],zero,zero,zero
+; SSE2-NEXT:    unpcklps        %xmm1, %xmm0    # xmm0 = xmm0[0],xmm1[0],xmm0[1],xmm1[1]
+; SSE2-NEXT:    movss   8(%rdi), %xmm1          # xmm1 = mem[0],zero,zero,zero
+; SSE2-NEXT:    shufps  $4, %xmm1, %xmm0        # xmm0 = xmm0[0,1],xmm1[0,0]
+; SSE2-NEXT:    retq
+;
+; SSSE3-LABEL: load_float4_float3_trunc_0122:
+; SSSE3:       # %bb.0:
+; SSSE3-NEXT:    movss   (%rdi), %xmm0           # xmm0 = mem[0],zero,zero,zero
+; SSSE3-NEXT:    movss   4(%rdi), %xmm1          # xmm1 = mem[0],zero,zero,zero
+; SSSE3-NEXT:    unpcklps        %xmm1, %xmm0    # xmm0 = xmm0[0],xmm1[0],xmm0[1],xmm1[1]
+; SSSE3-NEXT:    movss   8(%rdi), %xmm1          # xmm1 = mem[0],zero,zero,zero
+; SSSE3-NEXT:    shufps  $4, %xmm1, %xmm0        # xmm0 = xmm0[0,1],xmm1[0,0]
+; SSSE3-NEXT:    retq
+;
+; SSE41-LABEL: load_float4_float3_trunc_0122:
+; SSE41:       # %bb.0:
+; SSE41-NEXT:    movss   (%rdi), %xmm0           # xmm0 = mem[0],zero,zero,zero
+; SSE41-NEXT:    movss   8(%rdi), %xmm1          # xmm1 = mem[0],zero,zero,zero
+; SSE41-NEXT:    insertps        $16, 4(%rdi), %xmm0 # xmm0 = xmm0[0],mem[0],xmm0[2,3]
+; SSE41-NEXT:    shufps  $4, %xmm1, %xmm0        # xmm0 = xmm0[0,1],xmm1[0,0]
+; SSE41-NEXT:    retq
 ;
 ; AVX-LABEL: load_float4_float3_trunc_0122:
 ; AVX:       # %bb.0:
-; AVX-NEXT:    vmovss {{.*#+}} xmm0 = mem[0],zero,zero,zero
-; AVX-NEXT:    vmovaps (%rdi), %xmm1
-; AVX-NEXT:    vshufps {{.*#+}} xmm0 = xmm1[0,1],xmm0[0,0]
+; AVX-NEXT:    vmovss  (%rdi), %xmm0           # xmm0 = mem[0],zero,zero,zero
+; AVX-NEXT:    vmovss  8(%rdi), %xmm1          # xmm1 = mem[0],zero,zero,zero
+; AVX-NEXT:    vinsertps       $16, 4(%rdi), %xmm0, %xmm0 # xmm0 = xmm0[0],mem[0],xmm0[2,3]
+; AVX-NEXT:    vshufps $4, %xmm1, %xmm0, %xmm0 # xmm0 = xmm0[0,1],xmm1[0,0]
 ; AVX-NEXT:    retq
   %2 = bitcast <4 x float>* %0 to i64*
   %3 = load i64, i64* %2, align 16
   %4 = getelementptr inbounds <4 x float>, <4 x float>* %0, i64 0, i64 2
   %5 = bitcast float* %4 to i64*
   %6 = load i64, i64* %5, align 8
   %7 = trunc i64 %3 to i32
   %8 = bitcast i32 %7 to float
   %9 = insertelement <4 x float> undef, float %8, i32 0
   %10 = lshr i64 %3, 32
   %11 = trunc i64 %10 to i32
   %12 = bitcast i32 %11 to float
   %13 = insertelement <4 x float> %9, float %12, i32 1
   %14 = trunc i64 %6 to i32
   %15 = bitcast i32 %14 to float
   %16 = insertelement <4 x float> %13, float %15, i32 2
   %17 = insertelement <4 x float> %16, float %15, i32 3
   ret <4 x float> %17
 }
 
 define <4 x float> @load_float4_float3_trunc_0123(<4 x float>* nocapture readonly dereferenceable(16)) {
 ; SSE2-LABEL: load_float4_float3_trunc_0123:
 ; SSE2:       # %bb.0:
-; SSE2-NEXT:    movaps (%rdi), %xmm0
-; SSE2-NEXT:    movhps {{.*#+}} xmm0 = xmm0[0,1],mem[0,1]
+; SSE2-NEXT:    movss   (%rdi), %xmm0           # xmm0 = mem[0],zero,zero,zero
+; SSE2-NEXT:    movss   4(%rdi), %xmm1          # xmm1 = mem[0],zero,zero,zero
+; SSE2-NEXT:    unpcklps        %xmm1, %xmm0    # xmm0 = xmm0[0],xmm1[0],xmm0[1],xmm1[1]
+; SSE2-NEXT:    movss   8(%rdi), %xmm1          # xmm1 = mem[0],zero,zero,zero
+; SSE2-NEXT:    movss   12(%rdi), %xmm2         # xmm2 = mem[0],zero,zero,zero
+; SSE2-NEXT:    unpcklps        %xmm2, %xmm1    # xmm1 = xmm1[0],xmm2[0],xmm1[1],xmm2[1]
+; SSE2-NEXT:    movlhps %xmm1, %xmm0            # xmm0 = xmm0[0],xmm1[0]
 ; SSE2-NEXT:    retq
 ;
 ; SSSE3-LABEL: load_float4_float3_trunc_0123:
 ; SSSE3:       # %bb.0:
-; SSSE3-NEXT:    movaps (%rdi), %xmm0
-; SSSE3-NEXT:    movhps {{.*#+}} xmm0 = xmm0[0,1],mem[0,1]
+; SSSE3-NEXT:    movss   (%rdi), %xmm0           # xmm0 = mem[0],zero,zero,zero
+; SSSE3-NEXT:    movss   4(%rdi), %xmm1          # xmm1 = mem[0],zero,zero,zero
+; SSSE3-NEXT:    unpcklps        %xmm1, %xmm0    # xmm0 = xmm0[0],xmm1[0],xmm0[1],xmm1[1]
+; SSSE3-NEXT:    movss   8(%rdi), %xmm1          # xmm1 = mem[0],zero,zero,zero
+; SSSE3-NEXT:    movss   12(%rdi), %xmm2         # xmm2 = mem[0],zero,zero,zero
+; SSSE3-NEXT:    unpcklps        %xmm2, %xmm1    # xmm1 = xmm1[0],xmm2[0],xmm1[1],xmm2[1]
+; SSSE3-NEXT:    movlhps %xmm1, %xmm0            # xmm0 = xmm0[0],xmm1[0]
 ; SSSE3-NEXT:    retq
 ;
 ; SSE41-LABEL: load_float4_float3_trunc_0123:
 ; SSE41:       # %bb.0:
-; SSE41-NEXT:    movaps (%rdi), %xmm0
-; SSE41-NEXT:    insertps {{.*#+}} xmm0 = xmm0[0,1],mem[0],xmm0[3]
-; SSE41-NEXT:    insertps {{.*#+}} xmm0 = xmm0[0,1,2],mem[0]
+; SSE41-NEXT:    movss   (%rdi), %xmm0           # xmm0 = mem[0],zero,zero,zero
+; SSE41-NEXT:    insertps        $16, 4(%rdi), %xmm0 # xmm0 = xmm0[0],mem[0],xmm0[2,3]
+; SSE41-NEXT:    insertps        $32, 8(%rdi), %xmm0 # xmm0 = xmm0[0,1],mem[0],xmm0[3]
+; SSE41-NEXT:    insertps        $48, 12(%rdi), %xmm0 # xmm0 = xmm0[0,1,2],mem[0]
 ; SSE41-NEXT:    retq
 ;
 ; AVX-LABEL: load_float4_float3_trunc_0123:
 ; AVX:       # %bb.0:
-; AVX-NEXT:    vmovaps (%rdi), %xmm0
-; AVX-NEXT:    vinsertps {{.*#+}} xmm0 = xmm0[0,1],mem[0],xmm0[3]
-; AVX-NEXT:    vinsertps {{.*#+}} xmm0 = xmm0[0,1,2],mem[0]
+; AVX-NEXT:    vmovss  (%rdi), %xmm0           # xmm0 = mem[0],zero,zero,zero
+; AVX-NEXT:    vinsertps       $16, 4(%rdi), %xmm0, %xmm0 # xmm0 = xmm0[0],mem[0],xmm0[2,3]
+; AVX-NEXT:    vinsertps       $32, 8(%rdi), %xmm0, %xmm0 # xmm0 = xmm0[0,1],mem[0],xmm0[3]
+; AVX-NEXT:    vinsertps       $48, 12(%rdi), %xmm0, %xmm0 # xmm0 = xmm0[0,1,2],mem[0]
 ; AVX-NEXT:    retq
   %2 = bitcast <4 x float>* %0 to i64*
   %3 = load i64, i64* %2, align 16
   %4 = getelementptr inbounds <4 x float>, <4 x float>* %0, i64 0, i64 2
   %5 = bitcast float* %4 to i64*
   %6 = load i64, i64* %5, align 8
   %7 = trunc i64 %3 to i32
   %8 = bitcast i32 %7 to float