diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs index 6e3a4cae2f62b..51b2194978806 100644 --- a/compiler/rustc_codegen_llvm/src/abi.rs +++ b/compiler/rustc_codegen_llvm/src/abi.rs @@ -348,50 +348,18 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { PassMode::Direct(_) => { // ABI-compatible Rust types have the same `layout.abi` (up to validity ranges), // and for Scalar ABIs the LLVM type is fully determined by `layout.abi`, - // guarnateeing that we generate ABI-compatible LLVM IR. Things get tricky for - // aggregates... - if matches!(arg.layout.abi, abi::Abi::Aggregate { .. }) { - assert!( - arg.layout.is_sized(), - "`PassMode::Direct` for unsized type: {}", - arg.layout.ty - ); - // This really shouldn't happen, since `immediate_llvm_type` will use - // `layout.fields` to turn this Rust type into an LLVM type. This means all - // sorts of Rust type details leak into the ABI. However wasm sadly *does* - // currently use this mode so we have to allow it -- but we absolutely - // shouldn't let any more targets do that. - // (Also see .) - // - // The unstable abi `PtxKernel` also uses Direct for now. - // It needs to switch to something else before stabilization can happen. - // (See issue: https://github.com/rust-lang/rust/issues/117271) - assert!( - matches!(&*cx.tcx.sess.target.arch, "wasm32" | "wasm64") - || self.conv == Conv::PtxKernel, - "`PassMode::Direct` for aggregates only allowed on wasm and `extern \"ptx-kernel\"` fns\nProblematic type: {:#?}", - arg.layout, - ); - } + // guarnateeing that we generate ABI-compatible LLVM IR. arg.layout.immediate_llvm_type(cx) } PassMode::Pair(..) => { // ABI-compatible Rust types have the same `layout.abi` (up to validity ranges), // so for ScalarPair we can easily be sure that we are generating ABI-compatible // LLVM IR. - assert!( - matches!(arg.layout.abi, abi::Abi::ScalarPair(..)), - "PassMode::Pair for type {}", - arg.layout.ty - ); llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 0, true)); llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 1, true)); continue; } - PassMode::Indirect { attrs: _, meta_attrs: Some(_), on_stack } => { - // `Indirect` with metadata is only for unsized types, and doesn't work with - // on-stack passing. - assert!(arg.layout.is_unsized() && !on_stack); + PassMode::Indirect { attrs: _, meta_attrs: Some(_), on_stack: _ } => { // Construct the type of a (wide) pointer to `ty`, and pass its two fields. // Any two ABI-compatible unsized types have the same metadata type and // moreover the same metadata value leads to the same dynamic size and @@ -402,13 +370,8 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { llargument_tys.push(ptr_layout.scalar_pair_element_llvm_type(cx, 1, true)); continue; } - PassMode::Indirect { attrs: _, meta_attrs: None, on_stack: _ } => { - assert!(arg.layout.is_sized()); - cx.type_ptr() - } + PassMode::Indirect { attrs: _, meta_attrs: None, on_stack: _ } => cx.type_ptr(), PassMode::Cast { cast, pad_i32 } => { - // `Cast` means "transmute to `CastType`"; that only makes sense for sized types. - assert!(arg.layout.is_sized()); // add padding if *pad_i32 { llargument_tys.push(Reg::i32().llvm_type(cx)); diff --git a/compiler/rustc_ty_utils/src/abi.rs b/compiler/rustc_ty_utils/src/abi.rs index fcf6626bbf05e..40a9497b73371 100644 --- a/compiler/rustc_ty_utils/src/abi.rs +++ b/compiler/rustc_ty_utils/src/abi.rs @@ -327,6 +327,74 @@ 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_arg_sanity_check<'tcx>( + cx: &LayoutCx<'tcx, TyCtxt<'tcx>>, + fn_abi: &FnAbi<'tcx, Ty<'tcx>>, + arg: &ArgAbi<'tcx, Ty<'tcx>>, + ) { + match &arg.mode { + PassMode::Ignore => {} + PassMode::Direct(_) => { + // Here the Rust type is used to determine the actual ABI, so we have to be very + // careful. Scalar/ScalarPair is fine, since backends will generally use + // `layout.abi` and ignore everything else. We should just reject `Aggregate` + // entirely here, but some targets need to be fixed first. + if matches!(arg.layout.abi, Abi::Aggregate { .. }) { + assert!( + arg.layout.is_sized(), + "`PassMode::Direct` for unsized type in ABI: {:#?}", + fn_abi + ); + // This really shouldn't happen, since `immediate_llvm_type` will use + // `layout.fields` to turn this Rust type into an LLVM type. This means all + // sorts of Rust type details leak into the ABI. However wasm sadly *does* + // currently use this mode so we have to allow it -- but we absolutely + // shouldn't let any more targets do that. + // (Also see .) + // + // The unstable abi `PtxKernel` also uses Direct for now. + // It needs to switch to something else before stabilization can happen. + // (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: {:#?}", + arg.layout, + ); + } + } + PassMode::Pair(_, _) => { + // Similar to `Direct`, we need to make sure that backends use `layout.abi` and + // ignore the rest of the layout. + assert!( + matches!(arg.layout.abi, Abi::ScalarPair(..)), + "PassMode::Pair for type {}", + arg.layout.ty + ); + } + PassMode::Cast { .. } => { + // `Cast` means "transmute to `CastType`"; that only makes sense for sized types. + assert!(arg.layout.is_sized()); + } + PassMode::Indirect { meta_attrs: None, .. } => { + // No metadata, must be sized. + assert!(arg.layout.is_sized()); + } + PassMode::Indirect { meta_attrs: Some(_), on_stack, .. } => { + // With metadata. Must be unsized and not on the stack. + assert!(arg.layout.is_unsized() && !on_stack); + } + } + } + + for arg in fn_abi.args.iter() { + fn_arg_sanity_check(cx, fn_abi, arg); + } + fn_arg_sanity_check(cx, fn_abi, &fn_abi.ret); +} + // FIXME(eddyb) perhaps group the signature/type-containing (or all of them?) // arguments of this method, into a separate `struct`. #[tracing::instrument(level = "debug", skip(cx, caller_location, fn_def_id, force_thin_self_ptr))] @@ -453,6 +521,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); Ok(cx.tcx.arena.alloc(fn_abi)) } diff --git a/tests/ui/abi/compatibility.rs b/tests/ui/abi/compatibility.rs index 987952a0bcd36..008a28a1c3732 100644 --- a/tests/ui/abi/compatibility.rs +++ b/tests/ui/abi/compatibility.rs @@ -37,9 +37,11 @@ // revisions: wasi //[wasi] compile-flags: --target wasm32-wasi //[wasi] needs-llvm-components: webassembly -// revisions: nvptx64 -//[nvptx64] compile-flags: --target nvptx64-nvidia-cuda -//[nvptx64] needs-llvm-components: nvptx +// FIXME: disabled on nvptx64 since the target ABI fails the sanity check +/* revisions: nvptx64 + [nvptx64] compile-flags: --target nvptx64-nvidia-cuda + [nvptx64] needs-llvm-components: nvptx +*/ #![feature(rustc_attrs, unsized_fn_params, transparent_unions)] #![cfg_attr(not(host), feature(no_core, lang_items), no_std, no_core)] #![allow(unused, improper_ctypes_definitions, internal_features)]