Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[docs][passes] Update documentation of Analysis and Transform Passes #80835

Merged
merged 2 commits into from
Feb 7, 2024

Conversation

bjope
Copy link
Collaborator

@bjope bjope commented Feb 6, 2024

  • Added a warning about the pass list being incomplete (and hint about "opt -print-passes" listing known passes).
  • Replaced legacy PM example involving "opt -analyze" with a proper example using -passes option.
  • print-function and print-module does not exist with those names in the new PM. Since the document has been updated to reflect the new PM names earlier, those names were changed into referring to function(print) and module(print) instead.
  • Resolved some FIXME:s around strip* passes.

@@ -199,8 +202,9 @@ information query.
This pass decodes the debug info metadata in a module and prints in a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

print<module-debuginfo> is the name of the pass, there's no module-debuginfo analysis, can you update this section?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed the name of the pass to print<module-debuginfo>. But it is still listed among the analysis passes.
I got a feeling that this document already is a bit messy as it lists analysis passes and transform passes. But many of these print passes that are listed as analysis passes are actually function/module passes. So it would take a bit more work if we want to be exact in this document about how passes actually are classified (but maybe that cassification isn't very important for this document).

- Added a warning about the pass list being incomplete (and hint about
  "opt -print-passes" listing known passes).
- Replaced legacy PM example involving "opt -analyze" with a proper
  example using -passes option.
- print-function and print-module does not exist with those names
  in the new PM. Since the document has been updated to reflect
  the new PM names earlier, those names were changed into referring to
  function(print) and module(print) instead.
- Resolved some FIXME:s around strip* passes.
Copy link
Contributor

@aeubanks aeubanks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lg with a comment


This pass decodes the debug info metadata in a module and prints in a
(sufficiently-prepared-) human-readable form.

For example, run this pass from ``opt`` along with the ``-analyze`` option, and
it'll print to standard output.
For example, run this pass from ``opt`` along with the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

along with the ... option is confusing since that's true for every pass listed here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I agree. This was only here to refer to -analyze in the past. I'll drop this example completely as there is nothing special with this pass compared to all other passes listed in the document.

As usuel @aeubanks, thanks a lot for prompt feedback and good advice!

Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I've been meaning to write a script that parses the opt -print-passes output and compares it with the sections here to automatically identify gaps (as it existed before in some hacky form), and maybe even run it in CI. It hasn't been high on my priority list however.

@bjope bjope merged commit 93962ea into llvm:main Feb 7, 2024
5 checks passed
@bjope bjope deleted the doc_passes branch March 20, 2024 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants