From a06f3556aa1ce9b02f059dfecc15ac8ebee318f2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 21 Nov 2023 10:15:59 +0100 Subject: [PATCH 1/2] the unadjusted ABI needs to pass aggregates by-value --- compiler/rustc_target/src/abi/call/mod.rs | 1 + compiler/rustc_ty_utils/src/abi.rs | 35 +++++++++++++++++++---- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index f7c860cf56b91..e9730947389fe 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -382,6 +382,7 @@ impl HomogeneousAggregate { } impl<'a, Ty> TyAndLayout<'a, Ty> { + /// Returns `true` if this is an aggregate type (including a ScalarPair!) fn is_aggregate(&self) -> bool { match self.abi { Abi::Uninhabited | Abi::Scalar(_) | Abi::Vector { .. } => false, diff --git a/compiler/rustc_ty_utils/src/abi.rs b/compiler/rustc_ty_utils/src/abi.rs index ec2fb4888ea6b..9ad7a1f588edd 100644 --- a/compiler/rustc_ty_utils/src/abi.rs +++ b/compiler/rustc_ty_utils/src/abi.rs @@ -327,10 +327,15 @@ fn adjust_for_rust_scalar<'tcx>( } /// Ensure that the ABI makes basic sense. -fn fn_abi_sanity_check<'tcx>(cx: &LayoutCx<'tcx, TyCtxt<'tcx>>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>) { +fn fn_abi_sanity_check<'tcx>( + cx: &LayoutCx<'tcx, TyCtxt<'tcx>>, + fn_abi: &FnAbi<'tcx, Ty<'tcx>>, + spec_abi: SpecAbi, +) { fn fn_arg_sanity_check<'tcx>( cx: &LayoutCx<'tcx, TyCtxt<'tcx>>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>, + spec_abi: SpecAbi, arg: &ArgAbi<'tcx, Ty<'tcx>>, ) { match &arg.mode { @@ -360,8 +365,8 @@ fn fn_abi_sanity_check<'tcx>(cx: &LayoutCx<'tcx, TyCtxt<'tcx>>, fn_abi: &FnAbi<' // (See issue: https://github.com/rust-lang/rust/issues/117271) assert!( matches!(&*cx.tcx.sess.target.arch, "wasm32" | "wasm64") - || fn_abi.conv == Conv::PtxKernel, - "`PassMode::Direct` for aggregates only allowed on wasm and `extern \"ptx-kernel\"` fns\nProblematic type: {:#?}", + || matches!(spec_abi, SpecAbi::PtxKernel | SpecAbi::Unadjusted), + r#"`PassMode::Direct` for aggregates only allowed for "unadjusted" and "ptx-kernel" functions and on wasm\nProblematic type: {:#?}"#, arg.layout, ); } @@ -391,9 +396,9 @@ fn fn_abi_sanity_check<'tcx>(cx: &LayoutCx<'tcx, TyCtxt<'tcx>>, fn_abi: &FnAbi<' } for arg in fn_abi.args.iter() { - fn_arg_sanity_check(cx, fn_abi, arg); + fn_arg_sanity_check(cx, fn_abi, spec_abi, arg); } - fn_arg_sanity_check(cx, fn_abi, &fn_abi.ret); + fn_arg_sanity_check(cx, fn_abi, spec_abi, &fn_abi.ret); } // FIXME(eddyb) perhaps group the signature/type-containing (or all of them?) @@ -522,7 +527,7 @@ fn fn_abi_new_uncached<'tcx>( }; fn_abi_adjust_for_abi(cx, &mut fn_abi, sig.abi, fn_def_id)?; debug!("fn_abi_new_uncached = {:?}", fn_abi); - fn_abi_sanity_check(cx, &fn_abi); + fn_abi_sanity_check(cx, &fn_abi, sig.abi); Ok(cx.tcx.arena.alloc(fn_abi)) } @@ -534,6 +539,24 @@ fn fn_abi_adjust_for_abi<'tcx>( fn_def_id: Option, ) -> Result<(), &'tcx FnAbiError<'tcx>> { if abi == SpecAbi::Unadjusted { + // The "unadjusted" ABI passes aggregates in "direct" mode. That's fragile but needed for + // some LLVM intrinsics. + fn unadjust<'tcx>(arg: &mut ArgAbi<'tcx, Ty<'tcx>>) { + // This still uses `PassMode::Pair` for ScalarPair types. That's unlikely to be intended, + // but who knows what breaks if we change this now. + if matches!(arg.layout.abi, Abi::Aggregate { .. }) { + assert!( + arg.layout.abi.is_sized(), + "'unadjusted' ABI does not support unsized arguments" + ); + } + arg.make_direct_deprecated(); + } + + unadjust(&mut fn_abi.ret); + for arg in fn_abi.args.iter_mut() { + unadjust(arg); + } return Ok(()); } From ebfb95a357122f8a1095d5078a6712dfa66a24f8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 22 Nov 2023 07:23:40 +0100 Subject: [PATCH 2/2] add a test --- tests/ui/abi/arm-unadjusted-intrinsic.rs | 54 ++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 tests/ui/abi/arm-unadjusted-intrinsic.rs diff --git a/tests/ui/abi/arm-unadjusted-intrinsic.rs b/tests/ui/abi/arm-unadjusted-intrinsic.rs new file mode 100644 index 0000000000000..33ea792752630 --- /dev/null +++ b/tests/ui/abi/arm-unadjusted-intrinsic.rs @@ -0,0 +1,54 @@ +// build-pass +// revisions: arm +//[arm] compile-flags: --target arm-unknown-linux-gnueabi +//[arm] needs-llvm-components: arm +// revisions: aarch64 +//[aarch64] compile-flags: --target aarch64-unknown-linux-gnu +//[aarch64] needs-llvm-components: aarch64 +#![feature( + no_core, lang_items, link_llvm_intrinsics, + abi_unadjusted, repr_simd, arm_target_feature, +)] +#![no_std] +#![no_core] +#![crate_type = "lib"] +#![allow(non_camel_case_types)] + +/// To work cross-target this test must be no_core. +/// This little prelude supplies what we need. +#[lang = "sized"] +pub trait Sized {} + +#[lang = "copy"] +pub trait Copy: Sized {} +impl Copy for i8 {} +impl Copy for *const T {} +impl Copy for *mut T {} + + +// Regression test for https://github.com/rust-lang/rust/issues/118124. + +#[repr(simd)] +pub struct int8x16_t( + pub(crate) i8, pub(crate) i8, pub(crate) i8, pub(crate) i8, + pub(crate) i8, pub(crate) i8, pub(crate) i8, pub(crate) i8, + pub(crate) i8, pub(crate) i8, pub(crate) i8, pub(crate) i8, + pub(crate) i8, pub(crate) i8, pub(crate) i8, pub(crate) i8, +); +impl Copy for int8x16_t {} + +#[repr(C)] +pub struct int8x16x4_t(pub int8x16_t, pub int8x16_t, pub int8x16_t, pub int8x16_t); +impl Copy for int8x16x4_t {} + +#[target_feature(enable = "neon")] +#[cfg_attr(target_arch = "arm", target_feature(enable = "v7"))] +pub unsafe fn vld1q_s8_x4(a: *const i8) -> int8x16x4_t { + #[allow(improper_ctypes)] + extern "unadjusted" { + #[cfg_attr(target_arch = "arm", link_name = "llvm.arm.neon.vld1x4.v16i8.p0i8")] + #[cfg_attr(target_arch = "aarch64", link_name = "llvm.aarch64.neon.ld1x4.v16i8.p0i8")] + fn vld1q_s8_x4_(a: *const i8) -> int8x16x4_t; + } + vld1q_s8_x4_(a) +}