Fix requires_lto targets needing lto set in cargo

[Flakebi]

Targets that set requires_lto = true were not actually using lto when compiling with cargo by default. They needed an extra lto = true in Cargo.toml to work.

Fix this by letting lto take precedence over the embed_bitcode flag when lto is required by a target.

If both these flags would be supplied by the user, an error is generated. However, this did not happen when lto was requested by the target instead of the user.

Fixes #148514
Tracking issue: #135024

[rustbot]

r? @jieyouxu

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rustbot]

Some changes occurred in src/doc/rustc/src/platform-support

cc @Noratrieb

[jieyouxu]

Hm, not entirely sure of the implications.
@rustbot reroll

[Flakebi]

The CI/tidy complain seems like a false-positive, the test uses --target, just not through //@ compile-flags:. Maybe that lint shouldn’t run on run-make-cargo tests?
(I’m happy to make the change, just want to get confirmation that it’s ok to do that.)

[jieyouxu]

The CI/tidy complain seems like a false-positive, the test uses --target, just not through //@ compile-flags:. Maybe that lint shouldn’t run on run-make-cargo tests? (I’m happy to make the change, just want to get confirmation that it’s ok to do that.)

Yes, I probably forgot to extend the tidy exception to run-make-cargo when I split the previous run-make test suite.

[bors]

☔ The latest upstream changes (presumably #150240) made this pull request unmergeable. Please resolve the merge conflicts.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[Flakebi]

Rebased to fix conflicts with #150237, no other changes

[Mark-Simulacrum]

Is No the right option here? If we're doing only thin-local LTO, I wouldn't expect bitcode to be needed in produced object files... I think? Not super familiar with this area.

[bjorn3]

Indeed. thin-local LTO shouldn't emit bitcode.

[Mark-Simulacrum]

Yeah, perf results seem to confirm this being not what we want :)

[Mark-Simulacrum]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: d1b36b7 (d1b36b73042e93fe4f006b06efda8d0018ccc5bd, parent: 9f54abeebae58244d0d1c692af6f37d2b521e136)

[rust-timer]

Finished benchmarking commit (d1b36b7): comparison URL.

Overall result: ❌ regressions - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	1.3%	[0.1%, 3.6%]	29
Regressions ❌ (secondary)	1.7%	[0.2%, 3.6%]	26
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.3%	[0.1%, 3.6%]	29

Max RSS (memory usage)

Results (primary 0.1%, secondary 2.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.9%	[2.9%, 2.9%]	1
Regressions ❌ (secondary)	2.8%	[1.4%, 4.0%]	4
Improvements ✅ (primary)	-2.7%	[-2.7%, -2.7%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[-2.7%, 2.9%]	2

Cycles

Results (primary 2.1%, secondary 1.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.1%	[2.0%, 2.2%]	3
Regressions ❌ (secondary)	2.3%	[2.1%, 2.5%]	7
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.1%	[-2.1%, -2.1%]	1
All ❌✅ (primary)	2.1%	[2.0%, 2.2%]	3

Binary size

Results (primary 31.4%, secondary 33.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	31.4%	[0.0%, 112.6%]	59
Regressions ❌ (secondary)	33.6%	[0.0%, 120.4%]	51
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	31.4%	[0.0%, 112.6%]	59

Bootstrap: 483.918s -> 491.944s (1.66%)
Artifact size: 390.81 MiB -> 390.86 MiB (0.01%)

[bjorn3]

Would adding a requires_lto check at

rust/compiler/rustc_codegen_ssa/src/back/write.rs

Line 142 in 2caa8fe

|| (sess.opts.cg.linker_plugin_lto.enabled() && !no_builtins)

instead work? Your current code would cause object files containing both machine code and bitcode to be generated rather than standalone bitcode files, even when only the latter is necessary.