[NewPM] Provide method to run all pipeline callbacks, used for -O0

[aeubanks]

Summary

Some targets may add required passes via
TargetMachine::registerPassBuilderCallbacks(). We need to run those even
under -O0. As an example, BPFTargetMachine adds
BPFAbstractMemberAccessPass, a required pass.

This also allows us to clean up BackendUtil.cpp (and out-of-tree Rust
usage of the NPM) by allowing us to share added passes like coroutines
and sanitizers between -O0 and other optimization levels.

Since callbacks may end up not adding passes, we need to check if the
pass managers are empty before adding them, so PassManager now has an
isEmpty() function. For example, polly adds callbacks but doesn't always
add passes in those callbacks, so this is necessary to keep
-debug-pass-manager tests' output from changing depending on if polly is
enabled or not.

Tests are a continuation of those added in
https://reviews.llvm.org/D89083.

[aeubanks]

rebase
add assert for O0

[nikic]

clang/lib/CodeGen/BackendUtil.cpp

1444

It should be possible to simplify this function a lot now. It currently duplicates nearly all logic (for sanitizers etc) between O0 and O1+. Or is that planned for a followup?

[aeubanks]

clang/lib/CodeGen/BackendUtil.cpp

1444

Yup, I have that ready to go after this :)

[ychen]

IIUC, this is the NPM version of EP_EarlyAsPossible in legacy PM for O0, right? Between the choice of (1) [this patch and ] reusing all existing EP callbacks and letting optnone filtering out non-required passes (which I think does not work for -disable-O0-optnone) and (2) a separate EP callback, say, O0EPCallbacks, I would vote for the latter since it is more explicit and does exactly what we expect it to do. The cost is that we also need to add passes to O0EPCallbacks separately which I think is justified since required passes should be rare.

[aeubanks]

only run passes in PipelineStartEPCallbacks

[aeubanks]

In D89158#2357850, @ychen wrote:

IIUC, this is the NPM version of EP_EarlyAsPossible in legacy PM for O0, right? Between the choice of (1) [this patch and ] reusing all existing EP callbacks and letting optnone filtering out non-required passes (which I think does not work for -disable-O0-optnone) and (2) a separate EP callback, say, O0EPCallbacks, I would vote for the latter since it is more explicit and does exactly what we expect it to do. The cost is that we also need to add passes to O0EPCallbacks separately which I think is justified since required passes should be rare.

I took a look at the legacy PM, it adds EP_EarlyAsPossible and EP_EnabledOnOptLevel0 for -O0. I changed this to match EP_EarlyAsPossible, then we can manually add passes corresponding to EP_EnabledOnOptLevel0. EP_EnabledOnOptLevel0 was only used for sanitizers and coroutines, which are already handled. So this should take care of target specific required IR passes, since those are typically run as EP_EarlyAsPossible in the legacy PM.
(I'll update the documentation once we've agreed)

[ychen]

llvm/include/llvm/Passes/PassBuilder.h

609

invokeO0EPCallbacks? I saw there is PassBuilder::invokePeepholeEPCallbacks.

llvm/lib/Passes/PassBuilder.cpp

1667

What I have in mind is a newly added O0EPCallbacks field in PassBuilder class. Then we can keep existing EPCallbacks (including PipelineStartEPCallbacks) for >O0 optimization pipeline. Yeah, then you need to add related passes to O0EPCallbacks (for BPF in this case).

[aeubanks]

run -> invoke

[aeubanks]

llvm/lib/Passes/PassBuilder.cpp

1667

It's a tradeoff between having to specify required passes in both O0EPCallbacks and PipelineStartEPCallbacks which is repetitive, versus making all callbacks in PipelineStartEPCallbacks run at -O0, meaning even optional passes in PipelineStartEPCallbacks will run at -O0 (may be skipped via optnone).

The legacy PM chooses the first, and I'm inclined to keep it that way just for consistency.

If we did go down the second route, we could just have a second TargetMachine API like TargetMachine::addO0Passes() which directly adds passes to a ModulePassManager.

[asbirlea]

llvm/lib/Passes/PassBuilder.cpp

1667

This seems more in line with the LPM behavior for O0.
If BPF needs those passes even for O0 they should be added as such.

[ychen]

llvm/lib/Passes/PassBuilder.cpp

1667

It's a tradeoff between having to specify required passes in both O0EPCallbacks and PipelineStartEPCallbacks which is repetitive, versus making all callbacks in PipelineStartEPCallbacks run at -O0, meaning even optional passes in PipelineStartEPCallbacks will run at -O0 (may be skipped via optnone).

Indeed.

The legacy PM chooses the first, and I'm inclined to keep it that way just for consistency.

I'm lost here. Do you mean to say the second?

If we did go down the second route, we could just have a second TargetMachine API like TargetMachine::addO0Passes() which directly adds passes to a ModulePassManager.

