implement vector-containing aggregate alignment for x86 darwin

compiler/rustc_target/src/abi/call/x86.rs

@@ -1,5 +1,5 @@
 use crate::abi::call::{ArgAttribute, FnAbi, PassMode, Reg, RegKind};
-use crate::abi::{Align, HasDataLayout, TyAbiInterface};
+use crate::abi::{Abi, Align, HasDataLayout, TyAbiInterface, TyAndLayout};
 use crate::spec::HasTargetSpec;
 
 #[derive(PartialEq)]
@@ -53,38 +53,58 @@ pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>, flavor: F
         if arg.is_ignore() {
             continue;
         }
-        if !arg.layout.is_aggregate() {
-            arg.extend_integer_width_to(32);
-            continue;
-        }
 
+        if arg.layout.is_aggregate() {
             // We need to compute the alignment of the `byval` argument. The rules can be found in
-        // `X86_32ABIInfo::getTypeStackAlignInBytes` in Clang's `TargetInfo.cpp`. Summarized here,
-        // they are:
+            // `X86_32ABIInfo::getTypeStackAlignInBytes` in Clang's `TargetInfo.cpp`. Summarized
+            // here, they are:
             //
-        // 1. If the natural alignment of the type is less than or equal to 4, the alignment is 4.
+            // 1. If the natural alignment of the type is <= 4, the alignment is 4.
             //
             // 2. Otherwise, on Linux, the alignment of any vector type is the natural alignment.
-        // (This doesn't matter here because we ensure we have an aggregate with the check above.)
+            // This doesn't matter here because we only pass aggregates via `byval`, not vectors.
             //
-        // 3. Otherwise, on Apple platforms, the alignment of anything that contains a vector type
-        // is 16.
+            // 3. Otherwise, on Apple platforms, the alignment of anything that contains a vector
+            // type is 16.
             //
             // 4. If none of these conditions are true, the alignment is 4.
+
+            fn contains_vector<'a, Ty, C>(cx: &C, layout: TyAndLayout<'a, Ty>) -> bool
+            where
+                Ty: TyAbiInterface<'a, C> + Copy,
+            {
+                match layout.abi {
+                    Abi::Uninhabited | Abi::Scalar(_) | Abi::ScalarPair(..) => false,
+                    Abi::Vector { .. } => true,
+                    Abi::Aggregate { .. } => {
+                        for i in 0..layout.fields.count() {
+                            if contains_vector(cx, layout.field(cx, i)) {
+                                return true;
+                            }
+                        }
+                        false
+                    }
+                }
+            }
+
             let t = cx.target_spec();
             let align_4 = Align::from_bytes(4).unwrap();
             let align_16 = Align::from_bytes(16).unwrap();
             let byval_align = if arg.layout.align.abi < align_4 {
+                // (1.)
                 align_4
-        } else if t.is_like_osx && arg.layout.align.abi >= align_16 {
-            // FIXME(pcwalton): This is dubious--we should actually be looking inside the type to
-            // determine if it contains SIMD vector values--but I think it's fine?
+            } else if t.is_like_osx && contains_vector(cx, arg.layout) {
+                // (3.)
                 align_16
             } else {
+                // (4.)
                 align_4
             };
 
             arg.make_indirect_byval(Some(byval_align));
+        } else {
+            arg.extend_integer_width_to(32);
+        }
     }
 
     if flavor == Flavor::FastcallOrVectorcall {

tests/codegen/align-byval-vector.rs

@@ -0,0 +1,58 @@
+// revisions:x86-linux x86-darwin
+
+//[x86-linux] compile-flags: --target i686-unknown-linux-gnu
+//[x86-linux] needs-llvm-components: x86
+//[x86-darwin] compile-flags: --target i686-apple-darwin
+//[x86-darwin] needs-llvm-components: x86
+
+// Tests that aggregates containing vector types get their alignment increased to 16 on Darwin.
+
+#![feature(no_core, lang_items, repr_simd, simd_ffi)]
+#![crate_type = "lib"]
+#![no_std]
+#![no_core]
+#![allow(non_camel_case_types)]
+
+#[lang = "sized"]
+trait Sized {}
+#[lang = "freeze"]
+trait Freeze {}
+#[lang = "copy"]
+trait Copy {}
+
+#[repr(simd)]
+pub struct i32x4(i32, i32, i32, i32);
+
+#[repr(C)]
+pub struct Foo {
+    a: i32x4,
+    b: i8,
+}
+
+// This tests that we recursively check for vector types, not just at the top level.
+#[repr(C)]
+pub struct DoubleFoo {
+    one: Foo,
+    two: Foo,
+}
+
+extern "C" {
+    // x86-linux: declare void @f({{.*}}byval(%Foo) align 4{{.*}})
+    // x86-darwin: declare void @f({{.*}}byval(%Foo) align 16{{.*}})
+    fn f(foo: Foo);
+
+    // x86-linux: declare void @g({{.*}}byval(%DoubleFoo) align 4{{.*}})
+    // x86-darwin: declare void @g({{.*}}byval(%DoubleFoo) align 16{{.*}})
+    fn g(foo: DoubleFoo);
+}
+
+pub fn main() {
+    unsafe { f(Foo { a: i32x4(1, 2, 3, 4), b: 0 }) }
+
+    unsafe {
+        g(DoubleFoo {
+            one: Foo { a: i32x4(1, 2, 3, 4), b: 0 },
+            two: Foo { a: i32x4(1, 2, 3, 4), b: 0 },
+        })
+    }
+}