chore: bump to cc@1.4.3 for rustc_llvm and library

[weihanglo]

View all comments

What is this?

Old cc-rs derives this from the -Clto rustflag on its own.
cc@1.2.39 starts gating that behind -Clinker-plugin-lto,
which bootstrap doesn't pass.
Therefore,
we need to pass this flag explicitly to keep LTO mode

Previous efforts:

• Update cc-rs to 1.2.39 #146186
• Update the cc crate for rustc_llvm #155438

I personally want this because of cc 1.3.0+ has the support of Cargo -Ztrim-paths,
which helps what I am experimenting in #161049.

How to review

Commit by commit.

To keep commits bisect-able,
I added the first commit without cc bump.
clang should be fine with duplicate -flto flags.

One thing I am not certain is whether we should probe -flto flag in this case,
or just make this fail if -flto isn't supported.
(I assume -flto is quote widely supported)

r? Kobzol

---

🤖 LLM disclosure: I used LLM for the experiment of #161049, but not the bootstrap LTO change in this PR.

[rustbot]

Kobzol is not on the review rotation at the moment.
They may take a while to respond.

[weihanglo]

@bors try @rust-timer queue

and cc @bjorn3 @Kobzol

[rust-bors]

☀️ Try build successful (CI)
Build commit: 7256718 (7256718699c6a701d58e50866ab3f86a2ffa8cef)
Base parent: 6d656b1 (6d656b1efca82491a110b476ac9bbe712628ccb6)

[rust-timer]

Finished benchmarking commit (7256718): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

This perf run didn't have relevant results for this metric.

Cycles

This perf run didn't have relevant results for this metric.

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 468.493s -> 459.635s (-1.89%)
Artifact size: 398.93 MiB -> 398.87 MiB (-0.01%)

[weihanglo]

@Kobzol od you know why there is no result? I guess because GitHub was down?

[weihanglo]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: 52a6c34 (52a6c343ae344ec730508ef19f1eb62167f1b4ad)
Base parent: e702eca (e702ecae8a3e3756e8a499f8592be1e3efac0e8f)

[rust-timer]

Finished benchmarking commit (52a6c34): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (secondary 1.6%)

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

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.6%	[0.6%, 2.5%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

This perf run didn't have relevant results for this metric.

Binary size

Results (primary 0.1%, secondary 0.1%)

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

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.0%, 0.1%]	75
Regressions ❌ (secondary)	0.1%	[0.0%, 0.1%]	114
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[0.0%, 0.1%]	75

Bootstrap: 457.751s -> 457.734s (-0.00%)
Artifact size: 398.90 MiB -> 398.92 MiB (0.00%)

[weihanglo]

Ah. I see. No relevant largely means insignificant. Then I think this is ready.

@rustbot ready

[weihanglo]

@bors try jobs=dist-aarch64-linux @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: a5859e2 (a5859e2a19c9cca181acab527d5003db8ae6e560)
Base parent: f7d782a (f7d782a3be46d6bb4b9792fe69a61db389ba1769)

[rust-timer]

Finished benchmarking commit (a5859e2): comparison URL.

Overall result: no relevant changes - BENCHMARK(S) FAILED

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

❗ ❗ ❗ ❗ ❗
Warning ⚠️: The following benchmark(s) failed to build:

• Job failure

❗ ❗ ❗ ❗ ❗

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

This perf run didn't have relevant results for this metric.

Cycles

This perf run didn't have relevant results for this metric.

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: missing data
Artifact size: missing data

[Kobzol]

(The try build didn't contain artifacts for x64, so the perf. run failed).

[weihanglo]

Ah. I guessed that. How do we want to proceed?

Looked over dist-aarch64-linux build logs of #161347, it has never got LTO applied, as the old CC failed on probe and didn't pass -fLTO flag.

warning: rustc_llvm@0.0.0: Inherited flag "-flto=thin" is not supported by the currently used CC

Do we want to make perf run support aarch64 to start with?

[Kobzol]

Do we want to make perf run support aarch64 to start with?

Umm, that is actually implemented already, but we don't have any ARM hardware upon which to run perf. on.

I'm not sure if I understand the question though. Is there something blocking this?

[weihanglo]

I'm not sure if I understand the question though. Is there something blocking this?

@Kobzol I added this fix 2f9f649, but I am not certain about it. And maybe there are more dockerfile/image need the same tweaks. We may need a systematic fix.

[Kobzol]

That seems fine, we use the same thing for x64. As long as the build passes, we should be good. Further failures we can only get from the ecosystem.

[weihanglo]

@rustbot ready

[weihanglo]

That seems fine, we use the same thing for x64. As long as the build passes, we should be good. Further failures we can only get from the ecosystem.

Considered this as a green light :)

@bors r=Kobzol

[rust-bors]

📌 Commit 055eace has been approved by Kobzol

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 10. This pull request will be tested once the tree is reopened.

Reason for tree closure: Main branch is broken

[rust-bors]

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

This pull request was unapproved.