Skip to content
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.

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

Tracking issue for RFC 2412, "The optimize attribute" #54882

Open
3 of 9 tasks
Tracked by #84
Centril opened this issue Oct 7, 2018 · 21 comments
Open
3 of 9 tasks
Tracked by #84

Tracking issue for RFC 2412, "The optimize attribute" #54882

Centril opened this issue Oct 7, 2018 · 21 comments
Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. B-RFC-implemented Blocker: Approved by a merged RFC and implemented. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC S-tracking-impl-incomplete Status: The implementation is incomplete. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Centril
Copy link
Contributor

Centril commented Oct 7, 2018

This is a tracking issue for the RFC "The optimize attribute" (rust-lang/rfcs#2412).

Steps:

Unresolved questions:

  • Should we also implement optimize(always)? optimize(level=x)?
    • Left for future discussion, but should make sure such extension is possible.
  • 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);
    • This may matter for users of #[optimize(speed)].
  • Are the propagation and unused_attr approaches right?
  • The RFC specifies we should emit an unused_attributes warning when misapplied, but similar attributes emit an error instead. Should this emit an error too?
@Centril Centril added B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. T-lang Relevant to the language team, which will review and decide on the PR/issue. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Oct 7, 2018
@Centril
Copy link
Contributor Author

Centril commented Oct 7, 2018

@nagisa I've added all unresolved questions from the RFC for completeness; the first 2 may already be resolved, if you feel that's the case; tick the boxes.

@nagisa
Copy link
Member

nagisa commented Oct 7, 2018

  • Should we also implement optimize(always)? optimize(level=x)?
    • Left for future discussion, but should make sure such extension is possible.

Marking as resolved just because these never came up during the RFC discussion, especially since the attribute design does not prohibit adding these as a feature later on.


I’m assigning myself for now as I intend to implement this. Failing to dedicate time for this within two or so weeks I pledge to write up some instructions.

@nagisa nagisa self-assigned this Oct 7, 2018
@nagisa
Copy link
Member

nagisa commented Oct 20, 2018

I’ve implementation for optimize(size) now and am looking into ways to implement the harder side – optimize(speed). I might end up splitting the work into two PRs depending on how things go.

@Eh2406
Copy link
Contributor

Eh2406 commented Oct 30, 2018

Would this allow a crate (proptest, quickcheck) that is performance sensitive, but often used in debug builds, to specify that it should always be built with release?

@nagisa
Copy link
Member

nagisa commented Jan 26, 2019

The implementation for optimize(size) and optimize(speed) has now landed. Currently RFC is not fully implemented as the attribute propagation mechanic specified in the RFC is missing from the implementation. Nevertheless the actual functionality is present and can be toyed with.

Another thing that I’ve realised just now is that I never tested and is implemented incorrectly currently is "unused_attribute". The lint does not fire for the attribute applied to non-function items. This should also be fixed before any sort of stabilisation can proceed.

My recent observation is that optimize(none) is fairly often used by people to debug functions in an otherwise optimized build. It may be sensible to add it as well at some point.

@Centril Centril added the B-unstable Blocker: Implemented in the nightly compiler and unstable. label Jan 26, 2019
@jonas-schievink jonas-schievink added B-RFC-implemented Blocker: Approved by a merged RFC and implemented. B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. and removed B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. labels Jun 9, 2019
@Centril
Copy link
Contributor Author

Centril commented Jun 23, 2019

@nagisa Any updates re. the remaining work?

@Jasper-Bekkers
Copy link

My recent observation is that optimize(none) is fairly often used by people to debug functions in an otherwise optimized build. It may be sensible to add it as well at some point.

+1 For this, in C++ code bases I've worked on this was very useful in a few cases;

  • debugging optimized builds (eg. you know which function breaks so you want to toggle some to off just to be able to debug it more easily)
  • assert handlers and other error handling things that you want to be able to quickly inspect in a debugger as a known "all the registers are sane" point in the code.

I'd love to see the optimize(none) attribute for working with large code-bases.

@steveklabnik
Copy link
Member

Triage: not aware of any movements here lately.

@dmitry-zakablukov
Copy link

optimize(none) will be very useful for code virtualization with VmProtect, for example.
It is known that optimized functions are very hard to virtualize, because they can have unexpected returns within, can be inlined, can include inlined code. All of this may lead to faulty code after virtualization or to failure of virtualization itself.
One way to solve this problem is to simplify a code. But very often this is not that easy to do or may be even impossible to do.
The other way is to disable code optimization for a single function that will be virtualized. And for now this is can't be done even with nightly rust compiler.

