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

[crater] Try enabling -Zgcc-ld=lld #96025

Closed
wants to merge 2 commits into from
Closed

[crater] Try enabling -Zgcc-ld=lld #96025

wants to merge 2 commits into from

Conversation

lqd
Copy link
Member

@lqd lqd commented Apr 13, 2022

To make progress towards using lld, this PR will test a build-and-test crater run enabling -Zgcc-ld, to see if it would cause issues on x86_64-unknown-linux-gnu.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 13, 2022
@rust-highfive
Copy link
Collaborator

r? @fee1-dead

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 13, 2022
@lqd
Copy link
Member Author

lqd commented Apr 13, 2022

r? @ghost

@bors try

@bors
Copy link
Contributor

bors commented Apr 13, 2022

⌛ Trying commit 05a2b94 with merge 1b74e096b9bfb06f84a3007193dcd2f059cbdf6a...

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-12 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Successfully built 4a28cf23dce6
Successfully tagged rust-ci:latest
Built container sha256:4a28cf23dce681dbde4f9adda00df7b44cb98b06495c74e851d3c9c67f750ef1
Uploading finished image to https://ci-caches.rust-lang.org/docker/7a933025cb171c4b5f5b6011a3583795b8dcc545c75914dd6be8b89a1706a0990abf8b940dcd0d758184ddd671b7cd225927f49feaa72b37c79d66fed681b775
upload failed: - to s3://rust-lang-ci-sccache2/docker/7a933025cb171c4b5f5b6011a3583795b8dcc545c75914dd6be8b89a1706a0990abf8b940dcd0d758184ddd671b7cd225927f49feaa72b37c79d66fed681b775 Unable to locate credentials
[CI_JOB_NAME=x86_64-gnu-llvm-12]
---
   Compiling std_detect v0.1.5 (/checkout/library/stdarch/crates/std_detect)
   Compiling miniz_oxide v0.4.0
   Compiling hashbrown v0.12.0
   Compiling addr2line v0.16.0
error: rust-lld (as ld) not found
error: could not compile `std` due to previous error
Build completed unsuccessfully in 0:04:52

@bors
Copy link
Contributor

bors commented Apr 13, 2022

☀️ Try build successful - checks-actions
Build commit: 1b74e096b9bfb06f84a3007193dcd2f059cbdf6a (1b74e096b9bfb06f84a3007193dcd2f059cbdf6a)

@lqd
Copy link
Member Author

lqd commented Apr 14, 2022

Tested locally with a helloworld, cargo clean && cargo +1b74e096b9bfb06f84a3007193dcd2f059cbdf6a build -q && readelf -p .comment target/debug/helloworld seemed to work as expected:

String dump of section '.comment':
  [     0]  Linker: LLD 14.0.0
  [    13]  GCC: (Debian 10.2.1-6) 10.2.1 20210110

I'll run it locally again, over our extended benchmark suite (for the few crates that link and execute code).

In the meantime, we also can make it go through the perf collector to see if something goes horribly wrong with the benchmarks that do involve linking.

(It's somewhat expected that bootstrapping with this PR would require rust-lld to be built: it is my understanding that is not done by default on the PR CI builder, or on the perf collector when gathering bootstrap times)

@rust-timer build 1b74e096b9bfb06f84a3007193dcd2f059cbdf6a

@rust-timer
Copy link
Collaborator

Queued 1b74e096b9bfb06f84a3007193dcd2f059cbdf6a with parent 0d13f6a, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (1b74e096b9bfb06f84a3007193dcd2f059cbdf6a): comparison url.

Summary:

  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: 🎉 relevant improvements found
Regressions 😿
(primary)
Regressions 😿
(secondary)
Improvements 🎉
(primary)
Improvements 🎉
(secondary)
All 😿 🎉
(primary)
count1 0 0 16 77 16
mean2 N/A N/A -47.3% -24.6% -47.3%
max N/A N/A -68.2% -67.1% -68.2%

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.
Warning ⚠: The following benchmark(s) failed to build:

  • rustc

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

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

Footnotes

  1. number of relevant changes

  2. the arithmetic mean of the percent change

@lqd
Copy link
Member Author

lqd commented Apr 22, 2022

According to rust-lang/compiler-team#510 (comment) there should be at least a few projects with incorrect build.rs even on x64 linux.

@craterbot run mode=build-and-test

The crater queue is empty enough that this won't delay anyone now. It can be de-prioritized if anyone has a more pressing run to do, like the ones for the betas.

@craterbot
Copy link
Collaborator

👌 Experiment pr-96025 created and queued.
🤖 Automatically detected try build 1b74e096b9bfb06f84a3007193dcd2f059cbdf6a
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 22, 2022
@craterbot
Copy link
Collaborator

🚧 Experiment pr-96025 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-96025 is completed!
📊 113 regressed and 1001 fixed (228240 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Apr 27, 2022
@lqd
Copy link
Member Author

lqd commented May 12, 2022

The crater queue is empty, so let's quickly check if some of the flakiness in the unrelated regressions reproduces.

@craterbot run mode=build-and-test crates=https://crater-reports.s3.amazonaws.com/pr-96025/retry-regressed-list.txt

@craterbot
Copy link
Collaborator

👌 Experiment pr-96025-1 created and queued.
🤖 Automatically detected try build 1b74e096b9bfb06f84a3007193dcd2f059cbdf6a
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 12, 2022
@craterbot
Copy link
Collaborator

🚧 Experiment pr-96025-1 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added the S-waiting-on-crater Status: Waiting on a crater run to be completed. label May 12, 2022
@craterbot
Copy link
Collaborator

🎉 Experiment pr-96025-1 is completed!
📊 47 regressed and 10 fixed (113 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels May 12, 2022
@lqd
Copy link
Member Author

lqd commented May 12, 2022

And since the queue is still empty, let's now check for flakiness in the fixed cases.

@craterbot run mode=build-and-test crates=https://gist.githubusercontent.com/lqd/03ab4552c58faa97ff33a9b3f2f1dc68/raw/5427f95aa75dfb1e01202d9864641b96cf5ac015/pr-96025-retry-fixed-list.txt

@craterbot
Copy link
Collaborator

👌 Experiment pr-96025-2 created and queued.
🤖 Automatically detected try build 1b74e096b9bfb06f84a3007193dcd2f059cbdf6a
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 12, 2022
@craterbot
Copy link
Collaborator

🚧 Experiment pr-96025-2 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-96025-2 is completed!
📊 6 regressed and 95 fixed (1000 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels May 12, 2022
@Mark-Simulacrum
Copy link
Member

I'm going to close this PR but we can always reopen and re-run crater when/if needed, seems like we finished the initial collection though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants