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

use target compiler on llvm-bitcode-linker #138148

Merged
merged 1 commit into from
Mar 7, 2025

Conversation

onur-ozkan
Copy link
Member

@onur-ozkan onur-ozkan commented Mar 7, 2025

The build compiler is already resolved inside the ToolBuild step, so we should pass only the target compilers for Mode::ToolRustc tools.

Fixes #138123

r? ghost

try-job: dist-powerpc64le-linux

The build compiler is already resolved inside the `ToolBuild` step,
so we should pass only the target compilers for `Mode::ToolRustc` tools.

Signed-off-by: onur-ozkan <work@onurozkan.dev>
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Mar 7, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 7, 2025

Some changes occurred in src/tools/opt-dist

cc @Kobzol

@onur-ozkan
Copy link
Member Author

@bors try

@bors
Copy link
Collaborator

bors commented Mar 7, 2025

⌛ Trying commit f881d7d with merge 2b0fbc0...

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 7, 2025
…=<try>

use target compiler on llvm-bitcode-linker

The build compiler is already resolved inside the `ToolBuild` step, so we should pass only the target compilers for `Mode::ToolRustc` tools.

Fixes rust-lang#138123

r? ghost

try-job: dist-powerpc64le-linux
@Kobzol
Copy link
Contributor

Kobzol commented Mar 7, 2025

I don't know if you saw my message in the previous PR, but you don't have to modify opt-dist for this 😆 It's not even executed for the powerpc job.

@bors
Copy link
Collaborator

bors commented Mar 7, 2025

☀️ Try build successful - checks-actions
Build commit: 2b0fbc0 (2b0fbc0cdc8351b6b4986eb731bc910610101ea8)

@onur-ozkan onur-ozkan force-pushed the fix-llvm-bitcode-build branch from f881d7d to 4227055 Compare March 7, 2025 10:02
@onur-ozkan
Copy link
Member Author

We are back to ~2h45m duration with this change.

@rustbot ready

r? bootstrap

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, r=me after PR CI is green.

@onur-ozkan
Copy link
Member Author

@bors r=jieyouxu p=1

@bors
Copy link
Collaborator

bors commented Mar 7, 2025

📌 Commit 4227055 has been approved by jieyouxu

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 Mar 7, 2025
@bors
Copy link
Collaborator

bors commented Mar 7, 2025

⌛ Testing commit 4227055 with merge c53af1c...

@bors
Copy link
Collaborator

bors commented Mar 7, 2025

☀️ Test successful - checks-actions
Approved by: jieyouxu
Pushing c53af1c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 7, 2025
@bors bors merged commit c53af1c into rust-lang:master Mar 7, 2025
9 of 13 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 7, 2025
@onur-ozkan onur-ozkan deleted the fix-llvm-bitcode-build branch March 7, 2025 19:57
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (c53af1c): comparison URL.

Overall result: ✅ improvements - 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)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.2% [-0.2%, -0.1%] 2
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -2.6%, secondary -5.8%)

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)
-5.8% [-5.8%, -5.8%] 1
All ❌✅ (primary) -2.6% [-2.6%, -2.6%] 1

Cycles

Results (primary 2.8%, secondary 1.4%)

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)
2.8% [2.8%, 2.8%] 1
Regressions ❌
(secondary)
3.0% [2.2%, 3.9%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.8% [-1.8%, -1.8%] 1
All ❌✅ (primary) 2.8% [2.8%, 2.8%] 1

Binary size

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

Bootstrap: 766.551s -> 765.734s (-0.11%)
Artifact size: 362.09 MiB -> 362.10 MiB (0.00%)

@Kobzol
Copy link
Contributor

Kobzol commented Mar 8, 2025

image

Awesome! :) (it's ordered from newest to oldest)

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
Development

Successfully merging this pull request may close these issues.

Some configurations build the compiler too many times
6 participants