-
Notifications
You must be signed in to change notification settings - Fork 13k
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 opt-level={s,z} #35784
Comments
Also note that rust-lang/cargo#3007 is adding Cargo support, currently through |
I don't even think there is a single word that describes what |
|
@matklad is right, "s" and "z" are not clear at all. I had no idea what "z" even did until I looked it up. Apparently it is the same as "s" but without loop vectorisation. According to the BSD man page it is intended to reduce size even further. I like the "size-1"/"size-2" idea. |
We are potentially interested in this for Gecko's integration of Rust code, see [1]. We'd need to do more measurements to determine if the performance impact is acceptable though. |
Team member @alexcrichton has proposed to merge this. The next step is review by the rest of the tagged teams:
No concerns currently listed. Once these reviewers reach consensus, 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. |
IS this properly documented? That can come after we've approved for stability, right? |
@nrc good point! I think we should definitely make sure it's documented at least in |
@nrc yes, that should come before the final PR itself lands though. |
It seems like there was an open concern that these tokens (especially for Cargo) aren't very clear. Do we want to take a short at single-word tokens before stabilizing? |
@wycats I'm not personally motivated to do so as this is such a niche feature it seems like it's not worth the effort, but others may feel differently. |
I feel differently! If I change the patch to |
@Timmmm I think we're definitely still able to change, we just might have to have a small period of time where we support everything to deprecate s/z |
@alexcrichton @Timmmm is there any particular reason that, for Cargo, we need more than just As far as I can tell, Cargo really only needs a way for people to say "I want you to optimize for size, please" and having multiple different versions of "optimize for size" at the Cargo level seems likely to introduce more confusion than help. I propose that we add |
Sounds plausible to me! In that case sounds like there's work to do, so I'll cancel the FCP @rfcbot fcp cancel |
@alexcrichton proposal cancelled. |
@wycats That's not what
Also see my previous link. By the way, there is also yet another optimisation setting!
Sounds like it could be useful. I suggest |
I came here because I wasn't sure what -Oz did - now I know. I would like to point out that "size1" and "size2" is not much better, imho. Ok, so there's a size difference. But which one is supposed to be smaller? The bigger number, because it tries more agressively to make it smaller, or the smaller number, for intuitive reasons? I can see the parallel with O1, O2, O3 -> increasing optimization for speed, and S1, S2, S3 -> increasing optimization for size, but perhaps this should be made more explicit then? We can always keep "s", "o", and so on as shorthand aliases |
Hello This seems to have gone to sleep. I mean, the name for it is important, but no discussion seems to be happening for like 9 months ‒ is it likely to move forward? As for me, I'd rather have the feature stabilized (I'm often aiming at devices with chronic lack of storage space), no matter the actual values used. Or is it blocked on anything else beside choosing the right name? |
Side note: the crash in #48967 only reproduces with -Os (probably an LLVM bug) |
Since the lastest LLVM upgrade rustc / LLVM does more agressive As we are shooting for embedded Rust on stable it would be great to also provide a way to optimize for size (which is pretty important for embedded applications that target resource constrained devices) on stable. @alexcrichton What can we do to move this forward. Can we at least stabilize "s" on rustc? Cargo already allows @aturon is this (opt-level) being discussed as part of the Cargo profile revamp? |
@japaric I think those optimizations have baked long enough that it should be fine to stabilize now personally! |
stabilize opt-level={s,z} closes #35784 closes #47651 ### Rationale Since the lastest LLVM upgrade rustc / LLVM does more agressive loop unrolling. This results in increased binary size of embedded / no_std programs: a hundreds of bytes increase, or about a 7x increase, in the case of the smallest Cortex-M binary cf. #49260. As we are shooting for embedded Rust on stable it would be great to also provide a way to optimize for size (which is pretty important for embedded applications that target resource constrained devices) on stable. Also this has been baking in nightly for a long time. r? @alexcrichton which team has to sign off this?
Added in #32386 these options are still unstable, this issue will track their progress to becoming stable!
The text was updated successfully, but these errors were encountered: