[llvm] [DomTree] Prepend children instead of appending (PR #218178)
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Sat Aug 22 18:48:58 PDT 2026
https://github.com/MaskRay created https://github.com/llvm/llvm-project/pull/218178
Follow-up to #176409: `AppendPtr` exists only to keep a node's children
in the order they are added, and a lot of code depends on the order.
Implement an altrenative: in `attachNewSubtree`, create nodes in DFS
order and then link them in reverse. Trees built from scratch are
unchanged. Two tests that print an updated tree are adjusted.
>From ae8c08916450799ff22049875442aa793d4b9c75 Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Sat, 22 Aug 2026 17:46:10 -0700
Subject: [PATCH] [DomTree] Prepend children instead of appending
Follow-up to #176409: `AppendPtr` exists only to keep a node's children
in the order they are added, and a lot of code depends on the order.
Implement an altrenative: in `attachNewSubtree`, create nodes in DFS
order and then link them in reverse. Trees built from scratch are
unchanged. Two tests that print an updated tree are adjusted.
---
llvm/include/llvm/Support/GenericDomTree.h | 24 +++++++++----------
.../llvm/Support/GenericDomTreeConstruction.h | 15 +++++++-----
...i-lower-control-flow-preserve-dom-tree.mir | 4 ++--
.../JumpThreading/domtree-updates.ll | 10 ++++----
4 files changed, 28 insertions(+), 25 deletions(-)
diff --git a/llvm/include/llvm/Support/GenericDomTree.h b/llvm/include/llvm/Support/GenericDomTree.h
index cc06c6e486aaf..964e8412ce910 100644
--- a/llvm/include/llvm/Support/GenericDomTree.h
+++ b/llvm/include/llvm/Support/GenericDomTree.h
@@ -66,7 +66,6 @@ template <class NodeT> class DomTreeNodeBase {
unsigned Level;
DomTreeNodeBase *FirstChild = nullptr;
DomTreeNodeBase *Sibling = nullptr;
- DomTreeNodeBase **AppendPtr = &FirstChild;
mutable unsigned DFSNumIn = ~0;
mutable unsigned DFSNumOut = ~0;
@@ -155,9 +154,8 @@ template <class NodeT> class DomTreeNodeBase {
private:
void addChild(DomTreeNodeBase *C) {
assert(!C->Sibling && "cannot add child that already has siblings");
- assert(!*AppendPtr && "sibling of last child must be nullptr");
- *AppendPtr = C;
- AppendPtr = &C->Sibling;
+ C->Sibling = FirstChild;
+ FirstChild = C;
}
void removeChild(DomTreeNodeBase *C) {
@@ -166,13 +164,8 @@ template <class NodeT> class DomTreeNodeBase {
assert(*It != nullptr && "Not in immediate dominator children list!");
It = &(*It)->Sibling;
}
- assert(!*AppendPtr && "sibling of last child must be nullptr");
- assert(C->Sibling || AppendPtr == &C->Sibling);
*It = C->Sibling;
- if (C->Sibling)
- C->Sibling = nullptr;
- else
- AppendPtr = It;
+ C->Sibling = nullptr;
}
// Return true if this node is dominated by other. Use this only if DFS info
@@ -860,8 +853,9 @@ template <typename NodeT, bool IsPostDom> class DominatorTreeBase {
protected:
inline void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
- DomTreeNodeBase<NodeT> *createNode(NodeT *BB,
- DomTreeNodeBase<NodeT> *IDom = nullptr) {
+ /// Create a node for \p BB; the caller must link it with addChild.
+ DomTreeNodeBase<NodeT> *createNodeUnlinked(NodeT *BB,
+ DomTreeNodeBase<NodeT> *IDom) {
static_assert(std::is_trivially_destructible_v<DomTreeNodeBase<NodeT>>);
auto *Node = new (NodeAllocator) DomTreeNodeBase<NodeT>(BB, IDom);
unsigned Idx = getNodeIndex(BB);
@@ -872,6 +866,12 @@ template <typename NodeT, bool IsPostDom> class DominatorTreeBase {
DomTreeNodes.resize(Max);
}
DomTreeNodes[Idx] = Node;
+ return Node;
+ }
+
+ DomTreeNodeBase<NodeT> *createNode(NodeT *BB,
+ DomTreeNodeBase<NodeT> *IDom = nullptr) {
+ auto *Node = createNodeUnlinked(BB, IDom);
if (IDom)
IDom->addChild(Node);
return Node;
diff --git a/llvm/include/llvm/Support/GenericDomTreeConstruction.h b/llvm/include/llvm/Support/GenericDomTreeConstruction.h
index b2eb8c63dcfe7..96fb16e5d4bb3 100644
--- a/llvm/include/llvm/Support/GenericDomTreeConstruction.h
+++ b/llvm/include/llvm/Support/GenericDomTreeConstruction.h
@@ -592,15 +592,18 @@ template <typename DomTreeT> struct SemiNCAInfo {
}
// For each non-root node in a subtree, attach it to the immediate dominator.
+ // Link nodes in reverse: addChild prepends, so this leaves the children of
+ // each node in DFS order.
void attachNewSubtree(DomTreeT &DT) {
- for (unsigned Num = 1, E = NumToNode.size(); Num != E; ++Num) {
+ const unsigned E = NumToNode.size();
+ for (unsigned Num = 1; Num != E; ++Num) {
NodePtr W = NumToNode[Num];
assert(!DT.getNode(W) && "node was already attached");
-
- // Add a new tree node for this BasicBlock, and link it as a child of
- // IDomNode.
- auto IDomNode = DT.getNode(NumToNode[getNodeInfo(W).IDom]);
- DT.createNode(W, IDomNode);
+ DT.createNodeUnlinked(W, DT.getNode(NumToNode[getNodeInfo(W).IDom]));
+ }
+ for (unsigned Num = E; --Num;) {
+ const TreeNodePtr TN = DT.getNode(NumToNode[Num]);
+ TN->getIDom()->addChild(TN);
}
}
diff --git a/llvm/test/CodeGen/AMDGPU/si-lower-control-flow-preserve-dom-tree.mir b/llvm/test/CodeGen/AMDGPU/si-lower-control-flow-preserve-dom-tree.mir
index 4c984b80f3f1b..e0312a4d3e329 100644
--- a/llvm/test/CodeGen/AMDGPU/si-lower-control-flow-preserve-dom-tree.mir
+++ b/llvm/test/CodeGen/AMDGPU/si-lower-control-flow-preserve-dom-tree.mir
@@ -3,9 +3,9 @@
# CHECK: Inorder PostDominator Tree:
# CHECK-NEXT: [1] <<exit node>> {4294967295,4294967295} [0]
# CHECK-NEXT: [2] %bb.4 {4294967295,4294967295} [1]
-# CHECK-NEXT: [3] %bb.0 {4294967295,4294967295} [2]
-# CHECK-NEXT: [3] %bb.1 {4294967295,4294967295} [2]
# CHECK-NEXT: [3] %bb.2 {4294967295,4294967295} [2]
+# CHECK-NEXT: [3] %bb.1 {4294967295,4294967295} [2]
+# CHECK-NEXT: [3] %bb.0 {4294967295,4294967295} [2]
# CHECK: Inorder Dominator Tree:
# CHECK-NEXT: [1] %bb.0 {4294967295,4294967295} [0]
diff --git a/llvm/test/Transforms/JumpThreading/domtree-updates.ll b/llvm/test/Transforms/JumpThreading/domtree-updates.ll
index ec73dbe13864c..d2a2bf73b4813 100644
--- a/llvm/test/Transforms/JumpThreading/domtree-updates.ll
+++ b/llvm/test/Transforms/JumpThreading/domtree-updates.ll
@@ -18,17 +18,17 @@
; CHECK: Inorder Dominator Tree: DFSNumbers invalid: 0 slow queries.
; CHECK-NEXT: [1] %entry {4294967295,4294967295} [0]
; CHECK-NEXT: [2] %for.cond1 {4294967295,4294967295} [1]
+; CHECK-NEXT: [3] %for.inc19 {4294967295,4294967295} [2]
+; CHECK-NEXT: [3] %cleanup16.thread {4294967295,4294967295} [2]
; CHECK-NEXT: [3] %if.then {4294967295,4294967295} [2]
; CHECK-NEXT: [4] %for.cond5.preheader {4294967295,4294967295} [3]
-; CHECK-NEXT: [5] %for.body7 {4294967295,4294967295} [4]
-; CHECK-NEXT: [6] %for.inc {4294967295,4294967295} [5]
+; CHECK-NEXT: [5] %return {4294967295,4294967295} [4]
; CHECK-NEXT: [5] %cleanup {4294967295,4294967295} [4]
; CHECK-NEXT: [6] %cleanup16 {4294967295,4294967295} [5]
; CHECK-NEXT: [7] %unreachable {4294967295,4294967295} [6]
; CHECK-NEXT: [7] %for.end21 {4294967295,4294967295} [6]
-; CHECK-NEXT: [5] %return {4294967295,4294967295} [4]
-; CHECK-NEXT: [3] %cleanup16.thread {4294967295,4294967295} [2]
-; CHECK-NEXT: [3] %for.inc19 {4294967295,4294967295} [2]
+; CHECK-NEXT: [5] %for.body7 {4294967295,4294967295} [4]
+; CHECK-NEXT: [6] %for.inc {4294967295,4294967295} [5]
; CHECK-NEXT: [2] %infinite.loop {4294967295,4294967295} [1]
; CHECK-NEXT: Roots: %entry
More information about the llvm-commits
mailing list