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.

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

[traviscross]

The attribute can be written as {#[optimize(size)] |x| x} or similar, but that seems to have no effect as far as I can tell. So that probably requires some fixing.

FWIW, specifically for closures, I think we should at least make an optimize attribute applied to a closure do something if the closure is not inlined...

As best I can tell, optimize on closures does have effect and seems to work in the way I'd expect (i.e., in the same way as inline). E.g., Godbolt link.

@veluca93: Are we looking at the same thing?

[veluca93]

The attribute can be written as {#[optimize(size)] |x| x} or similar, but that seems to have no effect as far as I can tell. So that probably requires some fixing.

FWIW, specifically for closures, I think we should at least make an optimize attribute applied to a closure do something if the closure is not inlined...

As best I can tell, optimize on closures does have effect and seems to work in the way I'd expect (i.e., in the same way as inline). E.g., Godbolt link.

@veluca93: Are we looking at the same thing?

I think we are looking at similar things, at least - I tried (size) instead of (none) and its behaviour seems a bit less predictable, see https://rust.godbolt.org/z/jGjc51PKn :-)

[traviscross]

I think we are looking at similar things, at least - I tried (size) instead of (none) and its behaviour seems a bit less predictable, see https://rust.godbolt.org/z/jGjc51PKn :-)

What's important here, as best I can tell, is the coercion to a function pointer. Due to that, we're calling the call_once shim. The closure body then gets inlined into that shim which (currently) doesn't inherit the optimize attribute applied to the closure. Setting #[inline(never)] on the closure or calling the closure directly (without coercion to a function pointer) causes the body to stay rolled. See: Godbolt link.

This seems a QoI matter. @veluca93, do you want to look into whether optimize should be inherited by the shim from the closure, or would you prefer to move forward and look into this later?

[veluca93]

I think we are looking at similar things, at least - I tried (size) instead of (none) and its behaviour seems a bit less predictable, see https://rust.godbolt.org/z/jGjc51PKn :-)

What's important here, as best I can tell, is the coercion to a function pointer. Due to that, we're calling the call_once shim. The closure body then gets inlined into that shim which (currently) doesn't inherit the optimize attribute applied to the closure. Setting #[inline(never)] on the closure or calling the closure directly (without coercion to a function pointer) causes the body to stay rolled. See: Godbolt link.

This seems a QoI matter. @veluca93, do you want to look into whether optimize should be inherited by the shim from the closure, or would you prefer to move forward and look into this later?

#157802 ought to fix the problem (I think, not 100% sure that the approach is right).

If we decide that we are OK with optimize(size) mapping to MinSize, and with closures not inheriting the function's optimize attributes, I believe all the issues are resolved, or am I missing something?

[traviscross]

Thanks @veluca93 for pushing forward this work. I propose that we accept this stabilization.

(If we're to accept #157802 or a similar PR, we'll ensure that lands before this PR is merged.)

@rfcbot fcp merge lang,compiler

[rust-rfcbot]

@traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Mark-Simulacrum
• @TaKO8Ki
• @davidtwco
• @estebank
• @jdonszelmann
• @jieyouxu
• @joshtriplett
• @nikomatsakis
• @nnethercote
• @oli-obk
• @petrochenkov
• @scottmcm
• @spastorino
• @tmandry
• @traviscross
• @wesleywiser

Concerns:

• optimize-speed-does-nothing (Stabilize optimize attribute #157273 (comment))

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[mejrs]

Also, can we confirm whether we can write the attribute in a fashion that applies to closures?

Yes, this PR will stabilize the following:

#![feature(optimize_attribute)]

fn main() {
    f(
        #[optimize(size)] || { },
    );
}

fn f(_: impl Fn()) {}

Note the absence of #![feature(stmt_expr_attributes)]. See also #127436 (comment) for more relevant discussion.

[tmandry]

I think it's weird for this attribute not to apply to nested functions, and I finally put my finger on why.

Optimization is a knob you usually set at the level of an entire compilation unit, and in fact, even an entire project. This attribute is great as a way of making that knob more fine-grained. But that implies it should work on modules. In other words, it would be surprising if there was a discontinuity here:

1. Entire project: opt-level in cargo config
2. Compilation unit (crate): -C opt-level to compiler
3. Module: ???
4. Function: #[optimize]

If we accept that #[optimize] should work on a module, we should also apply the attribute to inner functions and closures.

In contrast, there is no equivalent of inline(always) at the whole-program level (thank goodness). Practically speaking, the inline attribute only makes sense at the level of an individual function. I'm sure there are special llvm flags to disable inlining globally and set the inline threshold, but they are not commonly used.

[veluca93]

I think it's weird for this attribute not to apply to nested functions, and I finally put my finger on why.

Optimization is a knob you usually set at the level of an entire compilation unit, and in fact, even an entire project. This attribute is great as a way of making that knob more fine-grained. But that implies it should work on modules. In other words, it would be surprising if there was a discontinuity here:

1. Entire project: opt-level in cargo config
2. Compilation unit (crate): -C opt-level to compiler
3. Module: ???
4. Function: #[optimize]

If we accept that #[optimize] should work on a module, we should also apply the attribute to inner functions and closures.

In contrast, there is no equivalent of inline(always) at the whole-program level (thank goodness). Practically speaking, the inline attribute only makes sense at the level of an individual function. I'm sure there are special llvm flags to disable inlining globally and set the inline threshold, but they are not commonly used.

The intended semantics would be that:

• by default, inner functions (including closures) inherit the optimize attribute from the outer functions
• unless they have an optimize attribute of their own, which takes precedence

Is that correct? If so, I can make a PR for that once #157802 is merged.

This naturally extends to allowing the optimize attribute on modules, but we can leave that to a future extension IMO.

[traviscross]

Practically speaking, the inline attribute only makes sense at the level of an individual function.

I've wanted inline(never) to apply to regions of code for reasons similar to why one might want that for optimize(none). And inline also often makes sense to apply in a consistent way to many functions. I ran across a case of that just the other day in #137598 (comment).

Optimization is a knob you usually set at the level of an entire compilation unit, and in fact, even an entire project. This attribute is great as a way of making that knob more fine-grained. But that implies it should work on modules. In other words, it would be surprising if there was a discontinuity here...

Would you feel the same way if we had attributes to control loop unrolling or vectorization (i.e., some of the underlying things controlled by an optimize(…) attribute along with inlining)? What about for instruction_set?

While I understand the UX intuition that you mention (i.e., that some knobs feel more like compilation-unit ones), when adding attributes to control those, it seems better to me, as a language matter, for our codegen attributes to apply to items in a consistent way rather than having them differ based on those intuitions (which may vary between people).

If we want to support automatic application to nested items, I'd suggest we do that (later) as an orthogonal axis in a syntactically distinct way. As one comparable, that's what GCC does:

// Compile compilation unit with `-O2`.

static int helper(int x) { return x * 7 + 3; }

__attribute__((optimize("O0")))
int with_attribute(int x) {
    int attr_nested(int y) { return helper(y) + 1; } // Still `-O2`.
    return attr_nested(x);
}

#pragma GCC push_options
#pragma GCC optimize ("O0")
int in_region(int x) {
    int region_nested(int y) { return helper(y) + 2; } // This is `-O0`.
    return region_nested(x);
}
#pragma GCC pop_options

Godbolt link.

[petrochenkov]

@rfcbot reviewed
(In assumption that it doesn't apply to nested items.)

[nnethercote]

This seems fine to me, but one thing did surprise me:

• 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 {

Looking at the code I see that #[optimize(speed)] is a no-op. Why is that?

[veluca93]

This seems fine to me, but one thing did surprise me:

• 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 {

Looking at the code I see that #[optimize(speed)] is a no-op. Why is that?

I don't personally know, perhaps @nagisa would? (as - I believe - the person to implement that part)

[nagisa]

There isn't a way to tell LLVM that you want to optimize a single function for speed; or at least there wasn't. My memory is that I had planned to implement this as setting -Copt-level=X crate-wide and then annotating every other function with either optsize or optnone depending on what the original optimization level was. Turned out to be a relatively non-trivial task.

EDIT: though now with the improvements on LLVM side that Nikita mentioned maybe we can do tell LLVM that we want speed pretty much unconditionally and then mark all the functions not otherwise annotated with an #[optimize(...)] with optnone/optsize as needed to implement -Copt-size=0/s/z.

[nnethercote]

There isn't a way to tell LLVM that you want to optimize a single function for speed

Whatever ends up happening, a comment on that empty match arm would be helpful.

[tmandry]

We shouldn't stabilize #[optimize(speed)] if it does nothing in our primary backend.

@rfcbot concern optimize-speed-does-nothing

[veluca93]

We shouldn't stabilize #[optimize(speed)] if it does nothing in our primary backend.

@rfcbot concern optimize-speed-does-nothing

Agreed. @nikic, is there a way to get optimize(speed) to work? How does clang handle GCC-style push-option attributes?

My two cents: it'd be nice to stabilize optimize(speed) if LLVM can easily support it, but if it can't, then I think we should go forward with size and none.

[nikic]

optimize(speed) is supported (or at least it's supposed to be, I haven't verified). The way it currently works is by raising the global optimization level to O2, and not annotating the optimize(speed) functions with minsize. In LLVM 23, the global optimization level will always be O2 by default, which will avoid any kind of weird behavior where using optimize(speed) on one function could impact some other code due to the change to the global optimization level.

tl;dr optimize(speed) is the default from the LLVM perspective, so the implementation of it is to just not add attributes to optimize for size.

[nikic]

See

rust/compiler/rustc_codegen_ssa/src/base.rs

Lines 1101 to 1118 in 8e15021

	providers.backend_optimization_level = |tcx, cratenum| {
	let for_speed = match tcx.sess.opts.optimize {
	// If globally no optimisation is done, #[optimize] has no effect.
	//
	// This is done because if we ended up "upgrading" to `-O2` here, we’d populate the
	// pass manager and it is likely that some module-wide passes (such as inliner or
	// cross-function constant propagation) would ignore the `optnone` annotation we put
	// on the functions, thus necessarily involving these functions into optimisations.
	config::OptLevel::No => return config::OptLevel::No,
	// If globally optimise-speed is already specified, just use that level.
	config::OptLevel::Less => return config::OptLevel::Less,
	config::OptLevel::More => return config::OptLevel::More,
	config::OptLevel::Aggressive => return config::OptLevel::Aggressive,
	// If globally optimize-for-size has been requested, use -O2 instead (if optimize(size)
	// are present).
	config::OptLevel::Size => config::OptLevel::More,
	config::OptLevel::SizeMin => config::OptLevel::More,
	};

for the relevant code.