<div dir="ltr">No. How do I do that? Also, I have a better fix planned. Should get around to implement that early next week.</div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jan 22, 2015 at 3:16 PM, Bruno Cardoso Lopes <span dir="ltr"><<a href="mailto:bruno.cardoso@gmail.com" target="_blank">bruno.cardoso@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 Daniel,<br>
<br>
I've tried the exact same approach back on Sep 2014 but I got a ~4%<br>
performance degradation in<br>
External/SPEC/CINT2006/462_libquantum/462_libquantum. Back in the day<br>
it would affect switch codegen in the "quantum_objcode_run" function,<br>
in objcode.c => jump tables and a bunch of code started to be emitted<br>
after the patch.<br>
<br>
Since it has been a while, much could have changed and that could no<br>
longer apply.<br>
Have you measured the impact of this on the test-suite?<br>
<br>
Thanks,<br>
<div class="HOEnZb"><div class="h5"><br>
On Tue, Jan 20, 2015 at 5:43 PM, Daniel Jasper <<a href="mailto:djasper@google.com">djasper@google.com</a>> wrote:<br>
> Author: djasper<br>
> Date: Tue Jan 20 13:43:33 2015<br>
> New Revision: 226600<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=226600&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=226600&view=rev</a><br>
> Log:<br>
> Prevent binary-tree deterioration in sparse switch statements.<br>
><br>
> This addresses part of <a href="http://llvm.org/PR22262" target="_blank">llvm.org/PR22262</a>. Specifically, it prevents<br>
> considering the densities of sub-ranges that have fewer than<br>
> TLI.getMinimumJumpTableEntries() elements. Those densities won't help<br>
> jump tables.<br>
><br>
> This is not a complete solution but works around the most pressing<br>
> issue.<br>
><br>
> Review: <a href="http://reviews.llvm.org/D7070" target="_blank">http://reviews.llvm.org/D7070</a><br>
><br>
> Modified:<br>
> llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp<br>
> llvm/trunk/test/CodeGen/X86/switch-bt.ll<br>
><br>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp?rev=226600&r1=226599&r2=226600&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp?rev=226600&r1=226599&r2=226600&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (original)<br>
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp Tue Jan 20 13:43:33 2015<br>
> @@ -2420,6 +2420,7 @@ bool SelectionDAGBuilder::handleBTSplitS<br>
> DEBUG(dbgs() << "Selecting best pivot: \n"<br>
> << "First: " << First << ", Last: " << Last <<'\n'<br>
> << "LSize: " << LSize << ", RSize: " << RSize << '\n');<br>
> + const TargetLowering &TLI = DAG.getTargetLoweringInfo();<br>
> for (CaseItr I = CR.Range.first, J=I+1, E = CR.Range.second;<br>
> J!=E; ++I, ++J) {<br>
> const APInt &LEnd = cast<ConstantInt>(I->High)->getValue();<br>
> @@ -2429,10 +2430,16 @@ bool SelectionDAGBuilder::handleBTSplitS<br>
> "Invalid case distance");<br>
> // Use volatile double here to avoid excess precision issues on some hosts,<br>
> // e.g. that use 80-bit X87 registers.<br>
> + // Only consider the density of sub-ranges that actually have sufficient<br>
> + // entries to be lowered as a jump table.<br>
> volatile double LDensity =<br>
> - LSize.roundToDouble() / (LEnd - First + 1ULL).roundToDouble();<br>
> + LSize.ult(TLI.getMinimumJumpTableEntries())<br>
> + ? 0.0<br>
> + : LSize.roundToDouble() / (LEnd - First + 1ULL).roundToDouble();<br>
> volatile double RDensity =<br>
> - RSize.roundToDouble() / (Last - RBegin + 1ULL).roundToDouble();<br>
> + RSize.ult(TLI.getMinimumJumpTableEntries())<br>
> + ? 0.0<br>
> + : RSize.roundToDouble() / (Last - RBegin + 1ULL).roundToDouble();<br>
> volatile double Metric = Range.logBase2() * (LDensity + RDensity);<br>
> // Should always split in some non-trivial place<br>
> DEBUG(dbgs() <<"=>Step\n"<br>
> @@ -2450,13 +2457,8 @@ bool SelectionDAGBuilder::handleBTSplitS<br>
> RSize -= J->size();<br>
> }<br>
><br>
> - const TargetLowering &TLI = DAG.getTargetLoweringInfo();<br>
> - if (areJTsAllowed(TLI)) {<br>
> - // If our case is dense we *really* should handle it earlier!<br>
> - assert((FMetric > 0) && "Should handle dense range earlier!");<br>
> - } else {<br>
> + if (FMetric == 0 || !areJTsAllowed(TLI))<br>
> Pivot = CR.Range.first + Size/2;<br>
> - }<br>
> splitSwitchCase(CR, Pivot, WorkList, SV, SwitchBB);<br>
> return true;<br>
> }<br>
><br>
> Modified: llvm/trunk/test/CodeGen/X86/switch-bt.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/switch-bt.ll?rev=226600&r1=226599&r2=226600&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/switch-bt.ll?rev=226600&r1=226599&r2=226600&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/CodeGen/X86/switch-bt.ll (original)<br>
> +++ llvm/trunk/test/CodeGen/X86/switch-bt.ll Tue Jan 20 13:43:33 2015<br>
> @@ -99,3 +99,61 @@ if.then:<br>
> if.end:<br>
> ret void<br>
> }<br>
> +<br>
> +; Ensure that optimizing for jump tables doesn't needlessly deteriorate the<br>
> +; created binary tree search. See PR22262.<br>
> +define void @test4(i32 %x, i32* %y) {<br>
> +; CHECK-LABEL: test4:<br>
> +<br>
> +entry:<br>
> + switch i32 %x, label %sw.default [<br>
> + i32 10, label %<a href="http://sw.bb" target="_blank">sw.bb</a><br>
> + i32 20, label %sw.bb1<br>
> + i32 30, label %sw.bb2<br>
> + i32 40, label %sw.bb3<br>
> + i32 50, label %sw.bb4<br>
> + i32 60, label %sw.bb5<br>
> + ]<br>
> +<a href="http://sw.bb" target="_blank">sw.bb</a>:<br>
> + store i32 1, i32* %y<br>
> + br label %sw.epilog<br>
> +sw.bb1:<br>
> + store i32 2, i32* %y<br>
> + br label %sw.epilog<br>
> +sw.bb2:<br>
> + store i32 3, i32* %y<br>
> + br label %sw.epilog<br>
> +sw.bb3:<br>
> + store i32 4, i32* %y<br>
> + br label %sw.epilog<br>
> +sw.bb4:<br>
> + store i32 5, i32* %y<br>
> + br label %sw.epilog<br>
> +sw.bb5:<br>
> + store i32 6, i32* %y<br>
> + br label %sw.epilog<br>
> +sw.default:<br>
> + store i32 7, i32* %y<br>
> + br label %sw.epilog<br>
> +sw.epilog:<br>
> + ret void<br>
> +<br>
> +; The balanced binary switch here would start with a comparison against 39, but<br>
> +; it is currently starting with 29 because of the density-sum heuristic.<br>
> +; CHECK: cmpl $29<br>
> +; CHECK: jg<br>
> +; CHECK: cmpl $10<br>
> +; CHECK: jne<br>
> +; CHECK: cmpl $49<br>
> +; CHECK: jg<br>
> +; CHECK: cmpl $30<br>
> +; CHECK: jne<br>
> +; CHECK: cmpl $20<br>
> +; CHECK: jne<br>
> +; CHECK: cmpl $50<br>
> +; CHECK: jne<br>
> +; CHECK: cmpl $40<br>
> +; CHECK: jne<br>
> +; CHECK: cmpl $60<br>
> +; CHECK: jne<br>
> +}<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto: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>
<br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">--<br>
Bruno Cardoso Lopes<br>
<a href="http://www.brunocardoso.cc" target="_blank">http://www.brunocardoso.cc</a><br>
</font></span></blockquote></div><br></div>