Do you mean to say the first? This is not much different from adding to O0EPCallbacks in TargetMachine::registerPassBuilderCallbacks.

My proposal to have both O0EPCallbacks and PipelineStartEPCallbacks is that I'm not sure why we want to run all EP callbacks at O0. Do we have use cases for that?

[aeubanks]

llvm/lib/Passes/PassBuilder.cpp

1667

I am so sorry, yes I got them flipped.

As of the current patch, we're not running all callbacks, just PipelineStartEPCallbacks which is in line with the legacy PM.

[ychen]

llvm/lib/Passes/PassBuilder.cpp

1667

I am so sorry, yes I got them flipped.

As of the current patch, we're not running all callbacks, just PipelineStartEPCallbacks which is in line with the legacy PM.

Definitely agree that this is in line with legacy PM. I don't feel strongly to go either way. Since @echristo is actively tuning O0, it would be helpful to know his opinion.

[nikic]

If you are not invoking all the needed callbacks in invokeRegisteredO0EPCallbacks(), I would suggest to instead provide a set of methods like invokePipelineStartEPCallbacks(), invokeOptimizerLastEPCallbacks() that allow the consumer to chose which callbacks to invoke. (Just those two would be sufficient for my purposes.)

I'd really like to avoid this dance: https://github.com/rust-lang/rust/blob/c71248b70870960af9993de4f31d3cba9bbce7e8/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp#L796 Where you need to separately collect all callbacks yourself, because the PassBuilder neither makes the necessary members public, nor provides a means to invoke the callbacks explicitly.

[aeubanks]

Looking at BackendUtil.cpp in Clang as well as the Rust code, I'm back to thinking that we should provide a way to to all callbacks. Then in the case of passes added via TargetMachine, we should extend TargetMachine::registerPassBuilderCallbacks to also take a PassBuilder::OptimizationLevel. Then targets can skip adding optional optimization passes at -O0. Does that make sense? It would allow us to clean up Clang and Rust.

In fact I see in AMDGPUTargetMachine::adjustPassManager a check for an optimization level, so some targets are already doing this, although it looks like it's more for codegen and not IR passes.

[aeubanks]

I'm more convinced now that my initial direction was the cleanest way. It would simplify users of PassBuilder quite a bit.

I don't want to include PassManager.h into TargetMachine.h just to get PassBuilder::OptimizationLevel, so either we could factor out OptimizationLevel into its own header file so that we can pass an OptimizationLevel to TargetMachine::registerPassBuilderCallbacks, or we could just pass a OnlyAddRequiredPasses bool to TargetMachine::registerPassBuilderCallbacks. But either way, that can happen in a future change when the need arises (such as when we implement AMDGPUTargetMachine::registerPassBuilderCallbacks).

[echristo]

In D89158#2363220, @aeubanks wrote:

Looking at BackendUtil.cpp in Clang as well as the Rust code, I'm back to thinking that we should provide a way to to all callbacks. Then in the case of passes added via TargetMachine, we should extend TargetMachine::registerPassBuilderCallbacks to also take a PassBuilder::OptimizationLevel. Then targets can skip adding optional optimization passes at -O0. Does that make sense? It would allow us to clean up Clang and Rust.

In fact I see in AMDGPUTargetMachine::adjustPassManager a check for an optimization level, so some targets are already doing this, although it looks like it's more for codegen and not IR passes.

This sounds great. I'd love to see this.

-eric

[aeubanks]

revert back to running all callbacks

[aeubanks]

https://reviews.llvm.org/D90486 for adding OptimizationLevel to registerPassBuilderCallbacks(), although this one should land first since that one needs to update PassBuilder::runRegisteredEPCallbacks() to call TargetMachine::registerPassBuilderCallbacks()

[nathanchance]

$ cmake \
-G Ninja \
-DCMAKE_BUILD_TYPE=Release \
-DCMAKE_C_COMPILER=$(command -v clang) \
-DCMAKE_CXX_COMPILER=$(command -v clang++) \
-DLLVM_CCACHE_BUILD=ON \
-DLLVM_ENABLE_PROJECTS="clang;polly" \
-DLLVM_TARGETS_TO_BUILD=X86 \
-DLLVM_USE_LINKER=$(command -v ld.lld) \
../llvm

$ ninja check-clang
...
[0/1] Running the Clang regression tests
-- Testing: 26861 tests, 64 workers --
Testing: llvm-lit: /tmp/llvm-project/llvm/utils/lit/lit/llvm/config.py:347: note: using clang: /tmp/llvm-project/build/bin/clang
 0.. 10
