<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 25, 2015 at 8:46 AM, Robinson, Paul <span dir="ltr"><<a href="mailto:Paul_Robinson@playstation.sony.com" target="_blank">Paul_Robinson@playstation.sony.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div lang="EN-US" link="blue" vlink="purple">
<div><span class="">
<p style="margin-left:.5in"><a name="14c519ce5563d341__MailEndCompose">Leave the test case in and switch the expectation (& perhaps add a comment explaining why this the right choice and the previous one wasn't) so there's breadcrumbs if anyone else gets the same idea again
in the future?<u></u><u></u></a></p>
</span><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hmmmm the conditions where the test failed were pretty obscure and we used a sledgehammer to drive a finishing nail. But I could see a way to make the test
prove that if an optnone function bails out of fast-isel, the DAG combiner does run, which would be the point you're driving at. It'd get a false failure if fast-isel got smarter but with the right commentary that ought to be okay. Will work on this later
today.</span></p></div></div></blockquote><div><br>To keep the test non-brittle (though perhaps it'd become pointless, rather than a false failure) you could phrase the test in terms of comparing optnone and non-optnone behavior, ensuring they produce identical answers? Maybe - perhaps that's not a good idea, I'm not sure.<br><br>- David<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-US" link="blue" vlink="purple"><div><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">--paulr<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> David Blaikie [mailto:<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>]
<br>
<b>Sent:</b> Tuesday, March 24, 2015 7:13 PM<br>
<b>To:</b> Robinson, Paul<br>
<b>Cc:</b> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<b>Subject:</b> Re: [llvm] r233153 - 'optnone' should not disable DAG combiner.<u></u><u></u></span></p>
</div>
</div><div><div class="h5">
<p class="MsoNormal"><u></u> <u></u></p>
<p><br>
On Mar 24, 2015 5:18 PM, "Paul Robinson" <<a href="mailto:paul_robinson@playstation.sony.com" target="_blank">paul_robinson@playstation.sony.com</a>> wrote:<br>
><br>
> Author: probinson<br>
> Date: Tue Mar 24 19:10:24 2015<br>
> New Revision: 233153<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=233153&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=233153&view=rev</a><br>
> Log:<br>
> 'optnone' should not disable DAG combiner.<br>
><br>
> Reverts the code change from r221168 and the relevant test.<br>
> It was a mistake to disable the combiner, and based on the ultimate<br>
> definition of 'optnone' we shouldn't have considered the test case<br>
> as failing in the first place.<u></u><u></u></p>
<p>Leave the test case in and switch the expectation (& perhaps add a comment explaining why this the right choice and the previous one wasn't) so there's breadcrumbs if anyone else gets the same idea again in the future?<u></u><u></u></p>
<p>><br>
> Removed:<br>
> llvm/trunk/test/CodeGen/X86/fastmath-optnone.ll<br>
> Modified:<br>
> llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<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=233153&r1=233152&r2=233153&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=233153&r1=233152&r2=233153&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)<br>
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Tue Mar 24 19:10:24 2015<br>
> @@ -1183,11 +1183,6 @@ void DAGCombiner::Run(CombineLevel AtLev<br>
> LegalOperations = Level >= AfterLegalizeVectorOps;<br>
> LegalTypes = Level >= AfterLegalizeTypes;<br>
><br>
> - // Early exit if this basic block is in an optnone function.<br>
> - if (DAG.getMachineFunction().getFunction()->hasFnAttribute(<br>
> - Attribute::OptimizeNone))<br>
> - return;<br>
> -<br>
> // Add all the dag nodes to the worklist.<br>
> for (SelectionDAG::allnodes_iterator I = DAG.allnodes_begin(),<br>
> E = DAG.allnodes_end(); I != E; ++I)<br>
><br>
> Removed: llvm/trunk/test/CodeGen/X86/fastmath-optnone.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/fastmath-optnone.ll?rev=233152&view=auto" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/fastmath-optnone.ll?rev=233152&view=auto</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/CodeGen/X86/fastmath-optnone.ll (original)<br>
> +++ llvm/trunk/test/CodeGen/X86/fastmath-optnone.ll (removed)<br>
> @@ -1,35 +0,0 @@<br>
> -; RUN: llc < %s -mcpu=corei7 -march=x86-64 -mattr=+sse2 | FileCheck %s<br>
> -; Verify that floating-point operations inside 'optnone' functions<br>
> -; are not optimized even if unsafe-fp-math is set.<br>
> -<br>
> -define float @foo(float %x) #0 {<br>
> -entry:<br>
> - %add = fadd fast float %x, %x<br>
> - %add1 = fadd fast float %add, %x<br>
> - ret float %add1<br>
> -}<br>
> -<br>
> -; CHECK-LABEL: @foo<br>
> -; CHECK-NOT: add<br>
> -; CHECK: mul<br>
> -; CHECK-NOT: add<br>
> -; CHECK: ret<br>
> -<br>
> -define float @fooWithOptnone(float %x) #1 {<br>
> -entry:<br>
> - %add = fadd fast float %x, %x<br>
> - %add1 = fadd fast float %add, %x<br>
> - ret float %add1<br>
> -}<br>
> -<br>
> -; CHECK-LABEL: @fooWithOptnone<br>
> -; CHECK-NOT: mul<br>
> -; CHECK: add<br>
> -; CHECK-NOT: mul<br>
> -; CHECK: add<br>
> -; CHECK-NOT: mul<br>
> -; CHECK: ret<br>
> -<br>
> -<br>
> -attributes #0 = { "unsafe-fp-math"="true" }<br>
> -attributes #1 = { noinline optnone "unsafe-fp-math"="true" }<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">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><u></u><u></u></p>
</div></div></div>
</div>
</div>
</blockquote></div><br></div></div>