[llvm] [Suppprt][NFC] Use DomTreeBase methods in SemiNCA (PR #101059)

[Alexis Engelke via llvm-commits]

https://github.com/aengelke created https://github.com/llvm/llvm-project/pull/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.

>From a875e8d5d9ff13e6e4f3023aa805268a8c0600c8 Mon Sep 17 00:00:00 2001
From: Alexis Engelke <engelke at in.tum.de>
Date: Mon, 29 Jul 2024 09:27:12 +0000
Subject: [PATCH] [Suppprt][NFC] Use DomTreeBase methods in SemiNCA

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.
---
 llvm/include/llvm/Support/GenericDomTree.h    |  3 +-
 .../llvm/Support/GenericDomTreeConstruction.h | 28 ++++---------------
 .../Transforms/SCCP/ipsccp-preserve-pdt.ll    |  4 +--
 3 files changed, 10 insertions(+), 25 deletions(-)

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..c670d45ed6a3b 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
@@ -586,7 +586,8 @@ struct SemiNCAInfo {
     // 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; // Haven't calculated this node yet?
 
       NodePtr ImmDom = getIDom(W);
 
@@ -1078,10 +1079,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 +1109,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>