<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Apr 9, 2012, at 5:32 PM, Chad Rosier <<a href="mailto:mcrosier@apple.com">mcrosier@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>Hi Jim,<br>After digging a little deeper I've determine the problem is that this combine doesn't know how to efficiently handle undefined values. See below for an explanation.<br><br>On Apr 9, 2012, at 2:33 PM, Jim Grosbach wrote:<br><br><blockquote type="cite"><br>On Apr 9, 2012, at 1:32 PM, Chad Rosier <<a href="mailto:mcrosier@apple.com">mcrosier@apple.com</a>> wrote:<br><br><blockquote type="cite">Author: mcrosier<br>Date: Mon Apr 9 15:32:02 2012<br>New Revision: 154340<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=154340&view=rev">http://llvm.org/viewvc/llvm-project?rev=154340&view=rev</a><br>Log:<br>When performing a truncating store, it's possible to rearrange the data <br>in-register, such that we can use a single vector store rather then a <br>series of scalar stores.<br><br>For func_4_8 the generated code<br><br><span class="Apple-tab-span" style="white-space:pre"> </span>vldr<span class="Apple-tab-span" style="white-space:pre"> </span>d16, LCPI0_0<br><span class="Apple-tab-span" style="white-space:pre"> </span>vmov<span class="Apple-tab-span" style="white-space:pre"> </span>d17, r0, r1<br><span class="Apple-tab-span" style="white-space:pre"> </span>vadd.i16<span class="Apple-tab-span" style="white-space:pre"> </span>d16, d17, d16<br><span class="Apple-tab-span" style="white-space:pre"> </span>vmov.u16<span class="Apple-tab-span" style="white-space:pre"> </span>r0, d16[3]<br><span class="Apple-tab-span" style="white-space:pre"> </span>strb<span class="Apple-tab-span" style="white-space:pre"> </span>r0, [r2, #3]<br><span class="Apple-tab-span" style="white-space:pre"> </span>vmov.u16<span class="Apple-tab-span" style="white-space:pre"> </span>r0, d16[2]<br><span class="Apple-tab-span" style="white-space:pre"> </span>strb<span class="Apple-tab-span" style="white-space:pre"> </span>r0, [r2, #2]<br><span class="Apple-tab-span" style="white-space:pre"> </span>vmov.u16<span class="Apple-tab-span" style="white-space:pre"> </span>r0, d16[1]<br><span class="Apple-tab-span" style="white-space:pre"> </span>strb<span class="Apple-tab-span" style="white-space:pre"> </span>r0, [r2, #1]<br><span class="Apple-tab-span" style="white-space:pre"> </span>vmov.u16<span class="Apple-tab-span" style="white-space:pre"> </span>r0, d16[0]<br><span class="Apple-tab-span" style="white-space:pre"> </span>strb<span class="Apple-tab-span" style="white-space:pre"> </span>r0, [r2]<br><span class="Apple-tab-span" style="white-space:pre"> </span>bx<span class="Apple-tab-span" style="white-space:pre"> </span>lr<br><br>becomes<br><br><span class="Apple-tab-span" style="white-space:pre"> </span>vldr<span class="Apple-tab-span" style="white-space:pre"> </span>d16, LCPI0_0<br><span class="Apple-tab-span" style="white-space:pre"> </span>vmov<span class="Apple-tab-span" style="white-space:pre"> </span>d17, r0, r1<br><span class="Apple-tab-span" style="white-space:pre"> </span>vadd.i16<span class="Apple-tab-span" style="white-space:pre"> </span>d16, d17, d16<br><span class="Apple-tab-span" style="white-space:pre"> </span>vuzp.8<span class="Apple-tab-span" style="white-space:pre"> </span>d16, d17<br><span class="Apple-tab-span" style="white-space:pre"> </span>vst1.32<span class="Apple-tab-span" style="white-space:pre"> </span>{d16[0]}, [r2, :32]<br><span class="Apple-tab-span" style="white-space:pre"> </span>bx<span class="Apple-tab-span" style="white-space:pre"> </span>lr<br><br>I'm not fond of how this combine pessimizes 2012-03-13-DAGCombineBug.ll,<br>but I couldn't think of a way to judiciously apply this combine.<br><br>This<br><br><span class="Apple-tab-span" style="white-space:pre"> </span>ldrh<span class="Apple-tab-span" style="white-space:pre"> </span>r0, [r0, #4]<br><span class="Apple-tab-span" style="white-space:pre"> </span>strh<span class="Apple-tab-span" style="white-space:pre"> </span>r0, [r1]<br><br>becomes<br><br><span class="Apple-tab-span" style="white-space:pre"> </span>vldr<span class="Apple-tab-span" style="white-space:pre"> </span>d16, [r0]<br><span class="Apple-tab-span" style="white-space:pre"> </span>vmov.u16<span class="Apple-tab-span" style="white-space:pre"> </span>r0, d16[2]<br><span class="Apple-tab-span" style="white-space:pre"> </span>vmov.32<span class="Apple-tab-span" style="white-space:pre"> </span>d16[0], r0<br><span class="Apple-tab-span" style="white-space:pre"> </span>vuzp.16<span class="Apple-tab-span" style="white-space:pre"> </span>d16, d17<br><span class="Apple-tab-span" style="white-space:pre"> </span>vst1.32<span class="Apple-tab-span" style="white-space:pre"> </span>{d16[0]}, [r1, :32]<br></blockquote><br>This worries me. We're now touching more memory than we were before. Can we perhaps use that sort of information to fine-tune when to fire the combine?<br></blockquote><br>Here's the test case:<br>define void @test_hi_short3(<3 x i16> * nocapture %srcA, <2 x i16> * nocapture %dst) nounwind {<br>entry:<br>CHECK: vst1.32<br> %0 = load <3 x i16> * %srcA, align 8<br> %1 = shufflevector <3 x i16> %0, <3 x i16> undef, <2 x i32> <i32 2, i32 undef><br> store <2 x i16> %1, <2 x i16> * %dst, align 4<br> ret void<br>}<br><br>Prior to this commit the vector loads and stores were being scalarized, which resulted in the following DAGCombines to fire:<br><br>// (vextract (v4f32 load $addr), c) -> (f32 load $addr+c*size)<br>This combine results in the original ldrh+stdh pair.<br><br>// 'store undef, Ptr' -> nothing.<br>This eliminates the dead store.<br><br>However, when the scalars are shuffled back into the vector we can't eliminate the dead half of the vector store. I don't think we have a correctness issue, but certainly there might be potential for improving performance. Overall, I don't think this is going to be a very common case (famous last words), so I'm not going to spend any more time digging deeper.<br><br></div></blockquote><div><br></div><div>Makes perfect sense. Thanks, Chad!</div><div><br></div><div>-Jim</div><blockquote type="cite"><div><font color="#000000"><br></font><blockquote type="cite"><br><blockquote type="cite"><br>PR11158<br><a href="rdar://10703339">rdar://10703339</a><br><br>Added:<br> llvm/trunk/test/CodeGen/ARM/opt-shuff-tstore.ll<br>Modified:<br> llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp<br> llvm/trunk/test/CodeGen/ARM/2012-03-13-DAGCombineBug.ll<br> llvm/trunk/test/CodeGen/ARM/vrev.ll<br><br>Modified: llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp<br>URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp?rev=154340&r1=154339&r2=154340&view=diff<br>==============================================================================<br>--- llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp (original)<br>+++ llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp Mon Apr 9 15:32:02 2012<br>@@ -7339,8 +7339,92 @@<br>static SDValue PerformSTORECombine(SDNode *N,<br> TargetLowering::DAGCombinerInfo &DCI) {<br> StoreSDNode *St = cast<StoreSDNode>(N);<br>+ if (St->isVolatile())<br>+ return SDValue();<br>+<br>+ // Optimize trunc store (of multiple scalars) to shuffle and store. First, <br>+ // pack all of the elements in one place. Next, store to memory in fewer<br>+ // chunks.<br> SDValue StVal = St->getValue();<br>- if (!ISD::isNormalStore(St) || St->isVolatile())<br>+ EVT VT = StVal.getValueType();<br>+ if (St->isTruncatingStore() && VT.isVector()) {<br>+ SelectionDAG &DAG = DCI.DAG;<br>+ const TargetLowering &TLI = DAG.getTargetLoweringInfo();<br>+ EVT StVT = St->getMemoryVT();<br>+ unsigned NumElems = VT.getVectorNumElements();<br>+ assert(StVT != VT && "Cannot truncate to the same type");<br>+ unsigned FromEltSz = VT.getVectorElementType().getSizeInBits();<br>+ unsigned ToEltSz = StVT.getVectorElementType().getSizeInBits();<br>+<br>+ // From, To sizes and ElemCount must be pow of two<br>+ if (!isPowerOf2_32(NumElems * FromEltSz * ToEltSz)) return SDValue();<br>+<br>+ // We are going to use the original vector elt for storing.<br>+ // Accumulated smaller vector elements must be a multiple of the store size.<br>+ if (0 != (NumElems * FromEltSz) % ToEltSz) return SDValue();<br>+<br>+ unsigned SizeRatio = FromEltSz / ToEltSz;<br>+ assert(SizeRatio * NumElems * ToEltSz == VT.getSizeInBits());<br>+<br>+ // Create a type on which we perform the shuffle.<br>+ EVT WideVecVT = EVT::getVectorVT(*DAG.getContext(), StVT.getScalarType(),<br>+ NumElems*SizeRatio);<br>+ assert(WideVecVT.getSizeInBits() == VT.getSizeInBits());<br>+<br>+ DebugLoc DL = St->getDebugLoc();<br>+ SDValue WideVec = DAG.getNode(ISD::BITCAST, DL, WideVecVT, StVal);<br>+ SmallVector<int, 8> ShuffleVec(NumElems * SizeRatio, -1);<br>+ for (unsigned i = 0; i < NumElems; ++i) ShuffleVec[i] = i * SizeRatio;<br>+<br>+ // Can't shuffle using an illegal type.<br>+ if (!TLI.isTypeLegal(WideVecVT)) return SDValue();<br>+<br>+ SDValue Shuff = DAG.getVectorShuffle(WideVecVT, DL, WideVec,<br>+ DAG.getUNDEF(WideVec.getValueType()),<br>+ ShuffleVec.data());<br>+ // At this point all of the data is stored at the bottom of the<br>+ // register. We now need to save it to mem.<br>+<br>+ // Find the largest store unit<br>+ MVT StoreType = MVT::i8;<br>+ for (unsigned tp = MVT::FIRST_INTEGER_VALUETYPE;<br>+ tp < MVT::LAST_INTEGER_VALUETYPE; ++tp) {<br>+ MVT Tp = (MVT::SimpleValueType)tp;<br>+ if (TLI.isTypeLegal(Tp) && Tp.getSizeInBits() <= NumElems * ToEltSz)<br>+ StoreType = Tp;<br>+ }<br>+ // Didn't find a legal store type.<br>+ if (!TLI.isTypeLegal(StoreType))<br>+ return SDValue();<br>+<br>+ // Bitcast the original vector into a vector of store-size units<br>+ EVT StoreVecVT = EVT::getVectorVT(*DAG.getContext(),<br>+ StoreType, VT.getSizeInBits()/EVT(StoreType).getSizeInBits());<br>+ assert(StoreVecVT.getSizeInBits() == VT.getSizeInBits());<br>+ SDValue ShuffWide = DAG.getNode(ISD::BITCAST, DL, StoreVecVT, Shuff);<br>+ SmallVector<SDValue, 8> Chains;<br>+ SDValue Increment = DAG.getConstant(StoreType.getSizeInBits()/8,<br>+ TLI.getPointerTy());<br>+ SDValue BasePtr = St->getBasePtr();<br>+<br>+ // Perform one or more big stores into memory.<br>+ unsigned E = (ToEltSz*NumElems)/StoreType.getSizeInBits();<br>+ for (unsigned I = 0; I < E; I++) {<br>+ SDValue SubVec = DAG.getNode(ISD::EXTRACT_VECTOR_ELT, DL,<br>+ StoreType, ShuffWide,<br>+ DAG.getIntPtrConstant(I));<br>+ SDValue Ch = DAG.getStore(St->getChain(), DL, SubVec, BasePtr,<br>+ St->getPointerInfo(), St->isVolatile(),<br>+ St->isNonTemporal(), St->getAlignment());<br>+ BasePtr = DAG.getNode(ISD::ADD, DL, BasePtr.getValueType(), BasePtr,<br>+ Increment);<br>+ Chains.push_back(Ch);<br>+ }<br>+ return DAG.getNode(ISD::TokenFactor, DL, MVT::Other, &Chains[0],<br>+ Chains.size());<br>+ }<br>+<br>+ if (!ISD::isNormalStore(St))<br> return SDValue();<br><br> // Split a store of a VMOVDRR into two integer stores to avoid mixing NEON and<br><br>Modified: llvm/trunk/test/CodeGen/ARM/2012-03-13-DAGCombineBug.ll<br>URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/2012-03-13-DAGCombineBug.ll?rev=154340&r1=154339&r2=154340&view=diff<br>==============================================================================<br>--- llvm/trunk/test/CodeGen/ARM/2012-03-13-DAGCombineBug.ll (original)<br>+++ llvm/trunk/test/CodeGen/ARM/2012-03-13-DAGCombineBug.ll Mon Apr 9 15:32:02 2012<br>@@ -6,8 +6,7 @@<br>; (i32 extload $addr+c*sizeof(i16)<br>define void @test_hi_short3(<3 x i16> * nocapture %srcA, <2 x i16> * nocapture %dst) nounwind {<br>entry:<br>-; CHECK: ldrh [[REG:r[0-9]+]]<br>-; CHECK: strh [[REG]]<br>+; CHECK: vst1.32<br> %0 = load <3 x i16> * %srcA, align 8<br> %1 = shufflevector <3 x i16> %0, <3 x i16> undef, <2 x i32> <i32 2, i32 undef><br> store <2 x i16> %1, <2 x i16> * %dst, align 4<br><br>Added: llvm/trunk/test/CodeGen/ARM/opt-shuff-tstore.ll<br>URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/opt-shuff-tstore.ll?rev=154340&view=auto<br>==============================================================================<br>--- llvm/trunk/test/CodeGen/ARM/opt-shuff-tstore.ll (added)<br>+++ llvm/trunk/test/CodeGen/ARM/opt-shuff-tstore.ll Mon Apr 9 15:32:02 2012<br>@@ -0,0 +1,19 @@<br>+; RUN: llc -mcpu=cortex-a9 -mtriple=arm-linux-unknown -promote-elements -mattr=+neon < %s | FileCheck %s<br>+<br>+; CHECK: func_4_8<br>+; CHECK: vst1.32<br>+; CHECK-NEXT: bx lr<br>+define void @func_4_8(<4 x i8> %param, <4 x i8>* %p) {<br>+ %r = add <4 x i8> %param, <i8 1, i8 2, i8 3, i8 4><br>+ store <4 x i8> %r, <4 x i8>* %p<br>+ ret void<br>+}<br>+<br>+; CHECK: func_2_16<br>+; CHECK: vst1.32<br>+; CHECK-NEXT: bx lr<br>+define void @func_2_16(<2 x i16> %param, <2 x i16>* %p) {<br>+ %r = add <2 x i16> %param, <i16 1, i16 2><br>+ store <2 x i16> %r, <2 x i16>* %p<br>+ ret void<br>+}<br><br>Modified: llvm/trunk/test/CodeGen/ARM/vrev.ll<br>URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/vrev.ll?rev=154340&r1=154339&r2=154340&view=diff<br>==============================================================================<br>--- llvm/trunk/test/CodeGen/ARM/vrev.ll (original)<br>+++ llvm/trunk/test/CodeGen/ARM/vrev.ll Mon Apr 9 15:32:02 2012<br>@@ -149,12 +149,10 @@<br>}<br><br>; The type <2 x i16> is legalized to <2 x i32> and need to be trunc-stored<br>-; to <2 x i16> when stored to memory. Currently ARM scalarizes these stores.<br>-; See PR 11158<br>+; to <2 x i16> when stored to memory.<br>define void @test_vrev64(<4 x i16>* nocapture %source, <2 x i16>* nocapture %dst) nounwind ssp {<br>; CHECK: test_vrev64:<br>-; CHECK: vst1.16<br>-; CHECK: vst1.16<br>+; CHECK: vst1.32<br>entry:<br> %0 = bitcast <4 x i16>* %source to <8 x i16>*<br> %tmp2 = load <8 x i16>* %0, align 4<br><br><br>_______________________________________________<br>llvm-commits mailing list<br>llvm-commits@cs.uiuc.edu<br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br></blockquote><br></blockquote><br></div></blockquote></div><br></body></html>