[llvm] f773426 - [CGSCC] Properly handle invalidating analyses for invalidated SCCs

[Arthur Eubanks via llvm-commits]

Author: Arthur Eubanks
Date: 2022-09-21T09:50:00-07:00
New Revision: f77342693b46e1ae7924a340128a5b14e4137b2e

URL: https://github.com/llvm/llvm-project/commit/f77342693b46e1ae7924a340128a5b14e4137b2e
DIFF: https://github.com/llvm/llvm-project/commit/f77342693b46e1ae7924a340128a5b14e4137b2e.diff

LOG: [CGSCC] Properly handle invalidating analyses for invalidated SCCs

Currently if we mark an SCC as invalid, if we haven't set UR.UpdatedC, we won't propagate the PreservedAnalyses up to the parent pass (adaptor/pass manager).

In the provided test case, we inline the function into itself then delete it as it has no users. The SCC is marked as invalid without providing a replacement UR.UpdatedC. Then the CGSCC pass manager and adaptor discard the PreservedAnalyses. Instead, handle PreservedAnalyses first before bailing due to the invalid SCC.

Fixes crashes due to out of date analyses.

Added: 
    llvm/test/Other/invalidated-cgscc-invalidate-analyses.ll

Modified: 
    llvm/lib/Analysis/CGSCCPassManager.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/CGSCCPassManager.cpp b/llvm/lib/Analysis/CGSCCPassManager.cpp
index 7a802fe9fe020..0e96057b69a8b 100644
--- a/llvm/lib/Analysis/CGSCCPassManager.cpp
+++ b/llvm/lib/Analysis/CGSCCPassManager.cpp
@@ -100,22 +100,23 @@ PassManager<LazyCallGraph::SCC, CGSCCAnalysisManager, LazyCallGraph &,
       ResultFAMCP->updateFAM(FAM);
     }
 
+    // Intersect the final preserved analyses to compute the aggregate
+    // preserved set for this pass manager.
+    PA.intersect(PassPA);
+
     // If the CGSCC pass wasn't able to provide a valid updated SCC, the
     // current SCC may simply need to be skipped if invalid.
     if (UR.InvalidatedSCCs.count(C)) {
       LLVM_DEBUG(dbgs() << "Skipping invalidated root or island SCC!\n");
       break;
     }
+
     // Check that we didn't miss any update scenario.
     assert(C->begin() != C->end() && "Cannot have an empty SCC!");
 
     // Update the analysis manager as each pass runs and potentially
     // invalidates analyses.
     AM.invalidate(*C, PassPA);
-
-    // Finally, we intersect the final preserved analyses to compute the
-    // aggregate preserved set for this pass manager.
-    PA.intersect(std::move(PassPA));
   }
 
   // Before we mark all of *this* SCC's analyses as preserved below, intersect
@@ -290,12 +291,20 @@ ModuleToPostOrderCGSCCPassAdaptor::run(Module &M, ModuleAnalysisManager &AM) {
                 FAM);
           }
 
+          // Intersect with the cross-SCC preserved set to capture any
+          // cross-SCC invalidation.
+          UR.CrossSCCPA.intersect(PassPA);
+          // Intersect the preserved set so that invalidation of module
+          // analyses will eventually occur when the module pass completes.
+          PA.intersect(PassPA);
+
           // If the CGSCC pass wasn't able to provide a valid updated SCC,
           // the current SCC may simply need to be skipped if invalid.
           if (UR.InvalidatedSCCs.count(C)) {
             LLVM_DEBUG(dbgs() << "Skipping invalidated root or island SCC!\n");
             break;
           }
+
           // Check that we didn't miss any update scenario.
           assert(C->begin() != C->end() && "Cannot have an empty SCC!");
 
@@ -307,13 +316,6 @@ ModuleToPostOrderCGSCCPassAdaptor::run(Module &M, ModuleAnalysisManager &AM) {
           // processed.
           CGAM.invalidate(*C, PassPA);
 
-          // Then intersect the preserved set so that invalidation of module
-          // analyses will eventually occur when the module pass completes.
-          // Also intersect with the cross-SCC preserved set to capture any
-          // cross-SCC invalidation.
-          UR.CrossSCCPA.intersect(PassPA);
-          PA.intersect(std::move(PassPA));
-
           // The pass may have restructured the call graph and refined the
           // current SCC and/or RefSCC. We need to update our current SCC and
           // RefSCC pointers to follow these. Also, when the current SCC is

diff  --git a/llvm/test/Other/invalidated-cgscc-invalidate-analyses.ll b/llvm/test/Other/invalidated-cgscc-invalidate-analyses.ll
new file mode 100644
index 0000000000000..5dac08a704c8f
--- /dev/null
+++ b/llvm/test/Other/invalidated-cgscc-invalidate-analyses.ll
@@ -0,0 +1,18 @@
+; RUN: opt -passes='require<no-op-module>,inline' < %s -S -debug-pass-manager 2>&1 | FileCheck %s
+
+; CHECK: Invalidating analysis: NoOpModuleAnalysis
+
+; CHECK: ModuleID
+; CHECK-NOT: define
+
+define linkonce_odr i32 @f(i1 %0) {
+  br i1 %0, label %2, label %4
+
+2:                                                ; preds = %1
+  %3 = call i32 @f(i1 false)
+  br label %4
+
+4:                                                ; preds = %2, %1
+  ret i32 0
+}
+