From 83de4b55ef5b02c6edb5b687896671a8c5313ba6 Mon Sep 17 00:00:00 2001 From: Folkert Date: Fri, 22 Oct 2021 12:24:55 +0200 Subject: [PATCH 1/4] remove another instance of RocCallResult --- compiler/gen_llvm/src/llvm/build.rs | 19 +++---------------- examples/benchmarks/platform/host.zig | 12 ++---------- examples/effect/thing/platform-dir/host.zig | 16 ++++++++++++++-- 3 files changed, 19 insertions(+), 28 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index fc94166f7e2..ba2205d2feb 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -3186,7 +3186,6 @@ fn expose_function_to_host_help_c_abi_generic<'a, 'ctx, 'env>( let call_unwrapped_result = call_unwrapped.try_as_basic_value().left().unwrap(); - // make_good_roc_result(env, call_unwrapped_result) call_unwrapped_result } }; @@ -3464,7 +3463,6 @@ fn expose_function_to_host_help_c_abi<'a, 'ctx, 'env>( let call_unwrapped_result = call_unwrapped.try_as_basic_value().left().unwrap(); - // make_good_roc_result(env, call_unwrapped_result) call_unwrapped_result }; @@ -4044,10 +4042,7 @@ pub fn build_closure_caller<'a, 'ctx, 'env>( let result_type = basic_type_from_layout(env, result); - let roc_call_result_type = - context.struct_type(&[context.i64_type().into(), result_type], false); - - let output_type = { roc_call_result_type.ptr_type(AddressSpace::Generic) }; + let output_type = { result_type.ptr_type(AddressSpace::Generic) }; argument_types.push(output_type.into()); // STEP 1: build function header @@ -4104,9 +4099,7 @@ pub fn build_closure_caller<'a, 'ctx, 'env>( call.set_call_convention(evaluator.get_call_conventions()); - let call_result = call.try_as_basic_value().left().unwrap(); - - make_good_roc_result(env, call_result) + call.try_as_basic_value().left().unwrap() }; builder.build_store(output, call_result); @@ -4114,13 +4107,7 @@ pub fn build_closure_caller<'a, 'ctx, 'env>( builder.build_return(None); // STEP 3: build a {} -> u64 function that gives the size of the return type - build_host_exposed_alias_size_help( - env, - def_name, - alias_symbol, - Some("result"), - roc_call_result_type.into(), - ); + build_host_exposed_alias_size_help(env, def_name, alias_symbol, Some("result"), result_type); // STEP 4: build a {} -> u64 function that gives the size of the closure build_host_exposed_alias_size( diff --git a/examples/benchmarks/platform/host.zig b/examples/benchmarks/platform/host.zig index deed8ce10f4..33b9fdd740a 100644 --- a/examples/benchmarks/platform/host.zig +++ b/examples/benchmarks/platform/host.zig @@ -29,7 +29,6 @@ extern fn roc__mainForHost_1_Fx_caller(*const u8, [*]u8, [*]u8) void; extern fn roc__mainForHost_1_Fx_size() i64; extern fn roc__mainForHost_1_Fx_result_size() i64; - const Align = 2 * @alignOf(usize); extern fn malloc(size: usize) callconv(.C) ?*align(Align) c_void; extern fn realloc(c_ptr: [*]align(Align) u8, size: usize) callconv(.C) ?*c_void; @@ -137,15 +136,8 @@ fn call_the_closure(closure_data_pointer: [*]u8) void { roc__mainForHost_1_Fx_caller(&flags, closure_data_pointer, output); - const elements = @ptrCast([*]u64, @alignCast(8, output)); - - var flag = elements[0]; - - if (flag == 0) { - return; - } else { - unreachable; - } + // The closure returns result, nothing interesting to do with it + return; } pub export fn roc_fx_putInt(int: i64) i64 { diff --git a/examples/effect/thing/platform-dir/host.zig b/examples/effect/thing/platform-dir/host.zig index 49fad7a204d..35d30c99abb 100644 --- a/examples/effect/thing/platform-dir/host.zig +++ b/examples/effect/thing/platform-dir/host.zig @@ -54,11 +54,11 @@ export fn roc_panic(c_ptr: *c_void, tag_id: u32) callconv(.C) void { std.process.exit(0); } -export fn roc_memcpy(dst: [*]u8, src: [*]u8, size: usize) callconv(.C) void{ +export fn roc_memcpy(dst: [*]u8, src: [*]u8, size: usize) callconv(.C) void { return memcpy(dst, src, size); } -export fn roc_memset(dst: [*]u8, value: i32, size: usize) callconv(.C) void{ +export fn roc_memset(dst: [*]u8, value: i32, size: usize) callconv(.C) void { return memset(dst, value, size); } @@ -104,6 +104,18 @@ fn call_the_closure(closure_data_pointer: [*]u8) void { const allocator = std.heap.page_allocator; const size = roc__mainForHost_1_Fx_result_size(); + + if (size == 0) { + // the function call returns an empty record + // allocating 0 bytes causes issues because the allocator will return a NULL pointer + // So it's special-cased + const flags: u8 = 0; + var result: [1]u8 = .{0}; + roc__mainForHost_1_Fx_caller(&flags, closure_data_pointer, &result); + + return; + } + const raw_output = allocator.allocAdvanced(u8, @alignOf(u64), @intCast(usize, size), .at_least) catch unreachable; var output = @ptrCast([*]u8, raw_output); From e6edfda9a52a31d4624b926b676e4c67ab141871 Mon Sep 17 00:00:00 2001 From: Folkert Date: Fri, 22 Oct 2021 12:32:37 +0200 Subject: [PATCH 2/4] refactor call to roc function --- compiler/gen_llvm/src/llvm/build.rs | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index ba2205d2feb..6c3f194517d 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -4342,7 +4342,6 @@ fn function_value_by_name_help<'a, 'ctx, 'env>( }) } -// #[allow(clippy::cognitive_complexity)] #[inline(always)] fn roc_call_with_args<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, @@ -4350,18 +4349,32 @@ fn roc_call_with_args<'a, 'ctx, 'env>( result_layout: &Layout<'a>, symbol: Symbol, func_spec: FuncSpec, - args: &[BasicValueEnum<'ctx>], + arguments: &[BasicValueEnum<'ctx>], ) -> BasicValueEnum<'ctx> { let fn_val = function_value_by_func_spec(env, func_spec, symbol, argument_layouts, result_layout); - let call = env.builder.build_call(fn_val, args, "call"); + call_roc_function(env, fn_val, result_layout, arguments) +} - call.set_call_convention(fn_val.get_call_conventions()); +fn call_roc_function<'a, 'ctx, 'env>( + env: &Env<'a, 'ctx, 'env>, + roc_function: FunctionValue<'ctx>, + _result_layout: &Layout<'a>, + arguments: &[BasicValueEnum<'ctx>], +) -> BasicValueEnum<'ctx> { + let call = env.builder.build_call(roc_function, arguments, "call"); + + // roc functions should have the fast calling convention + debug_assert_eq!(roc_function.get_call_conventions(), FAST_CALL_CONV); + call.set_call_convention(FAST_CALL_CONV); - call.try_as_basic_value() - .left() - .unwrap_or_else(|| panic!("LLVM error: Invalid call by name for name {:?}", symbol)) + call.try_as_basic_value().left().unwrap_or_else(|| { + panic!( + "LLVM error: Invalid call by name for name {:?}", + roc_function.get_name() + ) + }) } /// Translates a target_lexicon::Triple to a LLVM calling convention u32 From 10b9307ab639bd35ec73d74c347384ce9f3aafbf Mon Sep 17 00:00:00 2001 From: Folkert Date: Fri, 22 Oct 2021 12:56:58 +0200 Subject: [PATCH 3/4] centralize calling roc functions in llvm --- compiler/gen_llvm/src/llvm/build.rs | 58 +++++++++-------------------- 1 file changed, 17 insertions(+), 41 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 6c3f194517d..696884872d3 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -3095,6 +3095,7 @@ fn expose_function_to_host_help_c_abi_generic<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, roc_function: FunctionValue<'ctx>, arguments: &[Layout<'a>], + return_layout: Layout<'a>, c_function_name: &str, ) -> FunctionValue<'ctx> { // NOTE we ingore env.is_gen_test here @@ -3171,22 +3172,11 @@ fn expose_function_to_host_help_c_abi_generic<'a, 'ctx, 'env>( builder.position_at_end(entry); - let call_wrapped = builder.build_call( - roc_wrapper_function, - arguments_for_call, - "call_wrapped_function", - ); - call_wrapped.set_call_convention(FAST_CALL_CONV); - - call_wrapped.try_as_basic_value().left().unwrap() + let elements = [Layout::Builtin(Builtin::Int64), return_layout]; + let wrapped_layout = Layout::Struct(&elements); + call_roc_function(env, roc_function, &wrapped_layout, arguments_for_call) } else { - let call_unwrapped = - builder.build_call(roc_function, arguments_for_call, "call_unwrapped_function"); - call_unwrapped.set_call_convention(FAST_CALL_CONV); - - let call_unwrapped_result = call_unwrapped.try_as_basic_value().left().unwrap(); - - call_unwrapped_result + call_roc_function(env, roc_function, &return_layout, arguments_for_call) } }; @@ -3208,6 +3198,7 @@ fn expose_function_to_host_help_c_abi_gen_test<'a, 'ctx, 'env>( ident_string: &str, roc_function: FunctionValue<'ctx>, arguments: &[Layout<'a>], + return_layout: Layout<'a>, c_function_name: &str, ) -> FunctionValue<'ctx> { let context = env.context; @@ -3291,14 +3282,12 @@ fn expose_function_to_host_help_c_abi_gen_test<'a, 'ctx, 'env>( builder.position_at_end(entry); - let call_wrapped = builder.build_call( + call_roc_function( + env, roc_wrapper_function, + &return_layout, arguments_for_call, - "call_wrapped_function", - ); - call_wrapped.set_call_convention(FAST_CALL_CONV); - - call_wrapped.try_as_basic_value().left().unwrap() + ) }; let output_arg_index = args_length - 1; @@ -3354,6 +3343,7 @@ fn expose_function_to_host_help_c_abi<'a, 'ctx, 'env>( ident_string, roc_function, arguments, + return_layout, c_function_name, ); } @@ -3363,6 +3353,7 @@ fn expose_function_to_host_help_c_abi<'a, 'ctx, 'env>( env, roc_function, arguments, + return_layout, &format!("{}_generic", c_function_name), ); @@ -3456,15 +3447,7 @@ fn expose_function_to_host_help_c_abi<'a, 'ctx, 'env>( let arguments_for_call = &arguments_for_call.into_bump_slice(); - let call_result = { - let call_unwrapped = - builder.build_call(roc_function, arguments_for_call, "call_unwrapped_function"); - call_unwrapped.set_call_convention(FAST_CALL_CONV); - - let call_unwrapped_result = call_unwrapped.try_as_basic_value().left().unwrap(); - - call_unwrapped_result - }; + let call_result = call_roc_function(env, roc_function, &return_layout, arguments_for_call); match cc_return { CCReturn::Void => { @@ -4018,6 +4001,7 @@ pub fn build_closure_caller<'a, 'ctx, 'env>( evaluator: FunctionValue<'ctx>, alias_symbol: Symbol, arguments: &[Layout<'a>], + return_layout: &Layout<'a>, lambda_set: LambdaSet<'a>, result: &Layout<'a>, ) { @@ -4093,13 +4077,7 @@ pub fn build_closure_caller<'a, 'ctx, 'env>( result_type, ) } else { - let call = env - .builder - .build_call(evaluator, &evaluator_arguments, "call_function"); - - call.set_call_convention(evaluator.get_call_conventions()); - - call.try_as_basic_value().left().unwrap() + call_roc_function(env, evaluator, return_layout, &evaluator_arguments) }; builder.build_store(output, call_result); @@ -4235,7 +4213,7 @@ pub fn build_proc<'a, 'ctx, 'env>( let fn_name: String = format!("{}_1", ident_string); build_closure_caller( - env, &fn_name, evaluator, name, arguments, closure, result, + env, &fn_name, evaluator, name, arguments, result, closure, result, ) } @@ -5848,9 +5826,7 @@ fn build_foreign_symbol<'a, 'ctx, 'env>( } }; - let call = env.builder.build_call(fastcc_function, &arguments, "tmp"); - call.set_call_convention(FAST_CALL_CONV); - return call.try_as_basic_value().left().unwrap(); + call_roc_function(env, fastcc_function, ret_layout, &arguments) } fn throw_on_overflow<'a, 'ctx, 'env>( From ad9f0b06091d1181c42ff3f12f443b4f017bd61f Mon Sep 17 00:00:00 2001 From: Folkert Date: Fri, 22 Oct 2021 13:07:45 +0200 Subject: [PATCH 4/4] clippy --- compiler/gen_llvm/src/llvm/build.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 696884872d3..bacda91a69d 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -3995,6 +3995,7 @@ fn build_proc_header<'a, 'ctx, 'env>( fn_val } +#[allow(clippy::too_many_arguments)] pub fn build_closure_caller<'a, 'ctx, 'env>( env: &'a Env<'a, 'ctx, 'env>, def_name: &str,