[llvm] 933b800 - [Support][NFC] Use DomTreeBase methods in SemiNCA (#101059)

[via llvm-commits]

Author: Alexis Engelke
Date: 2024-07-30T12:00:59+02:00
New Revision: 933b80006af5801f7fe9327d160c04b12bb8d8b8

URL: https://github.com/llvm/llvm-project/commit/933b80006af5801f7fe9327d160c04b12bb8d8b8
DIFF: https://github.com/llvm/llvm-project/commit/933b80006af5801f7fe9327d160c04b12bb8d8b8.diff

LOG: [Support][NFC] Use DomTreeBase methods in SemiNCA (#101059)

Previously, there were two implementations with identical behavior to
erase a node from a dominator tree, one in the DomTreeBase and one in
SemiNCAInfo. Remove the latter, as it is completely redundant.

Also, use getNode() instead of a direct access into DomTreeNodes. This
will simplify replacing the data structure of DomTreeNodes later on.

While at it, also use swap+pop_back instead of erase when removing a
node from the children vector to avoid O(n) copy. This slightly changes
the order of the tree nodes after removal, but should have no impact.

Added: 
    

Modified: 
    llvm/include/llvm/Support/GenericDomTree.h
    llvm/include/llvm/Support/GenericDomTreeConstruction.h
    llvm/test/Transforms/SCCP/ipsccp-preserve-pdt.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Support/GenericDomTree.h b/llvm/include/llvm/Support/GenericDomTree.h
index a8e178d6461e0..6d78e96c033bd 100644
--- a/llvm/include/llvm/Support/GenericDomTree.h
+++ b/llvm/include/llvm/Support/GenericDomTree.h
@@ -695,7 +695,8 @@ class DominatorTreeBase {
       assert(I != IDom->Children.end() &&
              "Not in immediate dominator children set!");
       // I am no longer your child...
-      IDom->Children.erase(I);
+      std::swap(*I, IDom->Children.back());
+      IDom->Children.pop_back();
     }
 
     DomTreeNodes.erase(BB);

diff  --git a/llvm/include/llvm/Support/GenericDomTreeConstruction.h b/llvm/include/llvm/Support/GenericDomTreeConstruction.h
index 57cbe993d8739..bf244115d9be9 100644
--- a/llvm/include/llvm/Support/GenericDomTreeConstruction.h
+++ b/llvm/include/llvm/Support/GenericDomTreeConstruction.h
@@ -137,7 +137,7 @@ struct SemiNCAInfo {
     // immediate dominator.
     NodePtr IDom = getIDom(BB);
 
-    assert(IDom || DT.DomTreeNodes[nullptr]);
+    assert(IDom || DT.getNode(nullptr));
     TreeNodePtr IDomNode = getNodeForBlock(IDom, DT);
 
     // Add a new tree node for this NodeT, and link it as a child of
@@ -585,8 +585,8 @@ struct SemiNCAInfo {
     NodeToInfo[NumToNode[1]].IDom = AttachTo->getBlock();
     // Loop over all of the discovered blocks in the function...
     for (NodePtr W : llvm::drop_begin(NumToNode)) {
-      // Don't replace this with 'count', the insertion side effect is important
-      if (DT.DomTreeNodes[W]) continue;  // Haven't calculated this node yet?
+      if (DT.getNode(W))
+        continue; // Already calculated the node before
 
       NodePtr ImmDom = getIDom(W);
 
@@ -1078,10 +1078,9 @@ struct SemiNCAInfo {
     // before deleting their parent.
     for (unsigned i = LastDFSNum; i > 0; --i) {
       const NodePtr N = SNCA.NumToNode[i];
-      const TreeNodePtr TN = DT.getNode(N);
-      LLVM_DEBUG(dbgs() << "Erasing node " << BlockNamePrinter(TN) << "\n");
-
-      EraseNode(DT, TN);
+      LLVM_DEBUG(dbgs() << "Erasing node " << BlockNamePrinter(DT.getNode(N))
+                        << "\n");
+      DT.eraseNode(N);
     }
 
     // The affected subtree start at the To node -- there's no extra work to do.
@@ -1109,22 +1108,6 @@ struct SemiNCAInfo {
     SNCA.reattachExistingSubtree(DT, PrevIDom);
   }
 
-  // Removes leaf tree nodes from the dominator tree.
-  static void EraseNode(DomTreeT &DT, const TreeNodePtr TN) {
-    assert(TN);
-    assert(TN->getNumChildren() == 0 && "Not a tree leaf");
-
-    const TreeNodePtr IDom = TN->getIDom();
-    assert(IDom);
-
-    auto ChIt = llvm::find(IDom->Children, TN);
-    assert(ChIt != IDom->Children.end());
-    std::swap(*ChIt, IDom->Children.back());
-    IDom->Children.pop_back();
-
-    DT.DomTreeNodes.erase(TN->getBlock());
-  }
-
   //~~
   //===--------------------- DomTree Batch Updater --------------------------===
   //~~

diff  --git a/llvm/test/Transforms/SCCP/ipsccp-preserve-pdt.ll b/llvm/test/Transforms/SCCP/ipsccp-preserve-pdt.ll
index d605611ee0de6..ff57569d12788 100644
--- a/llvm/test/Transforms/SCCP/ipsccp-preserve-pdt.ll
+++ b/llvm/test/Transforms/SCCP/ipsccp-preserve-pdt.ll
@@ -17,11 +17,11 @@
 ; CHECK-NOT: <badref>
 ; CHECK: Inorder PostDominator Tree: DFSNumbers invalid: 0 slow queries.
 ; CHECK-NEXT:   [1]  <<exit node>> {4294967295,4294967295} [0]
+; CHECK-NEXT:     [2] %for.cond34 {4294967295,4294967295} [1]
+; CHECK-NEXT:       [3] %for.cond16 {4294967295,4294967295} [2]
 ; CHECK-NEXT:     [2] %for.body {4294967295,4294967295} [1]
 ; CHECK-NEXT:     [2] %if.end4 {4294967295,4294967295} [1]
 ; CHECK-NEXT:       [3] %entry {4294967295,4294967295} [2]
-; CHECK-NEXT:     [2] %for.cond34 {4294967295,4294967295} [1]
-; CHECK-NEXT:       [3] %for.cond16 {4294967295,4294967295} [2]
 ; CHECK-NEXT: Roots: %for.cond34 %for.body
 ; CHECK-NEXT: PostDominatorTree for function: bar
 ; CHECK-NOT: <badref>