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

Update to LLVM 18.1.0 rc 4 #121395

Merged
merged 2 commits into from
Mar 2, 2024
Merged

Update to LLVM 18.1.0 rc 4 #121395

merged 2 commits into from
Mar 2, 2024

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Feb 21, 2024

Fixes #120819.
Fixes #121180.
Fixes #121239.
Fixes #121367.

@rustbot
Copy link
Collaborator

rustbot commented Feb 21, 2024

r? @cuviper

rustbot has assigned @cuviper.
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
Copy link
Collaborator

rustbot commented Feb 21, 2024

⚠️ Warning ⚠️

  • These commits modify submodules.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 21, 2024
@cuviper
Copy link
Member

cuviper commented Feb 21, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 21, 2024

📌 Commit 17a7f3c has been approved by cuviper

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 Feb 21, 2024
@cuviper
Copy link
Member

cuviper commented Feb 21, 2024

@bors rollup=never

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 21, 2024
@bors
Copy link
Contributor

bors commented Feb 21, 2024

⌛ Testing commit 17a7f3c with merge 729a664...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Feb 21, 2024

💔 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 Feb 21, 2024
@nikic
Copy link
Contributor Author

nikic commented Feb 21, 2024

Probably related to the libLLVM soname change in llvm/llvm-project@235306b.

@rustbot
Copy link
Collaborator

rustbot commented Feb 22, 2024

This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp.

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Feb 22, 2024
@nikic
Copy link
Contributor Author

nikic commented Feb 22, 2024

@bors try

@bors
Copy link
Contributor

bors commented Feb 22, 2024

⌛ Trying commit 87481e3 with merge da675ab...

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 22, 2024
@bors
Copy link
Contributor

bors commented Feb 22, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 22, 2024
@rust-log-analyzer

This comment has been minimized.

@nikic
Copy link
Contributor Author

nikic commented Feb 22, 2024

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 22, 2024
@bors
Copy link
Contributor

bors commented Feb 22, 2024

⌛ Trying commit bb1d57b with merge 6793580...

@cuviper
Copy link
Member

cuviper commented Mar 1, 2024

Aw, I was hoping that the binary size actually did get better...
(IIRC, BOLT was essentially doubling .text with its rewrite.)

@Kobzol
Copy link
Contributor

Kobzol commented Mar 1, 2024

https://perf.rust-lang.org/compare.html?start=6cbf0926d54c80ea6d15df333be9281f65bbeb36&end=5e826decbc9c72668d6e1e8e7116a68c86f877c9&stat=instructions%3Au&tab=artifact-size it would have to be infinitely better, since the libLLVM.so file simply disappeared from the artifact size metrics 😁

@nikic nikic changed the title Update to LLVM 18.1.0 rc 3 Update to LLVM 18.1.0 rc 4 Mar 1, 2024
@cuviper
Copy link
Member

cuviper commented Mar 1, 2024

I don't think not distributing the symlink is possible, so what I did now is to limit it to just the rust-dev component (aka ci-llvm). That's the only place that should need it. All the "normal" dist tarballs continue to be symlink-free.

I can see that the symlink survives into the rust-dev tarball, but it looks like its install.sh is dereferencing that, because the installed copy gets duplicated as a whole file.

@nikic
Copy link
Contributor Author

nikic commented Mar 1, 2024

I don't think not distributing the symlink is possible, so what I did now is to limit it to just the rust-dev component (aka ci-llvm). That's the only place that should need it. All the "normal" dist tarballs continue to be symlink-free.

I can see that the symlink survives into the rust-dev tarball, but it looks like its install.sh is dereferencing that, because the installed copy gets duplicated as a whole file.

Does the behavior of install.sh matter though? I do get the symlink when using the try build in download-ci-llvm, which I believe is the only thing this component should get used for.

@cuviper
Copy link
Member

cuviper commented Mar 1, 2024

Hmm, I was thinking of it as a rustup component, but I guess we don't actually distribute it that way. (and rustup doesn't invoke install.sh either.)

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Mar 1, 2024

📌 Commit 1a652fa has been approved by cuviper

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 1, 2024
@bors
Copy link
Contributor

bors commented Mar 1, 2024

⌛ Testing commit 1a652fa with merge e612d07...

@bors
Copy link
Contributor

bors commented Mar 2, 2024

☀️ Test successful - checks-actions
Approved by: cuviper
Pushing e612d07 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 2, 2024
@bors bors merged commit e612d07 into rust-lang:master Mar 2, 2024
12 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Mar 2, 2024
@cuviper
Copy link
Member

cuviper commented Mar 2, 2024

All good -- with that merged, a new ./x build does extract rust-dev with that symlink preserved!

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (e612d07): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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)
- - 0
Regressions ❌
(secondary)
1.7% [1.7%, 1.7%] 1
Improvements ✅
(primary)
-2.7% [-2.7%, -2.7%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.7% [-2.7%, -2.7%] 1

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)
4.4% [3.9%, 4.8%] 2
Improvements ✅
(primary)
-1.0% [-1.0%, -1.0%] 1
Improvements ✅
(secondary)
-3.4% [-4.7%, -2.0%] 2
All ❌✅ (primary) -1.0% [-1.0%, -1.0%] 1

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.8% [-2.8%, -2.8%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.8% [-2.8%, -2.8%] 1

Binary size

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

Bootstrap: 650.452s -> 652.171s (0.26%)
Artifact size: 311.17 MiB -> 175.40 MiB (-43.63%)

@RalfJung
Copy link
Member

RalfJung commented Mar 2, 2024

Something seems to still be wrong with the new soname: #121889

@lqd
Copy link
Member

lqd commented Mar 2, 2024

syn-opt still looks currently noisy, and the coercions change looks noisy as well, albeit with a bigger amplitude than recently: it is a few merges later trending down close to prior to this PR, via some steps that didn't change compilation (a cargo update).

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Mar 2, 2024
@Mark-Simulacrum Mark-Simulacrum removed perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. labels Mar 5, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 7, 2024
…mulacrum

Modify opt-dist logic for finding LLVM artifacts

This is the `rustc` side of fixing rust-lang#121395 (comment).
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Mar 8, 2024
Modify opt-dist logic for finding LLVM artifacts

This is the `rustc` side of fixing rust-lang/rust#121395 (comment).
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Apr 7, 2024
Modify opt-dist logic for finding LLVM artifacts

This is the `rustc` side of fixing rust-lang/rust#121395 (comment).
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
Modify opt-dist logic for finding LLVM artifacts

This is the `rustc` side of fixing rust-lang/rust#121395 (comment).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
10 participants