[PATCH] D15537: limit the number of instructions per block examined by dead store elimination

[Teresa Johnson via llvm-commits]

On Mon, Apr 11, 2016 at 10:37 AM, Teresa Johnson <tejohnson at google.com>
wrote:

>
>
> On Mon, Apr 11, 2016 at 10:28 AM, Daniel Berlin <dberlin at dberlin.org>
> wrote:
>
>>
>>
>> On Mon, Apr 11, 2016 at 10:18 AM, Teresa Johnson <tejohnson at google.com>
>> wrote:
>>
>>> tejohnson added a comment.
>>>
>>> In http://reviews.llvm.org/D15537#397257, @davidxl wrote:
>>>
>>> > yes -- MemSSA will get there.  Short term workaround might also be
>>> needed :)
>>>
>>>
>>> I think we need a stopgap fix for this non-linear compile time issue.
>>
>>
>> Again, my main concern is that literally every other "stopgap" fix around
>> memory optimization issues has not been a stopgap.
>> There is no other way to put it.
>> These things have compounded *very* badly over time.
>> I'm not approving yet another compile time knob for another N^2/N^3 pass
>> that we know how to fix the right way.
>> (In part because i am the one who ends up having to fix it later :P)
>>
>> We have to hold the line somewhere. This is where i am holding it. If you
>> want to get someone else to approve it, i won't stand in the way.
>>
>>
>>> I don't think waiting until it is reimplemented to use MemorySSA is a
>>> good strategy given the extreme compile time issues,
>>
>>
>> Which, for the most part, only occur in new build modes or very
>> artificial code.
>>
>
> Machine generated code (not specifically related to ThinLTO although that
> was where I found it - I can reproduce with all ThinLTO importing
> disabled). Regardless of how artificial, it is still blocking my -O2
> compilation.
>

FYI,  bkramer found this in a non-ThinLTO build of the same source file,
and determined it was provoked by http://reviews.llvm.org/rL265762


>
>>
>>> and I can't even find a switch to disable this pass.
>>>
>>
>> This is definitely a problem.  Honestly, at this point, i would rather
>> see us disable the pass entirely with a large number of stores than add yet
>> another knob to control other things (like how many instructions it visits).
>>
>
> I'm fine with that approach, just arguing that we need something to
> address this as soon as possible.
>
>
>>
>>> BTW, the author had provided a test case on the mailing list (there are
>>> two threads for this patch):
>>>
>>>
>>> http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160201/330335.html
>>
>>
>> If you look at the testcase, it is *very* artificial :)
>>
>
>>
>>
>>>
>>>
>>> http://reviews.llvm.org/D15537
>>>
>>>
>>>
>>>
>>
>
>
> --
> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
> 408-460-2413
>



-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |  408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160412/21717459/attachment.html>