The optimize attribute

[nagisa]

This is an RFC that has baked out after receiving feedback on the pre-RFC. Notably optimise(no) has been removed as it is not as important and seemed to have way more contention than optimise(size), while not being as useful or necessary as optimise(size).

Rendered
Tracking issue

[jonas-schievink]

Nice to see progress on this!

[jonas-schievink]

Due to the precedence set by GCC and the fact that the vast majority of conversations I've read use american english spelling ("optimize", "optimizer") I'd prefer that over "optimise".

[Centril]

For sure not both. As sad as this makes me (given that I prefer BrE), use of AmE is standard in the software industry, so we should stick with that.

[jesskfullwood]

At the risk of sounding pedantic (or worse, boring), I feel I should point out that the OED considers 'ize' to be perfectly acceptable and indeed preferred BrE spelling.

[Centril]

@jesskfullwood You always learn something useful; and today is such a day :)

[jonas-schievink]

in some way

I think Clang supports __attribute__((optnone)) syntax for these

[jonas-schievink]

Per-function optimization can also be configured by using #pragma GCC optimize (...).

(see https://gcc.gnu.org/onlinedocs/gcc/Function-Specific-Option-Pragmas.html)

[Centril]

I think so yes. At least, you should be able to specify #[optimize(size)] on mod and impl with the semantics that it applies to every fn inside those transitively.

[clarfonthey]

Counter point: people might assume that optimise(size) at the crate level also asserts repr(packed). We don't do this with inline so I'd assume not for optimise.

[Centril]

I think it would be quite tedious to have to do this on every single function if a module if that is what you want. If we want to be more clear that it is about functions, you could write #[optimize(fn_size)] mod foobar { .. }.

[joshtriplett]

I do think it makes sense to allow #![optimize(size)] in a crate to apply to all functions (that don't override it).

I certainly don't think optimize(size) should ever imply repr(packed), because the latter can affect semantics.

[Havvy]

Two questions.

1. If I apply this attribute to a function that creates a closure, does that closure also get optimized for size?
2. What happens when I apply this attribute to a non-function, such as a struct.

[hanna-kruppe]

If I apply this attribute to a function that creates a closure, does that closure also get optimized for size?

Interesting question. Precedent from inline would be "no" (closures are always inline and not inline(never) regardless of where they are) with the possibility of adding the attribute to the closure expression as well (e.g. #[inline(never)] #[optimize(size)] |x| x + 1). On the other hand, especially if the attribute can be applied to modules, it is very reasonable to expect it to be passed down.

What happens when I apply this attribute to a non-function, such as a struct.

Should be an error. (With the possible exception of closure expressions.)

[nagisa]

Currently attributes that weren't used are warned by a unused_attribute (or some such) lint. I don't see that changing or being any different in this scenario.

…

On Wed, Apr 25, 2018, 11:13 Robin Kruppe ***@***.***> wrote: If I apply this attribute to a function that creates a closure, does that closure also get optimized for size? Interesting question. Precedent from inline would be "no" (closures are always inline and not inline(never) regardless of where they are) with the possibility of adding the attribute to the closure expression as well (e.g. #[inline(never)] #[optimize(size)] |x| x + 1). On the other hand, especially if the attribute can be applied to modules, it is very reasonable to expect it to be passed down. What happens when I apply this attribute to a non-function, such as a struct. Should be an error. (With the possible exception of closure expressions.) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2412 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AApc0hn-DLeqQc09LAJ2mS1yD1-8UqLsks5tsDAygaJpZM4TedOW> .

[hanna-kruppe]

Some misuses of known attributes are hard errors (example). Others aren't, but AFAIK that's mostly for backwards compatibility because rust 1.0 wasn't very comprehensive at identifying all such attributes.

[retep998]

I insist on the American optimize over optimise.

[kornelski]

Most of the functions will be "cold", so I think it would make more sense to use -C opt-level=s to make all of them small, and then add #[optimize(speed)] on the few hot functions.

[nagisa]

Many of the "speed" optimisatios are global (cross-function), so even if implementing such attribute was feasible, it wouldn't have nearly as much effect as the usual -O. Agreed with the sentiment though.

…

On Thu, Apr 26, 2018, 04:50 Kornel ***@***.***> wrote: Most of the code will be "cold", so I think it would make more sense to use -C opt-level=s to make all of them small, and then add #[optimize(speed)] on the few hot functions. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2412 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AApc0vnjL6AE41lBc9Y6CD5B1zDEOCk-ks5tsSfHgaJpZM4TedOW> .

[clarfonthey]

@nagisa some attributes error when used in the wrong position, and I'd say precedent is that this should error when used on a non-function for now.

[nikomatsakis]

@rfcbot fcp merge

It seems like conversation here has reached a fixed point. I personally think exposing these sorts of knobs is a good idea — naturally they should come with no firm promises. I'd like to hear from some members of the Embedded Domain WG (cc @japaric) but I'm going to assume they are in favor. =)

[rfcbot]

Team member @nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged teams:

• @aturon
• @cramertj
• @eddyb
• @joshtriplett
• @nikomatsakis
• @nrc
• @pnkfelix
• @scottmcm
• @withoutboats

Concerns:

• optimize-speed resolved by The optimize attribute #2412 (comment)

Once a majority of reviewers approve (and none object), 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!

See this document for info about what commands tagged team members can give me.

[nikomatsakis]

It might be that this should be more of a @rust-lang/compiler RFC? In any case, cc compiler folks — take a look.

[joshtriplett]

Does it really make sense to globally optimize a program and then tag specific functions as size-optimized? I'd think for the embedded use case, you'd want the opposite: optimize the whole program for size and then tag a few hotspots as performance-optimized.

I don't have a fundamental objection to this, but I'm curious to what degree it reflects "people actively want to use this right now" versus "people kinda think unspecified other people might want a thing kinda like this".

[cramertj]

@joshtriplett I'd definitely appreciate something like this and would use it if it were available, but i agree with you that I think what I really want is the ability to have opt-level=z by default, and opt into performance improvements in areas that need it. Note that @kornelski and @nagisa discussed this above, and it doesn't sound like it's necessarily feasible.

[joshtriplett]

@cramertj That was my expectation as well. Even despite the lack of global optimization, the ability to optimize specific functions for speed ought to help address hotspots found via profiling (with subsequent profiling after adding the attributes to see if it worked).

[joshtriplett]

I've been thinking about this RFC for a while, ever since it was proposed for FCP, and debating whether to check the box for it. After careful discussion:

@rfcbot concern optimize-speed

Most programs needing functionality like this will want global size optimization and selective optimize(speed). It seems exceedingly unlikely that a program will want to globally optimize for performance and selectively optimize a few specific functions for size. (I can imagine very unusual scenarios that might want to use that, but it seems like the far less common case.)

I acknowledge the point that optimize(speed) might make global optimizations harder. However, it would still allow function-local optimizations, and some limited degree of global optimizations. Making optimize(speed) more useful would be the domain of the compiler, and I can imagine people making future improvements or feature requests along the lines of "I used optimize(speed) on this function and I expected the compiler to make this optimization but it didn't". Those are implementation details, not blockers for the concept of optimize(speed).

Based on that, I'd like to propose revising the RFC to define both optimize(speed) and optimize(size).

[nagisa]

I thought about optimise(speed) and I think it could be implemented, but I’m not sure if the implementation approach would be equivalent to the -Copt-level flags, still.

The approach would basically involve always compiling with what is now -Copt-level=2/3 and for -Copt-level=s/z adding the optimise(size) attribute to all functions not annotated with optimise(speed).

[nagisa]

If we decide to go towards optimise(speed) we have to decide what level of opt-level that would refer to, because optimise(size=2) and optimise(size=3) is definitely not something that you could implement in the LLVM backend.

[nikomatsakis]

@nagisa

If we decide to go towards optimise(speed) we have to decide what level of opt-level that would refer to...

This seems like something that users might want to select (e.g., by specifying -Copt-level=s3 or something). For now I think it'd be reasonable to pick an default and reserve the right to tweak it -- I'd go with whatever level of optimization cargo build --release uses by default. In any case, this feels like something that the RFC can plausibly kick down the road as an unresolved question to me(i.e., what mechanism, if any, should we offer to let users override the default?).

[joshtriplett]

I like the idea of having optimize(speed) default to the standard optimization level used by release builds, and then letting users specify the speed optimization level separately if desired.

At the risk of bikeshedding: rather than -Copt-level=s3, how about -Copt-level=z -Copt-level-speed=3? (Likewise for Cargo profile options.)

[nagisa]

@pnkfelix @scottmcm @withoutboats It has been 4 months since the pre-FCP has begun, and your checkboxes are still unchecked without any outstanding concerns from either of you.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[Centril]

@nagisa I see no mention of the treatment of closures in the RFC but it has been discussed in comments without a resolution. Could you please record an unresolved question, in the text, to consider before stabilizing?

EDIT: could you also clarify which unresolved questions are to be resolved during the evaluation/stabilization process and which would need subsequent RFCs?

[nagisa]

I adjusted wording of the RFC to consider closures. Also clarified wording around propagation somewhat which should answer everything /wrt closures. My feeling is that these modifications are sufficiently simple and an obvious extension of the previous wording, but just to be safe, I also added an item in unresolved questions to make sure it ends up in the tracking issue.

could you also clarify which unresolved questions are to be resolved during the evaluation/stabilization process and which would need subsequent RFCs?

I don’t think any of the unresolved questions will need to be discussed for stabilisation, except for the one I added in the most recent commit. They were intended to be more a question of whether we care about those use cases enough for this RFC, rather than that they are outright unresolved.

[Centril]

@nagisa That also works for me :)

[Centril]

PS: If you want to care of the nits in #2412 (comment) I can deal with the merge procedure once the FCP is over.

[vi]

"Rendered" link is 404.

s to z

Would there be an auto-suggestion if mistyped with s?

[Centril]

"Rendered" link is 404.

Fixed. :)

[nagisa]

Would there be an auto-suggestion if mistyped with s?

A better place for this would be some functionality common to attributes in general (similar to how we suggest similar names when variables are misspelled), but that would be another RFC altogether. For now you’d just get an unused attribute warning, I guess.

[Centril]

@nagisa might just be a pure diagnostics issue (e.g. just apply levenshtein) in which case it doesn't need an RFC and the compiler team can just do it?

[nagisa]

@Centril attributes have more subtlety compared to local variables. This is especially notable when syntax plugins can introduce arbitrary attribute names into a program.

[Centril]

@nagisa right; but shouldn't the set of attribute names be in scope such that when you try to apply an attribute, if resolution does not find it in the attr-macro sub-namespace then an error is emitted in which the compiler checks which paths look similar (using the familiar similarity algorithms...)? E.g. shouldn't it be similar to use foo::bar::bez; which doesn't exist because I should have written baz instead?

[nagisa]

@Centril There is no scope for attribute names, they are all global AFAIK. Yet, attributes generally may only be used with certain logical constructs (e.g. optimize can only be used with function-like things). You don’t want to levenstein-suggest to change #[boline] into #[inline] on top of a struct, when there’s also plugin-introduced #[boolinate] which is applicable in that location :)

This is probably not the right place to discuss such a feature in depth. As far as the original question is concerned, I feel that the preexisting unused_attribute lint is an appropriate solution for the scope of this RFC. I’d love something more directed, but I’m of an opinion that such solution would be better off developed separately and in the context of attributes in general, not this specific attribute.

