<div dir="ltr">Yikes! It looks like in my shuffling of patches around to I merged the functional ext load input change in with the change directly generating legalized memory operations directly instead of waiting until legalization. Thanks for catching it. <div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 15, 2017 at 7:33 PM, Ahmed Bougacha <span dir="ltr"><<a href="mailto:ahmed.bougacha@gmail.com" target="_blank">ahmed.bougacha@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Nirav,<br>
<br>
I reverted this in r305527 because it causes assertion failures (PR33475).<br>
<br>
More generally, this change (and others) look pretty functional to me.<br>
Is this all enabled by some future commit? If so, the commit message<br>
should probably mention that this really is a functional change<br>
required for correctness; it just happens to not be testable right<br>
now (or even better, commit everything together to avoid adding dead<br>
code).<br>
<br>
Thanks!<br>
<span class="HOEnZb"><font color="#888888"><br>
-Ahmed<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
On Thu, Jun 15, 2017 at 7:04 AM, Nirav Dave via llvm-commits<br>
<<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
> Author: niravd<br>
> Date: Thu Jun 15 09:04:07 2017<br>
> New Revision: 305468<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=305468&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=305468&view=rev</a><br>
> Log:<br>
> [DAG] Allow truncated and extend memory operations in Store Merge. NFCI.<br>
><br>
> As all store merges checks are based on the memory operation<br>
> performed, allow use of truncated stores and extended loads as valid<br>
> input candidates for merging.<br>
><br>
> Modified:<br>
> llvm/trunk/lib/CodeGen/<wbr>SelectionDAG/DAGCombiner.cpp<br>
><br>
> Modified: llvm/trunk/lib/CodeGen/<wbr>SelectionDAG/DAGCombiner.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=305468&r1=305467&r2=305468&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>CodeGen/SelectionDAG/<wbr>DAGCombiner.cpp?rev=305468&r1=<wbr>305467&r2=305468&view=diff</a><br>
> ==============================<wbr>==============================<wbr>==================<br>
> --- llvm/trunk/lib/CodeGen/<wbr>SelectionDAG/DAGCombiner.cpp (original)<br>
> +++ llvm/trunk/lib/CodeGen/<wbr>SelectionDAG/DAGCombiner.cpp Thu Jun 15 09:04:07 2017<br>
> @@ -12874,9 +12874,6 @@ bool DAGCombiner::<wbr>MergeConsecutiveStores<br>
> if (Ld->isVolatile() || Ld->isIndexed())<br>
> break;<br>
><br>
> - // We do not accept ext loads.<br>
> - if (Ld->getExtensionType() != ISD::NON_EXTLOAD)<br>
> - break;<br>
><br>
> // The stored memory type must be the same.<br>
> if (Ld->getMemoryVT() != MemVT)<br>
> @@ -13014,17 +13011,31 @@ bool DAGCombiner::<wbr>MergeConsecutiveStores<br>
><br>
> // The merged loads are required to have the same incoming chain, so<br>
> // using the first's chain is acceptable.<br>
> - SDValue NewLoad = DAG.getLoad(JointMemOpVT, LoadDL, FirstLoad->getChain(),<br>
> - FirstLoad->getBasePtr(),<br>
> - FirstLoad->getPointerInfo(), FirstLoadAlign);<br>
><br>
> SDValue NewStoreChain = getMergeStoreChains(<wbr>StoreNodes, NumElem);<br>
> -<br>
> AddToWorklist(NewStoreChain.<wbr>getNode());<br>
><br>
> - SDValue NewStore = DAG.getStore(<br>
> - NewStoreChain, StoreDL, NewLoad, FirstInChain->getBasePtr(),<br>
> - FirstInChain->getPointerInfo()<wbr>, FirstStoreAlign);<br>
> + SDValue NewLoad, NewStore;<br>
> + if (TLI.isTypeLegal(JointMemOpVT)<wbr>) {<br>
> + NewLoad = DAG.getLoad(JointMemOpVT, LoadDL, FirstLoad->getChain(),<br>
> + FirstLoad->getBasePtr(),<br>
> + FirstLoad->getPointerInfo(), FirstLoadAlign);<br>
> + NewStore = DAG.getStore(NewStoreChain, StoreDL, NewLoad,<br>
> + FirstInChain->getBasePtr(),<br>
> + FirstInChain->getPointerInfo()<wbr>, FirstStoreAlign);<br>
> + } else { // This must be the truncstore/extload case<br>
> + EVT ExtendedTy =<br>
> + TLI.getTypeToTransformTo(*DAG.<wbr>getContext(), JointMemOpVT);<br>
> + NewLoad = DAG.getExtLoad(ISD::EXTLOAD, LoadDL, ExtendedTy,<br>
> + FirstLoad->getChain(), FirstLoad->getBasePtr(),<br>
> + FirstLoad->getPointerInfo(), JointMemOpVT,<br>
> + FirstLoadAlign);<br>
> + NewStore = DAG.getTruncStore(<wbr>NewStoreChain, StoreDL, NewLoad,<br>
> + FirstInChain->getBasePtr(),<br>
> + FirstInChain->getPointerInfo()<wbr>, JointMemOpVT,<br>
> + FirstInChain->getAlignment(),<br>
> + FirstInChain->getMemOperand()-<wbr>>getFlags());<br>
> + }<br>
><br>
> // Transfer chain users from old loads to the new load.<br>
> for (unsigned i = 0; i < NumElem; ++i) {<br>
><br>
><br>
> ______________________________<wbr>_________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>