[X86] Cleanup type conversion of 64-bit load-store pairs.

[niravd]

Summary

Simplify and generalize chain handling and search for 64-bit load-store pairs.
Nontemporal test now converts 64-bit integer load-store into f64 which it realizes directly instead of splitting into two i32 pairs.

[niravd]

This is missing a fix in the store generation. Fixing now.

[niravd]

Fix chain computation in 64-bit to 2x32-bit path.

[craig.topper]

llvm/lib/Target/X86/X86ISelLowering.cpp

34373 ↗

(On Diff #125793)

Is NewChain an unused variable now?

[craig.topper]

Are we supposed to be losing the non temporal property during this transformation?

[niravd]

Remove unused NewStore definitions.

[niravd]

Are we supposed to be losing the non temporal property during this transformation

It's still listed as a 8 byte nontermporal store in the DAG post transformation. IIUC this should be MOVNTQ.

[craig.topper]

What about the i32 and i64 stores from the original IR? It looks like we emited a movl and a movsd?

[niravd]

Previously this transformation missed the the i64 load/non-temporal store pair and as a result they get split into a i32 operations. Because this aligns with the nontemporal store of the doubleword value in 12(%ebp) another transformation elides that pair (which given what this test look at is probably indicative that this test needs some sort of fencing added).

[craig.topper]

I was wondering why we weren't using movntsd, but I forgot that's an AMD SSE4A instruction.

[niravd]

Rebase past nontemporal store update.

[craig.topper]

LGTM, but I wonder if we should be avoiding nontemporal stores if we can't preserve that property in the generated instructions. I assume that issue exists for other cases even without this patch though.

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

static SDValue combineStore(SDNode *N, SelectionDAG &DAG,
   bool NoImplicitFloatOps = F->hasFnAttribute(Attribute::NoImplicitFloat);
   bool F64IsLegal =
       !Subtarget.useSoftFloat() && !NoImplicitFloatOps && Subtarget.hasSSE2();
   if ((VT.isVector() ||
        (VT == MVT::i64 && F64IsLegal && !Subtarget.is64Bit())) &&
       isa<LoadSDNode>(St->getValue()) &&
       !cast<LoadSDNode>(St->getValue())->isVolatile() &&
       St->getChain().hasOneUse() && !St->isVolatile()) {
-    SDNode* LdVal = St->getValue().getNode();
-    LoadSDNode *Ld = nullptr;
-    int TokenFactorIndex = -1;
+    LoadSDNode *Ld = cast<LoadSDNode>(St->getValue().getNode());
     SmallVector<SDValue, 8> Ops;
-    SDNode* ChainVal = St->getChain().getNode();
-    // Must be a store of a load.  We currently handle two cases:  the load
-    // is a direct child, and it's under an intervening TokenFactor.  It is
-    // possible to dig deeper under nested TokenFactors.
-    if (ChainVal == LdVal)
-      Ld = cast<LoadSDNode>(St->getChain());
-    else if (St->getValue().hasOneUse() &&
-             ChainVal->getOpcode() == ISD::TokenFactor) {
-      for (unsigned i = 0, e = ChainVal->getNumOperands(); i != e; ++i) {
-        if (ChainVal->getOperand(i).getNode() == LdVal) {
-          TokenFactorIndex = i;
-          Ld = cast<LoadSDNode>(St->getValue());
-        } else
-          Ops.push_back(ChainVal->getOperand(i));
-      }
-    }
 
-    if (!Ld || !ISD::isNormalLoad(Ld))
+    if (!ISD::isNormalLoad(Ld))
       return SDValue();
 
     // If this is not the MMX case, i.e. we are just turning i64 load/store
     // into f64 load/store, avoid the transformation if there are multiple
     // uses of the loaded value.
     if (!VT.isVector() && !Ld->hasNUsesOfValue(1, 0))
       return SDValue();
 
     SDLoc LdDL(Ld);
     SDLoc StDL(N);
     // If we are a 64-bit capable x86, lower to a single movq load/store pair.
     // Otherwise, if it's legal to use f64 SSE instructions, use f64 load/store
     // pair instead.
     if (Subtarget.is64Bit() || F64IsLegal) {
       MVT LdVT = Subtarget.is64Bit() ? MVT::i64 : MVT::f64;
       SDValue NewLd = DAG.getLoad(LdVT, LdDL, Ld->getChain(), Ld->getBasePtr(),
-                                  Ld->getPointerInfo(), Ld->getAlignment(),
-                                  Ld->getMemOperand()->getFlags());
+                                  Ld->getMemOperand());
+
       // Make sure new load is placed in same chain order.
-      SDValue NewChain = DAG.makeEquivalentMemoryOrdering(Ld, NewLd);
-      if (TokenFactorIndex >= 0) {
-        Ops.push_back(NewChain);
-        NewChain = DAG.getNode(ISD::TokenFactor, LdDL, MVT::Other, Ops);
-      }
-      return DAG.getStore(NewChain, StDL, NewLd, St->getBasePtr(),
-                          St->getPointerInfo(), St->getAlignment(),
-                          St->getMemOperand()->getFlags());
+      DAG.makeEquivalentMemoryOrdering(Ld, NewLd);
+      return DAG.getStore(St->getChain(), StDL, NewLd, St->getBasePtr(),
+                          St->getMemOperand());
     }
 
     // Otherwise, lower to two pairs of 32-bit loads / stores.
     SDValue LoAddr = Ld->getBasePtr();
     SDValue HiAddr = DAG.getMemBasePlusOffset(LoAddr, 4, LdDL);
 
     SDValue LoLd = DAG.getLoad(MVT::i32, LdDL, Ld->getChain(), LoAddr,
                                Ld->getPointerInfo(), Ld->getAlignment(),
                                Ld->getMemOperand()->getFlags());
     SDValue HiLd = DAG.getLoad(MVT::i32, LdDL, Ld->getChain(), HiAddr,
                                Ld->getPointerInfo().getWithOffset(4),
                                MinAlign(Ld->getAlignment(), 4),
                                Ld->getMemOperand()->getFlags());
     // Make sure new loads are placed in same chain order.
-    SDValue NewChain = DAG.makeEquivalentMemoryOrdering(Ld, LoLd);
-    NewChain = DAG.makeEquivalentMemoryOrdering(Ld, HiLd);
-
-    if (TokenFactorIndex >= 0) {
-      Ops.push_back(NewChain);
-      NewChain = DAG.getNode(ISD::TokenFactor, LdDL, MVT::Other, Ops);
-    }
+    DAG.makeEquivalentMemoryOrdering(Ld, LoLd);
+    DAG.makeEquivalentMemoryOrdering(Ld, HiLd);
 
     LoAddr = St->getBasePtr();
     HiAddr = DAG.getMemBasePlusOffset(LoAddr, 4, StDL);
 
     SDValue LoSt =
-        DAG.getStore(NewChain, StDL, LoLd, LoAddr, St->getPointerInfo(),
+        DAG.getStore(St->getChain(), StDL, LoLd, LoAddr, St->getPointerInfo(),
                      St->getAlignment(), St->getMemOperand()->getFlags());
-    SDValue HiSt = DAG.getStore(
-        NewChain, StDL, HiLd, HiAddr, St->getPointerInfo().getWithOffset(4),
-        MinAlign(St->getAlignment(), 4), St->getMemOperand()->getFlags());
+    SDValue HiSt = DAG.getStore(St->getChain(), StDL, HiLd, HiAddr,
+                                St->getPointerInfo().getWithOffset(4),
+                                MinAlign(St->getAlignment(), 4),
+                                St->getMemOperand()->getFlags());
     return DAG.getNode(ISD::TokenFactor, StDL, MVT::Other, LoSt, HiSt);
   }
 
   // This is similar to the above case, but here we handle a scalar 64-bit
   // integer store that is extracted from a vector on a 32-bit target.
   // If we have SSE2, then we can treat it like a floating-point double
   // to get past legalization. The execution dependencies fixup pass will
   // choose the optimal machine instruction for the store if this really is

llvm/trunk/test/CodeGen/X86/nontemporal.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=i686-unknown-unknown -mattr=+sse2 | FileCheck %s --check-prefix=X32-SSE
 ; RUN: llc < %s -mtriple=i686-unknown-unknown -mattr=+avx | FileCheck %s --check-prefix=X32-AVX
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+sse2 | FileCheck %s --check-prefix=X64-SSE
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+avx | FileCheck %s --check-prefix=X64-AVX
 
 define i32 @f(<4 x float> %A, i8* %B, <2 x double> %C, i32 %D, <2 x i64> %E, <4 x i32> %F, <8 x i16> %G, <16 x i8> %H, i64 %I, i32* %loadptr) nounwind {
 ; X32-SSE-LABEL: f:
 ; X32-SSE:       # %bb.0:
 ; X32-SSE-NEXT:    pushl %ebp
 ; X32-SSE-NEXT:    movl %esp, %ebp
-; X32-SSE-NEXT:    pushl %edi
 ; X32-SSE-NEXT:    pushl %esi
 ; X32-SSE-NEXT:    andl $-16, %esp
-; X32-SSE-NEXT:    movl 76(%ebp), %ecx
+; X32-SSE-NEXT:    subl $16, %esp
+; X32-SSE-NEXT:    movsd {{.*#+}} xmm3 = mem[0],zero
 ; X32-SSE-NEXT:    movl 12(%ebp), %eax
-; X32-SSE-NEXT:    movdqa 56(%ebp), %xmm3
-; X32-SSE-NEXT:    movdqa 40(%ebp), %xmm4
-; X32-SSE-NEXT:    movdqa 24(%ebp), %xmm5
-; X32-SSE-NEXT:    movl 8(%ebp), %esi
-; X32-SSE-NEXT:    movl 80(%ebp), %edx
-; X32-SSE-NEXT:    movl (%edx), %edi
+; X32-SSE-NEXT:    movdqa 56(%ebp), %xmm4
+; X32-SSE-NEXT:    movdqa 40(%ebp), %xmm5
+; X32-SSE-NEXT:    movdqa 24(%ebp), %xmm6
+; X32-SSE-NEXT:    movl 8(%ebp), %edx
+; X32-SSE-NEXT:    movl 80(%ebp), %ecx
+; X32-SSE-NEXT:    movl (%ecx), %esi
 ; X32-SSE-NEXT:    addps {{\.LCPI.*}}, %xmm0
-; X32-SSE-NEXT:    movntps %xmm0, (%esi)
+; X32-SSE-NEXT:    movntps %xmm0, (%edx)
 ; X32-SSE-NEXT:    paddq {{\.LCPI.*}}, %xmm2
-; X32-SSE-NEXT:    addl (%edx), %edi
-; X32-SSE-NEXT:    movntdq %xmm2, (%esi)
+; X32-SSE-NEXT:    addl (%ecx), %esi
+; X32-SSE-NEXT:    movntdq %xmm2, (%edx)
 ; X32-SSE-NEXT:    addpd {{\.LCPI.*}}, %xmm1
-; X32-SSE-NEXT:    addl (%edx), %edi
-; X32-SSE-NEXT:    movntpd %xmm1, (%esi)
-; X32-SSE-NEXT:    paddd {{\.LCPI.*}}, %xmm5
-; X32-SSE-NEXT:    addl (%edx), %edi
-; X32-SSE-NEXT:    movntdq %xmm5, (%esi)
-; X32-SSE-NEXT:    paddw {{\.LCPI.*}}, %xmm4
-; X32-SSE-NEXT:    addl (%edx), %edi
-; X32-SSE-NEXT:    movntdq %xmm4, (%esi)
-; X32-SSE-NEXT:    paddb {{\.LCPI.*}}, %xmm3
-; X32-SSE-NEXT:    addl (%edx), %edi
-; X32-SSE-NEXT:    movntdq %xmm3, (%esi)
-; X32-SSE-NEXT:    addl (%edx), %edi
-; X32-SSE-NEXT:    movntil %eax, (%esi)
-; X32-SSE-NEXT:    movl (%edx), %eax
-; X32-SSE-NEXT:    movntil %ecx, 4(%esi)
-; X32-SSE-NEXT:    movl 72(%ebp), %ecx
-; X32-SSE-NEXT:    movntil %ecx, (%esi)
-; X32-SSE-NEXT:    addl %edi, %eax
-; X32-SSE-NEXT:    addl (%edx), %eax
-; X32-SSE-NEXT:    leal -8(%ebp), %esp
+; X32-SSE-NEXT:    addl (%ecx), %esi
+; X32-SSE-NEXT:    movntpd %xmm1, (%edx)
+; X32-SSE-NEXT:    paddd {{\.LCPI.*}}, %xmm6
+; X32-SSE-NEXT:    addl (%ecx), %esi
+; X32-SSE-NEXT:    movntdq %xmm6, (%edx)
+; X32-SSE-NEXT:    paddw {{\.LCPI.*}}, %xmm5
+; X32-SSE-NEXT:    addl (%ecx), %esi
+; X32-SSE-NEXT:    movntdq %xmm5, (%edx)
+; X32-SSE-NEXT:    paddb {{\.LCPI.*}}, %xmm4
+; X32-SSE-NEXT:    addl (%ecx), %esi
+; X32-SSE-NEXT:    movntdq %xmm4, (%edx)
+; X32-SSE-NEXT:    addl (%ecx), %esi
+; X32-SSE-NEXT:    movntil %eax, (%edx)
+; X32-SSE-NEXT:    movl (%ecx), %eax
+; X32-SSE-NEXT:    addl %esi, %eax
+; X32-SSE-NEXT:    movsd %xmm3, (%edx)
+; X32-SSE-NEXT:    addl (%ecx), %eax
+; X32-SSE-NEXT:    leal -4(%ebp), %esp
 ; X32-SSE-NEXT:    popl %esi
-; X32-SSE-NEXT:    popl %edi
 ; X32-SSE-NEXT:    popl %ebp
 ; X32-SSE-NEXT:    retl
 ;
 ; X32-AVX-LABEL: f:
 ; X32-AVX:       # %bb.0:
 ; X32-AVX-NEXT:    pushl %ebp
 ; X32-AVX-NEXT:    movl %esp, %ebp
-; X32-AVX-NEXT:    pushl %edi
 ; X32-AVX-NEXT:    pushl %esi
 ; X32-AVX-NEXT:    andl $-16, %esp
-; X32-AVX-NEXT:    movl 76(%ebp), %ecx
+; X32-AVX-NEXT:    subl $16, %esp
+; X32-AVX-NEXT:    vmovsd {{.*#+}} xmm3 = mem[0],zero
 ; X32-AVX-NEXT:    movl 12(%ebp), %eax
-; X32-AVX-NEXT:    vmovdqa 56(%ebp), %xmm3
-; X32-AVX-NEXT:    vmovdqa 40(%ebp), %xmm4
-; X32-AVX-NEXT:    vmovdqa 24(%ebp), %xmm5
-; X32-AVX-NEXT:    movl 8(%ebp), %esi
+; X32-AVX-NEXT:    vmovdqa 56(%ebp), %xmm4
+; X32-AVX-NEXT:    vmovdqa 40(%ebp), %xmm5
+; X32-AVX-NEXT:    vmovdqa 24(%ebp), %xmm6
+; X32-AVX-NEXT:    movl 8(%ebp), %ecx
 ; X32-AVX-NEXT:    movl 80(%ebp), %edx
-; X32-AVX-NEXT:    movl (%edx), %edi
+; X32-AVX-NEXT:    movl (%edx), %esi
 ; X32-AVX-NEXT:    vaddps {{\.LCPI.*}}, %xmm0, %xmm0
-; X32-AVX-NEXT:    vmovntps %xmm0, (%esi)
+; X32-AVX-NEXT:    vmovntps %xmm0, (%ecx)
 ; X32-AVX-NEXT:    vpaddq {{\.LCPI.*}}, %xmm2, %xmm0
-; X32-AVX-NEXT:    addl (%edx), %edi
-; X32-AVX-NEXT:    vmovntdq %xmm0, (%esi)
+; X32-AVX-NEXT:    addl (%edx), %esi
+; X32-AVX-NEXT:    vmovntdq %xmm0, (%ecx)
 ; X32-AVX-NEXT:    vaddpd {{\.LCPI.*}}, %xmm1, %xmm0
-; X32-AVX-NEXT:    addl (%edx), %edi
-; X32-AVX-NEXT:    vmovntpd %xmm0, (%esi)
-; X32-AVX-NEXT:    vpaddd {{\.LCPI.*}}, %xmm5, %xmm0
-; X32-AVX-NEXT:    addl (%edx), %edi
-; X32-AVX-NEXT:    vmovntdq %xmm0, (%esi)
-; X32-AVX-NEXT:    vpaddw {{\.LCPI.*}}, %xmm4, %xmm0
-; X32-AVX-NEXT:    addl (%edx), %edi
-; X32-AVX-NEXT:    vmovntdq %xmm0, (%esi)
-; X32-AVX-NEXT:    vpaddb {{\.LCPI.*}}, %xmm3, %xmm0
-; X32-AVX-NEXT:    addl (%edx), %edi
-; X32-AVX-NEXT:    vmovntdq %xmm0, (%esi)
-; X32-AVX-NEXT:    addl (%edx), %edi
-; X32-AVX-NEXT:    movntil %eax, (%esi)
+; X32-AVX-NEXT:    addl (%edx), %esi
+; X32-AVX-NEXT:    vmovntpd %xmm0, (%ecx)
+; X32-AVX-NEXT:    vpaddd {{\.LCPI.*}}, %xmm6, %xmm0
+; X32-AVX-NEXT:    addl (%edx), %esi
+; X32-AVX-NEXT:    vmovntdq %xmm0, (%ecx)
+; X32-AVX-NEXT:    vpaddw {{\.LCPI.*}}, %xmm5, %xmm0
+; X32-AVX-NEXT:    addl (%edx), %esi
+; X32-AVX-NEXT:    vmovntdq %xmm0, (%ecx)
+; X32-AVX-NEXT:    vpaddb {{\.LCPI.*}}, %xmm4, %xmm0
+; X32-AVX-NEXT:    addl (%edx), %esi
+; X32-AVX-NEXT:    vmovntdq %xmm0, (%ecx)
+; X32-AVX-NEXT:    addl (%edx), %esi
+; X32-AVX-NEXT:    movntil %eax, (%ecx)
 ; X32-AVX-NEXT:    movl (%edx), %eax
-; X32-AVX-NEXT:    movntil %ecx, 4(%esi)
-; X32-AVX-NEXT:    movl 72(%ebp), %ecx
-; X32-AVX-NEXT:    movntil %ecx, (%esi)
-; X32-AVX-NEXT:    addl %edi, %eax
+; X32-AVX-NEXT:    addl %esi, %eax
+; X32-AVX-NEXT:    vmovsd %xmm3, (%ecx)
 ; X32-AVX-NEXT:    addl (%edx), %eax
-; X32-AVX-NEXT:    leal -8(%ebp), %esp
+; X32-AVX-NEXT:    leal -4(%ebp), %esp
 ; X32-AVX-NEXT:    popl %esi
-; X32-AVX-NEXT:    popl %edi
 ; X32-AVX-NEXT:    popl %ebp
 ; X32-AVX-NEXT:    retl
 ;
 ; X64-SSE-LABEL: f:
 ; X64-SSE:       # %bb.0:
 ; X64-SSE-NEXT:    movl (%rcx), %eax
 ; X64-SSE-NEXT:    addps {{.*}}(%rip), %xmm0
 ; X64-SSE-NEXT:    movntps %xmm0, (%rdi)