Skip to content

[DeveloperPolicy] Add guidelines for adding/enabling passes #158591

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

[DeveloperPolicy] Add guidelines for adding/enabling passes #158591

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0b352ed
[DeveloperPolicy] Add guidelins for adding/enabling passes
nikic Sep 15, 2025
5c8b522
Suggest adding to an existing pass
nikic Sep 22, 2025
b5ea3c7
Drop old/new distinction in one place
nikic Sep 22, 2025
14595d4
Clarify what basic version means
nikic Sep 22, 2025
750a5bf
Exclude passes which shouldn't be in the default pipeline
nikic Sep 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions llvm/docs/DeveloperPolicy.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1185,6 +1185,55 @@ Suggested disclaimer for the project README and the main project web page:
necessarily a reflection of the completeness or stability of the code, it
does indicate that the project is not yet endorsed as a component of LLVM.

Adding or enabling a new LLVM pass
----------------------------------

The guidelines here are primarily targeted at the enablement of new major
passes in the target-independent optimization pipeline. Small additions, or
backend-specific passes, require a lesser degree of care. Before creating a new
pass, consider whether the functionality can be integrated into an existing
pass first. This is often both faster and more powerful.

When adding a new pass, the goal should be to enable it as part of the default
optimization pipeline as early as possible and then continue development
incrementally. (This does not apply to passes that are only relevant for
specific uses of LLVM, such as GC support passes.)

The recommended workflow is:

1. Implement a basic version of the pass and add it to the pass pipeline behind
a flag that is disabled by default. The initial version should focus on
handling simple cases correctly and efficiently.
2. Enable the pass by default. Separating this step allows easily disabling the
pass if issues are encountered, without having to revert the entire
implementation.
Copy link
Collaborator

@joker-eph joker-eph Sep 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This seems implicit, but may be worth making it explicit a bit more: this strategy seems to rely on the first implementation to be "conservative": that is the "basic version" of the pass is about "doing less" in term of usefulness, but "not miscompiling anything" right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added an extra sentence "The initial version should focus on handling simple cases correctly and efficiently" here. Is that clearer? Happy to take suggestions on how to improve the wording here.

3. Incrementally extend the pass with new functionality. As the pass is already
enabled, it becomes easier to identify the specific change that has caused a
regression in correctness, optimization quality or compile-time.

When enabling a pass, certain requirements must be met (in no particular order):

* **Maintenance:** The pass (and any analyses it depends on) must have at
least one maintainer.
* **Usefulness:** There should be evidence that the pass improves performance
(or whatever metric it optimizes for) on real-world workloads. Improvements
seen only on synthetic benchmarks may be insufficient.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should consider distinguishing -O2 and -O3? We already run a lot of passes that do nothing for most functions. For -O2 (the average RelWithDebInfo build) we might want to focus on transforms that have an impact for a broader range of programs and not just a few special workloads.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, I guess aspirationally the answer to this is "yes", but I think in practice it is "no". We don't really have a strong distinction between the -O2 and -O3 pipelines, beyond using different inlining/unrolling thresholds. The differences that do exist between the pipelines are not good ones (for example, there is no good reason why ArgumentPromotion should be an O3 pass, it is very much generally useful).

Copy link
Contributor

Choose a reason for hiding this comment

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

I know, but maybe we want to change this at some point? Loop interchange, for example, is likely beneficial for certain programs (probably numerics), but for the average integer-heavy C++ program, it probably has no impact (except for increasing compile times). Perhaps we can start by asking authors for an assessment of the range of programs that will benefit from the transformation if they believe that it should be enabled at -O2?

* **Compile-Time:** The pass should not have a large impact on compile-time,
where the evaluation of what "large" means is up to reviewer discretion, and
may differ based on the value the pass provides. In any case, it is expected
that a concerted effort has been made to mitigate the compile-time impact,
both for the average case, and for pathological cases.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you want to suggest to use your LLVM compile-time tracker for this?

Copy link
Contributor

@madhur13490 madhur13490 Sep 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, a reference to the latest compile-time tracker results should be required. How much impact on compile-time is acceptable? (For loop interchange, we heard 0.19% is "too much"). While we are at it, let's also quantify how much is too much.
Edit: I see that it is left to the reviewer's discretion, but isn't that open-ended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's not really possible to name a specific number here, because the compile-time impact is a tradeoff against the value the pass provides. A pass that provides a performance benefit on a wide range of code will have a larger compile-time budget than a pass that only benefits rare cases.

* **Correctness:** The pass should have no known correctness issues (except
global correctness issues that affect all of LLVM). If an old pass is being
enabled (rather than implementing a new one incrementally), additional due
diligence is required. The pass should be fully reviewed to ensure that it
still complies with current quality standards. Fuzzing with disabled
profitability checks may help gain additional confidence in the
implementation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fuzzing with disabled profitability checks may help gain additional confidence in the implementation.

Yeah, I agree, and for the record, that's exactly what I did with loop-interchange:

Anyway, that is slightly irrelevant for this policy, but just wanted to get this out of the way from previous discussions.

Running fuzzers is slightly less straightforward than it sounds. A good documentation contribution would be to add a howto for this. I am not suggesting it should be here in this section, but I can look where that belongs and prepare a draft how to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'm aware that you did this. I thought it was a sufficiently good idea to include it as a general recommendation :)

Running fuzzers is slightly less straightforward than it sounds. A good documentation contribution would be to add a howto for this. I am not suggesting it should be here in this section, but I can look where that belongs and prepare a draft how to do this.

Sounds good to me.

If non-trivial issues are found in a newly enabled pass, it may be temporarily
disabled again, until the issues have been resolved.

.. _copyright-license-patents:

Copyright, License, and Patents
Expand Down
Loading