From 872f6101f9d6ddf3b01f1ffca1bbb8f44ff42bdb Mon Sep 17 00:00:00 2001 From: Brendan Hansknecht Date: Sat, 16 Jul 2022 11:34:47 -0700 Subject: [PATCH 01/10] remove constraint around memcpy when storing to stack --- crates/compiler/gen_dev/src/generic64/storage.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/compiler/gen_dev/src/generic64/storage.rs b/crates/compiler/gen_dev/src/generic64/storage.rs index 1334e5ae7c..6e07c2d4a8 100644 --- a/crates/compiler/gen_dev/src/generic64/storage.rs +++ b/crates/compiler/gen_dev/src/generic64/storage.rs @@ -734,7 +734,11 @@ impl< ASM::mov_base32_freg64(buf, to_offset, reg); } _ if layout.stack_size(self.target_info) == 0 => {} - _ if layout.safe_to_memcpy() && layout.stack_size(self.target_info) > 8 => { + // TODO: Verify this is always true. + // The dev backend does not deal with refcounting and does not care about if data is safe to memcpy. + // It is just temporarily storing the value due to needing to free registers. + // Later, it will be reloaded and stored in refcounted as needed. + _ if layout.stack_size(self.target_info) > 8 => { let (from_offset, size) = self.stack_offset_and_size(sym); debug_assert!(from_offset % 8 == 0); debug_assert!(size % 8 == 0); From 9ba74c02e7cae3a904f2b3328c6c07e26e24168b Mon Sep 17 00:00:00 2001 From: Brendan Hansknecht Date: Sat, 16 Jul 2022 12:35:10 -0700 Subject: [PATCH 02/10] switch quicksort away from _generic function to run with dev --- examples/algorithms/quicksort-platform/host.zig | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/examples/algorithms/quicksort-platform/host.zig b/examples/algorithms/quicksort-platform/host.zig index a7f7f6b9b1..9800c95d3d 100644 --- a/examples/algorithms/quicksort-platform/host.zig +++ b/examples/algorithms/quicksort-platform/host.zig @@ -23,7 +23,7 @@ comptime { const mem = std.mem; const Allocator = mem.Allocator; -extern fn roc__mainForHost_1_exposed_generic(output: *RocList, input: *RocList) void; +extern fn roc__mainForHost_1_exposed(input: *RocList) RocList; const Align = 2 * @alignOf(usize); extern fn malloc(size: usize) callconv(.C) ?*align(Align) anyopaque; @@ -108,8 +108,7 @@ pub export fn main() u8 { std.os.clock_gettime(std.os.CLOCK.REALTIME, &ts1) catch unreachable; // actually call roc to populate the callresult - var callresult: RocList = undefined; - roc__mainForHost_1_exposed_generic(&callresult, &roc_list); + var callresult: RocList = roc__mainForHost_1_exposed(&roc_list); // const callresult: RocList = roc__mainForHost_1_exposed_generic(&roc_list); From 08be0dc8aaa7b11f548666e09b625f9a282a01b6 Mon Sep 17 00:00:00 2001 From: Brendan Hansknecht Date: Sat, 16 Jul 2022 16:34:51 -0700 Subject: [PATCH 03/10] fix llvm backend c abi for parameters passed by reference on the stack --- crates/compiler/gen_llvm/src/llvm/build.rs | 52 +++++++++++++++---- .../algorithms/quicksort-platform/host.zig | 10 ++-- 2 files changed, 46 insertions(+), 16 deletions(-) diff --git a/crates/compiler/gen_llvm/src/llvm/build.rs b/crates/compiler/gen_llvm/src/llvm/build.rs index 4ebb95b2c3..3ea7b0075f 100644 --- a/crates/compiler/gen_llvm/src/llvm/build.rs +++ b/crates/compiler/gen_llvm/src/llvm/build.rs @@ -3836,8 +3836,9 @@ fn expose_function_to_host_help_c_abi_v2<'a, 'ctx, 'env>( return_layout: Layout<'a>, c_function_name: &str, ) -> FunctionValue<'ctx> { - let it = arguments.iter().map(|l| basic_type_from_layout(env, l)); + let it = arguments.iter().map(|l| to_cc_type(env, l)); let argument_types = Vec::from_iter_in(it, env.arena); + let return_type = basic_type_from_layout(env, &return_layout); let cc_return = to_cc_return(env, &return_layout); @@ -3888,15 +3889,46 @@ fn expose_function_to_host_help_c_abi_v2<'a, 'ctx, 'env>( param_types.len() ); - let it = params.iter().zip(param_types).map(|(arg, fastcc_type)| { - let arg_type = arg.get_type(); - if arg_type == *fastcc_type { - // the C and Fast calling conventions agree - *arg - } else { - complex_bitcast_check_size(env, *arg, *fastcc_type, "to_fastcc_type_2") - } - }); + let it = params + .iter() + .zip(param_types) + .enumerate() + .map(|(i, (arg, fastcc_type))| { + let arg_type = arg.get_type(); + if arg_type == *fastcc_type { + // the C and Fast calling conventions agree + *arg + } else { + // not pretty, but seems to cover all our current cases + if arg_type.is_pointer_type() && !fastcc_type.is_pointer_type() { + // Modify the argument to specify it is passed by value and nonnull + let byval = context.create_type_attribute( + Attribute::get_named_enum_kind_id("byval"), + arg_type.into_pointer_type().get_element_type(), + ); + let nonnull = context.create_type_attribute( + Attribute::get_named_enum_kind_id("nonnull"), + arg_type.into_pointer_type().get_element_type(), + ); + c_function.add_attribute(AttributeLoc::Param((i + 1) as u32), byval); + c_function.add_attribute(AttributeLoc::Param((i + 1) as u32), nonnull); + // bitcast the ptr + let fastcc_ptr = env + .builder + .build_bitcast( + *arg, + fastcc_type.ptr_type(AddressSpace::Generic), + "bitcast_arg", + ) + .into_pointer_value(); + + let loaded = env.builder.build_load(fastcc_ptr, "load_arg"); + loaded + } else { + complex_bitcast_check_size(env, *arg, *fastcc_type, "to_fastcc_type_2") + } + } + }); let arguments = Vec::from_iter_in(it, env.arena); diff --git a/examples/algorithms/quicksort-platform/host.zig b/examples/algorithms/quicksort-platform/host.zig index 9800c95d3d..c2e70f3d31 100644 --- a/examples/algorithms/quicksort-platform/host.zig +++ b/examples/algorithms/quicksort-platform/host.zig @@ -23,7 +23,7 @@ comptime { const mem = std.mem; const Allocator = mem.Allocator; -extern fn roc__mainForHost_1_exposed(input: *RocList) RocList; +extern fn roc__mainForHost_1_exposed(input: RocList) callconv(.C) RocList; const Align = 2 * @alignOf(usize); extern fn malloc(size: usize) callconv(.C) ?*align(Align) anyopaque; @@ -83,7 +83,7 @@ export fn roc_memset(dst: [*]u8, value: i32, size: usize) callconv(.C) void { // warning! the array is currently stack-allocated so don't make this too big const NUM_NUMS = 100; -const RocList = extern struct { elements: [*]i64, length: usize }; +const RocList = extern struct { elements: [*]i64, length: usize, capacity: usize }; const Unit = extern struct {}; @@ -101,16 +101,14 @@ pub export fn main() u8 { numbers[i] = @mod(@intCast(i64, i), 12); } - var roc_list = RocList{ .elements = numbers, .length = NUM_NUMS }; + var roc_list = RocList{ .elements = numbers, .length = NUM_NUMS, .capacity = NUM_NUMS }; // start time var ts1: std.os.timespec = undefined; std.os.clock_gettime(std.os.CLOCK.REALTIME, &ts1) catch unreachable; // actually call roc to populate the callresult - var callresult: RocList = roc__mainForHost_1_exposed(&roc_list); - - // const callresult: RocList = roc__mainForHost_1_exposed_generic(&roc_list); + const callresult: RocList = roc__mainForHost_1_exposed(roc_list); // stdout the result const length = std.math.min(20, callresult.length); From e0cd63d7fc99148a309691cede092e69ba24e0c8 Mon Sep 17 00:00:00 2001 From: Brendan Hansknecht Date: Sat, 16 Jul 2022 19:03:35 -0700 Subject: [PATCH 04/10] clippy fix --- crates/compiler/gen_llvm/src/llvm/build.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/compiler/gen_llvm/src/llvm/build.rs b/crates/compiler/gen_llvm/src/llvm/build.rs index 3ea7b0075f..41e96d582a 100644 --- a/crates/compiler/gen_llvm/src/llvm/build.rs +++ b/crates/compiler/gen_llvm/src/llvm/build.rs @@ -3922,8 +3922,7 @@ fn expose_function_to_host_help_c_abi_v2<'a, 'ctx, 'env>( ) .into_pointer_value(); - let loaded = env.builder.build_load(fastcc_ptr, "load_arg"); - loaded + env.builder.build_load(fastcc_ptr, "load_arg") } else { complex_bitcast_check_size(env, *arg, *fastcc_type, "to_fastcc_type_2") } From 837ce380665569070805e6dcbc4dc1078dc8ac3f Mon Sep 17 00:00:00 2001 From: Brendan Hansknecht Date: Sun, 17 Jul 2022 07:32:16 -0700 Subject: [PATCH 05/10] correct parameter offset --- crates/compiler/gen_llvm/src/llvm/build.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/crates/compiler/gen_llvm/src/llvm/build.rs b/crates/compiler/gen_llvm/src/llvm/build.rs index 41e96d582a..3eb22e49fc 100644 --- a/crates/compiler/gen_llvm/src/llvm/build.rs +++ b/crates/compiler/gen_llvm/src/llvm/build.rs @@ -3910,8 +3910,15 @@ fn expose_function_to_host_help_c_abi_v2<'a, 'ctx, 'env>( Attribute::get_named_enum_kind_id("nonnull"), arg_type.into_pointer_type().get_element_type(), ); - c_function.add_attribute(AttributeLoc::Param((i + 1) as u32), byval); - c_function.add_attribute(AttributeLoc::Param((i + 1) as u32), nonnull); + // C return pointer goes at the beginning of params, and we must skip it if it exists. + let param_index = (i + + (if matches!(cc_return, CCReturn::ByPointer) { + 1 + } else { + 0 + })) as u32; + c_function.add_attribute(AttributeLoc::Param(param_index), byval); + c_function.add_attribute(AttributeLoc::Param(param_index), nonnull); // bitcast the ptr let fastcc_ptr = env .builder From b190b09b78bceada4a99ccbacb7a6e4a1b339281 Mon Sep 17 00:00:00 2001 From: Brendan Hansknecht Date: Mon, 18 Jul 2022 21:53:01 -0700 Subject: [PATCH 06/10] use general roc store instead of specific llvm store --- crates/compiler/gen_llvm/src/llvm/build.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/compiler/gen_llvm/src/llvm/build.rs b/crates/compiler/gen_llvm/src/llvm/build.rs index 3eb22e49fc..aa49d5806c 100644 --- a/crates/compiler/gen_llvm/src/llvm/build.rs +++ b/crates/compiler/gen_llvm/src/llvm/build.rs @@ -3955,7 +3955,7 @@ fn expose_function_to_host_help_c_abi_v2<'a, 'ctx, 'env>( CCReturn::ByPointer => { let out_ptr = c_function.get_nth_param(0).unwrap().into_pointer_value(); - env.builder.build_store(out_ptr, value); + store_roc_value(env, return_layout, out_ptr, value); env.builder.build_return(None); } CCReturn::Void => { From 9c35d29c240d9a7d54cd5028a74d7226b72d6be8 Mon Sep 17 00:00:00 2001 From: Brendan Hansknecht Date: Mon, 18 Jul 2022 22:04:34 -0700 Subject: [PATCH 07/10] correct sret to use the element type and not the pointer type --- crates/compiler/gen_llvm/src/llvm/build.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/compiler/gen_llvm/src/llvm/build.rs b/crates/compiler/gen_llvm/src/llvm/build.rs index aa49d5806c..cdc3a1bcaa 100644 --- a/crates/compiler/gen_llvm/src/llvm/build.rs +++ b/crates/compiler/gen_llvm/src/llvm/build.rs @@ -6411,8 +6411,10 @@ impl<'ctx> FunctionSpec<'ctx> { let sret_attribute_id = Attribute::get_named_enum_kind_id("sret"); debug_assert!(sret_attribute_id > 0); let ret_typ = self.typ.get_param_types()[param_index as usize]; + // ret_typ is a pointer type. We need the base type here. + let ret_base_typ = ret_typ.into_pointer_type().get_element_type(); let sret_attribute = - ctx.create_type_attribute(sret_attribute_id, ret_typ.as_any_type_enum()); + ctx.create_type_attribute(sret_attribute_id, ret_base_typ.as_any_type_enum()); fn_val.add_attribute(AttributeLoc::Param(0), sret_attribute); } } From 057e4ea57a861908069ce3312941a965b630db2e Mon Sep 17 00:00:00 2001 From: Brendan Hansknecht Date: Mon, 18 Jul 2022 22:35:39 -0700 Subject: [PATCH 08/10] ensure sret is actually pointer type instead of just assuming --- crates/compiler/gen_llvm/src/llvm/build.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/compiler/gen_llvm/src/llvm/build.rs b/crates/compiler/gen_llvm/src/llvm/build.rs index 2e3e533122..374904fe95 100644 --- a/crates/compiler/gen_llvm/src/llvm/build.rs +++ b/crates/compiler/gen_llvm/src/llvm/build.rs @@ -6460,10 +6460,13 @@ impl<'ctx> FunctionSpec<'ctx> { let sret_attribute_id = Attribute::get_named_enum_kind_id("sret"); debug_assert!(sret_attribute_id > 0); let ret_typ = self.typ.get_param_types()[param_index as usize]; - // ret_typ is a pointer type. We need the base type here. - let ret_base_typ = ret_typ.into_pointer_type().get_element_type(); - let sret_attribute = - ctx.create_type_attribute(sret_attribute_id, ret_base_typ.as_any_type_enum()); + // if ret_typ is a pointer type. We need the base type here. + let ret_base_typ = if ret_typ.is_pointer_type() { + ret_typ.into_pointer_type().get_element_type() + } else { + ret_typ.as_any_type_enum() + }; + let sret_attribute = ctx.create_type_attribute(sret_attribute_id, ret_base_typ); fn_val.add_attribute(AttributeLoc::Param(0), sret_attribute); } } From 7526f7069bde61a9876ad186d246c14d8a6c5cd5 Mon Sep 17 00:00:00 2001 From: Brendan Hansknecht Date: Mon, 18 Jul 2022 22:35:52 -0700 Subject: [PATCH 09/10] Revert "use general roc store instead of specific llvm store" Not sure why but this is wrong for some pointer reasons? This reverts commit b190b09b78bceada4a99ccbacb7a6e4a1b339281. --- crates/compiler/gen_llvm/src/llvm/build.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/compiler/gen_llvm/src/llvm/build.rs b/crates/compiler/gen_llvm/src/llvm/build.rs index 374904fe95..f50714a1bf 100644 --- a/crates/compiler/gen_llvm/src/llvm/build.rs +++ b/crates/compiler/gen_llvm/src/llvm/build.rs @@ -3956,7 +3956,7 @@ fn expose_function_to_host_help_c_abi_v2<'a, 'ctx, 'env>( CCReturn::ByPointer => { let out_ptr = c_function.get_nth_param(0).unwrap().into_pointer_value(); - store_roc_value(env, return_layout, out_ptr, value); + env.builder.build_store(out_ptr, value); env.builder.build_return(None); } CCReturn::Void => { From fd833df64971f743d4c573d95f74aec13f5d477f Mon Sep 17 00:00:00 2001 From: Brendan Hansknecht Date: Fri, 22 Jul 2022 23:04:11 -0700 Subject: [PATCH 10/10] correct aarch64 c call conv --- crates/compiler/gen_llvm/src/llvm/build.rs | 42 ++++++++++++---------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/crates/compiler/gen_llvm/src/llvm/build.rs b/crates/compiler/gen_llvm/src/llvm/build.rs index f50714a1bf..712a2accde 100644 --- a/crates/compiler/gen_llvm/src/llvm/build.rs +++ b/crates/compiler/gen_llvm/src/llvm/build.rs @@ -3902,24 +3902,30 @@ fn expose_function_to_host_help_c_abi_v2<'a, 'ctx, 'env>( } else { // not pretty, but seems to cover all our current cases if arg_type.is_pointer_type() && !fastcc_type.is_pointer_type() { - // Modify the argument to specify it is passed by value and nonnull - let byval = context.create_type_attribute( - Attribute::get_named_enum_kind_id("byval"), - arg_type.into_pointer_type().get_element_type(), - ); - let nonnull = context.create_type_attribute( - Attribute::get_named_enum_kind_id("nonnull"), - arg_type.into_pointer_type().get_element_type(), - ); - // C return pointer goes at the beginning of params, and we must skip it if it exists. - let param_index = (i - + (if matches!(cc_return, CCReturn::ByPointer) { - 1 - } else { - 0 - })) as u32; - c_function.add_attribute(AttributeLoc::Param(param_index), byval); - c_function.add_attribute(AttributeLoc::Param(param_index), nonnull); + // On x86_*, Modify the argument to specify it is passed by value and nonnull + // Aarch*, just passes in the pointer directly. + if matches!( + env.target_info.architecture, + roc_target::Architecture::X86_32 | roc_target::Architecture::X86_64 + ) { + let byval = context.create_type_attribute( + Attribute::get_named_enum_kind_id("byval"), + arg_type.into_pointer_type().get_element_type(), + ); + let nonnull = context.create_type_attribute( + Attribute::get_named_enum_kind_id("nonnull"), + arg_type.into_pointer_type().get_element_type(), + ); + // C return pointer goes at the beginning of params, and we must skip it if it exists. + let param_index = (i + + (if matches!(cc_return, CCReturn::ByPointer) { + 1 + } else { + 0 + })) as u32; + c_function.add_attribute(AttributeLoc::Param(param_index), byval); + c_function.add_attribute(AttributeLoc::Param(param_index), nonnull); + } // bitcast the ptr let fastcc_ptr = env .builder