[PM] Finish implementing and fix a chain of bugs uncovered by testing

the invalidation propagation logic from an SCC to a Function.

I wrote the infrastructure to test this but didn't actually use it in
the unit test where it was designed to be used. =[ My bad. Once
I actually added it to the test case I discovered that it also hadn't
been properly implemented, so I've implemented it. The logic in the FAM
proxy for an SCC pass to propagate invalidation follows the same ideas
as the FAM proxy for a Module pass, but the implementation is a bit
different to reflect the fact that it is forwarding just for an SCC.

However, implementing this correctly uncovered a surprising "bug" (it
was conservatively correct but relatively very expensive) in how we
handle invalidation when splitting one SCC into multiple SCCs. We did an
eager invalidation when in reality we should be deferring invaliadtion
for the *current* SCC to the CGSCC pass manager and just invaliating the
newly constructed SCCs. Otherwise we end up invalidating too much too
soon. This was exposed by the inliner test case that I've updated. Now,
we invalidate *just* the split off '(test1_f)' SCC when doing the CG
update, and then the inliner finishes and invalidates the '(test1_g,
test1_h)' SCC's analyses. The first few attempts at fixing this hit
still more bugs, but all of those are covered by existing tests. For
example, the inliner should also preserve the FAM proxy to avoid
unnecesasry invalidation, and this is safe because the CG update
routines it uses handle any necessary adjustments to the FAM proxy.

Finally, the unittests for the CGSCC pass manager needed a bunch of
updates where we weren't correctly preserving the FAM proxy because it
hadn't been fully implemented and failing to preserve it didn't matter.

Note that this doesn't yet fix the current crasher due to MemSSA finding
a stale dominator tree, but without this the fix to that crasher doesn't
really make any sense when testing because it relies on the proxy
behavior.

llvm-svn: 307487

1 files changed, 24 insertions, 6 deletions

diff --git a/llvm/unittests/Analysis/CGSCCPassManagerTest.cpp b/llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
index ab5d1862c23..6ab30732b82 100644
--- a/llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
+++ b/llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
@@ -680,6 +680,7 @@ TEST_F(CGSCCPassManagerTest, TestSCCPassCanPreserveFunctionAnalysis) {
                                  LazyCallGraph &, CGSCCUpdateResult &) {
     PreservedAnalyses PA;
     PA.preserve<LazyCallGraphAnalysis>();
+    PA.preserve<FunctionAnalysisManagerCGSCCProxy>();
     PA.preserve<TestFunctionAnalysis>();
     return PA;
   }));
@@ -719,12 +720,14 @@ TEST_F(CGSCCPassManagerTest,
   CGPM1.addPass(createCGSCCToFunctionPassAdaptor(std::move(FPM1)));
   MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM1)));
 
-  // Now run a module pass that preserves the LazyCallGraph and proxy but not
+  // Now run a module pass that preserves the LazyCallGraph and proxies but not
   // the Function analysis.
   MPM.addPass(LambdaModulePass([&](Module &M, ModuleAnalysisManager &) {
     PreservedAnalyses PA;
     PA.preserve<LazyCallGraphAnalysis>();
     PA.preserve<CGSCCAnalysisManagerModuleProxy>();
+    PA.preserve<FunctionAnalysisManagerCGSCCProxy>();
+    PA.preserve<FunctionAnalysisManagerModuleProxy>();
     return PA;
   }));
 
@@ -741,7 +744,7 @@ TEST_F(CGSCCPassManagerTest,
   EXPECT_EQ(2 * 6, FunctionAnalysisRuns);
 }
 
-// Check that by marking the function pass and FAM proxy as preserved, this
+// Check that by marking the function pass and proxies as preserved, this
 // propagates all the way through.
 TEST_F(CGSCCPassManagerTest,
        TestModulePassCanPreserveFunctionAnalysisNestedInCGSCC) {
@@ -765,6 +768,7 @@ TEST_F(CGSCCPassManagerTest,
     PreservedAnalyses PA;
     PA.preserve<LazyCallGraphAnalysis>();
     PA.preserve<CGSCCAnalysisManagerModuleProxy>();
+    PA.preserve<FunctionAnalysisManagerCGSCCProxy>();
     PA.preserve<FunctionAnalysisManagerModuleProxy>();
     PA.preserve<TestFunctionAnalysis>();
     return PA;
@@ -1014,6 +1018,9 @@ TEST_F(CGSCCPassManagerTest, TestIndirectAnalysisInvalidation) {
         FunctionCount += IndirectResult.SCCDep.FunctionCount;
         return PreservedAnalyses::all();
       }));
