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

Rollup of 7 pull requests #135095

Merged
merged 18 commits into from
Jan 4, 2025
Merged

Rollup of 7 pull requests #135095

merged 18 commits into from
Jan 4, 2025

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

joboet and others added 18 commits December 6, 2024 15:07
Co-authored-by: Yotam Ofek <yotam.ofek@gmail.com>
Co-authored-by: Hanna Kruppe <hanna.kruppe@gmail.com>
…ntained"` is used

Before, we just used the global `lld` anyway.
Not all targets support these flags, so we cannot just pass them to the tests unconditionally. Before, we were using a linker arg (`-Clink-arg=-fuse-ld=lld`), which circumvented this in a hacky way.
LLVM 20 adds nuw to GEP operations in this code, tolerate them.
…oss35

core: implement `bool::select_unpredictable`

Tracking issue: rust-lang#133962
ACP: rust-lang/libs-team#468
…zkan

Allow using self-contained LLD in bootstrap

In rust-lang#116278, I added a `"self-contained"` mode to the `rust.use-lld` bootstrap option, which was designed for using the built-in LLD for linking compiler artifacts. However, this was later reverted in rust-lang#118810.

This PR brings the old logic back, which switches LLD in bootstrap from `-fuse-ld=lld` to [MCP510](rust-lang/compiler-team#510 way of passing linker flags to enable LLD (both external and self-contained). So this does two changes:
1) Goes from `-fuse-ld=lld` to MCP510
2) Actually makes it possible to use the self-contained LLD to compile compiler artifacts

Regarding the second commit: Since rust-lang#86113, we have been passing `-fuse-ld=lld` as a target flag to all tests when `use-lld = true` is enabled. This kind of worked for all tests, since it was just a linker argument, which has bypassed any compiler checks, and probably resulted only in some warning if the given target linker didn't actually support LLD. However, after the first commit, some tests actually start failing with this approach:
```
error: linker flavor `gnu-lld-cc` is incompatible with the current target
   |
   = note: compatible flavors are: llbc, ptx
```
So the second commit removes the passing of LLD flags as target flags to tests. I don't think that it's a good idea to pass specific compiler flags to all tests unconditionally, tbh. The doctest command from rust-lang#86113 doesn't go through compiletest anymore, and doctests should be quite a lot faster since rust-lang#126245 in general.

CC `@the8472`

If someone has a beefy machine, it would be nice to test whether this doesn't regress test execution speed. How to do that:
1) Enable `rust.use-lld = true` and `rust.lld = true` in `config.toml`
2) Benchmark `./x test tests/ui --force-rerun` between `master` and this PR

Once this is tested in the wild, I would like to make the self-contained LLD the default in CI, hopefully to make CI builds faster.

r? `@onur-ozkan`
…ricter-impl, r=estebank

Report impl method has stricter requirements even when RPITIT inference gets in the way

See the comment I added in the code. Fixes rust-lang#122506.
…ot-const, r=compiler-errors

const-in-pattern: test that the PartialEq impl does not need to be const

Fixes rust-lang#119398 by adding a test.

`@compiler-errors`  is there some place in the code where we could add a comment saying "as a backcompat hack, here we only require `PartialEq` and not `const PartialEq`"?

r? `@compiler-errors`
…=clubby789

bootstrap: support `./x check run-make-support`

Mostly for working on `src/tools/run-make-support` locally.
Update carrying_mul_add test to tolerate `nuw`

LLVM 20 adds nuw to GEP operations in this code, tolerate them.

`@rustbot` label: +llvm-main

r? `@durin42`
@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) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Jan 4, 2025
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=7

@bors
Copy link
Contributor

bors commented Jan 4, 2025

📌 Commit 75e412b has been approved by matthiaskrgr

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 Jan 4, 2025
@bors
Copy link
Contributor

bors commented Jan 4, 2025

⌛ Testing commit 75e412b with merge f17cf74...

@bors
Copy link
Contributor

bors commented Jan 4, 2025

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing f17cf74 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 4, 2025
@bors bors merged commit f17cf74 into rust-lang:master Jan 4, 2025
7 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Jan 4, 2025
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#133964 core: implement bool::select_unpredictable e97851da17ae5cb4515f1b00186afc8115aa38c6 (link)
#135001 Allow using self-contained LLD in bootstrap 4ff4f361246d9a8a1ba926be10dd6ee684bec10d (link)
#135055 Report impl method has stricter requirements even when RPIT… 4dd10f7fd597dd01f29e813082044292457e41bf (link)
#135064 const-in-pattern: test that the PartialEq impl does not nee… b7a0c810c976aba905a0cd4397d8e5017f413d2f (link)
#135066 bootstrap: support ./x check run-make-support 3df8bd0f7331790fb9a9a4db0f76d54ad5c03462 (link)
#135069 remove unused function params 3a3067ac725305422d62182e11b24deee3f94fd0 (link)
#135084 Update carrying_mul_add test to tolerate nuw 2049719625e403493256605981844cb36c94f640 (link)

previous master: 49761b073c

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (f17cf74): 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 (primary 0.7%)

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.4% [2.3%, 2.5%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.8% [-2.8%, -2.8%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.7% [-2.8%, 2.5%] 3

Cycles

Results (secondary 2.5%)

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

Binary size

Results (primary 0.0%, secondary 0.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)
0.0% [0.0%, 0.0%] 10
Regressions ❌
(secondary)
0.3% [0.3%, 0.3%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.0% [0.0%, 0.0%] 10

Bootstrap: 762.724s -> 764.045s (0.17%)
Artifact size: 325.69 MiB -> 325.60 MiB (-0.03%)

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. rollup A PR which is a rollup 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-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants