[flang][Driver] Add support for -mllvm -print-pipeline-passes

[tarunprabhu]

The behavior deliberately mimics that of clang.

[llvmbot]

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-flang-driver

Author: Tarun Prabhu (tarunprabhu)

Changes

The behavior deliberately mimics that of clang.

---

Full diff: https://github.com/llvm/llvm-project/pull/106141.diff

2 Files Affected:

• (modified) flang/lib/Frontend/FrontendActions.cpp (+16)
• (added) flang/test/Driver/print-pipeline-passes.f90 (+10)

diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index 5c86bd947ce73f..012649b2ba16bd 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -79,6 +79,10 @@
 
 #include "flang/Tools/CLOptions.inc"
 
+namespace llvm {
+extern cl::opt<bool> PrintPipelinePasses;
+} // namespace llvm
+
 using namespace Fortran::frontend;
 
 // Declare plugin extension function declarations.
@@ -1015,6 +1019,18 @@ void CodeGenAction::runOptimizationPipeline(llvm::raw_pwrite_stream &os) {
   else if (action == BackendActionTy::Backend_EmitLL)
     mpm.addPass(llvm::PrintModulePass(os));
 
+  // Print a textual, '-passes=' compatible, representation of pipeline if
+  // requested. In this case, don't run the passes. This mimics the behavior of
+  // clang.
+  if (llvm::PrintPipelinePasses) {
+    mpm.printPipeline(llvm::outs(), [&pic](llvm::StringRef className) {
+      auto passName = pic.getPassNameForClassName(className);
+      return passName.empty() ? className : passName;
+    });
+    llvm::outs() << "\n";
+    return;
+  }
+
   // Run the passes.
   mpm.run(*llvmModule, mam);
 }
diff --git a/flang/test/Driver/print-pipeline-passes.f90 b/flang/test/Driver/print-pipeline-passes.f90
new file mode 100644
index 00000000000000..4b413cbc07b691
--- /dev/null
+++ b/flang/test/Driver/print-pipeline-passes.f90
@@ -0,0 +1,10 @@
+! Test that -print-pipeline-passes works in flang
+
+! RUN: %flang_fc1 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null -mllvm -print-pipeline-passes -O0 %s 2>&1 | FileCheck %s
+
+! Don't try to check all passes, just a few to make sure that something is
+! actually printed.
+! CHECK: always-inline
+! CHECK-SAME: annotation-remarks
+
+end program

[AlexisPerry]

LGTM. All builds and passes on my setup.

[banach-space]

How is this different to debug-pass-manager?

[tarunprabhu]

How is this different to debug-pass-manager?

In essence, -mllvm -print-pipeline-passes will provide a list of passes that will be run for some invocation of the compiler. -fdebug-pass-manager will show what was actually run. More details below.

-fdebug-pass-manager executes the underlying action, while this simply prints the pipeline and terminates. This is the same as clang's behavior when it is passed -mllvm -print-pipeline-passes

