[llvm] [DomTree] Prepend children instead of appending (PR #218178)
via llvm-commits
llvm-commits at lists.llvm.org
Sat Aug 22 18:49:38 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
Author: Fangrui Song (MaskRay)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/218178.diff
4 Files Affected:
- (modified) llvm/include/llvm/Support/GenericDomTree.h (+12-12)
- (modified) llvm/include/llvm/Support/GenericDomTreeConstruction.h (+9-6)
- (modified) llvm/test/CodeGen/AMDGPU/si-lower-control-flow-preserve-dom-tree.mir (+2-2)
- (modified) llvm/test/Transforms/JumpThreading/domtree-updates.ll (+5-5)
``````````diff
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
``````````
</details>
https://github.com/llvm/llvm-project/pull/218178
More information about the llvm-commits
mailing list