From a1341756a2d31c0137071de4fab2dec17fc3775b Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Mon, 16 Sep 2024 13:01:29 -0700 Subject: [PATCH 1/4] compiler: Use make_indirect for the wasm ABI This is ignored by LLVM, but is still incorrect. --- compiler/rustc_target/src/abi/call/wasm.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_target/src/abi/call/wasm.rs b/compiler/rustc_target/src/abi/call/wasm.rs index 4ae8b9490ddab..3c4cd76a75464 100644 --- a/compiler/rustc_target/src/abi/call/wasm.rs +++ b/compiler/rustc_target/src/abi/call/wasm.rs @@ -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(); } } From 9fbdbc345333b0c8ade7952418e66c27f10626c8 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Mon, 16 Sep 2024 13:04:24 -0700 Subject: [PATCH 2/4] compiler: s/make_indirect_byval/pass_by_stack_offset/ The previous name is just an LLVMism, which conveys almost nothing about what is actually meant by the function relative to the ABI. In doing so, remove an already-addressed FIXME. --- compiler/rustc_target/src/abi/call/m68k.rs | 2 +- compiler/rustc_target/src/abi/call/mod.rs | 7 +++---- compiler/rustc_target/src/abi/call/x86.rs | 2 +- compiler/rustc_target/src/abi/call/x86_64.rs | 2 +- compiler/rustc_target/src/abi/call/xtensa.rs | 2 +- 5 files changed, 7 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_target/src/abi/call/m68k.rs b/compiler/rustc_target/src/abi/call/m68k.rs index b6bd68b66fcc4..82fe81f8c52ae 100644 --- a/compiler/rustc_target/src/abi/call/m68k.rs +++ b/compiler/rustc_target/src/abi/call/m68k.rs @@ -14,7 +14,7 @@ fn classify_arg(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); } diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index c2826b55dc5b1..060ee4a1bc221 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -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, on_stack: bool }, @@ -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) { + pub fn pass_by_stack_offset(&mut self, byval_align: Option) { assert!(!self.layout.is_unsized(), "used byval ABI for unsized layout"); self.make_indirect(); match self.mode { @@ -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(()); } diff --git a/compiler/rustc_target/src/abi/call/x86.rs b/compiler/rustc_target/src/abi/call/x86.rs index bdb14350ded4b..d9af83d3205bc 100644 --- a/compiler/rustc_target/src/abi/call/x86.rs +++ b/compiler/rustc_target/src/abi/call/x86.rs @@ -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); } diff --git a/compiler/rustc_target/src/abi/call/x86_64.rs b/compiler/rustc_target/src/abi/call/x86_64.rs index 1209204debfa3..9910e623ac9b5 100644 --- a/compiler/rustc_target/src/abi/call/x86_64.rs +++ b/compiler/rustc_target/src/abi/call/x86_64.rs @@ -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(); diff --git a/compiler/rustc_target/src/abi/call/xtensa.rs b/compiler/rustc_target/src/abi/call/xtensa.rs index d7b5fe9d4cc71..e1728b08a396b 100644 --- a/compiler/rustc_target/src/abi/call/xtensa.rs +++ b/compiler/rustc_target/src/abi/call/xtensa.rs @@ -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. From a48e6d8991f61ecd3dcdb973bc0b006703e6fa48 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Mon, 16 Sep 2024 14:30:19 -0700 Subject: [PATCH 3/4] tests: Remove test for wrong wasm codegen --- tests/codegen/align-byval.rs | 26 +------------------------- 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/tests/codegen/align-byval.rs b/tests/codegen/align-byval.rs index 223696229cb1b..3af42d50fd84c 100644 --- a/tests/codegen/align-byval.rs +++ b/tests/codegen/align-byval.rs @@ -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 @@ -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)] @@ -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 @@ -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 @@ -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) @@ -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( @@ -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( @@ -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( @@ -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( @@ -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( @@ -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( @@ -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( @@ -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( From 30be609eacdbf63774d3ae910af9fac0e2ff7759 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Mon, 16 Sep 2024 18:43:23 -0700 Subject: [PATCH 4/4] tests: Fission transparent-struct-ptr.rs for wasm --- .../repr/transparent-struct-ptr-non-byval.rs | 108 ++++++++++++++++++ tests/codegen/repr/transparent-struct-ptr.rs | 4 +- 2 files changed, 109 insertions(+), 3 deletions(-) create mode 100644 tests/codegen/repr/transparent-struct-ptr-non-byval.rs diff --git a/tests/codegen/repr/transparent-struct-ptr-non-byval.rs b/tests/codegen/repr/transparent-struct-ptr-non-byval.rs new file mode 100644 index 0000000000000..3d876c73af594 --- /dev/null +++ b/tests/codegen/repr/transparent-struct-ptr-non-byval.rs @@ -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 {} +} diff --git a/tests/codegen/repr/transparent-struct-ptr.rs b/tests/codegen/repr/transparent-struct-ptr.rs index 9cffd6c7f7321..38e20a28f10cb 100644 --- a/tests/codegen/repr/transparent-struct-ptr.rs +++ b/tests/codegen/repr/transparent-struct-ptr.rs @@ -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 @@ -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