Skip to content

[Support] Use block numbers for DomTree construction #101706

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 6, 2024
Merged

[Support] Use block numbers for DomTree construction #101706

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fb0dda6
[Support] Use block numbers for DomTree construction
aengelke Aug 1, 2024
a65756c
Use if-constexpr + move number check to GraphTraits
aengelke Aug 6, 2024
e484698
Use conditional_t to switch type of NodeInfos
aengelke Aug 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions llvm/include/llvm/ADT/GraphTraits.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#ifndef LLVM_ADT_GRAPHTRAITS_H
#define LLVM_ADT_GRAPHTRAITS_H

#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/iterator_range.h"

namespace llvm {
Expand Down Expand Up @@ -80,6 +81,16 @@ struct GraphTraits {
using NodeRef = typename GraphType::UnknownGraphTypeError;
};

namespace detail {
template <typename T>
using has_number_t = decltype(GraphTraits<T>::getNumber(std::declval<T>()));
} // namespace detail

/// Indicate whether a GraphTraits<NodeT>::getNumber() is supported.
template <typename NodeT>
constexpr bool GraphHasNodeNumbers =
is_detected<detail::has_number_t, NodeT>::value;
Comment on lines +90 to +92
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this has a single user in the codebase, is it worth putting it in GraphTraits instead of GenericDomTreeConstruction?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DomTreeBase also has has_number_t, which I'll deduplicate as a follow up. I expect that there'll be more users over time (GenericLoopInfo is something I have on my list).


// Inverse - This class is used as a little marker class to tell the graph
// iterator to iterate over the graph in a graph defined "Inverse" ordering.
// Not all graphs define an inverse ordering, and if they do, it depends on
Expand Down
58 changes: 37 additions & 21 deletions llvm/include/llvm/Support/GenericDomTreeConstruction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,11 @@ struct SemiNCAInfo {
// Number to node mapping is 1-based. Initialize the mapping to start with
// a dummy element.
SmallVector<NodePtr, 64> NumToNode = {nullptr};
DenseMap<NodePtr, InfoRec> NodeToInfo;
// 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, 64>,
DenseMap<NodePtr, InfoRec>>
NodeInfos;

using UpdateT = typename DomTreeT::UpdateType;
using UpdateKind = typename DomTreeT::UpdateKind;
Expand All @@ -99,7 +103,7 @@ struct SemiNCAInfo {

void clear() {
NumToNode = {nullptr}; // Restore to initial state with a dummy start node.
NodeToInfo.clear();
NodeInfos.clear();
// Don't reset the pointer to BatchUpdateInfo here -- if there's an update
// in progress, we need this information to continue it.
}
Expand All @@ -123,13 +127,25 @@ struct SemiNCAInfo {
return Res;
}

NodePtr getIDom(NodePtr BB) const {
auto InfoIt = NodeToInfo.find(BB);
if (InfoIt == NodeToInfo.end()) return nullptr;

return InfoIt->second.IDom;
InfoRec &getNodeInfo(NodePtr BB) {
if constexpr (GraphHasNodeNumbers<NodePtr>) {
unsigned Idx = BB ? GraphTraits<NodePtr>::getNumber(BB) + 1 : 0;
if (Idx >= NodeInfos.size()) {
unsigned Max = 0;
if (BB)
Max = GraphTraits<decltype(BB->getParent())>::getMaxNumber(
BB->getParent());
// Max might be zero, graphs might not support getMaxNumber().
NodeInfos.resize(Max ? Max + 1 : Idx + 1);
}
return NodeInfos[Idx];
} else {
return NodeInfos[BB];
}
}

NodePtr getIDom(NodePtr BB) { return getNodeInfo(BB).IDom; }

TreeNodePtr getNodeForBlock(NodePtr BB, DomTreeT &DT) {
if (TreeNodePtr Node = DT.getNode(BB)) return Node;

Expand Down Expand Up @@ -181,11 +197,11 @@ struct SemiNCAInfo {
const NodeOrderMap *SuccOrder = nullptr) {
assert(V);
SmallVector<std::pair<NodePtr, unsigned>, 64> WorkList = {{V, AttachToNum}};
NodeToInfo[V].Parent = AttachToNum;
getNodeInfo(V).Parent = AttachToNum;

while (!WorkList.empty()) {
const auto [BB, ParentNum] = WorkList.pop_back_val();
auto &BBInfo = NodeToInfo[BB];
auto &BBInfo = getNodeInfo(BB);
BBInfo.ReverseChildren.push_back(ParentNum);

// Visited nodes always have positive DFS numbers.
Expand Down Expand Up @@ -264,7 +280,7 @@ struct SemiNCAInfo {
// Initialize IDoms to spanning tree parents.
for (unsigned i = 1; i < NextDFSNum; ++i) {
const NodePtr V = NumToNode[i];
auto &VInfo = NodeToInfo[V];
auto &VInfo = getNodeInfo(V);
VInfo.IDom = NumToNode[VInfo.Parent];
NumToInfo.push_back(&VInfo);
}
Expand Down Expand Up @@ -292,7 +308,7 @@ struct SemiNCAInfo {
const unsigned SDomNum = NumToInfo[WInfo.Semi]->DFSNum;
NodePtr WIDomCandidate = WInfo.IDom;
while (true) {
auto &WIDomCandidateInfo = NodeToInfo.find(WIDomCandidate)->second;
auto &WIDomCandidateInfo = getNodeInfo(WIDomCandidate);
if (WIDomCandidateInfo.DFSNum <= SDomNum)
break;
WIDomCandidate = WIDomCandidateInfo.IDom;
Expand All @@ -311,7 +327,7 @@ struct SemiNCAInfo {
assert(IsPostDom && "Only postdominators have a virtual root");
assert(NumToNode.size() == 1 && "SNCAInfo must be freshly constructed");

auto &BBInfo = NodeToInfo[nullptr];
auto &BBInfo = getNodeInfo(nullptr);
BBInfo.DFSNum = BBInfo.Semi = BBInfo.Label = 1;

NumToNode.push_back(nullptr); // NumToNode[1] = nullptr;
Expand Down Expand Up @@ -393,7 +409,7 @@ struct SemiNCAInfo {
auto InitSuccOrderOnce = [&]() {
SuccOrder = NodeOrderMap();
for (const auto Node : nodes(DT.Parent))
if (SNCA.NodeToInfo.count(Node) == 0)
if (SNCA.getNodeInfo(Node).DFSNum == 0)
for (const auto Succ : getChildren<false>(Node, SNCA.BatchUpdates))
SuccOrder->try_emplace(Succ, 0);

Expand All @@ -417,7 +433,7 @@ struct SemiNCAInfo {
// unreachable node once, we may just visit it in two directions,
// depending on how lucky we get.
for (const NodePtr I : nodes(DT.Parent)) {
if (SNCA.NodeToInfo.count(I) == 0) {
if (SNCA.getNodeInfo(I).DFSNum == 0) {
LLVM_DEBUG(dbgs()
<< "\t\t\tVisiting node " << BlockNamePrinter(I) << "\n");
// Find the furthest away we can get by following successors, then
Expand Down Expand Up @@ -449,7 +465,7 @@ struct SemiNCAInfo {
const NodePtr N = SNCA.NumToNode[i];
LLVM_DEBUG(dbgs() << "\t\t\t\tRemoving DFS info for "
<< BlockNamePrinter(N) << "\n");
SNCA.NodeToInfo.erase(N);
SNCA.getNodeInfo(N) = {};
SNCA.NumToNode.pop_back();
}
const unsigned PrevNum = Num;
Expand Down Expand Up @@ -582,7 +598,7 @@ struct SemiNCAInfo {

void attachNewSubtree(DomTreeT& DT, const TreeNodePtr AttachTo) {
// Attach the first unreachable block to AttachTo.
NodeToInfo[NumToNode[1]].IDom = AttachTo->getBlock();
getNodeInfo(NumToNode[1]).IDom = AttachTo->getBlock();
// Loop over all of the discovered blocks in the function...
for (NodePtr W : llvm::drop_begin(NumToNode)) {
if (DT.getNode(W))
Expand All @@ -600,11 +616,11 @@ struct SemiNCAInfo {
}

void reattachExistingSubtree(DomTreeT &DT, const TreeNodePtr AttachTo) {
NodeToInfo[NumToNode[1]].IDom = AttachTo->getBlock();
getNodeInfo(NumToNode[1]).IDom = AttachTo->getBlock();
for (const NodePtr N : llvm::drop_begin(NumToNode)) {
const TreeNodePtr TN = DT.getNode(N);
assert(TN);
const TreeNodePtr NewIDom = DT.getNode(NodeToInfo[N].IDom);
const TreeNodePtr NewIDom = DT.getNode(getNodeInfo(N).IDom);
TN->setIDom(NewIDom);
}
}
Expand Down Expand Up @@ -1237,7 +1253,7 @@ struct SemiNCAInfo {
// Virtual root has a corresponding virtual CFG node.
if (DT.isVirtualRoot(TN)) continue;

if (NodeToInfo.count(BB) == 0) {
if (getNodeInfo(BB).DFSNum == 0) {
errs() << "DomTree node " << BlockNamePrinter(BB)
<< " not found by DFS walk!\n";
errs().flush();
Expand Down Expand Up @@ -1445,7 +1461,7 @@ struct SemiNCAInfo {
});

for (TreeNodePtr Child : TN->children())
if (NodeToInfo.count(Child->getBlock()) != 0) {
if (getNodeInfo(Child->getBlock()).DFSNum != 0) {
errs() << "Child " << BlockNamePrinter(Child)
<< " reachable after its parent " << BlockNamePrinter(BB)
<< " is removed!\n";
Expand Down Expand Up @@ -1481,7 +1497,7 @@ struct SemiNCAInfo {
for (const TreeNodePtr S : TN->children()) {
if (S == N) continue;

if (NodeToInfo.count(S->getBlock()) == 0) {
if (getNodeInfo(S->getBlock()).DFSNum == 0) {
errs() << "Node " << BlockNamePrinter(S)
<< " not reachable when its sibling " << BlockNamePrinter(N)
<< " is removed!\n";
Expand Down
Loading