[llvm] 468fa03 - Only verify LazyCallGraph under expensive checks
Arthur Eubanks via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 22 20:19:18 PST 2021
Author: Arthur Eubanks
Date: 2021-02-22T20:18:59-08:00
New Revision: 468fa037b2a15a40f17329a995d058dda6049d28
URL: https://github.com/llvm/llvm-project/commit/468fa037b2a15a40f17329a995d058dda6049d28
DIFF: https://github.com/llvm/llvm-project/commit/468fa037b2a15a40f17329a995d058dda6049d28.diff
LOG: Only verify LazyCallGraph under expensive checks
These verify calls are causing a lot of slowdown on some files, up to 8x.
The LazyCallGraph infra has been tested a lot over the years, so I'm fairly confident that we don't always need to run the verifys.
These verifies took >90% of total time in one of the compilations I looked at.
Reviewed By: thakis
Differential Revision: https://reviews.llvm.org/D97225
Added:
Modified:
llvm/lib/Analysis/CGSCCPassManager.cpp
llvm/lib/Analysis/LazyCallGraph.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Analysis/CGSCCPassManager.cpp b/llvm/lib/Analysis/CGSCCPassManager.cpp
index f43a2de2d4e0..0a05c3c875e0 100644
--- a/llvm/lib/Analysis/CGSCCPassManager.cpp
+++ b/llvm/lib/Analysis/CGSCCPassManager.cpp
@@ -980,8 +980,10 @@ static LazyCallGraph::SCC &updateCGAndAnalysisManagerForPass(
RefSCC &TargetRC = TargetC.getOuterRefSCC();
(void)TargetRC;
// TODO: This only allows trivial edges to be added for now.
+#ifdef EXPENSIVE_CHECKS
assert((RC == &TargetRC ||
RC->isAncestorOf(TargetRC)) && "New ref edge is not trivial!");
+#endif
RC->insertTrivialRefEdge(N, *RefTarget);
}
@@ -991,8 +993,10 @@ static LazyCallGraph::SCC &updateCGAndAnalysisManagerForPass(
RefSCC &TargetRC = TargetC.getOuterRefSCC();
(void)TargetRC;
// TODO: This only allows trivial edges to be added for now.
+#ifdef EXPENSIVE_CHECKS
assert((RC == &TargetRC ||
RC->isAncestorOf(TargetRC)) && "New call edge is not trivial!");
+#endif
// Add a trivial ref edge to be promoted later on alongside
// PromotedRefTargets.
RC->insertTrivialRefEdge(N, *CallTarget);
@@ -1085,8 +1089,10 @@ static LazyCallGraph::SCC &updateCGAndAnalysisManagerForPass(
// The easy case is when the target RefSCC is not this RefSCC. This is
// only supported when the target RefSCC is a child of this RefSCC.
if (&TargetRC != RC) {
+#ifdef EXPENSIVE_CHECKS
assert(RC->isAncestorOf(TargetRC) &&
"Cannot potentially form RefSCC cycles here!");
+#endif
RC->switchOutgoingEdgeToRef(N, *RefTarget);
LLVM_DEBUG(dbgs() << "Switch outgoing call edge to a ref edge from '" << N
<< "' to '" << *RefTarget << "'\n");
@@ -1119,8 +1125,10 @@ static LazyCallGraph::SCC &updateCGAndAnalysisManagerForPass(
// The easy case is when the target RefSCC is not this RefSCC. This is
// only supported when the target RefSCC is a child of this RefSCC.
if (&TargetRC != RC) {
+#ifdef EXPENSIVE_CHECKS
assert(RC->isAncestorOf(TargetRC) &&
"Cannot potentially form RefSCC cycles here!");
+#endif
RC->switchOutgoingEdgeToCall(N, *CallTarget);
LLVM_DEBUG(dbgs() << "Switch outgoing ref edge to a call edge from '" << N
<< "' to '" << *CallTarget << "'\n");
diff --git a/llvm/lib/Analysis/LazyCallGraph.cpp b/llvm/lib/Analysis/LazyCallGraph.cpp
index f2c85a69f125..16ea59ec32c2 100644
--- a/llvm/lib/Analysis/LazyCallGraph.cpp
+++ b/llvm/lib/Analysis/LazyCallGraph.cpp
@@ -588,9 +588,7 @@ LazyCallGraph::RefSCC::switchInternalEdgeToCall(
assert(!(*SourceN)[TargetN].isCall() && "Must start with a ref edge!");
SmallVector<SCC *, 1> DeletedSCCs;
-#ifndef NDEBUG
- // In a debug build, verify the RefSCC is valid to start with and when this
- // routine finishes.
+#ifdef EXPENSIVE_CHECKS
verify();
auto VerifyOnExit = make_scope_exit([&]() { verify(); });
#endif
@@ -620,7 +618,7 @@ LazyCallGraph::RefSCC::switchInternalEdgeToCall(
// Compute the SCCs which (transitively) reach the source.
auto ComputeSourceConnectedSet = [&](SmallPtrSetImpl<SCC *> &ConnectedSet) {
-#ifndef NDEBUG
+#ifdef EXPENSIVE_CHECKS
// Check that the RefSCC is still valid before computing this as the
// results will be nonsensical of we've broken its invariants.
verify();
@@ -646,7 +644,7 @@ LazyCallGraph::RefSCC::switchInternalEdgeToCall(
// but because this is forward connectivity we just "recurse" through the
// edges.
auto ComputeTargetConnectedSet = [&](SmallPtrSetImpl<SCC *> &ConnectedSet) {
-#ifndef NDEBUG
+#ifdef EXPENSIVE_CHECKS
// Check that the RefSCC is still valid before computing this as the
// results will be nonsensical of we've broken its invariants.
verify();
@@ -694,7 +692,7 @@ LazyCallGraph::RefSCC::switchInternalEdgeToCall(
return false; // No new cycle.
}
-#ifndef NDEBUG
+#ifdef EXPENSIVE_CHECKS
// Before merging, check that the RefSCC remains valid after all the
// postorder updates.
verify();
@@ -735,9 +733,7 @@ void LazyCallGraph::RefSCC::switchTrivialInternalEdgeToRef(Node &SourceN,
Node &TargetN) {
assert((*SourceN)[TargetN].isCall() && "Must start with a call edge!");
-#ifndef NDEBUG
- // In a debug build, verify the RefSCC is valid to start with and when this
- // routine finishes.
+#ifdef EXPENSIVE_CHECKS
verify();
auto VerifyOnExit = make_scope_exit([&]() { verify(); });
#endif
@@ -757,9 +753,7 @@ iterator_range<LazyCallGraph::RefSCC::iterator>
LazyCallGraph::RefSCC::switchInternalEdgeToRef(Node &SourceN, Node &TargetN) {
assert((*SourceN)[TargetN].isCall() && "Must start with a call edge!");
-#ifndef NDEBUG
- // In a debug build, verify the RefSCC is valid to start with and when this
- // routine finishes.
+#ifdef EXPENSIVE_CHECKS
verify();
auto VerifyOnExit = make_scope_exit([&]() { verify(); });
#endif
@@ -956,8 +950,7 @@ void LazyCallGraph::RefSCC::switchOutgoingEdgeToCall(Node &SourceN,
// just flip the edge here.
SourceN->setEdgeKind(TargetN, Edge::Call);
-#ifndef NDEBUG
- // Check that the RefSCC is still valid.
+#ifdef EXPENSIVE_CHECKS
verify();
#endif
}
@@ -978,8 +971,7 @@ void LazyCallGraph::RefSCC::switchOutgoingEdgeToRef(Node &SourceN,
// just flip the edge here.
SourceN->setEdgeKind(TargetN, Edge::Ref);
-#ifndef NDEBUG
- // Check that the RefSCC is still valid.
+#ifdef EXPENSIVE_CHECKS
verify();
#endif
}
@@ -991,8 +983,7 @@ void LazyCallGraph::RefSCC::insertInternalRefEdge(Node &SourceN,
SourceN->insertEdgeInternal(TargetN, Edge::Ref);
-#ifndef NDEBUG
- // Check that the RefSCC is still valid.
+#ifdef EXPENSIVE_CHECKS
verify();
#endif
}
@@ -1011,8 +1002,7 @@ void LazyCallGraph::RefSCC::insertOutgoingEdge(Node &SourceN, Node &TargetN,
"Target must be a descendant of the Source.");
#endif
-#ifndef NDEBUG
- // Check that the RefSCC is still valid.
+#ifdef EXPENSIVE_CHECKS
verify();
#endif
}
@@ -1029,9 +1019,7 @@ LazyCallGraph::RefSCC::insertIncomingRefEdge(Node &SourceN, Node &TargetN) {
SmallVector<RefSCC *, 1> DeletedRefSCCs;
-#ifndef NDEBUG
- // In a debug build, verify the RefSCC is valid to start with and when this
- // routine finishes.
+#ifdef EXPENSIVE_CHECKS
verify();
auto VerifyOnExit = make_scope_exit([&]() { verify(); });
#endif
@@ -1167,9 +1155,7 @@ void LazyCallGraph::RefSCC::removeOutgoingEdge(Node &SourceN, Node &TargetN) {
assert(G->lookupRefSCC(TargetN) != this &&
"The target must not be a member of this RefSCC");
-#ifndef NDEBUG
- // In a debug build, verify the RefSCC is valid to start with and when this
- // routine finishes.
+#ifdef EXPENSIVE_CHECKS
verify();
auto VerifyOnExit = make_scope_exit([&]() { verify(); });
#endif
@@ -1186,10 +1172,10 @@ LazyCallGraph::RefSCC::removeInternalRefEdge(Node &SourceN,
// We return a list of the resulting *new* RefSCCs in post-order.
SmallVector<RefSCC *, 1> Result;
-#ifndef NDEBUG
- // In a debug build, verify the RefSCC is valid to start with and that either
- // we return an empty list of result RefSCCs and this RefSCC remains valid,
- // or we return new RefSCCs and this RefSCC is dead.
+#ifdef EXPENSIVE_CHECKS
+ // Verify the RefSCC is valid to start with and that either we return an empty
+ // list of result RefSCCs and this RefSCC remains valid, or we return new
+ // RefSCCs and this RefSCC is dead.
verify();
auto VerifyOnExit = make_scope_exit([&]() {
// If we didn't replace our RefSCC with new ones, check that this one
@@ -1407,7 +1393,7 @@ LazyCallGraph::RefSCC::removeInternalRefEdge(Node &SourceN,
SCCs.clear();
SCCIndices.clear();
-#ifndef NDEBUG
+#ifdef EXPENSIVE_CHECKS
// Verify the new RefSCCs we've built.
for (RefSCC *RC : Result)
RC->verify();
@@ -1419,11 +1405,9 @@ LazyCallGraph::RefSCC::removeInternalRefEdge(Node &SourceN,
void LazyCallGraph::RefSCC::insertTrivialCallEdge(Node &SourceN,
Node &TargetN) {
-#ifndef NDEBUG
- // Check that the RefSCC is still valid when we finish.
+#ifdef EXPENSIVE_CHECKS
auto ExitVerifier = make_scope_exit([this] { verify(); });
-#ifdef EXPENSIVE_CHECKS
// Check that we aren't breaking some invariants of the SCC graph. Note that
// this is quadratic in the number of edges in the call graph!
SCC &SourceC = *G->lookupSCC(SourceN);
@@ -1431,8 +1415,7 @@ void LazyCallGraph::RefSCC::insertTrivialCallEdge(Node &SourceN,
if (&SourceC != &TargetC)
assert(SourceC.isAncestorOf(TargetC) &&
"Call edge is not trivial in the SCC graph!");
-#endif // EXPENSIVE_CHECKS
-#endif // NDEBUG
+#endif
// First insert it into the source or find the existing edge.
auto InsertResult =
@@ -1450,19 +1433,16 @@ void LazyCallGraph::RefSCC::insertTrivialCallEdge(Node &SourceN,
}
void LazyCallGraph::RefSCC::insertTrivialRefEdge(Node &SourceN, Node &TargetN) {
-#ifndef NDEBUG
- // Check that the RefSCC is still valid when we finish.
+#ifdef EXPENSIVE_CHECKS
auto ExitVerifier = make_scope_exit([this] { verify(); });
-#ifdef EXPENSIVE_CHECKS
// Check that we aren't breaking some invariants of the RefSCC graph.
RefSCC &SourceRC = *G->lookupRefSCC(SourceN);
RefSCC &TargetRC = *G->lookupRefSCC(TargetN);
if (&SourceRC != &TargetRC)
assert(SourceRC.isAncestorOf(TargetRC) &&
"Ref edge is not trivial in the RefSCC graph!");
-#endif // EXPENSIVE_CHECKS
-#endif // NDEBUG
+#endif
// First insert it into the source or find the existing edge.
auto InsertResult =
@@ -1478,8 +1458,7 @@ void LazyCallGraph::RefSCC::insertTrivialRefEdge(Node &SourceN, Node &TargetN) {
void LazyCallGraph::RefSCC::replaceNodeFunction(Node &N, Function &NewF) {
Function &OldF = N.getFunction();
-#ifndef NDEBUG
- // Check that the RefSCC is still valid when we finish.
+#ifdef EXPENSIVE_CHECKS
auto ExitVerifier = make_scope_exit([this] { verify(); });
assert(G->lookupRefSCC(N) == this &&
@@ -1638,7 +1617,7 @@ void LazyCallGraph::addSplitFunction(Function &OriginalFunction,
SCC *OriginalC = lookupSCC(OriginalN);
RefSCC *OriginalRC = lookupRefSCC(OriginalN);
-#ifndef NDEBUG
+#ifdef EXPENSIVE_CHECKS
OriginalRC->verify();
auto VerifyOnExit = make_scope_exit([&]() { OriginalRC->verify(); });
#endif
@@ -1717,14 +1696,12 @@ void LazyCallGraph::addSplitRefRecursiveFunctions(
Node &OriginalN = get(OriginalFunction);
RefSCC *OriginalRC = lookupRefSCC(OriginalN);
-#ifndef NDEBUG
+#ifdef EXPENSIVE_CHECKS
OriginalRC->verify();
auto VerifyOnExit = make_scope_exit([&]() {
OriginalRC->verify();
-#ifdef EXPENSIVE_CHECKS
for (Function *NewFunction : NewFunctions)
lookupRefSCC(get(*NewFunction))->verify();
-#endif
});
#endif
@@ -1979,7 +1956,7 @@ void LazyCallGraph::buildRefSCCs() {
(void)Inserted;
assert(Inserted && "Cannot already have this RefSCC in the index map!");
PostOrderRefSCCs.push_back(NewRC);
-#ifndef NDEBUG
+#ifdef EXPENSIVE_CHECKS
NewRC->verify();
#endif
});
More information about the llvm-commits
mailing list