-
Notifications
You must be signed in to change notification settings - Fork 14k
Stabilize s390x vector target feature and is_s390x_feature_detected! macro
#145656
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
base: master
Are you sure you want to change the base?
Conversation
|
cc @Amanieu, @folkertdev, @sayantn |
|
@rfcbot merge |
|
Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:
No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
I think this is fine since we don't have a soft-float s390x target and don't expose the |
|
|
|
On s390x, in LLVM (and GCC), the In LLVM in particular, the back-end does actually implement that In practice, the only user of |
|
We've revised and approved the Reference PR, so this will be good to go on that front when FCP completes. (cc @rust-lang/lang-docs) On the lang side, this looks good to me. @rfcbot reviewed |
|
The linux kernel needs a separate target that has soft-float set. We cannot support such ABI-changing target features with |
|
☔ The latest upstream changes (presumably #145728) made this pull request unmergeable. Please resolve the merge conflicts. |
|
As for the soft-float target feature, I don't think we need to worry about it here. It is not supported on rustc's target feature interface (rustc emits a "unknown and unstable feature" warning that says "use of this feature might be unsound" when it is used), and as discussed in the vector ABI PR (#131586 (comment)), it will eventually be rejected for setting via -Ctarget-feature. For the Linux kernel, a bare-metal soft-float (custom or new builtin) target should be used. |
|
@rfcbot reviewed |
… r=Amanieu Stabilize s390x `vector` target feature and `is_s390x_feature_detected!` macro closes rust-lang#145649 closes rust-lang#135413 cc: rust-lang#130869 reference PR: rust-lang/reference#1972 # Stabilization report ## Summary This PR stabilizes the following s390x target features: - `vector` - `vector-enhancements-1` - `vector-enhancements-2` - `vector-enhancements-3` - `vector-packed-decimal` - `vector-packed-decimal-enhancement` - `vector-packed-decimal-enhancement-2` - `vector-packed-decimal-enhancement-3` - `nnp-assist` - `miscellaneous-extensions-2` - `miscellaneous-extensions-3` - `miscellaneous-extensions-4` Additionally, it stabilizes the `std::arch::is_s390x_feature_detected!` macro itself and stably accepts the target features listed above. ## Tests & ABI details Only the `vector` target feature changes the ABI, much like e.g. `avx2` it will, depending on the ABI, pass vector types in vector registers. This behavior is tested extensively: - [tests/assembly-llvm/s390x-vector-abi.rs](https://github.com/rust-lang/rust/blob/22a86f8280becb12c34ee3efd952baf5cf086fa0/tests/assembly-llvm/s390x-vector-abi.rs) - [tests/codegen-llvm/s390x-simd.rs](https://github.com/rust-lang/rust/blob/22a86f8280becb12c34ee3efd952baf5cf086fa0/tests/assembly-llvm/s390x-vector-abi.rs) - [tests/ui/abi/simd-abi-checks-s390x.rs ](https://github.com/rust-lang/rust/blob/22a86f8280becb12c34ee3efd952baf5cf086fa0/tests/ui/abi/simd-abi-checks-s390x.rs ) The remaining features don't influence the ABI, they only influence instruction selection. In stdarch we test that the expected instructions are in fact generated when the target feature is enabled. ## Implementation history For `is_s390x_feature_detected!`: - rust-lang/stdarch#1699 - rust-lang#138275 - rust-lang/stdarch#1720 - rust-lang/stdarch#1832 For `vector` and friends - rust-lang#127506 - rust-lang#135630 - rust-lang#141250 ## Unresolved questions There is a fixme in [tests/ui/abi/simd-abi-checks-s390x.rs](https://github.com/rust-lang/rust/blob/22a86f8280becb12c34ee3efd952baf5cf086fa0/tests/ui/abi/simd-abi-checks-s390x.rs): ``` // FIXME: +soft-float itself doesn't set -vector //`@[z13_soft_float]` compile-flags: --target s390x-unknown-linux-gnu -C target-cpu=z13 -C target-feature=-vector,+soft-float //`@[z13_soft_float]` needs-llvm-components: systemz ``` I'm not sure whether that blocks stabilization? --- The implementation first extracts the listed target features into their own `s390x_target_feature_vector` rust feature, and then stabilizes that. best reviewed commit-by-commit r? `@Amanieu` cc `@uweigand` `@taiki-e`
Rollup of 5 pull requests Successful merges: - #145656 (Stabilize s390x `vector` target feature and `is_s390x_feature_detected!` macro) - #148204 (Modify contributor email entries in .mailmap) - #148556 (Fix suggestion for returning async closures) - #148585 ([rustdoc] Replace `print` methods with functions to improve code readability) - #148608 (Add test for --test-builder success path) r? `@ghost` `@rustbot` modify labels: rollup
… r=Amanieu Stabilize s390x `vector` target feature and `is_s390x_feature_detected!` macro closes rust-lang#145649 closes rust-lang#135413 cc: rust-lang#130869 reference PR: rust-lang/reference#1972 # Stabilization report ## Summary This PR stabilizes the following s390x target features: - `vector` - `vector-enhancements-1` - `vector-enhancements-2` - `vector-enhancements-3` - `vector-packed-decimal` - `vector-packed-decimal-enhancement` - `vector-packed-decimal-enhancement-2` - `vector-packed-decimal-enhancement-3` - `nnp-assist` - `miscellaneous-extensions-2` - `miscellaneous-extensions-3` - `miscellaneous-extensions-4` Additionally, it stabilizes the `std::arch::is_s390x_feature_detected!` macro itself and stably accepts the target features listed above. ## Tests & ABI details Only the `vector` target feature changes the ABI, much like e.g. `avx2` it will, depending on the ABI, pass vector types in vector registers. This behavior is tested extensively: - [tests/assembly-llvm/s390x-vector-abi.rs](https://github.com/rust-lang/rust/blob/22a86f8280becb12c34ee3efd952baf5cf086fa0/tests/assembly-llvm/s390x-vector-abi.rs) - [tests/codegen-llvm/s390x-simd.rs](https://github.com/rust-lang/rust/blob/22a86f8280becb12c34ee3efd952baf5cf086fa0/tests/assembly-llvm/s390x-vector-abi.rs) - [tests/ui/abi/simd-abi-checks-s390x.rs ](https://github.com/rust-lang/rust/blob/22a86f8280becb12c34ee3efd952baf5cf086fa0/tests/ui/abi/simd-abi-checks-s390x.rs ) The remaining features don't influence the ABI, they only influence instruction selection. In stdarch we test that the expected instructions are in fact generated when the target feature is enabled. ## Implementation history For `is_s390x_feature_detected!`: - rust-lang/stdarch#1699 - rust-lang#138275 - rust-lang/stdarch#1720 - rust-lang/stdarch#1832 For `vector` and friends - rust-lang#127506 - rust-lang#135630 - rust-lang#141250 ## Unresolved questions There is a fixme in [tests/ui/abi/simd-abi-checks-s390x.rs](https://github.com/rust-lang/rust/blob/22a86f8280becb12c34ee3efd952baf5cf086fa0/tests/ui/abi/simd-abi-checks-s390x.rs): ``` // FIXME: +soft-float itself doesn't set -vector //``@[z13_soft_float]`` compile-flags: --target s390x-unknown-linux-gnu -C target-cpu=z13 -C target-feature=-vector,+soft-float //``@[z13_soft_float]`` needs-llvm-components: systemz ``` I'm not sure whether that blocks stabilization? --- The implementation first extracts the listed target features into their own `s390x_target_feature_vector` rust feature, and then stabilizes that. best reviewed commit-by-commit r? ``@Amanieu`` cc ``@uweigand`` ``@taiki-e``
Rollup of 7 pull requests Successful merges: - #145656 (Stabilize s390x `vector` target feature and `is_s390x_feature_detected!` macro) - #148204 (Modify contributor email entries in .mailmap) - #148556 (Fix suggestion for returning async closures) - #148585 ([rustdoc] Replace `print` methods with functions to improve code readability) - #148600 (re-use `self.get_all_attrs` result for pass indirectly attribute) - #148612 (Add note for identifier with attempted hygiene violation) - #148613 (Switch hexagon targets to rust-lld) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 7 pull requests Successful merges: - #145656 (Stabilize s390x `vector` target feature and `is_s390x_feature_detected!` macro) - #148204 (Modify contributor email entries in .mailmap) - #148556 (Fix suggestion for returning async closures) - #148585 ([rustdoc] Replace `print` methods with functions to improve code readability) - #148600 (re-use `self.get_all_attrs` result for pass indirectly attribute) - #148612 (Add note for identifier with attempted hygiene violation) - #148613 (Switch hexagon targets to rust-lld) r? `@ghost` `@rustbot` modify labels: rollup
Stabilize s390x `vector` target feature and `is_s390x_feature_detected!` macro closes #145649 closes #135413 cc: #130869 reference PR: rust-lang/reference#1972 # Stabilization report ## Summary This PR stabilizes the following s390x target features: - `vector` - `vector-enhancements-1` - `vector-enhancements-2` - `vector-enhancements-3` - `vector-packed-decimal` - `vector-packed-decimal-enhancement` - `vector-packed-decimal-enhancement-2` - `vector-packed-decimal-enhancement-3` - `nnp-assist` - `miscellaneous-extensions-2` - `miscellaneous-extensions-3` - `miscellaneous-extensions-4` Additionally, it stabilizes the `std::arch::is_s390x_feature_detected!` macro itself and stably accepts the target features listed above. ## Tests & ABI details Only the `vector` target feature changes the ABI, much like e.g. `avx2` it will, depending on the ABI, pass vector types in vector registers. This behavior is tested extensively: - [tests/assembly-llvm/s390x-vector-abi.rs](https://github.com/rust-lang/rust/blob/22a86f8280becb12c34ee3efd952baf5cf086fa0/tests/assembly-llvm/s390x-vector-abi.rs) - [tests/codegen-llvm/s390x-simd.rs](https://github.com/rust-lang/rust/blob/22a86f8280becb12c34ee3efd952baf5cf086fa0/tests/assembly-llvm/s390x-vector-abi.rs) - [tests/ui/abi/simd-abi-checks-s390x.rs ](https://github.com/rust-lang/rust/blob/22a86f8280becb12c34ee3efd952baf5cf086fa0/tests/ui/abi/simd-abi-checks-s390x.rs ) The remaining features don't influence the ABI, they only influence instruction selection. In stdarch we test that the expected instructions are in fact generated when the target feature is enabled. ## Implementation history For `is_s390x_feature_detected!`: - rust-lang/stdarch#1699 - #138275 - rust-lang/stdarch#1720 - rust-lang/stdarch#1832 For `vector` and friends - #127506 - #135630 - #141250 ## Unresolved questions There is a fixme in [tests/ui/abi/simd-abi-checks-s390x.rs](https://github.com/rust-lang/rust/blob/22a86f8280becb12c34ee3efd952baf5cf086fa0/tests/ui/abi/simd-abi-checks-s390x.rs): ``` // FIXME: +soft-float itself doesn't set -vector //`@[z13_soft_float]` compile-flags: --target s390x-unknown-linux-gnu -C target-cpu=z13 -C target-feature=-vector,+soft-float //`@[z13_soft_float]` needs-llvm-components: systemz ``` I'm not sure whether that blocks stabilization? --- The implementation first extracts the listed target features into their own `s390x_target_feature_vector` rust feature, and then stabilizes that. best reviewed commit-by-commit r? `@Amanieu` cc `@uweigand` `@taiki-e`
|
💔 Test failed - checks-actions |
|
The queue is closed, and only high-priority PRs are allowed. This won't be retried until the problem is fixed. |
Ah cool, didn't know that, thanks! |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
Stabilize s390x `vector` target feature and `is_s390x_feature_detected!` macro closes #145649 closes #135413 cc: #130869 reference PR: rust-lang/reference#1972 # Stabilization report ## Summary This PR stabilizes the following s390x target features: - `vector` - `vector-enhancements-1` - `vector-enhancements-2` - `vector-enhancements-3` - `vector-packed-decimal` - `vector-packed-decimal-enhancement` - `vector-packed-decimal-enhancement-2` - `vector-packed-decimal-enhancement-3` - `nnp-assist` - `miscellaneous-extensions-2` - `miscellaneous-extensions-3` - `miscellaneous-extensions-4` Additionally, it stabilizes the `std::arch::is_s390x_feature_detected!` macro itself and stably accepts the target features listed above. ## Tests & ABI details Only the `vector` target feature changes the ABI, much like e.g. `avx2` it will, depending on the ABI, pass vector types in vector registers. This behavior is tested extensively: - [tests/assembly-llvm/s390x-vector-abi.rs](https://github.com/rust-lang/rust/blob/22a86f8280becb12c34ee3efd952baf5cf086fa0/tests/assembly-llvm/s390x-vector-abi.rs) - [tests/codegen-llvm/s390x-simd.rs](https://github.com/rust-lang/rust/blob/22a86f8280becb12c34ee3efd952baf5cf086fa0/tests/assembly-llvm/s390x-vector-abi.rs) - [tests/ui/abi/simd-abi-checks-s390x.rs ](https://github.com/rust-lang/rust/blob/22a86f8280becb12c34ee3efd952baf5cf086fa0/tests/ui/abi/simd-abi-checks-s390x.rs ) The remaining features don't influence the ABI, they only influence instruction selection. In stdarch we test that the expected instructions are in fact generated when the target feature is enabled. ## Implementation history For `is_s390x_feature_detected!`: - rust-lang/stdarch#1699 - #138275 - rust-lang/stdarch#1720 - rust-lang/stdarch#1832 For `vector` and friends - #127506 - #135630 - #141250 ## Unresolved questions There is a fixme in [tests/ui/abi/simd-abi-checks-s390x.rs](https://github.com/rust-lang/rust/blob/22a86f8280becb12c34ee3efd952baf5cf086fa0/tests/ui/abi/simd-abi-checks-s390x.rs): ``` // FIXME: +soft-float itself doesn't set -vector //`@[z13_soft_float]` compile-flags: --target s390x-unknown-linux-gnu -C target-cpu=z13 -C target-feature=-vector,+soft-float //`@[z13_soft_float]` needs-llvm-components: systemz ``` I'm not sure whether that blocks stabilization? --- The implementation first extracts the listed target features into their own `s390x_target_feature_vector` rust feature, and then stabilizes that. best reviewed commit-by-commit r? `@Amanieu` cc `@uweigand` `@taiki-e`
|
💔 Test failed - checks-actions |
|
@bors retry |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
Rollup of 7 pull requests Successful merges: - #145656 (Stabilize s390x `vector` target feature and `is_s390x_feature_detected!` macro) - #148204 (Modify contributor email entries in .mailmap) - #148556 (Fix suggestion for returning async closures) - #148585 ([rustdoc] Replace `print` methods with functions to improve code readability) - #148600 (re-use `self.get_all_attrs` result for pass indirectly attribute) - #148612 (Add note for identifier with attempted hygiene violation) - #148613 (Switch hexagon targets to rust-lld) r? `@ghost` `@rustbot` modify labels: rollup
closes #145649
closes #135413
cc: #130869
reference PR: rust-lang/reference#1972
Stabilization report
Summary
This PR stabilizes the following s390x target features:
vectorvector-enhancements-1vector-enhancements-2vector-enhancements-3vector-packed-decimalvector-packed-decimal-enhancementvector-packed-decimal-enhancement-2vector-packed-decimal-enhancement-3nnp-assistmiscellaneous-extensions-2miscellaneous-extensions-3miscellaneous-extensions-4Additionally, it stabilizes the
std::arch::is_s390x_feature_detected!macro itself and stably accepts the target features listed above.Tests & ABI details
Only the
vectortarget feature changes the ABI, much like e.g.avx2it will, depending on the ABI, pass vector types in vector registers. This behavior is tested extensively:The remaining features don't influence the ABI, they only influence instruction selection. In stdarch we test that the expected instructions are in fact generated when the target feature is enabled.
Implementation history
For
is_s390x_feature_detected!:is_s390x_feature_detectedstdarch#1699is_s390x_feature_detected!fromstd::arch#138275s390x_is_feature_detected!: detect more features stdarch#1720For
vectorand friendss390xtarget features #135630Unresolved questions
There is a fixme in tests/ui/abi/simd-abi-checks-s390x.rs:
I'm not sure whether that blocks stabilization?
The implementation first extracts the listed target features into their own
s390x_target_feature_vectorrust feature, and then stabilizes that. best reviewed commit-by-commitr? @Amanieu
cc @uweigand @taiki-e