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

Author: bors

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<Uniform> {
     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);
     }
 }

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);

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;
+}

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);
     }
 }

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 <assert.h>
+
 #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;
 }

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;