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

Jakub Kuderski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 12 05:23:55 PST 2018


kuhar added a comment.

In https://reviews.llvm.org/D43140#1004849, @dmgreen wrote:

> But any idea why verifyRoots did not catch this case already? If the newly calculated roots are different from what is in the PDT.


The reasoning behind the original implementation was that there are multiple correct postdominator trees for some CFG-s (in particular, when there are infinite loops), but we wanted to have just one canonical postdomtree that we end up with, no matter if we get it as a result of an update or by fresh construction. I though that it would be sufficient to check whether the new set of roots if also a root for the current CFG, but your example with InsertReachable2 showed there are cases were it's not enough and we have to look deeper. And I think that your previous patch to the root selection made some cases like this that crashed the original implementation to silently continue instead, which is how we ended up in the current situation.


Repository:
  rL LLVM

https://reviews.llvm.org/D43140





More information about the llvm-commits mailing list