[x86] avoid vector load narrowing with extracted store uses (PR42305)

[spatel]

Summary

This is an exception to the rule that we should prefer xmm ops to ymm ops. As shown in PR42305:
https://bugs.llvm.org/show_bug.cgi?id=42305
...the store folding opportunity with vextractf128 may result in better perf by reducing the instruction count.

[craig.topper]

llvm/lib/Target/X86/X86ISelLowering.cpp

4819 ↗

(On Diff #205429)

I wonder if it would be better to use use_begin()/use_end() for the loop. Then you can use getUse().getResNo() to check if its the chain or not. That would be more efficient than scanning the operands of the user.

[spatel]

Patch updated:
Use iterator loop rather than range loop. That allows checking the result number more efficiently from the SDUse.

[RKSimon]

LGTM - cheers

llvm/test/CodeGen/X86/sandybridge-loads.ll

44 ↗

(On Diff #205600)

Not (very) related - but do we need a variant of this with that has an unaligned load:

%v0 = load <8 x float>, <8 x float>* %a, align 32 ; <--- aligned
%v1 = load <8 x float>, <8 x float>* %b, align 16 ; <--- unaligned
store <8 x float> %v0, <8 x float>* %b, align 32 ; <--- aligned
store <8 x float> %v1, <8 x float>* %a, align 16 ; <--- unaligned
ret void

[spatel]

llvm/test/CodeGen/X86/sandybridge-loads.ll

44 ↗

(On Diff #205600)

I think for the purpose of this patch, that's covered by the next test diff (in widen_load-3.ll).

And for the purpose of SandyBridge-specific (actual CPU feature is -slow-unaligned-mem-32), the test just above here covers it. But I can add that suggestion for completeness.

llvm/trunk/lib/Target/X86/X86ISelLowering.cpp

bool X86TargetLowering::shouldReduceLoadWidth(SDNode *Load,
                                               ISD::LoadExtType ExtTy,
                                               EVT NewVT) const {
   // "ELF Handling for Thread-Local Storage" specifies that R_X86_64_GOTTPOFF
   // relocation target a movq or addq instruction: don't let the load shrink.
   SDValue BasePtr = cast<LoadSDNode>(Load)->getBasePtr();
   if (BasePtr.getOpcode() == X86ISD::WrapperRIP)
     if (const auto *GA = dyn_cast<GlobalAddressSDNode>(BasePtr.getOperand(0)))
       return GA->getTargetFlags() != X86II::MO_GOTTPOFF;
+
+  // If this is an (1) AVX vector load with (2) multiple uses and (3) all of
+  // those uses are extracted directly into a store, then the extract + store
+  // can be store-folded. Therefore, it's probably not worth splitting the load.
+  EVT VT = Load->getValueType(0);
+  if ((VT.is256BitVector() || VT.is512BitVector()) && !Load->hasOneUse()) {
+    for (auto UI = Load->use_begin(), UE = Load->use_end(); UI != UE; ++UI) {
+      // Skip uses of the chain value. Result 0 of the node is the load value.
+      if (UI.getUse().getResNo() != 0)
+        continue;
+
+      // If this use is not an extract + store, it's probably worth splitting.
+      if (UI->getOpcode() != ISD::EXTRACT_SUBVECTOR || !UI->hasOneUse() ||
+          UI->use_begin()->getOpcode() != ISD::STORE)
+        return true;
+    }
+    // All non-chain uses are extract + store.
+    return false;
+  }
+
   return true;
 }
 
 /// Returns true if it is beneficial to convert a load of a constant
 /// to just the constant itself.
 bool X86TargetLowering::shouldConvertConstantLoadToIntImm(const APInt &Imm,
                                                           Type *Ty) const {
   assert(Ty->isIntegerTy());

llvm/trunk/test/CodeGen/X86/sandybridge-loads.ll

; CHECK-NEXT:    retq
   store <8 x i32> %r, <8 x i32>* undef, align 32
   ret void
 }
 
 define void @widestores(<8 x float>* %a, <8 x float>* %b, <8 x float>* %c) nounwind {
 ; CHECK-LABEL: widestores:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    vmovaps (%rdi), %ymm0
-; CHECK-NEXT:    vmovaps (%rsi), %xmm1
-; CHECK-NEXT:    vmovaps 16(%rsi), %xmm2
+; CHECK-NEXT:    vmovaps (%rsi), %ymm1
 ; CHECK-NEXT:    vmovaps %ymm0, (%rsi)
-; CHECK-NEXT:    vmovaps %xmm2, 16(%rdi)
+; CHECK-NEXT:    vextractf128 $1, %ymm1, 16(%rdi)
 ; CHECK-NEXT:    vmovaps %xmm1, (%rdi)
 ; CHECK-NEXT:    vzeroupper
 ; CHECK-NEXT:    retq
   %v0 = load <8 x float>, <8 x float>* %a, align 32
   %v1 = load <8 x float>, <8 x float>* %b, align 32
   store <8 x float> %v0, <8 x float>* %b, align 32 ; <--- aligned
   store <8 x float> %v1, <8 x float>* %a, align 16 ; <--- unaligned
   ret void

llvm/trunk/test/CodeGen/X86/widen_load-3.ll

 ; X86-SSE-NEXT:    movups %xmm1, (%eax)
 ; X86-SSE-NEXT:    retl
 ;
 ; X86-AVX-LABEL: load_split:
 ; X86-AVX:       # %bb.0:
 ; X86-AVX-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X86-AVX-NEXT:    movl {{[0-9]+}}(%esp), %ecx
 ; X86-AVX-NEXT:    movl {{[0-9]+}}(%esp), %edx
-; X86-AVX-NEXT:    vmovups (%edx), %xmm0
-; X86-AVX-NEXT:    vmovups 16(%edx), %xmm1
+; X86-AVX-NEXT:    vmovups (%edx), %ymm0
 ; X86-AVX-NEXT:    vmovups %xmm0, (%ecx)
-; X86-AVX-NEXT:    vmovups %xmm1, (%eax)
+; X86-AVX-NEXT:    vextractf128 $1, %ymm0, (%eax)
+; X86-AVX-NEXT:    vzeroupper
 ; X86-AVX-NEXT:    retl
 ;
 ; X64-SSE-LABEL: load_split:
 ; X64-SSE:       # %bb.0:
 ; X64-SSE-NEXT:    movups (%rdi), %xmm0
 ; X64-SSE-NEXT:    movups 16(%rdi), %xmm1
 ; X64-SSE-NEXT:    movups %xmm0, (%rsi)
 ; X64-SSE-NEXT:    movups %xmm1, (%rdx)
 ; X64-SSE-NEXT:    retq
 ;
 ; X64-AVX-LABEL: load_split:
 ; X64-AVX:       # %bb.0:
-; X64-AVX-NEXT:    vmovups (%rdi), %xmm0
-; X64-AVX-NEXT:    vmovups 16(%rdi), %xmm1
+; X64-AVX-NEXT:    vmovups (%rdi), %ymm0
 ; X64-AVX-NEXT:    vmovups %xmm0, (%rsi)
-; X64-AVX-NEXT:    vmovups %xmm1, (%rdx)
+; X64-AVX-NEXT:    vextractf128 $1, %ymm0, (%rdx)
+; X64-AVX-NEXT:    vzeroupper
 ; X64-AVX-NEXT:    retq
   %t256 = load <8 x float>, <8 x float>* %ld, align 1
   %b128 = shufflevector <8 x float> %t256, <8 x float> undef, <4 x i32> <i32 0, i32 1, i32 2, i32 3>
   store <4 x float> %b128, <4 x float>* %st1, align 1
   %t128 = shufflevector <8 x float> %t256, <8 x float> undef, <4 x i32> <i32 4, i32 5, i32 6, i32 7>
   store <4 x float> %t128, <4 x float>* %st2, align 1
   ret void
 }
 
 define void @load_split_more(float* %src, i32* %idx, float* %dst) nounwind {
 ; X86-SSE-LABEL: load_split_more:
 ; X86-SSE:       # %bb.0:
-; X86-SSE-NEXT:    pushl %esi
 ; X86-SSE-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X86-SSE-NEXT:    movl {{[0-9]+}}(%esp), %ecx
 ; X86-SSE-NEXT:    movl {{[0-9]+}}(%esp), %edx
-; X86-SSE-NEXT:    movl (%edx), %esi
-; X86-SSE-NEXT:    movups (%ecx), %xmm0
-; X86-SSE-NEXT:    movups 16(%ecx), %xmm1
-; X86-SSE-NEXT:    movups %xmm0, (%eax,%esi,4)
-; X86-SSE-NEXT:    movl 4(%edx), %ecx
+; X86-SSE-NEXT:    movups (%edx), %xmm0
+; X86-SSE-NEXT:    movups 16(%edx), %xmm1
+; X86-SSE-NEXT:    movl (%ecx), %edx
+; X86-SSE-NEXT:    movups %xmm0, (%eax,%edx,4)
+; X86-SSE-NEXT:    movl 4(%ecx), %ecx
 ; X86-SSE-NEXT:    movups %xmm1, (%eax,%ecx,4)
-; X86-SSE-NEXT:    popl %esi
 ; X86-SSE-NEXT:    retl
 ;
 ; X86-AVX-LABEL: load_split_more:
 ; X86-AVX:       # %bb.0:
-; X86-AVX-NEXT:    pushl %esi
 ; X86-AVX-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X86-AVX-NEXT:    movl {{[0-9]+}}(%esp), %ecx
 ; X86-AVX-NEXT:    movl {{[0-9]+}}(%esp), %edx
-; X86-AVX-NEXT:    movl (%edx), %esi
-; X86-AVX-NEXT:    vmovups (%ecx), %xmm0
-; X86-AVX-NEXT:    vmovups 16(%ecx), %xmm1
-; X86-AVX-NEXT:    vmovups %xmm0, (%eax,%esi,4)
-; X86-AVX-NEXT:    movl 4(%edx), %ecx
-; X86-AVX-NEXT:    vmovups %xmm1, (%eax,%ecx,4)
-; X86-AVX-NEXT:    popl %esi
+; X86-AVX-NEXT:    vmovups (%edx), %ymm0
+; X86-AVX-NEXT:    movl (%ecx), %edx
+; X86-AVX-NEXT:    vmovups %xmm0, (%eax,%edx,4)
+; X86-AVX-NEXT:    movl 4(%ecx), %ecx
+; X86-AVX-NEXT:    vextractf128 $1, %ymm0, (%eax,%ecx,4)
+; X86-AVX-NEXT:    vzeroupper
 ; X86-AVX-NEXT:    retl
 ;
 ; X64-SSE-LABEL: load_split_more:
 ; X64-SSE:       # %bb.0:
-; X64-SSE-NEXT:    movslq (%rsi), %rax
 ; X64-SSE-NEXT:    movups (%rdi), %xmm0
 ; X64-SSE-NEXT:    movups 16(%rdi), %xmm1
+; X64-SSE-NEXT:    movslq (%rsi), %rax
 ; X64-SSE-NEXT:    movups %xmm0, (%rdx,%rax,4)
 ; X64-SSE-NEXT:    movslq 4(%rsi), %rax
 ; X64-SSE-NEXT:    movups %xmm1, (%rdx,%rax,4)
 ; X64-SSE-NEXT:    retq
 ;
 ; X64-AVX-LABEL: load_split_more:
 ; X64-AVX:       # %bb.0:
+; X64-AVX-NEXT:    vmovups (%rdi), %ymm0
 ; X64-AVX-NEXT:    movslq (%rsi), %rax
-; X64-AVX-NEXT:    vmovups (%rdi), %xmm0
-; X64-AVX-NEXT:    vmovups 16(%rdi), %xmm1
 ; X64-AVX-NEXT:    vmovups %xmm0, (%rdx,%rax,4)
 ; X64-AVX-NEXT:    movslq 4(%rsi), %rax
-; X64-AVX-NEXT:    vmovups %xmm1, (%rdx,%rax,4)
+; X64-AVX-NEXT:    vextractf128 $1, %ymm0, (%rdx,%rax,4)
+; X64-AVX-NEXT:    vzeroupper
 ; X64-AVX-NEXT:    retq
   %v.i = bitcast float* %src to <8 x float>*
   %tmp = load <8 x float>, <8 x float>* %v.i, align 1
   %tmp1 = load i32, i32* %idx, align 4
   %idx.ext = sext i32 %tmp1 to i64
   %add.ptr1 = getelementptr inbounds float, float* %dst, i64 %idx.ext
   %extract = shufflevector <8 x float> %tmp, <8 x float> undef, <4 x i32> <i32 0, i32 1, i32 2, i32 3>
   %v.i11 = bitcast float* %add.ptr1 to <4 x float>*