[AliasAnalysis/NewPassManager] Invalidate AAManager less often.

[asbirlea]

Summary

This is a redo of D60914.

The objective is to not invalidate AAManager, which is stateless, unless
there is an explicit invalidate in one of the AAResults.

To achieve this, this patch adds an API to PAC, to check precisely this:
is this analysis not invalidated explicitly == is this analysis not abandoned == is this analysis stateless, so preserved without explicitly being marked as preserved by everyone

[chandlerc]

LGTM, very nice! Thanks for all the work tracking down this subtle case!

I've left some tiny suggestions below, but feel free to submit.

include/llvm/Analysis/AliasAnalysis.h

1104 ↗

(On Diff #197219)

stray blank line?

include/llvm/IR/PassManager.h

289–291 ↗

(On Diff #197219)

Make a doxygen comment?

lib/Analysis/AliasAnalysis.cpp

86–89 ↗

(On Diff #197219)

I'd move everything before the "However" to a comment on the PAC line. The rest makes sense here.

llvm/trunk/include/llvm/Analysis/AliasAnalysis.h

 /// that dispatches across all of the alias analysis results available.
 ///
 /// Note that the order in which analyses are registered is very significant.
 /// That is the order in which the results will be aggregated and queried.
 ///
 /// This manager effectively wraps the AnalysisManager for registering alias
 /// analyses. When you register your alias analysis with this manager, it will
 /// ensure the analysis itself is registered with its AnalysisManager.
+///
+/// The result of this analysis is only invalidated if one of the particular
+/// aggregated AA results end up being invalidated. This removes the need to
+/// explicitly preserve the results of `AAManager`. Note that analyses should no
+/// longer be registered once the `AAManager` is run.
 class AAManager : public AnalysisInfoMixin<AAManager> {
 public:
   using Result = AAResults;
 
   /// Register a specific AA result.
   template <typename AnalysisT> void registerFunctionAnalysis() {
     ResultGetters.push_back(&getFunctionAAResultImpl<AnalysisT>);
   }

llvm/trunk/include/llvm/IR/PassManager.h

public:
     /// Returns true if the checker's analysis was not abandoned and either
     ///  - the analysis is explicitly preserved or
     ///  - all analyses are preserved.
     bool preserved() {
       return !IsAbandoned && (PA.PreservedIDs.count(&AllAnalysesKey) ||
                               PA.PreservedIDs.count(ID));
     }
 
+    /// Return true if the checker's analysis was not abandoned, i.e. it was not
+    /// explicitly invalidated. Even if the analysis is not explicitly
+    /// preserved, if the analysis is known stateless, then it is preserved.
+    bool preservedWhenStateless() {
+      return !IsAbandoned;
+    }
+
     /// Returns true if the checker's analysis was not abandoned and either
     ///  - \p AnalysisSetT is explicitly preserved or
     ///  - all analyses are preserved.
     template <typename AnalysisSetT> bool preservedSet() {
       AnalysisSetKey *SetID = AnalysisSetT::ID();
       return !IsAbandoned && (PA.PreservedIDs.count(&AllAnalysesKey) ||
                               PA.PreservedIDs.count(SetID));
     }

llvm/trunk/lib/Analysis/AliasAnalysis.cpp

 #if 0
   for (auto &AA : AAs)
     AA->setAAResults(nullptr);
 #endif
 }
 
 bool AAResults::invalidate(Function &F, const PreservedAnalyses &PA,
                            FunctionAnalysisManager::Invalidator &Inv) {
-  // Check if the AA manager itself has been invalidated.
+  // AAResults preserves the AAManager by default, due to the stateless nature
+  // of AliasAnalysis. There is no need to check whether it has been preserved
+  // explicitly. Check if any module dependency was invalidated and caused the
+  // AAManager to be invalidated. Invalidate ourselves in that case.
   auto PAC = PA.getChecker<AAManager>();
-  if (!PAC.preserved() && !PAC.preservedSet<AllAnalysesOn<Function>>())
-    return true; // The manager needs to be blown away, clear everything.
+  if (!PAC.preservedWhenStateless())
+    return true;
 
-  // Check all of the dependencies registered.
+  // Check if any of the function dependencies were invalidated, and invalidate
+  // ourselves in that case.
   for (AnalysisKey *ID : AADeps)
     if (Inv.invalidate(ID, F, PA))
       return true;
 
   // Everything we depend on is still fine, so are we. Nothing to invalidate.
   return false;
 }
 

llvm/trunk/test/Analysis/MemoryDependenceAnalysis/invalidation.ll

 ; Test that memdep gets invalidated when the analyses it depends on are
 ; invalidated.
 ;
-; Check AA specifically.
+; Check AA. AA is stateless, there's nothing to invalidate.
 ; RUN: opt -disable-output -debug-pass-manager -aa-pipeline='basic-aa' %s 2>&1 \
 ; RUN:     -passes='require<memdep>,invalidate<aa>,gvn' \
 ; RUN:     | FileCheck %s --check-prefix=CHECK-AA-INVALIDATE
 ; CHECK-AA-INVALIDATE: Running pass: RequireAnalysisPass
 ; CHECK-AA-INVALIDATE: Running analysis: MemoryDependenceAnalysis
 ; CHECK-AA-INVALIDATE: Running pass: InvalidateAnalysisPass
-; CHECK-AA-INVALIDATE: Invalidating analysis: AAManager
-; CHECK-AA-INVALIDATE: Invalidating analysis: MemoryDependenceAnalysis
+; CHECK-NOT-AA-INVALIDATE: Invalidating analysis: MemoryDependenceAnalysis
 ; CHECK-AA-INVALIDATE: Running pass: GVN
-; CHECK-AA-INVALIDATE: Running analysis: MemoryDependenceAnalysis
+; CHECK-NOT-AA-INVALIDATE: Running analysis: MemoryDependenceAnalysis
 ;
 ; Check domtree specifically.
 ; RUN: opt -disable-output -debug-pass-manager %s 2>&1 \
 ; RUN:     -passes='require<memdep>,invalidate<domtree>,gvn' \
 ; RUN:     | FileCheck %s --check-prefix=CHECK-DT-INVALIDATE
 ; CHECK-DT-INVALIDATE: Running pass: RequireAnalysisPass
 ; CHECK-DT-INVALIDATE: Running analysis: MemoryDependenceAnalysis
 ; CHECK-DT-INVALIDATE: Running pass: InvalidateAnalysisPass

llvm/trunk/test/Other/new-pass-manager.ll

 ; CHECK-AA-MODULE-INVALIDATE: Starting llvm::Module pass manager run
 ; CHECK-AA-MODULE-INVALIDATE: Running pass: RequireAnalysisPass
 ; CHECK-AA-MODULE-INVALIDATE: Running analysis: GlobalsAA
 ; CHECK-AA-MODULE-INVALIDATE: Starting llvm::Function pass manager run
 ; CHECK-AA-MODULE-INVALIDATE: Running pass: RequireAnalysisPass
 ; CHECK-AA-MODULE-INVALIDATE: Running analysis: AAManager
 ; CHECK-AA-MODULE-INVALIDATE: Finished llvm::Function pass manager run
 ; CHECK-AA-MODULE-INVALIDATE: Running pass: InvalidateAnalysisPass
-; CHECK-AA-MODULE-INVALIDATE: Invalidating analysis: AAManager
 ; CHECK-AA-MODULE-INVALIDATE: Invalidating analysis: GlobalsAA
 ; CHECK-AA-MODULE-INVALIDATE: Running pass: RequireAnalysisPass
 ; CHECK-AA-MODULE-INVALIDATE: Running analysis: GlobalsAA
 ; CHECK-AA-MODULE-INVALIDATE: Starting llvm::Function pass manager run
 ; CHECK-AA-MODULE-INVALIDATE: Running pass: AAEvaluator
-; CHECK-AA-MODULE-INVALIDATE: Running analysis: AAManager
 ; CHECK-AA-MODULE-INVALIDATE: Finished llvm::Function pass manager run
 ; CHECK-AA-MODULE-INVALIDATE: Finished llvm::Module pass manager run
 
 ; RUN: opt -disable-output -disable-verify -debug-pass-manager %s 2>&1 \
 ; RUN:     -passes='require<memdep>' \
 ; RUN:     | FileCheck %s --check-prefix=CHECK-MEMDEP
 ; CHECK-MEMDEP: Starting llvm::Module pass manager run
 ; CHECK-MEMDEP: Running pass: RequireAnalysisPass

llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp

TEST_F(LoopPassManagerTest, InvalidationOfBundledAnalyses) {
   // No need to re-run if we require again from a fresh loop pass manager.
   FPM.addPass(createFunctionToLoopPassAdaptor(
       RequireAnalysisLoopPass<MockLoopAnalysisHandle::Analysis>()));
 
   // Preserving everything but the loop analyses themselves results in
   // invalidation and running.
   EXPECT_CALL(MFPHandle, run(HasName("f"), _))
       .WillOnce(Return(getLoopPassPreservedAnalyses()));
-  EXPECT_CALL(MLAHandle, invalidate(_, _, _)).Times(3);
   EXPECT_CALL(MLAHandle, run(HasName("loop.0.0"), _, _));
   EXPECT_CALL(MLAHandle, run(HasName("loop.0.1"), _, _));
   EXPECT_CALL(MLAHandle, run(HasName("loop.0"), _, _));
   FPM.addPass(MFPHandle.getPass());
   FPM.addPass(createFunctionToLoopPassAdaptor(
       RequireAnalysisLoopPass<MockLoopAnalysisHandle::Analysis>()));
 
   // The rest don't invalidate analyses, they only trigger re-runs because we