[PassBuilder] Use loop-mssa for licm

[nikic]

Summary

Currently specifying -licm or -passes=licm will implicitly create -passes=loop(licm). This does not match the intended default (used by the legacy PM and by the default pipeline) of using the MemorySSA-based licm implementation. As I plan to drop the non-MSSA implementation, this will stop working entirely...

This special-cases licm to create a loop-mssa manager instead. At this point it's still possible to use -passes='loop(licm)' to opt into the AST-based implementation.

[nikic]

llvm/lib/Passes/PassBuilder.cpp

2422

I dropped this code because I found it confusing, and it's effectively dead: We call isFunctionPassName() first, which will already handle these.

[aeubanks]

hmm, this is super hacky, but I can't think of a better way

at some point should we enable MSSA by default for loop passes?

[nikic]

In D108155#2949687, @aeubanks wrote:

at some point should we enable MSSA by default for loop passes?

Not in general. There are basically three types of loop passes: Those that don't preserve MSSA (using them with loop-mssa is illegal), those that preserve but don't use MSSA (using them with loop-mssa only makes sense as part of a pipeline) and those that can use MSSA (those probably should use loop-mssa by default).

[jroelofs]

After some bisection, it appears this change broke llvm/test/Transforms/GuardWidening/loop-schedule.ll. The NewPM half of that test fails with:

Assertion failed: (AL->size() == ActualAccesses.size() && "We don't have the same number of accesses in the block as on the " "access list"), function verifyOrderingDominationAndDefUses, file MemorySSA.cpp, line 2018.
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /Users/jroelofs/llvm-upstream/build/bin/opt -S -passes=licm,guard-widening,licm -debug-pass-manager
Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
0  opt                      0x0000000108c3d587 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 39
1  opt                      0x0000000108c3c318 llvm::sys::RunSignalHandlers() + 248
2  opt                      0x0000000108c3dbd0 SignalHandler(int) + 288
3  libsystem_platform.dylib 0x00007fff204dfd7d _sigtramp + 29
4  libsystem_platform.dylib 000000000000000000 _sigtramp + 18446603339974181536
5  libsystem_c.dylib        0x00007fff203ef411 abort + 120
6  libsystem_c.dylib        0x00007fff203ee7e8 err + 0
7  opt                      0x0000000109809d13 llvm::MemorySSA::verifyOrderingDominationAndDefUses(llvm::Function&) const (.cold.8) + 35
8  opt                      0x0000000107c54da9 llvm::MemorySSA::verifyOrderingDominationAndDefUses(llvm::Function&) const + 2841
9  opt                      0x0000000107c54272 llvm::MemorySSA::verifyMemorySSA() const + 18
10 opt                      0x00000001089593e0 llvm::sinkRegion(llvm::DomTreeNodeBase<llvm::BasicBlock>*, llvm::AAResults*, llvm::LoopInfo*, llvm::DominatorTree*, llvm::BlockFrequencyInfo*, llvm::TargetLibraryInfo*, llvm::TargetTransformInfo*, llvm::Loop*, llvm::AliasSetTracker*, llvm::MemorySSAUpdater*, llvm::ICFLoopSafetyInfo*, llvm::SinkAndHoistLICMFlags&, llvm::OptimizationRemarkEmitter*, llvm::Loop*) + 10640
11 opt                      0x0000000108956196 (anonymous namespace)::LoopInvariantCodeMotion::runOnLoop(llvm::Loop*, llvm::AAResults*, llvm::LoopInfo*, llvm::DominatorTree*, llvm::BlockFrequencyInfo*, llvm::TargetLibraryInfo*, llvm::TargetTransformInfo*, llvm::ScalarEvolution*, llvm::MemorySSA*, llvm::OptimizationRemarkEmitter*, bool) + 5862
12 opt                      0x0000000108954968 llvm::LICMPass::run(llvm::Loop&, llvm::AnalysisManager<llvm::Loop, llvm::LoopStandardAnalysisResults&>&, llvm::LoopStandardAnalysisResults&, llvm::LPMUpdater&) + 152

https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-expensive/20177/

Would you mind taking a look?

[nikic]

@jroelofs This should be fixed by D108386.

[jroelofs]

Thank you!

llvm/lib/Passes/PassBuilder.cpp

#define FUNCTION_ANALYSIS(NAME, CREATE_PASS)                                   \
   if (Name == "require<" NAME ">" || Name == "invalidate<" NAME ">")           \
     return true;
 #include "PassRegistry.def"
 
   return callbacksAcceptPassName<FunctionPassManager>(Name, Callbacks);
 }
 
 template <typename CallbacksT>
-static bool isLoopPassName(StringRef Name, CallbacksT &Callbacks) {
-  // Explicitly handle pass manager names.
-  if (Name == "loop" || Name == "loop-mssa")
-    return true;
+static bool isLoopPassName(StringRef Name, CallbacksT &Callbacks,
+                           bool &UseMemorySSA) {
+  UseMemorySSA = false;
 
   // Explicitly handle custom-parsed pass names.
   if (parseRepeatPassName(Name))
     return true;
 
+  if (Name == "licm") {
+    UseMemorySSA = true;
+    return true;
+  }
+
 #define LOOP_PASS(NAME, CREATE_PASS)                                           \
   if (Name == NAME)                                                            \
     return true;
 #define LOOP_PASS_WITH_PARAMS(NAME, CLASS, CREATE_PASS, PARSER, PARAMS)        \
   if (checkParametrizedPassName(Name, NAME))                                   \
     return true;
 #define LOOP_ANALYSIS(NAME, CREATE_PASS)                                       \
   if (Name == "require<" NAME ">" || Name == "invalidate<" NAME ">")           \
return make_error<StringError>(
         formatv("invalid pipeline '{0}'", PipelineText).str(),
         inconvertibleErrorCode());
 
   // If the first name isn't at the module layer, wrap the pipeline up
   // automatically.
   StringRef FirstName = Pipeline->front().Name;
 
   if (!isModulePassName(FirstName, ModulePipelineParsingCallbacks)) {
+    bool UseMemorySSA;
     if (isCGSCCPassName(FirstName, CGSCCPipelineParsingCallbacks)) {
       Pipeline = {{"cgscc", std::move(*Pipeline)}};
     } else if (isFunctionPassName(FirstName,
                                   FunctionPipelineParsingCallbacks)) {
       Pipeline = {{"function", std::move(*Pipeline)}};
-    } else if (isLoopPassName(FirstName, LoopPipelineParsingCallbacks)) {
-      Pipeline = {{"function", {{"loop", std::move(*Pipeline)}}}};
+    } else if (isLoopPassName(FirstName, LoopPipelineParsingCallbacks,
+                              UseMemorySSA)) {
+      Pipeline = {{"function", {{UseMemorySSA ? "loop-mssa" : "loop",
+                                 std::move(*Pipeline)}}}};
     } else {
       for (auto &C : TopLevelPipelineParsingCallbacks)
         if (C(MPM, *Pipeline))
           return Error::success();
 
       // Unknown pass or pipeline name!
       auto &InnerPipeline = Pipeline->front().InnerPipeline;
       return make_error<StringError>(