[PATCH] D104812: [docs][NewPM] Add some instructions on how to invoke opt

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers accepted this revision.
nickdesaulniers added a comment.
This revision is now accepted and ready to land.

This is incredibly helpful, thank you so much.

General feedback about NPM, completely orthogonal to this review.

I find the nesting a little confusing.  Not the order, but the adapters.  It seems they describe what they're wrapping, not what they're producing (unlike casting in C).  For example, `function(foo)` wraps `foo` (which is a function pass) in a `module` pass adapter.  It seems more intuitive IMO for the syntax to be `module(foo)`, not `function(foo)`.  Does this have to do with the optional cgscc wrapper?

The help text from `opt` for incorrect nesting could be improved.  Rather than feedback like `unknown function pass 'no-op-module'` for a valid pass name, but invalid nesting, it would be nice if `opt` could provide more helpful feedback in such a case like: `'no-op-module' is a function pass, not a module pass. Reinvoke with 'function(no-op-module)' in pass list.` or something.

For a more concrete example,

  $ opt -passes='adce,always-inline' foo.ll -S
  opt: unknown function pass 'always-inline'

So this seems to be using the implicit module adapter you spoke of, but the error message is confusing.  I assume that always-inline, a module level pass, is being wrapped in the function adapter since adce was first, and needed one.

  $ opt -passes='function(adce),always-inline' foo.ll -S

would be the correct invocation.  But I need to invoke `opt --print-passes` to find out what IR unit such a pass operates on.  `opt` *knows* what IR unit each pass runs on (assuming it is a valid pass for some unit of IR); it should just tell me politely via a helpful diagnostic, or figure out what I meant, rather than error out and require me to look up and think about nesting adapters.



================
Comment at: llvm/docs/NewPassManager.rst:423
+
+For a list of available passes and analyses, including the IR unit they operate
+on, run
----------------
maybe after "the IR unit" add "(module, cgscc, function, or loop)" to reiterate?


================
Comment at: llvm/docs/NewPassManager.rst:429
+
+  $ opt -passes='function(require<my-function-analysis>),my-module-pass' /tmp/a.ll -S
+
----------------
aeubanks wrote:
> nickdesaulniers wrote:
> > How does this differ from say:
> > `-passes='function(my-function-analysis,my-module-pass'`
> > ?
> `-passes=my-function-analysis` just isn't a thing
> I've made it clearer what this actually does
Sorry, I meant:

What is the difference between:
`opt -passes='function(require<my-function-analysis>),my-module-pass' /tmp/a.ll -S`
vs
`opt -passes='function(my-function-analysis),my-module-pass' /tmp/a.ll -S`

Wouldn't `my-function-analysis` run either way? If so, what's the point of `require<>`? If not, what difference does `require<>` imply?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104812