<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Exchange Server">
<!-- converted from text --><style><!-- .EmailQuote { margin-left: 1pt; padding-left: 4pt; border-left: #800000 2px solid; } --></style>
</head>
<body>
<meta content="text/html; charset=UTF-8">
<style type="text/css" style="">
<!--
p
{margin-top:0;
margin-bottom:0}
-->
</style>
<div dir="ltr">
<div id="x_divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:#000000; font-family:Calibri,Arial,Helvetica,sans-serif">
<p>LGTM</p>
</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="x_divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> hwennborg@google.com <hwennborg@google.com> on behalf of Hans Wennborg <hans@chromium.org><br>
<b>Sent:</b> Monday, January 23, 2017 10:28:05 AM<br>
<b>To:</b> Jan Vesely; Arsenault, Matthew<br>
<b>Cc:</b> llvm-commits; Stellard, Thomas<br>
<b>Subject:</b> Re: [llvm] r292651 - AMDGPU/R600: Serialize vector trunc stores to private AS</font>
<div> </div>
</div>
</div>
<font size="2"><span style="font-size:10pt;">
<div class="PlainText">Matt: Ok to merge this?<br>
<br>
On Sat, Jan 21, 2017 at 4:27 PM, Jan Vesely <jan.vesely@rutgers.edu> wrote:<br>
> Hi Hans,<br>
><br>
> I'd like this commit to be included in 4.0 release.<br>
> It fixes invalid code generation for sub-int vectors.<br>
><br>
> thanks,<br>
> Jan<br>
><br>
> On Fri, 2017-01-20 at 21:24 +0000, Jan Vesely via llvm-commits wrote:<br>
>> Author: jvesely<br>
>> Date: Fri Jan 20 15:24:26 2017<br>
>> New Revision: 292651<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=292651&view=rev">http://llvm.org/viewvc/llvm-project?rev=292651&view=rev</a><br>
>> Log:<br>
>> AMDGPU/R600: Serialize vector trunc stores to private AS<br>
>><br>
>> Add DUMMY_CHAIN SDNode to denote stores of interest<br>
>><br>
>> Bugzilla: <a href="https://llvm.org/bugs/show_bug.cgi?id=28915">https://llvm.org/bugs/show_bug.cgi?id=28915</a><br>
>> Bugzilla: <a href="https://llvm.org/bugs/show_bug.cgi?id=30411">https://llvm.org/bugs/show_bug.cgi?id=30411</a><br>
>><br>
>> Differential Revision: <a href="https://reviews.llvm.org/D27964">https://reviews.llvm.org/D27964</a><br>
>><br>
>> Modified:<br>
>> llvm/trunk/lib/Target/AMDGPU/AMDGPUISelLowering.cpp<br>
>> llvm/trunk/lib/Target/AMDGPU/AMDGPUISelLowering.h<br>
>> llvm/trunk/lib/Target/AMDGPU/AMDGPUInstrInfo.td<br>
>> llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp<br>
>> llvm/trunk/lib/Target/AMDGPU/R600Instructions.td<br>
>> llvm/trunk/test/CodeGen/AMDGPU/load-local-i8.ll<br>
>><br>
>> Modified: llvm/trunk/lib/Target/AMDGPU/AMDGPUISelLowering.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/AMDGPUISelLowering.cpp?rev=292651&r1=292650&r2=292651&view=diff">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/AMDGPUISelLowering.cpp?rev=292651&r1=292650&r2=292651&view=diff</a><br>
>> ==============================================================================<br>
>> --- llvm/trunk/lib/Target/AMDGPU/AMDGPUISelLowering.cpp (original)<br>
>> +++ llvm/trunk/lib/Target/AMDGPU/AMDGPUISelLowering.cpp Fri Jan 20 15:24:26 2017<br>
>> @@ -3278,6 +3278,7 @@ const char* AMDGPUTargetLowering::getTar<br>
>> NODE_NAME_CASE(CONST_DATA_PTR)<br>
>> NODE_NAME_CASE(PC_ADD_REL_OFFSET)<br>
>> NODE_NAME_CASE(KILL)<br>
>> + NODE_NAME_CASE(DUMMY_CHAIN)<br>
>> case AMDGPUISD::FIRST_MEM_OPCODE_NUMBER: break;<br>
>> NODE_NAME_CASE(SENDMSG)<br>
>> NODE_NAME_CASE(SENDMSGHALT)<br>
>><br>
>> Modified: llvm/trunk/lib/Target/AMDGPU/AMDGPUISelLowering.h<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/AMDGPUISelLowering.h?rev=292651&r1=292650&r2=292651&view=diff">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/AMDGPUISelLowering.h?rev=292651&r1=292650&r2=292651&view=diff</a><br>
>> ==============================================================================<br>
>> --- llvm/trunk/lib/Target/AMDGPU/AMDGPUISelLowering.h (original)<br>
>> +++ llvm/trunk/lib/Target/AMDGPU/AMDGPUISelLowering.h Fri Jan 20 15:24:26 2017<br>
>> @@ -330,6 +330,7 @@ enum NodeType : unsigned {<br>
>> INTERP_P2,<br>
>> PC_ADD_REL_OFFSET,<br>
>> KILL,<br>
>> + DUMMY_CHAIN,<br>
>> FIRST_MEM_OPCODE_NUMBER = ISD::FIRST_TARGET_MEMORY_OPCODE,<br>
>> STORE_MSKOR,<br>
>> LOAD_CONSTANT,<br>
>><br>
>> Modified: llvm/trunk/lib/Target/AMDGPU/AMDGPUInstrInfo.td<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/AMDGPUInstrInfo.td?rev=292651&r1=292650&r2=292651&view=diff">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/AMDGPUInstrInfo.td?rev=292651&r1=292650&r2=292651&view=diff</a><br>
>> ==============================================================================<br>
>> --- llvm/trunk/lib/Target/AMDGPU/AMDGPUInstrInfo.td (original)<br>
>> +++ llvm/trunk/lib/Target/AMDGPU/AMDGPUInstrInfo.td Fri Jan 20 15:24:26 2017<br>
>> @@ -54,6 +54,9 @@ def AMDGPUconstdata_ptr : SDNode<<br>
>> // This argument to this node is a dword address.<br>
>> def AMDGPUdwordaddr : SDNode<"AMDGPUISD::DWORDADDR", SDTIntUnaryOp>;<br>
>><br>
>> +// Force dependencies for vector trunc stores<br>
>> +def R600dummy_chain : SDNode<"AMDGPUISD::DUMMY_CHAIN", SDTNone, [SDNPHasChain]>;<br>
>> +<br>
>> def AMDGPUcos : SDNode<"AMDGPUISD::COS_HW", SDTFPUnaryOp>;<br>
>> def AMDGPUsin : SDNode<"AMDGPUISD::SIN_HW", SDTFPUnaryOp>;<br>
>><br>
>><br>
>> Modified: llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp?rev=292651&r1=292650&r2=292651&view=diff">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp?rev=292651&r1=292650&r2=292651&view=diff</a><br>
>> ==============================================================================<br>
>> --- llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp (original)<br>
>> +++ llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp Fri Jan 20 15:24:26 2017<br>
>> @@ -1120,7 +1120,10 @@ SDValue R600TargetLowering::lowerPrivate<br>
>> llvm_unreachable("Unsupported private trunc store");<br>
>> }<br>
>><br>
>> - SDValue Chain = Store->getChain();<br>
>> + SDValue OldChain = Store->getChain();<br>
>> + bool VectorTrunc = (OldChain.getOpcode() == AMDGPUISD::DUMMY_CHAIN);<br>
>> + // Skip dummy<br>
>> + SDValue Chain = VectorTrunc ? OldChain->getOperand(0) : OldChain;<br>
>> SDValue BasePtr = Store->getBasePtr();<br>
>> SDValue Offset = Store->getOffset();<br>
>> EVT MemVT = Store->getMemoryVT();<br>
>> @@ -1176,7 +1179,15 @@ SDValue R600TargetLowering::lowerPrivate<br>
>><br>
>> // Store dword<br>
>> // TODO: Can we be smarter about MachinePointerInfo?<br>
>> - return DAG.getStore(Chain, DL, Value, Ptr, MachinePointerInfo());<br>
>> + SDValue NewStore = DAG.getStore(Chain, DL, Value, Ptr, MachinePointerInfo());<br>
>> +<br>
>> + // If we are part of expanded vector, make our neighbors depend on this store<br>
>> + if (VectorTrunc) {<br>
>> + // Make all other vector elements depend on this store<br>
>> + Chain = DAG.getNode(AMDGPUISD::DUMMY_CHAIN, DL, MVT::Other, NewStore);<br>
>> + DAG.ReplaceAllUsesOfValueWith(OldChain, Chain);<br>
>> + }<br>
>> + return NewStore;<br>
>> }<br>
>><br>
>> SDValue R600TargetLowering::LowerSTORE(SDValue Op, SelectionDAG &DAG) const {<br>
>> @@ -1196,6 +1207,17 @@ SDValue R600TargetLowering::LowerSTORE(S<br>
>> // Neither LOCAL nor PRIVATE can do vectors at the moment<br>
>> if ((AS == AMDGPUAS::LOCAL_ADDRESS || AS == AMDGPUAS::PRIVATE_ADDRESS) &&<br>
>> VT.isVector()) {<br>
>> + if ((AS == AMDGPUAS::PRIVATE_ADDRESS) && StoreNode->isTruncatingStore()) {<br>
>> + // Add an extra level of chain to isolate this vector<br>
>> + SDValue NewChain = DAG.getNode(AMDGPUISD::DUMMY_CHAIN, DL, MVT::Other, Chain);<br>
>> + // TODO: can the chain be replaced without creating a new store?<br>
>> + SDValue NewStore = DAG.getTruncStore(<br>
>> + NewChain, DL, Value, Ptr, StoreNode->getPointerInfo(),<br>
>> + MemVT, StoreNode->getAlignment(),<br>
>> + StoreNode->getMemOperand()->getFlags(), StoreNode->getAAInfo());<br>
>> + StoreNode = cast<StoreSDNode>(NewStore);<br>
>> + }<br>
>> +<br>
>> return scalarizeVectorStore(StoreNode, DAG);<br>
>> }<br>
>><br>
>> @@ -1230,7 +1252,7 @@ SDValue R600TargetLowering::LowerSTORE(S<br>
>> // Put the mask in correct place<br>
>> SDValue Mask = DAG.getNode(ISD::SHL, DL, VT, MaskConstant, BitShift);<br>
>><br>
>> - // Put the mask in correct place<br>
>> + // Put the value bits in correct place<br>
>> SDValue TruncValue = DAG.getNode(ISD::AND, DL, VT, Value, MaskConstant);<br>
>> SDValue ShiftedValue = DAG.getNode(ISD::SHL, DL, VT, TruncValue, BitShift);<br>
>><br>
>><br>
>> Modified: llvm/trunk/lib/Target/AMDGPU/R600Instructions.td<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/R600Instructions.td?rev=292651&r1=292650&r2=292651&view=diff">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/R600Instructions.td?rev=292651&r1=292650&r2=292651&view=diff</a><br>
>> ==============================================================================<br>
>> --- llvm/trunk/lib/Target/AMDGPU/R600Instructions.td (original)<br>
>> +++ llvm/trunk/lib/Target/AMDGPU/R600Instructions.td Fri Jan 20 15:24:26 2017<br>
>> @@ -727,6 +727,20 @@ def FLOOR : R600_1OP_Helper <0x14, "FLOO<br>
>><br>
>> def MOV : R600_1OP <0x19, "MOV", []>;<br>
>><br>
>> +<br>
>> +// This is a hack to get rid of DUMMY_CHAIN nodes.<br>
>> +// Most DUMMY_CHAINs should be eliminated during legalization, but undef<br>
>> +// values can sneak in some to selection.<br>
>> +let isPseudo = 1, isCodeGenOnly = 1 in {<br>
>> +def DUMMY_CHAIN : AMDGPUInst <<br>
>> + (outs),<br>
>> + (ins),<br>
>> + "DUMMY_CHAIN",<br>
>> + [(R600dummy_chain)]<br>
>> +>;<br>
>> +} // end let isPseudo = 1, isCodeGenOnly = 1<br>
>> +<br>
>> +<br>
>> let isPseudo = 1, isCodeGenOnly = 1, usesCustomInserter = 1 in {<br>
>><br>
>> class MOV_IMM <ValueType vt, Operand immType> : AMDGPUInst <<br>
>><br>
>> Modified: llvm/trunk/test/CodeGen/AMDGPU/load-local-i8.ll<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AMDGPU/load-local-i8.ll?rev=292651&r1=292650&r2=292651&view=diff">
http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AMDGPU/load-local-i8.ll?rev=292651&r1=292650&r2=292651&view=diff</a><br>
>> ==============================================================================<br>
>> --- llvm/trunk/test/CodeGen/AMDGPU/load-local-i8.ll (original)<br>
>> +++ llvm/trunk/test/CodeGen/AMDGPU/load-local-i8.ll Fri Jan 20 15:24:26 2017<br>
>> @@ -708,10 +708,11 @@ define void @local_zextload_v4i8_to_v4i1<br>
>> ; FUNC-LABEL: {{^}}local_sextload_v4i8_to_v4i16:<br>
>><br>
>> ; EG: LDS_READ_RET<br>
>> +; TODO: these do LSHR + BFE_INT, instead of just BFE_INT/ASHR<br>
>> +; EG-DAG: BFE_INT<br>
>> ; EG-DAG: BFE_INT<br>
>> ; EG-DAG: BFE_INT<br>
>> ; EG-DAG: BFE_INT<br>
>> -; EG-DAG: ASHR<br>
>> ; EG: LDS_WRITE<br>
>> ; EG: LDS_WRITE<br>
>> define void @local_sextload_v4i8_to_v4i16(<4 x i16> addrspace(3)* %out, <4 x i8> addrspace(3)* %in) #0 {<br>
>> @@ -740,14 +741,15 @@ define void @local_zextload_v8i8_to_v8i1<br>
>><br>
>> ; EG: LDS_READ_RET<br>
>> ; EG: LDS_READ_RET<br>
>> +; TODO: these do LSHR + BFE_INT, instead of just BFE_INT/ASHR<br>
>> +; EG-DAG: BFE_INT<br>
>> +; EG-DAG: BFE_INT<br>
>> ; EG-DAG: BFE_INT<br>
>> ; EG-DAG: BFE_INT<br>
>> ; EG-DAG: BFE_INT<br>
>> ; EG-DAG: BFE_INT<br>
>> ; EG-DAG: BFE_INT<br>
>> ; EG-DAG: BFE_INT<br>
>> -; EG-DAG: ASHR<br>
>> -; EG-DAG: ASHR<br>
>> ; EG: LDS_WRITE<br>
>> ; EG: LDS_WRITE<br>
>> ; EG: LDS_WRITE<br>
>> @@ -786,6 +788,11 @@ define void @local_zextload_v16i8_to_v16<br>
>> ; EG: LDS_READ_RET<br>
>> ; EG: LDS_READ_RET<br>
>> ; EG: LDS_READ_RET<br>
>> +; TODO: these do LSHR + BFE_INT, instead of just BFE_INT/ASHR<br>
>> +; EG-DAG: BFE_INT<br>
>> +; EG-DAG: BFE_INT<br>
>> +; EG-DAG: BFE_INT<br>
>> +; EG-DAG: BFE_INT<br>
>> ; EG-DAG: BFE_INT<br>
>> ; EG-DAG: BFE_INT<br>
>> ; EG-DAG: BFE_INT<br>
>> @@ -798,10 +805,6 @@ define void @local_zextload_v16i8_to_v16<br>
>> ; EG-DAG: BFE_INT<br>
>> ; EG-DAG: BFE_INT<br>
>> ; EG-DAG: BFE_INT<br>
>> -; EG-DAG: ASHR<br>
>> -; EG-DAG: ASHR<br>
>> -; EG-DAG: ASHR<br>
>> -; EG-DAG: ASHR<br>
>> ; EG: LDS_WRITE<br>
>> ; EG: LDS_WRITE<br>
>> ; EG: LDS_WRITE<br>
>> @@ -860,6 +863,11 @@ define void @local_zextload_v32i8_to_v32<br>
>> ; EG: LDS_READ_RET<br>
>> ; EG: LDS_READ_RET<br>
>> ; EG: LDS_READ_RET<br>
>> +; TODO: these do LSHR + BFE_INT, instead of just BFE_INT/ASHR<br>
>> +; EG-DAG: BFE_INT<br>
>> +; EG-DAG: BFE_INT<br>
>> +; EG-DAG: BFE_INT<br>
>> +; EG-DAG: BFE_INT<br>
>> ; EG-DAG: BFE_INT<br>
>> ; EG-DAG: BFE_INT<br>
>> ; EG-DAG: BFE_INT<br>
>> @@ -884,14 +892,6 @@ define void @local_zextload_v32i8_to_v32<br>
>> ; EG-DAG: BFE_INT<br>
>> ; EG-DAG: BFE_INT<br>
>> ; EG-DAG: BFE_INT<br>
>> -; EG-DAG: ASHR<br>
>> -; EG-DAG: ASHR<br>
>> -; EG-DAG: ASHR<br>
>> -; EG-DAG: ASHR<br>
>> -; EG-DAG: ASHR<br>
>> -; EG-DAG: ASHR<br>
>> -; EG-DAG: ASHR<br>
>> -; EG-DAG: ASHR<br>
>> ; EG: LDS_WRITE<br>
>> ; EG: LDS_WRITE<br>
>> ; EG: LDS_WRITE<br>
>><br>
>><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> llvm-commits@lists.llvm.org<br>
>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</div>
</span></font>
</body>
</html>