[PATCH] D108298: [NPM] Print '-passes' compatible string for built pipeline.

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: llvm/include/llvm/IR/PassManager.h:387
+    auto ClassName = name();
+    auto PassName = PIC.getPassNameForClassName(ClassName);
+    if (!PassName.empty()) {
----------------
IMHO the getPassNameForClassName function is pretty trivial. Maybe you should just pass around a reference to the ClassToPassName StringMap instead. That will show that we do not really depend on or use the PassInstrumentationCallbacks class. We are only interested in the StringMap (and in the future that could be implemented somewhere else and not in the PassInstrumentationCallbacks class). Or what do you think about that?


================
Comment at: llvm/lib/Passes/PassBuilder.cpp:442
 bool shouldPopulateClassToPassNames() {
-  return !printBeforePasses().empty() || !printAfterPasses().empty();
+  return true || !printBeforePasses().empty() || !printAfterPasses().empty();
 }
----------------
Given the addition of the new PrintPipelinePasses option, wouldn't it be possible to use that one here now instead of true somehow.

A problem might be that PassBuilder is used by clang etc. So you'd need to move the PrintPipelinePasses option to PassBuilder.cpp (or maybe PrintPasses.cpp?).



================
Comment at: llvm/tools/opt/NewPMDriver.cpp:482
+  // requested.
+  if (PrintPipelinePasses) {
+    errs() << "-passes='";
----------------
I wonder if we perhaps want this to print to `outs()` instead, and then return true (instead of running the pipeline) here. Kind of similar to the `-print-passes` option.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108298/new/

https://reviews.llvm.org/D108298