[PATCH] D108757: [X86][Codegen] PR51615: don't replace wide volatile load with narrow broadcast-from-memory

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri created this revision.
lebedev.ri added reviewers: RKSimon, craig.topper, spatel.
lebedev.ri added a project: LLVM.
Herald added subscribers: pengfei, hiraditya.
lebedev.ri requested review of this revision.

Even though https://bugs.llvm.org/show_bug.cgi?id=51615 appears to be introduced by D105390 <https://reviews.llvm.org/D105390>,
the fix lies here.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108757

Files:
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/test/CodeGen/X86/pr51615-wide-volatile-load-vs-narrow-broadcast-from-memory.ll


Index: llvm/test/CodeGen/X86/pr51615-wide-volatile-load-vs-narrow-broadcast-from-memory.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/X86/pr51615-wide-volatile-load-vs-narrow-broadcast-from-memory.ll
@@ -0,0 +1,43 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+avx  | FileCheck %s --check-prefix=AVX
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+avx2 | FileCheck %s --check-prefix=AVX2
+
+; https://bugs.llvm.org/show_bug.cgi?id=51615
+; We can not replace a wide volatile load with a broadcast-from-memory,
+; because that would narrow the load, which isn't legal for volatiles.
+
+ at test5_id1234 = external dso_local global <2 x double>, align 16
+
+define void @_Z5test5v() {
+; AVX-LABEL: _Z5test5v:
+; AVX:       # %bb.0:
+; AVX-NEXT:    vmovaps test5_id1234(%rip), %xmm0
+; AVX-NEXT:    vmovddup {{.*#+}} xmm0 = xmm0[0,0]
+; AVX-NEXT:    vinsertf128 $1, %xmm0, %ymm0, %ymm1
+; AVX-NEXT:    vpermilpd {{.*#+}} ymm1 = ymm1[0,0,3,2]
+; AVX-NEXT:    vxorpd %xmm2, %xmm2, %xmm2
+; AVX-NEXT:    vblendpd {{.*#+}} ymm1 = ymm2[0,1],ymm1[2],ymm2[3]
+; AVX-NEXT:    vinsertf128 $1, %xmm0, %ymm0, %ymm0
+; AVX-NEXT:    vblendpd {{.*#+}} ymm0 = ymm2[0],ymm0[1],ymm2[2],ymm0[3]
+; AVX-NEXT:    vmovapd %ymm0, (%rax)
+; AVX-NEXT:    vmovapd %ymm1, (%rax)
+; AVX-NEXT:    vzeroupper
+; AVX-NEXT:    retq
+;
+; AVX2-LABEL: _Z5test5v:
+; AVX2:       # %bb.0:
+; AVX2-NEXT:    vmovaps test5_id1234(%rip), %xmm0
+; AVX2-NEXT:    vbroadcastsd %xmm0, %ymm0
+; AVX2-NEXT:    vxorps %xmm1, %xmm1, %xmm1
+; AVX2-NEXT:    vblendps {{.*#+}} ymm2 = ymm1[0,1,2,3],ymm0[4,5],ymm1[6,7]
+; AVX2-NEXT:    vblendps {{.*#+}} ymm0 = ymm1[0,1],ymm0[2,3],ymm1[4,5],ymm0[6,7]
+; AVX2-NEXT:    vmovaps %ymm0, (%rax)
+; AVX2-NEXT:    vmovaps %ymm2, (%rax)
+; AVX2-NEXT:    vzeroupper
+; AVX2-NEXT:    retq
+  %i = load volatile <2 x double>, <2 x double>* @test5_id1234, align 16
+  %i1 = shufflevector <2 x double> %i, <2 x double> poison, <4 x i32> <i32 undef, i32 0, i32 undef, i32 0>
+  %shuffle1 = shufflevector <4 x double> %i1, <4 x double> zeroinitializer, <8 x i32> <i32 6, i32 7, i32 3, i32 6, i32 7, i32 1, i32 7, i32 1>
+  store volatile <8 x double> %shuffle1, <8 x double>* undef, align 64
+  ret void
+}
Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===================================================================
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -5036,6 +5036,19 @@
   return Op.hasOneUse() && ISD::isNormalLoad(Op.getNode());
 }
 
+static bool MayFoldLoadIntoBroadcastFromMem(SDValue Op, MVT EltVT) {
+  if (!MayFoldLoad(Op))
+    return false;
+
+  // X86ISD::VBROADCAST will only perform the load of a single scalar element,
+  // but if the original load was volatile, the load must have been loading
+  // exactly that single scalar element, because we can not change the size
+  // of volatile loads.
+  const LoadSDNode *Ld = dyn_cast<LoadSDNode>(Op.getNode());
+  return !Ld->isVolatile() ||
+         Ld->getValueSizeInBits(0) == EltVT.getScalarSizeInBits();
+}
+
 static bool MayFoldIntoStore(SDValue Op) {
   return Op.hasOneUse() && ISD::isNormalStore(*Op.getNode()->use_begin());
 }
@@ -50876,7 +50889,8 @@
 
     // concat_vectors(movddup(x),movddup(x)) -> broadcast(x)
     if (Op0.getOpcode() == X86ISD::MOVDDUP && VT == MVT::v4f64 &&
-        (Subtarget.hasAVX2() || MayFoldLoad(Op0.getOperand(0))))
+        (Subtarget.hasAVX2() || MayFoldLoadIntoBroadcastFromMem(
+                                    Op0.getOperand(0), VT.getScalarType())))
       return DAG.getNode(X86ISD::VBROADCAST, DL, VT,
                          DAG.getNode(ISD::EXTRACT_VECTOR_ELT, DL, MVT::f64,
                                      Op0.getOperand(0),


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D108757.368842.patch
Type: text/x-patch
Size: 3857 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210826/98a59b73/attachment.bin>