[llvm-branch-commits] [llvm-branch] r325108 - Merging r324962: (only the first hunk; see PR36375)

[Hans Wennborg via llvm-branch-commits]

Author: hans
Date: Wed Feb 14 02:16:43 2018
New Revision: 325108

URL: http://llvm.org/viewvc/llvm-project?rev=325108&view=rev
Log:
Merging r324962: (only the first hunk; see PR36375)

------------------------------------------------------------------------
r324962 | kuhar | 2018-02-13 00:37:27 +0100 (Tue, 13 Feb 2018) | 16 lines

[Dominators] Always recalculate postdominators when update yields different roots

Summary:
This patch makes postdominators always recalculate the tree when an update causes to change the tree roots.
As @dmgreen noticed in [[ https://reviews.llvm.org/D41298 | D41298 ]], the previous implementation was not conservative enough and it was possible to end up with a PostDomTree that was different than a freshly computed one.
The patch also compares postdominators with a freshly computed tree at the end of full verification to make sure we don't hit similar issues in the future.

This should (ideally) be also backported to 6.0 before the release, although I don't have any reports of this causing an observable error. It should be safe to do it even if it's late in the release, as the change only makes the current behavior more conservative.

Reviewers: dmgreen, dberlin, davide, brzycki, grosser

Reviewed By: brzycki, grosser

Subscribers: llvm-commits, dmgreen

Differential Revision: https://reviews.llvm.org/D43140
------------------------------------------------------------------------

Modified:
    llvm/branches/release_60/   (props changed)
    llvm/branches/release_60/include/llvm/Support/GenericDomTreeConstruction.h

Propchange: llvm/branches/release_60/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Feb 14 02:16:43 2018
@@ -1,3 +1,3 @@
 /llvm/branches/Apple/Pertwee:110850,110961
 /llvm/branches/type-system-rewrite:133420-134817
-/llvm/trunk:155241,321751,321789,321791,321806,321862,321870,321872,321878,321911,321980,321991,321993-321994,322003,322016,322053,322056,322103,322106,322108,322123,322131,322223,322272,322313,322372,322473,322623,322644,322724,322767,322875,322878-322879,322900,322904-322905,322973,322993,323034,323155,323190,323307,323331,323355,323369,323371,323384,323469,323515,323536,323582,323643,323671-323672,323706,323710,323759,323781,323810-323811,323813,323857,323907-323909,323913,323915,324002,324039,324422,324449,324497,324576,324645,324746,324772,325049,325085
+/llvm/trunk:155241,321751,321789,321791,321806,321862,321870,321872,321878,321911,321980,321991,321993-321994,322003,322016,322053,322056,322103,322106,322108,322123,322131,322223,322272,322313,322372,322473,322623,322644,322724,322767,322875,322878-322879,322900,322904-322905,322973,322993,323034,323155,323190,323307,323331,323355,323369,323371,323384,323469,323515,323536,323582,323643,323671-323672,323706,323710,323759,323781,323810-323811,323813,323857,323907-323909,323913,323915,324002,324039,324422,324449,324497,324576,324645,324746,324772,324962,325049,325085

Modified: llvm/branches/release_60/include/llvm/Support/GenericDomTreeConstruction.h
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_60/include/llvm/Support/GenericDomTreeConstruction.h?rev=325108&r1=325107&r2=325108&view=diff
==============================================================================
--- llvm/branches/release_60/include/llvm/Support/GenericDomTreeConstruction.h (original)
+++ llvm/branches/release_60/include/llvm/Support/GenericDomTreeConstruction.h Wed Feb 14 02:16:43 2018
@@ -698,24 +698,20 @@ struct SemiNCAInfo {
       return;
 
     // Recalculate the set of roots.
-    DT.Roots = FindRoots(DT, BUI);
-    for (const NodePtr R : DT.Roots) {
-      const TreeNodePtr TN = DT.getNode(R);
-      // A CFG node was selected as a tree root, but the corresponding tree node
-      // is not connected to the virtual root. This is because the incremental
-      // algorithm does not really know or use the set of roots and can make a
-      // different (implicit) decision about which nodes within an infinite loop
-      // becomes a root.
-      if (TN && !DT.isVirtualRoot(TN->getIDom())) {
-        DEBUG(dbgs() << "Root " << BlockNamePrinter(R)
-                     << " is not virtual root's child\n"
-                     << "The entire tree needs to be rebuilt\n");
-        // It should be possible to rotate the subtree instead of recalculating
-        // the whole tree, but this situation happens extremely rarely in
-        // practice.
-        CalculateFromScratch(DT, BUI);
-        return;
-      }
+    auto Roots = FindRoots(DT, BUI);
+    if (DT.Roots.size() != Roots.size() ||
+        !std::is_permutation(DT.Roots.begin(), DT.Roots.end(), Roots.begin())) {
+      // The roots chosen in the CFG have changed. This is because the
+      // incremental algorithm does not really know or use the set of roots and
+      // can make a different (implicit) decision about which node within an
+      // infinite loop becomes a root.
+
+      DEBUG(dbgs() << "Roots are different in updated trees\n"
+                   << "The entire tree needs to be rebuilt\n");
+      // It may be possible to update the tree without recalculating it, but
+      // we do not know yet how to do it, and it happens rarely in practise.
+      CalculateFromScratch(DT, BUI);
+      return;
     }
   }