[PATCH] D66376: [LoopPassManager + MemorySSA] Only enable use of MemorySSA for LPMs known to preserve it.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.

Some nits below. LGTM with those fixed!



================
Comment at: include/llvm/Analysis/LoopAnalysisManager.h:117
 
+  /// Mark MemorySSA as used so we can invalidate self is MSSA is invalidated.
+  void markMSSAUsed() { MSSAUsed = true; }
----------------
`self is MSSA` -> `self if MSSA`.


================
Comment at: include/llvm/Transforms/Scalar/LoopPassManager.h:316
+    if (UseMemorySSA)
+      AM.getResult<LoopAnalysisManagerFunctionProxy>(F).markMSSAUsed();
     LoopAnalysisManager &LAM =
----------------
I would cache the result of `AM.getResult<LoopAnalysisManagerFunctionProxy>(F)` in a variable -- it's cheap but not free to lookup.


================
Comment at: lib/Passes/PassBuilder.cpp:484
   FPM.addPass(InstCombinePass());
-  FPM.addPass(createFunctionToLoopPassAdaptor(std::move(LPM2), DebugLogging));
+  FPM.addPass(createFunctionToLoopPassAdaptor(
+      std::move(LPM2), /*UseMemorySSA=*/false, DebugLogging));
----------------
Maybe comment explaining that the second LPM does unrolling and so doesn't preserve MemorySSA?


================
Comment at: lib/Passes/PassBuilder.cpp:2002
       // Add the nested pass manager with the appropriate adaptor.
-      FPM.addPass(
-          createFunctionToLoopPassAdaptor(std::move(LPM), DebugLogging));
+      bool UseMemorySSA = (Name == "loop") ? false : true;
+      FPM.addPass(createFunctionToLoopPassAdaptor(std::move(LPM), UseMemorySSA,
----------------
Maybe: `= (Name == "loop-mssa");`?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66376/new/

https://reviews.llvm.org/D66376