rustc_target: add known safe s390x target features by liushuyu · Pull Request #127506 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation19 Commits4 Checks6 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

This pull request adds known safe target features for s390x (aka IBM Z systems).
Currently, these features are unstable since stabilizing the target features requires submitting proposals.

The vector feature was added in IBM Z13 (arch11), and this is a SIMD feature for the newer IBM Z systems.
The backchain attribute is the IBM Z way of adding frame pointers like unwinding capabilities (the "frame-pointer" switch on IBM Z and IBM POWER platforms will add emulated frame pointers to the binary, which profilers can't use for unwinding the stack).

Both attributes can be applied at the LLVM module or function levels. However, the backchain attribute has to be enabled for all the functions in the call stack to get a successful unwind process.

r? @davidtwco

rustbot has assigned @davidtwco.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

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

Jul 9, 2024

This comment has been minimized.

liushuyu marked this pull request as ready for review

July 10, 2024 05:32

Some changes occurred in tests/ui/check-cfg

cc @Urgau

Choose a reason for hiding this comment

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

Apologies for the delay in reviewing, looks good to me, r=me after rebasing.

... this is a special attribute that was made to be a target-feature in LLVM 18+, but in all previous versions, this "feature" is a naked attribute. We will have to handle this situation differently than all other target-features.

📌 Commit 01e6e60 has been approved by davidtwco

It is now in the queue for this repository.

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

Jul 22, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request

Jul 22, 2024

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Jul 22, 2024

Rollup merge of rust-lang#127506 - liushuyu:s390x-target-features, r=davidtwco

rustc_target: add known safe s390x target features

This pull request adds known safe target features for s390x (aka IBM Z systems). Currently, these features are unstable since stabilizing the target features requires submitting proposals.

The vector feature was added in IBM Z13 (arch11), and this is a SIMD feature for the newer IBM Z systems. The backchain attribute is the IBM Z way of adding frame pointers like unwinding capabilities (the "frame-pointer" switch on IBM Z and IBM POWER platforms will add emulated frame pointers to the binary, which profilers can't use for unwinding the stack).

Both attributes can be applied at the LLVM module or function levels. However, the backchain attribute has to be enabled for all the functions in the call stack to get a successful unwind process.

// skip checking special features, as LLVM may not understands them
if RUSTC_SPECIAL_FEATURES.contains(feature) {
return true;
}

Choose a reason for hiding this comment

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

This makes it so that cfg!(target_feature = "backchain") will always be true, even if it is not enabled. Is that the intended behavior? It seems very odd.

Choose a reason for hiding this comment

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

No, this is to skip checking if LLVM supports this attribute. The logic for adding this attribute is somewhere else.
If you are unconvinced, check out this Godbolt playground: https://godbolt.org/z/77hrrsbo1.

Choose a reason for hiding this comment

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

I am convinced that the logic is wrong, see #129927.

Choose a reason for hiding this comment

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

Also, I have no idea what I am supposed to look at in this Godbolt.^^ I know nothing about s390x or backchain, I am just noticing odd things in the rustc source code.

// if the target-feature is "backchain" and LLVM version is greater than 18
// then we also need to add "+backchain" to the target-features attribute.
// otherwise, we will only add the naked `backchain` attribute to the attribute-group.
if feature == "backchain" && llvm_major < 18 {

Choose a reason for hiding this comment

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

This entire RUSTC_SPECIAL_FEATURES is a temporary hack we can drop when we start requiring LLVM 18, right? Please file an issue to track this cleanup.

Choose a reason for hiding this comment

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

This entire RUSTC_SPECIAL_FEATURES is a temporary hack we can drop when we start requiring LLVM 18, right? Please file an issue to track this cleanup.

Yes, this is due to older LLVM does not support adding this attribute to the target-features attribute.

This was referenced

Sep 2, 2024

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Sep 5, 2024

…RalfJung

s390x: Fix a regression related to backchain feature

In rust-lang#127506, we introduced a new IBM Z-specific target feature, backchain.

This particular target-feature was available as a function-level attribute in LLVM 17 and below, so some hacks were used to avoid blowing up LLVM when querying the supported LLVM features.

This led to an unfortunate regression where cfg!(target-feature = "backchain") will always return true.

This pull request aims to fix this issue, and a test has been introduced to ensure it will never happen again.

Fixes rust-lang#129927.

r? @RalfJung

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Sep 5, 2024

…RalfJung

s390x: Fix a regression related to backchain feature

In rust-lang#127506, we introduced a new IBM Z-specific target feature, backchain.

This particular target-feature was available as a function-level attribute in LLVM 17 and below, so some hacks were used to avoid blowing up LLVM when querying the supported LLVM features.

This led to an unfortunate regression where cfg!(target-feature = "backchain") will always return true.

This pull request aims to fix this issue, and a test has been introduced to ensure it will never happen again.

Fixes rust-lang#129927.

r? @RalfJung

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Sep 5, 2024

…RalfJung

s390x: Fix a regression related to backchain feature

In rust-lang#127506, we introduced a new IBM Z-specific target feature, backchain.

This particular target-feature was available as a function-level attribute in LLVM 17 and below, so some hacks were used to avoid blowing up LLVM when querying the supported LLVM features.

This led to an unfortunate regression where cfg!(target-feature = "backchain") will always return true.

This pull request aims to fix this issue, and a test has been introduced to ensure it will never happen again.

Fixes rust-lang#129927.

r? @RalfJung

compiler-errors added a commit to compiler-errors/rust that referenced this pull request

Sep 7, 2024

…RalfJung

s390x: Fix a regression related to backchain feature

In rust-lang#127506, we introduced a new IBM Z-specific target feature, backchain.

This particular target-feature was available as a function-level attribute in LLVM 17 and below, so some hacks were used to avoid blowing up LLVM when querying the supported LLVM features.

This led to an unfortunate regression where cfg!(target-feature = "backchain") will always return true.

This pull request aims to fix this issue, and a test has been introduced to ensure it will never happen again.

Fixes rust-lang#129927.

r? @RalfJung

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Sep 7, 2024

Rollup merge of rust-lang#129940 - liushuyu:s390x-target-features, r=RalfJung

s390x: Fix a regression related to backchain feature

In rust-lang#127506, we introduced a new IBM Z-specific target feature, backchain.

This particular target-feature was available as a function-level attribute in LLVM 17 and below, so some hacks were used to avoid blowing up LLVM when querying the supported LLVM features.

This led to an unfortunate regression where cfg!(target-feature = "backchain") will always return true.

This pull request aims to fix this issue, and a test has been introduced to ensure it will never happen again.

Fixes rust-lang#129927.

r? @RalfJung

This was referenced

Sep 20, 2024

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request

Oct 1, 2024

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 or GCC, 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, and using a[2-15] causes errors. 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

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Oct 1, 2024

Rollup merge of rust-lang#130630 - taiki-e:s390x-clobber-abi, r=Amanieu

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 or GCC, 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, and using a[2-15] causes errors. 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

antoyo pushed a commit to rust-lang/rustc_codegen_gcc that referenced this pull request

Oct 9, 2024

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 #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/rust#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/rust#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 or GCC, 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/rust#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/rust#119431)...

Note:

GCC seems to recognize only a0 and a1, and using a[2-15] causes errors. Given that cg_gcc has a similar problem with other architecture (#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/rust#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/rust#88245 (comment)

cc @uweigand

r? @Amanieu

@rustbot label +O-SystemZ

fneddy added a commit to fneddy/rust that referenced this pull request

Apr 29, 2025

the s390x target_feature(enable = "vector") was merged in rust-lang#127506 but is still gated by #![feature(s390x_target_feature)].

fneddy added a commit to fneddy/rust that referenced this pull request

Apr 29, 2025

Fix the test for s390x by enabling s390x vector extension via target_feature(enable = "vector")(rust-lang#127506). As this is is still gated by #![feature(s390x_target_feature)] we need that attribute also.

fneddy added a commit to fneddy/rust that referenced this pull request

Apr 29, 2025

fneddy added a commit to fneddy/rust that referenced this pull request

May 2, 2025

fneddy added a commit to fneddy/rust that referenced this pull request

May 2, 2025

fneddy added a commit to fneddy/rust that referenced this pull request

May 3, 2025

fneddy added a commit to fneddy/rust that referenced this pull request

May 3, 2025

Zalathar added a commit to Zalathar/rust that referenced this pull request

May 4, 2025