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

CI: build CMake 3.20 to support LLVM 17 #113714

Merged
merged 2 commits into from
Jul 17, 2023
Merged

CI: build CMake 3.20 to support LLVM 17 #113714

merged 2 commits into from
Jul 17, 2023

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Jul 15, 2023

LLVM 17 will require CMake at least 3.20, so we have to go back to building our own CMake on the Linux x64 dist builder.

r? @nikic

@Kobzol
Copy link
Contributor Author

Kobzol commented Jul 15, 2023

@bors try

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Jul 15, 2023
@bors
Copy link
Contributor

bors commented Jul 15, 2023

⌛ Trying commit 224f7af769cab428407d8aa1a1a89016d4b14166 with merge 925c053abfb4df2511f5c69a1a21db5e3c09f87a...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Jul 15, 2023

☀️ Try build successful - checks-actions
Build commit: 925c053abfb4df2511f5c69a1a21db5e3c09f87a (925c053abfb4df2511f5c69a1a21db5e3c09f87a)

@Kobzol
Copy link
Contributor Author

Kobzol commented Jul 15, 2023

@rust-timer build 925c053abfb4df2511f5c69a1a21db5e3c09f87a

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (925c053abfb4df2511f5c69a1a21db5e3c09f87a): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

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.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
18.3% [0.5%, 36.8%] 250
Regressions ❌
(secondary)
16.8% [0.4%, 48.6%] 258
Improvements ✅
(primary)
-1.7% [-3.6%, -0.3%] 6
Improvements ✅
(secondary)
-0.5% [-0.6%, -0.4%] 2
All ❌✅ (primary) 17.8% [-3.6%, 36.8%] 256

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
3.6% [0.4%, 14.7%] 232
Regressions ❌
(secondary)
5.3% [0.4%, 16.5%] 243
Improvements ✅
(primary)
-2.0% [-6.7%, -0.4%] 14
Improvements ✅
(secondary)
-0.8% [-1.2%, -0.6%] 3
All ❌✅ (primary) 3.3% [-6.7%, 14.7%] 246

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
20.0% [0.5%, 56.3%] 265
Regressions ❌
(secondary)
20.7% [0.6%, 77.0%] 260
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.5% [-0.5%, -0.5%] 1
All ❌✅ (primary) 20.0% [0.5%, 56.3%] 265

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 657.713s -> 705.864s (7.32%)

@rustbot rustbot added the perf-regression Performance regression. label Jul 15, 2023
@Kobzol
Copy link
Contributor Author

Kobzol commented Jul 15, 2023

(The above perf results are for using the trunk commit of LLVM as the host compiler).

😁 Looks like updating to LLVM 17+ (as the host compiler) will be fun. But we don't need to deal with that now.

@rustbot ready

@Kobzol Kobzol marked this pull request as ready for review July 15, 2023 14:18
@nikic
Copy link
Contributor

nikic commented Jul 15, 2023

Those results are certainly wild ... the largest regressions are on check builds, while I would have assumed that the host compiler shouldn't affect rustc itself much.

My first guess was that the new llvm-profdata can't handle the profiles generated by LLVM 16 and we lose PGO on rustc, but at least the log doesn't indicate something like that.

@Kobzol
Copy link
Contributor Author

Kobzol commented Jul 15, 2023

My guess was also the profiles, but as you said, logs look OK. BOLT could become worse, but such a large effect definitely shouldn't be BOLT alone. And it's not used in check builds anyway.

Debug/Opt builds could become worse if LLVM 17 optimized src/llvm-project worse, but why are checks builds bad, that's a mastery.

@Kobzol Kobzol force-pushed the ci-cmake branch 2 times, most recently from 70a1b93 to c6aadf4 Compare July 15, 2023 16:34
@Kobzol
Copy link
Contributor Author

Kobzol commented Jul 15, 2023

I went through the builders modified in #95026, most of them now use Ubuntu 22.04, which has new enough CMake (3.22). I updated the rest. I'm not sure which builders do actually build LLVM, so if I missed some, we'll probably see it when we bump LLVM to 17.

@nikic
Copy link
Contributor

nikic commented Jul 15, 2023

I went through the builders modified in #95026, most of them now use Ubuntu 22.04, which has new enough CMake (3.22). I updated the rest. I'm not sure which builders do actually build LLVM, so if I missed some, we'll probably see it when we bump LLVM to 17.

Doesn't everything build LLVM on commits that update LLVM?

Edit: Well, apart from x86_64-gnu-llvm-* and mingw-check*.