The output format is also very different. This prints a list of all the passes in a format that is compatible with -passes . From what I can tell of -fdebug-pass-manager, it prints something closer to what the legacy pass manager would print with -debug-pass=Structure (I can't remember the exact syntax off the top of my head).

Since -fdebug-pass-manager actually executes the underlying action, it will also print out invalidations of analysis passes which this, obviously, cannot.

[banach-space]

Thanks for the summary!

-fdebug-pass-manager executes the underlying action, while this simply prints the pipeline and terminates.

This is something that I struggle with. It is basically making this one flag (which is meant as LLVM's internal option) a very very special flower. We should avoid that. This was already raised when that flag was introduced in Clang:

• https://reviews.llvm.org/D127221

Here's one follow-up question - could you compare the output from -mllvm -print-pipeline-passes and -fdebug-pass-manager and paste it here? Is it very different and is the former the preferred format? If yes, why not update -fdebug-pass-manager accordingly (in both Clang and Flang)? Is "stopping" the compilation the desired behaviour here?

I'm 100% supportive of improving the end-user and developer's experience, but I want to make sure that there's no other, more canonical way to achieve the same. Right now it feels that indeed there is :)

[tarunprabhu]

Output of -fdebug-pass-manager

Running analysis: InnerAnalysisManagerProxy<FunctionAnalysisManager, Module> on [module]
Running pass: EntryExitInstrumenterPass on _QQmain (4 instructions)
Running pass: EntryExitInstrumenterPass on main (4 instructions)
Running pass: AlwaysInlinerPass on [module]
Running analysis: ProfileSummaryAnalysis on [module]
Running pass: CoroConditionalWrapper on [module]
Running pass: AnnotationRemarksPass on _QQmain (4 instructions)
Running analysis: TargetLibraryAnalysis on _QQmain
Running pass: AnnotationRemarksPass on main (4 instructions)
Running analysis: TargetLibraryAnalysis on main
Running pass: PrintModulePass on [module]

Output of `-mllvm -print-pipeline-passes

function(ee-instrument<>),always-inline,coro-cond(coro-early,cgscc(coro-split),coro-cleanup,globaldce),function(annotation-remarks),print

The outputs above are for -O0 on a simple 3-line hello world program. It gets more interesting when optimizations are enabled and the input is non-trivial.

Is it very different and is the former the preferred format?

In the second case, the output can be passed as is to the -passes option of opt. This is potentially useful to folks who want to experiment with a sequence of passes that is very similar to the compiler's defaults with potential additions/deletions. The former is "friendlier" and provides insights into analysis invalidations which is interesting in its own right. I don't think one is "more correct" or "better" than the other. I think having both is valuable.

Is "stopping" the compilation the desired behaviour here?

My only concern with changing the behavior that is that we would have two frontends that are part of the same project with different behavior. It looks like they stopped compilation in clang to be consistent with the behavior of opt.

If tangling this with an "internal" LLVM command line option is objectionable (and I agree that it feels messy) we could make this a separate, first class option in flang. Would the clang folks be open to a similar change?

[EDIT: fix formatting]

[tblah]

Is it very different and is the former the preferred format?

In the second case, the output can be passed as is to the -passes option of opt. This is potentially useful to folks who want to experiment with a sequence of passes that is very similar to the compiler's defaults with potential additions/deletions. The former is "friendlier" and provides insights into analysis invalidations which is interesting in its own right. I don't think one is "more correct" or "better" than the other. I think having both is valuable.

For me at least, these two different use cases justify two different flags. It would be useful to have something to pass straight to opt.

Is "stopping" the compilation the desired behaviour here?

My only concern with changing the behavior that is that we would have two frontends that are part of the same project with different behavior. It looks like they stopped compilation in clang to be consistent with the behavior of opt.

I agree that whatever we do should match clang (or clang should be adapted to match us).

If tangling this with an "internal" LLVM command line option is objectionable (and I agree that it feels messy) we could make this a separate, first class option in flang. Would the clang folks be open to a similar change?

In my opinion, support for -mllvm and -mmlir options is long established and comes in useful for people working on both frontends. All this PR is doing is enabling another of these developer options that already exists inside of clang. We are not careful about what -mllvm options are added when new patches add new llvm::cl::opt instances - I always considered things hidden behind -mllvm as outside of the user interface.

This LGTM, but don't merge without agreement from Andrzej

[banach-space]

Thanks for getting back to me.

these two different use cases justify two different flags

+1

All this PR is doing is enabling another of these developer options that already exists inside of clang.

Well, not quite :) Using -mllvm -<option> means "take this LLVM and forward it to LLVM" - nothing more than that. However, this approach implements extra semantics for -mllvm -print-pipeline-passes inside Clang. Put differently, it is "elevating" -mllvm -print-pipeline-passes to a proper compiler driver flag. However, compiler driver flags are of the form -<option> rather than -mllvm -<option>.

we could make this a separate, first class option in flang

+1

Would the clang folks be open to a similar change?

Why not? :) Similar concerns were raised when this flag was introduced in Clang, so I imagine that a sounder approach would be welcome.

Also, while keeping Flang consistent with Clang should be a very high priority, it's OK to diverge when there's good justification. Also, even more importantly, if there's scope to improve things in Flang, please don't shy away from bringing similar improvements to Clang.

To unblock you, I propose the following:

1. Keep this as is, but add a comment saying that this should be a proper compiler option instead of a secret handshake like this. Similar comment should be added in Clang. I will happily approve that.
2. Re-implement this as a proper flag in both Flang and Clang (as a separate PR). If we struggle to get reviewers from Clang, I suggest posting a short RFC/PSA.

Alternatively, go with 2. straight away :)

[tarunprabhu]

To unblock you, I propose the following:

1. Keep this as is, but add a comment saying that this should be a proper compiler option instead of a secret handshake like this. Similar comment should be added in Clang. I will happily approve that.

I have added a comment to both clang and flang indicating that we should eventually replace mllvm -print-pipeline-passes with a proper compiler driver option. I am happy to implement it, but it might have to wait for a bit, and I'd like to get the functionality in for now.

Thanks for the review and suggestions, Andrzej.

[banach-space]

Approving with the caveat that we should avoid leveraging LLVM options like this and that long-term this should be refactored as a dedicated, first-class driver option. This can be implemented in a seperate PR.

Thank you for the discussion and for addressing my comment @tarunprabhu 🙏🏻 LGTM