+  CGPM.addPass(createCGSCCToFunctionPassAdaptor(
+      RequireAnalysisPass<TestIndirectFunctionAnalysis, Function>()));
+
   // Next, invalidate
   //   - both analyses for the (f) and (x) SCCs,
   //   - just the underlying (indirect) analysis for (g) SCC, and
@@ -1026,14 +1033,16 @@ TEST_F(CGSCCPassManagerTest, TestIndirectAnalysisInvalidation) {
         auto &IndirectResult = DoublyIndirectResult.IDep;
         FunctionCount += IndirectResult.SCCDep.FunctionCount;
         auto PA = PreservedAnalyses::none();
+        PA.preserve<FunctionAnalysisManagerCGSCCProxy>();
+        PA.preserveSet<AllAnalysesOn<Function>>();
         if (C.getName() == "(g)")
           PA.preserve<TestSCCAnalysis>();
         else if (C.getName() == "(h3, h1, h2)")
           PA.preserve<TestIndirectSCCAnalysis>();
         return PA;
       }));
-  // Finally, use the analysis again on each function, forcing re-computation
-  // for all of them.
+  // Finally, use the analysis again on each SCC (and function), forcing
+  // re-computation for all of them.
   CGPM.addPass(
       LambdaSCCPass([&](LazyCallGraph::SCC &C, CGSCCAnalysisManager &AM,
                         LazyCallGraph &CG, CGSCCUpdateResult &) {
@@ -1043,6 +1052,8 @@ TEST_F(CGSCCPassManagerTest, TestIndirectAnalysisInvalidation) {
         FunctionCount += IndirectResult.SCCDep.FunctionCount;
         return PreservedAnalyses::all();
       }));
+  CGPM.addPass(createCGSCCToFunctionPassAdaptor(
+      RequireAnalysisPass<TestIndirectFunctionAnalysis, Function>()));
 
   // Create a second CGSCC pass manager. This will cause the module-level
   // invalidation to occur, which will force yet another invalidation of the
@@ -1058,13 +1069,15 @@ TEST_F(CGSCCPassManagerTest, TestIndirectAnalysisInvalidation) {
         FunctionCount += IndirectResult.SCCDep.FunctionCount;
         return PreservedAnalyses::all();
       }));
+  CGPM2.addPass(createCGSCCToFunctionPassAdaptor(
+      RequireAnalysisPass<TestIndirectFunctionAnalysis, Function>()));
 
-  // Add a requires pass to populate the module analysis and then our function
+  // Add a requires pass to populate the module analysis and then our CGSCC
   // pass pipeline.
   MPM.addPass(RequireAnalysisPass<TestModuleAnalysis, Module>());
   MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM)));
   // Now require the module analysis again (it will have been invalidated once)
-  // and then use it again from a function pass manager.
+  // and then use it again from our second CGSCC pipeline..
   MPM.addPass(RequireAnalysisPass<TestModuleAnalysis, Module>());
   MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM2)));
   MPM.run(*M, MAM);
@@ -1080,6 +1093,11 @@ TEST_F(CGSCCPassManagerTest, TestIndirectAnalysisInvalidation) {
   EXPECT_EQ(3 * 4, IndirectSCCAnalysisRuns);
   EXPECT_EQ(3 * 4, DoublyIndirectSCCAnalysisRuns);
 
+  // We run the indirect function analysis once per function the first time.
+  // Then we re-run it for every SCC but "(g)". Then we re-run it for every
+  // function again.
+  EXPECT_EQ(6 + 5 + 6, IndirectFunctionAnalysisRuns);
+
   // Four passes count each of six functions once (via SCCs).
   EXPECT_EQ(4 * 6, FunctionCount);
 }