FAIL: Clang :: CodeGen/lto-newpm-pipeline.c (3731 of 26861)
******************** TEST 'Clang :: CodeGen/lto-newpm-pipeline.c' FAILED ********************
Script:
--
: 'RUN: at line 3';   /tmp/llvm-project/build/bin/clang -cc1 -internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include -nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null -fexperimental-new-pass-manager -fdebug-pass-manager -flto=full -O0 /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | /tmp/llvm-project/build/bin/FileCheck /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c    -check-prefix=CHECK-FULL-O0
: 'RUN: at line 5';   /tmp/llvm-project/build/bin/clang -cc1 -internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include -nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null -fexperimental-new-pass-manager -fdebug-pass-manager -flto=thin -O0 /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | /tmp/llvm-project/build/bin/FileCheck /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c    -check-prefix=CHECK-THIN-O0
: 'RUN: at line 7';   /tmp/llvm-project/build/bin/clang -cc1 -internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include -nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null -fexperimental-new-pass-manager -fdebug-pass-manager -flto=full -O1 /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | /tmp/llvm-project/build/bin/FileCheck /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c    -check-prefix=CHECK-FULL-OPTIMIZED
: 'RUN: at line 9';   /tmp/llvm-project/build/bin/clang -cc1 -internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include -nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null -fexperimental-new-pass-manager -fdebug-pass-manager -flto=thin -O1 /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | /tmp/llvm-project/build/bin/FileCheck /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c    -check-prefix=CHECK-THIN-OPTIMIZED
: 'RUN: at line 11';   /tmp/llvm-project/build/bin/clang -cc1 -internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include -nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null -fexperimental-new-pass-manager -fdebug-pass-manager -flto=full -O2 /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | /tmp/llvm-project/build/bin/FileCheck /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c    -check-prefix=CHECK-FULL-OPTIMIZED
: 'RUN: at line 13';   /tmp/llvm-project/build/bin/clang -cc1 -internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include -nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null -fexperimental-new-pass-manager -fdebug-pass-manager -flto=thin -O2 /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | /tmp/llvm-project/build/bin/FileCheck /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c    -check-prefix=CHECK-THIN-OPTIMIZED
: 'RUN: at line 15';   /tmp/llvm-project/build/bin/clang -cc1 -internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include -nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null -fexperimental-new-pass-manager -fdebug-pass-manager -flto=full -O3 /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | /tmp/llvm-project/build/bin/FileCheck /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c    -check-prefix=CHECK-FULL-OPTIMIZED
: 'RUN: at line 17';   /tmp/llvm-project/build/bin/clang -cc1 -internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include -nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null -fexperimental-new-pass-manager -fdebug-pass-manager -flto=thin -O3 /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | /tmp/llvm-project/build/bin/FileCheck /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c    -check-prefix=CHECK-THIN-OPTIMIZED
: 'RUN: at line 19';   /tmp/llvm-project/build/bin/clang -cc1 -internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include -nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null -fexperimental-new-pass-manager -fdebug-pass-manager -flto=full -Os /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | /tmp/llvm-project/build/bin/FileCheck /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c    -check-prefix=CHECK-FULL-OPTIMIZED
: 'RUN: at line 21';   /tmp/llvm-project/build/bin/clang -cc1 -internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include -nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null -fexperimental-new-pass-manager -fdebug-pass-manager -flto=thin -Os /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | /tmp/llvm-project/build/bin/FileCheck /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c    -check-prefix=CHECK-THIN-OPTIMIZED
: 'RUN: at line 23';   /tmp/llvm-project/build/bin/clang -cc1 -internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include -nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null -fexperimental-new-pass-manager -fdebug-pass-manager -flto=full -Oz /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | /tmp/llvm-project/build/bin/FileCheck /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c    -check-prefix=CHECK-FULL-OPTIMIZED
: 'RUN: at line 25';   /tmp/llvm-project/build/bin/clang -cc1 -internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include -nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null -fexperimental-new-pass-manager -fdebug-pass-manager -flto=thin -Oz /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | /tmp/llvm-project/build/bin/FileCheck /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c    -check-prefix=CHECK-THIN-OPTIMIZED
--
Exit Code: 1

Command Output (stderr):
--
/tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c:34:24: error: CHECK-FULL-O0-NEXT: is not on the line after the previous match
// CHECK-FULL-O0-NEXT: Running pass: BitcodeWriterPass
                       ^
<stdin>:9:1: note: 'next' match was here
Running pass: BitcodeWriterPass on /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c
^
<stdin>:6:33: note: previous match ended here
Running pass: NameAnonGlobalPass on /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c
                                ^
<stdin>:7:1: note: non-matching line after previous match is here
Starting llvm::Function pass manager run.
^

Input file: <stdin>
Check file: /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c

-dump-input=help explains the following input dump.

Input was:
<<<<<<
         1: Starting llvm::Module pass manager run.
         2: Running pass: AlwaysInlinerPass on /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c
         3: Running analysis: InnerAnalysisManagerProxy<llvm::FunctionAnalysisManager, llvm::Module> on /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c
         4: Running analysis: ProfileSummaryAnalysis on /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c
         5: Running pass: CanonicalizeAliasesPass on /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c
         6: Running pass: NameAnonGlobalPass on /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c
         7: Starting llvm::Function pass manager run.
         8: Finished llvm::Function pass manager run.
         9: Running pass: BitcodeWriterPass on /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c
