[llvm] [DomTree] Remove support for unnumbered graphs (PR #212026)
via llvm-commits
llvm-commits at lists.llvm.org
Sat Jul 25 07:46:49 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-support
Author: Alexis Engelke (aengelke)
<details>
<summary>Changes</summary>
After VPlan, all our in-tree users of the dominator tree have numbered
nodes. Remove the support for unnumbered graphs.
Also slightly simplify the numbering for pre-dominator trees to avoid an
extra +1 on every node number.
---
Full diff: https://github.com/llvm/llvm-project/pull/212026.diff
2 Files Affected:
- (modified) llvm/include/llvm/Support/GenericDomTree.h (+28-53)
- (modified) llvm/include/llvm/Support/GenericDomTreeConstruction.h (+9-20)
``````````diff
diff --git a/llvm/include/llvm/Support/GenericDomTree.h b/llvm/include/llvm/Support/GenericDomTree.h
index 45bc6269177c7..905fbfa29c2cd 100644
--- a/llvm/include/llvm/Support/GenericDomTree.h
+++ b/llvm/include/llvm/Support/GenericDomTree.h
@@ -272,6 +272,8 @@ template <typename NodeT> struct DomTreeNodeTraits {
template <typename NodeT, bool IsPostDom>
class DominatorTreeBase {
public:
+ static_assert(GraphHasNodeNumbers<NodeT *>,
+ "DominatorTreeBase requires graphs with numbered nodes");
static_assert(std::is_pointer_v<typename GraphTraits<NodeT *>::NodeRef>,
"Currently DominatorTreeBase supports only pointer nodes");
using NodeTrait = DomTreeNodeTraits<NodeT>;
@@ -296,11 +298,6 @@ class DominatorTreeBase {
using DomTreeNodeStorageTy = SmallVector<DomTreeNodeBase<NodeT> *>;
DomTreeNodeStorageTy DomTreeNodes;
- // For graphs where blocks don't have numbers, create a numbering here.
- // TODO: use an empty struct with [[no_unique_address]] in C++20.
- std::conditional_t<!GraphHasNodeNumbers<NodeT *>,
- DenseMap<const NodeT *, unsigned>, std::tuple<>>
- NodeNumberMap;
DomTreeNodeBase<NodeT> *RootNode = nullptr;
ParentPtr Parent = nullptr;
@@ -392,41 +389,27 @@ class DominatorTreeBase {
});
}
- std::optional<unsigned> getNodeIndex(const NodeT *BB) const {
- if constexpr (GraphHasNodeNumbers<NodeT *>) {
- assert(BlockNumberEpoch ==
- GraphTraits<ParentPtr>::getNumberEpoch(Parent) &&
- "dominator tree used with outdated block numbers");
- if constexpr (IsPostDom) {
- if (!BB)
- return 0; // BB may be nullptr for post-dominator tree, map to 0.
- } else
- assert(BB && "dominator tree block must be non-null");
- return GraphTraits<const NodeT *>::getNumber(BB) + 1;
- } else {
- if (auto It = NodeNumberMap.find(BB); It != NodeNumberMap.end())
- return It->second;
- return std::nullopt;
- }
+ unsigned getNodeIndex(const NodeT *BB) const {
+ assert(BlockNumberEpoch ==
+ GraphTraits<ParentPtr>::getNumberEpoch(Parent) &&
+ "dominator tree used with outdated block numbers");
+ if constexpr (IsPostDom) {
+ if (!BB)
+ return 0; // BB may be nullptr for post-dominator tree, map to 0.
+ } else
+ assert(BB && "dominator tree block must be non-null");
+ return GraphTraits<const NodeT *>::getNumber(BB) + IsPostDom;
}
unsigned getNodeIndexForInsert(const NodeT *BB) {
- if constexpr (GraphHasNodeNumbers<NodeT *>) {
- // getNodeIndex will never fail if nodes have getNumber().
- unsigned Idx = *getNodeIndex(BB);
- if (Idx >= DomTreeNodes.size()) {
- unsigned Max = GraphTraits<ParentPtr>::getMaxNumber(Parent);
- DomTreeNodes.resize(Max > Idx + 1 ? Max : Idx + 1);
- }
- return Idx;
- } else {
- // We might already have a number stored for BB.
- unsigned Idx =
- NodeNumberMap.try_emplace(BB, DomTreeNodes.size()).first->second;
- if (Idx >= DomTreeNodes.size())
- DomTreeNodes.resize(Idx + 1);
- return Idx;
+ unsigned Idx = getNodeIndex(BB);
+ if (Idx >= DomTreeNodes.size()) {
+ // Add 1 for post-dominator trees, 0 is nullptr block.
+ unsigned Max = GraphTraits<ParentPtr>::getMaxNumber(Parent) + IsPostDom;
+ assert(Idx < Max && "getMaxNumber returned too small value");
+ DomTreeNodes.resize(Max + IsPostDom);
}
+ return Idx;
}
public:
@@ -437,8 +420,8 @@ class DominatorTreeBase {
DomTreeNodeBase<NodeT> *getNode(const NodeT *BB) const {
assert((!BB || Parent == NodeTrait::getParent(const_cast<NodeT *>(BB))) &&
"cannot get DomTreeNode of block with different parent");
- if (auto Idx = getNodeIndex(BB); Idx && *Idx < DomTreeNodes.size())
- return DomTreeNodes[*Idx];
+ if (unsigned Idx = getNodeIndex(BB); Idx < DomTreeNodes.size())
+ return DomTreeNodes[Idx];
return nullptr;
}
@@ -771,10 +754,9 @@ class DominatorTreeBase {
/// dominate any other blocks. Removes node from its immediate dominator's
/// children list. Deletes dominator node associated with basic block BB.
void eraseNode(NodeT *BB) {
- std::optional<unsigned> IdxOpt = getNodeIndex(BB);
- assert(IdxOpt && DomTreeNodes[*IdxOpt] &&
- "Removing node that isn't in dominator tree.");
- DomTreeNodeBase<NodeT> *Node = DomTreeNodes[*IdxOpt];
+ unsigned Idx = getNodeIndex(BB);
+ DomTreeNodeBase<NodeT> *Node = DomTreeNodes[Idx];
+ assert(Node && "Removing node that isn't in dominator tree.");
assert(Node->isLeaf() && "Node is not a leaf node.");
DFSInfoValid = false;
@@ -783,9 +765,7 @@ class DominatorTreeBase {
if (DomTreeNodeBase<NodeT> *IDom = Node->getIDom())
IDom->removeChild(Node);
- DomTreeNodes[*IdxOpt] = nullptr;
- if constexpr (!GraphHasNodeNumbers<NodeT *>)
- NodeNumberMap.erase(BB);
+ DomTreeNodes[Idx] = nullptr;
if (!IsPostDom) return;
@@ -878,9 +858,7 @@ class DominatorTreeBase {
private:
void updateBlockNumberEpoch() {
- // Nothing to do for graphs that don't number their blocks.
- if constexpr (GraphHasNodeNumbers<NodeT *>)
- BlockNumberEpoch = GraphTraits<ParentPtr>::getNumberEpoch(Parent);
+ BlockNumberEpoch = GraphTraits<ParentPtr>::getNumberEpoch(Parent);
}
public:
@@ -898,8 +876,7 @@ class DominatorTreeBase {
}
/// Update dominator tree after renumbering blocks.
- template <typename T = NodeT>
- std::enable_if_t<GraphHasNodeNumbers<T *>, void> updateBlockNumbers() {
+ void updateBlockNumbers() {
updateBlockNumberEpoch();
unsigned MaxNumber = GraphTraits<ParentPtr>::getMaxNumber(Parent);
@@ -908,7 +885,7 @@ class DominatorTreeBase {
for (auto &Node : DomTreeNodes) {
if (!Node)
continue;
- unsigned Idx = *getNodeIndex(Node->getBlock());
+ unsigned Idx = getNodeIndex(Node->getBlock());
// getMaxNumber is not necessarily supported
if (Idx >= NewVector.size())
NewVector.resize(Idx + 1);
@@ -937,8 +914,6 @@ class DominatorTreeBase {
void reset() {
DomTreeNodes.clear();
- if constexpr (!GraphHasNodeNumbers<NodeT *>)
- NodeNumberMap.clear();
Roots.clear();
RootNode = nullptr;
Parent = nullptr;
diff --git a/llvm/include/llvm/Support/GenericDomTreeConstruction.h b/llvm/include/llvm/Support/GenericDomTreeConstruction.h
index 05504571aa246..56fb396494307 100644
--- a/llvm/include/llvm/Support/GenericDomTreeConstruction.h
+++ b/llvm/include/llvm/Support/GenericDomTreeConstruction.h
@@ -77,11 +77,7 @@ template <typename DomTreeT> struct SemiNCAInfo {
// Map a 0-based DFS number to the node. 0 is the DFS root, or the virtual
// root for postdominators.
SmallVector<NodePtr, 32> NumToNode;
- // If blocks have numbers (e.g., BasicBlock, MachineBasicBlock), store node
- // infos in a vector. Otherwise, store them in a map.
- std::conditional_t<GraphHasNodeNumbers<NodePtr>, SmallVector<InfoRec, 32>,
- DenseMap<NodePtr, InfoRec>>
- NodeInfos;
+ SmallVector<InfoRec, 32> NodeInfos;
/// Reverse children of nodes; pairs of (DFSNum (predecessor), next-or-zero);
/// forms a linked list in this vector.
@@ -108,19 +104,14 @@ template <typename DomTreeT> struct SemiNCAInfo {
// If BUI is a nullptr, then there's no batch update in progress.
SemiNCAInfo(const DomTreeT &DT, BatchUpdatePtr BUI) : BatchUpdates(BUI) {
- if constexpr (GraphHasNodeNumbers<NodePtr>) {
- unsigned MaxNodeNumber =
- GraphTraits<typename DomTreeT::ParentPtr>::getMaxNumber(DT.Parent);
- NodeInfos.resize(MaxNodeNumber + 1); // nullptr block is zero.
- }
+ unsigned MaxNodeNumber =
+ GraphTraits<typename DomTreeT::ParentPtr>::getMaxNumber(DT.Parent);
+ NodeInfos.resize(MaxNodeNumber + IsPostDom); // post-dom null block is zero.
}
void clear() {
NumToNode.clear();
- if constexpr (GraphHasNodeNumbers<NodePtr>)
- NodeInfos.assign(NodeInfos.size(), InfoRec{});
- else
- NodeInfos.clear();
+ NodeInfos.assign(NodeInfos.size(), InfoRec{});
ReverseChildren.clear();
// Don't reset the pointer to BatchUpdateInfo here -- if there's an update
// in progress, we need this information to continue it.
@@ -143,12 +134,10 @@ template <typename DomTreeT> struct SemiNCAInfo {
}
InfoRec &getNodeInfo(NodePtr BB) {
- if constexpr (GraphHasNodeNumbers<NodePtr>) {
- unsigned Idx = BB ? GraphTraits<NodePtr>::getNumber(BB) + 1 : 0;
- return NodeInfos[Idx];
- } else {
- return NodeInfos[BB];
- }
+ // For post-dominator trees, index 0 is the null block.
+ if constexpr (IsPostDom)
+ return NodeInfos[BB ? GraphTraits<NodePtr>::getNumber(BB) + 1 : 0];
+ return NodeInfos[GraphTraits<NodePtr>::getNumber(BB)];
}
NodePtr getIDom(NodePtr BB) { return getNodeInfo(BB).IDom; }
``````````
</details>
https://github.com/llvm/llvm-project/pull/212026
More information about the llvm-commits
mailing list