Revert "DomTree: Make PostDomTree immune to block successors swap"

This reverts commit c35585e209efe69e2233bdc5ecd23bed7b735ba3.

The MLIR is broken with this patch, reproduce by adding
-DLLVM_ENABLE_PROJECTS=mlir to the cmake configuration and
build `ninja tools/mlir/lib/IR/CMakeFiles/obj.MLIRIR.dir/Dominance.cpp.o`

diff --git a/llvm/include/llvm/Support/GenericDomTreeConstruction.h b/llvm/include/llvm/Support/GenericDomTreeConstruction.h
index 3c85caf..6a9d38b 100644 (file)
--- a/llvm/include/llvm/Support/GenericDomTreeConstruction.h
+++ b/llvm/include/llvm/Support/GenericDomTreeConstruction.h
@@ -151,8 +151,6 @@ struct SemiNCAInfo {
     }
   };
 
-  using NodeOrderMap = DenseMap<NodePtr, unsigned>;
-
   // Custom DFS implementation which can skip nodes based on a provided
   // predicate. It also collects ReverseChildren so that we don't have to spend
   // time getting predecessors in SemiNCA.
@@ -160,13 +158,9 @@ struct SemiNCAInfo {
   // If IsReverse is set to true, the DFS walk will be performed backwards
   // relative to IsPostDom -- using reverse edges for dominators and forward
   // edges for postdominators.
-  //
-  // If SuccOrder is specified then in this order the DFS traverses the children
-  // otherwise the order is implied by the results of getChildren().
   template <bool IsReverse = false, typename DescendCondition>
   unsigned runDFS(NodePtr V, unsigned LastNum, DescendCondition Condition,
-                  unsigned AttachToNum,
-                  const NodeOrderMap *SuccOrder = nullptr) {
+                  unsigned AttachToNum) {
     assert(V);
     SmallVector<NodePtr, 64> WorkList = {V};
     if (NodeToInfo.count(V) != 0) NodeToInfo[V].Parent = AttachToNum;
@@ -182,14 +176,7 @@ struct SemiNCAInfo {
       NumToNode.push_back(BB);
 
       constexpr bool Direction = IsReverse != IsPostDom;  // XOR.
-      auto Successors = getChildren<Direction>(BB, BatchUpdates);
-      if (SuccOrder && Successors.size() > 1)
-        llvm::sort(
-            Successors.begin(), Successors.end(), [=](NodePtr A, NodePtr B) {
-              return SuccOrder->find(A)->second < SuccOrder->find(B)->second;
-            });
-
-      for (const NodePtr Succ : Successors) {
+      for (const NodePtr Succ : getChildren<Direction>(BB, BatchUpdates)) {
         const auto SIT = NodeToInfo.find(Succ);
         // Don't visit nodes more than once but remember to collect
         // ReverseChildren.
@@ -385,34 +372,6 @@ struct SemiNCAInfo {
     // nodes.
     if (Total + 1 != Num) {
       HasNonTrivialRoots = true;
-
-      // SuccOrder is the order of blocks in the function. It is needed to make
-      // the calculation of the FurthestAway node and the whole PostDomTree
-      // immune to swap successors transformation (e.g. canonicalizing branch
-      // predicates). SuccOrder is initialized lazily only for successors of
-      // reverse unreachable nodes.
-      Optional<NodeOrderMap> SuccOrder;
-      auto InitSuccOrderOnce = [&]() {
-        SuccOrder = NodeOrderMap();
-        for (const auto Node : nodes(DT.Parent))
-          if (SNCA.NodeToInfo.count(Node) == 0)
-            for (const auto Succ : getChildren<false>(Node, SNCA.BatchUpdates))
-              SuccOrder->try_emplace(Succ, 0);
-
-        // Add mapping for all entries of SuccOrder.
-        unsigned NodeNum = 0;
-        for (const auto Node : nodes(DT.Parent)) {
-          ++NodeNum;
-          auto Order = SuccOrder->find(Node);
-          if (Order != SuccOrder->end()) {
-            assert(Order->second == 0);
-            Order->second = NodeNum;
-            LLVM_DEBUG(dbgs() << "\t\t\tSuccOrder " << NodeNum << ": "
-                              << Node->getName() << "\n");
-          }
-        }
-      };
-
       // Make another DFS pass over all other nodes to find the
       // reverse-unreachable blocks, and find the furthest paths we'll be able
       // to make.
@@ -437,12 +396,7 @@ struct SemiNCAInfo {
           // expensive and does not always lead to a minimal set of roots.
           LLVM_DEBUG(dbgs() << "\t\t\tRunning forward DFS\n");
 
-          if (!SuccOrder)
-            InitSuccOrderOnce();
-          assert(SuccOrder);
-
-          const unsigned NewNum =
-              SNCA.runDFS<true>(I, Num, AlwaysDescend, Num, &*SuccOrder);
+          const unsigned NewNum = SNCA.runDFS<true>(I, Num, AlwaysDescend, Num);
           const NodePtr FurthestAway = SNCA.NumToNode[NewNum];
           LLVM_DEBUG(dbgs() << "\t\t\tFound a new furthest away node "
                             << "(non-trivial root): "

diff --git a/llvm/test/Transforms/InstCombine/infinite-loop-postdom.ll b/llvm/test/Transforms/InstCombine/infinite-loop-postdom.ll
deleted file mode 100644 (file)
index a6ce1fa..0000000
--- a/llvm/test/Transforms/InstCombine/infinite-loop-postdom.ll
+++ /dev/null
@@ -1,222 +0,0 @@
-; RUN: opt %s -disable-output -branch-prob -instcombine -block-freq -verify-dom-info\r
-; RUN: opt %s -postdomtree -analyze | FileCheck --check-prefixes=CHECK-POSTDOM %s\r
-; RUN: opt %s -passes='print<postdomtree>' 2>&1 | FileCheck --check-prefixes=CHECK-POSTDOM %s\r
-\r
-; Demonstrate that Predicate Canonicalization (InstCombine) does not invalidate PostDomTree\r
-; if the basic block is post-dom unreachable.\r
-\r
-define void @test1(i24 %a, i24 %b) {\r
-entry:\r
-  br label %LOOP\r
-\r
-LOOP:\r
-  %f = icmp uge i24 %a, %b\r
-  br i1 %f, label %B1, label %B2\r
-\r
-B1:\r
-  %x = add i24 %a, %b\r
-  br label %B2\r
-\r
-B2:\r
-  br label %LOOP\r
-}\r
-\r
-; The same as @test1 except the LOOP condition canonicalized (as by instcombine).\r
-define void @test1-canonicalized(i24 %a, i24 %b) {\r
-entry:\r
-  br label %LOOP\r
-\r
-LOOP:\r
-  %f.not = icmp ult i24 %a, %b\r
-  br i1 %f.not, label %B2, label %B1\r
-\r
-B1:\r
-  %x = add i24 %a, %b\r
-  br label %B2\r
-\r
-B2:\r
-  br label %LOOP\r
-}\r
-\r
-; The same as @test1 but different order of B1 and B2 in the function.\r
-; The different order makes PostDomTree different in presense of postdom\r
-; unreachable blocks.\r
-define void @test2(i24 %a, i24 %b) {\r
-entry:\r
-  br label %LOOP\r
-\r
-LOOP:\r
-  %f = icmp uge i24 %a, %b\r
-  br i1 %f, label %B1, label %B2\r
-\r
-B2:\r
-  br label %LOOP\r
-\r
-B1:\r
-  %x = add i24 %a, %b\r
-  br label %B2\r
-}\r
-\r
-; The same as @test2 except the LOOP condition canonicalized (as by instcombine).\r
-define void @test2-canonicalized(i24 %a, i24 %b) {\r
-entry:\r
-  br label %LOOP\r
-\r
-LOOP:\r
-  %f.not = icmp ult i24 %a, %b\r
-  br i1 %f.not, label %B2, label %B1\r
-\r
-B2:\r
-  br label %LOOP\r
-\r
-B1:\r
-  %x = add i24 %a, %b\r
-  br label %B2\r
-}\r
-\r
-; Two reverse unreachable subgraphs with RU1* and RU2* basic blocks respectively.\r
-define void @test3(i24 %a, i24 %b, i32 %flag) {\r
-entry:\r
-  switch i32 %flag, label %EXIT [\r
-    i32 1, label %RU1\r
-    i32 2, label %RU2\r
-    i32 3, label %RU2_B1\r
-  ]\r
-\r
-RU1:\r
-  %f = icmp uge i24 %a, %b\r
-  br label %RU1_LOOP\r
-\r
-RU1_LOOP:\r
-  br i1 %f, label %RU1_B1, label %RU1_B2\r
-\r
-RU1_B1:\r
-  %x = add i24 %a, %b\r
-  br label %RU1_B2\r
-\r
-RU1_B2:\r
-  br label %RU1_LOOP\r
-\r
-RU2:\r
-  %f2 = icmp uge i24 %a, %b\r
-  br i1 %f2, label %RU2_B1, label %RU2_B2\r
-\r
-RU2_B1:\r
-  br label %RU2_B2\r
-\r
-RU2_B2:\r
-  br label %RU2_B1\r
-\r
-EXIT:\r
-  ret void\r
-}\r
-\r
-; The same as @test3 except the icmp conditions are canonicalized (as by instcombine).\r
-define void @test3-canonicalized(i24 %a, i24 %b, i32 %flag) {\r
-entry:\r
-  switch i32 %flag, label %EXIT [\r
-    i32 1, label %RU1\r
-    i32 2, label %RU2\r
-    i32 3, label %RU2_B1\r
-  ]\r
-\r
-RU1:\r
-  %f.not = icmp ult i24 %a, %b\r
-  br label %RU1_LOOP\r
-\r
-RU1_LOOP:\r
-  br i1 %f.not, label %RU1_B2, label %RU1_B1\r
-\r
-RU1_B1:\r
-  %x = add i24 %a, %b\r
-  br label %RU1_B2\r
-\r
-RU1_B2:\r
-  br label %RU1_LOOP\r
-\r
-RU2:\r
-  %f2.not = icmp ult i24 %a, %b\r
-  br i1 %f2.not, label %RU2_B2, label %RU2_B1\r
-\r
-RU2_B1:\r
-  br label %RU2_B2\r
-\r
-RU2_B2:\r
-  br label %RU2_B1\r
-\r
-EXIT:\r
-  ret void\r
-}\r
-\r
-; PostDomTrees of @test1(), @test2() and @test3() are different.\r
-; PostDomTrees of @testX() and @testX-canonicalize() are the same.\r
-\r
-; CHECK-POSTDOM-LABEL: test1\r
-; CHECK-POSTDOM-NEXT: =============================--------------------------------\r
-; CHECK-POSTDOM-NEXT: Inorder PostDominator Tree: DFSNumbers invalid: 0 slow queries.\r
-; CHECK-POSTDOM-NEXT:   [1]  <<exit node>>\r
-; CHECK-POSTDOM-NEXT:     [2] %B1\r
-; CHECK-POSTDOM-NEXT:       [3] %LOOP\r
-; CHECK-POSTDOM-NEXT:         [4] %entry\r
-; CHECK-POSTDOM-NEXT:         [4] %B2\r
-; CHECK-POSTDOM-NEXT: Roots: %B1\r
-\r
-; CHECK-POSTDOM-LABEL: test1-canonicalized\r
-; CHECK-POSTDOM-NEXT: =============================--------------------------------\r
-; CHECK-POSTDOM-NEXT: Inorder PostDominator Tree: DFSNumbers invalid: 0 slow queries.\r
-; CHECK-POSTDOM-NEXT:   [1]  <<exit node>>\r
-; CHECK-POSTDOM-NEXT:     [2] %B1\r
-; CHECK-POSTDOM-NEXT:       [3] %LOOP\r
-; CHECK-POSTDOM-NEXT:         [4] %entry\r
-; CHECK-POSTDOM-NEXT:         [4] %B2\r
-; CHECK-POSTDOM-NEXT: Roots: %B1
-
-; CHECK-POSTDOM-LABEL: test2
-; CHECK-POSTDOM-NEXT: =============================--------------------------------
-; CHECK-POSTDOM-NEXT: Inorder PostDominator Tree: DFSNumbers invalid: 0 slow queries.
-; CHECK-POSTDOM-NEXT:   [1]  <<exit node>>
-; CHECK-POSTDOM-NEXT:     [2] %B2
-; CHECK-POSTDOM-NEXT:       [3] %LOOP
-; CHECK-POSTDOM-NEXT:         [4] %entry
-; CHECK-POSTDOM-NEXT:       [3] %B1
-; CHECK-POSTDOM-NEXT: Roots: %B2
-
-; CHECK-POSTDOM-LABEL: test2-canonicalized
-; CHECK-POSTDOM-NEXT: =============================--------------------------------
-; CHECK-POSTDOM-NEXT: Inorder PostDominator Tree: DFSNumbers invalid: 0 slow queries.
-; CHECK-POSTDOM-NEXT:   [1]  <<exit node>>
-; CHECK-POSTDOM-NEXT:     [2] %B2
-; CHECK-POSTDOM-NEXT:       [3] %LOOP
-; CHECK-POSTDOM-NEXT:         [4] %entry
-; CHECK-POSTDOM-NEXT:       [3] %B1
-; CHECK-POSTDOM-NEXT: Roots: %B2
-
-; CHECK-POSTDOM-LABEL: test3
-; CHECK-POSTDOM-NEXT:=============================--------------------------------
-; CHECK-POSTDOM-NEXT:Inorder PostDominator Tree: DFSNumbers invalid: 0 slow queries.
-; CHECK-POSTDOM-NEXT:  [1]  <<exit node>>
-; CHECK-POSTDOM-NEXT:    [2] %EXIT
-; CHECK-POSTDOM-NEXT:    [2] %entry
-; CHECK-POSTDOM-NEXT:    [2] %RU1_B1
-; CHECK-POSTDOM-NEXT:      [3] %RU1_LOOP
-; CHECK-POSTDOM-NEXT:        [4] %RU1
-; CHECK-POSTDOM-NEXT:        [4] %RU1_B2
-; CHECK-POSTDOM-NEXT:    [2] %RU2_B1
-; CHECK-POSTDOM-NEXT:      [3] %RU2
-; CHECK-POSTDOM-NEXT:      [3] %RU2_B2
-; CHECK-POSTDOM-NEXT:Roots: %EXIT %RU1_B1 %RU2_B1
-
-; CHECK-POSTDOM-LABEL: test3-canonicalized
-; CHECK-POSTDOM-NEXT:=============================--------------------------------
-; CHECK-POSTDOM-NEXT:Inorder PostDominator Tree: DFSNumbers invalid: 0 slow queries.
-; CHECK-POSTDOM-NEXT:  [1]  <<exit node>>
-; CHECK-POSTDOM-NEXT:    [2] %EXIT
-; CHECK-POSTDOM-NEXT:    [2] %entry
-; CHECK-POSTDOM-NEXT:    [2] %RU1_B1
-; CHECK-POSTDOM-NEXT:      [3] %RU1_LOOP
-; CHECK-POSTDOM-NEXT:        [4] %RU1
-; CHECK-POSTDOM-NEXT:        [4] %RU1_B2
-; CHECK-POSTDOM-NEXT:    [2] %RU2_B1
-; CHECK-POSTDOM-NEXT:      [3] %RU2
-; CHECK-POSTDOM-NEXT:      [3] %RU2_B2
-; CHECK-POSTDOM-NEXT:Roots: %EXIT %RU1_B1 %RU2_B1