From 128ceec92d04a9b4feaf55804f5e7d1f3f1dbfd2 Mon Sep 17 00:00:00 2001 From: Petr Sumbera Date: Wed, 1 Dec 2021 10:03:45 +0100 Subject: [PATCH] fix sparc64 ABI for aggregates with floating point members --- .../src/abi/pass_mode.rs | 2 +- compiler/rustc_codegen_gcc/src/abi.rs | 4 +- compiler/rustc_codegen_llvm/src/abi.rs | 22 ++- compiler/rustc_target/src/abi/call/mips64.rs | 15 ++- compiler/rustc_target/src/abi/call/mod.rs | 39 ++++-- compiler/rustc_target/src/abi/call/sparc64.rs | 126 +++++++++++++----- src/test/assembly/sparc-struct-abi.rs | 64 +++++++++ src/test/codegen/sparc-struct-abi.rs | 58 +++++++- 8 files changed, 272 insertions(+), 58 deletions(-) create mode 100644 src/test/assembly/sparc-struct-abi.rs diff --git a/compiler/rustc_codegen_cranelift/src/abi/pass_mode.rs b/compiler/rustc_codegen_cranelift/src/abi/pass_mode.rs index 2144e7ed67acb..45d4906259312 100644 --- a/compiler/rustc_codegen_cranelift/src/abi/pass_mode.rs +++ b/compiler/rustc_codegen_cranelift/src/abi/pass_mode.rs @@ -71,7 +71,7 @@ fn cast_target_to_abi_params(cast: CastTarget) -> SmallVec<[AbiParam; 2]> { .prefix .iter() .flatten() - .map(|&kind| reg_to_abi_param(Reg { kind, size: cast.prefix_chunk_size })) + .map(|®| reg_to_abi_param(reg)) .chain((0..rest_count).map(|_| reg_to_abi_param(cast.rest.unit))) .collect::>(); diff --git a/compiler/rustc_codegen_gcc/src/abi.rs b/compiler/rustc_codegen_gcc/src/abi.rs index ce428c589a478..f1b9c310e728a 100644 --- a/compiler/rustc_codegen_gcc/src/abi.rs +++ b/compiler/rustc_codegen_gcc/src/abi.rs @@ -48,8 +48,8 @@ impl GccType for CastTarget { let mut args: Vec<_> = self .prefix .iter() - .flat_map(|option_kind| { - option_kind.map(|kind| Reg { kind, size: self.prefix_chunk_size }.gcc_type(cx)) + .flat_map(|option_reg| { + option_reg.map(|reg| reg.gcc_type(cx)) }) .chain((0..rest_count).map(|_| rest_gcc_unit)) .collect(); diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs index bedd3523d899e..4fab743b3ec5f 100644 --- a/compiler/rustc_codegen_llvm/src/abi.rs +++ b/compiler/rustc_codegen_llvm/src/abi.rs @@ -181,9 +181,7 @@ impl LlvmType for CastTarget { let mut args: Vec<_> = self .prefix .iter() - .flat_map(|option_kind| { - option_kind.map(|kind| Reg { kind, size: self.prefix_chunk_size }.llvm_type(cx)) - }) + .flat_map(|option_reg| option_reg.map(|reg| reg.llvm_type(cx))) .chain((0..rest_count).map(|_| rest_ll_unit)) .collect(); @@ -466,6 +464,9 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { ); } } + PassMode::Cast(cast) => { + cast.attrs.apply_attrs_to_llfn(llvm::AttributePlace::ReturnValue, cx, llfn); + } _ => {} } for arg in &self.args { @@ -497,8 +498,8 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { apply(a); apply(b); } - PassMode::Cast(_) => { - apply(&ArgAttributes::new()); + PassMode::Cast(cast) => { + apply(&cast.attrs); } } } @@ -533,6 +534,13 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { ); } } + PassMode::Cast(cast) => { + cast.attrs.apply_attrs_to_callsite( + llvm::AttributePlace::ReturnValue, + &bx.cx, + callsite, + ); + } _ => {} } if let abi::Abi::Scalar(scalar) = self.ret.layout.abi { @@ -577,8 +585,8 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { apply(bx.cx, a); apply(bx.cx, b); } - PassMode::Cast(_) => { - apply(bx.cx, &ArgAttributes::new()); + PassMode::Cast(cast) => { + apply(bx.cx, &cast.attrs); } } } diff --git a/compiler/rustc_target/src/abi/call/mips64.rs b/compiler/rustc_target/src/abi/call/mips64.rs index 2e00ffc7e1473..1ac454be5e9ad 100644 --- a/compiler/rustc_target/src/abi/call/mips64.rs +++ b/compiler/rustc_target/src/abi/call/mips64.rs @@ -1,4 +1,6 @@ -use crate::abi::call::{ArgAbi, ArgExtension, CastTarget, FnAbi, PassMode, Reg, RegKind, Uniform}; +use crate::abi::call::{ + ArgAbi, ArgAttribute, ArgAttributes, ArgExtension, CastTarget, FnAbi, PassMode, Reg, Uniform, +}; use crate::abi::{self, HasDataLayout, Size, TyAbiInterface}; fn extend_integer_width_mips(arg: &mut ArgAbi<'_, Ty>, bits: u64) { @@ -115,7 +117,7 @@ where for _ in 0..((offset - last_offset).bits() / 64) .min((prefix.len() - prefix_index) as u64) { - prefix[prefix_index] = Some(RegKind::Integer); + prefix[prefix_index] = Some(Reg::i64()); prefix_index += 1; } @@ -123,7 +125,7 @@ where break; } - prefix[prefix_index] = Some(RegKind::Float); + prefix[prefix_index] = Some(Reg::f64()); prefix_index += 1; last_offset = offset + Reg::f64().size; } @@ -137,8 +139,13 @@ where let rest_size = size - Size::from_bytes(8) * prefix_index as u64; arg.cast_to(CastTarget { prefix, - prefix_chunk_size: Size::from_bytes(8), rest: Uniform { unit: Reg::i64(), total: rest_size }, + attrs: ArgAttributes { + regular: ArgAttribute::default(), + arg_ext: ArgExtension::None, + pointee_size: Size::ZERO, + pointee_align: None, + }, }); } diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index 4768c9e2db5e1..735b7e76e3862 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -214,9 +214,9 @@ impl Uniform { #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, HashStable_Generic)] pub struct CastTarget { - pub prefix: [Option; 8], - pub prefix_chunk_size: Size, + pub prefix: [Option; 8], pub rest: Uniform, + pub attrs: ArgAttributes, } impl From for CastTarget { @@ -227,29 +227,48 @@ impl From for CastTarget { impl From for CastTarget { fn from(uniform: Uniform) -> CastTarget { - CastTarget { prefix: [None; 8], prefix_chunk_size: Size::ZERO, rest: uniform } + CastTarget { + prefix: [None; 8], + rest: uniform, + attrs: ArgAttributes { + regular: ArgAttribute::default(), + arg_ext: ArgExtension::None, + pointee_size: Size::ZERO, + pointee_align: None, + }, + } } } impl CastTarget { pub fn pair(a: Reg, b: Reg) -> CastTarget { CastTarget { - prefix: [Some(a.kind), None, None, None, None, None, None, None], - prefix_chunk_size: a.size, + prefix: [Some(a), None, None, None, None, None, None, None], rest: Uniform::from(b), + attrs: ArgAttributes { + regular: ArgAttribute::default(), + arg_ext: ArgExtension::None, + pointee_size: Size::ZERO, + pointee_align: None, + }, } } - pub fn size(&self, cx: &C) -> Size { - (self.prefix_chunk_size * self.prefix.iter().filter(|x| x.is_some()).count() as u64) - .align_to(self.rest.align(cx)) - + self.rest.total + pub fn size(&self, _cx: &C) -> Size { + let mut size = self.rest.total; + for i in 0..self.prefix.iter().count() { + match self.prefix[i] { + Some(v) => size += Size { raw: v.size.bytes() }, + None => {} + } + } + return size; } pub fn align(&self, cx: &C) -> Align { self.prefix .iter() - .filter_map(|x| x.map(|kind| Reg { kind, size: self.prefix_chunk_size }.align(cx))) + .filter_map(|x| x.map(|reg| reg.align(cx))) .fold(cx.data_layout().aggregate_align.abi.max(self.rest.align(cx)), |acc, align| { acc.max(align) }) diff --git a/compiler/rustc_target/src/abi/call/sparc64.rs b/compiler/rustc_target/src/abi/call/sparc64.rs index 5d74c94e2c6f6..39d80c4c7e76d 100644 --- a/compiler/rustc_target/src/abi/call/sparc64.rs +++ b/compiler/rustc_target/src/abi/call/sparc64.rs @@ -1,7 +1,9 @@ // FIXME: This needs an audit for correctness and completeness. -use crate::abi::call::{ArgAbi, FnAbi, Reg, RegKind, Uniform}; -use crate::abi::{HasDataLayout, TyAbiInterface}; +use crate::abi::call::{ + ArgAbi, ArgAttribute, ArgAttributes, ArgExtension, CastTarget, FnAbi, Reg, RegKind, Uniform, +}; +use crate::abi::{self, HasDataLayout, Size, TyAbiInterface}; fn is_homogeneous_aggregate<'a, Ty, C>(cx: &C, arg: &mut ArgAbi<'a, Ty>) -> Option where @@ -16,7 +18,7 @@ where let valid_unit = match unit.kind { RegKind::Integer => false, - RegKind::Float => true, + RegKind::Float => false, RegKind::Vector => arg.layout.size.bits() == 128, }; @@ -24,33 +26,7 @@ where }) } -fn classify_ret<'a, Ty, C>(cx: &C, ret: &mut ArgAbi<'a, Ty>) -where - Ty: TyAbiInterface<'a, C> + Copy, - C: HasDataLayout, -{ - if !ret.layout.is_aggregate() { - ret.extend_integer_width_to(64); - return; - } - - if let Some(uniform) = is_homogeneous_aggregate(cx, ret) { - ret.cast_to(uniform); - return; - } - let size = ret.layout.size; - let bits = size.bits(); - if bits <= 256 { - let unit = Reg::i64(); - ret.cast_to(Uniform { unit, total: size }); - return; - } - - // don't return aggregates in registers - ret.make_indirect(); -} - -fn classify_arg<'a, Ty, C>(cx: &C, arg: &mut ArgAbi<'a, Ty>) +fn classify_arg<'a, Ty, C>(cx: &C, arg: &mut ArgAbi<'a, Ty>, in_registers_max: Size) where Ty: TyAbiInterface<'a, C> + Copy, C: HasDataLayout, @@ -60,13 +36,97 @@ where return; } + // This doesn't intentionally handle structures with floats which needs + // special care below. if let Some(uniform) = is_homogeneous_aggregate(cx, arg) { arg.cast_to(uniform); return; } + if let abi::FieldsShape::Arbitrary { .. } = arg.layout.fields { + let dl = cx.data_layout(); + let size = arg.layout.size; + let mut prefix = [None; 8]; + let mut prefix_index = 0; + let mut last_offset = Size::ZERO; + let mut has_float = false; + let mut arg_attribute = ArgAttribute::default(); + + for i in 0..arg.layout.fields.count() { + let field = arg.layout.field(cx, i); + let offset = arg.layout.fields.offset(i); + + if let abi::Abi::Scalar(scalar) = &field.abi { + if scalar.value == abi::F32 || scalar.value == abi::F64 { + has_float = true; + + if !last_offset.is_aligned(dl.f64_align.abi) && last_offset < offset { + if prefix_index == prefix.len() { + break; + } + prefix[prefix_index] = Some(Reg::i32()); + prefix_index += 1; + last_offset = last_offset + Reg::i32().size; + } + + for _ in 0..((offset - last_offset).bits() / 64) + .min((prefix.len() - prefix_index) as u64) + { + prefix[prefix_index] = Some(Reg::i64()); + prefix_index += 1; + last_offset = last_offset + Reg::i64().size; + } + + if last_offset < offset { + if prefix_index == prefix.len() { + break; + } + prefix[prefix_index] = Some(Reg::i32()); + prefix_index += 1; + last_offset = last_offset + Reg::i32().size; + } + + if prefix_index == prefix.len() { + break; + } + + if scalar.value == abi::F32 { + arg_attribute = ArgAttribute::InReg; + prefix[prefix_index] = Some(Reg::f32()); + last_offset = offset + Reg::f32().size; + } else { + prefix[prefix_index] = Some(Reg::f64()); + last_offset = offset + Reg::f64().size; + } + prefix_index += 1; + } + } + } + + if has_float && arg.layout.size <= in_registers_max { + let mut rest_size = size - last_offset; + + if (rest_size.raw % 8) != 0 && prefix_index < prefix.len() { + prefix[prefix_index] = Some(Reg::i32()); + rest_size = rest_size - Reg::i32().size; + } + + arg.cast_to(CastTarget { + prefix, + rest: Uniform { unit: Reg::i64(), total: rest_size }, + attrs: ArgAttributes { + regular: arg_attribute, + arg_ext: ArgExtension::None, + pointee_size: Size::ZERO, + pointee_align: None, + }, + }); + return; + } + } + let total = arg.layout.size; - if total.bits() > 128 { + if total > in_registers_max { arg.make_indirect(); return; } @@ -80,13 +140,13 @@ where C: HasDataLayout, { if !fn_abi.ret.is_ignore() { - classify_ret(cx, &mut fn_abi.ret); + classify_arg(cx, &mut fn_abi.ret, Size { raw: 32 }); } for arg in &mut fn_abi.args { if arg.is_ignore() { continue; } - classify_arg(cx, arg); + classify_arg(cx, arg, Size { raw: 16 }); } } diff --git a/src/test/assembly/sparc-struct-abi.rs b/src/test/assembly/sparc-struct-abi.rs new file mode 100644 index 0000000000000..dd8e6f614df11 --- /dev/null +++ b/src/test/assembly/sparc-struct-abi.rs @@ -0,0 +1,64 @@ +// Test SPARC64 ABI +// - float structure members are passes in floating point registers +// (#86163) + +// assembly-output: emit-asm +// needs-llvm-components: sparc +// compile-flags: --target=sparcv9-sun-solaris -Copt-level=3 +#![crate_type = "lib"] +#![feature(no_core, lang_items)] +#![no_core] + +#[lang = "sized"] +pub trait Sized {} +#[lang = "copy"] +pub trait Copy {} + +#[repr(C)] +pub struct Franta { + a: f32, + b: f32, + c: f32, + d: f32, +} + +// NB: due to delay slots the `ld` following the call is actually executed before the call. +#[no_mangle] +pub unsafe extern "C" fn callee(arg: Franta) { + // CHECK-LABEL: callee: + // CHECK: st %f3, [[PLACE_D:.*]] + // CHECK: st %f2, [[PLACE_C:.*]] + // CHECK: st %f1, [[PLACE_B:.*]] + // CHECK: st %f0, [[PLACE_A:.*]] + // CHECK: call tst_use + // CHECK-NEXT: ld [[PLACE_A]], %f1 + // CHECK: call tst_use + // CHECK-NEXT: ld [[PLACE_B]], %f1 + // CHECK: call tst_use + // CHECK-NEXT: ld [[PLACE_C]], %f1 + // CHECK: call tst_use + // CHECK-NEXT: ld [[PLACE_D]], %f1 + clobber(); + tst_use(arg.a); + tst_use(arg.b); + tst_use(arg.c); + tst_use(arg.d); +} + +extern "C" { + fn opaque_callee(arg: Franta, intarg: i32); + fn tst_use(arg: f32); + fn clobber(); +} + +#[no_mangle] +pub unsafe extern "C" fn caller() { + // CHECK-LABEL: caller: + // CHECK: ld [{{.*}}], %f0 + // CHECK: ld [{{.*}}], %f1 + // CHECK: ld [{{.*}}], %f2 + // CHECK: ld [{{.*}}], %f3 + // CHECK: call opaque_callee + // CHECK: mov 3, %o2 + opaque_callee(Franta { a: 1.0, b: 2.0, c: 3.0, d: 4.0 }, 3); +} diff --git a/src/test/codegen/sparc-struct-abi.rs b/src/test/codegen/sparc-struct-abi.rs index f228d7c550084..b531dba46079d 100644 --- a/src/test/codegen/sparc-struct-abi.rs +++ b/src/test/codegen/sparc-struct-abi.rs @@ -1,5 +1,5 @@ // Checks that we correctly codegen extern "C" functions returning structs. -// See issue #52638. +// See issues #52638 and #86163. // compile-flags: -O --target=sparc64-unknown-linux-gnu --crate-type=rlib // needs-llvm-components: sparc @@ -25,3 +25,59 @@ pub struct Bool { pub extern "C" fn structbool() -> Bool { Bool { b: true } } + + +#[repr(C)] +pub struct BoolFloat { + b: bool, + f: f32, +} + +// CHECK: define inreg { i32, float } @structboolfloat() +// CHECK-NEXT: start: +// CHECK-NEXT: ret { i32, float } { i32 16777216, float 0x40091EB860000000 } +#[no_mangle] +pub extern "C" fn structboolfloat() -> BoolFloat { + BoolFloat { b: true, f: 3.14 } +} + +// CHECK: define void @structboolfloat_input({ i32, float } inreg %0) +// CHECK-NEXT: start: +#[no_mangle] +pub extern "C" fn structboolfloat_input(a: BoolFloat) { } + + +#[repr(C)] +pub struct ShortDouble { + s: i16, + d: f64, +} + +// CHECK: define { i64, double } @structshortdouble() +// CHECK-NEXT: start: +// CHECK-NEXT: ret { i64, double } { i64 34621422135410688, double 3.140000e+00 } +#[no_mangle] +pub extern "C" fn structshortdouble() -> ShortDouble { + ShortDouble { s: 123, d: 3.14 } +} + +// CHECK: define void @structshortdouble_input({ i64, double } %0) +// CHECK-NEXT: start: +#[no_mangle] +pub extern "C" fn structshortdouble_input(a: ShortDouble) { } + + +#[repr(C)] +pub struct FloatLongFloat { + f: f32, + i: i64, + g: f32, +} + +// CHECK: define inreg { float, i32, i64, float, i32 } @structfloatlongfloat() +// CHECK-NEXT: start: +// CHECK-NEXT: ret { float, i32, i64, float, i32 } { float 0x3FB99999A0000000, i32 undef, i64 123, float 0x40091EB860000000, i32 undef } +#[no_mangle] +pub extern "C" fn structfloatlongfloat() -> FloatLongFloat { + FloatLongFloat { f: 0.1, i: 123, g: 3.14 } +}