So please, add support of optimize(none)!

@hydra
Copy link

hydra commented Sep 9, 2021

Would just like to add my two cents to this:

After working on many embedded projects on processors with limited storage space it was critical to be able to change the optimization levels, here's some use-cases:

  • different optimization levels for initialisation code vs code that runs thousands of times a second (e.g. size optimized vs speed optimized, respectively)
  • no optimization on certain functions, e.g. so that you can be sure that the optimizer won't do something unexpected. e.g. when purposely causing a bus fault exception and using the stack frame in the bus fault exception handler.
  • no optimization on certain methods that you want to debug, i.e. everything is compiled with normal optimization except the problem area. this leads to reduced debugging time.

@obsgolem
Copy link
Contributor

Just wanted to hop in to agree that optimize(none) is pretty important for debugging.

@hydra
Copy link

hydra commented Apr 7, 2022

Another two cents:

  • When working with a debugger you only usually care about a small subset of the system. Usually you trust the libraries (i.e. core,std) and external crates but not the code you've just written that you want to debug on a code-storage-constrained embedded system. (i.e. MCU with limited flash space).

Perhaps some options when building crates like 'ignore optimize attribute', 'default', 'debug', 'release', 'size', 'speed', 'none' that can be applied per-dependency which overrides the default optimization level. e.g. you would likely set 'core' and 'std' to 'release', and perhaps a bugged workspace crate to 'debug' and build using '--release' so you can debug the bugged workspace crate and not have everything in the bugged workspace crate optimized away.

  • Often you generate linker .map files and inspect the size of the generated code, or you use a profiler to see which bits should be 'speed' optimized. So it would be awesome to allow the user of external crates to decide the optimization level for modules within a crate, e.g. maybe you want 'core::...::option' to be 'speed' and 'core::...::format' to be 'size'.

@jgarvin
Copy link

jgarvin commented Apr 7, 2022

@hydra Cargo already let's you override the optimization level for dependencies, it's great: https://doc.rust-lang.org/nightly/cargo/reference/profiles.html#overrides

I use it for gamedev so that ggez/bevy/etc are built optimized but my game logic is debug. But per function control is much more surgical and a nice feature.

@hydra
Copy link

hydra commented Apr 7, 2022

@hydra Cargo already let's you override the optimization level for dependencies, it's great: https://doc.rust-lang.org/nightly/cargo/reference/profiles.html#overrides

I use it for gamedev so that ggez/bevy/etc are built optimized but my game logic is debug. But per function control is much more surgical and a nice feature.

@jgarvin cool, i'll check that out. With regard to the per-module comment above I was sort of wanting something between a surgeon's knife (function attribute) and a spoon (dependency level) though. 😃

@joshtriplett joshtriplett added the S-tracking-impl-incomplete Status: The implementation is incomplete. label Jun 8, 2022
@joshtriplett
Copy link
Member

In 2019 @nagisa mentioned that the propagation mechanism was not implemented. Is that still the case?

We discussed this in today's @rust-lang/lang meeting, and we're interested in getting this over the finish line.

We also do see the multiple people asking after optimize(none), and we'd be open to that, but we feel that that needs a follow-up RFC or MCP defining the exact expectations there, since different prospective users of that may have different needs. So we'd like to keep this tracking issue just for what was defined in RFC 2412.

@nagisa
Copy link
Member

nagisa commented Jun 8, 2022

