stabilize optimize attribute

[veluca93]

View all comments

This commit stabilizes the #[optimize] attribute, which allows for more fine-grained control of optimization levels.

Stabilization report

Summary

Tracking issue: #54882
Reference PRs: rust-lang/reference#2278

cc @rust-lang/lang @rust-lang/lang-advisors @rust-lang/t-compiler

What is stabilized

// Instructs the optimization pipeline to prioritize smaller code size over speed.
#[optimize(size)]
pub fn binary_size_sensitive() {
    // ...
}

// Instructs the optimization pipeline to prioritize execution speed over code size.
#[optimize(speed)]
pub fn performance_critical() {
    // ...
}

// Disables optimizations entirely for this item.
#[optimize(none)]
pub fn debug_only() {
    // ...
}

What isn't stabilized

We might want to allow specifying a per-function optimization level (i.e. #[optimize(level = 3)]). To my understanding, backend support for this is not quite there yet (and this - or other - extensions have consequently not been implemented yet).

Applying the attribute on a module level is also not implemented yet.

Design

Reference

• Description of the new attribute: Stabilize #[optimize] attribute reference#2278

RFC history

• https://rust-lang.github.io/rfcs/2412-optimize-attr.html

Answers to unresolved question

• Should we also implement optimize(always)? optimize(level=x)?

Not yet.

• Should there be any way to specify what global optimization for speed level is used in conjunction with the optimization for speed option (e.g. -Copt-level=s3 could be equivalent to -Copt-level=3 and #[optimize(size)] on the crate item).

Not yet.

Post-RFC changes

The optimize(none) variant was added. The variants above were discussed here: #54882 (comment)

Key points

• #[optimize(none)] implies #[inline(never)] to be effective (#[optimize(none)] should imply #[inline(never)] #136329)
• As with all optimization-controlling flags, the effects on speed and binary size can be hard to predict: Tracking issue for RFC 2412, "The optimize attribute" #54882 (comment)
• Applying the attribute to incompatible items emits an error: Emit an error if #[optimize] is applied to an incompatible item #128458

Nightly uses

The standard library itself uses this attribute in a few places

Implementation

Major part

• Wiring it up to the llvm codegen backend:

  rust/compiler/rustc_codegen_llvm/src/attributes.rs

  Line 416 in c0bb140

  match codegen_fn_attrs.optimize {

• Catching usage on incompatible items: Emit an error if #[optimize] is applied to an incompatible item #128458
• Implementing optimize(none): Add #[optimize(none)] #128657 #[optimize(none)] implies #[inline(never)] #136358 (includes disabling MIR opts)

Coverage

• Basic tests: https://github.com/rust-lang/rust/blob/c0bb140a37c81cf59a0b40c21c9413059644e294/tests/ui/attributes/optimize.rs
• Testing the effect on compilation: https://github.com/rust-lang/rust/blob/c0bb140a37c81cf59a0b40c21c9413059644e294/tests/codegen-llvm/optimize-attr-1.rs

Tool changes

Trivial changes to rust-analyzer to support the new attribute as an inert attribute: rust-lang/rust-analyzer#22511

Type system, opsem

Breaks the AM?

As far as the AM is concerned, this attribute doesn't exist.

Acknowledgments

Most of the work was not done by me, I'm just writing the stabilization report :-)

@nagisa did the initial implementation, @clubby789 implemented optimize(none) and fixed the attribute being allowed in too many places.

[rustbot]

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

[rustbot]

r? @JohnTitor

rustbot has assigned @JohnTitor.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 73 candidates
• Random selection from 17 candidates

[tgross35]

Given the interactions here, I expect this will need a lang+compiler FCP

@rustbot label +I-lang-nominated +I-compiler-nominated

See some recent discussion about this feature on Zulip #t-lang > status of #[optimize] attribute

[JohnTitor]

Marking as S-blocked given the current status.

[rustbot]

These commits modify tests/rustdoc-json.
rustdoc-json is a public (but unstable) interface.

Please ensure that if you've changed the output:

• It's intentional.
• The FORMAT_VERSION in src/librustdoc-json-types is bumped if necessary.

cc @aDotInTheVoid, @obi1kenobi

[obi1kenobi]

With my cargo-semver-checks hat on: do we expect this attribute to have SemVer implications?

For example, can applying or removing this attribute on an item influence the contexts in which that item may be used without compile errors or other changes that would be considered public-API-breaking?

[veluca93]

With my cargo-semver-checks hat on: do we expect this attribute to have SemVer implications?

For example, can applying or removing this attribute on an item influence the contexts in which that item may be used without compile errors or other changes that would be considered public-API-breaking?

I don't believe this is possible - the attribute should only influence how the compiler optimizes functions, and compiler optimizations should not have any visible effects.

[jieyouxu]

Discussion: hm, I wonder if this attribute deserves an entry in the rustc book (a bit like lint levels), because while sure there's the Reference PR, but this is more like a compiler knob?

View changes since the review

[jieyouxu]

Suggestion: I was looking at the test coverage, do we have any //@ run-pass smoke tests for testing that the attribute E2E-wise doesn't trivially "blow up"? (It obviously will not prove the attribute and its effects work in all possible cases, but a smoke test is still valuable). E.g. this might just be a main that calls a function annotated with #[optimize].

View changes since the review

[jieyouxu]

Question (non-blocking): hm, do we know what happened to optimize-attr-2.rs 🙈

View changes since the review

[nagisa]

I believe the overarching question in stabilizing this attribute should be whether we're happy with the capability and scope of this attribute. Code contained in functions with optimize(none) for example code can still participate in optimizations despite the inline(never) due to global cross-function analyses and there being many configurations in which the attribute can be lost (various LTO configurations?) It really is hard in a modern backend to ensure that optimization restrictions are propagated all the way through.

I realize that I'm basically reiterating the following comment: #54882 (comment). I don't necessarily think that we must block the stabilization of these attributes on this point, but I think it is important that we set ourselves' and the users' expectations on what this attribute can and cannot do. It might very well end up the case that in process of doing so we realize this feature does not pass the bar of quality we expect out of Rust. At the same time this feature is genuinely useful despite all its flaws and those flaws aren't likely to be addressed in any meaningful way, so stabilizing it might be the only way forward for it.

[jieyouxu]

(Thought of a few more cases)

View changes since this review

[jieyouxu]

Question: this is more from UX / language design PoV, but consider this example:

#[optimize(speed)]
fn outer() {
    fn inner() {}
}

• Does #[optimize(speed)]'s hint "effect" apply to outer but not inner? Should it apply to both?

(I don't think there's test coverage for this)

[jieyouxu]

Question: currently, it seems that e.g.

const _: () = {
    #[optimize(speed)]
    fn outer() {
    }
};

is accepted. What is the expected effect in this case?

(I don't think there's test coverage for this)

[jieyouxu]

Question: it seems that one can also do

#[optimize(speed)]
const fn foo() {}

What is the expected effect in this case?

(I don't think there's test coverage for this)

[veluca93]

I believe the overarching question in stabilizing this attribute should be whether we're happy with the capability and scope of this attribute. Code contained in functions with optimize(none) for example code can still participate in optimizations despite the inline(never) due to global cross-function analyses and there being many configurations in which the attribute can be lost (various LTO configurations?) It really is hard in a modern backend to ensure that optimization restrictions are propagated all the way through.

I realize that I'm basically reiterating the following comment: #54882 (comment). I don't necessarily think that we must block the stabilization of these attributes on this point, but I think it is important that we set ourselves' and the users' expectations on what this attribute can and cannot do. It might very well end up the case that in process of doing so we realize this feature does not pass the bar of quality we expect out of Rust. At the same time this feature is genuinely useful despite all its flaws and those flaws aren't likely to be addressed in any meaningful way, so stabilizing it might be the only way forward for it.

In general, optimization-controlling attributes don't have strictly defined semantics already, i.e. inline(never) and inline(always) don't guarantee that the function they are applied to are never/always inlined, despite what the name might seem to imply. Thus, I believe there is precedent for attributes that hint towards a desired optimization behaviour but guarantee nothing. The wording in the reference matches this by using "suggests" instead of stronger wording.

[nikic]

From the LLVM perspective, stabilizing these should be fine. I've done some work (culminating in llvm/llvm-project#191363) to ensure that -C opt-level=s/z is strictly equivalent to -C opt-level=2 with optsize/minsize attributes on every function. These changes are not in our current LLVM version, but I think the state in LLVM 22 is already sufficiently good to stabilize.

One thing I would like to see more explicitly discussed is the meaning of #[optimize(size)], which IIRC currently maps to minsize (opt-level=z) rather than optsize (opt-level=s). This choice should be optimized. We could also map to optsize instead, or we could expect both options. I don't have any specific preference here, I just want to make sure we're making a deliberate decision.

[rust-bors]

☔ The latest upstream changes (presumably #157303) made this pull request unmergeable. Please resolve the merge conflicts.