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.

Sign up for GitHub

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

Jump to bottom

use PassMode::Direct for vector types on s390x #135785

Merged
merged 1 commit into from
Jan 26, 2025
Merged

use PassMode::Direct for vector types on s390x #135785

merged 1 commit into from
Jan 26, 2025
+154 −3

Conversation

folkertdev
Copy link
Contributor

closes #135744
tracking issue: #130869

Previously, all vector types were type erased to Ni8, now we pass non-wrapped vector types directly. That skips emitting a bunch of casting logic in rustc, that LLVM then has to clean up. The initial LLVM IR is also a bit more readable.

This calling convention is tested extensively in tests/assembly/s390x-vector-abi.rs, showing that this change has no impact on the ABI in practice.

r? @taiki-e

@rustbot
Copy link
Collaborator

rustbot commented Jan 20, 2025

Failed to set assignee to taiki-e: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 20, 2025
@taiki-e
Copy link
Member

taiki-e commented Jan 20, 2025 •
edited
Loading

Thanks! The change looks good to me. Could you add a test that checks generated LLVM IR is expected (i.e., test case for #135744) to tests/codegen/?

@tgross35
Copy link
Contributor

cc @uweigand

@folkertdev folkertdev force-pushed the s390x-vector-passmode-direct branch from 916b1da to 893d81f Compare January 20, 2025 20:02
@folkertdev
Copy link
Contributor Author

I added a codegen test, using compile-flags: --target s390x-unknown-linux-gnu -O so hopefully the LLVM IR is stable

@folkertdev
Copy link
Contributor Author

r? compiler

@jieyouxu jieyouxu added the O-SystemZ Target: SystemZ processors (s390x) label Jan 21, 2025
@jieyouxu
Copy link
Member

Maybe r? @bjorn3 since you discussed this on the linked issue

@rustbot rustbot assigned bjorn3 and unassigned fee1-dead Jan 21, 2025
@bjorn3
Copy link
Member

bjorn3 commented Jan 23, 2025

@bors r+

@bors
Copy link
Contributor

bors commented Jan 23, 2025

📌 Commit 893d81f has been approved by bjorn3

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 23, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 25, 2025
@matthiaskrgr
Rollup merge of rust-lang#135785 - folkertdev:s390x-vector-passmode-d…
b73f4c2 
…irect, r=bjorn3

use `PassMode::Direct` for vector types on `s390x`

closes rust-lang#135744
tracking issue: rust-lang#130869

Previously, all vector types were type erased to `Ni8`, now we pass non-wrapped vector types directly. That skips emitting a bunch of casting logic in rustc, that LLVM then has to clean up. The initial LLVM IR is also a bit more readable.

This calling convention is tested extensively in `tests/assembly/s390x-vector-abi.rs`, showing that this change has no impact on the ABI in practice.

r? `@taiki-e`
@matthiaskrgr matthiaskrgr mentioned this pull request Jan 25, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 25, 2025
@bors
Auto merge of rust-lang#136045 - matthiaskrgr:rollup-70c22eb, r=<try>
9859fda 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#130808 (Fix linking for symbols starting with ? on i686-pc-windows-msvc)
 - rust-lang#133138 (Target modifiers (special marked options) are recorded in metainfo)
 - rust-lang#133154 (Reword resolve errors caused by likely missing crate in dep tree)
 - rust-lang#135707 (Shorten linker output even more when `--verbose` is not present)
 - rust-lang#135764 (Fix tests on LLVM 20)
 - rust-lang#135785 (use `PassMode::Direct` for vector types on `s390x`)
 - rust-lang#135818 (tests: Port `translation` to rmake.rs)

Failed merges:

 - rust-lang#133951 (Make the wasm_c_abi future compat warning a hard error)

r? `@ghost`
`@rustbot` modify labels: rollup

try-job: aarch64-apple
try-job: i686-mingw
try-job: x86_64-gnu-llvm-19-3
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 25, 2025
@matthiaskrgr
Rollup merge of rust-lang#135785 - folkertdev:s390x-vector-passmode-d…
9d2f089 
…irect, r=bjorn3

use `PassMode::Direct` for vector types on `s390x`

closes rust-lang#135744
tracking issue: rust-lang#130869

Previously, all vector types were type erased to `Ni8`, now we pass non-wrapped vector types directly. That skips emitting a bunch of casting logic in rustc, that LLVM then has to clean up. The initial LLVM IR is also a bit more readable.

This calling convention is tested extensively in `tests/assembly/s390x-vector-abi.rs`, showing that this change has no impact on the ABI in practice.

r? ``@taiki-e``
@matthiaskrgr matthiaskrgr mentioned this pull request Jan 25, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 25, 2025
@bors
Auto merge of rust-lang#136049 - matthiaskrgr:rollup-2hfmkoq, r=<try>
a37c61f 
Rollup of 6 pull requests

Successful merges:

 - rust-lang#130808 (Fix linking for symbols starting with ? on i686-pc-windows-msvc)
 - rust-lang#133154 (Reword resolve errors caused by likely missing crate in dep tree)
 - rust-lang#135707 (Shorten linker output even more when `--verbose` is not present)
 - rust-lang#135764 (Fix tests on LLVM 20)
 - rust-lang#135785 (use `PassMode::Direct` for vector types on `s390x`)
 - rust-lang#135818 (tests: Port `translation` to rmake.rs)

r? `@ghost`
`@rustbot` modify labels: rollup

try-job: aarch64-apple
try-job: i686-mingw
try-job: x86_64-gnu-llvm-19-3
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 25, 2025
@matthiaskrgr
Rollup merge of rust-lang#135785 - folkertdev:s390x-vector-passmode-d…
abf7651 
…irect, r=bjorn3

use `PassMode::Direct` for vector types on `s390x`

closes rust-lang#135744
tracking issue: rust-lang#130869

Previously, all vector types were type erased to `Ni8`, now we pass non-wrapped vector types directly. That skips emitting a bunch of casting logic in rustc, that LLVM then has to clean up. The initial LLVM IR is also a bit more readable.

This calling convention is tested extensively in `tests/assembly/s390x-vector-abi.rs`, showing that this change has no impact on the ABI in practice.

r? ```@taiki-e```
@matthiaskrgr matthiaskrgr mentioned this pull request Jan 25, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 25, 2025
@bors
Auto merge of rust-lang#136051 - matthiaskrgr:rollup-g2pbn2w, r=<try>
9f5ff93 
Rollup of 4 pull requests

Successful merges:

 - rust-lang#135707 (Shorten linker output even more when `--verbose` is not present)
 - rust-lang#135764 (Fix tests on LLVM 20)
 - rust-lang#135785 (use `PassMode::Direct` for vector types on `s390x`)
 - rust-lang#135818 (tests: Port `translation` to rmake.rs)

r? `@ghost`
`@rustbot` modify labels: rollup

try-job: aarch64-apple
try-job: i686-mingw
try-job: x86_64-gnu-llvm-19-3
@jhpratt jhpratt mentioned this pull request Jan 26, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 26, 2025
@bors
Auto merge of rust-lang#136085 - jhpratt:rollup-dxmb2h2, r=jhpratt
c2270be 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#133951 (Make the wasm_c_abi future compat warning a hard error)
 - rust-lang#134283 (fix(libtest): Deprecate '--logfile')
 - rust-lang#135785 (use `PassMode::Direct` for vector types on `s390x`)
 - rust-lang#135948 (Update emscripten std tests)
 - rust-lang#135951 (Use `fmt::from_fn` in more places in the compiler)
 - rust-lang#136031 (Expand polonius MIR dump)
 - rust-lang#136032 (Account for mutable borrow in argument suggestion)

Failed merges:

 - rust-lang#135635 (Move `std::io::pipe` code into its own file)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 61e572b into rust-lang:master Jan 26, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Jan 26, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 26, 2025
@rust-timer
Unrolled build for rust-lang#135785
5965fc0 
Rollup merge of rust-lang#135785 - folkertdev:s390x-vector-passmode-direct, r=bjorn3

use `PassMode::Direct` for vector types on `s390x`

closes rust-lang#135744
tracking issue: rust-lang#130869

Previously, all vector types were type erased to `Ni8`, now we pass non-wrapped vector types directly. That skips emitting a bunch of casting logic in rustc, that LLVM then has to clean up. The initial LLVM IR is also a bit more readable.

This calling convention is tested extensively in `tests/assembly/s390x-vector-abi.rs`, showing that this change has no impact on the ABI in practice.

r? ````@taiki-e````
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@bjorn3 bjorn3

Labels
O-SystemZ Target: SystemZ processors (s390x) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.86.0
Development

Successfully merging this pull request may close these issues.

vector types are passed incorrectly on s390x
8 participants
@folkertdev @rustbot @taiki-e @tgross35 @jieyouxu @bjorn3 @bors @fee1-dead