next:34     !~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                              error: match on wrong line
        10: Running analysis: ModuleSummaryIndexAnalysis on /tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c
        11: Running analysis: BlockFrequencyAnalysis on Foo
        12: Running analysis: BranchProbabilityAnalysis on Foo
        13: Running analysis: LoopAnalysis on Foo
        14: Running analysis: DominatorTreeAnalysis on Foo
        15: Running analysis: TargetLibraryAnalysis on Foo
        16: Running analysis: PostDominatorTreeAnalysis on Foo
        17: Finished llvm::Module pass manager run.
>>>>>>

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
********************
Failed Tests (1):
  Clang :: CodeGen/lto-newpm-pipeline.c


Testing Time: 71.92s
  Unsupported      :   756
  Passed           : 26079
  Expectedly Failed:    25
  Failed           :     1
FAILED: tools/clang/test/CMakeFiles/check-clang 
cd /tmp/llvm-project/build/tools/clang/test && /usr/bin/python3.8 /tmp/llvm-project/build/./bin/llvm-lit -sv --param USE_Z3_SOLVER=0 /tmp/llvm-project/build/tools/clang/test
ninja: build stopped: subcommand failed.

[aeubanks]

Sorry about that, https://reviews.llvm.org/D91019 should fix it.

[aeubanks]

need to fix new-pass-manager.ll when polly is enabled

[aeubanks]

check if pass manager is empty before adding

[Meinersbur]

Did you consider just removing the -NEXT suffix? Polly might not be the only case, other statically linked pass-plugins may also insert themselves at extension points.

@serge-sans-paille might have some input as well.

[aeubanks]

Are there passes that should be running at -O0 that aren't target specific? Otherwise the callbacks should only add passes if the optimization level isn't -O0.

[Meinersbur]

In D89158#2384156, @aeubanks wrote:

Are there passes that should be running at -O0 that aren't target specific? Otherwise the callbacks should only add passes if the optimization level isn't -O0.

At least for the legacy pass manager, there is an EP_EnabledOnOptLevel0 insertion point, that pass plugins can use to insert passes at -O0.

[aeubanks]

In D89158#2384337, @Meinersbur wrote:

In D89158#2384156, @aeubanks wrote:

Are there passes that should be running at -O0 that aren't target specific? Otherwise the callbacks should only add passes if the optimization level isn't -O0.

At least for the legacy pass manager, there is an EP_EnabledOnOptLevel0 insertion point, that pass plugins can use to insert passes at -O0.

That should be obsolete by the OptimizationLevel parameter in the callbacks. The callbacks can inspect the OptimizationLevel to see if it's O0 or not when deciding to add a pass to the pipeline.

[Meinersbur]

My point is that it is possible for a pass plugin (such as llvm/examples/Bye) that can add themselves to the -O0 pipeline. If those plugins are linked statically, it will break tests that assume that no passes are added at those extension points.

I don't quite understand why Polly breaks anything: Unlike the "Bye" pass, it only adds something to the pass pipeline when -polly is passed to the command line. If it helps, I could change Polly to not even add a callback unless enabled. However, this does not solve the general pass plugin problem.

[aeubanks]

With this change in its current state, it's ok to add callbacks, as long as they don't add passes at -O0.
Polly adds some callbacks. When I first submitted this, polly would add some callbacks to VectorizerStartEPCallbacks. Those callbacks didn't actually add any passes (I didn't investigate that, but it makes sense at -O0). However, the previous logic would add a FunctionPassManager if there were any callbacks, so we had an extra empty FunctionPassManager being run, which changed the output of -debug-pass-manager. Now I've changed it so that we check if the FunctionPassManager actually has any passes added to it before adding it to the ModulePassManager.

So previously it was

if (!VectorizerStartEPCallbacks.empty()) {
  FunctionPassManager FPM(DebugLogging);
  for (auto &C : VectorizerStartEPCallbacks)
    C(FPM, Level);
  MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
}

Now it is

if (!VectorizerStartEPCallbacks.empty()) {
  FunctionPassManager FPM(DebugLogging);
  for (auto &C : VectorizerStartEPCallbacks)
    C(FPM, Level);
  if (!FPM.isEmpty())
    MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
}

which should take care of added registered callbacks that don't actually add anything at -O0.

[Meinersbur]

Could you try configuring LLVM with -DLLVM_BYE_LINK_INTO_TOOLS=ON? I assume the test will break because it unconditionally adds a FunctionPass.

[aeubanks]

fix tests when Bye plugin is present

[aeubanks]

In D89158#2388991, @Meinersbur wrote:

