<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 14 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
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.MsoAcetate, li.MsoAcetate, div.MsoAcetate
{mso-style-priority:99;
mso-style-link:"Balloon Text Char";
margin:0cm;
margin-bottom:.0001pt;
font-size:8.0pt;
font-family:"Tahoma","sans-serif";}
p.MsoListParagraph, li.MsoListParagraph, div.MsoListParagraph
{mso-style-priority:34;
margin-top:0cm;
margin-right:0cm;
margin-bottom:0cm;
margin-left:36.0pt;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
span.BalloonTextChar
{mso-style-name:"Balloon Text Char";
mso-style-priority:99;
mso-style-link:"Balloon Text";
font-family:"Tahoma","sans-serif";}
span.EmailStyle20
{mso-style-type:personal;
font-family:"Calibri","sans-serif";
color:#1F497D;}
span.EmailStyle21
{mso-style-type:personal;
font-family:"Calibri","sans-serif";
color:windowtext;}
span.EmailStyle22
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:72.0pt 72.0pt 72.0pt 72.0pt;}
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-GB" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">I've just refreshed D14900. Would you be willing to review it?<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p> </o:p></span></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0cm 0cm 0cm 4.0pt">
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Daniel Sanders
<br>
<b>Sent:</b> 11 July 2016 13:53<br>
<b>To:</b> 'Kaylor, Andrew'; Michael Kuperstein; hfinkel@anl.gov; llvm-commits<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>
<p class="MsoNormal"><span lang="EN-US" 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. <o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">There's a patch to fix this for Mips (<a href="http://reviews.llvm.org/D14900">http://reviews.llvm.org/D14900</a>) that I've been meaning to refresh and ping for a while
but I've been busy on other things. I'll re-test it and refresh the patch if all goes well.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p> </o:p></span></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0cm 0cm 0cm 4.0pt">
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> llvm-commits [mailto:llvm-commits-bounces@lists.llvm.org]
<b>On Behalf Of </b>Kaylor, Andrew via llvm-commits<br>
<b>Sent:</b> 08 July 2016 03:35<br>
<b>To:</b> Michael Kuperstein; hfinkel@anl.gov<br>
<b>Cc:</b> llvm-commits<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>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Hi Michael,<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" 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 lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" 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 lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" 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 lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" 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 lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" 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 lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" 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 lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" 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 lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" 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 lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Thanks,<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" 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"></a><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><a name="_____replyseparator"></a><b><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">From:</span></b><span lang="EN-US" 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"><span lang="EN-US"><o:p> </o:p></span></p>
<div>
<p class="MsoNormal"><span lang="EN-US">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 lang="EN-US"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">Thanks,<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US"> Michael<o:p></o:p></span></p>
</div>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<div>
<p class="MsoNormal"><span lang="EN-US">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 #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0cm;margin-bottom:5.0pt">
<p class="MsoNormal"><span lang="EN-US">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></span></p>
</blockquote>
</div>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
</div>
</div>
</div>
</div>
</body>
</html>