[PATCH] D43140: [Dominators] Always recalculate postdominators when update yields different roots

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added a comment.

In https://reviews.llvm.org/D43140#1005376, @kuhar wrote:

> In https://reviews.llvm.org/D43140#1005362, @dmgreen wrote:
>
> > > But any idea why verifyRoots did not catch this case already?
> >
> > Ah, I see. The roots were the same as newly calculated roots, but roots and the tree didn't match up. What do you think about adding a check for that to verifyRoots? (Not in this commit, I'm just throwing around ideas here). Or do you think a check against the recalculated tree is good enough? Something like:
> >
> >   for each node in tree:
> >     if node in roots:
> >       check node->idom == virtual root
> >     else
> >       check node->idom != virtual root
> >
>
>
> Yes, I think that is missing from the current implementation.


Forgot to mention: your pseudocode is not entirely correct -- there can be nodes connected to the virtual root that are not tree roots (e.g. diamond: entry, if then, else, exit) . But all roots have to be connected to the virtual root.


Repository:
  rL LLVM

https://reviews.llvm.org/D43140