Could you try configuring LLVM with -DLLVM_BYE_LINK_INTO_TOOLS=ON? I assume the test will break because it unconditionally adds a FunctionPass.

Thanks, I wasn't aware of this. Looks like other opt -passes='default<O#>' tests also look at if it present or not.

The following already fail with -DLLVM_BYE_LINK_INTO_TOOLS=ON

LLVM :: Other/opt-O0-pipeline.ll
LLVM :: Other/opt-O2-pipeline.ll
LLVM :: Other/opt-O3-pipeline-enable-matrix.ll
LLVM :: Other/opt-O3-pipeline.ll
LLVM :: Other/opt-Os-pipeline.ll

these are all legacy PM related.

[asbirlea]

AFAICT all issues were addressed.
@Meinersbur: are there any concerns left with this patch?

[aeubanks]

forgot to amend commit with test updates

[Meinersbur]

In D89158#2389520, @aeubanks wrote:

Thanks, I wasn't aware of this. Looks like other opt -passes='default<O#>' tests also look at if it present or not.

I'd prefer a solution that is agnostic to which pass plugins are compiled statically; however, this is also how Bye pass tests itself whether it us added to the pass pipeline too.

The following already fail with -DLLVM_BYE_LINK_INTO_TOOLS=ON

LLVM :: Other/opt-O0-pipeline.ll
LLVM :: Other/opt-O2-pipeline.ll
LLVM :: Other/opt-O3-pipeline-enable-matrix.ll
LLVM :: Other/opt-O3-pipeline.ll
LLVM :: Other/opt-Os-pipeline.ll

these are all legacy PM related.

Probably a consequence of not being enabled by default. Tests do not get updated.

clang/lib/CodeGen/BackendUtil.cpp

