Only passes that preserve MemorySSA must mark it as preserved.

[asbirlea]

Summary

The method getLoopPassPreservedAnalyses should not mark MemorySSA as
preserved, because it's being called in a lot of passes that do not
preserve MemorySSA.
Instead, mark the MemorySSA analysis as preserved by each pass that does
preserve it.
These changes only affect the new pass mananger.

[chandlerc]

LGTM

Any chance you can add a test that shows one of the problems where verifying MemorySSA fails because we get a "preserved" copy after a loop pass that doesn't actually preserve it?

Fine to submit the patch whenever, and just try ta add a test case when its convenient.

[asbirlea]

I wasn't yet able to reproduce this with opt -aa-pipeline=default -passes="default<O3>", only with clang -cc1 -O3 -fexperimental-new-pass-manager.
I'll keep working on getting a test, but I checked this in for now, since it's necessary to fix crashes for the combination of MemorySSA + NewPassManager.

llvm/trunk/lib/Analysis/LoopAnalysisManager.cpp

 }
 
 PreservedAnalyses llvm::getLoopPassPreservedAnalyses() {
   PreservedAnalyses PA;
   PA.preserve<DominatorTreeAnalysis>();
   PA.preserve<LoopAnalysis>();
   PA.preserve<LoopAnalysisManagerFunctionProxy>();
   PA.preserve<ScalarEvolutionAnalysis>();
-  if (EnableMSSALoopDependency)
-    PA.preserve<MemorySSAAnalysis>();
   // FIXME: What we really want to do here is preserve an AA category, but that
   // concept doesn't exist yet.
   PA.preserve<AAManager>();
   PA.preserve<BasicAA>();
   PA.preserve<GlobalsAA>();
   PA.preserve<SCEVAA>();
   return PA;
 }

llvm/trunk/lib/Transforms/Scalar/LICM.cpp

PreservedAnalyses LICMPass::run(Loop &L, LoopAnalysisManager &AM,
   if (!LICM.runOnLoop(&L, &AR.AA, &AR.LI, &AR.DT, &AR.TLI, &AR.TTI, &AR.SE,
                       AR.MSSA, ORE, true))
     return PreservedAnalyses::all();
 
   auto PA = getLoopPassPreservedAnalyses();
 
   PA.preserve<DominatorTreeAnalysis>();
   PA.preserve<LoopAnalysis>();
+  if (EnableMSSALoopDependency)
+    PA.preserve<MemorySSAAnalysis>();
 
   return PA;
 }
 
 char LegacyLICMPass::ID = 0;
 INITIALIZE_PASS_BEGIN(LegacyLICMPass, "licm", "Loop Invariant Code Motion",
                       false, false)
 INITIALIZE_PASS_DEPENDENCY(LoopPass)

llvm/trunk/lib/Transforms/Scalar/LoopInstSimplify.cpp

if (AR.MSSA) {
     AR.MSSA->verifyMemorySSA();
   }
   if (!simplifyLoopInst(L, AR.DT, AR.LI, AR.AC, AR.TLI,
                         MSSAU.hasValue() ? MSSAU.getPointer() : nullptr))
     return PreservedAnalyses::all();
 
   auto PA = getLoopPassPreservedAnalyses();
   PA.preserveSet<CFGAnalyses>();
+  if (EnableMSSALoopDependency)
+    PA.preserve<MemorySSAAnalysis>();
   return PA;
 }
 
 char LoopInstSimplifyLegacyPass::ID = 0;
 
 INITIALIZE_PASS_BEGIN(LoopInstSimplifyLegacyPass, "loop-instsimplify",
                       "Simplify instructions in loops", false, false)
 INITIALIZE_PASS_DEPENDENCY(AssumptionCacheTracker)

llvm/trunk/lib/Transforms/Scalar/LoopRotation.cpp

bool Changed = LoopRotation(&L, &AR.LI, &AR.TTI, &AR.AC, &AR.DT, &AR.SE,
                               SQ, false, Threshold, false);
 
   if (!Changed)
     return PreservedAnalyses::all();
 
   if (AR.MSSA && VerifyMemorySSA)
     AR.MSSA->verifyMemorySSA();
 
