<div dir="ltr"><div>The fact is that sometimes domtree is not available. If a function is `available_externally`  all codegen passes are skipped, although they are marked as required. MachineDominatorTree is also skipped and there is no domtree at all. Attempt to call `getRoot` for such domtree will obviously cause a crash. There is no way to get 'right' domtree in this case, so prior to using it we must either:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">      </span>1. Check if corresponding pass was indeed run, or</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">      </span>2. Check the domtree state trying to reveal if it is valid.</div><div>The first approach was taken in <a href="https://reviews.llvm.org/D27190">https://reviews.llvm.org/D27190</a> but it was not accepted. This fix tries to use variant 2.</div><div><br></div><div>Just to emphasize a key point: there are cases when domtree is absent because the pass that it creates was not run and this behavior is by design.</div><div><br></div></div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature" data-smartmail="gmail_signature">Thanks,<br>--Serge<br></div></div>
<br><div class="gmail_quote">2017-01-25 21:08 GMT+07:00 Daniel Berlin via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">dberlin added a comment.<br>
<br>
This looks, IMHO, very wrong.<br>
We require domtrees to have roots at all times.<br>
See getRoot, for example, which says:<br>
<br>
  NodeT *getRoot() const {<br>
    assert(this->Roots.size() == 1 && "Should always have entry node!");<br>
    return this->Roots[0];<br>
  }<br>
<br>
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.<br>
<br>
I'm not sure why we are creating such a tree, but it's *definitely invalid* according to how we work right now.<br>
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).<br>
<br>
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.<br>
(Preferably, the former).<br>
<br>
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.<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D28767" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D28767</a><br>
<br>
<br>
<br>
</div></div></blockquote></div><br></div>