[X86][AVX] Access a scalar float/double as a free extract from a broadcast load (PR43217)

[RKSimon]

Summary

If a fp scalar is loaded and then used as both a scalar and a vector broadcast, perform the load as a broadcast and then extract the scalar for 'free' from the 0th element.

This involved switching the order of the X86ISD::BROADCAST combines so we only convert to X86ISD::BROADCAST_LOAD once all other canonicalizations have been attempted.

Fixed PR43217

[craig.topper]

llvm/lib/Target/X86/X86ISelLowering.cpp

33458 ↗

(On Diff #223387)

Should we be using the same pattern we use for forming ExtLoads and truncating other users?

// If the load value is used only by N, replace it via CombineTo N.
bool NoReplaceTrunc = SDValue(LN0, 0).hasOneUse();
Combiner.CombineTo(N, ExtLoad);
if (NoReplaceTrunc) {
  DAG.ReplaceAllUsesOfValueWith(SDValue(LN0, 1), ExtLoad.getValue(1));
  Combiner.recursivelyDeleteUnusedNodes(LN0);
} else {
  SDValue Trunc =
      DAG.getNode(ISD::TRUNCATE, SDLoc(N0), N0.getValueType(), ExtLoad);
  Combiner.CombineTo(LN0, Trunc, ExtLoad.getValue(1));
}
return SDValue(N, 0); // Return N so it doesn't get rechecked!

[RKSimon]

Use same pattern as ExtLoads

[craig.topper]

LGTM

[dantrushin]

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

33437

Shouldn't we check for simple types equality here?
One of my fuzzy tests fails because we try to replace

t589: v8i16 = X86ISD::VBROADCAST t5, Test::iMeth-482306:0

with

v8i32,ch = X86ISD::VBROADCAST_LOAD<(load 4 from %ir.52, addrspace 1)> t5:1, t30, Test::iMeth-482306:0
t680: v4i32 = extract_subvector t679, Constant:i64<0>, Test::iMeth-482306:0

Which result in assertion violation in SelectionDAG::ReplaceAllUsesWith(), because types are incompatible...

[RKSimon]

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

33437

@dantrushin Please can you send me a test case?

[dantrushin]

Sure, but it will take some time to minimize it etc.
Meantime, my original comment was wrong - basically, I have these DAG nodes:

t5: i16,ch = AtomicLoad<(load unordered 2 from %ir.51, align 8)> t0, t4, Test::iMeth-482306:0
...
 t589: v8i16 = X86ISD::VBROADCAST t5, Test::iMeth-482306:0
...
 t679: v8i32,ch = X86ISD::VBROADCAST_LOAD<(load 4 from %ir.52)> t5:1, t30, Test::iMeth-482306:0

t5:1 here is a chain, so we apparently combine unrelated nodes here?
Looks like VBROADCAST_LOAD needs a bit more checks?

[RKSimon]

@dantrushin did @craig.topper 's change at rL374862 solve your issue?

[dantrushin]

D68544.ll1 KBDownload

I noticed that this problem has been fixed already, but I'm attaching reduced testcase anyway

[dantrushin]

In D68544#1709112, @RKSimon wrote:

@dantrushin did @craig.topper 's change at rL374862 solve your issue?

Yes, it did. Thanks!

llvm/trunk/include/llvm/CodeGen/TargetLowering.h

public:
     CombineLevel getDAGCombineLevel() { return Level; }
     bool isCalledByLegalizer() const { return CalledByLegalizer; }
 
     void AddToWorklist(SDNode *N);
     SDValue CombineTo(SDNode *N, ArrayRef<SDValue> To, bool AddTo = true);
     SDValue CombineTo(SDNode *N, SDValue Res, bool AddTo = true);
     SDValue CombineTo(SDNode *N, SDValue Res0, SDValue Res1, bool AddTo = true);
 
+    bool recursivelyDeleteUnusedNodes(SDNode *N);
+
     void CommitTargetLoweringOpt(const TargetLoweringOpt &TLO);
   };
 
   /// Return if the N is a constant or constant vector equal to the true value
   /// from getBooleanContents().
   bool isConstTrueVal(const SDNode *N) const;
 
   /// Return if the N is a constant or constant vector equal to the false value

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

CombineTo(SDNode *N, SDValue Res, bool AddTo) {
   return ((DAGCombiner*)DC)->CombineTo(N, Res, AddTo);
 }
 
 SDValue TargetLowering::DAGCombinerInfo::
 CombineTo(SDNode *N, SDValue Res0, SDValue Res1, bool AddTo) {
   return ((DAGCombiner*)DC)->CombineTo(N, Res0, Res1, AddTo);
 }
 
