rustc_target: add known safe s390x target features #127506

liushuyu · 2024-07-09T06:54:42Z

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.

rustbot · 2024-07-09T06:54:49Z

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 · 2024-07-10T05:32:16Z

Some changes occurred in tests/ui/check-cfg

cc @Urgau

liushuyu · 2024-07-10T05:32:33Z

Ready for review

bors · 2024-07-12T18:11:46Z

☔ The latest upstream changes (presumably #127653) made this pull request unmergeable. Please resolve the merge conflicts.

davidtwco

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.

liushuyu · 2024-07-17T09:55:44Z

Rebased. Tests passed.

liushuyu · 2024-07-17T09:58:56Z

@rustbot ready

davidtwco · 2024-07-22T14:56:18Z

@bors r+ rollup

bors · 2024-07-22T14:56:21Z

📌 Commit 01e6e60 has been approved by davidtwco

It is now in the queue for this repository.

Rollup of 9 pull requests Successful merges: - rust-lang#117932 (Correct rustdoc section where we talk about rustdoc emitting errors on invalid code) - rust-lang#125990 (Rename `deprecated_safe` lint to `deprecated_safe_2024`) - rust-lang#127506 (rustc_target: add known safe s390x target features) - rust-lang#127820 (Rewrite and rename `issue-14698`. `issue-33329` and `issue-107094` `run-make` tests to rmake or ui) - rust-lang#127923 (Use reuse tool 4.0) - rust-lang#128008 (Start using `#[diagnostic::do_not_recommend]` in the standard library) - rust-lang#128036 (add more tests) - rust-lang#128051 (rustdoc: revert spacing change in item-table) - rust-lang#128059 (Add regression test for items list size (rust-lang#128023)) r? `@ghost` `@rustbot` modify labels: rollup

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.

RalfJung · 2024-09-02T10:00:53Z

compiler/rustc_codegen_llvm/src/llvm_util.rs

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


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.

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.

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

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.

RalfJung · 2024-09-02T10:13:54Z

compiler/rustc_codegen_llvm/src/llvm_util.rs

+            // 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 {


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.

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.

…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`

…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``

…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`

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`

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

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](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

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](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/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`](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 (#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

rustbot assigned davidtwco Jul 9, 2024

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.

Sign in to view

liushuyu force-pushed the s390x-target-features branch from e6d4236 to b5d21e0 Compare July 9, 2024 07:04

This comment has been minimized.

Sign in to view

liushuyu force-pushed the s390x-target-features branch from feeb092 to d5d39e8 Compare July 10, 2024 04:01

This comment has been minimized.

Sign in to view

liushuyu force-pushed the s390x-target-features branch from d5d39e8 to c139cc5 Compare July 10, 2024 04:29

liushuyu marked this pull request as ready for review July 10, 2024 05:32

workingjubilee added the O-SystemZ Target: SystemZ processors (s390x) label Jul 14, 2024

davidtwco approved these changes Jul 16, 2024

View reviewed changes

liushuyu added 4 commits July 17, 2024 07:53

rustc_target: add known safe s390x target features

efcf35e

rustc_target: fix UI tests for s390x

c940d52

rustc_codegen_ssa: add s390x_target_feature symbol

366bc86

liushuyu force-pushed the s390x-target-features branch from c139cc5 to 01e6e60 Compare July 16, 2024 23:56

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

tgross35 mentioned this pull request Jul 22, 2024

Rollup of 9 pull requests #128063

Merged

bors merged commit 5e8e46c into rust-lang:master Jul 22, 2024
6 checks passed

rustbot added this to the 1.82.0 milestone Jul 22, 2024

RalfJung reviewed Sep 2, 2024

View reviewed changes

This was referenced Sep 2, 2024

Add implied target features to target_feature attribute #128221

Merged

s390x-unknown-linux-gnu: cfg!(target_feature = "backchain") is always true #129927

Closed

liushuyu mentioned this pull request Sep 3, 2024

s390x: Fix a regression related to backchain feature #129940

Merged

This was referenced Sep 20, 2024

Support clobber_abi and vector/access registers (clobber-only) in s390x inline assembly #130630

Merged

s390x vector facilities support #130869

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustc_target: add known safe s390x target features #127506

rustc_target: add known safe s390x target features #127506

liushuyu commented Jul 9, 2024 •

edited

Loading

rustbot commented Jul 9, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rustbot commented Jul 10, 2024

liushuyu commented Jul 10, 2024

bors commented Jul 12, 2024

davidtwco left a comment

liushuyu commented Jul 17, 2024

liushuyu commented Jul 17, 2024

davidtwco commented Jul 22, 2024

bors commented Jul 22, 2024

RalfJung Sep 2, 2024

liushuyu Sep 3, 2024

RalfJung Sep 3, 2024

RalfJung Sep 3, 2024

RalfJung Sep 2, 2024

liushuyu Sep 3, 2024

rustc_target: add known safe s390x target features #127506

rustc_target: add known safe s390x target features #127506

Conversation

liushuyu commented Jul 9, 2024 • edited Loading

rustbot commented Jul 9, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rustbot commented Jul 10, 2024

liushuyu commented Jul 10, 2024

bors commented Jul 12, 2024

davidtwco left a comment

Choose a reason for hiding this comment

liushuyu commented Jul 17, 2024

liushuyu commented Jul 17, 2024

davidtwco commented Jul 22, 2024

bors commented Jul 22, 2024

RalfJung Sep 2, 2024

Choose a reason for hiding this comment

liushuyu Sep 3, 2024

Choose a reason for hiding this comment

RalfJung Sep 3, 2024

Choose a reason for hiding this comment

RalfJung Sep 3, 2024

Choose a reason for hiding this comment

RalfJung Sep 2, 2024

Choose a reason for hiding this comment

liushuyu Sep 3, 2024

Choose a reason for hiding this comment

liushuyu commented Jul 9, 2024 •

edited

Loading