Skip to content

Update LLVM submodule #131448

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Oct 12, 2024
Merged

Update LLVM submodule #131448

merged 2 commits into from
Oct 12, 2024

Conversation

dianqk
Copy link
Member

@dianqk dianqk commented Oct 9, 2024

Fixes (maybe after beta backport) #131164.

r? nikic

@rustbot
Copy link
Collaborator

rustbot commented Oct 9, 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 Oct 9, 2024
@lqd lqd added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 9, 2024
@dianqk
Copy link
Member Author

dianqk commented Oct 9, 2024

Backporting will also bring #131293, mostly clang changes. I can create a separate branch for backporting if necessary.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@dianqk
Copy link
Member Author

dianqk commented Oct 9, 2024

It broke something, I'll check it out tomorrow. 😣

@nikic nikic added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 9, 2024
@onur-ozkan
Copy link
Member

Maybe reason is because download-rustc="if-unchanged" is being utilized in non-dist runners. If that's the case (you can set NO_DOWNLOAD_CI_RUSTC=1 in the failed runner for testing), we should add a logic to skip download-rustc on LLVM bumps.

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Oct 9, 2024
@dianqk

This comment was marked as spam.

@dianqk dianqk marked this pull request as draft October 9, 2024 14:57
@rust-log-analyzer

This comment has been minimized.

@onur-ozkan
Copy link
Member

onur-ozkan commented Oct 9, 2024

Can you cherry pick commits from #131453 and #131305 (with reverting the last change)? I want to see if they can fix x86_64-gnu-llvm-18 and x86_64-gnu-tools issues.

Testing in #131455.

@rust-log-analyzer

This comment has been minimized.

@jieyouxu
Copy link
Member

thread 'core::config::tests::download_ci_llvm' panicked at src/core/config/tests.rs:34:5

Might need to rebase again?

@dianqk
Copy link
Member Author

dianqk commented Oct 10, 2024

thread 'core::config::tests::download_ci_llvm' panicked at src/core/config/tests.rs:34:5

Might need to rebase again?

This is a known error: #131303.

@dianqk dianqk marked this pull request as ready for review October 10, 2024 12:30
@nikic
Copy link
Contributor

nikic commented Oct 10, 2024

@bors r+ rollup=never

@bors
Copy link
Collaborator

bors commented Oct 10, 2024

📌 Commit 4ea9711 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 Oct 10, 2024
@bors bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 12, 2024
@onur-ozkan
Copy link
Member

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

Click to see the possible cause of the failure (guessed by this bot)

failures:

---- core::config::tests::download_ci_llvm stdout ----
Detected LLVM as non-available: running in CI and modified LLVM in this change
thread 'core::config::tests::download_ci_llvm' panicked at src/core/config/tests.rs:24:5:
assertion failed: parse("").llvm_from_ci
   0: rust_begin_unwind
             at /rustc/8c27a2ba6b21f3406a51118643080f0591949827/library/std/src/panicking.rs:662:5
   1: core::panicking::panic_fmt
             at /rustc/8c27a2ba6b21f3406a51118643080f0591949827/library/core/src/panicking.rs:74:14

lol this is getting really annoying

@onur-ozkan
Copy link
Member

Feel free to apply this patch:

diff --git a/src/bootstrap/src/core/config/tests.rs b/src/bootstrap/src/core/config/tests.rs
index 4b689aa3ecc..15600f70d6e 100644
--- a/src/bootstrap/src/core/config/tests.rs
+++ b/src/bootstrap/src/core/config/tests.rs
@@ -21,7 +21,6 @@ pub(crate) fn parse(config: &str) -> Config {

 #[test]
 fn download_ci_llvm() {
-    assert!(parse("").llvm_from_ci);
     assert!(parse("llvm.download-ci-llvm = true").llvm_from_ci);
     assert!(!parse("llvm.download-ci-llvm = false").llvm_from_ci);

as a first commit (make sure the LLVM submodule update is the last commit) and r=me.

@onur-ozkan
Copy link
Member

Feel free to apply this patch:

diff --git a/src/bootstrap/src/core/config/tests.rs b/src/bootstrap/src/core/config/tests.rs
index 4b689aa3ecc..15600f70d6e 100644
--- a/src/bootstrap/src/core/config/tests.rs
+++ b/src/bootstrap/src/core/config/tests.rs
@@ -21,7 +21,6 @@ pub(crate) fn parse(config: &str) -> Config {

 #[test]
 fn download_ci_llvm() {
-    assert!(parse("").llvm_from_ci);
     assert!(parse("llvm.download-ci-llvm = true").llvm_from_ci);
     assert!(!parse("llvm.download-ci-llvm = false").llvm_from_ci);

as a first commit (make sure the LLVM submodule update is the last commit) and r=me.

or even ignore that test with #[ignore] completely. I guess that will be much better until we have a stabilized change tracking logic (something like #131358).

@rustbot
Copy link
Collaborator

rustbot commented Oct 12, 2024

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Oct 12, 2024
@onur-ozkan
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 12, 2024

📌 Commit 4441a89 has been approved by onur-ozkan

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 Oct 12, 2024
@bors
Copy link
Collaborator

bors commented Oct 12, 2024

⌛ Testing commit 4441a89 with merge e200c7f...

@lqd
Copy link
Member

lqd commented Oct 12, 2024

The good news is that the backport has already landed on the beta branch

@cuviper cuviper mentioned this pull request Oct 12, 2024
@cuviper
Copy link
Member

cuviper commented Oct 12, 2024

Yes, but if #131560 lands before this, marking the version cutoff, then there will be a new beta needing backport .

@bors
Copy link
Collaborator

bors commented Oct 12, 2024

☀️ Test successful - checks-actions
Approved by: onur-ozkan
Pushing e200c7f to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 12, 2024
@bors bors merged commit e200c7f into rust-lang:master Oct 12, 2024
7 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Oct 12, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (e200c7f): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

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

Max RSS (memory usage)

Results (primary -0.3%, secondary 2.3%)

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)
1.4% [1.4%, 1.4%] 1
Regressions ❌
(secondary)
2.3% [0.9%, 3.7%] 3
Improvements ✅
(primary)
-2.0% [-2.0%, -2.0%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.3% [-2.0%, 1.4%] 2

Cycles

Results (secondary -5.1%)

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.1% [-6.9%, -3.4%] 6
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 779.421s -> 779.831s (0.05%)
Artifact size: 332.13 MiB -> 332.07 MiB (-0.02%)

@dianqk dianqk deleted the fixes-131164 branch October 12, 2024 22:08
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 13, 2024
…an68

enable `download_ci_llvm` test

This was ignored because it caused merge failures on [LLVM update PR](rust-lang#131448). The issue was not checking `is_ci_llvm_available` in the test which is crucial for enabling CI LLVM:

https://github.com/rust-lang/rust/blob/2aa26d8a722cf8810b27538c24b93d29324d4ac7/src/bootstrap/src/core/config/config.rs#L2835-L2844
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 14, 2024
Rollup merge of rust-lang#131659 - onur-ozkan:llvm-test, r=albertlarsan68

enable `download_ci_llvm` test

This was ignored because it caused merge failures on [LLVM update PR](rust-lang#131448). The issue was not checking `is_ci_llvm_available` in the test which is crucial for enabling CI LLVM:

https://github.com/rust-lang/rust/blob/2aa26d8a722cf8810b27538c24b93d29324d4ac7/src/bootstrap/src/core/config/config.rs#L2835-L2844
@cuviper cuviper modified the milestones: 1.83.0, 1.82.0 Oct 15, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc beta-accepted Accepted for backporting to the compiler in the beta channel. 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) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. 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.