Skip to content

Commit

Permalink
Auto merge of rust-lang#130450 - workingjubilee:these-names-are-indir…
Browse files Browse the repository at this point in the history
…ect, r=bjorn3

Reduce confusion about `make_indirect_byval` by renaming it

As part of doing so, remove the incorrect handling of the wasm target's `make_indirect_byval` (i.e. using it at all).
  • Loading branch information
bors committed Sep 17, 2024
2 parents 28e8f01 + 30be609 commit 8d3f80b
Show file tree
Hide file tree
Showing 9 changed files with 118 additions and 37 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_target/src/abi/call/m68k.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ fn classify_arg<Ty>(arg: &mut ArgAbi<'_, Ty>) {
return;
}
if arg.layout.is_aggregate() {
arg.make_indirect_byval(None);
arg.pass_by_stack_offset(None);
} else {
arg.extend_integer_width_to(32);
}
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_target/src/abi/call/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ pub enum PassMode {
/// (which ensures that padding is preserved and that we do not rely on LLVM's struct layout),
/// and will use the alignment specified in `attrs.pointee_align` (if `Some`) or the type's
/// alignment (if `None`). This means that the alignment will not always
/// match the Rust type's alignment; see documentation of `make_indirect_byval` for more info.
/// match the Rust type's alignment; see documentation of `pass_by_stack_offset` for more info.
///
/// `on_stack` cannot be true for unsized arguments, i.e., when `meta_attrs` is `Some`.
Indirect { attrs: ArgAttributes, meta_attrs: Option<ArgAttributes>, on_stack: bool },
Expand Down Expand Up @@ -681,7 +681,7 @@ impl<'a, Ty> ArgAbi<'a, Ty> {
/// either in the caller (if the type's alignment is lower than the byval alignment)
/// or in the callee (if the type's alignment is higher than the byval alignment),
/// to ensure that Rust code never sees an underaligned pointer.
pub fn make_indirect_byval(&mut self, byval_align: Option<Align>) {
pub fn pass_by_stack_offset(&mut self, byval_align: Option<Align>) {
assert!(!self.layout.is_unsized(), "used byval ABI for unsized layout");
self.make_indirect();
match self.mode {
Expand Down Expand Up @@ -879,8 +879,7 @@ impl<'a, Ty> FnAbi<'a, Ty> {
{
if abi == spec::abi::Abi::X86Interrupt {
if let Some(arg) = self.args.first_mut() {
// FIXME(pcwalton): This probably should use the x86 `byval` ABI...
arg.make_indirect_byval(None);
arg.pass_by_stack_offset(None);
}
return Ok(());
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_target/src/abi/call/wasm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ where
}
arg.extend_integer_width_to(32);
if arg.layout.is_aggregate() && !unwrap_trivial_aggregate(cx, arg) {
arg.make_indirect_byval(None);
arg.make_indirect();
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_target/src/abi/call/x86.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ where
align_4
};

arg.make_indirect_byval(Some(byval_align));
arg.pass_by_stack_offset(Some(byval_align));
} else {
arg.extend_integer_width_to(32);
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_target/src/abi/call/x86_64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ where
if is_arg {
// The x86_64 ABI doesn't have any special requirements for `byval` alignment,
// the type's alignment is always used.
arg.make_indirect_byval(None);
arg.pass_by_stack_offset(None);
} else {
// `sret` parameter thus one less integer register available
arg.make_indirect();
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_target/src/abi/call/xtensa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ where
*arg_gprs_left -= needed_arg_gprs;

if must_use_stack {
arg.make_indirect_byval(None);
arg.pass_by_stack_offset(None);
} else if is_xtensa_aggregate(arg) {
// Aggregates which are <= max_size will be passed in
// registers if possible, so coerce to integers.
Expand Down
26 changes: 1 addition & 25 deletions tests/codegen/align-byval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@

//@[m68k] compile-flags: --target m68k-unknown-linux-gnu
//@[m68k] needs-llvm-components: m68k
//@[wasm] compile-flags: --target wasm32-unknown-emscripten
//@[wasm] needs-llvm-components: webassembly
//@[x86_64-linux] compile-flags: --target x86_64-unknown-linux-gnu
//@[x86_64-linux] needs-llvm-components: x86
//@[x86_64-windows] compile-flags: --target x86_64-pc-windows-msvc
Expand All @@ -15,7 +13,7 @@
//@[i686-windows] needs-llvm-components: x86

// Tests that `byval` alignment is properly specified (#80127).
// The only targets that use `byval` are m68k, wasm, x86-64, and x86.
// The only targets that use `byval` are m68k, x86-64, and x86.
// Note also that Windows mandates a by-ref ABI here, so it does not use byval.

#![feature(no_core, lang_items)]
Expand Down Expand Up @@ -112,9 +110,6 @@ pub unsafe fn call_na1(x: NaturalAlign1) {
// m68k: [[ALLOCA:%[a-z0-9+]]] = alloca [2 x i8], align 1
// m68k: call void @natural_align_1({{.*}}byval([2 x i8]) align 1{{.*}} [[ALLOCA]])

// wasm: [[ALLOCA:%[a-z0-9+]]] = alloca [2 x i8], align 1
// wasm: call void @natural_align_1({{.*}}byval([2 x i8]) align 1{{.*}} [[ALLOCA]])

// x86_64-linux: call void @natural_align_1(i16

// x86_64-windows: call void @natural_align_1(i16
Expand All @@ -133,7 +128,6 @@ pub unsafe fn call_na2(x: NaturalAlign2) {
// CHECK: start:

// m68k-NEXT: call void @natural_align_2
// wasm-NEXT: call void @natural_align_2
// x86_64-linux-NEXT: call void @natural_align_2
// x86_64-windows-NEXT: call void @natural_align_2

Expand Down Expand Up @@ -204,8 +198,6 @@ pub unsafe fn call_fa16(x: ForceAlign16) {
extern "C" {
// m68k: declare void @natural_align_1({{.*}}byval([2 x i8]) align 1{{.*}})

// wasm: declare void @natural_align_1({{.*}}byval([2 x i8]) align 1{{.*}})

// x86_64-linux: declare void @natural_align_1(i16)

// x86_64-windows: declare void @natural_align_1(i16)
Expand All @@ -217,8 +209,6 @@ extern "C" {

// m68k: declare void @natural_align_2({{.*}}byval([34 x i8]) align 2{{.*}})

// wasm: declare void @natural_align_2({{.*}}byval([34 x i8]) align 2{{.*}})

// x86_64-linux: declare void @natural_align_2({{.*}}byval([34 x i8]) align 2{{.*}})

// x86_64-windows: declare void @natural_align_2(
Expand All @@ -232,8 +222,6 @@ extern "C" {

// m68k: declare void @force_align_4({{.*}}byval([20 x i8]) align 4{{.*}})

// wasm: declare void @force_align_4({{.*}}byval([20 x i8]) align 4{{.*}})

// x86_64-linux: declare void @force_align_4({{.*}}byval([20 x i8]) align 4{{.*}})

// x86_64-windows: declare void @force_align_4(
Expand All @@ -247,8 +235,6 @@ extern "C" {

// m68k: declare void @natural_align_8({{.*}}byval([24 x i8]) align 4{{.*}})

// wasm: declare void @natural_align_8({{.*}}byval([24 x i8]) align 8{{.*}})

// x86_64-linux: declare void @natural_align_8({{.*}}byval([24 x i8]) align 8{{.*}})

// x86_64-windows: declare void @natural_align_8(
Expand All @@ -262,8 +248,6 @@ extern "C" {

// m68k: declare void @force_align_8({{.*}}byval([24 x i8]) align 8{{.*}})

// wasm: declare void @force_align_8({{.*}}byval([24 x i8]) align 8{{.*}})

// x86_64-linux: declare void @force_align_8({{.*}}byval([24 x i8]) align 8{{.*}})

// x86_64-windows: declare void @force_align_8(
Expand All @@ -279,8 +263,6 @@ extern "C" {

// m68k: declare void @lower_fa8({{.*}}byval([24 x i8]) align 4{{.*}})

// wasm: declare void @lower_fa8({{.*}}byval([24 x i8]) align 8{{.*}})

// x86_64-linux: declare void @lower_fa8({{.*}}byval([24 x i8]) align 8{{.*}})

// x86_64-windows: declare void @lower_fa8(
Expand All @@ -294,8 +276,6 @@ extern "C" {

// m68k: declare void @wrapped_fa8({{.*}}byval([24 x i8]) align 8{{.*}})

// wasm: declare void @wrapped_fa8({{.*}}byval([24 x i8]) align 8{{.*}})

// x86_64-linux: declare void @wrapped_fa8({{.*}}byval([24 x i8]) align 8{{.*}})

// x86_64-windows: declare void @wrapped_fa8(
Expand All @@ -311,8 +291,6 @@ extern "C" {

// m68k: declare void @transparent_fa8({{.*}}byval([24 x i8]) align 8{{.*}})

// wasm: declare void @transparent_fa8({{.*}}byval([24 x i8]) align 8{{.*}})

// x86_64-linux: declare void @transparent_fa8({{.*}}byval([24 x i8]) align 8{{.*}})

// x86_64-windows: declare void @transparent_fa8(
Expand All @@ -328,8 +306,6 @@ extern "C" {

// m68k: declare void @force_align_16({{.*}}byval([80 x i8]) align 16{{.*}})

// wasm: declare void @force_align_16({{.*}}byval([80 x i8]) align 16{{.*}})

// x86_64-linux: declare void @force_align_16({{.*}}byval([80 x i8]) align 16{{.*}})

// x86_64-windows: declare void @force_align_16(
Expand Down
108 changes: 108 additions & 0 deletions tests/codegen/repr/transparent-struct-ptr-non-byval.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
//@ revisions: wasm32
//@ compile-flags: -O -C no-prepopulate-passes

//@[wasm32] compile-flags: --target wasm32-wasi
//@[wasm32] needs-llvm-components: webassembly

// See ./transparent.rs
// Some platforms pass large aggregates using immediate arrays in LLVMIR
// Other platforms pass large aggregates using struct pointer in LLVMIR
// This covers the "struct pointer" case, except without "byval".

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

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

impl Copy for [u32; 16] {}
impl Copy for BigS {}
impl Copy for BigU {}

#[repr(C)]
pub struct BigS([u32; 16]);

#[repr(transparent)]
pub struct TsBigS(BigS);

#[repr(transparent)]
pub union TuBigS {
field: BigS,
}

#[repr(transparent)]
pub enum TeBigS {
Variant(BigS),
}

// CHECK: define{{.*}}void @test_BigS(ptr [[BIGS_RET_ATTRS1:.*]] sret([64 x i8]) [[BIGS_RET_ATTRS2:.*]], ptr [[BIGS_ARG_ATTRS1:.*]] [[BIGS_ARG_ATTRS2:.*]])
#[no_mangle]
pub extern "C" fn test_BigS(_: BigS) -> BigS {
loop {}
}

// CHECK: define{{.*}}void @test_TsBigS(ptr [[BIGS_RET_ATTRS1]] sret([64 x i8]) [[BIGS_RET_ATTRS2]], ptr [[BIGS_ARG_ATTRS1]] [[BIGS_ARG_ATTRS2:.*]])
#[no_mangle]
pub extern "C" fn test_TsBigS(_: TsBigS) -> TsBigS {
loop {}
}

// CHECK: define{{.*}}void @test_TuBigS(ptr [[BIGS_RET_ATTRS1]] sret([64 x i8]) [[BIGS_RET_ATTRS2]], ptr [[BIGS_ARG_ATTRS1]] [[BIGS_ARG_ATTRS2:.*]])
#[no_mangle]
pub extern "C" fn test_TuBigS(_: TuBigS) -> TuBigS {
loop {}
}

// CHECK: define{{.*}}void @test_TeBigS(ptr [[BIGS_RET_ATTRS1]] sret([64 x i8]) [[BIGS_RET_ATTRS2]], ptr [[BIGS_ARG_ATTRS1]] [[BIGS_ARG_ATTRS2]])
#[no_mangle]
pub extern "C" fn test_TeBigS(_: TeBigS) -> TeBigS {
loop {}
}

#[repr(C)]
pub union BigU {
foo: [u32; 16],
}

#[repr(transparent)]
pub struct TsBigU(BigU);

#[repr(transparent)]
pub union TuBigU {
field: BigU,
}

#[repr(transparent)]
pub enum TeBigU {
Variant(BigU),
}

// CHECK: define{{.*}}void @test_BigU(ptr [[BIGU_RET_ATTRS1:.*]] sret([64 x i8]) [[BIGU_RET_ATTRS2:.*]], ptr [[BIGU_ARG_ATTRS1:.*]] [[BIGU_ARG_ATTRS2:.*]])
#[no_mangle]
pub extern "C" fn test_BigU(_: BigU) -> BigU {
loop {}
}

// CHECK: define{{.*}}void @test_TsBigU(ptr [[BIGU_RET_ATTRS1:.*]] sret([64 x i8]) [[BIGU_RET_ATTRS2:.*]], ptr [[BIGU_ARG_ATTRS1]] [[BIGU_ARG_ATTRS2]])
#[no_mangle]
pub extern "C" fn test_TsBigU(_: TsBigU) -> TsBigU {
loop {}
}

// CHECK: define{{.*}}void @test_TuBigU(ptr [[BIGU_RET_ATTRS1]] sret([64 x i8]) [[BIGU_RET_ATTRS2:.*]], ptr [[BIGU_ARG_ATTRS1]] [[BIGU_ARG_ATTRS2]])
#[no_mangle]
pub extern "C" fn test_TuBigU(_: TuBigU) -> TuBigU {
loop {}
}

// CHECK: define{{.*}}void @test_TeBigU(ptr [[BIGU_RET_ATTRS1]] sret([64 x i8]) [[BIGU_RET_ATTRS2:.*]], ptr [[BIGU_ARG_ATTRS1]] [[BIGU_ARG_ATTRS2]])
#[no_mangle]
pub extern "C" fn test_TeBigU(_: TeBigU) -> TeBigU {
loop {}
}
4 changes: 1 addition & 3 deletions tests/codegen/repr/transparent-struct-ptr.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//@ revisions: i686-linux i686-freebsd x64-linux x64-apple wasm32
//@ revisions: i686-linux i686-freebsd x64-linux x64-apple
//@ compile-flags: -O -C no-prepopulate-passes

//@[i686-linux] compile-flags: --target i686-unknown-linux-gnu
Expand All @@ -9,8 +9,6 @@
//@[x64-linux] needs-llvm-components: x86
//@[x64-apple] compile-flags: --target x86_64-apple-darwin
//@[x64-apple] needs-llvm-components: x86
//@[wasm32] compile-flags: --target wasm32-wasi
//@[wasm32] needs-llvm-components: webassembly

// See ./transparent.rs
// Some platforms pass large aggregates using immediate arrays in LLVMIR
Expand Down

0 comments on commit 8d3f80b

Please sign in to comment.