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

[Daniel Berlin via llvm-commits]

Thanks. No worries, just wanted to see if there was any chance to improve
this while you were in there :)


On Wed, Jan 27, 2016 at 11:18 AM, Sanjay Patel <spatel at rotateright.com>
wrote:

>
>
> 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/c0c1511b/attachment.html>