[PATCH] D29705: Fix PR 24415 (at least), by making our post-dominator tree behavior sane.

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added inline comments.


================
Comment at: include/llvm/Support/GenericDomTreeConstruction.h:46
+
+    return Result;
   }
----------------
Sorry, this was part of an alternate approach, will revert.


================
Comment at: include/llvm/Support/GenericDomTreeConstruction.h:172
-  MultipleRoots |= (DT.isPostDominator() && N != GraphTraits<FuncT*>::size(&F));
-
   // When naively implemented, the Lengauer-Tarjan algorithm requires a separate
----------------
Keen observers will note that this ::size call was O(N) since it's an ilist.
:(
So the code i added really only adds O(N) time worst case, since we already wasted O(N) time just counting blocks above.


================
Comment at: include/llvm/Support/GenericDomTreeConstruction.h:194
+          // points in the infinite loop.
+          auto *FurthestAway = *po_begin(*I);
+          ConnectToExitBlock.insert(FurthestAway);
----------------
This comment needs updating a bit. It really ends up finding the infinite loop backedge block, which is what we want.
(note that post-dom is not unique, so if there are multiple backedges, you can't win, there is no "best" one)



================
Comment at: include/llvm/Support/GenericDomTreeConstruction.h:249
       typename InvTraits::NodeRef N = *CI;
-      if (DT.Info.count(N)) {  // Only if this predecessor is reachable!
+        // Only if this predecessor is reachable.
+      if (DT.Info.count(N)) {
----------------
This got clang-formatted, i'll revert


================
Comment at: include/llvm/Support/GenericDomTreeConstruction.h:285
+  // Detect how many roots we need by checking which things ended up with null
+  // idoms.
 
----------------
and this is leftover from a dead approach too.
i'll remove.


https://reviews.llvm.org/D29705