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

[Chandler Carruth via llvm-commits]

Revert landed in r310816. Let me know if you need any help getting the
compile issues to reproduce!

-Chandler

On Sun, Aug 13, 2017 at 11:53 PM Chandler Carruth <chandlerc at gmail.com>
wrote:

> 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/8d0b4ca9/attachment.html>