+bool TargetLowering::DAGCombinerInfo::
+recursivelyDeleteUnusedNodes(SDNode *N) {
+  return ((DAGCombiner*)DC)->recursivelyDeleteUnusedNodes(N);
+}
+
 void TargetLowering::DAGCombinerInfo::
 CommitTargetLoweringOpt(const TargetLowering::TargetLoweringOpt &TLO) {
   return ((DAGCombiner*)DC)->CommitTargetLoweringOpt(TLO);
 }
 
 //===----------------------------------------------------------------------===//
 // Helper Functions
 //===----------------------------------------------------------------------===//

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

case X86ISD::VBROADCAST: {
     if (SrcVT.getSizeInBits() > 128)
       return DAG.getNode(X86ISD::VBROADCAST, DL, VT,
                          extract128BitVector(Src, 0, DAG, DL));
 
     // broadcast(scalar_to_vector(x)) -> broadcast(x).
     if (Src.getOpcode() == ISD::SCALAR_TO_VECTOR)
       return DAG.getNode(X86ISD::VBROADCAST, DL, VT, Src.getOperand(0));
 
+    // Share broadcast with the longest vector and extract low subvector (free).
+    for (SDNode *User : Src->uses())
+      if (User != N.getNode() &&
+          (User->getOpcode() == X86ISD::VBROADCAST ||
+           User->getOpcode() == X86ISD::VBROADCAST_LOAD) &&
+          User->getValueSizeInBits(0) > VT.getSizeInBits()) {
+        return extractSubVector(SDValue(User, 0), 0, DAG, DL,
+                                VT.getSizeInBits());
+      }
+
     // vbroadcast(scalarload X) -> vbroadcast_load X
-    if (!SrcVT.isVector() && Src.hasOneUse() &&
+    // For float loads, extract other uses of the scalar from the broadcast.
+    if (!SrcVT.isVector() && (Src.hasOneUse() || VT.isFloatingPoint()) &&
         ISD::isNormalLoad(Src.getNode())) {
       LoadSDNode *LN = cast<LoadSDNode>(Src);
       SDVTList Tys = DAG.getVTList(VT, MVT::Other);
       SDValue Ops[] = { LN->getChain(), LN->getBasePtr() };
       SDValue BcastLd =
           DAG.getMemIntrinsicNode(X86ISD::VBROADCAST_LOAD, DL, Tys, Ops,
                                   LN->getMemoryVT(), LN->getMemOperand());
+      // If the load value is used only by N, replace it via CombineTo N.
+      bool NoReplaceExtract = Src.hasOneUse();
+      DCI.CombineTo(N.getNode(), BcastLd);
+      if (NoReplaceExtract) {
         DAG.ReplaceAllUsesOfValueWith(SDValue(LN, 1), BcastLd.getValue(1));
-      return BcastLd;
+        DCI.recursivelyDeleteUnusedNodes(LN);
+      } else {
+        SDValue Scl = DAG.getNode(ISD::EXTRACT_VECTOR_ELT, DL, SrcVT, BcastLd,
+                                  DAG.getIntPtrConstant(0, DL));
+        DCI.CombineTo(LN, Scl, BcastLd.getValue(1));
       }
-
-    // Share broadcast with the longest vector and extract low subvector (free).
-    for (SDNode *User : Src->uses())
-      if (User != N.getNode() && User->getOpcode() == X86ISD::VBROADCAST &&
-          User->getValueSizeInBits(0) > VT.getSizeInBits()) {
-        return extractSubVector(SDValue(User, 0), 0, DAG, DL,
-                                VT.getSizeInBits());
-      }
+      return N; // Return N so it doesn't get rechecked!
 
     return SDValue();
   }
   case X86ISD::BLENDI: {
     SDValue N0 = N.getOperand(0);
     SDValue N1 = N.getOperand(1);
 
     // blend(bitcast(x),bitcast(y)) -> bitcast(blend(x,y)) to narrower types.

llvm/trunk/test/CodeGen/X86/avx-vbroadcast.ll

entry:
   ret <4 x double> %vecinit6.i
 }
 
 define <4 x double> @C2(double* %ptr, double* %ptr2) nounwind uwtable readnone ssp {
 ; X32-LABEL: C2:
 ; X32:       ## %bb.0: ## %entry
 ; X32-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
-; X32-NEXT:    vmovsd {{.*#+}} xmm0 = mem[0],zero
-; X32-NEXT:    vmovsd %xmm0, (%eax)
-; X32-NEXT:    vmovddup {{.*#+}} xmm0 = xmm0[0,0]
-; X32-NEXT:    vinsertf128 $1, %xmm0, %ymm0, %ymm0
+; X32-NEXT:    vbroadcastsd (%ecx), %ymm0
+; X32-NEXT:    vmovlps %xmm0, (%eax)
 ; X32-NEXT:    retl
 ;
 ; X64-LABEL: C2:
 ; X64:       ## %bb.0: ## %entry
-; X64-NEXT:    vmovsd {{.*#+}} xmm0 = mem[0],zero
-; X64-NEXT:    vmovsd %xmm0, (%rsi)
-; X64-NEXT:    vmovddup {{.*#+}} xmm0 = xmm0[0,0]
-; X64-NEXT:    vinsertf128 $1, %xmm0, %ymm0, %ymm0
+; X64-NEXT:    vbroadcastsd (%rdi), %ymm0
+; X64-NEXT:    vmovlps %xmm0, (%rsi)
 ; X64-NEXT:    retq
 entry:
   %q = load double, double* %ptr, align 8
   store double %q, double* %ptr2, align 8 ; to create a chain to prevent broadcast
   %vecinit.i = insertelement <4 x double> undef, double %q, i32 0
   %vecinit2.i = insertelement <4 x double> %vecinit.i, double %q, i32 1
   %vecinit4.i = insertelement <4 x double> %vecinit2.i, double %q, i32 2
   %vecinit6.i = insertelement <4 x double> %vecinit4.i, double %q, i32 3
entry:
   ret <8 x float> %vecinit14.i
 }
 
 define <8 x float> @D3(float* %ptr, float* %ptr2) nounwind uwtable readnone ssp {
 ; X32-LABEL: D3:
 ; X32:       ## %bb.0: ## %entry
 ; X32-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
-; X32-NEXT:    vmovss {{.*#+}} xmm0 = mem[0],zero,zero,zero
+; X32-NEXT:    vbroadcastss (%ecx), %ymm0
 ; X32-NEXT:    vmovss %xmm0, (%eax)
-; X32-NEXT:    vpermilps {{.*#+}} xmm0 = xmm0[0,0,0,0]
-; X32-NEXT:    vinsertf128 $1, %xmm0, %ymm0, %ymm0
 ; X32-NEXT:    retl
 ;
 ; X64-LABEL: D3:
 ; X64:       ## %bb.0: ## %entry
-; X64-NEXT:    vmovss {{.*#+}} xmm0 = mem[0],zero,zero,zero
+; X64-NEXT:    vbroadcastss (%rdi), %ymm0
 ; X64-NEXT:    vmovss %xmm0, (%rsi)
-; X64-NEXT:    vpermilps {{.*#+}} xmm0 = xmm0[0,0,0,0]
-; X64-NEXT:    vinsertf128 $1, %xmm0, %ymm0, %ymm0
 ; X64-NEXT:    retq
 entry:
   %q = load float, float* %ptr, align 4
   store float %q, float* %ptr2, align 4 ; to create a chain to prevent broadcast
   %vecinit.i = insertelement <8 x float> undef, float %q, i32 0
   %vecinit2.i = insertelement <8 x float> %vecinit.i, float %q, i32 1
   %vecinit4.i = insertelement <8 x float> %vecinit2.i, float %q, i32 2
   %vecinit6.i = insertelement <8 x float> %vecinit4.i, float %q, i32 3
entry:
   ret <4 x float> %vecinit6.i
 }
 
 define <4 x float> @e2(float* %ptr, float* %ptr2) nounwind uwtable readnone ssp {
 ; X32-LABEL: e2:
 ; X32:       ## %bb.0: ## %entry
 ; X32-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
-; X32-NEXT:    vmovss {{.*#+}} xmm0 = mem[0],zero,zero,zero
+; X32-NEXT:    vbroadcastss (%ecx), %xmm0
 ; X32-NEXT:    vmovss %xmm0, (%eax)
-; X32-NEXT:    vpermilps {{.*#+}} xmm0 = xmm0[0,0,0,0]
 ; X32-NEXT:    retl
 ;
 ; X64-LABEL: e2:
 ; X64:       ## %bb.0: ## %entry
-; X64-NEXT:    vmovss {{.*#+}} xmm0 = mem[0],zero,zero,zero
+; X64-NEXT:    vbroadcastss (%rdi), %xmm0
 ; X64-NEXT:    vmovss %xmm0, (%rsi)
-; X64-NEXT:    vpermilps {{.*#+}} xmm0 = xmm0[0,0,0,0]
 ; X64-NEXT:    retq
 entry:
   %q = load float, float* %ptr, align 4
   store float %q, float* %ptr2, align 4 ; to create a chain to prevent broadcast
   %vecinit.i = insertelement <4 x float> undef, float %q, i32 0
   %vecinit2.i = insertelement <4 x float> %vecinit.i, float %q, i32 1
   %vecinit4.i = insertelement <4 x float> %vecinit2.i, float %q, i32 2
   %vecinit6.i = insertelement <4 x float> %vecinit4.i, float %q, i32 3
entry:
   ret <2 x double> %vecinit2.i
 }
 
 define <2 x double> @I2(double* %ptr, double* %ptr2) nounwind uwtable readnone ssp {
 ; X32-LABEL: I2:
 ; X32:       ## %bb.0: ## %entry
 ; X32-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
-; X32-NEXT:    vmovsd {{.*#+}} xmm0 = mem[0],zero
-; X32-NEXT:    vmovsd %xmm0, (%eax)
-; X32-NEXT:    vmovddup {{.*#+}} xmm0 = xmm0[0,0]
+; X32-NEXT:    vmovddup {{.*#+}} xmm0 = mem[0,0]
+; X32-NEXT:    vmovlps %xmm0, (%eax)
 ; X32-NEXT:    retl
 ;
 ; X64-LABEL: I2:
 ; X64:       ## %bb.0: ## %entry
-; X64-NEXT:    vmovsd {{.*#+}} xmm0 = mem[0],zero
-; X64-NEXT:    vmovsd %xmm0, (%rsi)
-; X64-NEXT:    vmovddup {{.*#+}} xmm0 = xmm0[0,0]
+; X64-NEXT:    vmovddup {{.*#+}} xmm0 = mem[0,0]
+; X64-NEXT:    vmovlps %xmm0, (%rsi)
 ; X64-NEXT:    retq
 entry:
   %q = load double, double* %ptr, align 4
   store double %q, double* %ptr2, align 4 ; to create a chain to prevent broadcast
   %vecinit.i = insertelement <2 x double> undef, double %q, i32 0
   %vecinit2.i = insertelement <2 x double> %vecinit.i, double %q, i32 1
   ret <2 x double> %vecinit2.i
 }
; X64-NEXT:    retq
   %3 = shufflevector <8 x i32> %2, <8 x i32> undef, <8 x i32> zeroinitializer
   %4 = shufflevector <8 x i32> undef, <8 x i32> %3, <16 x i32> <i32 0, i32 8, i32 1, i32 9, i32 2, i32 10, i32 3, i32 11, i32 4, i32 12, i32 5, i32 13, i32 6, i32 14, i32 7, i32 15>
   store <16 x i32> %4, <16 x i32>* %b, align 4
   ret void
 }
 
 ;
 ; Broadcast scale factor for xyz vector - slp will have vectorized xy.
-; FIXME: Load as a broadcast and then use the scalar 0'th element.
 ;
 define double @broadcast_scale_xyz(double* nocapture readonly, double* nocapture readonly) nounwind {
 ; X32-LABEL: broadcast_scale_xyz:
 ; X32:       ## %bb.0:
 ; X32-NEXT:    subl $12, %esp
 ; X32-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
-; X32-NEXT:    vmovsd {{.*#+}} xmm0 = mem[0],zero
-; X32-NEXT:    vmovddup {{.*#+}} xmm1 = xmm0[0,0]
-; X32-NEXT:    vmulpd (%eax), %xmm1, %xmm1
+; X32-NEXT:    vmovddup {{.*#+}} xmm0 = mem[0,0]
+; X32-NEXT:    vmulpd (%eax), %xmm0, %xmm1
 ; X32-NEXT:    vmulsd 16(%eax), %xmm0, %xmm0
 ; X32-NEXT:    vpermilpd {{.*#+}} xmm2 = xmm1[1,0]
 ; X32-NEXT:    vaddsd %xmm2, %xmm1, %xmm1
 ; X32-NEXT:    vaddsd %xmm1, %xmm0, %xmm0
 ; X32-NEXT:    vmovsd %xmm0, (%esp)
 ; X32-NEXT:    fldl (%esp)
 ; X32-NEXT:    addl $12, %esp
 ; X32-NEXT:    retl
 ;
 ; X64-LABEL: broadcast_scale_xyz:
 ; X64:       ## %bb.0:
-; X64-NEXT:    vmovsd {{.*#+}} xmm0 = mem[0],zero
-; X64-NEXT:    vmovddup {{.*#+}} xmm1 = xmm0[0,0]
-; X64-NEXT:    vmulpd (%rsi), %xmm1, %xmm1
+; X64-NEXT:    vmovddup {{.*#+}} xmm0 = mem[0,0]
+; X64-NEXT:    vmulpd (%rsi), %xmm0, %xmm1
 ; X64-NEXT:    vmulsd 16(%rsi), %xmm0, %xmm0
 ; X64-NEXT:    vpermilpd {{.*#+}} xmm2 = xmm1[1,0]
 ; X64-NEXT:    vaddsd %xmm2, %xmm1, %xmm1
 ; X64-NEXT:    vaddsd %xmm1, %xmm0, %xmm0
 ; X64-NEXT:    retq
   %3 = bitcast double* %1 to <2 x double>*
   %4 = load <2 x double>, <2 x double>* %3, align 8
   %5 = getelementptr inbounds double, double* %1, i64 2