Oh wow, sorry. I knew there was a register pressure risk, but I guessed wrongly that it would be contained by the limited length of the transform. <div><br></div><div>If it's easier, please feel free to revert. I won't be at my dev machine until tomorrow AM.<div><br></div><div>I'm not seeing how in-order vs. OOO is a factor?<br><br>On Wednesday, April 29, 2015, Owen Anderson <<a href="mailto:resistor@mac.com">resistor@mac.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Woah! This change is very not OK for in-order targets! Additionally, we’re observing massive regressions, including catastrophic spilling and catastrophic compile time, resulting from it. We’re still reducing/sanitizing a testcase we can share, but *at least* this needs to move under some kind of target hook so that in-order targets can opt-out.<br>
<br>
—Owen<br>
<br>
> On Apr 28, 2015, at 2:03 PM, Sanjay Patel <<a href="javascript:;" onclick="_e(event, 'cvml', 'spatel@rotateright.com')">spatel@rotateright.com</a>> wrote:<br>
><br>
> Author: spatel<br>
> Date: Tue Apr 28 16:03:22 2015<br>
> New Revision: 236031<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=236031&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=236031&view=rev</a><br>
> Log:<br>
> transform fadd chains to increase parallelism<br>
><br>
> This is a compromise: with this simple patch, we should always handle a chain of exactly 3<br>
> operations optimally, but we're not generating the optimal balanced binary tree for a longer<br>
> sequence.<br>
><br>
> In general, this transform will reduce the dependency chain for a sequence of instructions<br>
> using N operands from a worst case N-1 dependent operations to N/2 dependent operations.<br>
> The optimal balanced binary tree would reduce the chain to log2(N).<br>
><br>
> The trade-off for not dealing with longer sequences is: (1) we have less complexity in the<br>
> compiler, (2) we avoid unknown compile-time blowup calculating a balanced tree, and (3) we<br>
> don't need to worry about the increased register pressure required to parallelize longer<br>
> sequences. It also seems unlikely that we would ever encounter really long strings of<br>
> dependent ops like that in the wild, but I'm not sure how to verify that speculation.<br>
> FWIW, I see no perf difference for test-suite running on btver2 (x86-64) with -ffast-math<br>
> and this patch.<br>
><br>
> We can extend this patch to cover other associative operations such as fmul, fmax, fmin,<br>
> integer add, integer mul.<br>
><br>
> This is a partial fix for:<br>
> <a href="https://llvm.org/bugs/show_bug.cgi?id=17305" target="_blank">https://llvm.org/bugs/show_bug.cgi?id=17305</a><br>
><br>
> and if extended:<br>
> <a href="https://llvm.org/bugs/show_bug.cgi?id=21768" target="_blank">https://llvm.org/bugs/show_bug.cgi?id=21768</a><br>
> <a href="https://llvm.org/bugs/show_bug.cgi?id=23116" target="_blank">https://llvm.org/bugs/show_bug.cgi?id=23116</a><br>
><br>
> The issue also came up in:<br>
> <a href="http://reviews.llvm.org/D8941" target="_blank">http://reviews.llvm.org/D8941</a><br>
><br>
> Differential Revision: <a href="http://reviews.llvm.org/D9232" target="_blank">http://reviews.llvm.org/D9232</a><br>
><br>
><br>
> Modified:<br>
> llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br>
> llvm/trunk/test/CodeGen/X86/fp-fast.ll<br>
><br>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=236031&r1=236030&r2=236031&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=236031&r1=236030&r2=236031&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)<br>
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Tue Apr 28 16:03:22 2015<br>
> @@ -7801,6 +7801,24 @@ SDValue DAGCombiner::visitFADD(SDNode *N<br>
> N0.getOperand(0), DAG.getConstantFP(4.0, DL, VT));<br>
> }<br>
> }<br>
> +<br>
> + // Canonicalize chains of adds to LHS to simplify the following transform.<br>
> + if (N0.getOpcode() != ISD::FADD && N1.getOpcode() == ISD::FADD)<br>
> + return DAG.getNode(ISD::FADD, SDLoc(N), VT, N1, N0);<br>
> +<br>
> + // Convert a chain of 3 dependent operations into 2 independent operations<br>
> + // and 1 dependent operation:<br>
> + // (fadd N0: (fadd N00: (fadd z, w), N01: y), N1: x) -><br>
> + // (fadd N00: (fadd z, w), (fadd N1: x, N01: y))<br>
> + if (N0.getOpcode() == ISD::FADD && N0.hasOneUse() &&<br>
> + N1.getOpcode() != ISD::FADD) {<br>
> + SDValue N00 = N0.getOperand(0);<br>
> + if (N00.getOpcode() == ISD::FADD) {<br>
> + SDValue N01 = N0.getOperand(1);<br>
> + SDValue NewAdd = DAG.getNode(ISD::FADD, SDLoc(N), VT, N1, N01);<br>
> + return DAG.getNode(ISD::FADD, SDLoc(N), VT, N00, NewAdd);<br>
> + }<br>
> + }<br>
> } // enable-unsafe-fp-math<br>
><br>
> // FADD -> FMA combines:<br>
><br>
> Modified: llvm/trunk/test/CodeGen/X86/fp-fast.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/fp-fast.ll?rev=236031&r1=236030&r2=236031&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/fp-fast.ll?rev=236031&r1=236030&r2=236031&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/CodeGen/X86/fp-fast.ll (original)<br>
> +++ llvm/trunk/test/CodeGen/X86/fp-fast.ll Tue Apr 28 16:03:22 2015<br>
> @@ -113,3 +113,46 @@ define float @test11(float %a) {<br>
> %t2 = fadd float %a, %t1<br>
> ret float %t2<br>
> }<br>
> +<br>
> +; Verify that the first two adds are independent; the destination registers<br>
> +; are used as source registers for the third add.<br>
> +<br>
> +define float @reassociate_adds1(float %a, float %b, float %c, float %d) {<br>
> +; CHECK-LABEL: reassociate_adds1:<br>
> +; CHECK: # BB#0:<br>
> +; CHECK-NEXT: vaddss %xmm1, %xmm0, %xmm0<br>
> +; CHECK-NEXT: vaddss %xmm2, %xmm3, %xmm1<br>
> +; CHECK-NEXT: vaddss %xmm1, %xmm0, %xmm0<br>
> +; CHECK-NEXT: retq<br>
> + %add0 = fadd float %a, %b<br>
> + %add1 = fadd float %add0, %c<br>
> + %add2 = fadd float %add1, %d<br>
> + ret float %add2<br>
> +}<br>
> +<br>
> +define float @reassociate_adds2(float %a, float %b, float %c, float %d) {<br>
> +; CHECK-LABEL: reassociate_adds2:<br>
> +; CHECK: # BB#0:<br>
> +; CHECK-NEXT: vaddss %xmm1, %xmm0, %xmm0<br>
> +; CHECK-NEXT: vaddss %xmm2, %xmm3, %xmm1<br>
> +; CHECK-NEXT: vaddss %xmm1, %xmm0, %xmm0<br>
> +; CHECK-NEXT: retq<br>
> + %add0 = fadd float %a, %b<br>
> + %add1 = fadd float %c, %add0<br>
> + %add2 = fadd float %add1, %d<br>
> + ret float %add2<br>
> +}<br>
> +<br>
> +define float @reassociate_adds3(float %a, float %b, float %c, float %d) {<br>
> +; CHECK-LABEL: reassociate_adds3:<br>
> +; CHECK: # BB#0:<br>
> +; CHECK-NEXT: vaddss %xmm1, %xmm0, %xmm0<br>
> +; CHECK-NEXT: vaddss %xmm2, %xmm3, %xmm1<br>
> +; CHECK-NEXT: vaddss %xmm1, %xmm0, %xmm0<br>
> +; CHECK-NEXT: retq<br>
> + %add0 = fadd float %a, %b<br>
> + %add1 = fadd float %add0, %c<br>
> + %add2 = fadd float %d, %add1<br>
> + ret float %add2<br>
> +}<br>
> +<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="javascript:;" onclick="_e(event, 'cvml', 'llvm-commits@cs.uiuc.edu')">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
</blockquote></div></div>