[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

191

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

191

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

191

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

191

Is line 191 what you refer to?

[aeubanks]

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

191

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

191

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

191

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

191

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

 //===- Parsing, selection, and construction of pass pipelines --*- C++ -*--===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 /// \file
 ///
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 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();                                                   \
+  }
+#define CGSCC_PASS(NAME, CREATE_PASS)                                          \
+  if (Name == NAME) {                                                          \
+    MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(CREATE_PASS));         \
+    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/Analysis/GlobalsModRef/aliastest.ll

 ; RUN: opt < %s -basicaa -globals-aa -gvn -S -enable-unsafe-globalsmodref-alias-results | FileCheck %s
-; RUN: opt < %s -aa-pipeline=basic-aa,globals-aa -passes="require<globals-aa>,function(gvn)" -S -enable-unsafe-globalsmodref-alias-results | FileCheck %s
+; RUN: opt < %s -aa-pipeline=basic-aa,globals-aa -passes="require<globals-aa>,gvn" -S -enable-unsafe-globalsmodref-alias-results | FileCheck %s
 ;
 ; Note that this test relies on an unsafe feature of GlobalsModRef. While this
 ; test is correct and safe, GMR's technique for handling this isn't generally.
 
 @X = internal global i32 4    ; <i32*> [#uses=1]
 
 define i32 @test(i32* %P) {
 ; CHECK:      @test
 ; CHECK-NEXT: store i32 7, i32* %P
 ; CHECK-NEXT: store i32 12, i32* @X
 ; CHECK-NEXT: ret i32 7
   store i32 7, i32* %P
   store i32 12, i32* @X
   %V = load i32, i32* %P    ; <i32> [#uses=1]
   ret i32 %V
 }

llvm/test/Other/cgscc-disconnected-invalidation.ll

 ; Test that patterns of transformations which disconnect a region of the call
 ; graph mid-traversal and then invalidate it function correctly.
 ;
-; RUN: opt -S -passes='cgscc(inline,function(simplify-cfg))' < %s | FileCheck %s
+; RUN: opt -S -passes='cgscc(inline,simplify-cfg)' < %s | FileCheck %s
 
 define internal void @test_scc_internal(i1 %flag) {
 ; CHECK-NOT: @test_scc_internal
 entry:
   br i1 %flag, label %then, label %else
 
 then:
   call void @test_scc_internal(i1 false)

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

 ; CHECK-MIXED-FP-AND-MP: Running pass: ModuleToFunctionPassAdaptor
 ; CHECK-MIXED-FP-AND-MP: Starting llvm::Function pass manager run
 ; CHECK-MIXED-FP-AND-MP: Running pass: NoOpFunctionPass
 ; CHECK-MIXED-FP-AND-MP: Running pass: NoOpFunctionPass
 ; CHECK-MIXED-FP-AND-MP: Finished llvm::Function pass manager run
 ; CHECK-MIXED-FP-AND-MP: Running pass: NoOpModulePass
 ; CHECK-MIXED-FP-AND-MP: Finished llvm::Module pass manager run
 
+; RUN: opt -disable-output -debug-pass-manager \
+; RUN:     -passes='no-op-module,no-op-function,no-op-module' %s 2>&1 \
+; RUN:     | FileCheck %s --check-prefix=CHECK-MIXED-FP-AND-MP2
+; CHECK-MIXED-FP-AND-MP2: Starting llvm::Module pass manager run
+; CHECK-MIXED-FP-AND-MP2: Running pass: NoOpModulePass
+; CHECK-MIXED-FP-AND-MP2: Running pass: ModuleToFunctionPassAdaptor{{.*}}NoOpFunctionPass
+; CHECK-MIXED-FP-AND-MP2: Running pass: NoOpModulePass
+; CHECK-MIXED-FP-AND-MP2: Finished llvm::Module pass manager run
+
 ; RUN: not opt -disable-output -debug-pass-manager \
 ; RUN:     -passes='no-op-module)' %s 2>&1 \
 ; RUN:     | FileCheck %s --check-prefix=CHECK-UNBALANCED1
 ; CHECK-UNBALANCED1: invalid pipeline 'no-op-module)'
 
 ; RUN: not opt -disable-output -debug-pass-manager \
 ; RUN:     -passes='module(no-op-module))' %s 2>&1 \
 ; RUN:     | FileCheck %s --check-prefix=CHECK-UNBALANCED2

 ; CHECK-NESTED-FP-LP: 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
 ; CHECK-EMPTY-INNER-PIPELINE: unknown function pass ''
 
 ; RUN: not opt -disable-output -debug-pass-manager \
 ; RUN:     -passes='no-op-module(no-op-module,whatever)' %s 2>&1 \
 ; RUN:     | FileCheck %s --check-prefix=CHECK-PIPELINE-ON-MODULE-PASS
 ; CHECK-PIPELINE-ON-MODULE-PASS: invalid use of 'no-op-module' pass as module pipeline
 

llvm/test/Transforms/Inline/2007-04-15-InlineEH.ll

 ; RUN: opt < %s -inline -S | FileCheck %s
-; RUN: opt < %s -passes='cgscc(inline)' -S | FileCheck %s
+; RUN: opt < %s -passes='inline' -S | FileCheck %s
 ; PR1335
 
 target triple = "i686-pc-linux-gnu"
 
 declare i32 @__gxx_personality_v0(...)
 
 declare void @a()