[NewPM] make parsePassPipeline parse adaptor-wrapped user passes

[ychen]

Summary

Currently, when parsing text pipeline, different kinds of passes always
introduce nested pass managers. This makes it impossible to test the
adaptor-wrapped user passes from the text pipeline interface which is needed
by D82344 test cases. This also seems useful in general. See comments above
parsePassPipeline.

The syntax would be like mixing passes of different types, but it is
not the same as inferring the correct pass type and then adding the
matching nested pass managers. Strictly speaking, the resulted pipelines
are different.

[asbirlea]

I think it's great to have a way to specify the exact nesting of managers we want.
Please extend the changes to all tests.

[ychen]

• update test

[ychen]

ping ?

[aeubanks]

I think this makes sense as long as it's not abused and doesn't make things harder to reason about. Should be fine since most (all?) uses are through opt.

llvm/test/Other/pass-pipeline-parsing.ll

182

what about a separate check for
Running pass: FunctionToLoopPassAdaptor<{{.*}}NoOpLoopPass>
and NoOpLoopPass itself?

[aeubanks]

Not sure exactly how to write a test for this, but it'd be nice to test a case where we have a module pass and a function pass both named the same. e.g. in PassRegistry.def there are multiple passes called "print".

[ychen]

llvm/test/Other/pass-pipeline-parsing.ll

182

I'm happy to do that. What is the motivation?

[ychen]

Add a test case for 'print' pass in CGSCC pipeline where 'print' should be a function pass.

[ychen]

In D82698#2145079, @aeubanks wrote:

Not sure exactly how to write a test for this, but it'd be nice to test a case where we have a module pass and a function pass both named the same. e.g. in PassRegistry.def there are multiple passes called "print".

Interesting. I would argue that passes with same name are extremely rare, probably just "print". For "print", it has a version for each pass type except CGSCC which means only CGSCC pipeline would possibly hit this adaptor code path. I've added a test case for this.

[aeubanks]

llvm/test/Other/pass-pipeline-parsing.ll

182

To make sure that the FunctionToLoopPassAdaptor itself isn't skipped for some reason, and the same with the NoOpLoopPass.

[ychen]

llvm/test/Other/pass-pipeline-parsing.ll

182

Is line 191 what you refer to?

[aeubanks]

llvm/test/Other/pass-pipeline-parsing.ll

182

no, that one is a different no-op-loop in -passes='module(no-op-function,no-op-loop,no-op-cgscc,cgscc(no-op-function,no-op-loop),function(no-op-loop))'.

line 191 refers to the very last no-op-loop, I meant the first one. basically I think every adaptor "Running pass" message should be checked.
1 for the first no-op-function (module -> function), 2 for the first no-op-loop (module -> function -> loop), 1 for the first no-op-cgscc (module -> cgscc), 1 for the second no-op-function (cgscc -> function), 2 for the second no-op-loop (cgscc -> function -> loop), and 1 for the last no-op-loop (function -> loop)

[ychen]

llvm/test/Other/pass-pipeline-parsing.ll

182

First no-op-loop is checked by line 182.

Aren't these covered by the existing Running pass:?

Sorry, I'm still confused.

[aeubanks]

llvm/test/Other/pass-pipeline-parsing.ll

182

Ah I think I see what I'm missing. I was expecting each adaptor pass to print "Running pass:" if it were to run the pass it's adapting, but it doesn't. I think that's a good idea since the adaptor may run but for some reason it might not run its pass (e.g. if it decides to skip it for some reason). WDYT? Maybe that's out of scope for this change.

[ychen]

llvm/test/Other/pass-pipeline-parsing.ll

182

I agree. We should fix this o/w, we will see Running pass: for skipped pass. Feel free to submit a patch.

[ychen]

@asbirlea ping?

[aeubanks]

lgtm

[asbirlea]

Thanks, this is good.

llvm/include/llvm/Passes/PassBuilder.h

public:
   ///
   /// And they are equivalent to the following (resp.):
   ///
   ///   module(function(fpass1,fpass2,fpass3))
   ///   module(cgscc(cgpass1,cgpass2,cgpass3))
   ///   module(function(loop(lpass1,lpass2,lpass3)))
   ///
   /// This shortcut is especially useful for debugging and testing small pass
