[PATCH] D41298: [Dominators] Remove verifyDomTree and add some verifying for Post Dom Trees

Dave Green via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 2 11:59:47 PST 2018


dmgreen added a comment.

Hello

Sorry, other stuff has been keeping me busy :) From what I understand, both the old and the new code was wrong for when we force a recalc of the postdomtree, based on when the roots change.

This InsertReachable2 test, if I remember correctly, has three roots before the update:
 "6" - a normal exit/no successor node
 "9" - an infinite loop
 "2" - the common descendant of both.
After the insert, the infinite loop becomes (reverse-)reachable from 6, so the 9 and 2 nodes are no longer roots. But the tree was updated with the implicit assumption that they were connected to the virtual root, so the tree is wrong and needs a recalc.

We update the roots to just "6", check that it has a virtual base (it does) so decide not to recalc. The old code would check and do the recalc if it _did_ have a virtual root. I don't think that's what was intended - correct me if I'm wrong. Changing that fixed cases where the tree only had infinite loops as roots, and the root gets changed during the update.

So I think we may need to do the recalc "if the roots have changed". Or scan though the nodes in the tree and recalc if there is a node that has a virtual base, but is not a root. Not 100% sure. My cryptic code in the comment above was a suggestion for the "recalc if the roots have changed" option.


https://reviews.llvm.org/D41298





More information about the llvm-commits mailing list