Skip to content
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

Jump to bottom

[DomTree] Reduce number of hash table lookups (NFC) #73097

Merged
merged 1 commit into from
Nov 27, 2023
Merged

[DomTree] Reduce number of hash table lookups (NFC) #73097

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 15 additions & 15 deletions llvm/include/llvm/Support/GenericDomTreeConstruction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ struct SemiNCAInfo {
unsigned DFSNum = 0;
unsigned Parent = 0;
unsigned Semi = 0;
NodePtr Label = nullptr;
unsigned Label = 0;
NodePtr IDom = nullptr;
SmallVector<NodePtr, 2> ReverseChildren;
};
Expand Down Expand Up @@ -189,8 +189,7 @@ struct SemiNCAInfo {

// Visited nodes always have positive DFS numbers.
if (BBInfo.DFSNum != 0) continue;
BBInfo.DFSNum = BBInfo.Semi = ++LastNum;
BBInfo.Label = BB;
BBInfo.DFSNum = BBInfo.Semi = BBInfo.Label = ++LastNum;
NumToNode.push_back(BB);

constexpr bool Direction = IsReverse != IsPostDom; // XOR.
Expand Down Expand Up @@ -237,8 +236,9 @@ struct SemiNCAInfo {
//
// For each vertex V, its Label points to the vertex with the minimal sdom(U)
// (Semi) in its path from V (included) to NodeToInfo[V].Parent (excluded).
NodePtr eval(NodePtr V, unsigned LastLinked,
SmallVectorImpl<InfoRec *> &Stack) {
unsigned eval(NodePtr V, unsigned LastLinked,
SmallVectorImpl<InfoRec *> &Stack,
ArrayRef<InfoRec *> NumToInfo) {
InfoRec *VInfo = &NodeToInfo[V];
if (VInfo->Parent < LastLinked)
return VInfo->Label;
Expand All @@ -247,17 +247,17 @@ struct SemiNCAInfo {
assert(Stack.empty());
do {
Stack.push_back(VInfo);
VInfo = &NodeToInfo[NumToNode[VInfo->Parent]];
VInfo = NumToInfo[VInfo->Parent];
} while (VInfo->Parent >= LastLinked);

// Path compression. Point each vertex's Parent to the root and update its
// Label if any of its ancestors (PInfo->Label) has a smaller Semi.
const InfoRec *PInfo = VInfo;
const InfoRec *PLabelInfo = &NodeToInfo[PInfo->Label];
const InfoRec *PLabelInfo = NumToInfo[PInfo->Label];
do {
VInfo = Stack.pop_back_val();
VInfo->Parent = PInfo->Parent;
const InfoRec *VLabelInfo = &NodeToInfo[VInfo->Label];
const InfoRec *VLabelInfo = NumToInfo[VInfo->Label];
if (PLabelInfo->Semi < VLabelInfo->Semi)
VInfo->Label = PInfo->Label;
else
Expand All @@ -270,18 +270,20 @@ struct SemiNCAInfo {
// This function requires DFS to be run before calling it.
void runSemiNCA(DomTreeT &DT, const unsigned MinLevel = 0) {
const unsigned NextDFSNum(NumToNode.size());
SmallVector<InfoRec *, 8> NumToInfo = {nullptr};
NumToInfo.reserve(NextDFSNum);
// Initialize IDoms to spanning tree parents.
for (unsigned i = 1; i < NextDFSNum; ++i) {
const NodePtr V = NumToNode[i];
auto &VInfo = NodeToInfo[V];
VInfo.IDom = NumToNode[VInfo.Parent];
NumToInfo.push_back(&VInfo);
}

// Step #1: Calculate the semidominators of all vertices.
SmallVector<InfoRec *, 32> EvalStack;
for (unsigned i = NextDFSNum - 1; i >= 2; --i) {
NodePtr W = NumToNode[i];
auto &WInfo = NodeToInfo[W];
auto &WInfo = *NumToInfo[i];

// Initialize the semi dominator to point to the parent node.
WInfo.Semi = WInfo.Parent;
Expand All @@ -294,7 +296,7 @@ struct SemiNCAInfo {
if (TN && TN->getLevel() < MinLevel)
continue;

unsigned SemiU = NodeToInfo[eval(N, i + 1, EvalStack)].Semi;
unsigned SemiU = NumToInfo[eval(N, i + 1, EvalStack, NumToInfo)]->Semi;
if (SemiU < WInfo.Semi) WInfo.Semi = SemiU;
}
}
Expand All @@ -304,8 +306,7 @@ struct SemiNCAInfo {
// Note that the parents were stored in IDoms and later got invalidated
// during path compression in Eval.
for (unsigned i = 2; i < NextDFSNum; ++i) {
const NodePtr W = NumToNode[i];
auto &WInfo = NodeToInfo[W];
auto &WInfo = *NumToInfo[i];
const unsigned SDomNum = NodeToInfo[NumToNode[WInfo.Semi]].DFSNum;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you're wondering why I didn't change this to NumToNode[WInfo.Semi] as well, the reason is that WInfo.Semi can be 0 for PDTs here. Because the PDT virtual root nullptr happens to be the same as the NumToNode sentinel, this ends up looking up the info for the virtual root.

I could easily work around this here, but I didn't do this because I think this indicates a (probably harmless) bug somewhere else, and Semi is supposed to be 1 (the virtual root) rather than 0 here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've put up a potential fix for this issue at #73148.

NodePtr WIDomCandidate = WInfo.IDom;
while (NodeToInfo[WIDomCandidate].DFSNum > SDomNum)
Expand All @@ -325,8 +326,7 @@ struct SemiNCAInfo {
assert(NumToNode.size() == 1 && "SNCAInfo must be freshly constructed");

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

NumToNode.push_back(nullptr); // NumToNode[1] = nullptr;
}
Expand Down