[CGSCC] Remove CGSCCUpdateResult::InvalidatedRefSCCs (llvm#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 llvm#94815. Fix some more CallGraphUpdater
function deletion methods as well.

Author: aeubanks

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.
   ///

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

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

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"