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

LTO status pollutes the metadata for dependencies #8343

Open
aidanhs opened this issue Jun 8, 2020 · 3 comments
Open

LTO status pollutes the metadata for dependencies #8343

aidanhs opened this issue Jun 8, 2020 · 3 comments
Labels
A-lto Area: link-time optimization A-rebuild-detection Area: rebuild detection and fingerprinting C-bug Category: bug

Comments

@aidanhs
Copy link
Member

aidanhs commented Jun 8, 2020

Problem

Given a Cargo.toml where the profiles are configured to only differ in LTO, I would expect to be able to share dependencies between different profiles (e.g. via sccache), as I wouldn't typically expect LTO to influence dependencies.

Steps

  1. given the provided cargo.toml in a freshly generated binary project (note only LTO differs) between the profiles
  2. run cargo build --verbose and cargo build --verbose --release and compare the metadata for byteorder
  3. note they aren't the same
[package]
name = "abc"
version = "0.1.0"
authors = ["Aidan Hobson Sayers <aidanhs@cantab.net>"]
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
byteorder = "*"

[profile.dev]
opt-level = 3
debug = false
debug-assertions = false
overflow-checks = false
lto = false
panic = 'unwind'
incremental = false
rpath = false

[profile.dev.package."*"]
opt-level = 3
debug = false
debug-assertions = false
overflow-checks = false
#lto = false # can't specify this for packages
#panic = 'unwind' # can't specify this for packages
incremental = false
#rpath = false # can't specify this for packages

[profile.release]
opt-level = 3
debug = false
debug-assertions = false
overflow-checks = false
lto = true # NOTE THIS LINE
panic = 'unwind'
incremental = false
rpath = false

Possible Solution(s)

After a quick look - metadata is derived from comparable() on a Profile, which looks at the profile lto, rather than the computed lto for that unit. A solution could be to use the computed lto instead.

Alternatively, this issue may just be a wontfix given that we now would expect lto to make dependencies look different (once #8337 is fixed).

This said - even if it is a wontfix, it may be worth using the computed lto anyway as it's strictly speaking more reflective of the logic within cargo than using the profile lto (i.e. if you switch the lto option in your profile but it doesn't actually have an effect on the unit, you don't need to recompile).

Notes

Output of cargo version:

  • cargo 1.44.0 (05d080f 2020-05-06)
@aidanhs aidanhs added the C-bug Category: bug label Jun 8, 2020
@alexcrichton
Copy link
Member

This is actually an intentional change for #8192 where specifying LTO or not will change how dependencies are built (whether they produce object code or just bitcode). In that sense libraries can no longer be shared between LTO and non-LTO builds.

@aidanhs
Copy link
Member Author

aidanhs commented Jun 8, 2020

Indeed I did spot #8192 when digging around the code, but I realised this issue is relevant even with that change.

For example - rather than lto = false, imagine lto = "thin" in the Cargo.toml above for the dev profile. Because metadata is currently generated from the profile lto rather than than the computed lto, this metadata is mismatched even though it could be reused.

@alexcrichton
Copy link
Member

Ah yes in that case we don't share artifacts. We could support more sharing of libraries between thin vs fat LTO for sure.

@ehuss ehuss added A-lto Area: link-time optimization A-rebuild-detection Area: rebuild detection and fingerprinting labels Jun 25, 2020
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-rebuild-detection Area: rebuild detection and fingerprinting C-bug Category: bug
Projects
None yet
Development

No branches or pull requests

3 participants