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 5 pull requests #131098

Merged
merged 12 commits into from
Oct 1, 2024
Merged

Rollup of 5 pull requests #131098

merged 12 commits into from
Oct 1, 2024

Conversation

GuillaumeGomez
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

taiki-e and others added 12 commits September 21, 2024 01:51
This updates the `wasm-component-ld` linker binary for the
`wasm32-wasip2` target to 0.5.9, pulling in a few bug fixes and recent
updates.
Support clobber_abi and vector/access registers (clobber-only) in s390x inline assembly

This supports `clobber_abi` which is one of the requirements of stabilization mentioned in rust-lang#93335.

This also supports vector registers (as `vreg`) and access registers (as `areg`) as clobber-only, which need to support clobbering of them to implement clobber_abi.

Refs:
- "1.2.1.1. Register Preservation Rules" section in ELF Application Binary Interface s390x Supplement, Version 1.6.1 (lzsabi_s390x.pdf in https://github.com/IBM/s390x-abi/releases/tag/v1.6.1)
- Register definition in LLVM:
  - Vector registers https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/SystemZ/SystemZRegisterInfo.td#L249
  - Access registers https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/SystemZ/SystemZRegisterInfo.td#L332

I have three questions:
- ~~ELF Application Binary Interface s390x Supplement says that `cc` (condition code, bits 18-19 of PSW) is "Volatile".
  However, we do not have a register class for `cc` and instead mark `cc` as clobbered unless `preserves_flags` is specified (rust-lang#111331).
  Therefore, in the current implementation, if both `preserves_flags` and `clobber_abi` are specified, `cc` is not marked as clobbered. Is this okay? Or even if `preserves_flags` is used, should `cc` be marked as clobbered if `clobber_abi` is used?~~ UPDATE: resolved rust-lang#130630 (comment)
- ~~ELF Application Binary Interface s390x Supplement says that `pm` (program mask, bits 20-23 of PSW) is "Cleared".
  There does not appear to be any registers associated with this in either [LLVM](https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/SystemZ/SystemZRegisterInfo.td) or [GCC](https://github.com/gcc-mirror/gcc/blob/33ccc1314dcdb0b988a9276ca6b6ce9b07bea21e/gcc/config/s390/s390.h#L407-L431), so at this point I don't see any way other than to just ignore it. Is this okay as-is?~~ UPDATE: resolved rust-lang#130630 (comment)
- Is "areg" a good name for register class name for access registers? It may be a bit confusing between that and `reg_addr`, which uses the “a” constraint (rust-lang#119431)...

Note:

- GCC seems to [recognize only `a0` and `a1`](https://github.com/gcc-mirror/gcc/blob/33ccc1314dcdb0b988a9276ca6b6ce9b07bea21e/gcc/config/s390/s390.h#L428-L429), and using `a[2-15]` [causes errors](https://godbolt.org/z/a46vx8jjn).
  Given that cg_gcc has a similar problem with other architecture (rust-lang/rustc_codegen_gcc#485), I don't feel this is a blocker for this PR, but it is worth mentioning here.
- `vreg` should be able to accept `#[repr(simd)]` types as input if the `vector` target feature added in rust-lang#127506 is enabled, but core_arch has no s390x vector type and both `#[repr(simd)]` and `core::simd` are unstable, so I have not implemented it in this PR. EDIT: And supporting it is probably more complex than doing the equivalent on other architectures... rust-lang#88245 (comment)

cc `@uweigand`

r? `@Amanieu`

`@rustbot` label +O-SystemZ
… r=lcnr

Instantiate binders in `supertrait_vtable_slot`

`supertrait_vtable_slot` was previously using structural equality when probing for the vtable slot, which led to an ICE since we need a *subtype* match, not an exact match.

Fixes rust-lang#131027

r? lcnr
…-ld, r=jieyouxu

Update wasm-component-ld to 0.5.9

This updates the `wasm-component-ld` linker binary for the `wasm32-wasip2` target to 0.5.9, pulling in a few bug fixes and recent updates.
make test_lots_of_insertions test take less long in Miri

This is by far the slowest `std` test in Miri, taking >2min in https://github.com/rust-lang/miri-test-libstd CI. So let's make this `count` smaller. The runtime should be quadratic in `count` so reducing it to around 2/3 of it's previous value should cut the total time down to less than half -- making it still the slowest test, but by less of a margin. (And this way we still insert >64 elements into the HashMap, in case that power of 2 matters.)
add fixme to remove LLVM_ENABLE_TERMINFO when minimal llvm version is 19

`LLVM_ENABLE_TERMINFO` was removed in llvm 19: https://github.com/llvm/llvm-project/blob/release/19.x/llvm/docs/ReleaseNotes.rst#changes-to-building-llvm; current minimal llvm is 18, so left fixme here.
@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 Oct 1, 2024
@GuillaumeGomez
Copy link
Member Author

@bors r+ p=5 rollup=never

@bors
Copy link
Contributor

bors commented Oct 1, 2024

📌 Commit 91f079a has been approved by GuillaumeGomez

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 1, 2024
@bors
Copy link
Contributor

bors commented Oct 1, 2024

⌛ Testing commit 91f079a with merge c817d5d...

@bors
Copy link
Contributor

bors commented Oct 1, 2024

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing c817d5d to master...

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

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#130630 Support clobber_abi and vector/access registers (clobber-on… 2f2a81ca127303f677c15ad761dd203dc870e6c2 (link)
#131042 Instantiate binders in supertrait_vtable_slot 25c47527185cdd87fd62af4169f42a4a04e9105a (link)
#131079 Update wasm-component-ld to 0.5.9 fd147cfc4235abb0ebf74b91af902d4702c2ebad (link)
#131085 make test_lots_of_insertions test take less long in Miri 6afa0f243bc38d627a8dfce1ac05485c02d0ff03 (link)
#131088 add fixme to remove LLVM_ENABLE_TERMINFO when minimal llvm … f4080d2d5967cc2467e58503d8f6a68bef8149f9 (link)

previous master: 8dd5cd0bc1

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

@GuillaumeGomez GuillaumeGomez deleted the rollup-kk74was branch October 1, 2024 19:30
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (c817d5d): 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.3% [0.2%, 0.3%] 5
Regressions ❌
(secondary)
0.2% [0.2%, 0.2%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.3% [-0.3%, -0.2%] 6
All ❌✅ (primary) 0.3% [0.2%, 0.3%] 5

Max RSS (memory usage)

Results (primary -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)
1.1% [1.1%, 1.1%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.0% [-1.1%, -1.0%] 2
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.3% [-1.1%, 1.1%] 3

Cycles

Results (primary -1.0%, secondary 0.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)
3.7% [3.7%, 3.7%] 1
Improvements ✅
(primary)
-1.0% [-1.2%, -0.9%] 2
Improvements ✅
(secondary)
-2.1% [-2.1%, -2.1%] 1
All ❌✅ (primary) -1.0% [-1.2%, -0.9%] 2

Binary size

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

Bootstrap: 770.002s -> 771.522s (0.20%)
Artifact size: 341.49 MiB -> 341.52 MiB (0.01%)

@rustbot rustbot added the perf-regression Performance regression. label Oct 1, 2024
@rylev
Copy link
Member

rylev commented Oct 9, 2024

The regressions are small and largely a jump back to a previous baseline so I don't think this merits investigation..

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Oct 9, 2024
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. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. 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