Stabilize optimize attribute

[veluca93]

View all comments
FCP proposal and checkboxes

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.

Note

Concerns (0 active)

• are-we-happy-wiht-capability-and-scope-of-this-attribute resolved in this comment
• meaning-of-optimize-size resolved in this comment
• explicitly-decide-on-effect-on-nested-functions resolved in this comment

Managed by @rustbot—see help for details.

[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

[nagisa]

Inheritance of this attribute was also a part of the original rfc that introduced the attribute: https://github.com/rust-lang/rfcs/blob/master/text/2412-optimize-attr.md#reference-level-explanation (4th paragraph.)

[traviscross]

Inheritance of this attribute was also a part of the original rfc that introduced the attribute: https://github.com/rust-lang/rfcs/blob/master/text/2412-optimize-attr.md#reference-level-explanation (4th paragraph.)

(It was left as an unresolved question.)

[scottmcm]

@rfcbot concern optimize-none-not-being-none-muddles-motivation

Ralf pointed out in #128657 (comment) that the PR implementing optimize(none) actually force-enabled some optimization passes that can remove UB.

If it's not "none" (and I guess also different from opt-level=0 or that change wouldn't have been needed?) but "well, sorta none but also optimized enough that operations on ZSTs don't make it to codegen (and maybe more things)", now I'm puzzled what the point of it is.

Did people actually want optimize(debug) or something?

[veluca93]

@rfcbot concern optimize-none-not-being-none-muddles-motivation

Ralf pointed out in #128657 (comment) that the PR implementing optimize(none) actually force-enabled some optimization passes that can remove UB.

If it's not "none" (and I guess also different from opt-level=0 or that change wouldn't have been needed?) but "well, sorta none but also optimized enough that operations on ZSTs don't make it to codegen (and maybe more things)", now I'm puzzled what the point of it is.

Did people actually want optimize(debug) or something?

FWIW I am significantly more interested in optimize(size) and optimize(speed) -- enough that I'd be happy to skip stabilizing optimize(none) for now.

(And among those, I moreover care more about optimize(size))

[veluca93]

@rfcbot concern optimize-none-not-being-none-muddles-motivation

Ralf pointed out in #128657 (comment) that the PR implementing optimize(none) actually force-enabled some optimization passes that can remove UB.

If it's not "none" (and I guess also different from opt-level=0 or that change wouldn't have been needed?) but "well, sorta none but also optimized enough that operations on ZSTs don't make it to codegen (and maybe more things)", now I'm puzzled what the point of it is.

Did people actually want optimize(debug) or something?

Reading back the comments on the tracking issue, it seems that people do indeed want optimize(debug). I don't have especially strong opinions on the naming, but I will note that optimize(none) matches the C++ name for equivalent functionality (https://clang.llvm.org/docs/AttributeReference.html#optnone, which does not guarantee no optimizations either)

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[tmandry]

@rustbot label I-lang-nominated

See #157273 (comment). Nominating for team guidance on whether to ship this as optimize(none), optimize(debug), or leave it out for the current stabilization.

[clubby789]

Ralf pointed out in #128657 (comment) that the PR implementing optimize(none) actually force-enabled some optimization passes that can remove UB.

Just for current context, that issue was actually unrelated to is_required/optimize(none): rust-lang/miri#5226 (comment)
The semantics of optimize(none) are being worked on in #160015 and #160057

[RalfJung]

The semantics of optimize(none)

Yeah so that's the question I was about to bring up here. :)

Should optimize(none) be like -Copt-level=0 or should it be somehow even less optimized than that? Personally it feels to me like -Copt-level=0 makes most sense. (That would be -Zmir-opt-level=1 at the moment but that exact mapping is an unstable implementation detail.)

