From 487853e94d3f8afa12503cdf1d894dcf375e56a6 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 1 Nov 2023 23:01:53 +0100 Subject: [PATCH] default Aggregate ABI to Indirect, and make sure it's never used for unsized --- compiler/rustc_target/src/abi/call/aarch64.rs | 8 +++++++ compiler/rustc_target/src/abi/call/arm.rs | 8 +++++++ .../rustc_target/src/abi/call/loongarch.rs | 8 +++++++ compiler/rustc_target/src/abi/call/mips.rs | 4 ++++ compiler/rustc_target/src/abi/call/mod.rs | 18 ++++++++++---- compiler/rustc_target/src/abi/call/nvptx64.rs | 9 +++++++ .../rustc_target/src/abi/call/powerpc64.rs | 8 +++++++ compiler/rustc_target/src/abi/call/riscv.rs | 8 +++++++ compiler/rustc_target/src/abi/call/s390x.rs | 4 ++++ compiler/rustc_target/src/abi/call/sparc.rs | 4 ++++ compiler/rustc_target/src/abi/call/wasm.rs | 24 +++++++++++++++---- compiler/rustc_ty_utils/src/abi.rs | 11 +++++---- tests/ui/abi/compatibility.rs | 1 + 13 files changed, 102 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_target/src/abi/call/aarch64.rs b/compiler/rustc_target/src/abi/call/aarch64.rs index b4c7b0f120f91..f99f6a3b72164 100644 --- a/compiler/rustc_target/src/abi/call/aarch64.rs +++ b/compiler/rustc_target/src/abi/call/aarch64.rs @@ -40,6 +40,10 @@ where Ty: TyAbiInterface<'a, C> + Copy, C: HasDataLayout, { + if !ret.layout.is_sized() { + // Not touching this... + return; + } if !ret.layout.is_aggregate() { if kind == AbiKind::DarwinPCS { // On Darwin, when returning an i8/i16, it must be sign-extended to 32 bits, @@ -67,6 +71,10 @@ where Ty: TyAbiInterface<'a, C> + Copy, C: HasDataLayout, { + if !arg.layout.is_sized() { + // Not touching this... + return; + } if !arg.layout.is_aggregate() { if kind == AbiKind::DarwinPCS { // On Darwin, when passing an i8/i16, it must be sign-extended to 32 bits, diff --git a/compiler/rustc_target/src/abi/call/arm.rs b/compiler/rustc_target/src/abi/call/arm.rs index 1923ea58838ba..95f6691d42aeb 100644 --- a/compiler/rustc_target/src/abi/call/arm.rs +++ b/compiler/rustc_target/src/abi/call/arm.rs @@ -30,6 +30,10 @@ where Ty: TyAbiInterface<'a, C> + Copy, C: HasDataLayout, { + if !ret.layout.is_sized() { + // Not touching this... + return; + } if !ret.layout.is_aggregate() { ret.extend_integer_width_to(32); return; @@ -56,6 +60,10 @@ where Ty: TyAbiInterface<'a, C> + Copy, C: HasDataLayout, { + if !arg.layout.is_sized() { + // Not touching this... + return; + } if !arg.layout.is_aggregate() { arg.extend_integer_width_to(32); return; diff --git a/compiler/rustc_target/src/abi/call/loongarch.rs b/compiler/rustc_target/src/abi/call/loongarch.rs index e649d58bbcab6..647b6500c52dd 100644 --- a/compiler/rustc_target/src/abi/call/loongarch.rs +++ b/compiler/rustc_target/src/abi/call/loongarch.rs @@ -152,6 +152,10 @@ fn classify_ret<'a, Ty, C>(cx: &C, arg: &mut ArgAbi<'a, Ty>, xlen: u64, flen: u6 where Ty: TyAbiInterface<'a, C> + Copy, { + if !arg.layout.is_sized() { + // Not touching this... + return false; // I guess? return value of this function is not documented + } if let Some(conv) = should_use_fp_conv(cx, &arg.layout, xlen, flen) { match conv { FloatConv::Float(f) => { @@ -214,6 +218,10 @@ fn classify_arg<'a, Ty, C>( ) where Ty: TyAbiInterface<'a, C> + Copy, { + if !arg.layout.is_sized() { + // Not touching this... + return; + } if !is_vararg { match should_use_fp_conv(cx, &arg.layout, xlen, flen) { Some(FloatConv::Float(f)) if *avail_fprs >= 1 => { diff --git a/compiler/rustc_target/src/abi/call/mips.rs b/compiler/rustc_target/src/abi/call/mips.rs index edcd1bab8b4b0..57ccfe2152bdf 100644 --- a/compiler/rustc_target/src/abi/call/mips.rs +++ b/compiler/rustc_target/src/abi/call/mips.rs @@ -17,6 +17,10 @@ fn classify_arg(cx: &C, arg: &mut ArgAbi<'_, Ty>, offset: &mut Size) where C: HasDataLayout, { + if !arg.layout.is_sized() { + // Not touching this... + return; + } let dl = cx.data_layout(); let size = arg.layout.size; let align = arg.layout.align.max(dl.i32_align).min(dl.i64_align).abi; diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index 5efd171b9dd76..daa6e21061752 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -422,7 +422,7 @@ impl<'a, Ty> TyAndLayout<'a, Ty> { })) } - Abi::ScalarPair(..) | Abi::Aggregate { .. } => { + Abi::ScalarPair(..) | Abi::Aggregate { sized: true } => { // Helper for computing `homogeneous_aggregate`, allowing a custom // starting offset (used below for handling variants). let from_fields_at = @@ -520,6 +520,7 @@ impl<'a, Ty> TyAndLayout<'a, Ty> { Ok(result) } } + Abi::Aggregate { sized: false } => Err(Heterogeneous), } } } @@ -555,8 +556,7 @@ impl<'a, Ty> ArgAbi<'a, Ty> { scalar_attrs(&layout, b, a.size(cx).align_to(b.align(cx).abi)), ), Abi::Vector { .. } => PassMode::Direct(ArgAttributes::new()), - // The `Aggregate` ABI should always be adjusted later. - Abi::Aggregate { .. } => PassMode::Direct(ArgAttributes::new()), + Abi::Aggregate { .. } => Self::indirect_pass_mode(&layout), }; ArgAbi { layout, mode } } @@ -580,10 +580,20 @@ impl<'a, Ty> ArgAbi<'a, Ty> { PassMode::Indirect { attrs, meta_attrs, on_stack: false } } + /// Pass this argument directly instead. Should NOT be used! + /// Only exists because of past ABI mistakes that will take time to fix + /// (see https://github.com/rust-lang/rust/issues/115666). + pub fn make_direct_deprecated(&mut self) { + self.mode = PassMode::Direct(ArgAttributes::new()); + } + pub fn make_indirect(&mut self) { match self.mode { PassMode::Direct(_) | PassMode::Pair(_, _) => {} - PassMode::Indirect { attrs: _, meta_attrs: None, on_stack: false } => return, + PassMode::Indirect { attrs: _, meta_attrs: _, on_stack: false } => { + // already indirect + return; + } _ => panic!("Tried to make {:?} indirect", self.mode), } diff --git a/compiler/rustc_target/src/abi/call/nvptx64.rs b/compiler/rustc_target/src/abi/call/nvptx64.rs index 4abe51cd697e0..5c040ce9c3b10 100644 --- a/compiler/rustc_target/src/abi/call/nvptx64.rs +++ b/compiler/rustc_target/src/abi/call/nvptx64.rs @@ -4,12 +4,18 @@ use crate::abi::{HasDataLayout, TyAbiInterface}; fn classify_ret(ret: &mut ArgAbi<'_, Ty>) { if ret.layout.is_aggregate() && ret.layout.size.bits() > 64 { ret.make_indirect(); + } else { + // FIXME: this is wrong! Need to decide which ABI we really want here. + ret.make_direct_deprecated(); } } fn classify_arg(arg: &mut ArgAbi<'_, Ty>) { if arg.layout.is_aggregate() && arg.layout.size.bits() > 64 { arg.make_indirect(); + } else { + // FIXME: this is wrong! Need to decide which ABI we really want here. + arg.make_direct_deprecated(); } } @@ -30,6 +36,9 @@ where _ => unreachable!("Align is given as power of 2 no larger than 16 bytes"), }; arg.cast_to(Uniform { unit, total: Size::from_bytes(2 * align_bytes) }); + } else { + // FIXME: find a better way to do this. See https://github.com/rust-lang/rust/issues/117271. + arg.make_direct_deprecated(); } } diff --git a/compiler/rustc_target/src/abi/call/powerpc64.rs b/compiler/rustc_target/src/abi/call/powerpc64.rs index 359bb8fc09a87..2d41f77e50e1e 100644 --- a/compiler/rustc_target/src/abi/call/powerpc64.rs +++ b/compiler/rustc_target/src/abi/call/powerpc64.rs @@ -46,6 +46,10 @@ where Ty: TyAbiInterface<'a, C> + Copy, C: HasDataLayout, { + if !ret.layout.is_sized() { + // Not touching this... + return; + } if !ret.layout.is_aggregate() { ret.extend_integer_width_to(64); return; @@ -89,6 +93,10 @@ where Ty: TyAbiInterface<'a, C> + Copy, C: HasDataLayout, { + if !arg.layout.is_sized() { + // Not touching this... + return; + } if !arg.layout.is_aggregate() { arg.extend_integer_width_to(64); return; diff --git a/compiler/rustc_target/src/abi/call/riscv.rs b/compiler/rustc_target/src/abi/call/riscv.rs index 93a2045632a84..cbde234d34cc2 100644 --- a/compiler/rustc_target/src/abi/call/riscv.rs +++ b/compiler/rustc_target/src/abi/call/riscv.rs @@ -158,6 +158,10 @@ fn classify_ret<'a, Ty, C>(cx: &C, arg: &mut ArgAbi<'a, Ty>, xlen: u64, flen: u6 where Ty: TyAbiInterface<'a, C> + Copy, { + if !arg.layout.is_sized() { + // Not touching this... + return false; // I guess? return value of this function is not documented + } if let Some(conv) = should_use_fp_conv(cx, &arg.layout, xlen, flen) { match conv { FloatConv::Float(f) => { @@ -220,6 +224,10 @@ fn classify_arg<'a, Ty, C>( ) where Ty: TyAbiInterface<'a, C> + Copy, { + if !arg.layout.is_sized() { + // Not touching this... + return; + } if !is_vararg { match should_use_fp_conv(cx, &arg.layout, xlen, flen) { Some(FloatConv::Float(f)) if *avail_fprs >= 1 => { diff --git a/compiler/rustc_target/src/abi/call/s390x.rs b/compiler/rustc_target/src/abi/call/s390x.rs index ea23692817f50..1a2191082d5de 100644 --- a/compiler/rustc_target/src/abi/call/s390x.rs +++ b/compiler/rustc_target/src/abi/call/s390x.rs @@ -17,6 +17,10 @@ where Ty: TyAbiInterface<'a, C> + Copy, C: HasDataLayout, { + if !arg.layout.is_sized() { + // Not touching this... + return; + } if !arg.layout.is_aggregate() && arg.layout.size.bits() <= 64 { arg.extend_integer_width_to(64); return; diff --git a/compiler/rustc_target/src/abi/call/sparc.rs b/compiler/rustc_target/src/abi/call/sparc.rs index edcd1bab8b4b0..57ccfe2152bdf 100644 --- a/compiler/rustc_target/src/abi/call/sparc.rs +++ b/compiler/rustc_target/src/abi/call/sparc.rs @@ -17,6 +17,10 @@ fn classify_arg(cx: &C, arg: &mut ArgAbi<'_, Ty>, offset: &mut Size) where C: HasDataLayout, { + if !arg.layout.is_sized() { + // Not touching this... + return; + } let dl = cx.data_layout(); let size = arg.layout.size; let align = arg.layout.align.max(dl.i32_align).min(dl.i64_align).abi; diff --git a/compiler/rustc_target/src/abi/call/wasm.rs b/compiler/rustc_target/src/abi/call/wasm.rs index 796b752ff9d41..a7a2b314a94f0 100644 --- a/compiler/rustc_target/src/abi/call/wasm.rs +++ b/compiler/rustc_target/src/abi/call/wasm.rs @@ -34,6 +34,10 @@ where Ty: TyAbiInterface<'a, C> + Copy, C: HasDataLayout, { + if !arg.layout.is_sized() { + // Not touching this... + return; + } arg.extend_integer_width_to(32); if arg.layout.is_aggregate() && !unwrap_trivial_aggregate(cx, arg) { arg.make_indirect_byval(None); @@ -67,21 +71,33 @@ where /// Also see . pub fn compute_wasm_abi_info(fn_abi: &mut FnAbi<'_, Ty>) { if !fn_abi.ret.is_ignore() { - classify_ret(&mut fn_abi.ret); + classify_ret_wasm_abi(&mut fn_abi.ret); } for arg in fn_abi.args.iter_mut() { if arg.is_ignore() { continue; } - classify_arg(arg); + classify_arg_wasm_abi(arg); } - fn classify_ret(ret: &mut ArgAbi<'_, Ty>) { + fn classify_ret_wasm_abi(ret: &mut ArgAbi<'_, Ty>) { + if !ret.layout.is_sized() { + // Not touching this... + return; + } + // FIXME: this is bad! https://github.com/rust-lang/rust/issues/115666 + ret.make_direct_deprecated(); ret.extend_integer_width_to(32); } - fn classify_arg(arg: &mut ArgAbi<'_, Ty>) { + fn classify_arg_wasm_abi(arg: &mut ArgAbi<'_, Ty>) { + if !arg.layout.is_sized() { + // Not touching this... + return; + } + // FIXME: this is bad! https://github.com/rust-lang/rust/issues/115666 + arg.make_direct_deprecated(); arg.extend_integer_width_to(32); } } diff --git a/compiler/rustc_ty_utils/src/abi.rs b/compiler/rustc_ty_utils/src/abi.rs index 7c044efdc0ad7..0db9c5a24a840 100644 --- a/compiler/rustc_ty_utils/src/abi.rs +++ b/compiler/rustc_ty_utils/src/abi.rs @@ -591,13 +591,14 @@ fn fn_abi_adjust_for_abi<'tcx>( _ => return, } - // `Aggregate` ABI must be adjusted to ensure that ABI-compatible Rust types are passed - // the same way. + // Compute `Aggregate` ABI. + + let is_indirect_not_on_stack = + matches!(arg.mode, PassMode::Indirect { on_stack: false, .. }); + assert!(is_indirect_not_on_stack, "{:?}", arg); let size = arg.layout.size; - if arg.layout.is_unsized() || size > Pointer(AddressSpace::DATA).size(cx) { - arg.make_indirect(); - } else { + if !arg.layout.is_unsized() && size <= Pointer(AddressSpace::DATA).size(cx) { // We want to pass small aggregates as immediates, but using // an LLVM aggregate type for this leads to bad optimizations, // so we pick an appropriately sized integer type instead. diff --git a/tests/ui/abi/compatibility.rs b/tests/ui/abi/compatibility.rs index 1e2e1dac9ee06..bf3cb6933d9cd 100644 --- a/tests/ui/abi/compatibility.rs +++ b/tests/ui/abi/compatibility.rs @@ -38,6 +38,7 @@ //[wasi] compile-flags: --target wasm32-wasi //[wasi] needs-llvm-components: webassembly // FIXME: disabled on nvptx64 since the target ABI fails the sanity check +// see https://github.com/rust-lang/rust/issues/117480 /* revisions: nvptx64 [nvptx64] compile-flags: --target nvptx64-nvidia-cuda [nvptx64] needs-llvm-components: nvptx