[llvm] 8d800e6 - [CGSCC] Remove CGSCCUpdateResult::InvalidatedRefSCCs (#98213)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 10 09:54:59 PDT 2024
Author: Arthur Eubanks
Date: 2024-07-10T09:54:56-07:00
New Revision: 8d800e6c9014e55ff729f0237efeba246750b12c
URL: https://github.com/llvm/llvm-project/commit/8d800e6c9014e55ff729f0237efeba246750b12c
DIFF: https://github.com/llvm/llvm-project/commit/8d800e6c9014e55ff729f0237efeba246750b12c.diff
LOG: [CGSCC] Remove CGSCCUpdateResult::InvalidatedRefSCCs (#98213)
The RefSCC that a function marked dead is in may still contain valid
SCCs that we want to visit. We rely on InvalidatedSCCs to skip SCCs
containing dead functions.
The addition of RefSCCs in CallGraphUpdater to InvalidatedRefSCCs was
causing asserts as reported in #94815. Fix some more CallGraphUpdater
function deletion methods as well.
Added:
Modified:
llvm/include/llvm/Analysis/CGSCCPassManager.h
llvm/lib/Analysis/CGSCCPassManager.cpp
llvm/lib/Transforms/Utils/CallGraphUpdater.cpp
llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Analysis/CGSCCPassManager.h b/llvm/include/llvm/Analysis/CGSCCPassManager.h
index 59974d146b56d..406d3492136fc 100644
--- a/llvm/include/llvm/Analysis/CGSCCPassManager.h
+++ b/llvm/include/llvm/Analysis/CGSCCPassManager.h
@@ -245,14 +245,6 @@ struct CGSCCUpdateResult {
/// in reverse post-order.
SmallPriorityWorklist<LazyCallGraph::SCC *, 1> &CWorklist;
- /// The set of invalidated RefSCCs which should be skipped if they are found
- /// in \c RCWorklist.
- ///
- /// This is used to quickly prune out RefSCCs when they get deleted and
- /// happen to already be on the worklist. We use this primarily to avoid
- /// scanning the list and removing entries from it.
- SmallPtrSetImpl<LazyCallGraph::RefSCC *> &InvalidatedRefSCCs;
-
/// The set of invalidated SCCs which should be skipped if they are found
/// in \c CWorklist.
///
diff --git a/llvm/lib/Analysis/CGSCCPassManager.cpp b/llvm/lib/Analysis/CGSCCPassManager.cpp
index 24b3c6eef1084..c32739a565541 100644
--- a/llvm/lib/Analysis/CGSCCPassManager.cpp
+++ b/llvm/lib/Analysis/CGSCCPassManager.cpp
@@ -150,9 +150,8 @@ ModuleToPostOrderCGSCCPassAdaptor::run(Module &M, ModuleAnalysisManager &AM) {
SmallPriorityWorklist<LazyCallGraph::RefSCC *, 1> RCWorklist;
SmallPriorityWorklist<LazyCallGraph::SCC *, 1> CWorklist;
- // Keep sets for invalidated SCCs and RefSCCs that should be skipped when
+ // Keep sets for invalidated SCCs that should be skipped when
// iterating off the worklists.
- SmallPtrSet<LazyCallGraph::RefSCC *, 4> InvalidRefSCCSet;
SmallPtrSet<LazyCallGraph::SCC *, 4> InvalidSCCSet;
SmallDenseSet<std::pair<LazyCallGraph::Node *, LazyCallGraph::SCC *>, 4>
@@ -161,7 +160,6 @@ ModuleToPostOrderCGSCCPassAdaptor::run(Module &M, ModuleAnalysisManager &AM) {
SmallVector<Function *, 4> DeadFunctions;
CGSCCUpdateResult UR = {CWorklist,
- InvalidRefSCCSet,
InvalidSCCSet,
nullptr,
PreservedAnalyses::all(),
@@ -194,11 +192,6 @@ ModuleToPostOrderCGSCCPassAdaptor::run(Module &M, ModuleAnalysisManager &AM) {
do {
LazyCallGraph::RefSCC *RC = RCWorklist.pop_back_val();
- if (InvalidRefSCCSet.count(RC)) {
- LLVM_DEBUG(dbgs() << "Skipping an invalid RefSCC...\n");
- continue;
- }
-
assert(CWorklist.empty() &&
"Should always start with an empty SCC worklist");
@@ -1172,7 +1165,6 @@ static LazyCallGraph::SCC &updateCGAndAnalysisManagerForPass(
}
assert(!UR.InvalidatedSCCs.count(C) && "Invalidated the current SCC!");
- assert(!UR.InvalidatedRefSCCs.count(RC) && "Invalidated the current RefSCC!");
assert(&C->getOuterRefSCC() == RC && "Current SCC not in current RefSCC!");
// Record the current SCC for higher layers of the CGSCC pass manager now that
diff --git a/llvm/lib/Transforms/Utils/CallGraphUpdater.cpp b/llvm/lib/Transforms/Utils/CallGraphUpdater.cpp
index e9f37d4044cb0..7cba4829d4839 100644
--- a/llvm/lib/Transforms/Utils/CallGraphUpdater.cpp
+++ b/llvm/lib/Transforms/Utils/CallGraphUpdater.cpp
@@ -59,20 +59,14 @@ bool CallGraphUpdater::finalize() {
auto *DeadSCC = LCG->lookupSCC(N);
assert(DeadSCC && DeadSCC->size() == 1 &&
&DeadSCC->begin()->getFunction() == DeadFn);
- auto &DeadRC = DeadSCC->getOuterRefSCC();
- FunctionAnalysisManager &FAM =
- AM->getResult<FunctionAnalysisManagerCGSCCProxy>(*DeadSCC, *LCG)
- .getManager();
-
- FAM.clear(*DeadFn, DeadFn->getName());
+ FAM->clear(*DeadFn, DeadFn->getName());
AM->clear(*DeadSCC, DeadSCC->getName());
LCG->markDeadFunction(*DeadFn);
// Mark the relevant parts of the call graph as invalid so we don't
// visit them.
- UR->InvalidatedSCCs.insert(DeadSCC);
- UR->InvalidatedRefSCCs.insert(&DeadRC);
+ UR->InvalidatedSCCs.insert(LCG->lookupSCC(N));
UR->DeadFunctions.push_back(DeadFn);
} else {
// The CGSCC infrastructure batch deletes functions at the end of the
diff --git a/llvm/unittests/Analysis/CGSCCPassManagerTest.cpp b/llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
index 1532379c9ba02..9fd782d1b7614 100644
--- a/llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
+++ b/llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
@@ -1878,6 +1878,61 @@ TEST_F(CGSCCPassManagerTest, TestInsertionOfNewFunctions2) {
ASSERT_TRUE(Ran);
}
+TEST_F(CGSCCPassManagerTest, TestDeletionOfFunctionInNonTrivialRefSCC) {
+ std::unique_ptr<Module> M = parseIR("define void @f1() {\n"
+ "entry:\n"
+ " call void @f2()\n"
+ " ret void\n"
+ "}\n"
+ "define void @f2() {\n"
+ "entry:\n"
+ " call void @f1()\n"
+ " ret void\n"
+ "}\n");
+
+ bool Ran = false;
+ CGSCCPassManager CGPM;
+ CGPM.addPass(LambdaSCCPassNoPreserve(
+ [&](LazyCallGraph::SCC &C, CGSCCAnalysisManager &AM, LazyCallGraph &CG,
+ CGSCCUpdateResult &UR) {
+ if (Ran)
+ return;
+
+ LazyCallGraph::Node *N1 = nullptr;
+
+ for (LazyCallGraph::Node *N : SCCNodes(C)) {
+ Function &F = N->getFunction();
+ if (F.getName() != "f1")
+ continue;
+ N1 = N;
+
+ Function &F2 = *F.getParent()->getFunction("f2");
+
+ // Remove f1 <-> f2 references
+ F.getEntryBlock().front().eraseFromParent();
+ F2.getEntryBlock().front().eraseFromParent();
+
+ CallGraphUpdater CGU;
+ CGU.initialize(CG, C, AM, UR);
+ CGU.removeFunction(F2);
+ CGU.reanalyzeFunction(F);
+
+ Ran = true;
+ }
+
+ // Check that updateCGAndAnalysisManagerForCGSCCPass() after
+ // CallGraphUpdater::removeFunction() succeeds.
+ updateCGAndAnalysisManagerForCGSCCPass(CG, *CG.lookupSCC(*N1), *N1, AM,
+ UR, FAM);
+ }));
+
+ ModulePassManager MPM;
+ MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM)));
+ MPM.run(*M, MAM);
+
+ ASSERT_TRUE(Ran);
+}
+
TEST_F(CGSCCPassManagerTest, TestInsertionOfNewNonTrivialCallEdge) {
std::unique_ptr<Module> M = parseIR("define void @f1() {\n"
"entry:\n"
More information about the llvm-commits
mailing list