[PATCH] D35279: [Dominators] Improve reachability verification

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar marked 2 inline comments as done.
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:
> kuhar wrote:
> > kuhar wrote:
> > > 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?
> > (Or in the future, a virtual entry block for dominators?)
> The fact that we don't hand the virtual CFG block to people is fine (but possibly annoying):
> 
> Remember the dominator tree does not have predecessor edges, etc.
> 
> So either you are hard coding the logic for it being a CFG block, or it's really got a virtual CFG block :P
> 
> Right now I believe it's the case we hard-code the logic during post-dom construction.
> 
Is it better now?


https://reviews.llvm.org/D35279