Skip to content
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

ABI checks: add support for some tier3 arches, warn on others. #133029

Merged
merged 1 commit into from
Nov 17, 2024
Merged

ABI checks: add support for some tier3 arches, warn on others. #133029

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions compiler/rustc_monomorphize/src/mono_checks/abi_check.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,7 @@ fn do_check_abi<'tcx>(
target_feature_def: DefId,
mut emit_err: impl FnMut(Option<&'static str>),
) {
let Some(feature_def) = tcx.sess.target.features_for_correct_vector_abi() else {
return;
};
let feature_def = tcx.sess.target.features_for_correct_vector_abi();
let codegen_attrs = tcx.codegen_fn_attrs(target_feature_def);
for arg_abi in abi.args.iter().chain(std::iter::once(&abi.ret)) {
let size = arg_abi.layout.size;
Expand Down
37 changes: 23 additions & 14 deletions compiler/rustc_target/src/target_features.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,12 @@ const S390X_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] = &[
const RISCV_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] =
&[/*(64, "zvl64b"), */ (128, "v")];
// 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")*/];
const SPARC_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] = &[/*(64, "vis")*/];
Copy link
Member

Choose a reason for hiding this comment

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

Based on #131800 (comment) the situation is actually even weirder than what is described here, but it seems like we can cross that bridge when we come to it, still. The main purpose here of the comment was always to leave something to hit during grepping the codebase.


const HEXAGON_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] =
&[/*(512, "hvx-length64b"),*/ (1024, "hvx-length128b")];
const MIPS_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] = &[(128, "msa")];
const CSKY_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] = &[(128, "vdspv1")];

impl super::spec::Target {
pub fn rust_target_features(&self) -> &'static [(&'static str, Stability, ImpliedFeatures)] {
Expand All @@ -620,20 +625,24 @@ impl super::spec::Target {
}
}

// Returns None if we do not support ABI checks on the given target yet.
pub fn features_for_correct_vector_abi(&self) -> Option<&'static [(u64, &'static str)]> {
pub fn features_for_correct_vector_abi(&self) -> &'static [(u64, &'static str)] {
match &*self.arch {
"x86" | "x86_64" => Some(X86_FEATURES_FOR_CORRECT_VECTOR_ABI),
"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
"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,
"x86" | "x86_64" => X86_FEATURES_FOR_CORRECT_VECTOR_ABI,
"aarch64" | "arm64ec" => AARCH64_FEATURES_FOR_CORRECT_VECTOR_ABI,
"arm" => ARM_FEATURES_FOR_CORRECT_VECTOR_ABI,
"powerpc" | "powerpc64" => POWERPC_FEATURES_FOR_CORRECT_VECTOR_ABI,
"loongarch64" => &[], // on-stack ABI, so we complain about all by-val vectors
"riscv32" | "riscv64" => RISCV_FEATURES_FOR_CORRECT_VECTOR_ABI,
"wasm32" | "wasm64" => WASM_FEATURES_FOR_CORRECT_VECTOR_ABI,
"s390x" => S390X_FEATURES_FOR_CORRECT_VECTOR_ABI,
"sparc" | "sparc64" => SPARC_FEATURES_FOR_CORRECT_VECTOR_ABI,
"hexagon" => HEXAGON_FEATURES_FOR_CORRECT_VECTOR_ABI,
"mips" | "mips32r6" | "mips64" | "mips64r6" => MIPS_FEATURES_FOR_CORRECT_VECTOR_ABI,
"bpf" => &[], // no vector ABI
Copy link
Member

Choose a reason for hiding this comment

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

specifically, it only has ten 64-bit GPRs (and a frame pointer which is read-only and may not be scratched).

Copy link
Member

@workingjubilee workingjubilee Nov 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

LLVM apparently generates a somewhat arbitrary on-stack ABI if presented with 6+ arguments (or crashes, depending on version?)

"csky" => CSKY_FEATURES_FOR_CORRECT_VECTOR_ABI,
// FIXME: for some tier3 targets, we are overly cautious and always give warnings
// when passing args in vector registers.
_ => &[],
}
}

Expand Down
Loading