[llvm] r310583 - [ValueTracking] Enabling ValueTracking patch by default (recommit). Part 2.

[Chandler Carruth via llvm-commits]

On Thu, Aug 10, 2017 at 4:25 AM Nikolai Bozhenov via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: n.bozhenov
> Date: Thu Aug 10 04:24:57 2017
> New Revision: 310583
>
> URL: http://llvm.org/viewvc/llvm-project?rev=310583&view=rev
> Log:
> [ValueTracking] Enabling ValueTracking patch by default (recommit). Part 2.
>
> The original patch was an improvement to IR ValueTracking on non-negative
> integers. It has been checked in to trunk (D18777, r284022). But was
> disabled by
> default due to performance regressions.
> Perf impact has improved. The patch would be enabled by default.
>

Where are the numbers? I couldn't track anything down because...

Differential Revision: https://reviews.llvm.org/D34101


This is the *wrong* code review link, and only links to a test update.

Also:

>
> ==============================================================================
> --- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)
> +++ llvm/trunk/lib/Analysis/ValueTracking.cpp Thu Aug 10 04:24:57 2017
> @@ -54,12 +54,6 @@ const unsigned MaxDepth = 6;
>  static cl::opt<unsigned> DomConditionsMaxUses("dom-conditions-max-uses",
>                                                cl::Hidden, cl::init(20));
>
> -// This optimization is known to cause performance regressions is some
> cases,
> -// keep it under a temporary flag for now.
> -static cl::opt<bool>
> -DontImproveNonNegativePhiBits("dont-improve-non-negative-phi-bits",
> -                              cl::Hidden, cl::init(true));
> -
>

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.

Indeed, we've found it still causes serious compile time issues with code
such as:
https://github.com/qemu/qemu/blob/stable-1.0/hw/ide/core.c

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170814/539e6ee6/attachment-0001.html>