Skip to content

Add f16 inline ASM support for s390x #150826

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
Merged
rust-bors merged 1 commit into from
Jan 13, 2026
Merged

Add f16 inline ASM support for s390x #150826

rust-bors merged 1 commit into from
Jan 13, 2026
+51 −7

Conversation

@folkertdev
Copy link
Contributor

@folkertdev folkertdev commented Jan 8, 2026 •
edited
Loading

tracking issue: #116909
cc #125398

Support the f16x8 type in inline assembly. Only with the nnp-assist feature are there any instructions that make use of this type. Based on the riscv implementation I now cast to i16x8 when that feature is not enabled.

As far as I'm aware there are no instructions operating on f16 scalar values. Should we still add support for using them in inline assembly?

r? @tgross35
cc @uweigand

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. 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 8, 2026
@folkertdev folkertdev added the F-f16_and_f128 `#![feature(f16)]`, `#![feature(f128)]` label Jan 8, 2026
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@folkertdev folkertdev force-pushed the s390x-asm-f16-vector branch from f0150f0 to 9ce64ac Compare January 8, 2026 19:31
@tgross35
Copy link
Contributor

tgross35 commented Jan 8, 2026

Implementation LGTM but I'll wait to hear back from the target maintainers (cc @cuviper who is listed as well)

@folkertdev folkertdev mentioned this pull request Jan 8, 2026
Open
@folkertdev folkertdev added the O-SystemZ Target: SystemZ processors (s390x) label Jan 8, 2026
@uweigand
Copy link
Contributor

uweigand commented Jan 9, 2026

I'm not sure I like tying this to nnp-assist - yes, there's the conversion instruction between f16 and the AIU private format, but that's really a niche use case. From a perspective of whether the type is "generally" supported by instructions (e.g. for core arithmetic), presence or absence of nnp-assist doesn't really make any difference.

I think it would make more sense to support f16 vectors whenever both f16 and vectors are supported, and then just scalarize operations when necessary.

And when we have f16 (or f16 vectors), it should be possible to pass those to inline asm. (Both for "generic" operations like moves or bitops that actually do work, and also just in case some future ISA does add HW instructions - inline asm would then allow exploitation before compiler support is added.)

As an aside, we recently noticed that f16 vector support in the LLVM back-end is currently broken (wrong ABI). We're working on a fix here: llvm/llvm-project#171066 . Not sure if this means we need to wait with adding Rust support until this is merged?

@folkertdev folkertdev force-pushed the s390x-asm-f16-vector branch from 9ce64ac to edd2fe0 Compare January 9, 2026 16:57
@folkertdev folkertdev changed the title s390x: f16 vectors in assembly Add f16 inline ASM support for s390x Jan 9, 2026
@folkertdev folkertdev changed the title Add f16 inline ASM support for s390x Add f16 inline ASM support for s390x Jan 9, 2026
@folkertdev
Copy link
Contributor Author

folkertdev commented Jan 9, 2026

Alright, I removed the nnp-assist stuff and added support for passing f16 in float and vector registers.

As an aside, we recently noticed that f16 vector support in the LLVM back-end is currently broken (wrong ABI). We're working on a fix here: llvm/llvm-project#171066 . Not sure if this means we need to wait with adding Rust support until this is merged?

The f16 type is not stable, and s390x vector registers are also not stable, so I think this should be fine. Especially if that fix makes it into LLVM 22.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@folkertdev folkertdev force-pushed the s390x-asm-f16-vector branch from edd2fe0 to 6f12b86 Compare January 9, 2026 17:42
@folkertdev
Copy link
Contributor Author

folkertdev commented Jan 9, 2026

Locally the test works, so I'm assuming f16 support was added changed in LLVM 21. I added min-llvm-version: 21 to work around that.

@tgross35
Copy link
Contributor

tgross35 commented Jan 11, 2026

@uweigand does this version look reasonable to you?

@uweigand
Copy link
Contributor

uweigand commented Jan 12, 2026

@uweigand does this version look reasonable to you?

Yes, this LGTM. Thanks!

@tgross35
Copy link
Contributor

tgross35 commented Jan 12, 2026

Thanks for taking a look!

@bors r=uweigand,tgross35

@rust-bors rust-bors bot added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jan 12, 2026
@rust-bors
Copy link
Contributor

rust-bors bot commented Jan 12, 2026

📌 Commit 6f12b86 has been approved by uweigand,tgross35

It is now in the queue for this repository.

@rust-bors rust-bors bot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 12, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jan 12, 2026
@JonathanBrouwer
Rollup merge of rust-lang#150826 - s390x-asm-f16-vector, r=uweigand,t…
586d65a 
…gross35

Add `f16` inline ASM support for s390x

tracking issue: rust-lang#116909
cc rust-lang#125398

Support the `f16x8` type in inline assembly. Only with the `nnp-assist` feature are there any instructions that make use of this type. Based on the riscv implementation I now cast to `i16x8` when that feature is not enabled.

As far as I'm aware there are no instructions operating on `f16` scalar values. Should we still add support for using them in inline assembly?

r? @tgross35
cc @uweigand
@JonathanBrouwer JonathanBrouwer mentioned this pull request Jan 12, 2026
Closed
rust-bors bot pushed a commit that referenced this pull request Jan 12, 2026
@bors
Auto merge of #151023 - JonathanBrouwer:rollup-f6vMoSd, r=JonathanBro…
8d04a9f 
…uwer

