[llvm] r258971 - [SimplifyCFG] limit recursion depth when speculating instructions (PR26308)

[Hans Wennborg via llvm-commits]

Thanks. r259066.

On Thu, Jan 28, 2016 at 10:20 AM, David Majnemer <majnemer at google.com> wrote:
> Yes, it is sufficiently low risk.
>
> On Thu, Jan 28, 2016 at 10:18 AM, Hans Wennborg <hans at chromium.org> wrote:
>>
>> On Wed, Jan 27, 2016 at 11:22 AM, Sanjay Patel via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>> > Author: spatel
>> > Date: Wed Jan 27 13:22:45 2016
>> > New Revision: 258971
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=258971&view=rev
>> > Log:
>> > [SimplifyCFG] limit recursion depth when speculating instructions
>> > (PR26308)
>> >
>> > This is a fix for:
>> > https://llvm.org/bugs/show_bug.cgi?id=26308
>> >
>> > With the switch to using the TTI cost model in:
>> > http://reviews.llvm.org/rL228826
>> > ...it became possible to hit a zero-cost cycle of instructions (gep ->
>> > phi -> gep...),
>> > so we need a cap for the recursion in DominatesMergePoint().
>> >
>> > A recursion depth parameter was already added for a different reason in:
>> > http://reviews.llvm.org/rL255660
>> > ...so we can just set a limit for it.
>> >
>> > I pulled "10" out of the air and made it an independent parameter that
>> > we can play with.
>> > It might be higher than it needs to be given the currently low default
>> > value of
>> > PHINodeFoldingThreshold (2). That's the starting cost value that we
>> > enter the recursion
>> > with, and most instructions have cost set to TCC_Basic (1), so I don't
>> > think we're going
>> > to speculate more than 2 instructions with the current parameters.
>> >
>> > As noted in the review and the TODO comment, we can do better than just
>> > limiting recursion
>> > depth.
>> >
>> > Differential Revision: http://reviews.llvm.org/D16637
>>
>> Should this be merged to 3.8?
>
>