[PATCH] D16637: [SimplifyCFG] limit recursion depth when speculating instructions (PR26308)

[Daniel Berlin via llvm-commits]

On Wed, Jan 27, 2016 at 10:50 AM, David Majnemer <david.majnemer at gmail.com>
wrote:

> majnemer added a comment.
>
> In http://reviews.llvm.org/D16637#337486, @dberlin wrote:
>
> > If you do it on literally every two entry phi nodes, why is the answer
> not:
> >
> > Number each node of dominator tree with depth in dom tree (IE level).
> >  Process dominator tree in level order, visiting all nodes of a given
> level
> >  before the next deeper level:
> >
> >   Try to speculate a given phi node.
> >   Stop anytime you hit a phi node with a level above you above you
> (because
> >
> > you will have already speculated it if you could).
> >
> >   Track visited phi nodes to avoid cycles.
> >
> >
> >
> > Or something similar.
> >  Over time, we seem to expand a lot of lazy-walking optimizations to do
> >  these "walk possibly everything backwards" types of algorithms, and at
> the
> >  point we want to walk *everything of a certain type* there is no point
> in
> >  doing it lazily.
>
>
> You will find no disagreement on this point from me :)
> On the other hand, there is no reason not to apply this patch.


Well, there is the "if we just continuously bandaid stuff, we never force
ourselves to fix it" argument
Yes, it often means someone gets asked to do a bit more work when they are
just trying to fix a bug :)
So i'm not going to make it in this particular case.

I just would rather see us not add random limits, even when they are
obviously what was intended, if we can avoid it altogether, and i can try
to convince the submitter to take the time to do it :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160127/c812c8a0/attachment.html>