Rollup of 13 pull requests

Successful merges:

 - #145343 (Dogfood `-Zno-embed-metadata` in the standard library)
 - #150151 (Destabilise `target-spec-json`)
 - #150723 (std: move `errno` and related functions into `sys::io`)
 - #150771 (Remove legacy homu `try` and `auto` branch mentions)
 - #150826 (Add `f16` inline ASM support for s390x)
 - #150934 (Move some checks from `check_doc_attrs` directly into `rustc_attr_parsing`)
 - #150943 (Port `#[must_not_suspend]` to attribute parser)
 - #150990 (std: sys: net: uefi: Make TcpStream Send)
 - #150995 (core: ptr: split_at_mut: fix typo in safety doc)
 - #150998 (Relax test expectation for @__llvm_profile_runtime_user)
 - #151002 (Remove a workaround for a bug (take 2))
 - #151005 (Fix typo in `MaybeUninit` docs)
 - #151011 (Update books)

r? @ghost
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 12, 2026
@matthiaskrgr
Rollup merge of rust-lang#150826 - s390x-asm-f16-vector, r=uweigand,t…
6230311 
…gross35

Add `f16` inline ASM support for s390x

tracking issue: rust-lang#116909
cc rust-lang#125398

Support the `f16x8` type in inline assembly. Only with the `nnp-assist` feature are there any instructions that make use of this type. Based on the riscv implementation I now cast to `i16x8` when that feature is not enabled.

As far as I'm aware there are no instructions operating on `f16` scalar values. Should we still add support for using them in inline assembly?

r? @tgross35
cc @uweigand
@matthiaskrgr matthiaskrgr mentioned this pull request Jan 12, 2026
Closed
rust-bors bot pushed a commit that referenced this pull request Jan 13, 2026
@bors
Auto merge of #151038 - matthiaskrgr:rollup-fBdEuCE, r=matthiaskrgr
56cc9e3 
Rollup of 12 pull requests

Successful merges:

 - #145343 (Dogfood `-Zno-embed-metadata` in the standard library)
 - #150151 (Destabilise `target-spec-json`)
 - #150723 (std: move `errno` and related functions into `sys::io`)
 - #150826 (Add `f16` inline ASM support for s390x)
 - #150934 (Move some checks from `check_doc_attrs` directly into `rustc_attr_parsing`)
 - #150943 (Port `#[must_not_suspend]` to attribute parser)
 - #150990 (std: sys: net: uefi: Make TcpStream Send)
 - #150995 (core: ptr: split_at_mut: fix typo in safety doc)
 - #150998 (Relax test expectation for @__llvm_profile_runtime_user)
 - #151002 (Remove a workaround for a bug (take 2))
 - #151005 (Fix typo in `MaybeUninit` docs)
 - #151011 (Update books)

r? @ghost
@JonathanBrouwer JonathanBrouwer mentioned this pull request Jan 13, 2026
Merged
rust-bors bot pushed a commit that referenced this pull request Jan 13, 2026
@bors
Auto merge of #151051 - JonathanBrouwer:rollup-SuaGLmP, r=JonathanBro…
db1484b 
…uwer

Rollup of 14 pull requests

Successful merges:

 - #150151 (Destabilise `target-spec-json`)
 - #150826 (Add `f16` inline ASM support for s390x)
 - #150883 (Improve span for "unresolved intra doc link" on `deprecated` attribute)
 - #150934 (Move some checks from `check_doc_attrs` directly into `rustc_attr_parsing`)
 - #150943 (Port `#[must_not_suspend]` to attribute parser)
 - #150990 (std: sys: net: uefi: Make TcpStream Send)
 - #150995 (core: ptr: split_at_mut: fix typo in safety doc)
 - #150998 (Relax test expectation for @__llvm_profile_runtime_user)
 - #151002 (Remove a workaround for a bug (take 2))
 - #151005 (Fix typo in `MaybeUninit` docs)
 - #151011 (Update books)
 - #151029 (rustc-dev-guide subtree update)
 - #151032 (fix: added missing backtick in triagebot.toml)
 - #151035 (Don't suggest replacing closure parameter with type name)

r? @ghost
@rust-bors rust-bors bot merged commit 002b68d into rust-lang:main Jan 13, 2026
11 checks passed
@rustbot rustbot added this to the 1.94.0 milestone Jan 13, 2026
rust-timer added a commit that referenced this pull request Jan 13, 2026
@rust-timer
Unrolled build for #150826
8c71d78 
Rollup merge of #150826 - s390x-asm-f16-vector, r=uweigand,tgross35

Add `f16` inline ASM support for s390x

tracking issue: #116909
cc #125398

Support the `f16x8` type in inline assembly. Only with the `nnp-assist` feature are there any instructions that make use of this type. Based on the riscv implementation I now cast to `i16x8` when that feature is not enabled.

As far as I'm aware there are no instructions operating on `f16` scalar values. Should we still add support for using them in inline assembly?

r? @tgross35
cc @uweigand
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tgross35 tgross35 tgross35 approved these changes

Assignees

@tgross35 tgross35

Labels

A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. F-f16_and_f128 `#![feature(f16)]`, `#![feature(f128)]` 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.94.0

Development

Successfully merging this pull request may close these issues.

5 participants

@folkertdev @rust-log-analyzer @tgross35 @uweigand @rustbot