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

Should lto = false be treated as LTO "off"? #7491

Open
ehuss opened this issue Oct 7, 2019 · 2 comments
Open

Should lto = false be treated as LTO "off"? #7491

ehuss opened this issue Oct 7, 2019 · 2 comments
Labels
A-lto Area: link-time optimization A-profiles Area: profiles S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@ehuss
Copy link
Contributor

ehuss commented Oct 7, 2019

Currently, a Cargo profile of lto = false is actually "thin local" LTO where LTO is performed across the local crate's codegen units. I'm wondering if that should actually be treated as -C lto=off where LTO is completely disabled?

This would change the behavior for manifests that set lto = false, which shows up occasionally since people copy/paste all the defaults.

Unfortunately there would be no way to explicitly express the default if this was changed, which I think would be a problem (or annoyance). We could add an explicit string ('thin-local'), and translate that to no -C lto flag, or add that to rustc itself.

Another option is to keep the current behavior, and just document that the default of false actually still performs some form of LTO, and that the string "off" should be used to completely turn it off. cc rust-lang/rust#65136 where I am proposing to document these flags on the rustc side.

I kinda lean towards the documentation side, since otherwise it would change behavior for some projects, and probably isn't too important to be absolutely correct here.

@ehuss ehuss added the A-profiles Area: profiles label Oct 7, 2019
@alexcrichton
Copy link
Member

I would personally be fine either way here, but I agree that if we change how we interpret lto=false that we whould add lto = 'thin-local' (or a better name).

@Rua
Copy link

Rua commented Apr 24, 2021

I got pointed here via the bug I submitted above.

I think things would be the clearest for the user if this option didn't accept boolean values at all, as the setting isn't boolean anyway and making it seem like it is, is just confusing. Only the strings full/fat, thin, thin-local and off would be accepted. This ensures that people's existing settings don't get reinterpreted, existing boolean values would give an error instead which is more explicit. full is my own proposal as alternative (or replacement?) for fat.

@epage epage added the S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. label Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lto Area: link-time optimization A-profiles Area: profiles S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

No branches or pull requests

4 participants