Skip to content

Commit

Permalink
Rollup merge of #122781 - nikic:ppc-abi-fix, r=cuviper
Browse files Browse the repository at this point in the history
Fix argument ABI for overaligned structs on ppc64le

When passing a 16 (or higher) aligned struct by value on ppc64le, it needs to be passed as an array of `i128` rather than an array of `i64`. This will force the use of an even starting doubleword.

For the case of a 16 byte struct with alignment 16 it is important that `[1 x i128]` is used instead of `i128` -- apparently, the latter will get treated similarly to `[2 x i64]`, not exhibiting the correct ABI. Add a `force_array` flag to `Uniform` to support this.

The relevant clang code can be found here:
https://github.com/llvm/llvm-project/blob/fe2119a7b08b6e468b2a67768904ea85b1bf0a45/clang/lib/CodeGen/Targets/PPC.cpp#L878-L884
https://github.com/llvm/llvm-project/blob/fe2119a7b08b6e468b2a67768904ea85b1bf0a45/clang/lib/CodeGen/Targets/PPC.cpp#L780-L784

I think the corresponding psABI wording is this:

> Fixed size aggregates and unions passed by value are mapped to as
> many doublewords of the parameter save area as the value uses in
> memory. Aggregrates and unions are aligned according to their
> alignment requirements. This may result in doublewords being
> skipped for alignment.

In particular the last sentence. Though I didn't find any wording for Clang's behavior of clamping the alignment to 16.

Fixes #122767.

r? `@cuviper`
  • Loading branch information
matthiaskrgr authored Apr 8, 2024
2 parents 537aab7 + 1b7342b commit ecfc338
Show file tree
Hide file tree
Showing 15 changed files with 156 additions and 41 deletions.
5 changes: 4 additions & 1 deletion compiler/rustc_codegen_llvm/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,10 @@ impl LlvmType for CastTarget {
// Simplify to a single unit or an array if there's no prefix.
// This produces the same layout, but using a simpler type.
if self.prefix.iter().all(|x| x.is_none()) {
if rest_count == 1 {
// We can't do this if is_consecutive is set and the unit would get
// split on the target. Currently, this is only relevant for i128
// registers.
if rest_count == 1 && (!self.rest.is_consecutive || self.rest.unit != Reg::i128()) {
return rest_ll_unit;
}

Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_target/src/abi/call/aarch64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ where
RegKind::Vector => size.bits() == 64 || size.bits() == 128,
};

valid_unit.then_some(Uniform { unit, total: size })
valid_unit.then_some(Uniform::consecutive(unit, size))
})
}

Expand Down Expand Up @@ -60,7 +60,7 @@ where
let size = ret.layout.size;
let bits = size.bits();
if bits <= 128 {
ret.cast_to(Uniform { unit: Reg::i64(), total: size });
ret.cast_to(Uniform::new(Reg::i64(), size));
return;
}
ret.make_indirect();
Expand Down Expand Up @@ -100,9 +100,9 @@ where
};
if size.bits() <= 128 {
if align.bits() == 128 {
arg.cast_to(Uniform { unit: Reg::i128(), total: size });
arg.cast_to(Uniform::new(Reg::i128(), size));
} else {
arg.cast_to(Uniform { unit: Reg::i64(), total: size });
arg.cast_to(Uniform::new(Reg::i64(), size));
}
return;
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_target/src/abi/call/arm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ where
RegKind::Vector => size.bits() == 64 || size.bits() == 128,
};

valid_unit.then_some(Uniform { unit, total: size })
valid_unit.then_some(Uniform::consecutive(unit, size))
})
}

Expand Down Expand Up @@ -49,7 +49,7 @@ where
let size = ret.layout.size;
let bits = size.bits();
if bits <= 32 {
ret.cast_to(Uniform { unit: Reg::i32(), total: size });
ret.cast_to(Uniform::new(Reg::i32(), size));
return;
}
ret.make_indirect();
Expand Down Expand Up @@ -78,7 +78,7 @@ where

let align = arg.layout.align.abi.bytes();
let total = arg.layout.size;
arg.cast_to(Uniform { unit: if align <= 4 { Reg::i32() } else { Reg::i64() }, total });
arg.cast_to(Uniform::consecutive(if align <= 4 { Reg::i32() } else { Reg::i64() }, total));
}

pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>)
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_target/src/abi/call/csky.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ fn classify_ret<Ty>(arg: &mut ArgAbi<'_, Ty>) {
if total.bits() > 64 {
arg.make_indirect();
} else if total.bits() > 32 {
arg.cast_to(Uniform { unit: Reg::i32(), total });
arg.cast_to(Uniform::new(Reg::i32(), total));
} else {
arg.cast_to(Reg::i32());
}
Expand All @@ -38,7 +38,7 @@ fn classify_arg<Ty>(arg: &mut ArgAbi<'_, Ty>) {
if arg.layout.is_aggregate() {
let total = arg.layout.size;
if total.bits() > 32 {
arg.cast_to(Uniform { unit: Reg::i32(), total });
arg.cast_to(Uniform::new(Reg::i32(), total));
} else {
arg.cast_to(Reg::i32());
}
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_target/src/abi/call/loongarch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ where
if total.bits() <= xlen {
arg.cast_to(xlen_reg);
} else {
arg.cast_to(Uniform { unit: xlen_reg, total: Size::from_bits(xlen * 2) });
arg.cast_to(Uniform::new(xlen_reg, Size::from_bits(xlen * 2)));
}
return false;
}
Expand Down Expand Up @@ -278,10 +278,10 @@ fn classify_arg<'a, Ty, C>(
if total.bits() > xlen {
let align_regs = align > xlen;
if is_loongarch_aggregate(arg) {
arg.cast_to(Uniform {
unit: if align_regs { double_xlen_reg } else { xlen_reg },
total: Size::from_bits(xlen * 2),
});
arg.cast_to(Uniform::new(
if align_regs { double_xlen_reg } else { xlen_reg },
Size::from_bits(xlen * 2),
));
}
if align_regs && is_vararg {
*avail_gprs -= *avail_gprs % 2;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_target/src/abi/call/mips.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ where

if arg.layout.is_aggregate() {
let pad_i32 = !offset.is_aligned(align);
arg.cast_to_and_pad_i32(Uniform { unit: Reg::i32(), total: size }, pad_i32);
arg.cast_to_and_pad_i32(Uniform::new(Reg::i32(), size), pad_i32);
} else {
arg.extend_integer_width_to(32);
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_target/src/abi/call/mips64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ where
}

// Cast to a uniform int structure
ret.cast_to(Uniform { unit: Reg::i64(), total: size });
ret.cast_to(Uniform::new(Reg::i64(), size));
} else {
ret.make_indirect();
}
Expand Down Expand Up @@ -139,7 +139,7 @@ where
let rest_size = size - Size::from_bytes(8) * prefix_index as u64;
arg.cast_to(CastTarget {
prefix,
rest: Uniform { unit: Reg::i64(), total: rest_size },
rest: Uniform::new(Reg::i64(), rest_size),
attrs: ArgAttributes {
regular: ArgAttribute::default(),
arg_ext: ArgExtension::None,
Expand Down
19 changes: 18 additions & 1 deletion compiler/rustc_target/src/abi/call/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,18 +255,35 @@ pub struct Uniform {
/// for 64-bit integers with a total size of 20 bytes. When the argument is actually passed,
/// this size will be rounded up to the nearest multiple of `unit.size`.
pub total: Size,

/// Indicate that the argument is consecutive, in the sense that either all values need to be
/// passed in register, or all on the stack. If they are passed on the stack, there should be
/// no additional padding between elements.
pub is_consecutive: bool,
}

impl From<Reg> for Uniform {
fn from(unit: Reg) -> Uniform {
Uniform { unit, total: unit.size }
Uniform { unit, total: unit.size, is_consecutive: false }
}
}

impl Uniform {
pub fn align<C: HasDataLayout>(&self, cx: &C) -> Align {
self.unit.align(cx)
}

/// Pass using one or more values of the given type, without requiring them to be consecutive.
/// That is, some values may be passed in register and some on the stack.
pub fn new(unit: Reg, total: Size) -> Self {
Uniform { unit, total, is_consecutive: false }
}

/// Pass using one or more consecutive values of the given type. Either all values will be
/// passed in registers, or all on the stack.
pub fn consecutive(unit: Reg, total: Size) -> Self {
Uniform { unit, total, is_consecutive: true }
}
}

/// Describes the type used for `PassMode::Cast`.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_target/src/abi/call/nvptx64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ where
16 => Reg::i128(),
_ => unreachable!("Align is given as power of 2 no larger than 16 bytes"),
};
arg.cast_to(Uniform { unit, total: Size::from_bytes(2 * align_bytes) });
arg.cast_to(Uniform::new(unit, Size::from_bytes(2 * align_bytes)));
} else {
// FIXME: find a better way to do this. See https://github.com/rust-lang/rust/issues/117271.
arg.make_direct_deprecated();
Expand Down
24 changes: 13 additions & 11 deletions compiler/rustc_target/src/abi/call/powerpc64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Alignment of 128 bit types is not currently handled, this will
// need to be fixed when PowerPC vector support is added.

use crate::abi::call::{ArgAbi, FnAbi, Reg, RegKind, Uniform};
use crate::abi::call::{Align, ArgAbi, FnAbi, Reg, RegKind, Uniform};
use crate::abi::{Endian, HasDataLayout, TyAbiInterface};
use crate::spec::HasTargetSpec;

Expand Down Expand Up @@ -37,7 +37,7 @@ where
RegKind::Vector => arg.layout.size.bits() == 128,
};

valid_unit.then_some(Uniform { unit, total: arg.layout.size })
valid_unit.then_some(Uniform::consecutive(unit, arg.layout.size))
})
}

Expand Down Expand Up @@ -81,7 +81,7 @@ where
Reg::i64()
};

ret.cast_to(Uniform { unit, total: size });
ret.cast_to(Uniform::new(unit, size));
return;
}

Expand All @@ -108,18 +108,20 @@ where
}

let size = arg.layout.size;
let (unit, total) = if size.bits() <= 64 {
if size.bits() <= 64 {
// Aggregates smaller than a doubleword should appear in
// the least-significant bits of the parameter doubleword.
(Reg { kind: RegKind::Integer, size }, size)
arg.cast_to(Reg { kind: RegKind::Integer, size })
} else {
// Aggregates larger than a doubleword should be padded
// at the tail to fill out a whole number of doublewords.
let reg_i64 = Reg::i64();
(reg_i64, size.align_to(reg_i64.align(cx)))
// Aggregates larger than i64 should be padded at the tail to fill out a whole number
// of i64s or i128s, depending on the aggregate alignment. Always use an array for
// this, even if there is only a single element.
let reg = if arg.layout.align.abi.bytes() > 8 { Reg::i128() } else { Reg::i64() };
arg.cast_to(Uniform::consecutive(
reg,
size.align_to(Align::from_bytes(reg.size.bytes()).unwrap()),
))
};

arg.cast_to(Uniform { unit, total });
}

pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>)
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_target/src/abi/call/riscv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ where
if total.bits() <= xlen {
arg.cast_to(xlen_reg);
} else {
arg.cast_to(Uniform { unit: xlen_reg, total: Size::from_bits(xlen * 2) });
arg.cast_to(Uniform::new(xlen_reg, Size::from_bits(xlen * 2)));
}
return false;
}
Expand Down Expand Up @@ -284,10 +284,10 @@ fn classify_arg<'a, Ty, C>(
if total.bits() > xlen {
let align_regs = align > xlen;
if is_riscv_aggregate(arg) {
arg.cast_to(Uniform {
unit: if align_regs { double_xlen_reg } else { xlen_reg },
total: Size::from_bits(xlen * 2),
});
arg.cast_to(Uniform::new(
if align_regs { double_xlen_reg } else { xlen_reg },
Size::from_bits(xlen * 2),
));
}
if align_regs && is_vararg {
*avail_gprs -= *avail_gprs % 2;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_target/src/abi/call/sparc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ where

if arg.layout.is_aggregate() {
let pad_i32 = !offset.is_aligned(align);
arg.cast_to_and_pad_i32(Uniform { unit: Reg::i32(), total: size }, pad_i32);
arg.cast_to_and_pad_i32(Uniform::new(Reg::i32(), size), pad_i32);
} else {
arg.extend_integer_width_to(32);
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_target/src/abi/call/sparc64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ where

arg.cast_to(CastTarget {
prefix: data.prefix,
rest: Uniform { unit: Reg::i64(), total: rest_size },
rest: Uniform::new(Reg::i64(), rest_size),
attrs: ArgAttributes {
regular: data.arg_attribute,
arg_ext: ArgExtension::None,
Expand All @@ -205,7 +205,7 @@ where
}
}

arg.cast_to(Uniform { unit: Reg::i64(), total });
arg.cast_to(Uniform::new(Reg::i64(), total));
}

pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>)
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_target/src/abi/call/wasm.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::abi::call::{ArgAbi, FnAbi, Uniform};
use crate::abi::call::{ArgAbi, FnAbi};
use crate::abi::{HasDataLayout, TyAbiInterface};

fn unwrap_trivial_aggregate<'a, Ty, C>(cx: &C, val: &mut ArgAbi<'a, Ty>) -> bool
Expand All @@ -10,7 +10,7 @@ where
if let Some(unit) = val.layout.homogeneous_aggregate(cx).ok().and_then(|ha| ha.unit()) {
let size = val.layout.size;
if unit.size == size {
val.cast_to(Uniform { unit, total: size });
val.cast_to(unit);
return true;
}
}
Expand Down
93 changes: 93 additions & 0 deletions tests/codegen/powerpc64le-struct-align-128.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
// Test that structs aligned to 128 bits are passed with the correct ABI on powerpc64le.
// This is similar to aarch64-struct-align-128.rs, but for ppc.

//@ compile-flags: --target powerpc64le-unknown-linux-gnu
//@ needs-llvm-components: powerpc

#![feature(no_core, lang_items)]
#![crate_type = "lib"]
#![no_core]

#[lang="sized"]
trait Sized { }
#[lang="freeze"]
trait Freeze { }
#[lang="copy"]
trait Copy { }

#[repr(C)]
pub struct Align8 {
pub a: u64,
pub b: u64,
}

#[repr(transparent)]
pub struct Transparent8 {
a: Align8
}

#[repr(C)]
pub struct Wrapped8 {
a: Align8,
}

extern "C" {
// CHECK: declare void @test_8([2 x i64], [2 x i64], [2 x i64])
fn test_8(a: Align8, b: Transparent8, c: Wrapped8);
}

#[repr(C)]
#[repr(align(16))]
pub struct Align16 {
pub a: u64,
pub b: u64,
}

#[repr(transparent)]
pub struct Transparent16 {
a: Align16
}

#[repr(C)]
pub struct Wrapped16 {
pub a: Align16,
}

extern "C" {
// It's important that this produces [1 x i128] rather than just i128!
// CHECK: declare void @test_16([1 x i128], [1 x i128], [1 x i128])
fn test_16(a: Align16, b: Transparent16, c: Wrapped16);
}

#[repr(C)]
#[repr(align(32))]
pub struct Align32 {
pub a: u64,
pub b: u64,
pub c: u64,
}

#[repr(transparent)]
pub struct Transparent32 {
a: Align32
}

#[repr(C)]
pub struct Wrapped32 {
pub a: Align32,
}

extern "C" {
// CHECK: declare void @test_32([2 x i128], [2 x i128], [2 x i128])
fn test_32(a: Align32, b: Transparent32, c: Wrapped32);
}

pub unsafe fn main(
a1: Align8, a2: Transparent8, a3: Wrapped8,
b1: Align16, b2: Transparent16, b3: Wrapped16,
c1: Align32, c2: Transparent32, c3: Wrapped32,
) {
test_8(a1, a2, a3);
test_16(b1, b2, b3);
test_32(c1, c2, c3);
}

0 comments on commit ecfc338

Please sign in to comment.