This is an archive of the discontinued LLVM Phabricator instance.

[test] Avoid legacy PM default pipelines (O0,O1 etc) when running opt
ClosedPublic

Authored by bjope on Nov 8 2022, 3:19 AM.

Details

Summary

Two lit tests were found running something like this:

opt -O<n> -pass-locked-to-legacy-PM ...

The expand-atomicrmw-xchg-fp.ll seem to have used -O1 just to ensure
that the -atomic-expand pass were thinking that it wasn't running at
O0 level. Same thing can be ensured by using the -codegen-opt-level=1
option, making it possible to avoid using O1 in that test case.

In the vector-reductions-expanded.ll test case it was possible to
split the RUN line into using two opt invocations. First running
"opt -O2" using the new PM, and then running "opt -expand-reductions"
using the legacy PM.

I think that given this patch we get closer to removing code related
to 'AddOptimizationPasses' in opt.cpp.

Diff Detail

Event Timeline

bjope created this revision.Nov 8 2022, 3:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 8 2022, 3:19 AM
bjope requested review of this revision.Nov 8 2022, 3:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 8 2022, 3:19 AM
nikic accepted this revision.Nov 8 2022, 3:43 AM
nikic added a subscriber: nikic.

LG

This revision is now accepted and ready to land.Nov 8 2022, 3:43 AM
bjope added a comment.Nov 8 2022, 5:47 AM

Just a side not here.

As I wrote in the descrption: "I think that given this patch we get close to removing code related to 'AddOptimizationPasses' in opt.cpp."

I'm not sure exactly how fast we can/want to move forward. But I've assumed that we perhaps want to get rid of the legacy default pipelines.
But if we for example want to remove PassManagerBuilder::populateModulePassManager and PassManagerBuilder::populateFunctionPassManager there are some more uses (and I'm not aware of any ongoing work in deprecating those).

clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:  Builder.populateModulePassManager(MPM);
llvm/lib/Transforms/IPO/PassManagerBuilder.cpp:  Builder->populateFunctionPassManager(*FPM);
llvm/lib/Transforms/IPO/PassManagerBuilder.cpp:  Builder->populateModulePassManager(*MPM);
llvm/tools/bugpoint/bugpoint.cpp:  Builder.populateFunctionPassManager(FPM);
llvm/tools/bugpoint/bugpoint.cpp:  Builder.populateModulePassManager(FPM);
llvm/tools/opt/opt.cpp:  Builder.populateFunctionPassManager(FPM);
llvm/tools/opt/opt.cpp:  Builder.populateModulePassManager(MPM);
polly/lib/CodeGen/PPCGCodeGeneration.cpp:    PassBuilder.populateModulePassManager(OptPasses);

Where the uses PassManagerBuilder.cpp relate to LLVMPassManagerBuilderPopulateFunctionPassManager and LLVMPassManagerBuilderPopulateModulePassManager.

So even if we remove the support for O<n> in opt (for legacy PM) there are still uses around of the legacy PM:s module/function pipelines in OCAML, LLVM-C, Polly, bugpoint, clang-fuzzer and MCJITCAPITest.

I know that there exists a github issue about bugpoint. But no idea if there has been any tickets or RFC:s about remove legacy PM from the rest.

we should add a check in opt.cpp that we don't mix -O# with other passes (there's already a similar check for the new PM)

I've been trying to slowly remove PassManagerBuilder but it's quite slow and I keep getting distracted by other things

bjope added a comment.Nov 8 2022, 12:27 PM

we should add a check in opt.cpp that we don't mix -O# with other passes (there's already a similar check for the new PM)

I was thinking about something like this, that remove the support for opt -O# with legacy PM: https://reviews.llvm.org/D137663

This revision was landed with ongoing or failed builds.Nov 9 2022, 12:58 AM
This revision was automatically updated to reflect the committed changes.