if (DwoOS)
     DwoOS->keep();
 }
 
 static PassBuilder::OptimizationLevel mapToLevel(const CodeGenOptions &Opts) {
   switch (Opts.OptimizationLevel) {
   default:
     llvm_unreachable("Invalid optimization level!");
 
+  case 0:
+    return PassBuilder::OptimizationLevel::O0;
+
   case 1:
     return PassBuilder::OptimizationLevel::O1;
 
   case 2:
     switch (Opts.OptimizeSize) {
     default:
       llvm_unreachable("Invalid optimization level for size!");
 
#include "llvm/Support/Extension.def"
   PB.registerCGSCCAnalyses(CGAM);
   PB.registerFunctionAnalyses(FAM);
   PB.registerLoopAnalyses(LAM);
   PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);
 
   ModulePassManager MPM(CodeGenOpts.DebugPassManager);
 
   if (!CodeGenOpts.DisableLLVMPasses) {
+    // Map our optimization levels into one of the distinct levels used to
+    // configure the pipeline.
+    PassBuilder::OptimizationLevel Level = mapToLevel(CodeGenOpts);
+
     bool IsThinLTO = CodeGenOpts.PrepareForThinLTO;
     bool IsLTO = CodeGenOpts.PrepareForLTO;
 
     if (CodeGenOpts.OptimizationLevel == 0) {
       // If we reached here with a non-empty index file name, then the index
       // file was empty and we are not performing ThinLTO backend compilation
       // (used in testing in a distributed build environment). Drop any the type
       // test assume sequences inserted for whole program vtables so that
if (CodeGenOpts.OptimizationLevel == 0) {
         MPM.addPass(createModuleToFunctionPassAdaptor(BoundsCheckingPass()));
 
       // Lastly, add semantically necessary passes for LTO.
       if (IsLTO || IsThinLTO) {
         MPM.addPass(CanonicalizeAliasesPass());
         MPM.addPass(NameAnonGlobalPass());
       }
     } else {
-      // Map our optimization levels into one of the distinct levels used to
-      // configure the pipeline.
-      PassBuilder::OptimizationLevel Level = mapToLevel(CodeGenOpts);
-
       // If we reached here with a non-empty index file name, then the index
       // file was empty and we are not performing ThinLTO backend compilation
       // (used in testing in a distributed build environment). Drop any the type
       // test assume sequences inserted for whole program vtables so that
       // codegen doesn't complain.
       if (!CodeGenOpts.ThinLTOIndexFile.empty())
         PB.registerPipelineStartEPCallback(
             [](ModulePassManager &MPM, PassBuilder::OptimizationLevel Level) {
if (CodeGenOpts.UniqueInternalLinkageNames)
       MPM.addPass(UniqueInternalLinkageNamesPass());
 
     if (!CodeGenOpts.MemoryProfileOutput.empty()) {
       MPM.addPass(createModuleToFunctionPassAdaptor(MemProfilerPass()));
       MPM.addPass(ModuleMemProfilerPass());
     }
 
     if (CodeGenOpts.OptimizationLevel == 0) {
+      PB.runRegisteredEPCallbacks(MPM, Level, CodeGenOpts.DebugPassManager);
+
       // FIXME: the backends do not handle matrix intrinsics currently. Make
       // sure they are also lowered in O0. A lightweight version of the pass
       // should run in the backend pipeline on demand.
       if (LangOpts.MatrixTypes)
         MPM.addPass(
             createModuleToFunctionPassAdaptor(LowerMatrixIntrinsicsPass()));
 
       addCoroutinePassesAtO0(MPM, LangOpts, CodeGenOpts);

clang/test/CodeGen/bpf-O0.c

+// RUN: %clang -O0 %s -target bpf -g -c -o /dev/null -fexperimental-new-pass-manager
+// REQUIRES: bpf-registered-target
+
+struct ss {
+  int a;
+};
+int foo() { return __builtin_btf_type_id(0, 0) + __builtin_preserve_type_info(*(struct ss *)0, 0); }

llvm/include/llvm/IR/PassManager.h

public:
   /// happen with nested pass managers of the same type.
   template <typename PassT>
   std::enable_if_t<std::is_same<PassT, PassManager>::value>
   addPass(PassT &&Pass) {
     for (auto &P : Pass.Passes)
       Passes.emplace_back(std::move(P));
   }
 
+  /// Returns if the pass manager contains any passes.
+  bool isEmpty() { return Passes.empty(); }
+
   static bool isRequired() { return true; }
 
 protected:
   using PassConceptT =
       detail::PassConcept<IRUnitT, AnalysisManagerT, ExtraArgTs...>;
 
   std::vector<std::unique_ptr<PassConceptT>> Passes;
 

llvm/include/llvm/Passes/PassBuilder.h

public:
   void registerPipelineStartEPCallback(
       const std::function<void(ModulePassManager &, OptimizationLevel)> &C) {
     PipelineStartEPCallbacks.push_back(C);
   }
 
   /// Register a callback for a default optimizer pipeline extension point
   ///
   /// This extension point allows adding optimizations at the very end of the
-  /// function optimization pipeline. A key difference between this and the
-  /// legacy PassManager's OptimizerLast callback is that this extension point
-  /// is not triggered at O0. Extensions to the O0 pipeline should append their
-  /// passes to the end of the overall pipeline.
+  /// function optimization pipeline.
   void registerOptimizerLastEPCallback(
       const std::function<void(ModulePassManager &, OptimizationLevel)> &C) {
     OptimizerLastEPCallbacks.push_back(C);
   }
 
+  /// Run all registered extension point callbacks
+  ///
+  /// This runs the registered callbacks in the order they would be run in a
+  /// typical build*Pipeline(). This allows for reusing register*EPCallback()
+  /// between O0 and O[123] pipelines.
+  void runRegisteredEPCallbacks(ModulePassManager &MPM, OptimizationLevel Level,
+                                bool DebugLogging);
+
   /// Register a callback for parsing an AliasAnalysis Name to populate
   /// the given AAManager \p AA
   void registerParseAACallback(
       const std::function<bool(StringRef Name, AAManager &AA)> &C) {
     AAParsingCallbacks.push_back(C);
   }
 
   /// {{@ Register callbacks for analysis registration with this PassBuilder

llvm/lib/Passes/PassBuilder.cpp

#endif
 
   // Now that we have optimized the program, discard unreachable functions.
   MPM.addPass(GlobalDCEPass());
 
   // FIXME: Maybe enable MergeFuncs conditionally after it's ported.
   return MPM;
 }
 
+void PassBuilder::runRegisteredEPCallbacks(ModulePassManager &MPM,
+                                           OptimizationLevel Level,
+                                           bool DebugLogging) {
+  assert(Level == OptimizationLevel::O0 &&
+         "runRegisteredEPCallbacks should only be used with O0");
+  for (auto &C : PipelineStartEPCallbacks)
+    C(MPM, Level);
+  if (!LateLoopOptimizationsEPCallbacks.empty()) {
+    LoopPassManager LPM(DebugLogging);
+    for (auto &C : LateLoopOptimizationsEPCallbacks)
+      C(LPM, Level);
+    if (!LPM.isEmpty()) {
+      MPM.addPass(createModuleToFunctionPassAdaptor(
+          createFunctionToLoopPassAdaptor(std::move(LPM))));
+    }
+  }
+  if (!LoopOptimizerEndEPCallbacks.empty()) {
+    LoopPassManager LPM(DebugLogging);
+    for (auto &C : LoopOptimizerEndEPCallbacks)
+      C(LPM, Level);
+    if (!LPM.isEmpty()) {
+      MPM.addPass(createModuleToFunctionPassAdaptor(
+          createFunctionToLoopPassAdaptor(std::move(LPM))));
+    }
+  }
+  if (!ScalarOptimizerLateEPCallbacks.empty()) {
+    FunctionPassManager FPM(DebugLogging);
+    for (auto &C : ScalarOptimizerLateEPCallbacks)
+      C(FPM, Level);
+    if (!FPM.isEmpty())
+      MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
+  }
+  if (!CGSCCOptimizerLateEPCallbacks.empty()) {
+    CGSCCPassManager CGPM(DebugLogging);
+    for (auto &C : CGSCCOptimizerLateEPCallbacks)
+      C(CGPM, Level);
+    if (!CGPM.isEmpty())
+      MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM)));
+  }
+  if (!VectorizerStartEPCallbacks.empty()) {
+    FunctionPassManager FPM(DebugLogging);
+    for (auto &C : VectorizerStartEPCallbacks)
+      C(FPM, Level);
+    if (!FPM.isEmpty())
+      MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
+  }
+  for (auto &C : OptimizerLastEPCallbacks)
+    C(MPM, Level);
+}
+
 AAManager PassBuilder::buildDefaultAAPipeline() {
   AAManager AA;
 
   // The order in which these are registered determines their priority when
   // being queried.
 
   // First we register the basic alias analysis that provides the majority of
   // per-function local AA logic. This is a stateless, on-demand local set of
if (L == OptimizationLevel::O0) {
         CGSCCPassManager CGPM(DebugLogging);
         CGPM.addPass(CoroSplitPass());
         CGPM.addPass(createCGSCCToFunctionPassAdaptor(CoroElidePass()));
         MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM)));
 
         MPM.addPass(createModuleToFunctionPassAdaptor(CoroCleanupPass()));
       }
 
+      runRegisteredEPCallbacks(MPM, L, DebugLogging);
+
       // Do nothing else at all!
       return Error::success();
     }
 
     // This is consistent with old pass manager invoked via opt, but
     // inconsistent with clang. Clang doesn't enable loop vectorization
     // but does enable slp vectorization at Oz.
     PTO.LoopVectorization =

llvm/test/CodeGen/BPF/optnone-2.ll

 ; RUN: opt < %s -passes='default<O2>' | llc -march=bpfel -filetype=asm -o /dev/null -
-; TODO: add -O0 once that's supported
+; RUN: opt < %s -passes='default<O0>' | llc -march=bpfel -filetype=asm -o /dev/null -
 
 ; IR generated by
 ; $ cat /tmp/a.c
 ; struct ss { int a; };
 ; int foo() { return __builtin_btf_type_id(0, 0) + __builtin_preserve_type_info(*(struct ss *)0, 0); }
 ; $ clang -target bpf -g -S -emit-llvm t.c -Xclang -disable-llvm-passes /tmp/a.c
 
 target triple = "bpf"

llvm/test/Feature/optnone-opt.ll

 ; RUN: opt     -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s --check-prefix=O0
 ; RUN: opt -O1 -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s --check-prefix=O1
 ; RUN: opt -O2 -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s --check-prefix=O1 --check-prefix=O2O3
 ; RUN: opt -O3 -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s --check-prefix=O1 --check-prefix=O2O3
 ; RUN: opt -dce -gvn-hoist -loweratomic -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s --check-prefix=MORE
 ; RUN: opt -indvars -licm -loop-deletion -loop-extract -loop-idiom -loop-instsimplify -loop-reduce -loop-reroll -loop-rotate -loop-unroll -loop-unswitch -enable-new-pm=0 -S -debug %s 2>&1 | FileCheck %s --check-prefix=LOOP
-; RUN: opt -enable-npm-optnone -passes='default<O0>' -S -debug-pass-manager %s 2>&1 | FileCheck %s --check-prefix=NPM-O0
+; RUN: opt -enable-npm-optnone -passes='default<O0>' -S -debug-pass-manager %s 2>&1 | FileCheck %s --check-prefix=%llvmcheckext-NPM-O0
 ; RUN: opt -enable-npm-optnone -passes='default<O1>' -S -debug-pass-manager %s 2>&1 | FileCheck %s --check-prefix=NPM-O1
 ; RUN: opt -enable-npm-optnone -passes='default<O2>' -S -debug-pass-manager %s 2>&1 | FileCheck %s --check-prefix=NPM-O1 --check-prefix=NPM-O2O3
 ; RUN: opt -enable-npm-optnone -passes='default<O3>' -S -debug-pass-manager %s 2>&1 | FileCheck %s --check-prefix=NPM-O1 --check-prefix=NPM-O2O3
 ; RUN: opt -enable-npm-optnone -passes='dce,gvn-hoist,loweratomic' -S -debug-pass-manager %s 2>&1 | FileCheck %s --check-prefix=NPM-MORE
 ; RUN: opt -enable-npm-optnone -passes='loop(indvars,licm,loop-deletion,loop-idiom,loop-instsimplify,loop-reduce,simple-loop-unswitch),loop-unroll' -S -debug-pass-manager %s 2>&1 | FileCheck %s --check-prefix=NPM-LOOP
 ; RUN: opt -enable-npm-optnone -passes='instsimplify,verify' -S -debug-pass-manager %s 2>&1 | FileCheck %s --check-prefix=NPM-REQUIRED
 
 ; REQUIRES: asserts
while.body:                                       ; preds = %while.cond
   br label %while.cond
 
 while.end:                                        ; preds = %while.cond
   ret i32 %dec
 }
 
 attributes #0 = { optnone noinline }
 
