[PATCH] D35279: [Dominators] Improve reachability verification

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added inline comments.


================
Comment at: include/llvm/Support/GenericDomTreeConstruction.h:302
 
-      if (NodeToInfo.count(BB) == 0) {
+      // Virtual root doesn't have a corresponding CFG node.
+      if (DT.isVirtualRoot(TN)) continue;
----------------
dberlin wrote:
> It's more accurate to state that the virtual root *is* a cfg node. Or at least it was.
> At least as used previously, the  virtual root was not really a dom tree root, it was really the thing that contained the edges to the blocks.
> 
> The virtual dom tree root was then the dom tree node that got built for the virtual cfg root.
> 
> I don't know if this is still the case.
I mean, if you ask a virtual root for its (CFG) block, it returns a nullptr. Do you consider this nullptr block a virtual CFG exit node?


================
Comment at: include/llvm/Support/GenericDomTreeConstruction.h:305
+
+      if (!BB || NodeToInfo.count(BB) == 0) {
         errs() << "DomTree node ";
----------------
dberlin wrote:
> When is it possible to have !BB but it's not a virtual root?
> If it's really possible, please document this condition.
It's just an oversight when cherry-picking changes :)


https://reviews.llvm.org/D35279