FWIW the same question comes up for optimize(speed). Is that like -O? (I'm not even sure how that one is currently implemented.)

[rust-bors]

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

[scottmcm]

We talked about my fuzzy concern (that I didn't want to leave open for long) in the lang triage meeting today.

There was general agreement that there's no intent here to create a "less than -C opt-level=0" category here: optimize(none) should be requesting one of the tiers that already exists, not creating a new one. For example: existing things like the always-on analysis pass in cg_ssa is not expected to be removed by this; that optimize(none) should still be running the same -Z mir-opt-level=1 passes as -C opt-level=0 already does; etc. For example, it's explicitly not a bug if someone's code "breaks" in optimize(none) because they have UB.

@rfcbot concern make-optimize-none-be-c-opt-level-0

TC is going to post a separate comment with a more lang-like phrasing of how to describe this. My one above is admittedly too compiler-implementation-terminology.

@rfcbot resolve optimize-none-not-being-none-muddles-motivation

[traviscross]

From a lang perspective, I think about the intent we write down in the Reference for these attributes. In that respect, I interpret optimize(none) as follows:

If you compile a program with -Copt-level=3, that's equivalent to applying to each function in the program an (unnameable) attribute that asks for the "level 3" optimizations. If you then write #[optimize(none)] on one function, that's equivalent to removing the other attribute from that function.

That is, optimize(none) says "I do not request any specific optimizations for this function". The compiler is, of course, always free to optimize at its discretion. optimize(none) simply means the programmer isn't asking for any, and if the ask was made globally, this retracts the ask for that function.

---

In the meeting, there was general agreement about this interpretation.

[RalfJung]

That sounds like a kind of unnecessarily complicated way of saying "behaves like -Copt-level=0 for a single function"?

(Note that there are other values for optimize, not just none.)

[RalfJung]

Maybe we should just spell this as #[optimize(level = 0)] or #[optimize(level0)] to make the parallel with -Copt-level more clear?

[veluca93]

FWIW, the current wording in the reference PR stays vague on purpose (something like "suggests the compiler to apply as few optimizations as possible").

Given that the semantics of optimize(none) are more controversial than the rest, I wonder whether we should remove optimize(none) from the scope of this stabilization PR. IMO optimize(size) is significantly more useful and that seems to be less controversial.

[RalfJung]

"As few as possible" sounds stronger than what we say about -Copt-level=0. I'd just say "apply few optimizations (similar to -Copt-level=0)".

[RalfJung]

After chatting with TC I now understand why I was so confused by his wording -- there's actually a difference in what we think we should say about #[optimize(none)]. I would treat it very similar to the other optimize(_), as a request for the compiler to move generally in a certain direction in the trade-off space of optimizations. TC would treat #[optimize(none)] as the absence of a request.

I think we both would have it actually do the same thing in the compiler implementation, this is a docs-only difference. Not sure if it has to block stabilization since the docs can change without the implementation changing. ;) But personally I would find it odd to treat #[optimize(none)] fundamentally different from #[optimize(speed/size)]

[tmandry]

I would treat it very similar to the other optimize(_), as a request for the compiler to move generally in a certain direction in the trade-off space of optimizations.

I came in with the intuition that none should be treated similarly to the others. What pushed me away from it being a "direction" is that it's hard to define what that direction is, and the name none doesn't imply one. It's the thing the compiler does by default, which favors compile times and debugging without being entirely optimized for either one.

Maybe we should just spell this as #[optimize(level = 0)] or #[optimize(level0)] to make the parallel with -Copt-level more clear?

There are benefits to that, but currently we have consensus on the lang team for a more semantic interpretation. The benefit of the semantic approach is it's less tied to a compiler implementation and it doesn't imply a one-dimensional goal, like a number does.

In the future the compiler could accept -Copt-level=none, just like it accepts -Cdebuginfo=none.

Given that the semantics of optimize(none) are more controversial than the rest, I wonder whether we should remove optimize(none) from the scope of this stabilization PR. IMO optimize(size) is significantly more useful and that seems to be less controversial.

It looks like we have consensus to ship optimize(none) that behaves like opt-level=0. If it turns out that's not the case, I agree taking it out is valid as a fallback option.

[veluca93]

Given that the semantics of optimize(none) are more controversial than the rest, I wonder whether we should remove optimize(none) from the scope of this stabilization PR. IMO optimize(size) is significantly more useful and that seems to be less controversial.

It looks like we have consensus to ship optimize(none) that behaves like opt-level=0. If it turns out that's not the case, I agree taking it out is valid as a fallback option.

Ok. How is that different from what's happening now, and what needs to change compiler-side for that to be the case? I am not familiar enough with llvm/mir optimizations to be able to tell.

[RalfJung]

I came in with the intuition that none should be treated similarly to the others. What pushed me away from it being a "direction" is that it's hard to define what that direction is, and the name none doesn't imply one. It's the thing the compiler does by default, which favors compile times and debugging without being entirely optimized for either one.

That sounds like a direction to me. We already have -Copt-level=0 to define basically the same thing. I don't think the fact that opt-level defaults to 0 has any significance here. There's no point in the tradeoff space that's naturally special, we just just decided that the default direction is "favor compile times and debuggability over runtime performance". We could have just as well picked a different default opt-level.

You have to pick some area of the tradeoff space, you cannot refuse to make a choice. In that sense optimize(none) is a choice. Maybe it should be called optimize(compiletime) instead to make that more clear...

Ok. How is that different from what's happening now, and what needs to change compiler-side for that to be the case? I am not familiar enough with llvm/mir optimizations to be able to tell.

For MIR passes, optimize(none) currently disables way more optimizations than opt-level=0. @clubby789 has been working on refactoring our pass handling. The natural next step would be to replace the sess: &Session in fn policy with something that also has access to the per-function data, so that when a pass accesses the mir-opt-level that takes into account the per-function attribute.

I have no idea what the attribute does on the LLVM side or how we even forward it to LLVM.

Also, I just realized an additional wrinkle: IMO, #[optimize(speed)] should be ignored in Miri. Otherwise, Miri might fail to detect UB in such functions, and I don't think that's what we want. So either -Zmir-opt-level overwrites #[optimize(speed)] or we need a new flag that tells the compiler to ignore #[optimize(speed)].