@Kobzol Kobzol force-pushed the ci-cmake branch 2 times, most recently from d0f4bde to ea8efd8 Compare July 15, 2023 16:52
@Kobzol
Copy link
Contributor Author

Kobzol commented Jul 15, 2023

Good point. I went through the non-dist builders and added CMake where they use Ubuntu 20.04. I wonder if we should just upgrade them to 22.04..

@nikic
Copy link
Contributor

nikic commented Jul 15, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Jul 15, 2023

📌 Commit ea8efd863a236742c2258aec66e90a490c0add19 has been approved by nikic

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 15, 2023
@Kobzol
Copy link
Contributor Author

Kobzol commented Jul 17, 2023

Ok, I tried linking to it. I also added netbsd as a temporary PR CI job to test if it works.

@rust-log-analyzer

This comment has been minimized.

@Kobzol
Copy link
Contributor Author

Kobzol commented Jul 17, 2023

Ok, looks like that has worked! (The failure is just being unable to upload artifacts to S3 from a PR CI run).

@Kobzol
Copy link
Contributor Author

Kobzol commented Jul 17, 2023

@rustbot ready

@Noratrieb Noratrieb removed the perf-regression Performance regression. label Jul 17, 2023
@nikic
Copy link
Contributor

nikic commented Jul 17, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Jul 17, 2023

📌 Commit e95caa7 has been approved by nikic

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 17, 2023
@bors
Copy link
Contributor

bors commented Jul 17, 2023

⌛ Testing commit e95caa7 with merge 63430236a32d03b5169377ae6f954ef46dadb4c8...

@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)
extracting /checkout/obj/build/cache/2023-07-12/rust-std-beta-aarch64-apple-darwin.tar.xz to /checkout/obj/build/aarch64-apple-darwin/stage0
downloading https://static.rust-lang.org/dist/2023-07-12/rustc-beta-aarch64-apple-darwin.tar.xz
extracting /checkout/obj/build/cache/2023-07-12/rustc-beta-aarch64-apple-darwin.tar.xz to /checkout/obj/build/aarch64-apple-darwin/stage0
downloading https://static.rust-lang.org/dist/2023-07-12/cargo-beta-aarch64-apple-darwin.tar.xz
thread 'config::tests::download_ci_llvm' panicked at 'status code: 1', /checkout/src/tools/build_helper/src/util.rs:18:9

failures:
    config::tests::download_ci_llvm

@bors
Copy link
Contributor

bors commented Jul 17, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 17, 2023
@Kobzol
Copy link
Contributor Author

Kobzol commented Jul 17, 2023

As much as it is suspicious in an iffy PR like this, 2023-07-17T12:06:44.8888610Z curl: (56) OpenSSL SSL_read: Connection reset by peer, errno 104 looks.. spurious?

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 17, 2023
@bors
Copy link
Contributor

bors commented Jul 17, 2023

⌛ Testing commit e95caa7 with merge c4e6fe9...

@bors
Copy link
Contributor

bors commented Jul 17, 2023

☀️ Test successful - checks-actions
Approved by: nikic
Pushing c4e6fe9 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 17, 2023
@bors bors merged commit c4e6fe9 into rust-lang:master Jul 17, 2023
11 checks passed
@rustbot rustbot added this to the 1.73.0 milestone Jul 17, 2023
@Kobzol Kobzol deleted the ci-cmake branch July 17, 2023 18:05
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (c4e6fe9): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-5.0% [-5.0%, -5.0%] 1
All ❌✅ (primary) - - 0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.6% [-2.6%, -2.6%] 1
Improvements ✅
(secondary)
-2.7% [-2.7%, -2.7%] 1
All ❌✅ (primary) -2.6% [-2.6%, -2.6%] 1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 656.058s -> 656.866s (0.12%)

@klensy
Copy link
Contributor

klensy commented Jul 25, 2023

Looking at fails in #114048 (comment)
In most docker files cmake installed via apt, created symlink into rustroot, and later build 3.20 via cmake.sh. Should previous version be deleted?

https://github.com/rust-lang-ci/rust/actions/runs/5577768220/job/15104051656#step:24:1907

    Your CMake version is 3.17.5.  Starting with LLVM 17.0.0, the minimum
    version of CMake required to build LLVM will become 3.20.0, and using an
    older CMake will become an error.  Please upgrade your CMake to at least
    3.20.0 now to avoid issues in the future!

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 28, 2023
…lacrum

ci: update ubuntu:20.04 builders to 22.04

This is mostly just maintenance to avoid bitrotting, but 22.04 also updates to cmake 3.22, so they don't need the manual builds from rust-lang#113714 anymore.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants