Skip to content

Commit 933b800

Browse files
authored
[Support][NFC] Use DomTreeBase methods in SemiNCA (llvm#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.
1 parent 2b2f4ae commit 933b800

File tree

3 files changed

+10
-26
lines changed

3 files changed

+10
-26
lines changed

llvm/include/llvm/Support/GenericDomTree.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -695,7 +695,8 @@ class DominatorTreeBase {
695695
assert(I != IDom->Children.end() &&
696696
"Not in immediate dominator children set!");
697697
// I am no longer your child...
698-
IDom->Children.erase(I);
698+
std::swap(*I, IDom->Children.back());
699+
IDom->Children.pop_back();
699700
}
700701

701702
DomTreeNodes.erase(BB);

llvm/include/llvm/Support/GenericDomTreeConstruction.h

Lines changed: 6 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ struct SemiNCAInfo {
137137
// immediate dominator.
138138
NodePtr IDom = getIDom(BB);
139139

140-
assert(IDom || DT.DomTreeNodes[nullptr]);
140+
assert(IDom || DT.getNode(nullptr));
141141
TreeNodePtr IDomNode = getNodeForBlock(IDom, DT);
142142

143143
// Add a new tree node for this NodeT, and link it as a child of
@@ -585,8 +585,8 @@ struct SemiNCAInfo {
585585
NodeToInfo[NumToNode[1]].IDom = AttachTo->getBlock();
586586
// Loop over all of the discovered blocks in the function...
587587
for (NodePtr W : llvm::drop_begin(NumToNode)) {
588-
// Don't replace this with 'count', the insertion side effect is important
589-
if (DT.DomTreeNodes[W]) continue; // Haven't calculated this node yet?
588+
if (DT.getNode(W))
589+
continue; // Already calculated the node before
590590

591591
NodePtr ImmDom = getIDom(W);
592592

@@ -1078,10 +1078,9 @@ struct SemiNCAInfo {
10781078
// before deleting their parent.
10791079
for (unsigned i = LastDFSNum; i > 0; --i) {
10801080
const NodePtr N = SNCA.NumToNode[i];
1081-
const TreeNodePtr TN = DT.getNode(N);
1082-
LLVM_DEBUG(dbgs() << "Erasing node " << BlockNamePrinter(TN) << "\n");
1083-
1084-
EraseNode(DT, TN);
1081+
LLVM_DEBUG(dbgs() << "Erasing node " << BlockNamePrinter(DT.getNode(N))
1082+
<< "\n");
1083+
DT.eraseNode(N);
10851084
}
10861085

10871086
// The affected subtree start at the To node -- there's no extra work to do.
@@ -1109,22 +1108,6 @@ struct SemiNCAInfo {
11091108
SNCA.reattachExistingSubtree(DT, PrevIDom);
11101109
}
11111110

1112-
// Removes leaf tree nodes from the dominator tree.
1113-
static void EraseNode(DomTreeT &DT, const TreeNodePtr TN) {
1114-
assert(TN);
1115-
assert(TN->getNumChildren() == 0 && "Not a tree leaf");
1116-
1117-
const TreeNodePtr IDom = TN->getIDom();
1118-
assert(IDom);
1119-
1120-
auto ChIt = llvm::find(IDom->Children, TN);
1121-
assert(ChIt != IDom->Children.end());
1122-
std::swap(*ChIt, IDom->Children.back());
1123-
IDom->Children.pop_back();
1124-
1125-
DT.DomTreeNodes.erase(TN->getBlock());
1126-
}
1127-
11281111
//~~
11291112
//===--------------------- DomTree Batch Updater --------------------------===
11301113
//~~

llvm/test/Transforms/SCCP/ipsccp-preserve-pdt.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@
1717
; CHECK-NOT: <badref>
1818
; CHECK: Inorder PostDominator Tree: DFSNumbers invalid: 0 slow queries.
1919
; CHECK-NEXT: [1] <<exit node>> {4294967295,4294967295} [0]
20+
; CHECK-NEXT: [2] %for.cond34 {4294967295,4294967295} [1]
21+
; CHECK-NEXT: [3] %for.cond16 {4294967295,4294967295} [2]
2022
; CHECK-NEXT: [2] %for.body {4294967295,4294967295} [1]
2123
; CHECK-NEXT: [2] %if.end4 {4294967295,4294967295} [1]
2224
; CHECK-NEXT: [3] %entry {4294967295,4294967295} [2]
23-
; CHECK-NEXT: [2] %for.cond34 {4294967295,4294967295} [1]
24-
; CHECK-NEXT: [3] %for.cond16 {4294967295,4294967295} [2]
2525
; CHECK-NEXT: Roots: %for.cond34 %for.body
2626
; CHECK-NEXT: PostDominatorTree for function: bar
2727
; CHECK-NOT: <badref>

0 commit comments

Comments
 (0)