From ef867d47bfccedd01ff64ebaa702563b384dc863 Mon Sep 17 00:00:00 2001 From: Simon Pilgrim Date: Tue, 8 Nov 2016 17:01:05 +0000 Subject: [PATCH] [3.9.1] Merging r283070 - [X86][AVX] Ensure broadcast loads respect dependencies To allow broadcast loads of a non-zero'th vector element, lowerVectorShuffleAsBroadcast can replace a load with a new load with an adjusted address, but unfortunately we weren't ensuring that the new load respected the same dependencies. This patch adds a TokenFactor and updates all dependencies of the old load to reference the new load instead. Bug found during internal testing. Differential Revision: https://reviews.llvm.org/D25039 As discussed on PR30596 git-svn-id: https://llvm.org/svn/llvm-project/llvm/branches/release_39@286251 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Target/X86/X86ISelLowering.cpp | 11 +++++++++++ test/CodeGen/X86/avx-vbroadcast.ll | 20 +++++++++++--------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/lib/Target/X86/X86ISelLowering.cpp b/lib/Target/X86/X86ISelLowering.cpp index 75c58525b77..ca205335013 100644 --- a/lib/Target/X86/X86ISelLowering.cpp +++ b/lib/Target/X86/X86ISelLowering.cpp @@ -8656,6 +8656,17 @@ static SDValue lowerVectorShuffleAsBroadcast(const SDLoc &DL, MVT VT, V = DAG.getLoad(SVT, DL, Ld->getChain(), NewAddr, DAG.getMachineFunction().getMachineMemOperand( Ld->getMemOperand(), Offset, SVT.getStoreSize())); + + // Make sure the newly-created LOAD is in the same position as Ld in + // terms of dependency. We create a TokenFactor for Ld and V, + // and update uses of Ld's output chain to use the TokenFactor. + if (Ld->hasAnyUseOfValue(1)) { + SDValue NewChain = DAG.getNode(ISD::TokenFactor, DL, MVT::Other, + SDValue(Ld, 1), SDValue(V.getNode(), 1)); + DAG.ReplaceAllUsesOfValueWith(SDValue(Ld, 1), NewChain); + DAG.UpdateNodeOperands(NewChain.getNode(), SDValue(Ld, 1), + SDValue(V.getNode(), 1)); + } } else if (!BroadcastFromReg) { // We can't broadcast from a vector register. return SDValue(); diff --git a/test/CodeGen/X86/avx-vbroadcast.ll b/test/CodeGen/X86/avx-vbroadcast.ll index 276360f468c..53467079589 100644 --- a/test/CodeGen/X86/avx-vbroadcast.ll +++ b/test/CodeGen/X86/avx-vbroadcast.ll @@ -548,38 +548,40 @@ define <4 x double> @splat_concat4(double* %p) { } ; -; FIXME: When VBROADCAST replaces an existing load, ensure it still respects lifetime dependencies. +; When VBROADCAST replaces an existing load, ensure it still respects lifetime dependencies. ; define float @broadcast_lifetime() nounwind { ; X32-LABEL: broadcast_lifetime: ; X32: ## BB#0: ; X32-NEXT: pushl %esi -; X32-NEXT: subl $40, %esp +; X32-NEXT: subl $56, %esp ; X32-NEXT: leal {{[0-9]+}}(%esp), %esi ; X32-NEXT: movl %esi, (%esp) ; X32-NEXT: calll _gfunc +; X32-NEXT: vbroadcastss {{[0-9]+}}(%esp), %xmm0 +; X32-NEXT: vmovaps %xmm0, {{[0-9]+}}(%esp) ## 16-byte Spill ; X32-NEXT: movl %esi, (%esp) ; X32-NEXT: calll _gfunc ; X32-NEXT: vbroadcastss {{[0-9]+}}(%esp), %xmm0 -; X32-NEXT: vbroadcastss {{[0-9]+}}(%esp), %xmm1 -; X32-NEXT: vsubss %xmm0, %xmm1, %xmm0 +; X32-NEXT: vsubss {{[0-9]+}}(%esp), %xmm0, %xmm0 ## 16-byte Folded Reload ; X32-NEXT: vmovss %xmm0, {{[0-9]+}}(%esp) ; X32-NEXT: flds {{[0-9]+}}(%esp) -; X32-NEXT: addl $40, %esp +; X32-NEXT: addl $56, %esp ; X32-NEXT: popl %esi ; X32-NEXT: retl ; ; X64-LABEL: broadcast_lifetime: ; X64: ## BB#0: -; X64-NEXT: subq $24, %rsp +; X64-NEXT: subq $40, %rsp ; X64-NEXT: leaq (%rsp), %rdi ; X64-NEXT: callq _gfunc +; X64-NEXT: vbroadcastss {{[0-9]+}}(%rsp), %xmm0 +; X64-NEXT: vmovaps %xmm0, {{[0-9]+}}(%rsp) ## 16-byte Spill ; X64-NEXT: leaq (%rsp), %rdi ; X64-NEXT: callq _gfunc ; X64-NEXT: vbroadcastss {{[0-9]+}}(%rsp), %xmm0 -; X64-NEXT: vbroadcastss {{[0-9]+}}(%rsp), %xmm1 -; X64-NEXT: vsubss %xmm0, %xmm1, %xmm0 -; X64-NEXT: addq $24, %rsp +; X64-NEXT: vsubss {{[0-9]+}}(%rsp), %xmm0, %xmm0 ## 16-byte Folded Reload +; X64-NEXT: addq $40, %rsp ; X64-NEXT: retq %1 = alloca <4 x float>, align 16 %2 = alloca <4 x float>, align 16 -- 2.40.0