<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<!--[if !mso]><style>v\:* {behavior:url(#default#VML);}
o\:* {behavior:url(#default#VML);}
w\:* {behavior:url(#default#VML);}
.shape {behavior:url(#default#VML);}
</style><![endif]--><style><!--
/* Font Definitions */
@font-face
{font-family:Helvetica;
panose-1:2 11 6 4 2 2 2 2 2 4;}
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman",serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p
{mso-style-priority:99;
margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman",serif;}
span.EmailStyle18
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:#1F497D;}
span.EmailStyle19
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">This isn’t the actual fix I’d suggest, but it was the easiest way to implement the behavior so to see what the result would be.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">I’d be happy to prepare a better patch that implements this behavior in the target-specific functions that instantiate SelectionDAGISel if we’re agreed that this
is the right direction.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">-Andy<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><a name="_MailEndCompose"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></a></p>
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><a name="_____replyseparator"></a><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> Hal Finkel [mailto:hfinkel@anl.gov]
<br>
<b>Sent:</b> Thursday, July 07, 2016 7:48 PM<br>
<b>To:</b> Kaylor, Andrew <andrew.kaylor@intel.com><br>
<b>Cc:</b> llvm-commits <llvm-commits@lists.llvm.org>; Michael Kuperstein <mkuper@google.com><br>
<b>Subject:</b> Re: [llvm] r274786 - Include SelectionDAGISel in the opt-bisect process<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial",sans-serif;color:black"><o:p> </o:p></span></p>
<div class="MsoNormal" align="center" style="text-align:center"><span style="font-size:10.0pt;font-family:"Arial",sans-serif;color:black">
<hr size="3" width="100%" align="center" id="zwchr">
</span></div>
<blockquote style="border:none;border-left:solid #1010FF 1.5pt;padding:0in 0in 0in 4.0pt;margin-left:3.75pt;margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal" style="margin-bottom:12.0pt"><b><span style="font-family:"Helvetica",sans-serif;color:black">From:
</span></b><span style="font-family:"Helvetica",sans-serif;color:black">"Andrew Kaylor" <<a href="mailto:andrew.kaylor@intel.com">andrew.kaylor@intel.com</a>><br>
<b>To: </b>"Michael Kuperstein" <<a href="mailto:mkuper@google.com">mkuper@google.com</a>>,
<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a><br>
<b>Cc: </b>"llvm-commits" <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>><br>
<b>Sent: </b>Thursday, July 7, 2016 9:34:42 PM<br>
<b>Subject: </b>RE: [llvm] r274786 - Include SelectionDAGISel in the opt-bisect process<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Hi Michael,</span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">The reason my changes broke the PPC buildbots is that the SelectionDAGISel is always run at CodeGenOpt::Default for PPC targets, even if -O0 has been specified
on the command line. My change added a check for the OptNone function attribute and the opt-bisect-limit value in the SelectionDAGISel base class. There were tests in place to verify that no passes are skipped based on either of these options that are otherwise
run at -O0. With my change the pass isn't actually skipped, but it behaves differently (doesn't optimize anything) and so it is reported as being skipped. I had added a check to see if we started at CodeGenOpt::None to avoid false reporting, but since the
OptLevel is never set to CodeGenOpt::None for PPC targets the "skip” was reported and the tests failed.</span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Looking at the code, I expect that AMDGPU, BPF, Mips, and Sparc would all fail for the same reason.</span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">So it turns out that the fix for this isn't entirely simple. I removed the test cases that were failing, but they really should be put back soon. That leaves
the problem of how to avoid the failure for the platforms that aren't respecting the global CodeGenOpt level for SelectionDAGISel.</span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">It seems to me that all platforms should be handling CodeGenOpt levels correctly. There's a default value in the SelectionDAGISel constructor which leads to
-O2 behavior if no other value is explicitly passed in. I think that's a bad idea.</span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">I could arguably work around the test failure by suppressing the skip message in some circumstances or modifying the test to tolerate this one pass being skipped,
but that feels like a step in the wrong direction. Ultimately, the test failure led to an awareness that this pass was performing optimizations when optimizations should have been disabled and that seems like a good thing. The intention of the test was to
alert us to passes being skipped that shouldn't have been skipped, but it seems that it can also help point to passes performing optimizations when they shouldn't be doing so.</span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">My inclination at this point is to reinstate the failing test cases and mark this test as XFAIL for the platforms that aren't handling opt level for SelectionDAGISel.
That's less than ideal, but I think it's the best short-term solution available at the moment.</span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">There are six PowerPC tests which fail it the SelectionDAGISel::OptLevel member is set to CodeGenOpt::None when TM->getOptLevel() returns CodeGenOpt::None, but
I don’t know if the failing cases are testing required behavior or were just written to match actual output.</span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">(I'm CC'ing Hal Finkel for his opinion on the PowerPC behavior. Hal, can you drop me a note so I know you saw this?)</span><span style="color:black"><o:p></o:p></span></p>
</blockquote>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial",sans-serif;color:black">I assume that the tests are probably just matching whatever is the current output. I can't think of any reason why we'd need any particular isel optimizations for
correctness, but obviously I'd need to look at what's happening to be sure. Can you please send the patch which fixes this (minus test-case changes)? I, or one of the other PowerPC-backend developers, will fix up the test cases, etc.<br>
<br>
Once all of the targets are fixed, removing the constructor's default argument seems reasonable.<br>
<br>
-Hal<o:p></o:p></span></p>
<blockquote style="border:none;border-left:solid #1010FF 1.5pt;padding:0in 0in 0in 4.0pt;margin-left:3.75pt;margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Thanks,</span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Andy</span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:black">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:black"> Michael Kuperstein [<a href="mailto:mkuper@google.com">mailto:mkuper@google.com</a>]
<br>
<b>Sent:</b> Thursday, July 07, 2016 4:38 PM<br>
<b>To:</b> Kaylor, Andrew <<a href="mailto:andrew.kaylor@intel.com">andrew.kaylor@intel.com</a>><br>
<b>Cc:</b> llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>><br>
<b>Subject:</b> Re: [llvm] r274786 - Include SelectionDAGISel in the opt-bisect process</span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:black"> <o:p></o:p></span></p>
<div>
<p class="MsoNormal"><span style="color:black">Andrew, I think this broke the PPC bots. Could you please take a look?<o:p></o:p></span></p>
<div>
<p class="MsoNormal"><span style="color:black"> <o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black">Thanks,<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black"> Michael<o:p></o:p></span></p>
</div>
</div>
<div>
<p class="MsoNormal"><span style="color:black"> <o:p></o:p></span></p>
<div>
<p class="MsoNormal"><span style="color:black">On Thu, Jul 7, 2016 at 11:55 AM, Andrew Kaylor via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<o:p></o:p></span></p>
<blockquote style="border:none;border-left:solid windowtext 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt;border-color:-moz-use-text-color -moz-use-text-color -moz-use-text-color rgb(204, 204, 204)">
<p class="MsoNormal"><span style="color:black">Author: akaylor<br>
Date: Thu Jul 7 13:55:02 2016<br>
New Revision: 274786<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=274786&view=rev" target="_blank">
http://llvm.org/viewvc/llvm-project?rev=274786&view=rev</a><br>
Log:<br>
Include SelectionDAGISel in the opt-bisect process<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D21143" target="_blank">http://reviews.llvm.org/D21143</a><br>
<br>
<br>
Added:<br>
llvm/trunk/test/Other/X86/opt-bisect-isel.ll<br>
Modified:<br>
llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp<br>
<br>
Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp?rev=274786&r1=274785&r2=274786&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp?rev=274786&r1=274785&r2=274786&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp Thu Jul 7 13:55:02 2016<br>
@@ -443,7 +443,7 @@ bool SelectionDAGISel::runOnMachineFunct<br>
TM.resetTargetOptions(Fn);<br>
// Reset OptLevel to None for optnone functions.<br>
CodeGenOpt::Level NewOptLevel = OptLevel;<br>
- if (Fn.hasFnAttribute(Attribute::OptimizeNone))<br>
+ if (OptLevel != CodeGenOpt::None && skipFunction(Fn))<br>
NewOptLevel = CodeGenOpt::None;<br>
OptLevelChanger OLC(*this, NewOptLevel);<br>
<br>
<br>
Added: llvm/trunk/test/Other/X86/opt-bisect-isel.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/X86/opt-bisect-isel.ll?rev=274786&view=auto" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/X86/opt-bisect-isel.ll?rev=274786&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Other/X86/opt-bisect-isel.ll (added)<br>
+++ llvm/trunk/test/Other/X86/opt-bisect-isel.ll Thu Jul 7 13:55:02 2016<br>
@@ -0,0 +1,22 @@<br>
+; This test verifies that no optimizations are performed on the @f function<br>
+; when the -opt-bisect-limit=0 option is used. In particular, the X86<br>
+; instruction selector will optimize the cmp instruction to a sub instruction<br>
+; if it is not run in -O0 mode.<br>
+<br>
+; RUN: llc -O3 -opt-bisect-limit=0 -o - %s | FileCheck %s<br>
+<br>
+target triple = "x86_64-unknown-linux-gnu"<br>
+<br>
+define void @f() {<br>
+entry:<br>
+ %cmp = icmp slt i32 undef, 8<br>
+ br i1 %cmp, label %middle, label %end<br>
+<br>
+middle:<br>
+ br label %end<br>
+<br>
+end:<br>
+ ret void<br>
+}<br>
+<br>
+; CHECK: cmpl $8, %eax<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><o:p></o:p></span></p>
</blockquote>
</div>
<p class="MsoNormal"><span style="color:black"> <o:p></o:p></span></p>
</div>
</blockquote>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial",sans-serif;color:black"><br>
<br>
<br>
-- <o:p></o:p></span></p>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial",sans-serif;color:black">Hal Finkel<br>
Assistant Computational Scientist<br>
Leadership Computing Facility<br>
Argonne National Laboratory<o:p></o:p></span></p>
</div>
</div>
</div>
</body>
</html>