-  /// combinations. Note that these shortcuts don't introduce any other magic.
-  /// If the sequence of passes aren't all the exact same kind of pass, it will
-  /// be an error. You cannot mix different levels implicitly, you must
-  /// explicitly form a pass manager in which to nest passes.
+  /// combinations.
+  ///
+  /// The sequence of passes aren't necessarily the exact same kind of pass.
+  /// You can mix different levels implicitly if adaptor passes are defined to
+  /// make them work. For example,
+  ///
+  ///   mpass1,fpass1,fpass2,mpass2,lpass1
+  ///
+  /// This pipeline uses only one pass manager: the top-level module manager.
+  /// fpass1,fpass2 and lpass1 are added into the the top-level module manager
+  /// using only adaptor passes. No nested function/loop pass managers are
+  /// added. The purpose is to allow easy pass testing when the user
+  /// specifically want the pass to run under a adaptor directly. This is
+  /// preferred when a pipeline is largely of one type, but one or just a few
+  /// passes are of different types(See PassBuilder.cpp for examples).
   Error parsePassPipeline(ModulePassManager &MPM, StringRef PipelineText,
                           bool VerifyEachPass = true,
                           bool DebugLogging = false);
 
   /// {{@ Parse a textual pass pipeline description into a specific PassManager
   ///
   /// Automatic deduction of an appropriate pass manager stack is not supported.
   /// For example, to insert a loop pass 'lpass' into a FunctionPassManager,

llvm/lib/Passes/PassBuilder.cpp

MPM.addPass(                                                               \
             std::remove_reference<decltype(CREATE_PASS)>::type, Module>());    \
     return Error::success();                                                   \
   }                                                                            \
   if (Name == "invalidate<" NAME ">") {                                        \
     MPM.addPass(InvalidateAnalysisPass<                                        \
                 std::remove_reference<decltype(CREATE_PASS)>::type>());        \
     return Error::success();                                                   \
   }
+#define CGSCC_PASS(NAME, CREATE_PASS)                                          \
+  if (Name == NAME) {                                                          \
+    MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(CREATE_PASS));         \
+    return Error::success();                                                   \
+  }
+#define FUNCTION_PASS(NAME, CREATE_PASS)                                       \
+  if (Name == NAME) {                                                          \
+    MPM.addPass(createModuleToFunctionPassAdaptor(CREATE_PASS));               \
+    return Error::success();                                                   \
+  }
+#define FUNCTION_PASS_WITH_PARAMS(NAME, CREATE_PASS, PARSER)                   \
+  if (checkParametrizedPassName(Name, NAME)) {                                 \
+    auto Params = parsePassParameters(PARSER, Name, NAME);                     \
+    if (!Params)                                                               \
+      return Params.takeError();                                               \
+    MPM.addPass(createModuleToFunctionPassAdaptor(CREATE_PASS(Params.get()))); \
+    return Error::success();                                                   \
+  }
+#define LOOP_PASS(NAME, CREATE_PASS)                                           \
+  if (Name == NAME) {                                                          \
+    MPM.addPass(createModuleToFunctionPassAdaptor(                             \
+        createFunctionToLoopPassAdaptor(CREATE_PASS, false, DebugLogging)));   \
+    return Error::success();                                                   \
+  }
+#define LOOP_PASS_WITH_PARAMS(NAME, CREATE_PASS, PARSER)                       \
+  if (checkParametrizedPassName(Name, NAME)) {                                 \
+    auto Params = parsePassParameters(PARSER, Name, NAME);                     \
+    if (!Params)                                                               \
+      return Params.takeError();                                               \
+    MPM.addPass(                                                               \
+        createModuleToFunctionPassAdaptor(createFunctionToLoopPassAdaptor(     \
+            CREATE_PASS(Params.get()), false, DebugLogging)));                 \
+    return Error::success();                                                   \
+  }
 #include "PassRegistry.def"
 
   for (auto &C : ModulePipelineParsingCallbacks)
     if (C(Name, MPM, InnerPipeline))
       return Error::success();
   return make_error<StringError>(
       formatv("unknown module pass '{0}'", Name).str(),
       inconvertibleErrorCode());
CGPM.addPass(RequireAnalysisPass<                                          \
                  CGSCCUpdateResult &>());                                      \
     return Error::success();                                                   \
   }                                                                            \
   if (Name == "invalidate<" NAME ">") {                                        \
     CGPM.addPass(InvalidateAnalysisPass<                                       \
                  std::remove_reference<decltype(CREATE_PASS)>::type>());       \
     return Error::success();                                                   \
   }
