From bc3a71ed00af01855b0ae8908ae271b83eca34f6 Mon Sep 17 00:00:00 2001 From: bors Date: Sat, 2 Sep 2017 19:46:51 +0000 Subject: [PATCH] Auto merge of #44066 - cuviper:powerpc64-extern-abi, r=alexcrichton powerpc64: improve extern struct ABI These fixes all have to do with the 64-bit PowerPC ELF ABI for big-endian targets. The ELF v2 ABI for powerpc64le already worked well. - Return after marking return aggregates indirect. Fixes #42757. - Pass one-member float aggregates as direct argument values. - Aggregate arguments less than 64-bit must be written in the least- significant bits of the parameter space. - Larger aggregates are instead padded at the tail. (i.e. filling MSBs, padding the remaining LSBs.) New tests were also added for the single-float aggregate, and a 3-byte aggregate to check that it's filled into LSBs. Overall, at least these formerly-failing tests now pass on powerpc64: - run-make/extern-fn-struct-passing-abi - run-make/extern-fn-with-packed-struct - run-pass/extern-pass-TwoU16s.rs - run-pass/extern-pass-TwoU8s.rs - run-pass/struct-return.rs --- src/librustc_trans/cabi_powerpc64.rs | 64 +++++++++++++++++----- src/librustc_trans/cabi_x86.rs | 41 ++++++++++++-- .../run-make/extern-fn-struct-passing-abi/test.c | 32 ++++++++++- .../run-make/extern-fn-struct-passing-abi/test.rs | 27 +++++++++ .../run-make/extern-fn-with-packed-struct/test.c | 5 ++ .../run-make/extern-fn-with-packed-struct/test.rs | 26 +-------- 6 files changed, 151 insertions(+), 44 deletions(-) diff --git a/src/librustc_trans/cabi_powerpc64.rs b/src/librustc_trans/cabi_powerpc64.rs index 5c695387236f..fb5472eb6ae1 100644 --- a/src/librustc_trans/cabi_powerpc64.rs +++ b/src/librustc_trans/cabi_powerpc64.rs @@ -14,14 +14,26 @@ use abi::{FnType, ArgType, LayoutExt, Reg, RegKind, Uniform}; use context::CrateContext; +use rustc::ty::layout; -fn is_homogeneous_aggregate<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, arg: &mut ArgType<'tcx>) +#[derive(Debug, Clone, Copy, PartialEq)] +enum ABI { + ELFv1, // original ABI used for powerpc64 (big-endian) + ELFv2, // newer ABI used for powerpc64le +} +use self::ABI::*; + +fn is_homogeneous_aggregate<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, + arg: &mut ArgType<'tcx>, + abi: ABI) -> Option { arg.layout.homogeneous_aggregate(ccx).and_then(|unit| { let size = arg.layout.size(ccx); - // Ensure we have at most eight uniquely addressable members. - if size > unit.size.checked_mul(8, ccx).unwrap() { + // ELFv1 only passes one-member aggregates transparently. + // ELFv2 passes up to eight uniquely addressable members. + if (abi == ELFv1 && size > unit.size) + || size > unit.size.checked_mul(8, ccx).unwrap() { return None; } @@ -42,21 +54,23 @@ fn is_homogeneous_aggregate<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, arg: &mut Ar }) } -fn classify_ret_ty<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, ret: &mut ArgType<'tcx>) { +fn classify_ret_ty<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, ret: &mut ArgType<'tcx>, abi: ABI) { if !ret.layout.is_aggregate() { ret.extend_integer_width_to(64); return; } - // The PowerPC64 big endian ABI doesn't return aggregates in registers - if ccx.sess().target.target.target_endian == "big" { + // The ELFv1 ABI doesn't return aggregates in registers + if abi == ELFv1 { ret.make_indirect(ccx); + return; } - if let Some(uniform) = is_homogeneous_aggregate(ccx, ret) { + if let Some(uniform) = is_homogeneous_aggregate(ccx, ret, abi) { ret.cast_to(ccx, uniform); return; } + let size = ret.layout.size(ccx); let bits = size.bits(); if bits <= 128 { @@ -80,31 +94,55 @@ fn classify_ret_ty<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, ret: &mut ArgType<'tc ret.make_indirect(ccx); } -fn classify_arg_ty<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, arg: &mut ArgType<'tcx>) { +fn classify_arg_ty<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, arg: &mut ArgType<'tcx>, abi: ABI) { if !arg.layout.is_aggregate() { arg.extend_integer_width_to(64); return; } - if let Some(uniform) = is_homogeneous_aggregate(ccx, arg) { + if let Some(uniform) = is_homogeneous_aggregate(ccx, arg, abi) { arg.cast_to(ccx, uniform); return; } - let total = arg.layout.size(ccx); + let size = arg.layout.size(ccx); + let (unit, total) = match abi { + ELFv1 => { + // In ELFv1, aggregates smaller than a doubleword should appear in + // the least-significant bits of the parameter doubleword. The rest + // should be padded at their tail to fill out multiple doublewords. + if size.bits() <= 64 { + (Reg { kind: RegKind::Integer, size }, size) + } else { + let align = layout::Align::from_bits(64, 64).unwrap(); + (Reg::i64(), size.abi_align(align)) + } + }, + ELFv2 => { + // In ELFv2, we can just cast directly. + (Reg::i64(), size) + }, + }; + arg.cast_to(ccx, Uniform { - unit: Reg::i64(), + unit, total }); } pub fn compute_abi_info<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, fty: &mut FnType<'tcx>) { + let abi = match ccx.sess().target.target.target_endian.as_str() { + "big" => ELFv1, + "little" => ELFv2, + _ => unimplemented!(), + }; + if !fty.ret.is_ignore() { - classify_ret_ty(ccx, &mut fty.ret); + classify_ret_ty(ccx, &mut fty.ret, abi); } for arg in &mut fty.args { if arg.is_ignore() { continue; } - classify_arg_ty(ccx, arg); + classify_arg_ty(ccx, arg, abi); } } diff --git a/src/librustc_trans/cabi_x86.rs b/src/librustc_trans/cabi_x86.rs index 8b024b8c97fa..49634d6e78ce 100644 --- a/src/librustc_trans/cabi_x86.rs +++ b/src/librustc_trans/cabi_x86.rs @@ -11,12 +11,30 @@ use abi::{ArgAttribute, FnType, LayoutExt, Reg, RegKind}; use common::CrateContext; +use rustc::ty::layout::{self, Layout, TyLayout}; + #[derive(PartialEq)] pub enum Flavor { General, Fastcall } +fn is_single_fp_element<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, + layout: TyLayout<'tcx>) -> bool { + match *layout { + Layout::Scalar { value: layout::F32, .. } | + Layout::Scalar { value: layout::F64, .. } => true, + Layout::Univariant { .. } => { + if layout.field_count() == 1 { + is_single_fp_element(ccx, layout.field(ccx, 0)) + } else { + false + } + } + _ => false + } +} + pub fn compute_abi_info<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, fty: &mut FnType<'tcx>, flavor: Flavor) { @@ -33,12 +51,23 @@ pub fn compute_abi_info<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, if t.options.is_like_osx || t.options.is_like_windows || t.options.is_like_openbsd { let size = fty.ret.layout.size(ccx); - match size.bytes() { - 1 => fty.ret.cast_to(ccx, Reg::i8()), - 2 => fty.ret.cast_to(ccx, Reg::i16()), - 4 => fty.ret.cast_to(ccx, Reg::i32()), - 8 => fty.ret.cast_to(ccx, Reg::i64()), - _ => fty.ret.make_indirect(ccx) + + // According to Clang, everyone but MSVC returns single-element + // float aggregates directly in a floating-point register. + if !t.options.is_like_msvc && is_single_fp_element(ccx, fty.ret.layout) { + match size.bytes() { + 4 => fty.ret.cast_to(ccx, Reg::f32()), + 8 => fty.ret.cast_to(ccx, Reg::f64()), + _ => fty.ret.make_indirect(ccx) + } + } else { + match size.bytes() { + 1 => fty.ret.cast_to(ccx, Reg::i8()), + 2 => fty.ret.cast_to(ccx, Reg::i16()), + 4 => fty.ret.cast_to(ccx, Reg::i32()), + 8 => fty.ret.cast_to(ccx, Reg::i64()), + _ => fty.ret.make_indirect(ccx) + } } } else { fty.ret.make_indirect(ccx); diff --git a/src/test/run-make/extern-fn-struct-passing-abi/test.c b/src/test/run-make/extern-fn-struct-passing-abi/test.c index 44a940a17a98..25cd6da10b8f 100644 --- a/src/test/run-make/extern-fn-struct-passing-abi/test.c +++ b/src/test/run-make/extern-fn-struct-passing-abi/test.c @@ -43,6 +43,16 @@ struct FloatPoint { double y; }; +struct FloatOne { + double x; +}; + +struct IntOdd { + int8_t a; + int8_t b; + int8_t c; +}; + // System V x86_64 ABI: // a, b, c, d, e should be in registers // s should be byval pointer @@ -283,7 +293,7 @@ struct Huge huge_struct(struct Huge s) { // p should be in registers // return should be in registers // -// Win64 ABI: +// Win64 ABI and 64-bit PowerPC ELFv1 ABI: // p should be a byval pointer // return should be in a hidden sret pointer struct FloatPoint float_point(struct FloatPoint p) { @@ -292,3 +302,23 @@ struct FloatPoint float_point(struct FloatPoint p) { return p; } + +// 64-bit PowerPC ELFv1 ABI: +// f1 should be in a register +// return should be in a hidden sret pointer +struct FloatOne float_one(struct FloatOne f1) { + assert(f1.x == 7.); + + return f1; +} + +// 64-bit PowerPC ELFv1 ABI: +// i should be in the least-significant bits of a register +// return should be in a hidden sret pointer +struct IntOdd int_odd(struct IntOdd i) { + assert(i.a == 1); + assert(i.b == 2); + assert(i.c == 3); + + return i; +} diff --git a/src/test/run-make/extern-fn-struct-passing-abi/test.rs b/src/test/run-make/extern-fn-struct-passing-abi/test.rs index aaae7ae4fb49..54a4f868eb4e 100644 --- a/src/test/run-make/extern-fn-struct-passing-abi/test.rs +++ b/src/test/run-make/extern-fn-struct-passing-abi/test.rs @@ -53,6 +53,20 @@ struct FloatPoint { y: f64 } +#[derive(Clone, Copy, Debug, PartialEq)] +#[repr(C)] +struct FloatOne { + x: f64, +} + +#[derive(Clone, Copy, Debug, PartialEq)] +#[repr(C)] +struct IntOdd { + a: i8, + b: i8, + c: i8, +} + #[link(name = "test", kind = "static")] extern { fn byval_rect(a: i32, b: i32, c: i32, d: i32, e: i32, s: Rect); @@ -83,6 +97,10 @@ extern { fn huge_struct(s: Huge) -> Huge; fn float_point(p: FloatPoint) -> FloatPoint; + + fn float_one(f: FloatOne) -> FloatOne; + + fn int_odd(i: IntOdd) -> IntOdd; } fn main() { @@ -91,6 +109,8 @@ fn main() { let u = FloatRect { a: 3489, b: 3490, c: 8. }; let v = Huge { a: 5647, b: 5648, c: 5649, d: 5650, e: 5651 }; let p = FloatPoint { x: 5., y: -3. }; + let f1 = FloatOne { x: 7. }; + let i = IntOdd { a: 1, b: 2, c: 3 }; unsafe { byval_rect(1, 2, 3, 4, 5, s); @@ -113,5 +133,12 @@ fn main() { assert_eq!(sret_byval_struct(1, 2, 3, 4, s), t); assert_eq!(sret_split_struct(1, 2, s), t); assert_eq!(float_point(p), p); + assert_eq!(int_odd(i), i); + + // MSVC/GCC/Clang are not consistent in the ABI of single-float aggregates. + // x86_64: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82028 + // i686: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82041 + #[cfg(not(all(windows, target_env = "gnu")))] + assert_eq!(float_one(f1), f1); } } diff --git a/src/test/run-make/extern-fn-with-packed-struct/test.c b/src/test/run-make/extern-fn-with-packed-struct/test.c index 506954fca461..4124e202c1dd 100644 --- a/src/test/run-make/extern-fn-with-packed-struct/test.c +++ b/src/test/run-make/extern-fn-with-packed-struct/test.c @@ -1,6 +1,8 @@ // ignore-license // Pragma needed cause of gcc bug on windows: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991 +#include + #ifdef _MSC_VER #pragma pack(push,1) struct Foo { @@ -18,5 +20,8 @@ struct __attribute__((packed)) Foo { #endif struct Foo foo(struct Foo foo) { + assert(foo.a == 1); + assert(foo.b == 2); + assert(foo.c == 3); return foo; } diff --git a/src/test/run-make/extern-fn-with-packed-struct/test.rs b/src/test/run-make/extern-fn-with-packed-struct/test.rs index 9e81636e3670..d2540ad61542 100644 --- a/src/test/run-make/extern-fn-with-packed-struct/test.rs +++ b/src/test/run-make/extern-fn-with-packed-struct/test.rs @@ -8,36 +8,14 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::fmt; - -#[repr(packed)] -#[derive(Copy, Clone)] +#[repr(C, packed)] +#[derive(Copy, Clone, Debug, PartialEq)] struct Foo { a: i8, b: i16, c: i8 } -impl PartialEq for Foo { - fn eq(&self, other: &Foo) -> bool { - self.a == other.a && self.b == other.b && self.c == other.c - } -} - -impl fmt::Debug for Foo { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let a = self.a; - let b = self.b; - let c = self.c; - - f.debug_struct("Foo") - .field("a", &a) - .field("b", &b) - .field("c", &c) - .finish() - } -} - #[link(name = "test", kind = "static")] extern { fn foo(f: Foo) -> Foo; -- 2.13.5