[llvm] r296535 - Fix PR 24415 (at least), by making our post-dominator tree behavior sane.

[Daniel Berlin via llvm-commits]

Hey Tobias,
Just wanted to ping this and see if you had a chance to think more.
Correctness here is now blocking both some optimization work (which relies
on the invariants of the post-dom tree being met) and some verification
work.


On Thu, Mar 2, 2017 at 3:01 PM, Daniel Berlin <dberlin at dberlin.org> wrote:

>
>
> On Thu, Mar 2, 2017 at 12:10 PM, Tobias Grosser <
> tobias.grosser at inf.ethz.ch> wrote:
>
>> On Thu, Mar 2, 2017, at 07:54 PM, Daniel Berlin wrote:
>> > (and, fwiw, i wrote this in a rush because i'm on vacation and running
>> > around, but wanted to make sure i responded.
>> > I didn't edit it very hard, so please don't assume I mean anything mean
>> > by
>> > saying "non-standard", etc, in a bunch of places :P)
>>
>> Hi Dani,
>>
>> I don't think there is a clear disagreement yet, I mostly want to get
>> time to carefully think about all implications. I know you have (and
>> there are) good reasons and arguments to go this way. Still, I believe
>> some of the arguments are not as black and white as you point them out
>> at the moment and not all implications / choices have been discussed
>> (certainly not with me)  and documented. Maybe they are indeed obvious
>> and just not clear to me, but in this case maybe we can also use this
>> opportunity to better document the choices we took and the guarantees we
>> give.
>>
>
> Sure.
>
>
>>
>> As you seem on vacation, there does not seem to be a huge urge to get
>> this change in (especially as the current status was there for almost
>> ten years). I will take the weekend to look into this again in great
>> detail, also taking into account the good arguments you give, the other
>> bug-reports that are affected, ...
>>
>
> Sounds good to me.
> Now that i have a second:
>
> There are two problems i see:
>
> 1. I don't believe it is possible to satisfy the normal path invariants of
> the post-dom tree with infinite loops without connecting them to a block
> that goes to virtual exit.
> If you can think of a way, awesome.  I'm pretty sure, but not positive,
> that it's not possible.  I know Tarjan didn't think it was possible, and
> he's a lot smarter than me :)
>
> 2. The other underlying problem is that reverse-unreachable is different
> than forward-unreachable.
> We can legally just ignore all forward-unreachable things, and delete
> them, or otherwise not care about them, because they can never execute.
> Reverse-unreachable things can still execute, so we can't just ignore
> them.  If we can't ignore them, and we want to form a post-dominator tree
> that includes them, we run into the first problem.
>
> The options i can think of:
> 1. Given an option to ignore reverse-unreachable  and build an analysis
> name for that.
> Note that we can actually do this in a more complete way than we did
> pre-patch. post-patch, we properly detect *all* which things do not reach
> program exit (instead of just some of them) and so we could simply not
> connect them anywhere, and you will get a post-dom tree that doesn't
> include them (and isn't "flattened")
> This would be more "correct" than what we did pre-patch, in the sense that
> you would get a post-dom tree without infinite loops in it, instead of
> "with some types of infinite loops ending up in weird places, and other
> types of loops and noreturns completely ignored".
>
> Note: This is also more correct in that the current region building code
> assumes the path invariants of the dom-tree and post-dom tree hold, and
> thus included, in a region, things that appear on a walk from one node to
> another (regardless of where the would otherwise appear in the dom/post-dom
> tree)
>
> So in that sense, removing things from both trees and building regions
> should give you regions that don't include those nodes, and would be
> correct if those nodes did not exist (as opposed, again, to pre-patch,
> where what you get depends on the CFG structure of the  infinite loop or
> noreturn that existed, etc)
>
> 2. (and i don't know if this helps) Create a second virtual node
> post-dom'd by virtual exit the represents all the fake-exits.
> This would let you ignore them all at once, but still leaves a "flattened"
> post-dom tree.
>
> IE
>
> virtual exit
>   /                                          |
> virtual infinite loop exit         each normal returning node as a sibling
>    |
> each infinite loop backedge as a child
>
>
> The problem is the dom-tree will still be "flatter" overall for reachable
> nodes.
> Note: It's already the case, that post-patch, any direct child of virtual
> exit not ending in a return must be fake, so this would just make it
> slightly easier.
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170310/d93c37b9/attachment-0001.html>