[PATCH] D12676: Ensure a complete post-dominance tree is built in the presence of unreachables

[Daniel Berlin via llvm-commits]

dberlin added a comment.

So, IIRC, in the dominance tree, unreachable blocks are deliberately part of the domtree, and we have dominance queries answerable about them.
What is the reason we should have different behavior for the two?

(Also, what happens if i try to make a post-dominates query about an unreachable block now?
It would be good to have a unit test for this)

I'm also not suggesting your patch is wrong, i'm suggesting our behavior in dominators may be undesirable as well, though i haven't thought about it.

Also note that your change to when we do addRoot will still not handle infinite loops properly:
We will end up with a null root node in some cases (infinite self-loops, for example), because it has  predecessors, (the entry and itself), and a successor (itself)
This will cause child_begin != child_end, and thus,  return false from isDomExit, which will cause us not to add a root, and end up with a null root node instead of a proper virtual root node.

It's one thing to leave the blocks out, it's another to say "there is no root node at all".   We are still supposed to have a root node, even if it's virtual.

Note that in general, it's not going to be possible to determine reachability, and what we should exclude anyway, without a DFS walk

Given that we know that the only way (at least, i can think of) to avoid adding another DFS walk is to make the root finding part of dom tree construction, i'd rather not add see us add isDomExit, because it's another thing that will need to be cleaned up to make this happen.
(The bug has more details).


================
Comment at: include/llvm/IR/Dominators.h:119
@@ +118,3 @@
+      GraphTraits<Function *>::child_end(N))
+    return false;
+
----------------
This is not a sufficient check for non-reachability.

What if i have a infinite self-loop?

https://llvm.org/bugs/show_bug.cgi?id=24415

(extend it to multiple blocks if you like)






http://reviews.llvm.org/D12676