Skip to content

Commit

Permalink
[LCG] Completely remove the parent set and leaf tracking for RefSCCs.
Browse files Browse the repository at this point in the history 
After the previous series of patches, this is now trivial and deletes
a pretty astonishing amount of complexity. This has been a long time
coming, as the move toward a PO sequence of RefSCCs started eroding the
underlying use cases for this half of the data structure.

Among the biggest advantages here is that now there aren't two
independent data structures that need to stay in sync.

Some of my profiling has also indicated that updating the parent sets
was among the most expensive parts of the lazy call graph. Eliminating
it whole sale is likely to be a nice win in terms of compile time.

Last but not least, I had discussed with some folks previously keeping
it around for asserts and other correctness checking, but once the
fundamentals of the parent and child checking were implemented without
the parent sets their value in correctness checking was tiny and no
where near worth the cost of the complexity required to keep everything
up-to-date.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@310171 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
@chandlerc
chandlerc committed Aug 5, 2017
1 parent 8d16ccb commit 3ae1e28
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 196 deletions.
20 changes: 0 additions & 20 deletions include/llvm/Analysis/LazyCallGraph.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,6 @@ class LazyCallGraph {
friend class LazyCallGraph::Node;

LazyCallGraph *G;
SmallPtrSet<RefSCC *, 1> Parents;

/// A postorder list of the inner SCCs.
SmallVector<SCC *, 4> SCCs;
Expand All @@ -557,7 +556,6 @@ class LazyCallGraph {
RefSCC(LazyCallGraph &G);

void clear() {
Parents.clear();
SCCs.clear();
SCCIndices.clear();
}
Expand Down Expand Up @@ -624,13 +622,6 @@ class LazyCallGraph {
return SCCs.begin() + SCCIndices.find(&C)->second;
}

parent_iterator parent_begin() const { return Parents.begin(); }
parent_iterator parent_end() const { return Parents.end(); }

iterator_range<parent_iterator> parents() const {
return make_range(parent_begin(), parent_end());
}

/// Test if this RefSCC is a parent of \a RC.
///
/// CAUTION: This method walks every edge in the \c RefSCC, it can be very
Expand Down Expand Up @@ -1142,11 +1133,6 @@ class LazyCallGraph {
/// RefSCCs.
DenseMap<RefSCC *, int> RefSCCIndices;

/// The leaf RefSCCs of the graph.
///
/// These are all of the RefSCCs which have no children.
SmallVector<RefSCC *, 4> LeafRefSCCs;

/// Defined functions that are also known library functions which the
/// optimizer can reason about and therefore might introduce calls to out of
/// thin air.
Expand Down Expand Up @@ -1193,12 +1179,6 @@ class LazyCallGraph {
/// Build the SCCs for a RefSCC out of a list of nodes.
void buildSCCs(RefSCC &RC, node_stack_range Nodes);

/// Connect a RefSCC into the larger graph.
///
/// This walks the edges to connect the RefSCC to its children's parent set,
/// and updates the root leaf list.
void connectRefSCC(RefSCC &RC);

/// Get the index of a RefSCC within the postorder traversal.
///
/// Requires that this RefSCC is a valid one in the (perhaps partial)
Expand Down
179 changes: 3 additions & 176 deletions lib/Analysis/LazyCallGraph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ LazyCallGraph::LazyCallGraph(Module &M, TargetLibraryInfo &TLI) {
LazyCallGraph::LazyCallGraph(LazyCallGraph &&G)
: BPA(std::move(G.BPA)), NodeMap(std::move(G.NodeMap)),
EntryEdges(std::move(G.EntryEdges)), SCCBPA(std::move(G.SCCBPA)),
SCCMap(std::move(G.SCCMap)), LeafRefSCCs(std::move(G.LeafRefSCCs)),
SCCMap(std::move(G.SCCMap)),
LibFunctions(std::move(G.LibFunctions)) {
updateGraphPtrs();
}
Expand All @@ -186,7 +186,6 @@ LazyCallGraph &LazyCallGraph::operator=(LazyCallGraph &&G) {
EntryEdges = std::move(G.EntryEdges);
SCCBPA = std::move(G.SCCBPA);
SCCMap = std::move(G.SCCMap);
LeafRefSCCs = std::move(G.LeafRefSCCs);
LibFunctions = std::move(G.LibFunctions);
updateGraphPtrs();
return *this;
Expand Down Expand Up @@ -313,24 +312,8 @@ void LazyCallGraph::RefSCC::verify() {
"Edge between SCCs violates post-order relationship.");
continue;
}
assert(TargetSCC.getOuterRefSCC().Parents.count(this) &&
"Edge to a RefSCC missing us in its parent set.");
}
}

// Check that our parents are actually parents.
for (RefSCC *ParentRC : Parents) {
assert(ParentRC != this && "Cannot be our own parent!");
auto HasConnectingEdge = [&] {
for (SCC &C : *ParentRC)
for (Node &N : C)
for (Edge &E : *N)
if (G->lookupRefSCC(E.getNode()) == this)
return true;
return false;
};
assert(HasConnectingEdge() && "No edge connects the parent to us!");
}
}
#endif

Expand Down Expand Up @@ -941,10 +924,6 @@ void LazyCallGraph::RefSCC::insertOutgoingEdge(Node &SourceN, Node &TargetN,
"Target must be a descendant of the Source.");
#endif

// The only change required is to add this SCC to the parent set of the
// callee.
TargetC.Parents.insert(this);

#ifndef NDEBUG
// Check that the RefSCC is still valid.
verify();
Expand Down Expand Up @@ -1048,29 +1027,15 @@ LazyCallGraph::RefSCC::insertIncomingRefEdge(Node &SourceN, Node &TargetN) {
assert(RC != this && "We're merging into the target RefSCC, so it "
"shouldn't be in the range.");

// Merge the parents which aren't part of the merge into the our parents.
for (RefSCC *ParentRC : RC->Parents)
if (!MergeSet.count(ParentRC))
Parents.insert(ParentRC);
RC->Parents.clear();

// Walk the inner SCCs to update their up-pointer and walk all the edges to
// update any parent sets.
// FIXME: We should try to find a way to avoid this (rather expensive) edge
// walk by updating the parent sets in some other manner.
for (SCC &InnerC : *RC) {
InnerC.OuterRefSCC = this;
SCCIndices[&InnerC] = SCCIndex++;
for (Node &N : InnerC) {
for (Node &N : InnerC)
G->SCCMap[&N] = &InnerC;
for (Edge &E : *N) {
RefSCC &ChildRC = *G->lookupRefSCC(E.getNode());
if (MergeSet.count(&ChildRC))
continue;
ChildRC.Parents.erase(RC);
ChildRC.Parents.insert(this);
}
}
}

// Now merge in the SCCs. We can actually move here so try to reuse storage
Expand Down Expand Up @@ -1116,9 +1081,6 @@ void LazyCallGraph::RefSCC::removeOutgoingEdge(Node &SourceN, Node &TargetN) {
RefSCC &TargetRC = *G->lookupRefSCC(TargetN);
assert(&TargetRC != this && "The target must not be a member of this RefSCC");

assert(!is_contained(G->LeafRefSCCs, this) &&
"Cannot have a leaf RefSCC source.");

#ifndef NDEBUG
// In a debug build, verify the RefSCC is valid to start with and when this
// routine finishes.
Expand All @@ -1130,47 +1092,6 @@ void LazyCallGraph::RefSCC::removeOutgoingEdge(Node &SourceN, Node &TargetN) {
bool Removed = SourceN->removeEdgeInternal(TargetN);
(void)Removed;
assert(Removed && "Target not in the edge set for this caller?");

bool HasOtherEdgeToChildRC = false;
bool HasOtherChildRC = false;
for (SCC *InnerC : SCCs) {
for (Node &N : *InnerC) {
for (Edge &E : *N) {
RefSCC &OtherChildRC = *G->lookupRefSCC(E.getNode());
if (&OtherChildRC == &TargetRC) {
HasOtherEdgeToChildRC = true;
break;
}
if (&OtherChildRC != this)
HasOtherChildRC = true;
}
if (HasOtherEdgeToChildRC)
break;
}
if (HasOtherEdgeToChildRC)
break;
}
// Because the SCCs form a DAG, deleting such an edge cannot change the set
// of SCCs in the graph. However, it may cut an edge of the SCC DAG, making
// the source SCC no longer connected to the target SCC. If so, we need to
// update the target SCC's map of its parents.
if (!HasOtherEdgeToChildRC) {
bool Removed = TargetRC.Parents.erase(this);
(void)Removed;
assert(Removed &&
"Did not find the source SCC in the target SCC's parent list!");

// It may orphan an SCC if it is the last edge reaching it, but that does
// not violate any invariants of the graph.
if (TargetRC.Parents.empty())
DEBUG(dbgs() << "LCG: Update removing " << SourceN.getFunction().getName()
<< " -> " << TargetN.getFunction().getName()
<< " edge orphaned the callee's SCC!\n");

// It may make the Source SCC a leaf SCC.
if (!HasOtherChildRC)
G->LeafRefSCCs.push_back(this);
}
}

SmallVector<LazyCallGraph::RefSCC *, 1>
Expand Down Expand Up @@ -1313,10 +1234,7 @@ LazyCallGraph::RefSCC::removeInternalRefEdge(Node &SourceN, Node &TargetN) {
}

// If this child isn't currently in this RefSCC, no need to process
// it. However, we do need to remove this RefSCC from its RefSCC's
// parent set.
RefSCC &ChildRC = *G->lookupRefSCC(ChildN);
ChildRC.Parents.erase(this);
// it.
++I;
continue;
}
Expand Down Expand Up @@ -1418,13 +1336,6 @@ LazyCallGraph::RefSCC::removeInternalRefEdge(Node &SourceN, Node &TargetN) {
C->OuterRefSCC = &RC;
}

// FIXME: We re-walk the edges in each RefSCC to establish whether it is
// a leaf and connect it to the rest of the graph's parents lists. This is
// really wasteful. We should instead do this during the DFS to avoid yet
// another edge walk.
for (RefSCC *RC : Result)
G->connectRefSCC(*RC);

// Now erase all but the root's SCCs.
SCCs.erase(remove_if(SCCs,
[&](SCC *C) {
Expand All @@ -1436,54 +1347,6 @@ LazyCallGraph::RefSCC::removeInternalRefEdge(Node &SourceN, Node &TargetN) {
for (int i = 0, Size = SCCs.size(); i < Size; ++i)
SCCIndices[SCCs[i]] = i;

#ifndef NDEBUG
// Now we need to reconnect the current (root) SCC to the graph. We do this
// manually because we can special case our leaf handling and detect errors.
bool IsLeaf = true;
#endif
for (SCC *C : SCCs)
for (Node &N : *C) {
for (Edge &E : *N) {
RefSCC &ChildRC = *G->lookupRefSCC(E.getNode());
if (&ChildRC == this)
continue;
ChildRC.Parents.insert(this);
#ifndef NDEBUG
IsLeaf = false;
#endif
}
}
#ifndef NDEBUG
if (!Result.empty())
assert(!IsLeaf && "This SCC cannot be a leaf as we have split out new "
"SCCs by removing this edge.");
if (none_of(G->LeafRefSCCs, [&](RefSCC *C) { return C == this; }))
assert(!IsLeaf && "This SCC cannot be a leaf as it already had child "
"SCCs before we removed this edge.");
#endif
// And connect both this RefSCC and all the new ones to the correct parents.
// The easiest way to do this is just to re-analyze the old parent set.
SmallVector<RefSCC *, 4> OldParents(Parents.begin(), Parents.end());
Parents.clear();
for (RefSCC *ParentRC : OldParents)
for (SCC &ParentC : *ParentRC)
for (Node &ParentN : ParentC)
for (Edge &E : *ParentN) {
RefSCC &RC = *G->lookupRefSCC(E.getNode());
if (&RC != ParentRC)
RC.Parents.insert(ParentRC);
}

// If this SCC stopped being a leaf through this edge removal, remove it from
// the leaf SCC list. Note that this DTRT in the case where this was never
// a leaf.
// FIXME: As LeafRefSCCs could be very large, we might want to not walk the
// entire list if this RefSCC wasn't a leaf before the edge removal.
if (!Result.empty())
G->LeafRefSCCs.erase(
std::remove(G->LeafRefSCCs.begin(), G->LeafRefSCCs.end(), this),
G->LeafRefSCCs.end());

#ifndef NDEBUG
// Verify all of the new RefSCCs.
for (RefSCC *RC : Result)
Expand Down Expand Up @@ -1511,9 +1374,6 @@ void LazyCallGraph::RefSCC::handleTrivialEdgeInsertion(Node &SourceN,
assert(TargetRC.isDescendantOf(*this) &&
"Target must be a descendant of the Source.");
#endif
// The only change required is to add this RefSCC to the parent set of the
// target. This is a set and so idempotent if the edge already existed.
TargetRC.Parents.insert(this);
}

void LazyCallGraph::RefSCC::insertTrivialCallEdge(Node &SourceN,
Expand Down Expand Up @@ -1671,16 +1531,6 @@ void LazyCallGraph::removeDeadFunction(Function &F) {
assert(C.size() == 1 && "Dead functions must be in a singular SCC");
assert(RC.size() == 1 && "Dead functions must be in a singular RefSCC");

// Now remove this RefSCC from any parents sets and the leaf list.
for (Edge &E : *N)
if (RefSCC *TargetRC = lookupRefSCC(E.getNode()))
TargetRC->Parents.erase(&RC);
// FIXME: This is a linear operation which could become hot and benefit from
// an index map.
auto LRI = find(LeafRefSCCs, &RC);
if (LRI != LeafRefSCCs.end())
LeafRefSCCs.erase(LRI);

auto RCIndexI = RefSCCIndices.find(&RC);
int RCIndex = RCIndexI->second;
PostOrderRefSCCs.erase(PostOrderRefSCCs.begin() + RCIndex);
Expand Down Expand Up @@ -1872,7 +1722,6 @@ void LazyCallGraph::buildRefSCCs() {
[this](node_stack_range Nodes) {
RefSCC *NewRC = createRefSCC(*this);
buildSCCs(*NewRC, Nodes);
connectRefSCC(*NewRC);

// Push the new node into the postorder list and remember its position
// in the index map.
Expand All @@ -1887,28 +1736,6 @@ void LazyCallGraph::buildRefSCCs() {
});
}

// FIXME: We should move callers of this to embed the parent linking and leaf
// tracking into their DFS in order to remove a full walk of all edges.
void LazyCallGraph::connectRefSCC(RefSCC &RC) {
// Walk all edges in the RefSCC (this remains linear as we only do this once
// when we build the RefSCC) to connect it to the parent sets of its
// children.
bool IsLeaf = true;
for (SCC &C : RC)
for (Node &N : C)
for (Edge &E : *N) {
RefSCC &ChildRC = *lookupRefSCC(E.getNode());
if (&ChildRC == &RC)
continue;
ChildRC.Parents.insert(&RC);
IsLeaf = false;
}

// For the SCCs where we find no child SCCs, add them to the leaf list.
if (IsLeaf)
LeafRefSCCs.push_back(&RC);
}

AnalysisKey LazyCallGraphAnalysis::Key;

LazyCallGraphPrinterPass::LazyCallGraphPrinterPass(raw_ostream &OS) : OS(OS) {}
Expand Down

0 comments on commit 3ae1e28

Please sign in to comment.