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

[Sanjay Patel via llvm-commits]

On Wed, Jan 27, 2016 at 11:53 AM, Daniel Berlin <dberlin at dberlin.org> wrote:

>
>
> 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 :)
>

I agree with this. Sorry that I only have time to apply the bandage today,
but I'll add a TODO comment and try to get back here.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160127/87e2d1dd/attachment.html>