+#define FUNCTION_PASS(NAME, CREATE_PASS)                                       \
+  if (Name == NAME) {                                                          \
+    CGPM.addPass(createCGSCCToFunctionPassAdaptor(CREATE_PASS));               \
+    return Error::success();                                                   \
+  }
+#define FUNCTION_PASS_WITH_PARAMS(NAME, CREATE_PASS, PARSER)                   \
+  if (checkParametrizedPassName(Name, NAME)) {                                 \
+    auto Params = parsePassParameters(PARSER, Name, NAME);                     \
+    if (!Params)                                                               \
+      return Params.takeError();                                               \
+    CGPM.addPass(createCGSCCToFunctionPassAdaptor(CREATE_PASS(Params.get()))); \
+    return Error::success();                                                   \
+  }
+#define LOOP_PASS(NAME, CREATE_PASS)                                           \
+  if (Name == NAME) {                                                          \
+    CGPM.addPass(createCGSCCToFunctionPassAdaptor(                             \
+        createFunctionToLoopPassAdaptor(CREATE_PASS, false, DebugLogging)));   \
+    return Error::success();                                                   \
+  }
+#define LOOP_PASS_WITH_PARAMS(NAME, CREATE_PASS, PARSER)                       \
+  if (checkParametrizedPassName(Name, NAME)) {                                 \
+    auto Params = parsePassParameters(PARSER, Name, NAME);                     \
+    if (!Params)                                                               \
+      return Params.takeError();                                               \
+    CGPM.addPass(                                                              \
+        createCGSCCToFunctionPassAdaptor(createFunctionToLoopPassAdaptor(      \
+            CREATE_PASS(Params.get()), false, DebugLogging)));                 \
+    return Error::success();                                                   \
+  }
 #include "PassRegistry.def"
 
   for (auto &C : CGSCCPipelineParsingCallbacks)
     if (C(Name, CGPM, InnerPipeline))
       return Error::success();
   return make_error<StringError>(
       formatv("unknown cgscc pass '{0}'", Name).str(),
       inconvertibleErrorCode());
FPM.addPass(                                                               \
             std::remove_reference<decltype(CREATE_PASS)>::type, Function>());  \
     return Error::success();                                                   \
   }                                                                            \
   if (Name == "invalidate<" NAME ">") {                                        \
     FPM.addPass(InvalidateAnalysisPass<                                        \
                 std::remove_reference<decltype(CREATE_PASS)>::type>());        \
     return Error::success();                                                   \
   }
+// FIXME: UseMemorySSA is set to false. Maybe we could do things like:
+//        bool UseMemorySSA = !("canon-freeze" || "loop-predication" ||
+//                              "guard-widening");
+//        The risk is that it may become obsolete if we're not careful.
+#define LOOP_PASS(NAME, CREATE_PASS)                                           \
+  if (Name == NAME) {                                                          \
+    FPM.addPass(                                                               \
+        createFunctionToLoopPassAdaptor(CREATE_PASS, false, DebugLogging));    \
+    return Error::success();                                                   \
+  }
+#define LOOP_PASS_WITH_PARAMS(NAME, CREATE_PASS, PARSER)                       \
+  if (checkParametrizedPassName(Name, NAME)) {                                 \
+    auto Params = parsePassParameters(PARSER, Name, NAME);                     \
+    if (!Params)                                                               \
+      return Params.takeError();                                               \
+    FPM.addPass(createFunctionToLoopPassAdaptor(CREATE_PASS(Params.get()),     \
+                                                false, DebugLogging));         \
+    return Error::success();                                                   \
+  }
 #include "PassRegistry.def"
 
   for (auto &C : FunctionPipelineParsingCallbacks)
     if (C(Name, FPM, InnerPipeline))
       return Error::success();
   return make_error<StringError>(
       formatv("unknown function pass '{0}'", Name).str(),
       inconvertibleErrorCode());

