<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;}
p.MsoPlainText, li.MsoPlainText, div.MsoPlainText
{mso-style-priority:99;
mso-style-link:"Plain Text Char";
margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
span.PlainTextChar
{mso-style-name:"Plain Text Char";
mso-style-priority:99;
mso-style-link:"Plain Text";
font-family:"Calibri",sans-serif;}
span.EmailStyle19
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:#1F497D;}
span.EmailStyle20
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;
font-family:"Calibri",sans-serif;}
@page WordSection1
{size:8.5in 11.0in;
margin:56.7pt 42.5pt 56.7pt 85.05pt;}
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="MsoPlainText"><a name="_MailEndCompose">Hi Chandler,<o:p></o:p></a></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText"><span style="color:#7F7F7F;mso-style-textfill-fill-color:#7F7F7F;mso-style-textfill-fill-alpha:100.0%">> This is the *wrong* code review link, and only links to a test update.<o:p></o:p></span></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">The review link is correct. However, Phabricator stopped displaying the patch properly after some point. Here is an “almost fixed” link:<o:p></o:p></p>
<p class="MsoPlainText"><a href="https://reviews.llvm.org/D34101?vs=106237&id=110556&whitespace=ignore-most#toc">https://reviews.llvm.org/D34101?vs=106237&id=110556&whitespace=ignore-most#toc</a><o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText"><span style="color:#7F7F7F;mso-style-textfill-fill-color:#7F7F7F;mso-style-textfill-fill-alpha:100.0%">> https://github.com/qemu/qemu/blob/stable-1.0/hw/ide/core.c
<o:p></o:p></span></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">Could you suggest what options one should pass to compiler for core.c to reproduce the problem?<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText"><span style="color:#7F7F7F;mso-style-textfill-fill-color:#7F7F7F;mso-style-textfill-fill-alpha:100.0%">> Where are the numbers?<o:p></o:p></span></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">I’m not sure what numbers you are asking us for. When the feature was first implemented, we saw some regressions on important benchmarks. That’s why it was disabled with this quite ugly flag. With newer versions of the compiler we cannot
reproduce those regressions, and with the old compiler the regressions were found to be random fluctuations that cannot be addressed in the compiler.<span style="color:#1F497D"><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:#525252">-Nikolai<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"><o:p> </o:p></span></p>
<p class="MsoNormal"><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"> Chandler Carruth [mailto:chandlerc@gmail.com]
<br>
<b>Sent:</b> Monday, August 14, 2017 9:05 AM<br>
<b>To:</b> Bozhenov, Nikolai <nikolai.bozhenov@intel.com>; llvm-commits@lists.llvm.org<br>
<b>Subject:</b> Re: [llvm] r310583 - [ValueTracking] Enabling ValueTracking patch by default (recommit). Part 2.<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">Revert landed in r310816. Let me know if you need any help getting the compile issues to reproduce!<o:p></o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">-Chandler<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Sun, Aug 13, 2017 at 11:53 PM Chandler Carruth <<a href="mailto:chandlerc@gmail.com">chandlerc@gmail.com</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<div>
<div>
<div>
<p class="MsoNormal">On Thu, Aug 10, 2017 at 4:25 AM Nikolai Bozhenov 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>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<p class="MsoNormal">Author: n.bozhenov<br>
Date: Thu Aug 10 04:24:57 2017<br>
New Revision: 310583<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=310583&view=rev" target="_blank">
http://llvm.org/viewvc/llvm-project?rev=310583&view=rev</a><br>
Log:<br>
[ValueTracking] Enabling ValueTracking patch by default (recommit). Part 2.<br>
<br>
The original patch was an improvement to IR ValueTracking on non-negative<br>
integers. It has been checked in to trunk (D18777, r284022). But was disabled by<br>
default due to performance regressions.<br>
Perf impact has improved. The patch would be enabled by default.<o:p></o:p></p>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</div>
<div>
<div>
<div>
<p class="MsoNormal">Where are the numbers? I couldn't track anything down because...<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<p class="MsoNormal">Differential Revision: <a href="https://reviews.llvm.org/D34101" target="_blank">
https://reviews.llvm.org/D34101</a><o:p></o:p></p>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">This is the *wrong* code review link, and only links to a test update.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Also:<o:p></o:p></p>
</div>
</div>
</div>
<div>
<div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<p class="MsoNormal">==============================================================================<br>
--- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)<br>
+++ llvm/trunk/lib/Analysis/ValueTracking.cpp Thu Aug 10 04:24:57 2017<br>
@@ -54,12 +54,6 @@ const unsigned MaxDepth = 6;<br>
static cl::opt<unsigned> DomConditionsMaxUses("dom-conditions-max-uses",<br>
cl::Hidden, cl::init(20));<br>
<br>
-// This optimization is known to cause performance regressions is some cases,<br>
-// keep it under a temporary flag for now.<br>
-static cl::opt<bool><br>
-DontImproveNonNegativePhiBits("dont-improve-non-negative-phi-bits",<br>
- cl::Hidden, cl::init(true));<br>
-<o:p></o:p></p>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</div>
<div>
<div>
<div>
<p class="MsoNormal">Given that this has already been disabled before (and had to be reverted a few times) it seems very prudent to leave the flag in place and just change the default value at first.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Indeed, we've found it still causes serious compile time issues with code such as:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><a href="https://github.com/qemu/qemu/blob/stable-1.0/hw/ide/core.c" target="_blank">https://github.com/qemu/qemu/blob/stable-1.0/hw/ide/core.c</a>
<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">I'm going to revert this for now. If you can't reproduce, we can get a test case together, but building qemu is likely sufficient.<o:p></o:p></p>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</body>
</html>