[llvm] r310171 - [LCG] Completely remove the parent set and leaf tracking for RefSCCs.
Chandler Carruth via llvm-commits
llvm-commits at lists.llvm.org
Sat Aug 5 00:37:00 PDT 2017
Author: chandlerc
Date: Sat Aug 5 00:37:00 2017
New Revision: 310171
URL: http://llvm.org/viewvc/llvm-project?rev=310171&view=rev
Log:
[LCG] Completely remove the parent set and leaf tracking for RefSCCs.
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.
Modified:
llvm/trunk/include/llvm/Analysis/LazyCallGraph.h
llvm/trunk/lib/Analysis/LazyCallGraph.cpp
Modified: llvm/trunk/include/llvm/Analysis/LazyCallGraph.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/LazyCallGraph.h?rev=310171&r1=310170&r2=310171&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/LazyCallGraph.h (original)
+++ llvm/trunk/include/llvm/Analysis/LazyCallGraph.h Sat Aug 5 00:37:00 2017
@@ -544,7 +544,6 @@ public:
friend class LazyCallGraph::Node;
LazyCallGraph *G;
- SmallPtrSet<RefSCC *, 1> Parents;
/// A postorder list of the inner SCCs.
SmallVector<SCC *, 4> SCCs;
@@ -557,7 +556,6 @@ public:
RefSCC(LazyCallGraph &G);
void clear() {
- Parents.clear();
SCCs.clear();
SCCIndices.clear();
}
@@ -624,13 +622,6 @@ public:
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
@@ -1142,11 +1133,6 @@ private:
/// 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.
@@ -1193,12 +1179,6 @@ private:
/// 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)
Modified: llvm/trunk/lib/Analysis/LazyCallGraph.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/LazyCallGraph.cpp?rev=310171&r1=310170&r2=310171&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/LazyCallGraph.cpp (original)
+++ llvm/trunk/lib/Analysis/LazyCallGraph.cpp Sat Aug 5 00:37:00 2017
@@ -175,7 +175,7 @@ LazyCallGraph::LazyCallGraph(Module &M,
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();
}
@@ -186,7 +186,6 @@ LazyCallGraph &LazyCallGraph::operator=(
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;
@@ -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
@@ -941,10 +924,6 @@ void LazyCallGraph::RefSCC::insertOutgoi
"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();
@@ -1048,12 +1027,6 @@ LazyCallGraph::RefSCC::insertIncomingRef
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
@@ -1061,16 +1034,8 @@ LazyCallGraph::RefSCC::insertIncomingRef
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
@@ -1116,9 +1081,6 @@ void LazyCallGraph::RefSCC::removeOutgoi
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.
@@ -1130,47 +1092,6 @@ void LazyCallGraph::RefSCC::removeOutgoi
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>
@@ -1313,10 +1234,7 @@ LazyCallGraph::RefSCC::removeInternalRef
}
// 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;
}
@@ -1418,13 +1336,6 @@ LazyCallGraph::RefSCC::removeInternalRef
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) {
@@ -1437,54 +1348,6 @@ LazyCallGraph::RefSCC::removeInternalRef
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)
RC->verify();
@@ -1511,9 +1374,6 @@ void LazyCallGraph::RefSCC::handleTrivia
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,
@@ -1671,16 +1531,6 @@ void LazyCallGraph::removeDeadFunction(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);
@@ -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.
@@ -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) {}
More information about the llvm-commits
mailing list