The propagation mechanism still hasn't been implemented and at this point I'm quite confident that it is going to be something we will want to have as a more general mechanism (e.g. this is desirable for attributes such as #[no_coverage] as well.)

My feeling is that we should actually remove the part about propagation from the RFC scope for this specific attribute and work on it separately, defining semantics for propagation that make sense in a broader context. (It might very well be the case that semantics specified in the #[optimize(...)] RFC are the right ones, but I don't think we should take any guesses there)

@ppannuto
Copy link

As one concrete use case for optimize(none): for Tock OS, many of our platforms are sufficiently space-constrained that we must always leave optimizations on in order to build kernel/application images which will fit on the board.

Unfortunately, this can make debugging somewhat challenging. We currently employ a mix of #[inline_never] and #[no_mangle] to help pinpoint troublesome functions, but being able to selectively disable optimizations in certain regions would be a huge debugging help for us. CC @gemarcano who ran into this particular hurdle earlier today, and was hoping for an optimize(none) directive.

@hiroki-chen
Copy link

hiroki-chen commented Mar 7, 2023

Optimization prevention is also strongly related to security issues. To prevent timing side-channel attacks against some cryptographic algorithm implementations, we must ensure some operations are constant-time, i.e., their execution time should not depend on the secret input, if any. OpenSSL, libsodium and many other crypto libraries all implement constant-time functions like swap, sort, etc.

The memory safety and the strong guarantee of type safety provided by the Rust programming language make it a good candidate for implementing security-sensitive applications like blockchain, and zero-knowledge-proof systems. However, the lack of optimization control subtly undermines the robustness of these systems because Rust only has some sort of blackbox function core::hint::black_box() which prevents compiler optimizations in a best-effort way.

The subtle crate provides a best-effort way to implement constant-time crypto algorithms using black_box or read_volatile, but I still wonder if we can take a step further and have control over LLVM's behavior so that we can avoid the boilerplate of writing complex code (e.g., inline assemblies).

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 1, 2024
…compiler-errors

Emit an error if `#[optimize]` is applied to an incompatible item

rust-lang#54882

The RFC specifies that this should emit a lint. I used the same allow logic as the `coverage` attribute (also allowing modules and impl blocks) - this should possibly be changed depending on if it's decided to allow 'propogation' of the attribute.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Aug 1, 2024
Rollup merge of rust-lang#128458 - clubby789:optimize-unused-attr, r=compiler-errors

Emit an error if `#[optimize]` is applied to an incompatible item

rust-lang#54882

The RFC specifies that this should emit a lint. I used the same allow logic as the `coverage` attribute (also allowing modules and impl blocks) - this should possibly be changed depending on if it's decided to allow 'propogation' of the attribute.
@clubby789
Copy link
Contributor

cc #128488 #128581 (comment)
The RFC specifies we should emit an unused_attributes warning when misapplied, but similar attributes emit an error instead.

bors added a commit to rust-lang-ci/rust that referenced this issue Aug 10, 2024
Add `#[optimize(none)]`

cc rust-lang#54882

This extends the `optimize` attribute to add `none`, which corresponds to the LLVM `OptimizeNone` attribute.

Not sure if an MCP is required for this, happy to file one if so.
@the8472
Copy link
Member

the8472 commented Aug 14, 2024

I have opened #129063 which will add some uses of #[optimize(size)] to std.

@peter-lyons-kehl
Copy link
Contributor

For anyone wanting to try (on nightly):

Someone please update "Feature Name" at https://rust-lang.github.io/rfcs/2412-optimize-attr.html.

bors added a commit to rust-lang-ci/rust that referenced this issue Aug 24, 2024
Apply size optimizations to panic machinery and some cold functions

* std dependencies gimli and addr2line are now built with opt-level=s
* various panic-related methods and `#[cold]` methods are now marked `#[optimize(size)]`

Panics should be cold enough that it doesn't make sense to optimize them for speed. The only tradeoff here is if someone does a lot of backtrace captures (without panics) and printing then the opt-level change might impact their perf.

Seems to be the first use of the optimize attribute. Tracking issue rust-lang#54882
bors added a commit to rust-lang-ci/rust that referenced this issue Aug 29, 2024
Apply size optimizations to panic machinery and some cold functions

* std dependencies gimli and addr2line are now built with opt-level=s
* various panic-related methods and `#[cold]` methods are now marked `#[optimize(size)]`

Panics should be cold enough that it doesn't make sense to optimize them for speed. The only tradeoff here is if someone does a lot of backtrace captures (without panics) and printing then the opt-level change might impact their perf.

Seems to be the first use of the optimize attribute. Tracking issue rust-lang#54882
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 1, 2024
Apply size optimizations to panic machinery and some cold functions

* std dependencies gimli and addr2line are now built with opt-level=s
* various panic-related methods and `#[cold]` methods are now marked `#[optimize(size)]`

Panics should be cold enough that it doesn't make sense to optimize them for speed. The only tradeoff here is if someone does a lot of backtrace captures (without panics) and printing then the opt-level change might impact their perf.

Seems to be the first use of the optimize attribute. Tracking issue rust-lang#54882
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 2, 2024
Apply size optimizations to panic machinery and some cold functions

* std dependencies gimli and addr2line are now built with opt-level=s
* various panic-related methods and `#[cold]` methods are now marked `#[optimize(size)]`

Panics should be cold enough that it doesn't make sense to optimize them for speed. The only tradeoff here is if someone does a lot of backtrace captures (without panics) and printing then the opt-level change might impact their perf.

Seems to be the first use of the optimize attribute. Tracking issue rust-lang#54882
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. B-RFC-implemented Blocker: Approved by a merged RFC and implemented. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC S-tracking-impl-incomplete Status: The implementation is incomplete. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests