Fix passing/returning structs with the 64-bit SPARC ABI

Author: beetrees

compiler/rustc_abi/src/callconv/reg.rs

@@ -35,6 +35,7 @@ impl Reg {
 
     reg_ctor!(f32, Float, 32);
     reg_ctor!(f64, Float, 64);
+    reg_ctor!(f128, Float, 128);
 }
 
 impl Reg {

compiler/rustc_target/src/callconv/sparc64.rs

@@ -1,210 +1,185 @@
-// FIXME: This needs an audit for correctness and completeness.
-
 use rustc_abi::{
-    BackendRepr, FieldsShape, Float, HasDataLayout, Primitive, Reg, Scalar, Size, TyAbiInterface,
-    TyAndLayout,
+    Align, BackendRepr, FieldsShape, Float, HasDataLayout, Primitive, Reg, Size, TyAbiInterface,
+    TyAndLayout, Variants,
 };
 
 use crate::callconv::{ArgAbi, ArgAttribute, CastTarget, FnAbi, Uniform};
 use crate::spec::HasTargetSpec;
 
-#[derive(Clone, Debug)]
-struct Sdata {
-    pub prefix: [Option<Reg>; 8],
-    pub prefix_index: usize,
-    pub last_offset: Size,
-    pub has_float: bool,
-    pub arg_attribute: ArgAttribute,
+#[derive(Copy, Clone)]
+enum DoubleWord {
+    F64,
+    F128Start,
+    F128End,
+    Words([Word; 2]),
 }
 
-fn arg_scalar<C>(cx: &C, scalar: &Scalar, offset: Size, mut data: Sdata) -> Sdata
-where
-    C: HasDataLayout,
-{
-    let dl = cx.data_layout();
-
-    if !matches!(scalar.primitive(), Primitive::Float(Float::F32 | Float::F64)) {
-        return data;
-    }
-
-    data.has_float = true;
-
-    if !data.last_offset.is_aligned(dl.f64_align.abi) && data.last_offset < offset {
-        if data.prefix_index == data.prefix.len() {
-            return data;
-        }
-        data.prefix[data.prefix_index] = Some(Reg::i32());
-        data.prefix_index += 1;
-        data.last_offset = data.last_offset + Reg::i32().size;
-    }
-
-    for _ in 0..((offset - data.last_offset).bits() / 64)
-        .min((data.prefix.len() - data.prefix_index) as u64)
-    {
-        data.prefix[data.prefix_index] = Some(Reg::i64());
-        data.prefix_index += 1;
-        data.last_offset = data.last_offset + Reg::i64().size;
-    }
-
-    if data.last_offset < offset {
-        if data.prefix_index == data.prefix.len() {
-            return data;
-        }
-        data.prefix[data.prefix_index] = Some(Reg::i32());
-        data.prefix_index += 1;
-        data.last_offset = data.last_offset + Reg::i32().size;
-    }
-
-    if data.prefix_index == data.prefix.len() {
-        return data;
-    }
-
-    if scalar.primitive() == Primitive::Float(Float::F32) {
-        data.arg_attribute = ArgAttribute::InReg;
-        data.prefix[data.prefix_index] = Some(Reg::f32());
-        data.last_offset = offset + Reg::f32().size;
-    } else {
-        data.prefix[data.prefix_index] = Some(Reg::f64());
-        data.last_offset = offset + Reg::f64().size;
-    }
-    data.prefix_index += 1;
-    data
+#[derive(Copy, Clone)]
+enum Word {
+    F32,
+    Integer,
 }
 
-fn arg_scalar_pair<C>(
+fn classify<'a, Ty, C>(
     cx: &C,
-    scalar1: &Scalar,
-    scalar2: &Scalar,
-    mut offset: Size,
-    mut data: Sdata,
-) -> Sdata
-where
-    C: HasDataLayout,
-{
-    data = arg_scalar(cx, scalar1, offset, data);
-    match (scalar1.primitive(), scalar2.primitive()) {
-        (Primitive::Float(Float::F32), _) => offset += Reg::f32().size,
-        (_, Primitive::Float(Float::F64)) => offset += Reg::f64().size,
-        (Primitive::Int(i, _signed), _) => offset += i.size(),
-        (Primitive::Pointer(_), _) => offset += Reg::i64().size,
-        _ => {}
-    }
-
-    if !offset.bytes().is_multiple_of(4)
-        && matches!(scalar2.primitive(), Primitive::Float(Float::F32 | Float::F64))
-    {
-        offset += Size::from_bytes(4 - (offset.bytes() % 4));
-    }
-    data = arg_scalar(cx, scalar2, offset, data);
-    data
-}
-
-fn parse_structure<'a, Ty, C>(
-    cx: &C,
-    layout: TyAndLayout<'a, Ty>,
-    mut data: Sdata,
-    mut offset: Size,
-) -> Sdata
-where
+    arg_layout: &TyAndLayout<'a, Ty>,
+    offset: Size,
+    double_words: &mut [DoubleWord; 4],
+) where
     Ty: TyAbiInterface<'a, C> + Copy,
     C: HasDataLayout,
 {
-    if let FieldsShape::Union(_) = layout.fields {
-        return data;
-    }
-
-    match layout.backend_repr {
-        BackendRepr::Scalar(scalar) => {
-            data = arg_scalar(cx, &scalar, offset, data);
-        }
-        BackendRepr::Memory { .. } => {
-            for i in 0..layout.fields.count() {
-                if offset < layout.fields.offset(i) {
-                    offset = layout.fields.offset(i);
+    match arg_layout.backend_repr {
+        BackendRepr::Scalar(scalar) => match scalar.primitive() {
+            Primitive::Float(float)
+                if offset.is_aligned(float.align(cx).abi.min(Align::from_bytes(8).unwrap())) =>
+            {
+                let index = offset.bytes_usize() / 8;
+                match float {
+                    Float::F128 => {
+                        double_words[index] = DoubleWord::F128Start;
+                        double_words[index + 1] = DoubleWord::F128End;
+                    }
+                    Float::F64 => {
+                        double_words[index] = DoubleWord::F64;
+                    }
+                    Float::F32 => {
+                        if let DoubleWord::Words(words) = &mut double_words[index] {
+                            words[(offset.bytes_usize() % 8) / 4] = Word::F32;
+                        } else {
+                            unreachable!();
+                        }
+                    }
+                    Float::F16 => {
+                        // Match LLVM by passing `f16` in integer registers.
+                    }
                 }
-                data = parse_structure(cx, layout.field(cx, i), data.clone(), offset);
             }
-        }
-        _ => {
-            if let BackendRepr::ScalarPair(scalar1, scalar2) = &layout.backend_repr {
-                data = arg_scalar_pair(cx, scalar1, scalar2, offset, data);
+            _ => {}
+        },
+        BackendRepr::SimdVector { .. } => {}
+        BackendRepr::ScalarPair(..) | BackendRepr::Memory { .. } => match arg_layout.fields {
+            FieldsShape::Primitive => {
+                unreachable!("aggregates can't have `FieldsShape::Primitive`")
             }
-        }
+            FieldsShape::Union(_) => {
+                if !arg_layout.is_zst() {
+                    if arg_layout.is_transparent() {
+                        let non_1zst_elem = arg_layout.non_1zst_field(cx).expect("not exactly one non-1-ZST field in non-ZST repr(transparent) union").1;
+                        classify(cx, &non_1zst_elem, offset, double_words);
+                    }
+                }
+            }
+            FieldsShape::Array { .. } => {}
+            FieldsShape::Arbitrary { .. } => match arg_layout.variants {
+                Variants::Multiple { .. } => {}
+                Variants::Single { .. } | Variants::Empty => {
+                    // Match Clang by ignoring whether a struct is packed and just considering
+                    // whether individual fields are aligned. GCC currently uses only integer
+                    // registers when passing packed structs.
+                    for i in arg_layout.fields.index_by_increasing_offset() {
+                        classify(
+                            cx,
+                            &arg_layout.field(cx, i),
+                            offset + arg_layout.fields.offset(i),
+                            double_words,
+                        );
+                    }
+                }
+            },
+        },
     }
-
-    data
 }
 
-fn classify_arg<'a, Ty, C>(cx: &C, arg: &mut ArgAbi<'a, Ty>, in_registers_max: Size)
-where
+fn classify_arg<'a, Ty, C>(
+    cx: &C,
+    arg: &mut ArgAbi<'a, Ty>,
+    in_registers_max: Size,
+    total_double_word_count: &mut usize,
+) where
     Ty: TyAbiInterface<'a, C> + Copy,
     C: HasDataLayout,
 {
+    // 64-bit SPARC allocates argument stack space in 64-bit chunks (double words), some of which
+    // are promoted to registers based on their position on the stack.
+
+    // Keep track of the total number of double words used by arguments so far. This allows padding
+    // arguments to be inserted where necessary to ensure that 16-aligned arguments are passed in an
+    // aligned set of registers.
+
+    let pad = !total_double_word_count.is_multiple_of(2) && arg.layout.align.abi.bytes() == 16;
+    // The number of double words used by this argument.
+    let double_word_count = arg.layout.size.bytes_usize().div_ceil(8);
+    // The number of double words before this argument, including any padding.
+    let start_double_word_count = *total_double_word_count + usize::from(pad);
     if !arg.layout.is_aggregate() {
         arg.extend_integer_width_to(64);
+        *total_double_word_count = start_double_word_count + double_word_count;
         return;
     }
 
     let total = arg.layout.size;
     if total > in_registers_max {
         arg.make_indirect();
+        *total_double_word_count += 1;
         return;
     }
 
-    match arg.layout.fields {
-        FieldsShape::Primitive => unreachable!(),
-        FieldsShape::Array { .. } => {
-            // Arrays are passed indirectly
-            arg.make_indirect();
-            return;
-        }
-        FieldsShape::Union(_) => {
-            // Unions and are always treated as a series of 64-bit integer chunks
-        }
-        FieldsShape::Arbitrary { .. } => {
-            // Structures with floating point numbers need special care.
+    *total_double_word_count = start_double_word_count + double_word_count;
 
-            let mut data = parse_structure(
-                cx,
-                arg.layout,
-                Sdata {
-                    prefix: [None; 8],
-                    prefix_index: 0,
-                    last_offset: Size::ZERO,
-                    has_float: false,
-                    arg_attribute: ArgAttribute::default(),
-                },
-                Size::ZERO,
-            );
+    let mut double_words = [DoubleWord::Words([Word::Integer; 2]); 4];
+    classify(cx, &arg.layout, Size::ZERO, &mut double_words);
 
-            if data.has_float {
-                // Structure { float, int, int } doesn't like to be handled like
-                // { float, long int }. Other way around it doesn't mind.
-                if data.last_offset < arg.layout.size
-                    && !data.last_offset.bytes().is_multiple_of(8)
-                    && data.prefix_index < data.prefix.len()
-                {
-                    data.prefix[data.prefix_index] = Some(Reg::i32());
-                    data.prefix_index += 1;
-                    data.last_offset += Reg::i32().size;
-                }
+    let mut regs = [None; 8];
+    let mut i = 0;
+    let mut push = |reg| {
+        regs[i] = Some(reg);
+        i += 1;
+    };
+    let mut attrs = ArgAttribute::empty();
 
-                let mut rest_size = arg.layout.size - data.last_offset;
-                if !rest_size.bytes().is_multiple_of(8) && data.prefix_index < data.prefix.len() {
-                    data.prefix[data.prefix_index] = Some(Reg::i32());
-                    rest_size = rest_size - Reg::i32().size;
+    for (index, double_word) in double_words.into_iter().enumerate() {
+        if arg.layout.size.bytes_usize() <= index * 8 {
+            break;
+        }
+        match double_word {
+            // `f128` must be aligned to be assigned a float register.
+            DoubleWord::F128Start if (start_double_word_count + index).is_multiple_of(2) => {
+                push(Reg::f128());
+            }
+            DoubleWord::F128Start => {
+                // Clang currently handles this case nonsensically, always returning a packed
+                // `struct { long double x; }` in an aligned quad floating-point register even when
+                // the `long double` isn't aligned on the stack, which also makes all future
+                // arguments get passed in the wrong registers. This passes the `f128` in integer
+                // registers when it is unaligned, same as with `f32` and `f64`.
+                push(Reg::i64());
+                push(Reg::i64());
+            }
+            DoubleWord::F128End => {} // Already handled by `F128Start`
+            DoubleWord::F64 => push(Reg::f64()),
+            DoubleWord::Words([Word::Integer, Word::Integer]) => push(Reg::i64()),
+            DoubleWord::Words(words) => {
+                attrs |= ArgAttribute::InReg;
+                for word in words {
+                    match word {
+                        Word::F32 => push(Reg::f32()),
+                        Word::Integer => push(Reg::i32()),
+                    }
                 }
-
-                arg.cast_to(
-                    CastTarget::prefixed(data.prefix, Uniform::new(Reg::i64(), rest_size))
-                        .with_attrs(data.arg_attribute.into()),
-                );
-                return;
             }
         }
     }
 
-    arg.cast_to(Uniform::new(Reg::i64(), total));
+    if let [Some(reg), None, ..] = regs {
+        arg.cast_to_and_pad_i32(CastTarget::from(reg).with_attrs(attrs.into()), pad);
+    } else {
+        arg.cast_to_and_pad_i32(
+            CastTarget::prefixed(regs, Uniform::new(Reg::i8(), Size::ZERO))
+                .with_attrs(attrs.into()),
+            pad,
+        );
+    }
 }
 
 pub(crate) fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>)
@@ -213,9 +188,10 @@ where
     C: HasDataLayout + HasTargetSpec,
 {
     if !fn_abi.ret.is_ignore() {
-        classify_arg(cx, &mut fn_abi.ret, Size::from_bytes(32));
+        classify_arg(cx, &mut fn_abi.ret, Size::from_bytes(32), &mut 0);
     }
 
+    let mut double_word_count = 0;
     for arg in fn_abi.args.iter_mut() {
         if arg.is_ignore() {
             // sparc64-unknown-linux-{gnu,musl,uclibc} doesn't ignore ZSTs.
@@ -224,9 +200,10 @@ where
                 && arg.layout.is_zst()
             {
                 arg.make_indirect_from_ignore();
+                double_word_count += 1;
             }
-            return;
+            continue;
         }
-        classify_arg(cx, arg, Size::from_bytes(16));
+        classify_arg(cx, arg, Size::from_bytes(16), &mut double_word_count);
     }
 }