[PATCH] D28767: Do not verify Dominator tree if it has no roots

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added a comment.

This looks, IMHO, very wrong.
We require domtrees to have roots at all times.
See getRoot, for example, which says:

  NodeT *getRoot() const {
    assert(this->Roots.size() == 1 && "Should always have entry node!");
    return this->Roots[0];
  }

So if we've created a DominatorTree to call verifyDomTree on,and it doesn't have a root, we have in fact, created a broken dom tree and verification *should* fail.

I'm not sure why we are creating such a tree, but it's *definitely invalid* according to how we work right now.
All this patch does is paper over that by calling getRoots instead of getRoot, because getRoots happens to not assert (we should add a non-emptyness assert there too).

I think you need to fix the underlying problem of creating DominatorTrees without roots, or start a discussion on llvmdev about whether the empty dom tree should be valid.
(Preferably, the former).

Because so far, we've never considered "the completely empty" domtree to be valid, and we haven't audited our functions to make sure they all work properly in such a world.


Repository:
  rL LLVM

https://reviews.llvm.org/D28767