<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)">
<style><!--
/* Font Definitions */
@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;}
span.EmailStyle17
        {mso-style-type:personal-reply;
        font-family:"Calibri",sans-serif;
        color:#1F497D;}
.MsoChpDefault
        {mso-style-type:export-only;
        font-family:"Calibri",sans-serif;}
@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">Hi Michael,<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">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.<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">Looking at the code, I expect that AMDGPU, BPF, Mips, and Sparc would all fail for the same reason.<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">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.<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">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.<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 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.<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">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.<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">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.<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'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?)<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">Thanks,<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"><a name="_MailEndCompose"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></a></p>
<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"> Michael Kuperstein [mailto:mkuper@google.com]
<br>
<b>Sent:</b> Thursday, July 07, 2016 4:38 PM<br>
<b>To:</b> Kaylor, Andrew <andrew.kaylor@intel.com><br>
<b>Cc:</b> llvm-commits <llvm-commits@lists.llvm.org><br>
<b>Subject:</b> Re: [llvm] r274786 - Include SelectionDAGISel in the opt-bisect process<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">Andrew, I think this broke the PPC bots. Could you please take a look?<o:p></o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Thanks,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">  Michael<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">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></p>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal">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">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></p>
</blockquote>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</body>
</html>