-; Nothing that runs at -O0 gets skipped.
+; Nothing that runs at -O0 gets skipped (except when the Bye extension is present).
 ; O0-NOT: Skipping pass
-; NPM-O0-NOT: Skipping pass
+; CHECK-EXT-NPM-O0: Skipping pass {{.*}}Bye
+; CHECK-NOEXT-NPM-O0-NOT: Skipping pass
 
 ; IR passes run at -O1 and higher.
 ; O1-DAG: Skipping pass 'Aggressive Dead Code Elimination'
 ; O1-DAG: Skipping pass 'Combine redundant instructions'
 ; O1-DAG: Skipping pass 'Early CSE'
 ; O1-DAG: Skipping pass 'Reassociate expressions'
 ; O1-DAG: Skipping pass 'Simplify the CFG'
 ; O1-DAG: Skipping pass 'Sparse Conditional Constant Propagation'

llvm/test/Other/new-pass-manager.ll

 ; RUN:     | FileCheck %s --check-prefix=CHECK-CALLGRAPH
 ; CHECK-CALLGRAPH: Starting llvm::Module pass manager run
 ; CHECK-CALLGRAPH: Running pass: RequireAnalysisPass
 ; CHECK-CALLGRAPH: Running analysis: CallGraphAnalysis
 ; CHECK-CALLGRAPH: Finished llvm::Module pass manager run
 
 ; RUN: opt -disable-output -disable-verify -debug-pass-manager \
 ; RUN:     -passes='default<O0>' %s 2>&1 \
