<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Thu, Aug 10, 2017 at 4:25 AM Nikolai Bozhenov via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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" rel="noreferrer" 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.<br></blockquote><div><br></div><div>Where are the numbers? I couldn't track anything down because...</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Differential Revision: <a href="https://reviews.llvm.org/D34101" rel="noreferrer" target="_blank">https://reviews.llvm.org/D34101</a></blockquote><div><br></div><div>This is the *wrong* code review link, and only links to a test update.</div><div><br></div><div>Also:</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">==============================================================================<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>
-<br></blockquote><div><br></div><div>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.</div><div><br></div><div>Indeed, we've found it still causes serious compile time issues with code such as:</div><div><a href="https://github.com/qemu/qemu/blob/stable-1.0/hw/ide/core.c">https://github.com/qemu/qemu/blob/stable-1.0/hw/ide/core.c</a><br></div><div><br></div><div>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.</div></div></div>