[llvm] [CGSCC] Remove CGSCCUpdateResult::InvalidatedRefSCCs (PR #98213)

[Arthur Eubanks via llvm-commits]

https://github.com/aeubanks created https://github.com/llvm/llvm-project/pull/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. I'm having trouble coming up with a reduced reproducer and plan on ripping out CallGraphUpdater anyway (since its purpose is to support both the legacy pass manager and new pass manager CGSCC infra, and the legacy pass manager CGSCC is no longer used), so no test.

>From b92fbf9cff096802c30bbc40cc53ef0ec22ff92c Mon Sep 17 00:00:00 2001
From: Arthur Eubanks <aeubanks at google.com>
Date: Tue, 9 Jul 2024 20:07:41 +0000
Subject: [PATCH] [CGSCC] Remove CGSCCUpdateResult::InvalidatedRefSCCs

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. I'm having trouble coming up with
a reduced reproducer and plan on ripping out CallGraphUpdater anyway
(since its purpose is to support both the legacy pass manager and new
pass manager CGSCC infra, and the legacy pass manager CGSCC is no
longer used), so no test.
---
 llvm/include/llvm/Analysis/CGSCCPassManager.h  |  8 --------
 llvm/lib/Analysis/CGSCCPassManager.cpp         | 10 +---------
 llvm/lib/Transforms/Utils/CallGraphUpdater.cpp |  2 --
 3 files changed, 1 insertion(+), 19 deletions(-)

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..2ddb1e54075c6 100644
--- a/llvm/lib/Transforms/Utils/CallGraphUpdater.cpp
+++ b/llvm/lib/Transforms/Utils/CallGraphUpdater.cpp
@@ -59,7 +59,6 @@ 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)
@@ -72,7 +71,6 @@ bool CallGraphUpdater::finalize() {
         // 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->DeadFunctions.push_back(DeadFn);
       } else {
         // The CGSCC infrastructure batch deletes functions at the end of the