-  return getLoopPassPreservedAnalyses();
+  auto PA = getLoopPassPreservedAnalyses();
+  if (EnableMSSALoopDependency)
+    PA.preserve<MemorySSAAnalysis>();
+  return PA;
 }
 
 namespace {
 
 class LoopRotateLegacyPass : public LoopPass {
   unsigned MaxHeaderSize;
 
 public:

llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp

PreservedAnalyses LoopSimplifyCFGPass::run(Loop &L, LoopAnalysisManager &AM,
   if (!simplifyLoopCFG(L, AR.DT, AR.LI, AR.SE,
                        MSSAU.hasValue() ? MSSAU.getPointer() : nullptr,
                        DeleteCurrentLoop))
     return PreservedAnalyses::all();
 
   if (DeleteCurrentLoop)
     LPMU.markLoopAsDeleted(L, "loop-simplifycfg");
 
-  return getLoopPassPreservedAnalyses();
+  auto PA = getLoopPassPreservedAnalyses();
+  if (EnableMSSALoopDependency)
+    PA.preserve<MemorySSAAnalysis>();
+  return PA;
 }
 
 namespace {
 class LoopSimplifyCFGLegacyPass : public LoopPass {
 public:
   static char ID; // Pass ID, replacement for typeid
   LoopSimplifyCFGLegacyPass() : LoopPass(ID) {
     initializeLoopSimplifyCFGLegacyPassPass(*PassRegistry::getPassRegistry());

llvm/trunk/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp

if (!unswitchLoop(L, AR.DT, AR.LI, AR.AC, AR.TTI, NonTrivial, UnswitchCB,
     return PreservedAnalyses::all();
 
   if (AR.MSSA && VerifyMemorySSA)
     AR.MSSA->verifyMemorySSA();
 
   // Historically this pass has had issues with the dominator tree so verify it
   // in asserts builds.
   assert(AR.DT.verify(DominatorTree::VerificationLevel::Fast));
-  return getLoopPassPreservedAnalyses();
+
+  auto PA = getLoopPassPreservedAnalyses();
+  if (EnableMSSALoopDependency)
+    PA.preserve<MemorySSAAnalysis>();
+  return PA;
 }
 
 namespace {
 
 class SimpleLoopUnswitchLegacyPass : public LoopPass {
   bool NonTrivial;
 
 public:

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

EXPECT_CALL(MLAHandle, run(HasName("loop.g.0"), _, _))
       .InSequence(GSequence)
       .WillOnce(DoDefault());
   FPM.addPass(createFunctionToLoopPassAdaptor(
       RequireAnalysisLoopPass<MockLoopAnalysisHandle::Analysis>()));
 
   // No need to re-run if we require again from a fresh loop pass manager.
   FPM.addPass(createFunctionToLoopPassAdaptor(
       RequireAnalysisLoopPass<MockLoopAnalysisHandle::Analysis>()));
-
   // For 'f', preserve most things but not the specific loop analyses.
+  auto PA = getLoopPassPreservedAnalyses();
+  if (EnableMSSALoopDependency)
+    PA.preserve<MemorySSAAnalysis>();
   EXPECT_CALL(MFPHandle, run(HasName("f"), _))
       .InSequence(FSequence)
-      .WillOnce(Return(getLoopPassPreservedAnalyses()));
+      .WillOnce(Return(PA));
   EXPECT_CALL(MLAHandle, invalidate(HasName("loop.0.0"), _, _))
       .InSequence(FSequence)
       .WillOnce(DoDefault());
   // On one loop, skip the invalidation (as though we did an internal update).
   EXPECT_CALL(MLAHandle, invalidate(HasName("loop.0.1"), _, _))
       .InSequence(FSequence)
       .WillOnce(Return(false));
   EXPECT_CALL(MLAHandle, invalidate(HasName("loop.0"), _, _))
TEST_F(LoopPassManagerTest, ModulePassInvalidationOfLoopAnalyses) {
   MPM.addPass(createModuleToFunctionPassAdaptor(createFunctionToLoopPassAdaptor(
       RequireAnalysisLoopPass<MockLoopAnalysisHandle::Analysis>())));
 
   // But a module pass that doesn't preserve the actual mock loop analysis
   // invalidates all the way down and forces recomputing.
   EXPECT_CALL(MMPHandle, run(_, _)).WillOnce(InvokeWithoutArgs([] {
     auto PA = getLoopPassPreservedAnalyses();
     PA.preserve<FunctionAnalysisManagerModuleProxy>();
+    if (EnableMSSALoopDependency)
+      PA.preserve<MemorySSAAnalysis>();
     return PA;
   }));
   // All the loop analyses from both functions get invalidated before we
   // recompute anything.
   EXPECT_CALL(MLAHandle, invalidate(HasName("loop.0.0"), _, _));
   // On one loop, again skip the invalidation (as though we did an internal
   // update).
   EXPECT_CALL(MLAHandle, invalidate(HasName("loop.0.1"), _, _))
TEST_F(LoopPassManagerTest, IndirectOuterPassInvalidation) {
   EXPECT_CALL(MLAHandle, run(HasName("loop.0.1"), _, _));
   EXPECT_CALL(MLAHandle, run(HasName("loop.0"), _, _));
 
   // Now invalidate the function analysis but preserve the loop analyses.
   // This should trigger immediate invalidation of the loop analyses, despite
   // the fact that they were preserved.
   EXPECT_CALL(MFPHandle, run(HasName("f"), _)).WillOnce(InvokeWithoutArgs([] {
     auto PA = getLoopPassPreservedAnalyses();
+    if (EnableMSSALoopDependency)
+      PA.preserve<MemorySSAAnalysis>();
     PA.preserveSet<AllAnalysesOn<Loop>>();
     return PA;
   }));
   EXPECT_CALL(MLAHandle, invalidate(HasName("loop.0.0"), _, _));
   EXPECT_CALL(MLAHandle, invalidate(HasName("loop.0.1"), _, _));
   EXPECT_CALL(MLAHandle, invalidate(HasName("loop.0"), _, _));
 
   // And re-running a requires pass recomputes them.
   EXPECT_CALL(MLAHandle, run(HasName("loop.0.0"), _, _));
   EXPECT_CALL(MLAHandle, run(HasName("loop.0.1"), _, _));
   EXPECT_CALL(MLAHandle, run(HasName("loop.0"), _, _));
 
   // When we run over 'g' we don't populate the cache with the function
   // analysis.
   EXPECT_CALL(MFPHandle, run(HasName("g"), _))
       .WillOnce(Return(PreservedAnalyses::all()));
   EXPECT_CALL(MLAHandle, run(HasName("loop.g.0"), _, _));
 
   // Which means that no extra invalidation occurs and cached values are used.
   EXPECT_CALL(MFPHandle, run(HasName("g"), _)).WillOnce(InvokeWithoutArgs([] {
     auto PA = getLoopPassPreservedAnalyses();
+    if (EnableMSSALoopDependency)
+      PA.preserve<MemorySSAAnalysis>();
     PA.preserveSet<AllAnalysesOn<Loop>>();
     return PA;
   }));
 
   // Build the pipeline and run it.
   ModulePassManager MPM(true);
   FunctionPassManager FPM(true);
   FPM.addPass(MFPHandle.getPass());