llvm/test/Other/pass-pipeline-parsing.ll

 ; CHECK-NESTED-FP-LP: Starting llvm::Function pass manager run
 ; CHECK-NESTED-FP-LP: Running pass: FunctionToLoopPassAdaptor
 ; CHECK-NESTED-FP-LP: Starting Loop pass manager run
 ; CHECK-NESTED-FP-LP: Running pass: NoOpLoopPass
 ; CHECK-NESTED-FP-LP: Finished Loop pass manager run
 ; CHECK-NESTED-FP-LP: Finished llvm::Function pass manager run
 ; CHECK-NESTED-FP-LP: Finished llvm::Module pass manager run
 
+; RUN: opt -disable-output -debug-pass-manager \
+; RUN:     -passes='module(no-op-function,no-op-loop,no-op-cgscc,cgscc(no-op-function,no-op-loop),function(no-op-loop))' %s 2>&1 \
+; RUN:     | FileCheck %s --check-prefix=CHECK-ADAPTORS
+; CHECK-ADAPTORS: Starting llvm::Module pass manager run
+; CHECK-ADAPTORS: Starting llvm::Module pass manager run
+; CHECK-ADAPTORS: Running pass: ModuleToFunctionPassAdaptor<{{.*}}NoOpFunctionPass>
+; CHECK-ADAPTORS: Running pass: ModuleToFunctionPassAdaptor<{{.*}}FunctionToLoopPassAdaptor<{{.*}}NoOpLoopPass>{{.*}}>
+; CHECK-ADAPTORS: Running pass: ModuleToPostOrderCGSCCPassAdaptor<{{.*}}NoOpCGSCCPass>
+; CHECK-ADAPTORS: Running pass: ModuleToPostOrderCGSCCPassAdaptor<{{.*}}LazyCallGraph{{.*}}>
+; CHECK-ADAPTORS: Starting CGSCC pass manager run
+; CHECK-ADAPTORS: Running pass: CGSCCToFunctionPassAdaptor<{{.*}}NoOpFunctionPass>
+; CHECK-ADAPTORS: Running pass: CGSCCToFunctionPassAdaptor<{{.*}}FunctionToLoopPassAdaptor<{{.*}}NoOpLoopPass>{{.*}}>
+; CHECK-ADAPTORS: Finished CGSCC pass manager run
+; CHECK-ADAPTORS: Running pass: ModuleToFunctionPassAdaptor<{{.*}}PassManager{{.*}}>
+; CHECK-ADAPTORS: Starting llvm::Function pass manager run
+; CHECK-ADAPTORS: Running pass: FunctionToLoopPassAdaptor<{{.*}}NoOpLoopPass>
+; CHECK-ADAPTORS: Finished llvm::Function pass manager run
+; CHECK-ADAPTORS: Finished llvm::Module pass manager run
+; CHECK-ADAPTORS: Finished llvm::Module pass manager run
+
+; RUN: opt -disable-output -debug-pass-manager \
+; RUN:     -passes='cgscc(print)' %s 2>&1 \
+; RUN:     | FileCheck %s --check-prefix=CHECK-PRINT-IN-CGSCC
+; CHECK-PRINT-IN-CGSCC: Starting llvm::Module pass manager run
+; CHECK-PRINT-IN-CGSCC: Running pass: ModuleToPostOrderCGSCCPassAdaptor<{{.*}}LazyCallGraph{{.*}}>
+; CHECK-PRINT-IN-CGSCC: Starting CGSCC pass manager run
+; CHECK-PRINT-IN-CGSCC: Running pass: CGSCCToFunctionPassAdaptor<{{.*}}PrintFunctionPass>
+; CHECK-PRINT-IN-CGSCC: Finished CGSCC pass manager run
+; CHECK-PRINT-IN-CGSCC: Running pass: VerifierPass
+; CHECK-PRINT-IN-CGSCC: Finished llvm::Module pass manager run
+
 ; RUN: not opt -disable-output -debug-pass-manager \
 ; RUN:     -passes='function(no-op-function)function(no-op-function)' %s 2>&1 \
 ; RUN:     | FileCheck %s --check-prefix=CHECK-MISSING-COMMA1
 ; CHECK-MISSING-COMMA1: invalid pipeline 'function(no-op-function)function(no-op-function)'
 
 ; RUN: not opt -disable-output -debug-pass-manager \
 ; RUN:     -passes='function()' %s 2>&1 \
 ; RUN:     | FileCheck %s --check-prefix=CHECK-EMPTY-INNER-PIPELINE