-
Notifications
You must be signed in to change notification settings - Fork 13k
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
ABI checks: add support for tier2 arches #132842
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -586,9 +586,20 @@ pub fn all_rust_features() -> impl Iterator<Item = (&'static str, Stability)> { | |
// certain size to have their "proper" ABI on each architecture. | ||
// Note that they must be kept sorted by vector size. | ||
const X86_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] = | ||
&[(128, "sse"), (256, "avx"), (512, "avx512f")]; | ||
&[(128, "sse"), (256, "avx"), (512, "avx512f")]; // FIXME: might need changes for AVX10. | ||
const AARCH64_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] = &[(128, "neon")]; | ||
|
||
// We might want to add "helium" too. | ||
const ARM_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] = &[(128, "neon")]; | ||
veluca93 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
const POWERPC_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] = &[(128, "altivec")]; | ||
const WASM_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] = &[(128, "simd128")]; | ||
const S390X_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] = &[(128, "vector")]; | ||
const RISCV_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] = | ||
&[/*(64, "zvl64b"), */ (128, "v")]; | ||
RalfJung marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Always warn on SPARC, as the necessary target features cannot be enabled in Rust at the moment. | ||
const SPARC_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] = &[/*(128, "vis")*/]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the source for the 128 here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @workingjubilee :-) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. checks I could've sworn it was 128-bit, but apparently it's only 64-bit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is already rolled up so let's let it land, and then @veluca93 can you prepare a follow-up to fix the comment? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know one of the SPARC CPUs has 128-bit registers, but I guess I was misremembering what extension defines that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can get it in the tier 3 pass, but yeah, it's just a comment. I probably should've just skipped noting down a size at all. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That also works. |
||
|
||
impl super::spec::Target { | ||
pub fn rust_target_features(&self) -> &'static [(&'static str, Stability, ImpliedFeatures)] { | ||
match &*self.arch { | ||
|
@@ -613,8 +624,15 @@ impl super::spec::Target { | |
pub fn features_for_correct_vector_abi(&self) -> Option<&'static [(u64, &'static str)]> { | ||
match &*self.arch { | ||
"x86" | "x86_64" => Some(X86_FEATURES_FOR_CORRECT_VECTOR_ABI), | ||
"aarch64" => Some(AARCH64_FEATURES_FOR_CORRECT_VECTOR_ABI), | ||
// FIXME: add support for non-tier1 architectures | ||
"aarch64" | "arm64ec" => Some(AARCH64_FEATURES_FOR_CORRECT_VECTOR_ABI), | ||
"arm" => Some(ARM_FEATURES_FOR_CORRECT_VECTOR_ABI), | ||
"powerpc" | "powerpc64" => Some(POWERPC_FEATURES_FOR_CORRECT_VECTOR_ABI), | ||
"loongarch64" => Some(&[]), // on-stack ABI, so we complain about all by-val vectors | ||
veluca93 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"riscv32" | "riscv64" => Some(RISCV_FEATURES_FOR_CORRECT_VECTOR_ABI), | ||
"wasm32" | "wasm64" => Some(WASM_FEATURES_FOR_CORRECT_VECTOR_ABI), | ||
"s390x" => Some(S390X_FEATURES_FOR_CORRECT_VECTOR_ABI), | ||
"sparc" | "sparc64" => Some(SPARC_FEATURES_FOR_CORRECT_VECTOR_ABI), | ||
// FIXME: add support for non-tier2 architectures | ||
_ => None, | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
//@ needs-llvm-components: sparc | ||
//@ compile-flags: --target=sparc-unknown-none-elf --crate-type=rlib | ||
//@ build-pass | ||
//@ ignore-pass (test emits codegen-time warnings) | ||
#![no_core] | ||
#![feature(no_core, lang_items, repr_simd)] | ||
#![allow(improper_ctypes_definitions)] | ||
#[lang = "sized"] | ||
trait Sized {} | ||
|
||
#[lang = "copy"] | ||
trait Copy {} | ||
|
||
#[repr(simd)] | ||
pub struct SimdVec([i32; 4]); | ||
|
||
pub extern "C" fn pass_by_vec(_: SimdVec) {} | ||
//~^ this function definition uses a SIMD vector type that is not currently supported with the chosen ABI | ||
//~| WARNING this was previously accepted by the compiler |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
warning: this function definition uses a SIMD vector type that is not currently supported with the chosen ABI | ||
--> $DIR/simd-abi-checks-empty-list.rs:17:1 | ||
| | ||
LL | pub extern "C" fn pass_by_vec(_: SimdVec) {} | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function defined here | ||
| | ||
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! | ||
= note: for more information, see issue #116558 <https://github.com/rust-lang/rust/issues/116558> | ||
= note: `#[warn(abi_unsupported_vector_types)]` on by default | ||
|
||
warning: 1 warning emitted | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should add a second line of
help: consider passing a reference to the value instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure if you control both sides of the FFI you can do that.