Skip to content

Stop using LLVM struct types for byval/sret #122050

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 11, 2024
Merged

Stop using LLVM struct types for byval/sret #122050

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
96a7267
use [N x i8] for byval/sret types
erikdesjardins Feb 25, 2024
c56ffaa
fix now-incorrect parenthetical about byval attr
erikdesjardins Mar 7, 2024
38324a1
improve byval abi docs
erikdesjardins Mar 9, 2024
549eac3
once byval abi is computed, the target abi isn't used further
erikdesjardins Mar 9, 2024
8fdd5e0
convert codegen/repr/transparent-* tests to no_core, fix discrepancies
erikdesjardins Mar 10, 2024
f18c2f8
add -O to some tests which depend on attributes being added
erikdesjardins Mar 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions compiler/rustc_codegen_llvm/src/abi.rs
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like the docs at https://doc.rust-lang.org/nightly/nightly-rustc/rustc_codegen_llvm/abi/enum.PassMode.html#variant.Indirect should be updated then? Currently they say

This corresponds to the byval LLVM argument attribute (using the Rust type of this argument).

The parenthetical is no longer true with this patch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the parenthetical. The important part is that byval is used, the specific type is not so important (it could be [N x i8], iM, the Rust struct type, etc.)

Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,10 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
PassMode::Indirect { attrs, meta_attrs: _, on_stack } => {
assert!(!on_stack);
let i = apply(attrs);
let sret = llvm::CreateStructRetAttr(cx.llcx, self.ret.layout.llvm_type(cx));
let sret = llvm::CreateStructRetAttr(
cx.llcx,
cx.type_array(cx.type_i8(), self.ret.layout.size.bytes()),
);
attributes::apply_to_llfn(llfn, llvm::AttributePlace::Argument(i), &[sret]);
}
PassMode::Cast { cast, pad_i32: _ } => {
Expand All @@ -437,7 +440,10 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
PassMode::Ignore => {}
PassMode::Indirect { attrs, meta_attrs: None, on_stack: true } => {
let i = apply(attrs);
let byval = llvm::CreateByValAttr(cx.llcx, arg.layout.llvm_type(cx));
let byval = llvm::CreateByValAttr(
cx.llcx,
cx.type_array(cx.type_i8(), arg.layout.size.bytes()),
);
attributes::apply_to_llfn(llfn, llvm::AttributePlace::Argument(i), &[byval]);
}
PassMode::Direct(attrs)
Expand Down Expand Up @@ -486,7 +492,10 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
PassMode::Indirect { attrs, meta_attrs: _, on_stack } => {
assert!(!on_stack);
let i = apply(bx.cx, attrs);
let sret = llvm::CreateStructRetAttr(bx.cx.llcx, self.ret.layout.llvm_type(bx));
let sret = llvm::CreateStructRetAttr(
bx.cx.llcx,
bx.cx.type_array(bx.cx.type_i8(), self.ret.layout.size.bytes()),
);
attributes::apply_to_callsite(callsite, llvm::AttributePlace::Argument(i), &[sret]);
}
PassMode::Cast { cast, pad_i32: _ } => {
Expand All @@ -513,7 +522,10 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
PassMode::Ignore => {}
PassMode::Indirect { attrs, meta_attrs: None, on_stack: true } => {
let i = apply(bx.cx, attrs);
let byval = llvm::CreateByValAttr(bx.cx.llcx, arg.layout.llvm_type(bx));
let byval = llvm::CreateByValAttr(
bx.cx.llcx,
bx.cx.type_array(bx.cx.type_i8(), arg.layout.size.bytes()),
);
attributes::apply_to_callsite(
callsite,
llvm::AttributePlace::Argument(i),
Expand Down
8 changes: 4 additions & 4 deletions tests/codegen/align-byval-vector.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ pub struct DoubleFoo {
}

extern "C" {
// x86-linux: declare void @f({{.*}}byval(%Foo) align 4{{.*}})
// x86-darwin: declare void @f({{.*}}byval(%Foo) align 16{{.*}})
// x86-linux: declare void @f({{.*}}byval([32 x i8]) align 4{{.*}})
// x86-darwin: declare void @f({{.*}}byval([32 x i8]) align 16{{.*}})
fn f(foo: Foo);

// x86-linux: declare void @g({{.*}}byval(%DoubleFoo) align 4{{.*}})
// x86-darwin: declare void @g({{.*}}byval(%DoubleFoo) align 16{{.*}})
// x86-linux: declare void @g({{.*}}byval([64 x i8]) align 4{{.*}})
// x86-darwin: declare void @g({{.*}}byval([64 x i8]) align 16{{.*}})
fn g(foo: DoubleFoo);
}

Expand Down
92 changes: 46 additions & 46 deletions tests/codegen/align-byval.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,20 +107,20 @@ pub unsafe fn call_na1(x: NaturalAlign1) {
// CHECK: start:

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

// wasm: [[ALLOCA:%[a-z0-9+]]] = alloca %NaturalAlign1, align 1
// wasm: call void @natural_align_1({{.*}}byval(%NaturalAlign1) align 1{{.*}} [[ALLOCA]])
// 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

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

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

Expand All @@ -135,10 +135,10 @@ pub unsafe fn call_na2(x: NaturalAlign2) {
// x86_64-windows-NEXT: call void @natural_align_2

// i686-linux: [[ALLOCA:%[0-9]+]] = alloca %NaturalAlign2, align 4
// i686-linux: call void @natural_align_2({{.*}}byval(%NaturalAlign2) align 4{{.*}} [[ALLOCA]])
// i686-linux: call void @natural_align_2({{.*}}byval([34 x i8]) align 4{{.*}} [[ALLOCA]])

// i686-windows: [[ALLOCA:%[0-9]+]] = alloca %NaturalAlign2, align 4
// i686-windows: call void @natural_align_2({{.*}}byval(%NaturalAlign2) align 4{{.*}} [[ALLOCA]])
// i686-windows: call void @natural_align_2({{.*}}byval([34 x i8]) align 4{{.*}} [[ALLOCA]])
natural_align_2(x);
}

Expand Down Expand Up @@ -199,141 +199,141 @@ pub unsafe fn call_fa16(x: ForceAlign16) {
}

extern "C" {
// m68k: declare void @natural_align_1({{.*}}byval(%NaturalAlign1) align 1{{.*}})
// m68k: declare void @natural_align_1({{.*}}byval([2 x i8]) align 1{{.*}})

// wasm: declare void @natural_align_1({{.*}}byval(%NaturalAlign1) 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)

// i686-linux: declare void @natural_align_1({{.*}}byval(%NaturalAlign1) align 4{{.*}})
// i686-linux: declare void @natural_align_1({{.*}}byval([2 x i8]) align 4{{.*}})

// i686-windows: declare void @natural_align_1({{.*}}byval(%NaturalAlign1) align 4{{.*}})
// i686-windows: declare void @natural_align_1({{.*}}byval([2 x i8]) align 4{{.*}})
fn natural_align_1(x: NaturalAlign1);

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

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

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

// x86_64-windows: declare void @natural_align_2(
// x86_64-windows-NOT: byval
// x86_64-windows-SAME: align 2{{.*}})

// i686-linux: declare void @natural_align_2({{.*}}byval(%NaturalAlign2) align 4{{.*}})
// i686-linux: declare void @natural_align_2({{.*}}byval([34 x i8]) align 4{{.*}})

// i686-windows: declare void @natural_align_2({{.*}}byval(%NaturalAlign2) align 4{{.*}})
// i686-windows: declare void @natural_align_2({{.*}}byval([34 x i8]) align 4{{.*}})
fn natural_align_2(x: NaturalAlign2);

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

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

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

// x86_64-windows: declare void @force_align_4(
// x86_64-windows-NOT: byval
// x86_64-windows-SAME: align 4{{.*}})

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

// i686-windows: declare void @force_align_4({{.*}}byval(%ForceAlign4) align 4{{.*}})
// i686-windows: declare void @force_align_4({{.*}}byval([20 x i8]) align 4{{.*}})
fn force_align_4(x: ForceAlign4);

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

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

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

// x86_64-windows: declare void @natural_align_8(
// x86_64-windows-NOT: byval
// x86_64-windows-SAME: align 8{{.*}})

// i686-linux: declare void @natural_align_8({{.*}}byval(%NaturalAlign8) align 4{{.*}})
// i686-linux: declare void @natural_align_8({{.*}}byval([24 x i8]) align 4{{.*}})

// i686-windows: declare void @natural_align_8({{.*}}byval(%NaturalAlign8) align 4{{.*}})
// i686-windows: declare void @natural_align_8({{.*}}byval([24 x i8]) align 4{{.*}})
fn natural_align_8(x: NaturalAlign8);

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

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

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

// x86_64-windows: declare void @force_align_8(
// x86_64-windows-NOT: byval
// x86_64-windows-SAME: align 8{{.*}})

// i686-linux: declare void @force_align_8({{.*}}byval(%ForceAlign8) align 4{{.*}})
// i686-linux: declare void @force_align_8({{.*}}byval([24 x i8]) align 4{{.*}})

// i686-windows: declare void @force_align_8(
// i686-windows-NOT: byval
// i686-windows-SAME: align 8{{.*}})
fn force_align_8(x: ForceAlign8);

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

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

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

// x86_64-windows: declare void @lower_fa8(
// x86_64-windows-NOT: byval
// x86_64-windows-SAME: align 8{{.*}})

// i686-linux: declare void @lower_fa8({{.*}}byval(%LowerFA8) align 4{{.*}})
// i686-linux: declare void @lower_fa8({{.*}}byval([24 x i8]) align 4{{.*}})

// i686-windows: declare void @lower_fa8({{.*}}byval(%LowerFA8) align 4{{.*}})
// i686-windows: declare void @lower_fa8({{.*}}byval([24 x i8]) align 4{{.*}})
fn lower_fa8(x: LowerFA8);

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

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

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

// x86_64-windows: declare void @wrapped_fa8(
// x86_64-windows-NOT: byval
// x86_64-windows-SAME: align 8{{.*}})

// i686-linux: declare void @wrapped_fa8({{.*}}byval(%WrappedFA8) align 4{{.*}})
// i686-linux: declare void @wrapped_fa8({{.*}}byval([24 x i8]) align 4{{.*}})

// i686-windows: declare void @wrapped_fa8(
// i686-windows-NOT: byval
// i686-windows-SAME: align 8{{.*}})
fn wrapped_fa8(x: WrappedFA8);

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

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

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

// x86_64-windows: declare void @transparent_fa8(
// x86_64-windows-NOT: byval
// x86_64-windows-SAME: align 8{{.*}})

// i686-linux: declare void @transparent_fa8({{.*}}byval(%TransparentFA8) align 4{{.*}})
// i686-linux: declare void @transparent_fa8({{.*}}byval([24 x i8]) align 4{{.*}})

// i686-windows: declare void @transparent_fa8(
// i686-windows-NOT: byval
// i686-windows-SAME: align 8{{.*}})
fn transparent_fa8(x: TransparentFA8);

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

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

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

// x86_64-windows: declare void @force_align_16(
// x86_64-windows-NOT: byval
// x86_64-windows-SAME: align 16{{.*}})

// i686-linux: declare void @force_align_16({{.*}}byval(%ForceAlign16) align 4{{.*}})
// i686-linux: declare void @force_align_16({{.*}}byval([80 x i8]) align 4{{.*}})

// i686-windows: declare void @force_align_16(
// i686-windows-NOT: byval
Expand Down
1 change: 0 additions & 1 deletion tests/codegen/align-enum.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ pub enum Align64 {
A(u32),
B(u32),
}
// CHECK: %Align64 = type { i32, [15 x i32] }

pub struct Nested64 {
a: u8,
Expand Down
4 changes: 0 additions & 4 deletions tests/codegen/align-struct.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,23 @@

#[repr(align(64))]
pub struct Align64(i32);
// CHECK: %Align64 = type { i32, [15 x i32] }

pub struct Nested64 {
a: Align64,
b: i32,
c: i32,
d: i8,
}
// CHECK: %Nested64 = type { %Align64, i32, i32, i8, [55 x i8] }

pub enum Enum4 {
A(i32),
B(i32),
}
// No Aggregate type, and hence nothing in LLVM IR.

pub enum Enum64 {
A(Align64),
B(i32),
}
// CHECK: %Enum64 = type { i32, [31 x i32] }

// CHECK-LABEL: @align64
#[no_mangle]
Expand Down
4 changes: 2 additions & 2 deletions tests/codegen/function-arguments-noopt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub fn borrow_call(x: &i32, f: fn(&i32) -> &i32) -> &i32 {
f(x)
}

// CHECK: void @struct_(ptr sret(%S) align 4{{( %_0)?}}, ptr align 4 %x)
// CHECK: void @struct_(ptr sret([32 x i8]) align 4{{( %_0)?}}, ptr align 4 %x)
#[no_mangle]
pub fn struct_(x: S) -> S {
x
Expand All @@ -51,7 +51,7 @@ pub fn struct_(x: S) -> S {
// CHECK-LABEL: @struct_call
#[no_mangle]
pub fn struct_call(x: S, f: fn(S) -> S) -> S {
// CHECK: call void %f(ptr sret(%S) align 4{{( %_0)?}}, ptr align 4 %{{.+}})
// CHECK: call void %f(ptr sret([32 x i8]) align 4{{( %_0)?}}, ptr align 4 %{{.+}})
f(x)
}

Expand Down
2 changes: 1 addition & 1 deletion tests/codegen/function-arguments.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ pub fn notunpin_box(x: Box<NotUnpin>) -> Box<NotUnpin> {
x
}

// CHECK: @struct_return(ptr noalias nocapture noundef sret(%S) align 4 dereferenceable(32){{( %_0)?}})
// CHECK: @struct_return(ptr noalias nocapture noundef sret([32 x i8]) align 4 dereferenceable(32){{( %_0)?}})
#[no_mangle]
pub fn struct_return() -> S {
S {
Expand Down
2 changes: 1 addition & 1 deletion tests/codegen/mem-replace-big-type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub fn replace_big(dst: &mut Big, src: Big) -> Big {
// CHECK-NOT: call void @llvm.memcpy

// For a large type, we expect exactly three `memcpy`s
// CHECK-LABEL: define internal void @{{.+}}mem{{.+}}replace{{.+}}sret(%Big)
// CHECK-LABEL: define internal void @{{.+}}mem{{.+}}replace{{.+}}sret([56 x i8])
// CHECK-NOT: call void @llvm.memcpy
// CHECK: call void @llvm.memcpy.{{.+}}(ptr align 8 %result, ptr align 8 %dest, i{{.*}} 56, i1 false)
// CHECK-NOT: call void @llvm.memcpy
Expand Down
Loading