-; RUN:     | FileCheck %s --check-prefix=CHECK-O0
+; RUN:     | FileCheck %s --check-prefix=CHECK-O0 --check-prefix=%llvmcheckext
 ; CHECK-O0: Starting llvm::Module pass manager run
+; CHECK-EXT-NEXT: Running analysis: InnerAnalysisManagerProxy<{{.*}}>
+; CHECK-EXT-NEXT: Starting llvm::Function pass manager run.
+; CHECK-EXT-NEXT: Running pass: {{.*}}Bye
+; CHECK-EXT-NEXT: Finished llvm::Function pass manager run.
 ; CHECK-O0-NEXT: Finished llvm::Module pass manager run
 
 ; RUN: opt -disable-output -disable-verify -debug-pass-manager \
 ; RUN:     -passes='repeat<3>(no-op-module)' %s 2>&1 \
 ; RUN:     | FileCheck %s --check-prefix=CHECK-REPEAT-MODULE-PASS
 ; CHECK-REPEAT-MODULE-PASS: Starting llvm::Module pass manager run
 ; CHECK-REPEAT-MODULE-PASS-NEXT: Running pass: RepeatedPass
 ; CHECK-REPEAT-MODULE-PASS-NEXT: Starting llvm::Module pass manager run

llvm/test/Other/new-pm-O0-ep-callbacks.ll

+; RUN: opt -disable-output -debug-pass-manager -passes-ep-late-loop-optimizations=no-op-loop -passes='default<O0>' 2>&1 < %s | FileCheck %s
+; RUN: opt -disable-output -debug-pass-manager -passes-ep-loop-optimizer-end=no-op-loop -passes='default<O0>' 2>&1 < %s | FileCheck %s
+; RUN: opt -disable-output -debug-pass-manager -passes-ep-scalar-optimizer-late=no-op-function -passes='default<O0>' 2>&1 < %s | FileCheck %s
+; RUN: opt -disable-output -debug-pass-manager -passes-ep-cgscc-optimizer-late=no-op-cgscc -passes='default<O0>' 2>&1 < %s | FileCheck %s
+; RUN: opt -disable-output -debug-pass-manager -passes-ep-vectorizer-start=no-op-function -passes='default<O0>' 2>&1 < %s | FileCheck %s
+; RUN: opt -disable-output -debug-pass-manager -passes-ep-pipeline-start=no-op-module -passes='default<O0>' 2>&1 < %s | FileCheck %s
+; RUN: opt -disable-output -debug-pass-manager -passes-ep-optimizer-last=no-op-function -passes='default<O0>' 2>&1 < %s | FileCheck %s
+
+; CHECK: Running pass: NoOp
+
+declare void @bar() local_unnamed_addr
+
+define void @foo(i32 %n) local_unnamed_addr {
+entry:
+  br label %loop
+loop:
+  %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ]
+  %iv.next = add i32 %iv, 1
+  tail call void @bar()
+  %cmp = icmp eq i32 %iv, %n
+  br i1 %cmp, label %exit, label %loop
+exit:
+  ret void
+}