[steveklabnik]

There are some namespaces attributes for tools now, right?

…

On Sep 26, 2018, at 12:52 PM, Simonas Kazlauskas ***@***.***> wrote: @Centril There is no scope for attribute names, they are all global AFAIK. Yet, attributes generally may only be used with certain logical constructs (e.g. optimize can only be used with function-like things). You don’t want to levenstein-suggest to change #[boline] into #[inline] on top of a struct, when there’s also plugin-introduced #[boolinate] which is applicable in that location :) This is probably not the right place to discuss such a feature in depth. As far as the original question is concerned, I feel that the preexisting unused_attribute lint is an appropriate solution for the scope of this RFC. I’d love something more directed, but I’m of an opinion that such solution would be better off developed separately and in the context of attributes in general, not this specific attribute. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[jrpascucci]

@nagisa This may be too late in the process to bring it up, and maybe there's a subtlety I've missed, but in the pre-RFC thread you said "...I don’t think optimise(none) is applicable for the crypto use-case."

The original questioner is correct about side-channel attacks (last rfc I could find about this kind of thing was this one. A particular case would be in implementing countermeasures against timing attacks) Many of these involve doing a bunch of things that a medium smart compiler of any language could trivially detect are not useful to the result and could be thrown away.

I've come across one or two crates that use different contortions to confuse the compiler into avoiding some optimization, and one could recourse to something unsafe, but even sneaky ways may be amenable to optimization and thereafter attack. As a practical instance, it's not clear to me that this crate guaranteeably works now or will in the future. I, for one, would be more confident in it if it had an optimize(none).

Being able to disable optimization of a particular routine as far as one can (which should get one to the same level as debug execution, I would think) should help.

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

[nagisa]

@jrpascucci you want something that would guarantee the properties you seek of your code and as far as I know the only way to get them is to write assembly, optimize(none) does not guarantee anything interesting. If there’s a constant-time operation implemented in Rust or C, that code is not to be trusted by default regardless of the flags on top of the function.

[Centril]

Huzzah! This RFC has been merged!

Tracking issue: rust-lang/rust#54882