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

[David Majnemer via llvm-commits]

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?
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160128/cd9581e9/attachment.html>