From e73ac60053acab900385a0917be8bbeb956f8f3a Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 20 Oct 2021 23:49:53 +0200 Subject: [PATCH 01/19] improve Tag literal generation --- compiler/gen_llvm/src/llvm/build.rs | 115 +++++++++++++++------------- 1 file changed, 61 insertions(+), 54 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index bacda91a69..15bba5307b 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -618,8 +618,8 @@ pub fn construct_optimization_passes<'a>( mpm.add_always_inliner_pass(); // tail-call elimination is always on - fpm.add_instruction_combining_pass(); - fpm.add_tail_call_elimination_pass(); + // fpm.add_instruction_combining_pass(); + // fpm.add_tail_call_elimination_pass(); let pmb = PassManagerBuilder::create(); match opt_level { @@ -629,7 +629,7 @@ pub fn construct_optimization_passes<'a>( OptLevel::Optimize => { pmb.set_optimization_level(OptimizationLevel::Aggressive); // this threshold seems to do what we want - pmb.set_inliner_with_threshold(275); + pmb.set_inliner_with_threshold(0); // TODO figure out which of these actually help @@ -1241,17 +1241,26 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( let struct_type = basic_type_from_layout(env, &struct_layout); - let struct_value = access_index_struct_value( - builder, - argument.into_struct_value(), - struct_type.into_struct_type(), + let argument_pointer = builder.build_alloca(argument.get_type(), "cast_alloca"); + builder.build_store(argument_pointer, argument); + + let struct_ptr = builder.build_pointer_cast( + argument_pointer, + struct_type.ptr_type(AddressSpace::Generic), + "foobar", ); + // let struct_value = access_index_struct_value( + // builder, + // argument.into_struct_value(), + // struct_type.into_struct_type(), + // ); + let result = builder - .build_extract_value(struct_value, *index as u32, "") + .build_struct_gep(struct_ptr, *index as u32, "") .expect("desired field did not decode"); - result + builder.build_load(result, "foobarbaz") } UnionLayout::Recursive(tag_layouts) => { debug_assert!(argument.is_pointer_value()); @@ -1453,7 +1462,13 @@ pub fn build_tag<'a, 'ctx, 'env>( UnionLayout::NonRecursive(tags) => { debug_assert!(union_size > 1); - let ctx = env.context; + let internal_type = block_of_memory_slices(env.context, tags, env.ptr_bytes); + + let tag_id_type = basic_type_from_layout(env, &tag_id_layout).into_int_type(); + let wrapper_type = env + .context + .struct_type(&[internal_type, tag_id_type.into()], false); + let result_alloca = env.builder.build_alloca(wrapper_type, "tag_opaque"); // Determine types let num_fields = arguments.len() + 1; @@ -1484,54 +1499,46 @@ pub fn build_tag<'a, 'ctx, 'env>( } } } - - // Create the struct_type - let struct_type = ctx.struct_type(field_types.into_bump_slice(), false); - - // Insert field exprs into struct_val - let struct_val = - struct_from_fields(env, struct_type, field_vals.into_iter().enumerate()); - - // How we create tag values - // - // The memory layout of tags can be different. e.g. in - // - // [ Ok Int, Err Str ] - // - // the `Ok` tag stores a 64-bit integer, the `Err` tag stores a struct. - // All tags of a union must have the same length, for easy addressing (e.g. array lookups). - // So we need to ask for the maximum of all tag's sizes, even if most tags won't use - // all that memory, and certainly won't use it in the same way (the tags have fields of - // different types/sizes) - // - // In llvm, we must be explicit about the type of value we're creating: we can't just - // make a unspecified block of memory. So what we do is create a byte array of the - // desired size. Then when we know which tag we have (which is here, in this function), - // we need to cast that down to the array of bytes that llvm expects - // - // There is the bitcast instruction, but it doesn't work for arrays. So we need to jump - // through some hoops using store and load to get this to work: the array is put into a - // one-element struct, which can be cast to the desired type. - // - // This tricks comes from - // https://github.com/raviqqe/ssf/blob/bc32aae68940d5bddf5984128e85af75ca4f4686/ssf-llvm/src/expression_compiler.rs#L116 - - let internal_type = block_of_memory_slices(env.context, tags, env.ptr_bytes); - - let data = cast_tag_to_block_of_memory(env, struct_val, internal_type); - let tag_id_type = basic_type_from_layout(env, &tag_id_layout).into_int_type(); - let wrapper_type = env - .context - .struct_type(&[data.get_type(), tag_id_type.into()], false); + // store the tag id + let tag_id_ptr = env + .builder + .build_struct_gep(result_alloca, TAG_ID_INDEX, "get_opaque_data") + .unwrap(); let tag_id_intval = tag_id_type.const_int(tag_id as u64, false); + env.builder.build_store(tag_id_ptr, tag_id_intval); - let field_vals = [ - (TAG_DATA_INDEX as usize, data), - (TAG_ID_INDEX as usize, tag_id_intval.into()), - ]; + // Create the struct_type + let struct_type = env + .context + .struct_type(field_types.into_bump_slice(), false); - struct_from_fields(env, wrapper_type, field_vals.iter().copied()).into() + let struct_opaque_ptr = env + .builder + .build_struct_gep(result_alloca, TAG_DATA_INDEX, "get_opaque_data") + .unwrap(); + let struct_ptr = env.builder.build_pointer_cast( + struct_opaque_ptr, + struct_type.ptr_type(AddressSpace::Generic), + "to_specific", + ); + + // Insert field exprs into struct_val + //let struct_val = + //struct_from_fields(env, struct_type, field_vals.into_iter().enumerate()); + + // Insert field exprs into struct_val + for (index, field_val) in field_vals.iter().copied().enumerate() { + let index: u32 = index as u32; + + let ptr = env + .builder + .build_struct_gep(struct_ptr, index, "get_tag_field_ptr") + .unwrap(); + env.builder.build_store(ptr, field_val); + } + + env.builder.build_load(result_alloca, "load_result") } UnionLayout::Recursive(tags) => { debug_assert!(union_size > 1); From 7d1bd0ffe7c562f2191f30e1697052f0a5af1a1b Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 21 Oct 2021 22:08:32 +0200 Subject: [PATCH 02/19] make refcount take tag union by reference --- compiler/gen_llvm/src/llvm/build.rs | 2 +- compiler/gen_llvm/src/llvm/refcounting.rs | 61 ++++++++++++++++++----- 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 15bba5307b..80c588bfb0 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -971,7 +971,7 @@ pub fn build_exp_call<'a, 'ctx, 'env>( } } -const TAG_ID_INDEX: u32 = 1; +pub const TAG_ID_INDEX: u32 = 1; pub const TAG_DATA_INDEX: u32 = 0; pub fn struct_from_fields<'a, 'ctx, 'env, I>( diff --git a/compiler/gen_llvm/src/llvm/refcounting.rs b/compiler/gen_llvm/src/llvm/refcounting.rs index 8a89c924d7..0e8f275554 100644 --- a/compiler/gen_llvm/src/llvm/refcounting.rs +++ b/compiler/gen_llvm/src/llvm/refcounting.rs @@ -2,7 +2,7 @@ use crate::debug_info_init; use crate::llvm::bitcode::call_void_bitcode_fn; use crate::llvm::build::{ add_func, cast_basic_basic, cast_block_of_memory_to_tag, get_tag_id, get_tag_id_non_recursive, - tag_pointer_clear_tag_id, Env, FAST_CALL_CONV, TAG_DATA_INDEX, + tag_pointer_clear_tag_id, Env, FAST_CALL_CONV, TAG_DATA_INDEX, TAG_ID_INDEX, }; use crate::llvm::build_list::{incrementing_elem_loop, list_len, load_list}; use crate::llvm::convert::{basic_type_from_layout, ptr_int}; @@ -542,6 +542,24 @@ fn modify_refcount_layout_help<'a, 'ctx, 'env>( } }, _ => { + if let Layout::Union(UnionLayout::NonRecursive(_)) = layout { + let alloca = env.builder.build_alloca(value.get_type(), "tag_union"); + env.builder.build_store(alloca, value); + call_help(env, function, call_mode, alloca.into()); + return; + } + + if let Layout::LambdaSet(lambda_set) = layout { + if let Layout::Union(UnionLayout::NonRecursive(_)) = + lambda_set.runtime_representation() + { + let alloca = env.builder.build_alloca(value.get_type(), "tag_union"); + env.builder.build_store(alloca, value); + call_help(env, function, call_mode, alloca.into()); + return; + } + } + call_help(env, function, call_mode, value); } } @@ -1603,7 +1621,9 @@ fn modify_refcount_union<'a, 'ctx, 'env>( Some(function_value) => function_value, None => { let basic_type = basic_type_from_layout(env, &layout); - let function_value = build_header(env, basic_type, mode, &fn_name); + // we pass tag unions by reference for efficiency + let ptr_type = basic_type.ptr_type(AddressSpace::Generic).into(); + let function_value = build_header(env, ptr_type, mode, &fn_name); modify_refcount_union_help( env, @@ -1647,18 +1667,27 @@ fn modify_refcount_union_help<'a, 'ctx, 'env>( // Add args to scope let arg_symbol = Symbol::ARG_1; - let arg_val = fn_val.get_param_iter().next().unwrap(); + let arg_ptr = fn_val.get_param_iter().next().unwrap().into_pointer_value(); - arg_val.set_name(arg_symbol.as_str(&env.interns)); + arg_ptr.set_name(arg_symbol.as_str(&env.interns)); let parent = fn_val; let before_block = env.builder.get_insert_block().expect("to be in a function"); + let arg_val = env.builder.build_load(arg_ptr, "load_tag_union"); let wrapper_struct = arg_val.into_struct_value(); // read the tag_id - let tag_id = get_tag_id_non_recursive(env, wrapper_struct); + let tag_id_ptr = env + .builder + .build_struct_gep(arg_ptr, TAG_ID_INDEX, "tag_id_ptr") + .unwrap(); + + let tag_id = env + .builder + .build_load(tag_id_ptr, "load_tag_id") + .into_int_value(); let tag_id_u8 = env .builder @@ -1686,12 +1715,18 @@ fn modify_refcount_union_help<'a, 'ctx, 'env>( let wrapper_type = basic_type_from_layout(env, &Layout::Struct(field_layouts)); debug_assert!(wrapper_type.is_struct_type()); - let data_bytes = env + let opaque_tag_data_ptr = env .builder - .build_extract_value(wrapper_struct, TAG_DATA_INDEX, "read_tag_id") - .unwrap() - .into_struct_value(); - let wrapper_struct = cast_block_of_memory_to_tag(env.builder, data_bytes, wrapper_type); + .build_struct_gep(arg_ptr, TAG_DATA_INDEX, "tag_id_ptr") + .unwrap(); + + let cast_tag_data_pointer = env.builder.build_pointer_cast( + opaque_tag_data_ptr, + wrapper_type.ptr_type(AddressSpace::Generic), + "cast_to_concrete_tag", + ); + + // let wrapper_struct = cast_block_of_memory_to_tag(env.builder, data_bytes, wrapper_type); for (i, field_layout) in field_layouts.iter().enumerate() { if let Layout::RecursivePointer = field_layout { @@ -1699,16 +1734,18 @@ fn modify_refcount_union_help<'a, 'ctx, 'env>( } else if field_layout.contains_refcounted() { let field_ptr = env .builder - .build_extract_value(wrapper_struct, i as u32, "modify_tag_field") + .build_struct_gep(cast_tag_data_pointer, i as u32, "modify_tag_field") .unwrap(); + let field_value = env.builder.build_load(field_ptr, "field_value"); + modify_refcount_layout_help( env, parent, layout_ids, mode.to_call_mode(fn_val), when_recursive, - field_ptr, + field_value, field_layout, ); } From 28b15cdf67949402b4e0019a82526be8b4a2a824 Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 21 Oct 2021 22:57:22 +0200 Subject: [PATCH 03/19] prettier --- compiler/gen_llvm/src/llvm/build.rs | 4 ++-- compiler/gen_llvm/src/llvm/refcounting.rs | 5 +---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 80c588bfb0..1ec0d634ed 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -618,8 +618,8 @@ pub fn construct_optimization_passes<'a>( mpm.add_always_inliner_pass(); // tail-call elimination is always on - // fpm.add_instruction_combining_pass(); - // fpm.add_tail_call_elimination_pass(); + fpm.add_instruction_combining_pass(); + fpm.add_tail_call_elimination_pass(); let pmb = PassManagerBuilder::create(); match opt_level { diff --git a/compiler/gen_llvm/src/llvm/refcounting.rs b/compiler/gen_llvm/src/llvm/refcounting.rs index 0e8f275554..f183e029f6 100644 --- a/compiler/gen_llvm/src/llvm/refcounting.rs +++ b/compiler/gen_llvm/src/llvm/refcounting.rs @@ -1675,9 +1675,6 @@ fn modify_refcount_union_help<'a, 'ctx, 'env>( let before_block = env.builder.get_insert_block().expect("to be in a function"); - let arg_val = env.builder.build_load(arg_ptr, "load_tag_union"); - let wrapper_struct = arg_val.into_struct_value(); - // read the tag_id let tag_id_ptr = env .builder @@ -1717,7 +1714,7 @@ fn modify_refcount_union_help<'a, 'ctx, 'env>( debug_assert!(wrapper_type.is_struct_type()); let opaque_tag_data_ptr = env .builder - .build_struct_gep(arg_ptr, TAG_DATA_INDEX, "tag_id_ptr") + .build_struct_gep(arg_ptr, TAG_DATA_INDEX, "field_ptr") .unwrap(); let cast_tag_data_pointer = env.builder.build_pointer_cast( From 171c0836e4fffd81226bad6bdc1fd0facb1aa5ec Mon Sep 17 00:00:00 2001 From: Folkert Date: Fri, 22 Oct 2021 11:45:24 +0200 Subject: [PATCH 04/19] return tag unions by pointer --- compiler/gen_llvm/src/llvm/build.rs | 121 +++++++++++++++++++++++----- 1 file changed, 101 insertions(+), 20 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 1ec0d634ed..82dfdd2b36 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -629,7 +629,7 @@ pub fn construct_optimization_passes<'a>( OptLevel::Optimize => { pmb.set_optimization_level(OptimizationLevel::Aggressive); // this threshold seems to do what we want - pmb.set_inliner_with_threshold(0); + pmb.set_inliner_with_threshold(275); // TODO figure out which of these actually help @@ -2377,15 +2377,42 @@ pub fn build_exp_stmt<'a, 'ctx, 'env>( result } Ret(symbol) => { - let value = load_symbol(scope, symbol); + let (value, layout) = load_symbol_and_layout(scope, symbol); - if let Some(block) = env.builder.get_insert_block() { - if block.get_terminator().is_none() { - env.builder.build_return(Some(&value)); + match RocReturn::from_layout(env, layout) { + RocReturn::Return => { + if let Some(block) = env.builder.get_insert_block() { + if block.get_terminator().is_none() { + env.builder.build_return(Some(&value)); + } + } + + value + } + RocReturn::ByPointer => { + // we need to write our value into the final argument of the current function + let parameters = parent.get_params(); + let out_parameter = parameters.last().unwrap(); + debug_assert!(out_parameter.is_pointer_value()); + + env.builder + .build_store(out_parameter.into_pointer_value(), value); + + if let Some(block) = env.builder.get_insert_block() { + match block.get_terminator() { + None => { + env.builder.build_return(None); + } + Some(terminator) => { + terminator.remove_from_basic_block(); + env.builder.build_return(None); + } + } + } + + env.context.i8_type().const_zero().into() } } - - value } Switch { @@ -3106,7 +3133,6 @@ fn expose_function_to_host_help_c_abi_generic<'a, 'ctx, 'env>( c_function_name: &str, ) -> FunctionValue<'ctx> { // NOTE we ingore env.is_gen_test here - let wrapper_return_type = roc_function.get_type().get_return_type().unwrap(); let mut cc_argument_types = Vec::with_capacity_in(arguments.len(), env.arena); for layout in arguments { @@ -3116,12 +3142,19 @@ fn expose_function_to_host_help_c_abi_generic<'a, 'ctx, 'env>( // STEP 1: turn `f : a,b,c -> d` into `f : a,b,c, &d -> {}` // let mut argument_types = roc_function.get_type().get_param_types(); let mut argument_types = cc_argument_types; - let return_type = wrapper_return_type; - let output_type = return_type.ptr_type(AddressSpace::Generic); - argument_types.push(output_type.into()); + let c_function_type = match roc_function.get_type().get_return_type() { + None => { + // this function already returns by-pointer + roc_function.get_type() + } + Some(return_type) => { + let output_type = return_type.ptr_type(AddressSpace::Generic); + argument_types.push(output_type.into()); - let c_function_type = env.context.void_type().fn_type(&argument_types, false); + env.context.void_type().fn_type(&argument_types, false) + } + }; let c_function = add_func( env.module, @@ -3167,10 +3200,10 @@ fn expose_function_to_host_help_c_abi_generic<'a, 'ctx, 'env>( let arguments_for_call = &arguments_for_call.into_bump_slice(); - debug_assert_eq!(args.len(), roc_function.get_params().len()); - let call_result = { if env.is_gen_test { + debug_assert_eq!(args.len(), roc_function.get_params().len()); + let roc_wrapper_function = make_exception_catcher(env, roc_function); debug_assert_eq!( arguments_for_call.len(), @@ -3375,7 +3408,8 @@ fn expose_function_to_host_help_c_abi<'a, 'ctx, 'env>( ) .into() } else { - roc_function.get_type().get_return_type().unwrap() + // roc_function.get_type().get_return_type().unwrap() + basic_type_from_layout(env, &return_layout) }; let mut cc_argument_types = Vec::with_capacity_in(arguments.len(), env.arena); @@ -3432,10 +3466,15 @@ fn expose_function_to_host_help_c_abi<'a, 'ctx, 'env>( CCReturn::Void => { debug_assert_eq!(args.len(), roc_function.get_params().len()); } - CCReturn::ByPointer => { - args = &args[..args.len() - 1]; - debug_assert_eq!(args.len(), roc_function.get_params().len()); - } + CCReturn::ByPointer => match RocReturn::from_layout(env, &return_layout) { + RocReturn::ByPointer => { + debug_assert_eq!(args.len(), roc_function.get_params().len()); + } + RocReturn::Return => { + args = &args[..args.len() - 1]; + debug_assert_eq!(args.len(), roc_function.get_params().len()); + } + }, } let mut arguments_for_call = Vec::with_capacity_in(args.len(), env.arena); @@ -3975,7 +4014,17 @@ fn build_proc_header<'a, 'ctx, 'env>( arg_basic_types.push(arg_type); } - let fn_type = ret_type.fn_type(&arg_basic_types, false); + let fn_type = match RocReturn::from_layout(env, &proc.ret_layout) { + RocReturn::Return => ret_type.fn_type(&arg_basic_types, false), + RocReturn::ByPointer => { + println!( + "{:?} will return void instead of {:?}", + symbol, proc.ret_layout + ); + arg_basic_types.push(ret_type.ptr_type(AddressSpace::Generic).into()); + env.context.void_type().fn_type(&arg_basic_types, false) + } + }; let fn_val = add_func( env.module, @@ -4340,6 +4389,7 @@ fn roc_call_with_args<'a, 'ctx, 'env>( let fn_val = function_value_by_func_spec(env, func_spec, symbol, argument_layouts, result_layout); +<<<<<<< HEAD call_roc_function(env, fn_val, result_layout, arguments) } @@ -5690,6 +5740,37 @@ fn to_cc_type_builtin<'a, 'ctx, 'env>( } } +enum RocReturn { + /// Return as normal + Return, + /// require an extra argument, a pointer + /// where the result is written into returns void + ByPointer, +} + +impl RocReturn { + fn roc_return_by_pointer(layout: Layout) -> bool { + match layout { + Layout::Union(union_layout) => match union_layout { + UnionLayout::NonRecursive(_) => true, + _ => false, + }, + Layout::LambdaSet(lambda_set) => { + RocReturn::roc_return_by_pointer(lambda_set.runtime_representation()) + } + _ => false, + } + } + + fn from_layout<'a, 'ctx, 'env>(env: &Env<'a, 'ctx, 'env>, layout: &Layout<'a>) -> Self { + if false && Self::roc_return_by_pointer(*layout) { + RocReturn::ByPointer + } else { + RocReturn::Return + } + } +} + enum CCReturn { /// Return as normal Return, From 2ff3a97adaa85bf2bcc6b02e87d8c4d8c9d401f2 Mon Sep 17 00:00:00 2001 From: Folkert Date: Fri, 22 Oct 2021 13:24:18 +0200 Subject: [PATCH 05/19] re-implement roc returning by pointer --- compiler/gen_llvm/src/llvm/build.rs | 58 ++++++++++++++++++++--------- 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 82dfdd2b36..9140b147a9 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -4389,28 +4389,55 @@ fn roc_call_with_args<'a, 'ctx, 'env>( let fn_val = function_value_by_func_spec(env, func_spec, symbol, argument_layouts, result_layout); -<<<<<<< HEAD call_roc_function(env, fn_val, result_layout, arguments) } fn call_roc_function<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, roc_function: FunctionValue<'ctx>, - _result_layout: &Layout<'a>, + result_layout: &Layout<'a>, arguments: &[BasicValueEnum<'ctx>], ) -> BasicValueEnum<'ctx> { - let call = env.builder.build_call(roc_function, arguments, "call"); + match RocReturn::from_layout(env, result_layout) { + RocReturn::Return => { + debug_assert_eq!( + roc_function.get_type().get_param_types().len(), + arguments.len() + ); + 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); + // 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 {:?}", - roc_function.get_name() - ) - }) + call.try_as_basic_value().left().unwrap_or_else(|| { + panic!( + "LLVM error: Invalid call by name for name {:?}", + roc_function.get_name() + ) + }) + } + RocReturn::ByPointer => { + let mut arguments = Vec::from_iter_in(arguments.iter().copied(), env.arena); + + let result_type = basic_type_from_layout(env, result_layout); + let result_alloca = env.builder.build_alloca(result_type, "result_value"); + + arguments.push(result_alloca.into()); + + debug_assert_eq!( + roc_function.get_type().get_param_types().len(), + arguments.len() + ); + 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); + + env.builder.build_load(result_alloca, "load_result") + } + } } /// Translates a target_lexicon::Triple to a LLVM calling convention u32 @@ -5751,10 +5778,7 @@ enum RocReturn { impl RocReturn { fn roc_return_by_pointer(layout: Layout) -> bool { match layout { - Layout::Union(union_layout) => match union_layout { - UnionLayout::NonRecursive(_) => true, - _ => false, - }, + Layout::Union(union_layout) => matches!(union_layout, UnionLayout::NonRecursive(_)), Layout::LambdaSet(lambda_set) => { RocReturn::roc_return_by_pointer(lambda_set.runtime_representation()) } @@ -5762,7 +5786,7 @@ impl RocReturn { } } - fn from_layout<'a, 'ctx, 'env>(env: &Env<'a, 'ctx, 'env>, layout: &Layout<'a>) -> Self { + fn from_layout<'a, 'ctx, 'env>(_env: &Env<'a, 'ctx, 'env>, layout: &Layout<'a>) -> Self { if false && Self::roc_return_by_pointer(*layout) { RocReturn::ByPointer } else { From 1d1bd3d0513f3ef810aa54e81f724ea62242a597 Mon Sep 17 00:00:00 2001 From: Folkert Date: Fri, 22 Oct 2021 14:54:15 +0200 Subject: [PATCH 06/19] working, but generates more code --- compiler/gen_llvm/src/llvm/build.rs | 62 +++++++++++++++++++---------- 1 file changed, 42 insertions(+), 20 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 9140b147a9..44c3565104 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -4398,26 +4398,30 @@ fn call_roc_function<'a, 'ctx, 'env>( result_layout: &Layout<'a>, arguments: &[BasicValueEnum<'ctx>], ) -> BasicValueEnum<'ctx> { + let pass_by_pointer = roc_function.get_type().get_param_types().len() == arguments.len() + 1; + match RocReturn::from_layout(env, result_layout) { - RocReturn::Return => { - debug_assert_eq!( - roc_function.get_type().get_param_types().len(), - arguments.len() - ); - 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 {:?}", - roc_function.get_name() - ) - }) - } RocReturn::ByPointer => { + if !pass_by_pointer { + let mut arguments = Vec::from_iter_in(arguments.iter().copied(), env.arena); + + let result_type = basic_type_from_layout(env, result_layout); + let result_alloca = env.builder.build_alloca(result_type, "result_value"); + + //arguments.push(result_alloca.into()); + + debug_assert_eq!( + roc_function.get_type().get_param_types().len(), + arguments.len() + ); + 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); + + return env.builder.build_load(result_alloca, "load_result"); + } let mut arguments = Vec::from_iter_in(arguments.iter().copied(), env.arena); let result_type = basic_type_from_layout(env, result_layout); @@ -4437,6 +4441,24 @@ fn call_roc_function<'a, 'ctx, 'env>( env.builder.build_load(result_alloca, "load_result") } + RocReturn::Return => { + debug_assert_eq!( + roc_function.get_type().get_param_types().len(), + arguments.len() + ); + 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 {:?}", + roc_function.get_name() + ) + }) + } } } @@ -5778,7 +5800,7 @@ enum RocReturn { impl RocReturn { fn roc_return_by_pointer(layout: Layout) -> bool { match layout { - Layout::Union(union_layout) => matches!(union_layout, UnionLayout::NonRecursive(_)), + Layout::Union(UnionLayout::NonRecursive(_)) => true, Layout::LambdaSet(lambda_set) => { RocReturn::roc_return_by_pointer(lambda_set.runtime_representation()) } @@ -5787,7 +5809,7 @@ impl RocReturn { } fn from_layout<'a, 'ctx, 'env>(_env: &Env<'a, 'ctx, 'env>, layout: &Layout<'a>) -> Self { - if false && Self::roc_return_by_pointer(*layout) { + if Self::roc_return_by_pointer(*layout) { RocReturn::ByPointer } else { RocReturn::Return From e378f0d2f997ff66b5af95f01acc961e223ec650 Mon Sep 17 00:00:00 2001 From: Folkert Date: Fri, 5 Nov 2021 10:35:17 +0100 Subject: [PATCH 07/19] fix tags tests --- compiler/gen_llvm/src/llvm/bitcode.rs | 20 +++---- compiler/gen_llvm/src/llvm/build.rs | 82 +++++++++++++++------------ 2 files changed, 54 insertions(+), 48 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/bitcode.rs b/compiler/gen_llvm/src/llvm/bitcode.rs index 52b604dff3..5116f5a033 100644 --- a/compiler/gen_llvm/src/llvm/bitcode.rs +++ b/compiler/gen_llvm/src/llvm/bitcode.rs @@ -191,6 +191,7 @@ pub fn build_transform_caller<'a, 'ctx, 'env>( function: FunctionValue<'ctx>, closure_data_layout: LambdaSet<'a>, argument_layouts: &[Layout<'a>], + result_layout: Layout<'a>, ) -> FunctionValue<'ctx> { let fn_name: &str = &format!( "{}_zig_function_caller", @@ -204,6 +205,7 @@ pub fn build_transform_caller<'a, 'ctx, 'env>( function, closure_data_layout, argument_layouts, + result_layout, fn_name, ), } @@ -214,6 +216,7 @@ fn build_transform_caller_help<'a, 'ctx, 'env>( roc_function: FunctionValue<'ctx>, closure_data_layout: LambdaSet<'a>, argument_layouts: &[Layout<'a>], + result_layout: Layout<'a>, fn_name: &str, ) -> FunctionValue<'ctx> { debug_assert!(argument_layouts.len() <= 7); @@ -288,17 +291,12 @@ fn build_transform_caller_help<'a, 'ctx, 'env>( } } - let call = { - env.builder - .build_call(roc_function, arguments_cast.as_slice(), "tmp") - }; - - call.set_call_convention(FAST_CALL_CONV); - - let result = call - .try_as_basic_value() - .left() - .unwrap_or_else(|| panic!("LLVM error: Invalid call by pointer.")); + let result = crate::llvm::build::call_roc_function( + env, + roc_function, + &result_layout, + arguments_cast.as_slice(), + ); let result_u8_ptr = function_value .get_nth_param(argument_layouts.len() as u32 + 1) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index ed17333c28..242695fa0e 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -3216,7 +3216,7 @@ fn expose_function_to_host_help_c_abi_generic<'a, 'ctx, 'env>( if env.is_gen_test { debug_assert_eq!(args.len(), roc_function.get_params().len()); - let roc_wrapper_function = make_exception_catcher(env, roc_function); + let roc_wrapper_function = make_exception_catcher(env, roc_function, return_layout); debug_assert_eq!( arguments_for_call.len(), roc_wrapper_function.get_params().len() @@ -3261,7 +3261,7 @@ fn expose_function_to_host_help_c_abi_gen_test<'a, 'ctx, 'env>( let wrapper_return_type = context.struct_type( &[ context.i64_type().into(), - roc_function.get_type().get_return_type().unwrap(), + basic_type_from_layout(env, &return_layout), ], false, ); @@ -3326,18 +3326,14 @@ fn expose_function_to_host_help_c_abi_gen_test<'a, 'ctx, 'env>( let arguments_for_call = &arguments_for_call.into_bump_slice(); let call_result = { - let roc_wrapper_function = make_exception_catcher(env, roc_function); - debug_assert_eq!( - arguments_for_call.len(), - roc_wrapper_function.get_params().len() - ); + let roc_wrapper_function = make_exception_catcher(env, roc_function, return_layout); builder.position_at_end(entry); call_roc_function( env, roc_wrapper_function, - &return_layout, + &Layout::Struct(&[Layout::Builtin(Builtin::Int64), return_layout]), arguments_for_call, ) }; @@ -3574,21 +3570,18 @@ pub fn get_sjlj_buffer<'a, 'ctx, 'env>(env: &Env<'a, 'ctx, 'env>) -> PointerValu .into_pointer_value() } -fn set_jump_and_catch_long_jump<'a, 'ctx, 'env, F, T>( +fn set_jump_and_catch_long_jump<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, parent: FunctionValue<'ctx>, - function: F, - calling_convention: u32, + roc_function: FunctionValue<'ctx>, arguments: &[BasicValueEnum<'ctx>], - return_type: T, -) -> BasicValueEnum<'ctx> -where - T: inkwell::types::BasicType<'ctx>, - F: Into>, -{ + return_layout: Layout<'a>, +) -> BasicValueEnum<'ctx> { let context = env.context; let builder = env.builder; + let return_type = basic_type_from_layout(env, &return_layout); + let call_result_type = context.struct_type( &[context.i64_type().into(), return_type.as_basic_type_enum()], false, @@ -3661,11 +3654,7 @@ where { builder.position_at_end(then_block); - let call = env.builder.build_call(function, arguments, "call_function"); - - call.set_call_convention(calling_convention); - - let call_result = call.try_as_basic_value().left().unwrap(); + let call_result = call_roc_function(env, roc_function, &return_layout, arguments); let return_value = make_good_roc_result(env, call_result); @@ -3738,10 +3727,12 @@ where fn make_exception_catcher<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, roc_function: FunctionValue<'ctx>, + return_layout: Layout<'a>, ) -> FunctionValue<'ctx> { let wrapper_function_name = format!("{}_catcher", roc_function.get_name().to_str().unwrap()); - let function_value = make_exception_catching_wrapper(env, roc_function, &wrapper_function_name); + let function_value = + make_exception_catching_wrapper(env, roc_function, return_layout, &wrapper_function_name); function_value.set_linkage(Linkage::Internal); @@ -3775,6 +3766,7 @@ fn make_good_roc_result<'a, 'ctx, 'env>( fn make_exception_catching_wrapper<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, roc_function: FunctionValue<'ctx>, + return_layout: Layout<'a>, wrapper_function_name: &str, ) -> FunctionValue<'ctx> { // build the C calling convention wrapper @@ -3783,12 +3775,20 @@ fn make_exception_catching_wrapper<'a, 'ctx, 'env>( let builder = env.builder; let roc_function_type = roc_function.get_type(); - let argument_types = roc_function_type.get_param_types(); + let argument_types = match RocReturn::from_layout(env, &return_layout) { + RocReturn::Return => roc_function_type.get_param_types(), + RocReturn::ByPointer => { + let mut types = roc_function_type.get_param_types(); + types.remove(0); + + types + } + }; let wrapper_return_type = context.struct_type( &[ context.i64_type().into(), - roc_function.get_type().get_return_type().unwrap(), + basic_type_from_layout(env, &return_layout), ], false, ); @@ -3825,9 +3825,8 @@ fn make_exception_catching_wrapper<'a, 'ctx, 'env>( env, wrapper_function, roc_function, - roc_function.get_call_conventions(), &arguments, - roc_function_type.get_return_type().unwrap(), + return_layout, ); builder.build_return(Some(&result)); @@ -4029,10 +4028,7 @@ fn build_proc_header<'a, 'ctx, 'env>( let fn_type = match RocReturn::from_layout(env, &proc.ret_layout) { RocReturn::Return => ret_type.fn_type(&arg_basic_types, false), RocReturn::ByPointer => { - println!( - "{:?} will return void instead of {:?}", - symbol, proc.ret_layout - ); + // println!( "{:?} will return void instead of {:?}", symbol, proc.ret_layout); arg_basic_types.push(ret_type.ptr_type(AddressSpace::Generic).into()); env.context.void_type().fn_type(&arg_basic_types, false) } @@ -4141,9 +4137,8 @@ pub fn build_closure_caller<'a, 'ctx, 'env>( env, function_value, evaluator, - evaluator.get_call_conventions(), &evaluator_arguments, - result_type, + *return_layout, ) } else { call_roc_function(env, evaluator, return_layout, &evaluator_arguments) @@ -4404,7 +4399,7 @@ fn roc_call_with_args<'a, 'ctx, 'env>( call_roc_function(env, fn_val, result_layout, arguments) } -fn call_roc_function<'a, 'ctx, 'env>( +pub fn call_roc_function<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, roc_function: FunctionValue<'ctx>, result_layout: &Layout<'a>, @@ -4510,6 +4505,7 @@ fn roc_function_call<'a, 'ctx, 'env>( lambda_set: LambdaSet<'a>, closure_data_is_owned: bool, argument_layouts: &[Layout<'a>], + result_layout: Layout<'a>, ) -> RocFunctionCall<'ctx> { use crate::llvm::bitcode::{build_inc_n_wrapper, build_transform_caller}; @@ -4518,9 +4514,10 @@ fn roc_function_call<'a, 'ctx, 'env>( .build_alloca(closure_data.get_type(), "closure_data_ptr"); env.builder.build_store(closure_data_ptr, closure_data); - let stepper_caller = build_transform_caller(env, transform, lambda_set, argument_layouts) - .as_global_value() - .as_pointer_value(); + let stepper_caller = + build_transform_caller(env, transform, lambda_set, argument_layouts, result_layout) + .as_global_value() + .as_pointer_value(); let inc_closure_data = build_inc_n_wrapper(env, layout_ids, &lambda_set.runtime_representation()) @@ -4593,6 +4590,7 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( closure_layout, function_owns_closure_data, argument_layouts, + *result_layout, ); crate::llvm::build_list::list_walk_generic( @@ -4634,6 +4632,7 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( closure_layout, function_owns_closure_data, argument_layouts, + **result_layout, ); list_map(env, roc_function_call, list, element_layout, result_layout) @@ -4663,6 +4662,7 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( closure_layout, function_owns_closure_data, argument_layouts, + **result_layout, ); list_map2( @@ -4706,6 +4706,7 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( closure_layout, function_owns_closure_data, argument_layouts, + **result_layout, ); list_map3( @@ -4764,6 +4765,7 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( closure_layout, function_owns_closure_data, argument_layouts, + **result_layout, ); list_map4( @@ -4810,6 +4812,7 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( closure_layout, function_owns_closure_data, argument_layouts, + **result_layout, ); list_map_with_index(env, roc_function_call, list, element_layout, result_layout) @@ -4836,6 +4839,7 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( closure_layout, function_owns_closure_data, argument_layouts, + *result_layout, ); list_keep_if(env, layout_ids, roc_function_call, list, element_layout) @@ -4866,6 +4870,7 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( closure_layout, function_owns_closure_data, argument_layouts, + *result_layout, ); list_keep_oks( @@ -4906,6 +4911,7 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( closure_layout, function_owns_closure_data, argument_layouts, + *result_layout, ); list_keep_errs( @@ -4958,6 +4964,7 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( closure_layout, function_owns_closure_data, argument_layouts, + *result_layout, ); list_sort_with( @@ -4993,6 +5000,7 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( closure_layout, function_owns_closure_data, argument_layouts, + *result_layout, ); dict_walk( From 51756cbc894eef2455cf9b63c34f3ce5726d8e0b Mon Sep 17 00:00:00 2001 From: Folkert Date: Fri, 5 Nov 2021 11:21:06 +0100 Subject: [PATCH 08/19] fix uninitialized alloca --- compiler/gen_llvm/src/llvm/build.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 242695fa0e..52d18411ed 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -3499,7 +3499,7 @@ fn expose_function_to_host_help_c_abi<'a, 'ctx, 'env>( } } - let arguments_for_call = &arguments_for_call.into_bump_slice(); + let arguments_for_call = arguments_for_call.into_bump_slice(); let call_result = call_roc_function(env, roc_function, &return_layout, arguments_for_call); @@ -4411,11 +4411,12 @@ pub fn call_roc_function<'a, 'ctx, 'env>( RocReturn::ByPointer => { if !pass_by_pointer { let mut arguments = Vec::from_iter_in(arguments.iter().copied(), env.arena); + arguments.pop(); let result_type = basic_type_from_layout(env, result_layout); let result_alloca = env.builder.build_alloca(result_type, "result_value"); - //arguments.push(result_alloca.into()); + arguments.push(result_alloca.into()); debug_assert_eq!( roc_function.get_type().get_param_types().len(), From 5cd232816b9ee4635cd1fa6a9cb7ac076f4271d7 Mon Sep 17 00:00:00 2001 From: Folkert Date: Fri, 5 Nov 2021 21:30:20 +0100 Subject: [PATCH 09/19] waypoint --- cli/src/build.rs | 2 +- compiler/gen_llvm/src/llvm/bitcode.rs | 72 +++--- compiler/gen_llvm/src/llvm/build.rs | 269 +++++++++++++++------- compiler/gen_llvm/src/llvm/convert.rs | 60 +++++ compiler/gen_llvm/src/llvm/refcounting.rs | 33 +-- compiler/mono/src/layout.rs | 50 +++- compiler/test_gen/src/gen_tags.rs | 3 +- compiler/test_gen/src/helpers/eval.rs | 4 + 8 files changed, 337 insertions(+), 156 deletions(-) diff --git a/cli/src/build.rs b/cli/src/build.rs index 39766a9e67..4f500974e9 100644 --- a/cli/src/build.rs +++ b/cli/src/build.rs @@ -255,7 +255,7 @@ pub fn build_file<'a>( link( target, binary_path.clone(), - &inputs, + &inputs, link_type ) .map_err(|_| { diff --git a/compiler/gen_llvm/src/llvm/bitcode.rs b/compiler/gen_llvm/src/llvm/bitcode.rs index 5116f5a033..d006b5b508 100644 --- a/compiler/gen_llvm/src/llvm/bitcode.rs +++ b/compiler/gen_llvm/src/llvm/bitcode.rs @@ -1,7 +1,7 @@ /// Helpers for interacting with the zig that generates bitcode use crate::debug_info_init; use crate::llvm::build::{struct_from_fields, Env, C_CALL_CONV, FAST_CALL_CONV, TAG_DATA_INDEX}; -use crate::llvm::convert::basic_type_from_layout; +use crate::llvm::convert::{basic_type_from_layout, basic_type_from_layout_1}; use crate::llvm::refcounting::{ decrement_refcount_layout, increment_n_refcount_layout, increment_refcount_layout, }; @@ -128,20 +128,19 @@ fn build_has_tag_id_help<'a, 'ctx, 'env>( [tag_id, tag_value_ptr] => { let tag_type = basic_type_from_layout(env, &Layout::Union(union_layout)); - let argument_cast = env - .builder - .build_bitcast( - *tag_value_ptr, - tag_type.ptr_type(AddressSpace::Generic), - "load_opaque", - ) - .into_pointer_value(); - - let tag_value = env.builder.build_load(argument_cast, "get_value"); + let tag_value = env.builder.build_pointer_cast( + tag_value_ptr.into_pointer_value(), + tag_type.ptr_type(AddressSpace::Generic), + "load_opaque_get_tag_id", + ); let actual_tag_id = { - let tag_id_i64 = - crate::llvm::build::get_tag_id(env, function_value, &union_layout, tag_value); + let tag_id_i64 = crate::llvm::build::get_tag_id( + env, + function_value, + &union_layout, + tag_value.into(), + ); env.builder .build_int_cast(tag_id_i64, env.context.i16_type(), "to_i16") @@ -263,12 +262,22 @@ fn build_transform_caller_help<'a, 'ctx, 'env>( for (argument_ptr, layout) in arguments.iter().zip(argument_layouts) { let basic_type = basic_type_from_layout(env, layout).ptr_type(AddressSpace::Generic); - let argument_cast = env - .builder - .build_bitcast(*argument_ptr, basic_type, "load_opaque") - .into_pointer_value(); + let argument = if layout.is_passed_by_reference() { + env.builder + .build_pointer_cast( + argument_ptr.into_pointer_value(), + basic_type, + "cast_ptr_to_tag", + ) + .into() + } else { + let argument_cast = env + .builder + .build_bitcast(*argument_ptr, basic_type, "load_opaque_1") + .into_pointer_value(); - let argument = env.builder.build_load(argument_cast, "load_opaque"); + env.builder.build_load(argument_cast, "load_opaque_2") + }; arguments_cast.push(argument); } @@ -300,17 +309,10 @@ fn build_transform_caller_help<'a, 'ctx, 'env>( let result_u8_ptr = function_value .get_nth_param(argument_layouts.len() as u32 + 1) - .unwrap(); - let result_ptr = env - .builder - .build_bitcast( - result_u8_ptr, - result.get_type().ptr_type(AddressSpace::Generic), - "write_result", - ) + .unwrap() .into_pointer_value(); - env.builder.build_store(result_ptr, result); + crate::llvm::build::store_roc_value_opaque(env, result_layout, result_u8_ptr, result); env.builder.build_return(None); env.builder.position_at_end(block); @@ -412,12 +414,18 @@ fn build_rc_wrapper<'a, 'ctx, 'env>( let value_type = basic_type_from_layout(env, layout).ptr_type(AddressSpace::Generic); - let value_cast = env - .builder - .build_bitcast(value_ptr, value_type, "load_opaque") - .into_pointer_value(); + let value = if layout.is_passed_by_reference() { + env.builder + .build_pointer_cast(value_ptr, value_type, "cast_ptr_to_tag") + .into() + } else { + let value_cast = env + .builder + .build_bitcast(value_ptr, value_type, "load_opaque") + .into_pointer_value(); - let value = env.builder.build_load(value_cast, "load_opaque"); + env.builder.build_load(value_cast, "load_opaque") + }; match rc_operation { Mode::Inc => { diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 52d18411ed..a6fe38761a 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -21,7 +21,8 @@ use crate::llvm::build_str::{ }; use crate::llvm::compare::{generic_eq, generic_neq}; use crate::llvm::convert::{ - basic_type_from_builtin, basic_type_from_layout, block_of_memory_slices, ptr_int, + basic_type_from_builtin, basic_type_from_layout, basic_type_from_layout_1, + block_of_memory_slices, ptr_int, }; use crate::llvm::refcounting::{ build_reset, decrement_refcount_layout, increment_refcount_layout, PointerToRefcount, @@ -618,8 +619,8 @@ pub fn construct_optimization_passes<'a>( mpm.add_always_inliner_pass(); // tail-call elimination is always on - fpm.add_instruction_combining_pass(); - fpm.add_tail_call_elimination_pass(); + // fpm.add_instruction_combining_pass(); + // fpm.add_tail_call_elimination_pass(); let pmb = PassManagerBuilder::create(); match opt_level { @@ -1235,32 +1236,21 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( match union_layout { UnionLayout::NonRecursive(tag_layouts) => { - debug_assert!(argument.is_struct_value()); + debug_assert!(argument.is_pointer_value()); + let field_layouts = tag_layouts[*tag_id as usize]; - let struct_layout = Layout::Struct(field_layouts); - let struct_type = basic_type_from_layout(env, &struct_layout); + let tag_id_type = + basic_type_from_layout(env, &union_layout.tag_id_layout()).into_int_type(); - let argument_pointer = builder.build_alloca(argument.get_type(), "cast_alloca"); - builder.build_store(argument_pointer, argument); - - let struct_ptr = builder.build_pointer_cast( - argument_pointer, - struct_type.ptr_type(AddressSpace::Generic), - "foobar", - ); - - // let struct_value = access_index_struct_value( - // builder, - // argument.into_struct_value(), - // struct_type.into_struct_type(), - // ); - - let result = builder - .build_struct_gep(struct_ptr, *index as u32, "") - .expect("desired field did not decode"); - - builder.build_load(result, "foobarbaz") + lookup_at_index_ptr2( + env, + union_layout, + tag_id_type, + field_layouts, + *index as usize, + argument.into_pointer_value(), + ) } UnionLayout::Recursive(tag_layouts) => { debug_assert!(argument.is_pointer_value()); @@ -1555,10 +1545,13 @@ pub fn build_tag<'a, 'ctx, 'env>( .builder .build_struct_gep(struct_ptr, index, "get_tag_field_ptr") .unwrap(); - env.builder.build_store(ptr, field_val); + + let field_layout = tag_field_layouts[index as usize]; + store_roc_value(env, field_layout, ptr, field_val); } - env.builder.build_load(result_alloca, "load_result") + // env.builder.build_load(result_alloca, "load_result") + result_alloca.into() } UnionLayout::Recursive(tags) => { debug_assert!(union_size > 1); @@ -1866,9 +1859,10 @@ pub fn get_tag_id<'a, 'ctx, 'env>( match union_layout { UnionLayout::NonRecursive(_) => { - let tag = argument.into_struct_value(); + debug_assert!(argument.is_pointer_value(), "{:?}", argument); - get_tag_id_non_recursive(env, tag) + let argument_ptr = argument.into_pointer_value(); + get_tag_id_wrapped(env, argument_ptr) } UnionLayout::Recursive(_) => { let argument_ptr = argument.into_pointer_value(); @@ -2008,7 +2002,26 @@ fn lookup_at_index_ptr2<'a, 'ctx, 'env>( .build_struct_gep(data_ptr, index as u32, "at_index_struct_gep") .unwrap(); - let result = builder.build_load(elem_ptr, "load_at_index_ptr"); + let field_layout = field_layouts[index]; + let result = if field_layout.is_passed_by_reference() { + let field_type = basic_type_from_layout(env, &field_layout); + + let align_bytes = field_layout.alignment_bytes(env.ptr_bytes); + let alloca = env.builder.build_alloca(field_type, "copied_tag"); + if align_bytes > 0 { + let size = env + .ptr_int() + .const_int(field_layout.stack_size(env.ptr_bytes) as u64, false); + + env.builder + .build_memcpy(alloca, align_bytes, elem_ptr, align_bytes, size) + .unwrap(); + } + + alloca.into() + } else { + builder.build_load(elem_ptr, "load_at_index_ptr") + }; if let Some(Layout::RecursivePointer) = field_layouts.get(index as usize) { // a recursive field is stored as a `i64*`, to use it we must cast it to @@ -2155,17 +2168,12 @@ pub fn allocate_with_refcount_help<'a, 'ctx, 'env>( let ptr_type = value_type.ptr_type(AddressSpace::Generic); unsafe { - builder - .build_bitcast( - env.builder.build_in_bounds_gep( - as_usize_ptr, - &[index_intvalue], - "get_data_ptr", - ), - ptr_type, - "alloc_cast_to_desired", - ) - .into_pointer_value() + builder.build_pointer_cast( + env.builder + .build_in_bounds_gep(as_usize_ptr, &[index_intvalue], "get_data_ptr"), + ptr_type, + "alloc_cast_to_desired", + ) } }; @@ -2191,13 +2199,13 @@ pub fn allocate_with_refcount_help<'a, 'ctx, 'env>( fn list_literal<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, scope: &Scope<'a, 'ctx>, - elem_layout: &Layout<'a>, + element_layout: &Layout<'a>, elems: &[ListLiteralElement], ) -> BasicValueEnum<'ctx> { let ctx = env.context; let builder = env.builder; - let element_type = basic_type_from_layout(env, elem_layout); + let element_type = basic_type_from_layout(env, element_layout); let list_length = elems.len(); let list_length_intval = env.ptr_int().const_int(list_length as _, false); @@ -2207,9 +2215,9 @@ fn list_literal<'a, 'ctx, 'env>( // if element_type.is_int_type() { if false { let element_type = element_type.into_int_type(); - let element_width = elem_layout.stack_size(env.ptr_bytes); + let element_width = element_layout.stack_size(env.ptr_bytes); let size = list_length * element_width as usize; - let alignment = elem_layout + let alignment = element_layout .alignment_bytes(env.ptr_bytes) .max(env.ptr_bytes); @@ -2241,7 +2249,7 @@ fn list_literal<'a, 'ctx, 'env>( for (index, element) in elems.iter().enumerate() { match element { ListLiteralElement::Literal(literal) => { - let val = build_exp_literal(env, elem_layout, literal); + let val = build_exp_literal(env, element_layout, literal); global_elements.push(val.into_int_value()); } ListLiteralElement::Symbol(symbol) => { @@ -2296,7 +2304,7 @@ fn list_literal<'a, 'ctx, 'env>( super::build_list::store_list(env, ptr, list_length_intval) } else { // some of our elements are non-constant, so we must allocate space on the heap - let ptr = allocate_list(env, elem_layout, list_length_intval); + let ptr = allocate_list(env, element_layout, list_length_intval); // then, copy the relevant segment from the constant section into the heap env.builder @@ -2320,26 +2328,69 @@ fn list_literal<'a, 'ctx, 'env>( super::build_list::store_list(env, ptr, list_length_intval) } } else { - let ptr = allocate_list(env, elem_layout, list_length_intval); + let ptr = allocate_list(env, element_layout, list_length_intval); // Copy the elements from the list literal into the array for (index, element) in elems.iter().enumerate() { let val = match element { ListLiteralElement::Literal(literal) => { - build_exp_literal(env, elem_layout, literal) + build_exp_literal(env, element_layout, literal) } ListLiteralElement::Symbol(symbol) => load_symbol(scope, symbol), }; let index_val = ctx.i64_type().const_int(index as u64, false); let elem_ptr = unsafe { builder.build_in_bounds_gep(ptr, &[index_val], "index") }; - builder.build_store(elem_ptr, val); + store_roc_value(env, *element_layout, elem_ptr, val); } super::build_list::store_list(env, ptr, list_length_intval) } } +pub fn store_roc_value_opaque<'a, 'ctx, 'env>( + env: &Env<'a, 'ctx, 'env>, + layout: Layout<'a>, + opaque_destination: PointerValue<'ctx>, + value: BasicValueEnum<'ctx>, +) { + let target_type = basic_type_from_layout(env, &layout).ptr_type(AddressSpace::Generic); + let destination = + env.builder + .build_pointer_cast(opaque_destination, target_type, "store_roc_value_opaque"); + + store_roc_value(env, layout, destination, value) +} + +pub fn store_roc_value<'a, 'ctx, 'env>( + env: &Env<'a, 'ctx, 'env>, + layout: Layout<'a>, + destination: PointerValue<'ctx>, + value: BasicValueEnum<'ctx>, +) { + if layout.is_passed_by_reference() { + let align_bytes = layout.alignment_bytes(env.ptr_bytes); + + if align_bytes > 0 { + let size = env + .ptr_int() + .const_int(layout.stack_size(env.ptr_bytes) as u64, false); + + env.builder + .build_memcpy( + destination, + align_bytes, + value.into_pointer_value(), + align_bytes, + size, + ) + .unwrap(); + } + } else { + env.builder.build_store(destination, value); + } +} + pub fn build_exp_stmt<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, layout_ids: &mut LayoutIds<'a>, @@ -2415,8 +2466,7 @@ pub fn build_exp_stmt<'a, 'ctx, 'env>( let out_parameter = parameters.last().unwrap(); debug_assert!(out_parameter.is_pointer_value()); - env.builder - .build_store(out_parameter.into_pointer_value(), value); + store_roc_value(env, *layout, out_parameter.into_pointer_value(), value); if let Some(block) = env.builder.get_insert_block() { match block.get_terminator() { @@ -3656,7 +3706,7 @@ fn set_jump_and_catch_long_jump<'a, 'ctx, 'env>( let call_result = call_roc_function(env, roc_function, &return_layout, arguments); - let return_value = make_good_roc_result(env, call_result); + let return_value = make_good_roc_result(env, return_layout, call_result); builder.build_store(result_alloca, return_value); @@ -3721,7 +3771,7 @@ fn set_jump_and_catch_long_jump<'a, 'ctx, 'env>( env.builder.position_at_end(cont_block); - builder.build_load(result_alloca, "load_result") + builder.build_load(result_alloca, "set_jump_and_catch_long_jump_load_result") } fn make_exception_catcher<'a, 'ctx, 'env>( @@ -3741,12 +3791,13 @@ fn make_exception_catcher<'a, 'ctx, 'env>( fn make_good_roc_result<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, + return_layout: Layout<'a>, return_value: BasicValueEnum<'ctx>, ) -> BasicValueEnum<'ctx> { let context = env.context; let builder = env.builder; - let content_type = return_value.get_type(); + let content_type = basic_type_from_layout(env, &return_layout); let wrapper_return_type = context.struct_type(&[context.i64_type().into(), content_type], false); @@ -3756,9 +3807,19 @@ fn make_good_roc_result<'a, 'ctx, 'env>( .build_insert_value(v1, context.i64_type().const_zero(), 0, "set_no_error") .unwrap(); - let v3 = builder - .build_insert_value(v2, return_value, 1, "set_call_result") - .unwrap(); + let v3 = if return_layout.is_passed_by_reference() { + let loaded = env.builder.build_load( + return_value.into_pointer_value(), + "load_call_result_passed_by_ptr", + ); + builder + .build_insert_value(v2, loaded, 1, "set_call_result") + .unwrap() + } else { + builder + .build_insert_value(v2, return_value, 1, "set_call_result") + .unwrap() + }; v3.into_struct_value().into() } @@ -3971,6 +4032,8 @@ fn build_procedures_help<'a, 'ctx, 'env>( app_ll_file, ); } else { + env.module.print_to_stderr(); + panic!( "The preceding code was from {:?}, which failed LLVM verification in {} build.", fn_val.get_name().to_str().unwrap(), @@ -4020,7 +4083,7 @@ fn build_proc_header<'a, 'ctx, 'env>( let mut arg_basic_types = Vec::with_capacity_in(args.len(), arena); for (layout, _) in args.iter() { - let arg_type = basic_type_from_layout(env, layout); + let arg_type = basic_type_from_layout_1(env, layout); arg_basic_types.push(arg_type); } @@ -4132,19 +4195,41 @@ pub fn build_closure_caller<'a, 'ctx, 'env>( } } - let call_result = if env.is_gen_test { - set_jump_and_catch_long_jump( + if env.is_gen_test { + let call_result = set_jump_and_catch_long_jump( env, function_value, evaluator, &evaluator_arguments, *return_layout, - ) - } else { - call_roc_function(env, evaluator, return_layout, &evaluator_arguments) - }; + ); - builder.build_store(output, call_result); + builder.build_store(output, call_result); + } else { + let call_result = call_roc_function(env, evaluator, return_layout, &evaluator_arguments); + + if return_layout.is_passed_by_reference() { + let align_bytes = return_layout.alignment_bytes(env.ptr_bytes); + + if align_bytes > 0 { + let size = env + .ptr_int() + .const_int(return_layout.stack_size(env.ptr_bytes) as u64, false); + + env.builder + .build_memcpy( + output, + align_bytes, + call_result.into_pointer_value(), + align_bytes, + size, + ) + .unwrap(); + } + } else { + builder.build_store(output, call_result); + } + }; builder.build_return(None); @@ -4408,29 +4493,10 @@ pub fn call_roc_function<'a, 'ctx, 'env>( let pass_by_pointer = roc_function.get_type().get_param_types().len() == arguments.len() + 1; match RocReturn::from_layout(env, result_layout) { - RocReturn::ByPointer => { - if !pass_by_pointer { - let mut arguments = Vec::from_iter_in(arguments.iter().copied(), env.arena); - arguments.pop(); - - let result_type = basic_type_from_layout(env, result_layout); - let result_alloca = env.builder.build_alloca(result_type, "result_value"); - - arguments.push(result_alloca.into()); - - debug_assert_eq!( - roc_function.get_type().get_param_types().len(), - arguments.len() - ); - 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); - - return env.builder.build_load(result_alloca, "load_result"); - } + RocReturn::ByPointer if !pass_by_pointer => { + // WARNING this is a hack!! let mut arguments = Vec::from_iter_in(arguments.iter().copied(), env.arena); + arguments.pop(); let result_type = basic_type_from_layout(env, result_layout); let result_alloca = env.builder.build_alloca(result_type, "result_value"); @@ -4449,6 +4515,31 @@ pub fn call_roc_function<'a, 'ctx, 'env>( env.builder.build_load(result_alloca, "load_result") } + RocReturn::ByPointer => { + let mut arguments = Vec::from_iter_in(arguments.iter().copied(), env.arena); + + let result_type = basic_type_from_layout(env, result_layout); + let result_alloca = env.builder.build_alloca(result_type, "result_value"); + + arguments.push(result_alloca.into()); + + debug_assert_eq!( + roc_function.get_type().get_param_types().len(), + arguments.len() + ); + 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); + + if result_layout.is_passed_by_reference() { + result_alloca.into() + } else { + env.builder + .build_load(result_alloca, "return_by_pointer_load_result") + } + } RocReturn::Return => { debug_assert_eq!( roc_function.get_type().get_param_types().len(), diff --git a/compiler/gen_llvm/src/llvm/convert.rs b/compiler/gen_llvm/src/llvm/convert.rs index d32216c10a..559945b083 100644 --- a/compiler/gen_llvm/src/llvm/convert.rs +++ b/compiler/gen_llvm/src/llvm/convert.rs @@ -76,6 +76,66 @@ pub fn basic_type_from_layout<'a, 'ctx, 'env>( } } +pub fn basic_type_from_layout_1<'a, 'ctx, 'env>( + env: &crate::llvm::build::Env<'a, 'ctx, 'env>, + layout: &Layout<'_>, +) -> BasicTypeEnum<'ctx> { + use Layout::*; + + match layout { + Struct(sorted_fields) => basic_type_from_record(env, sorted_fields), + LambdaSet(lambda_set) => { + basic_type_from_layout_1(env, &lambda_set.runtime_representation()) + } + Union(union_layout) => { + use UnionLayout::*; + + let tag_id_type = basic_type_from_layout_1(env, &union_layout.tag_id_layout()); + + match union_layout { + NonRecursive(tags) => { + let data = block_of_memory_slices(env.context, tags, env.ptr_bytes); + let struct_type = env.context.struct_type(&[data, tag_id_type], false); + + struct_type.ptr_type(AddressSpace::Generic).into() + } + Recursive(tags) + | NullableWrapped { + other_tags: tags, .. + } => { + let data = block_of_memory_slices(env.context, tags, env.ptr_bytes); + + if union_layout.stores_tag_id_as_data(env.ptr_bytes) { + env.context + .struct_type(&[data, tag_id_type], false) + .ptr_type(AddressSpace::Generic) + .into() + } else { + data.ptr_type(AddressSpace::Generic).into() + } + } + NullableUnwrapped { other_fields, .. } => { + let block = block_of_memory_slices(env.context, &[other_fields], env.ptr_bytes); + block.ptr_type(AddressSpace::Generic).into() + } + NonNullableUnwrapped(fields) => { + let block = block_of_memory_slices(env.context, &[fields], env.ptr_bytes); + block.ptr_type(AddressSpace::Generic).into() + } + } + } + RecursivePointer => { + // TODO make this dynamic + env.context + .i64_type() + .ptr_type(AddressSpace::Generic) + .as_basic_type_enum() + } + + Builtin(builtin) => basic_type_from_builtin(env, builtin), + } +} + pub fn basic_type_from_builtin<'a, 'ctx, 'env>( env: &crate::llvm::build::Env<'a, 'ctx, 'env>, builtin: &Builtin<'_>, diff --git a/compiler/gen_llvm/src/llvm/refcounting.rs b/compiler/gen_llvm/src/llvm/refcounting.rs index f183e029f6..5944f19e25 100644 --- a/compiler/gen_llvm/src/llvm/refcounting.rs +++ b/compiler/gen_llvm/src/llvm/refcounting.rs @@ -1,11 +1,11 @@ use crate::debug_info_init; use crate::llvm::bitcode::call_void_bitcode_fn; use crate::llvm::build::{ - add_func, cast_basic_basic, cast_block_of_memory_to_tag, get_tag_id, get_tag_id_non_recursive, - tag_pointer_clear_tag_id, Env, FAST_CALL_CONV, TAG_DATA_INDEX, TAG_ID_INDEX, + add_func, cast_basic_basic, get_tag_id, tag_pointer_clear_tag_id, Env, FAST_CALL_CONV, + TAG_DATA_INDEX, TAG_ID_INDEX, }; use crate::llvm::build_list::{incrementing_elem_loop, list_len, load_list}; -use crate::llvm::convert::{basic_type_from_layout, ptr_int}; +use crate::llvm::convert::{basic_type_from_layout, basic_type_from_layout_1, ptr_int}; use bumpalo::collections::Vec; use inkwell::basic_block::BasicBlock; use inkwell::context::Context; @@ -542,24 +542,6 @@ fn modify_refcount_layout_help<'a, 'ctx, 'env>( } }, _ => { - if let Layout::Union(UnionLayout::NonRecursive(_)) = layout { - let alloca = env.builder.build_alloca(value.get_type(), "tag_union"); - env.builder.build_store(alloca, value); - call_help(env, function, call_mode, alloca.into()); - return; - } - - if let Layout::LambdaSet(lambda_set) = layout { - if let Layout::Union(UnionLayout::NonRecursive(_)) = - lambda_set.runtime_representation() - { - let alloca = env.builder.build_alloca(value.get_type(), "tag_union"); - env.builder.build_store(alloca, value); - call_help(env, function, call_mode, alloca.into()); - return; - } - } - call_help(env, function, call_mode, value); } } @@ -1620,10 +1602,9 @@ fn modify_refcount_union<'a, 'ctx, 'env>( let function = match env.module.get_function(fn_name.as_str()) { Some(function_value) => function_value, None => { - let basic_type = basic_type_from_layout(env, &layout); - // we pass tag unions by reference for efficiency - let ptr_type = basic_type.ptr_type(AddressSpace::Generic).into(); - let function_value = build_header(env, ptr_type, mode, &fn_name); + let basic_type = basic_type_from_layout_1(env, &layout); + let function_value = build_header(env, basic_type, mode, &fn_name); + dbg!(function_value); modify_refcount_union_help( env, @@ -1675,6 +1656,8 @@ fn modify_refcount_union_help<'a, 'ctx, 'env>( let before_block = env.builder.get_insert_block().expect("to be in a function"); + dbg!(fn_val, arg_ptr); + // read the tag_id let tag_id_ptr = env .builder diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index 2c63b7658d..cf30803e3b 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -863,6 +863,16 @@ impl<'a> Layout<'a> { false } + pub fn is_passed_by_reference(&self) -> bool { + match self { + Layout::Union(UnionLayout::NonRecursive(_)) => true, + Layout::LambdaSet(lambda_set) => { + lambda_set.runtime_representation().is_passed_by_reference() + } + _ => false, + } + } + pub fn stack_size(&self, pointer_size: u32) -> u32 { let width = self.stack_size_without_alignment(pointer_size); let alignment = self.alignment_bytes(pointer_size); @@ -941,16 +951,16 @@ impl<'a> Layout<'a> { }) .max(); + let tag_id_builtin = variant.tag_id_builtin(); match max_alignment { - Some(align) => { - let tag_id_builtin = variant.tag_id_builtin(); - - round_up_to_alignment( - align, - tag_id_builtin.alignment_bytes(pointer_size), - ) + Some(align) => round_up_to_alignment( + align.max(tag_id_builtin.alignment_bytes(pointer_size)), + tag_id_builtin.alignment_bytes(pointer_size), + ), + None => { + // none of the tags had any payload, but the tag id still contains information + tag_id_builtin.alignment_bytes(pointer_size) } - None => 0, } } Recursive(_) @@ -2675,3 +2685,27 @@ impl<'a> std::convert::TryFrom<&Layout<'a>> for ListLayout<'a> { } } } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn width_and_alignment_union_empty_struct() { + let lambda_set = LambdaSet { + set: &[(Symbol::LIST_MAP, &[])], + representation: &Layout::Struct(&[]), + }; + + let a = &[Layout::Struct(&[])] as &[_]; + let b = &[Layout::LambdaSet(lambda_set)] as &[_]; + let tt = [a, b]; + + let layout = Layout::Union(UnionLayout::NonRecursive(&tt)); + + // at the moment, the tag id uses an I64, so + let ptr_width = 8; + assert_eq!(layout.stack_size(ptr_width), 8); + assert_eq!(layout.alignment_bytes(ptr_width), 8); + } +} diff --git a/compiler/test_gen/src/gen_tags.rs b/compiler/test_gen/src/gen_tags.rs index fe50a16cde..9c5989b174 100644 --- a/compiler/test_gen/src/gen_tags.rs +++ b/compiler/test_gen/src/gen_tags.rs @@ -3,6 +3,7 @@ use crate::assert_evals_to; // use crate::assert_wasm_evals_to as assert_evals_to; use indoc::indoc; +use roc_mono::layout::LambdaSet; use roc_std::{RocList, RocStr}; #[test] @@ -423,7 +424,7 @@ fn result_with_guard_pattern() { } #[test] -fn maybe_is_just() { +fn maybe_is_just_not_nested() { assert_evals_to!( indoc!( r#" diff --git a/compiler/test_gen/src/helpers/eval.rs b/compiler/test_gen/src/helpers/eval.rs index e844d420ea..f6e4bffad2 100644 --- a/compiler/test_gen/src/helpers/eval.rs +++ b/compiler/test_gen/src/helpers/eval.rs @@ -198,6 +198,10 @@ fn create_llvm_module<'a>( if name.starts_with("roc_builtins.dict") { function.add_attribute(AttributeLoc::Function, attr); } + + if name.starts_with("roc_builtins.list") { + function.add_attribute(AttributeLoc::Function, attr); + } } // Compile and add all the Procs before adding main From bd0f02c542343b3454fb52137ef2c1ea4c975a8d Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 6 Nov 2021 19:27:16 +0100 Subject: [PATCH 10/19] another waypoint --- compiler/gen_llvm/src/llvm/build.rs | 41 ++++++++++++++--- compiler/gen_llvm/src/llvm/build_hash.rs | 29 ++++-------- compiler/gen_llvm/src/llvm/compare.rs | 55 +++++++++++------------ compiler/gen_llvm/src/llvm/convert.rs | 2 +- compiler/gen_llvm/src/llvm/refcounting.rs | 18 ++++++-- 5 files changed, 84 insertions(+), 61 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index a6fe38761a..83d6d8d253 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -1065,7 +1065,16 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( if !field_layout.is_dropped_because_empty() { field_types.push(basic_type_from_layout(env, field_layout)); - field_vals.push(field_expr); + if field_layout.is_passed_by_reference() { + let field_value = env.builder.build_load( + field_expr.into_pointer_value(), + "load_tag_to_put_in_struct", + ); + + field_vals.push(field_value); + } else { + field_vals.push(field_expr); + } } } @@ -1173,14 +1182,29 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( match (value, layout) { (StructValue(argument), Layout::Struct(fields)) => { debug_assert!(!fields.is_empty()); - env.builder + + let field_value = env + .builder .build_extract_value( argument, *index as u32, env.arena .alloc(format!("struct_field_access_record_{}", index)), ) - .unwrap() + .unwrap(); + + let field_layout = fields[*index as usize]; + if field_layout.is_passed_by_reference() { + let alloca = env.builder.build_alloca( + basic_type_from_layout(env, &field_layout), + "struct_field_tag", + ); + env.builder.build_store(alloca, field_value); + + alloca.into() + } else { + field_value + } } ( PointerValue(argument), @@ -2555,7 +2579,11 @@ pub fn build_exp_stmt<'a, 'ctx, 'env>( builder.position_at_end(cont_block); for (ptr, param) in joinpoint_args.iter().zip(parameters.iter()) { - let value = env.builder.build_load(*ptr, "load_jp_argument"); + let value = if param.layout.is_passed_by_reference() { + (*ptr).into() + } else { + env.builder.build_load(*ptr, "load_jp_argument") + }; scope.insert(param.symbol, (param.layout, value)); } @@ -2582,8 +2610,9 @@ pub fn build_exp_stmt<'a, 'ctx, 'env>( let (cont_block, argument_pointers) = scope.join_points.get(join_point).unwrap(); for (pointer, argument) in argument_pointers.iter().zip(arguments.iter()) { - let value = load_symbol(scope, argument); - builder.build_store(*pointer, value); + let (value, layout) = load_symbol_and_layout(scope, argument); + + store_roc_value(env, *layout, *pointer, value); } builder.build_unconditional_branch(*cont_block); diff --git a/compiler/gen_llvm/src/llvm/build_hash.rs b/compiler/gen_llvm/src/llvm/build_hash.rs index d3dc006198..7064cc100d 100644 --- a/compiler/gen_llvm/src/llvm/build_hash.rs +++ b/compiler/gen_llvm/src/llvm/build_hash.rs @@ -4,7 +4,7 @@ use crate::llvm::build::tag_pointer_clear_tag_id; use crate::llvm::build::Env; use crate::llvm::build::{cast_block_of_memory_to_tag, get_tag_id, FAST_CALL_CONV, TAG_DATA_INDEX}; use crate::llvm::build_str; -use crate::llvm::convert::basic_type_from_layout; +use crate::llvm::convert::{basic_type_from_layout, basic_type_from_layout_1}; use bumpalo::collections::Vec; use inkwell::values::{ BasicValue, BasicValueEnum, FunctionValue, IntValue, PointerValue, StructValue, @@ -339,7 +339,8 @@ fn build_hash_tag<'a, 'ctx, 'env>( None => { let seed_type = env.context.i64_type(); - let arg_type = basic_type_from_layout(env, layout); + let arg_type = basic_type_from_layout_1(env, layout); + dbg!(layout, arg_type); let function_value = crate::llvm::refcounting::build_header_help( env, @@ -423,14 +424,6 @@ fn hash_tag<'a, 'ctx, 'env>( let block = env.context.append_basic_block(parent, "tag_id_modify"); env.builder.position_at_end(block); - let struct_layout = Layout::Struct(field_layouts); - - let wrapper_type = basic_type_from_layout(env, &struct_layout); - debug_assert!(wrapper_type.is_struct_type()); - - let as_struct = - cast_block_of_memory_to_tag(env.builder, tag.into_struct_value(), wrapper_type); - // hash the tag id let hash_bytes = store_and_use_as_u8_ptr( env, @@ -440,7 +433,6 @@ fn hash_tag<'a, 'ctx, 'env>( .into(), &tag_id_layout, ); - let seed = hash_bitcode_fn( env, seed, @@ -449,14 +441,9 @@ fn hash_tag<'a, 'ctx, 'env>( ); // hash the tag data - let answer = build_hash_struct( - env, - layout_ids, - field_layouts, - WhenRecursive::Unreachable, - seed, - as_struct, - ); + let tag = tag.into_pointer_value(); + let answer = + hash_ptr_to_struct(env, layout_ids, union_layout, field_layouts, seed, tag); merge_phi.add_incoming(&[(&answer, block)]); env.builder.build_unconditional_branch(merge_block); @@ -822,12 +809,12 @@ fn hash_ptr_to_struct<'a, 'ctx, 'env>( ) -> IntValue<'ctx> { use inkwell::types::BasicType; - let wrapper_type = basic_type_from_layout(env, &Layout::Union(*union_layout)); + let wrapper_type = basic_type_from_layout_1(env, &Layout::Union(*union_layout)); // cast the opaque pointer to a pointer of the correct shape let wrapper_ptr = env .builder - .build_bitcast(tag, wrapper_type, "opaque_to_correct") + .build_bitcast(tag, wrapper_type, "hash_ptr_to_struct_opaque_to_correct") .into_pointer_value(); let struct_ptr = env diff --git a/compiler/gen_llvm/src/llvm/compare.rs b/compiler/gen_llvm/src/llvm/compare.rs index 1ca6178c0b..83c8887ce8 100644 --- a/compiler/gen_llvm/src/llvm/compare.rs +++ b/compiler/gen_llvm/src/llvm/compare.rs @@ -1,10 +1,8 @@ use crate::llvm::bitcode::call_bitcode_fn; -use crate::llvm::build::{ - cast_block_of_memory_to_tag, get_tag_id, tag_pointer_clear_tag_id, Env, FAST_CALL_CONV, -}; +use crate::llvm::build::{get_tag_id, tag_pointer_clear_tag_id, Env, FAST_CALL_CONV}; use crate::llvm::build_list::{list_len, load_list_ptr}; use crate::llvm::build_str::str_equal; -use crate::llvm::convert::basic_type_from_layout; +use crate::llvm::convert::{basic_type_from_layout, basic_type_from_layout_1}; use bumpalo::collections::Vec; use inkwell::types::BasicType; use inkwell::values::{ @@ -751,7 +749,7 @@ fn build_tag_eq<'a, 'ctx, 'env>( let function = match env.module.get_function(fn_name.as_str()) { Some(function_value) => function_value, None => { - let arg_type = basic_type_from_layout(env, tag_layout); + let arg_type = basic_type_from_layout_1(env, tag_layout); let function_value = crate::llvm::refcounting::build_header_help( env, @@ -844,9 +842,29 @@ fn build_tag_eq_help<'a, 'ctx, 'env>( match union_layout { NonRecursive(tags) => { + let ptr_equal = env.builder.build_int_compare( + IntPredicate::EQ, + env.builder + .build_ptr_to_int(tag1.into_pointer_value(), env.ptr_int(), "pti"), + env.builder + .build_ptr_to_int(tag2.into_pointer_value(), env.ptr_int(), "pti"), + "compare_pointers", + ); + + let compare_tag_ids = ctx.append_basic_block(parent, "compare_tag_ids"); + + env.builder + .build_conditional_branch(ptr_equal, return_true, compare_tag_ids); + + env.builder.position_at_end(compare_tag_ids); + let id1 = get_tag_id(env, parent, union_layout, tag1); let id2 = get_tag_id(env, parent, union_layout, tag2); + // clear the tag_id so we get a pointer to the actual data + let tag1 = tag1.into_pointer_value(); + let tag2 = tag2.into_pointer_value(); + let compare_tag_fields = ctx.append_basic_block(parent, "compare_tag_fields"); let same_tag = @@ -866,31 +884,8 @@ fn build_tag_eq_help<'a, 'ctx, 'env>( let block = env.context.append_basic_block(parent, "tag_id_modify"); env.builder.position_at_end(block); - // TODO drop tag id? - let struct_layout = Layout::Struct(field_layouts); - - let wrapper_type = basic_type_from_layout(env, &struct_layout); - debug_assert!(wrapper_type.is_struct_type()); - - let struct1 = cast_block_of_memory_to_tag( - env.builder, - tag1.into_struct_value(), - wrapper_type, - ); - let struct2 = cast_block_of_memory_to_tag( - env.builder, - tag2.into_struct_value(), - wrapper_type, - ); - - let answer = build_struct_eq( - env, - layout_ids, - field_layouts, - when_recursive.clone(), - struct1, - struct2, - ); + let answer = + eq_ptr_to_struct(env, layout_ids, union_layout, field_layouts, tag1, tag2); env.builder.build_return(Some(&answer)); diff --git a/compiler/gen_llvm/src/llvm/convert.rs b/compiler/gen_llvm/src/llvm/convert.rs index 559945b083..98306210ad 100644 --- a/compiler/gen_llvm/src/llvm/convert.rs +++ b/compiler/gen_llvm/src/llvm/convert.rs @@ -90,7 +90,7 @@ pub fn basic_type_from_layout_1<'a, 'ctx, 'env>( Union(union_layout) => { use UnionLayout::*; - let tag_id_type = basic_type_from_layout_1(env, &union_layout.tag_id_layout()); + let tag_id_type = basic_type_from_layout(env, &union_layout.tag_id_layout()); match union_layout { NonRecursive(tags) => { diff --git a/compiler/gen_llvm/src/llvm/refcounting.rs b/compiler/gen_llvm/src/llvm/refcounting.rs index 5944f19e25..30c309f540 100644 --- a/compiler/gen_llvm/src/llvm/refcounting.rs +++ b/compiler/gen_llvm/src/llvm/refcounting.rs @@ -349,18 +349,30 @@ fn modify_refcount_struct_help<'a, 'ctx, 'env>( for (i, field_layout) in layouts.iter().enumerate() { if field_layout.contains_refcounted() { - let field_ptr = env + let raw_value = env .builder .build_extract_value(wrapper_struct, i as u32, "decrement_struct_field") .unwrap(); + let field_value = if field_layout.is_passed_by_reference() { + let field_type = basic_type_from_layout(env, field_layout); + let alloca = env + .builder + .build_alloca(field_type, "load_struct_tag_field_for_decrement"); + env.builder.build_store(alloca, raw_value); + + alloca.into() + } else { + raw_value + }; + modify_refcount_layout_help( env, parent, layout_ids, mode.to_call_mode(fn_val), when_recursive, - field_ptr, + field_value, field_layout, ); } @@ -1289,7 +1301,7 @@ fn build_rec_union_recursive_decrement<'a, 'ctx, 'env>( .build_bitcast( value_ptr, wrapper_type.ptr_type(AddressSpace::Generic), - "opaque_to_correct", + "opaque_to_correct_recursive_decrement", ) .into_pointer_value(); From 180575852aea9efe412054ef4e54132c6c800d3b Mon Sep 17 00:00:00 2001 From: Folkert Date: Sun, 7 Nov 2021 14:56:24 +0100 Subject: [PATCH 11/19] all tests passing --- compiler/gen_llvm/src/llvm/build.rs | 27 ++++++++++++++++++++--- compiler/gen_llvm/src/llvm/build_hash.rs | 10 ++++++++- compiler/gen_llvm/src/llvm/build_list.rs | 23 ++++++++++++++----- compiler/gen_llvm/src/llvm/refcounting.rs | 19 +++++++++++----- 4 files changed, 65 insertions(+), 14 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 83d6d8d253..35832f8f8d 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -2372,6 +2372,25 @@ fn list_literal<'a, 'ctx, 'env>( } } +pub fn load_roc_value<'a, 'ctx, 'env>( + env: &Env<'a, 'ctx, 'env>, + layout: Layout<'a>, + source: PointerValue<'ctx>, + name: &str, +) -> BasicValueEnum<'ctx> { + if layout.is_passed_by_reference() { + let alloca = env + .builder + .build_alloca(basic_type_from_layout(env, &layout), name); + + store_roc_value(env, layout, alloca, source.into()); + + alloca.into() + } else { + env.builder.build_load(source, name) + } +} + pub fn store_roc_value_opaque<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, layout: Layout<'a>, @@ -3318,7 +3337,7 @@ fn expose_function_to_host_help_c_abi_generic<'a, 'ctx, 'env>( .unwrap() .into_pointer_value(); - builder.build_store(output_arg, call_result); + store_roc_value(env, return_layout, output_arg, call_result); builder.build_return(None); c_function @@ -4218,8 +4237,10 @@ pub fn build_closure_caller<'a, 'ctx, 'env>( // NOTE this may be incorrect in the long run // here we load any argument that is a pointer - for param in evaluator_arguments.iter_mut() { - if param.is_pointer_value() { + let closure_layout = lambda_set.runtime_representation(); + let layouts_it = arguments.iter().chain(std::iter::once(&closure_layout)); + for (param, layout) in evaluator_arguments.iter_mut().zip(layouts_it) { + if param.is_pointer_value() && !layout.is_passed_by_reference() { *param = builder.build_load(param.into_pointer_value(), "load_param"); } } diff --git a/compiler/gen_llvm/src/llvm/build_hash.rs b/compiler/gen_llvm/src/llvm/build_hash.rs index 7064cc100d..b73fe7bd4e 100644 --- a/compiler/gen_llvm/src/llvm/build_hash.rs +++ b/compiler/gen_llvm/src/llvm/build_hash.rs @@ -780,7 +780,15 @@ fn hash_list<'a, 'ctx, 'env>( env.builder.build_store(result, answer); }; - incrementing_elem_loop(env, parent, ptr, length, "current_index", loop_fn); + incrementing_elem_loop( + env, + parent, + *element_layout, + ptr, + length, + "current_index", + loop_fn, + ); env.builder.build_unconditional_branch(done_block); diff --git a/compiler/gen_llvm/src/llvm/build_list.rs b/compiler/gen_llvm/src/llvm/build_list.rs index 8b84bb6677..b581c3653e 100644 --- a/compiler/gen_llvm/src/llvm/build_list.rs +++ b/compiler/gen_llvm/src/llvm/build_list.rs @@ -17,6 +17,8 @@ use morphic_lib::UpdateMode; use roc_builtins::bitcode; use roc_mono::layout::{Builtin, Layout, LayoutIds}; +use super::build::load_roc_value; + pub fn pass_update_mode<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, update_mode: UpdateMode, @@ -216,10 +218,15 @@ pub fn list_get_unsafe<'a, 'ctx, 'env>( // Assume the bounds have already been checked earlier // (e.g. by List.get or List.first, which wrap List.#getUnsafe) - let elem_ptr = - unsafe { builder.build_in_bounds_gep(array_data_ptr, &[elem_index], "elem") }; + let elem_ptr = unsafe { + builder.build_in_bounds_gep(array_data_ptr, &[elem_index], "list_get_element") + }; - let result = builder.build_load(elem_ptr, "List.get"); + let result = if elem_layout.is_passed_by_reference() { + elem_ptr.into() + } else { + builder.build_load(elem_ptr, "list_get_load_element") + }; increment_refcount_layout(env, parent, layout_ids, 1, result, elem_layout); @@ -975,6 +982,7 @@ where pub fn incrementing_elem_loop<'a, 'ctx, 'env, LoopFn>( env: &Env<'a, 'ctx, 'env>, parent: FunctionValue<'ctx>, + element_layout: Layout<'a>, ptr: PointerValue<'ctx>, len: IntValue<'ctx>, index_name: &str, @@ -987,9 +995,14 @@ where incrementing_index_loop(env, parent, len, index_name, |index| { // The pointer to the element in the list - let elem_ptr = unsafe { builder.build_in_bounds_gep(ptr, &[index], "load_index") }; + let element_ptr = unsafe { builder.build_in_bounds_gep(ptr, &[index], "load_index") }; - let elem = builder.build_load(elem_ptr, "get_elem"); + let elem = load_roc_value( + env, + element_layout, + element_ptr, + "incrementing_element_loop_load", + ); loop_fn(index, elem); }) diff --git a/compiler/gen_llvm/src/llvm/refcounting.rs b/compiler/gen_llvm/src/llvm/refcounting.rs index 30c309f540..37f4e4cf42 100644 --- a/compiler/gen_llvm/src/llvm/refcounting.rs +++ b/compiler/gen_llvm/src/llvm/refcounting.rs @@ -765,7 +765,15 @@ fn modify_refcount_list_help<'a, 'ctx, 'env>( ); }; - incrementing_elem_loop(env, parent, ptr, len, "modify_rc_index", loop_fn); + incrementing_elem_loop( + env, + parent, + *element_layout, + ptr, + len, + "modify_rc_index", + loop_fn, + ); } let refcount_ptr = PointerToRefcount::from_list_wrapper(env, original_wrapper); @@ -1616,7 +1624,6 @@ fn modify_refcount_union<'a, 'ctx, 'env>( None => { let basic_type = basic_type_from_layout_1(env, &layout); let function_value = build_header(env, basic_type, mode, &fn_name); - dbg!(function_value); modify_refcount_union_help( env, @@ -1668,8 +1675,6 @@ fn modify_refcount_union_help<'a, 'ctx, 'env>( let before_block = env.builder.get_insert_block().expect("to be in a function"); - dbg!(fn_val, arg_ptr); - // read the tag_id let tag_id_ptr = env .builder @@ -1729,7 +1734,11 @@ fn modify_refcount_union_help<'a, 'ctx, 'env>( .build_struct_gep(cast_tag_data_pointer, i as u32, "modify_tag_field") .unwrap(); - let field_value = env.builder.build_load(field_ptr, "field_value"); + let field_value = if field_layout.is_passed_by_reference() { + field_ptr.into() + } else { + env.builder.build_load(field_ptr, "field_value") + }; modify_refcount_layout_help( env, From 3138fc43ec8e9662f381bbc0325c8be52794173e Mon Sep 17 00:00:00 2001 From: Folkert Date: Sun, 7 Nov 2021 16:31:43 +0100 Subject: [PATCH 12/19] cosmetic changes --- compiler/gen_llvm/src/llvm/build.rs | 4 ++-- compiler/gen_llvm/src/llvm/build_list.rs | 6 +----- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index ea4107c91d..f83656da09 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -1536,7 +1536,7 @@ pub fn build_tag<'a, 'ctx, 'env>( // store the tag id let tag_id_ptr = env .builder - .build_struct_gep(result_alloca, TAG_ID_INDEX, "get_opaque_data") + .build_struct_gep(result_alloca, TAG_ID_INDEX, "tag_id_ptr") .unwrap(); let tag_id_intval = tag_id_type.const_int(tag_id as u64, false); @@ -1549,7 +1549,7 @@ pub fn build_tag<'a, 'ctx, 'env>( let struct_opaque_ptr = env .builder - .build_struct_gep(result_alloca, TAG_DATA_INDEX, "get_opaque_data") + .build_struct_gep(result_alloca, TAG_DATA_INDEX, "opaque_data_ptr") .unwrap(); let struct_ptr = env.builder.build_pointer_cast( struct_opaque_ptr, diff --git a/compiler/gen_llvm/src/llvm/build_list.rs b/compiler/gen_llvm/src/llvm/build_list.rs index b581c3653e..4c0e7423af 100644 --- a/compiler/gen_llvm/src/llvm/build_list.rs +++ b/compiler/gen_llvm/src/llvm/build_list.rs @@ -222,11 +222,7 @@ pub fn list_get_unsafe<'a, 'ctx, 'env>( builder.build_in_bounds_gep(array_data_ptr, &[elem_index], "list_get_element") }; - let result = if elem_layout.is_passed_by_reference() { - elem_ptr.into() - } else { - builder.build_load(elem_ptr, "list_get_load_element") - }; + let result = load_roc_value(env, **elem_layout, elem_ptr, "list_get_load_element"); increment_refcount_layout(env, parent, layout_ids, 1, result, elem_layout); From e7ec575a81f5a8b335e8e01d3dfc309e4c4e2462 Mon Sep 17 00:00:00 2001 From: Folkert Date: Sun, 7 Nov 2021 21:41:12 +0100 Subject: [PATCH 13/19] trying to track down uninitialized memory creation --- compiler/gen_llvm/src/llvm/build.rs | 79 ++++++++++++++++++----------- compiler/mono/src/alias_analysis.rs | 2 +- 2 files changed, 51 insertions(+), 30 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index f83656da09..480a5ccdfe 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -619,8 +619,8 @@ pub fn construct_optimization_passes<'a>( mpm.add_always_inliner_pass(); // tail-call elimination is always on - // fpm.add_instruction_combining_pass(); - // fpm.add_tail_call_elimination_pass(); + fpm.add_instruction_combining_pass(); + fpm.add_tail_call_elimination_pass(); let pmb = PassManagerBuilder::create(); match opt_level { @@ -1460,6 +1460,36 @@ fn build_wrapped_tag<'a, 'ctx, 'env>( } } +pub fn tag_alloca<'a, 'ctx, 'env>( + env: &Env<'a, 'ctx, 'env>, + type_: BasicTypeEnum<'ctx>, + name: &str, +) -> PointerValue<'ctx> { + let result_alloca = env.builder.build_alloca(type_, name); + + // Initialize all memory of the alloca. This _should_ not be required, but currently + // LLVM can access uninitialized memory after applying some optimizations. Hopefully + // we can in the future adjust code gen so this is no longer an issue. + // + // An example is + // + // main : Task.Task {} [] + // main = + // when List.len [ Ok "foo", Err 42, Ok "spam" ] is + // n -> Task.putLine (Str.fromInt n) + // + // Here the decrement function of result must first check it's an Ok tag, + // then defers to string decrement, which must check is the string is small or large + // + // After inlining, those checks are combined. That means that even if the tag is Err, + // a check is done on the "string" to see if it is big or small, which will touch the + // uninitialized memory. + let all_zeros = type_.const_zero(); + env.builder.build_store(result_alloca, all_zeros); + + result_alloca +} + pub fn build_tag<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, scope: &Scope<'a, 'ctx>, @@ -1482,27 +1512,7 @@ pub fn build_tag<'a, 'ctx, 'env>( let wrapper_type = env .context .struct_type(&[internal_type, tag_id_type.into()], false); - let result_alloca = env.builder.build_alloca(wrapper_type, "tag_opaque"); - - // Initialize all memory of the alloca. This _should_ not be required, but currently - // LLVM can access uninitialized memory after applying some optimizations. Hopefully - // we can in the future adjust code gen so this is no longer an issue. - // - // An example is - // - // main : Task.Task {} [] - // main = - // when List.len [ Ok "foo", Err 42, Ok "spam" ] is - // n -> Task.putLine (Str.fromInt n) - // - // Here the decrement function of result must first check it's an Ok tag, - // then defers to string decrement, which must check is the string is small or large - // - // After inlining, those checks are combined. That means that even if the tag is Err, - // a check is done on the "string" to see if it is big or small, which will touch the - // uninitialized memory. - let all_zeros = wrapper_type.const_zero(); - env.builder.build_store(result_alloca, all_zeros); + let result_alloca = tag_alloca(env, wrapper_type.into(), "opaque_tag"); // Determine types let num_fields = arguments.len() + 1; @@ -2031,7 +2041,7 @@ fn lookup_at_index_ptr2<'a, 'ctx, 'env>( let field_type = basic_type_from_layout(env, &field_layout); let align_bytes = field_layout.alignment_bytes(env.ptr_bytes); - let alloca = env.builder.build_alloca(field_type, "copied_tag"); + let alloca = tag_alloca(env, field_type, "copied_tag"); if align_bytes > 0 { let size = env .ptr_int() @@ -2379,9 +2389,7 @@ pub fn load_roc_value<'a, 'ctx, 'env>( name: &str, ) -> BasicValueEnum<'ctx> { if layout.is_passed_by_reference() { - let alloca = env - .builder - .build_alloca(basic_type_from_layout(env, &layout), name); + let alloca = tag_alloca(env, basic_type_from_layout(env, &layout), name); store_roc_value(env, layout, alloca, source.into()); @@ -2509,7 +2517,20 @@ pub fn build_exp_stmt<'a, 'ctx, 'env>( let out_parameter = parameters.last().unwrap(); debug_assert!(out_parameter.is_pointer_value()); - store_roc_value(env, *layout, out_parameter.into_pointer_value(), value); + // store_roc_value(env, *layout, out_parameter.into_pointer_value(), value); + + let destination = out_parameter.into_pointer_value(); + if layout.is_passed_by_reference() { + let align_bytes = layout.alignment_bytes(env.ptr_bytes); + + if align_bytes > 0 { + let value_ptr = value.into_pointer_value(); + + value_ptr.replace_all_uses_with(destination); + } + } else { + env.builder.build_store(destination, value); + } if let Some(block) = env.builder.get_insert_block() { match block.get_terminator() { @@ -4569,7 +4590,7 @@ pub fn call_roc_function<'a, 'ctx, 'env>( let mut arguments = Vec::from_iter_in(arguments.iter().copied(), env.arena); let result_type = basic_type_from_layout(env, result_layout); - let result_alloca = env.builder.build_alloca(result_type, "result_value"); + let result_alloca = tag_alloca(env, result_type, "result_value"); arguments.push(result_alloca.into()); diff --git a/compiler/mono/src/alias_analysis.rs b/compiler/mono/src/alias_analysis.rs index 2a0d4b1c18..e2f9bd582c 100644 --- a/compiler/mono/src/alias_analysis.rs +++ b/compiler/mono/src/alias_analysis.rs @@ -243,7 +243,7 @@ where match opt_level { OptLevel::Development | OptLevel::Normal => morphic_lib::solve_trivial(program), - OptLevel::Optimize => morphic_lib::solve(program), + OptLevel::Optimize => morphic_lib::solve_trivial(program), } } From 826628456786493018a698b5dda43075fd71ea64 Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 8 Nov 2021 22:31:08 +0100 Subject: [PATCH 14/19] clippy --- compiler/gen_llvm/src/llvm/bitcode.rs | 2 +- compiler/gen_llvm/src/llvm/build.rs | 64 +++++++++++++---------- compiler/gen_llvm/src/llvm/build_hash.rs | 2 +- compiler/gen_llvm/src/llvm/compare.rs | 38 +++++++++++--- compiler/gen_llvm/src/llvm/refcounting.rs | 21 +++----- compiler/load/src/file.rs | 2 +- compiler/mono/src/alias_analysis.rs | 2 +- compiler/test_gen/src/gen_tags.rs | 1 - 8 files changed, 79 insertions(+), 53 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/bitcode.rs b/compiler/gen_llvm/src/llvm/bitcode.rs index fa89ceacf9..0fad482fef 100644 --- a/compiler/gen_llvm/src/llvm/bitcode.rs +++ b/compiler/gen_llvm/src/llvm/bitcode.rs @@ -1,7 +1,7 @@ /// Helpers for interacting with the zig that generates bitcode use crate::debug_info_init; use crate::llvm::build::{struct_from_fields, Env, C_CALL_CONV, FAST_CALL_CONV, TAG_DATA_INDEX}; -use crate::llvm::convert::{basic_type_from_layout, basic_type_from_layout_1}; +use crate::llvm::convert::basic_type_from_layout; use crate::llvm::refcounting::{ decrement_refcount_layout, increment_n_refcount_layout, increment_refcount_layout, }; diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 480a5ccdfe..66a2e0013b 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -1194,17 +1194,7 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( .unwrap(); let field_layout = fields[*index as usize]; - if field_layout.is_passed_by_reference() { - let alloca = env.builder.build_alloca( - basic_type_from_layout(env, &field_layout), - "struct_field_tag", - ); - env.builder.build_store(alloca, field_value); - - alloca.into() - } else { - field_value - } + use_roc_value(env, field_layout, field_value, "struct_field_tag") } ( PointerValue(argument), @@ -1253,8 +1243,6 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( index, union_layout, } => { - let builder = env.builder; - // cast the argument bytes into the desired shape for this tag let (argument, _structure_layout) = load_symbol_and_layout(scope, structure); @@ -2399,6 +2387,23 @@ pub fn load_roc_value<'a, 'ctx, 'env>( } } +pub fn use_roc_value<'a, 'ctx, 'env>( + env: &Env<'a, 'ctx, 'env>, + layout: Layout<'a>, + source: BasicValueEnum<'ctx>, + name: &str, +) -> BasicValueEnum<'ctx> { + if layout.is_passed_by_reference() { + let alloca = tag_alloca(env, basic_type_from_layout(env, &layout), name); + + env.builder.build_store(alloca, source); + + alloca.into() + } else { + source + } +} + pub fn store_roc_value_opaque<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, layout: Layout<'a>, @@ -2526,7 +2531,23 @@ pub fn build_exp_stmt<'a, 'ctx, 'env>( if align_bytes > 0 { let value_ptr = value.into_pointer_value(); - value_ptr.replace_all_uses_with(destination); + if true { + value_ptr.replace_all_uses_with(destination); + } else { + let size = env + .ptr_int() + .const_int(layout.stack_size(env.ptr_bytes) as u64, false); + + env.builder + .build_memcpy( + destination, + align_bytes, + value.into_pointer_value(), + align_bytes, + size, + ) + .unwrap(); + } } } else { env.builder.build_store(destination, value); @@ -2794,20 +2815,6 @@ pub fn load_symbol_and_lambda_set<'a, 'ctx, 'b>( } } -fn access_index_struct_value<'ctx>( - builder: &Builder<'ctx>, - from_value: StructValue<'ctx>, - to_type: StructType<'ctx>, -) -> StructValue<'ctx> { - complex_bitcast( - builder, - from_value.into(), - to_type.into(), - "access_index_struct_value", - ) - .into_struct_value() -} - /// Cast a value to another value of the same (or smaller?) size pub fn cast_basic_basic<'ctx>( builder: &Builder<'ctx>, @@ -4660,6 +4667,7 @@ pub struct RocFunctionCall<'ctx> { pub data_is_owned: IntValue<'ctx>, } +#[allow(clippy::too_many_arguments)] fn roc_function_call<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, layout_ids: &mut LayoutIds<'a>, diff --git a/compiler/gen_llvm/src/llvm/build_hash.rs b/compiler/gen_llvm/src/llvm/build_hash.rs index b73fe7bd4e..261af4e7d4 100644 --- a/compiler/gen_llvm/src/llvm/build_hash.rs +++ b/compiler/gen_llvm/src/llvm/build_hash.rs @@ -2,7 +2,7 @@ use crate::debug_info_init; use crate::llvm::bitcode::call_bitcode_fn; use crate::llvm::build::tag_pointer_clear_tag_id; use crate::llvm::build::Env; -use crate::llvm::build::{cast_block_of_memory_to_tag, get_tag_id, FAST_CALL_CONV, TAG_DATA_INDEX}; +use crate::llvm::build::{get_tag_id, FAST_CALL_CONV, TAG_DATA_INDEX}; use crate::llvm::build_str; use crate::llvm::convert::{basic_type_from_layout, basic_type_from_layout_1}; use bumpalo::collections::Vec; diff --git a/compiler/gen_llvm/src/llvm/compare.rs b/compiler/gen_llvm/src/llvm/compare.rs index 83c8887ce8..c624890616 100644 --- a/compiler/gen_llvm/src/llvm/compare.rs +++ b/compiler/gen_llvm/src/llvm/compare.rs @@ -884,8 +884,15 @@ fn build_tag_eq_help<'a, 'ctx, 'env>( let block = env.context.append_basic_block(parent, "tag_id_modify"); env.builder.position_at_end(block); - let answer = - eq_ptr_to_struct(env, layout_ids, union_layout, field_layouts, tag1, tag2); + let answer = eq_ptr_to_struct( + env, + layout_ids, + union_layout, + Some(when_recursive.clone()), + field_layouts, + tag1, + tag2, + ); env.builder.build_return(Some(&answer)); @@ -941,8 +948,15 @@ fn build_tag_eq_help<'a, 'ctx, 'env>( let block = env.context.append_basic_block(parent, "tag_id_modify"); env.builder.position_at_end(block); - let answer = - eq_ptr_to_struct(env, layout_ids, union_layout, field_layouts, tag1, tag2); + let answer = eq_ptr_to_struct( + env, + layout_ids, + union_layout, + None, + field_layouts, + tag1, + tag2, + ); env.builder.build_return(Some(&answer)); @@ -998,6 +1012,7 @@ fn build_tag_eq_help<'a, 'ctx, 'env>( env, layout_ids, union_layout, + None, other_fields, tag1.into_pointer_value(), tag2.into_pointer_value(), @@ -1088,8 +1103,15 @@ fn build_tag_eq_help<'a, 'ctx, 'env>( let block = env.context.append_basic_block(parent, "tag_id_modify"); env.builder.position_at_end(block); - let answer = - eq_ptr_to_struct(env, layout_ids, union_layout, field_layouts, tag1, tag2); + let answer = eq_ptr_to_struct( + env, + layout_ids, + union_layout, + None, + field_layouts, + tag1, + tag2, + ); env.builder.build_return(Some(&answer)); @@ -1123,6 +1145,7 @@ fn build_tag_eq_help<'a, 'ctx, 'env>( env, layout_ids, union_layout, + None, field_layouts, tag1.into_pointer_value(), tag2.into_pointer_value(), @@ -1137,6 +1160,7 @@ fn eq_ptr_to_struct<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, layout_ids: &mut LayoutIds<'a>, union_layout: &UnionLayout<'a>, + opt_when_recursive: Option>, field_layouts: &'a [Layout<'a>], tag1: PointerValue<'ctx>, tag2: PointerValue<'ctx>, @@ -1179,7 +1203,7 @@ fn eq_ptr_to_struct<'a, 'ctx, 'env>( env, layout_ids, field_layouts, - WhenRecursive::Loop(*union_layout), + opt_when_recursive.unwrap_or(WhenRecursive::Loop(*union_layout)), struct1, struct2, ) diff --git a/compiler/gen_llvm/src/llvm/refcounting.rs b/compiler/gen_llvm/src/llvm/refcounting.rs index 9c40a8d367..de55b101ff 100644 --- a/compiler/gen_llvm/src/llvm/refcounting.rs +++ b/compiler/gen_llvm/src/llvm/refcounting.rs @@ -1,8 +1,8 @@ use crate::debug_info_init; use crate::llvm::bitcode::call_void_bitcode_fn; use crate::llvm::build::{ - add_func, cast_basic_basic, get_tag_id, tag_pointer_clear_tag_id, Env, FAST_CALL_CONV, - TAG_DATA_INDEX, TAG_ID_INDEX, + add_func, cast_basic_basic, get_tag_id, tag_pointer_clear_tag_id, use_roc_value, Env, + FAST_CALL_CONV, TAG_DATA_INDEX, TAG_ID_INDEX, }; use crate::llvm::build_list::{incrementing_elem_loop, list_len, load_list}; use crate::llvm::convert::{basic_type_from_layout, basic_type_from_layout_1, ptr_int}; @@ -354,17 +354,12 @@ fn modify_refcount_struct_help<'a, 'ctx, 'env>( .build_extract_value(wrapper_struct, i as u32, "decrement_struct_field") .unwrap(); - let field_value = if field_layout.is_passed_by_reference() { - let field_type = basic_type_from_layout(env, field_layout); - let alloca = env - .builder - .build_alloca(field_type, "load_struct_tag_field_for_decrement"); - env.builder.build_store(alloca, raw_value); - - alloca.into() - } else { - raw_value - }; + let field_value = use_roc_value( + env, + *field_layout, + raw_value, + "load_struct_tag_field_for_decrement", + ); modify_refcount_layout_help( env, diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index 60e8d7a5f2..e0dfe8fd0a 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -2152,7 +2152,7 @@ fn update<'a>( println!("{}", result); } - Proc::insert_refcount_operations(arena, &mut state.procedures); + // Proc::insert_refcount_operations(arena, &mut state.procedures); // This is not safe with the new non-recursive RC updates that we do for tag unions // diff --git a/compiler/mono/src/alias_analysis.rs b/compiler/mono/src/alias_analysis.rs index e2f9bd582c..2a0d4b1c18 100644 --- a/compiler/mono/src/alias_analysis.rs +++ b/compiler/mono/src/alias_analysis.rs @@ -243,7 +243,7 @@ where match opt_level { OptLevel::Development | OptLevel::Normal => morphic_lib::solve_trivial(program), - OptLevel::Optimize => morphic_lib::solve_trivial(program), + OptLevel::Optimize => morphic_lib::solve(program), } } diff --git a/compiler/test_gen/src/gen_tags.rs b/compiler/test_gen/src/gen_tags.rs index 9c5989b174..c62140fd96 100644 --- a/compiler/test_gen/src/gen_tags.rs +++ b/compiler/test_gen/src/gen_tags.rs @@ -3,7 +3,6 @@ use crate::assert_evals_to; // use crate::assert_wasm_evals_to as assert_evals_to; use indoc::indoc; -use roc_mono::layout::LambdaSet; use roc_std::{RocList, RocStr}; #[test] From 7ff4ad6f7ba6aa75dab5df3702d05ba572de6a1a Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 8 Nov 2021 23:57:56 +0100 Subject: [PATCH 15/19] fix merge conflict --- 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 ba9b67a858..642332171b 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -5166,6 +5166,7 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( closure_layout, function_owns_closure_data, argument_layouts, + Layout::Builtin(Builtin::Int1), ); list_any(env, roc_function_call, list, element_layout) From 4fdb8d354b501120972d736d026693b966eef20b Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 10 Nov 2021 00:22:49 +0100 Subject: [PATCH 16/19] turn on refcounting again, turning it off does not help (builtins still decrement and potentially free) --- compiler/load/src/file.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index e0dfe8fd0a..60e8d7a5f2 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -2152,7 +2152,7 @@ fn update<'a>( println!("{}", result); } - // Proc::insert_refcount_operations(arena, &mut state.procedures); + Proc::insert_refcount_operations(arena, &mut state.procedures); // This is not safe with the new non-recursive RC updates that we do for tag unions // From 38da99b1ac017abda1b8eb9274cce54e131410bd Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 11 Nov 2021 23:36:35 +0100 Subject: [PATCH 17/19] make it work --- cli/src/repl/gen.rs | 4 ++-- compiler/gen_llvm/src/llvm/bitcode.rs | 15 ++++----------- compiler/gen_llvm/src/llvm/build.rs | 13 +++++++------ compiler/gen_llvm/src/llvm/refcounting.rs | 23 +++++++++++++++++++---- 4 files changed, 32 insertions(+), 23 deletions(-) diff --git a/cli/src/repl/gen.rs b/cli/src/repl/gen.rs index 200941919d..3e789da85b 100644 --- a/cli/src/repl/gen.rs +++ b/cli/src/repl/gen.rs @@ -218,8 +218,8 @@ pub fn gen_and_eval<'a>( // Verify the module if let Err(errors) = env.module.verify() { panic!( - "Errors defining module: {}\n\nUncomment things nearby to see more details.", - errors + "Errors defining module:\n{}\n\nUncomment things nearby to see more details.", + errors.to_string() ); } diff --git a/compiler/gen_llvm/src/llvm/bitcode.rs b/compiler/gen_llvm/src/llvm/bitcode.rs index 0fad482fef..2afb1c7159 100644 --- a/compiler/gen_llvm/src/llvm/bitcode.rs +++ b/compiler/gen_llvm/src/llvm/bitcode.rs @@ -154,18 +154,11 @@ fn build_has_tag_id_help<'a, 'ctx, 'env>( ); let tag_data_ptr = { - let data_index = env - .context - .i64_type() - .const_int(TAG_DATA_INDEX as u64, false); + let ptr = env + .builder + .build_struct_gep(tag_value, TAG_DATA_INDEX, "get_data_ptr") + .unwrap(); - let ptr = unsafe { - env.builder.build_gep( - tag_value_ptr.into_pointer_value(), - &[data_index], - "get_data_ptr", - ) - }; env.builder.build_bitcast(ptr, i8_ptr_type, "to_opaque") }; diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index b89096f4a5..711116bc80 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -2532,7 +2532,10 @@ pub fn build_exp_stmt<'a, 'ctx, 'env>( if align_bytes > 0 { let value_ptr = value.into_pointer_value(); - if true { + // We can only do this if the function itself writes data into this + // pointer. If the pointer is passed as an argument, then we must copy + // from one pointer to our destination pointer + if value_ptr.get_first_use().is_some() { value_ptr.replace_all_uses_with(destination); } else { let size = env @@ -3382,8 +3385,7 @@ fn expose_function_to_host_help_c_abi_gen_test<'a, 'ctx, 'env>( // a tagged union to indicate to the test loader that a panic occurred. // especially when running 32-bit binaries on a 64-bit machine, there // does not seem to be a smarter solution - let wrapper_return_type = - roc_result_type(env, roc_function.get_type().get_return_type().unwrap()); + let wrapper_return_type = roc_result_type(env, basic_type_from_layout(env, &return_layout)); let mut cc_argument_types = Vec::with_capacity_in(arguments.len(), env.arena); for layout in arguments { @@ -3861,7 +3863,7 @@ fn make_good_roc_result<'a, 'ctx, 'env>( let context = env.context; let builder = env.builder; - let v1 = roc_result_type(env, return_value.get_type()).const_zero(); + let v1 = roc_result_type(env, basic_type_from_layout(env, &return_layout)).const_zero(); let v2 = builder .build_insert_value(v1, context.i64_type().const_zero(), 0, "set_no_error") @@ -3906,8 +3908,7 @@ fn make_exception_catching_wrapper<'a, 'ctx, 'env>( } }; - let wrapper_return_type = - roc_result_type(env, roc_function.get_type().get_return_type().unwrap()); + let wrapper_return_type = roc_result_type(env, basic_type_from_layout(env, &return_layout)); // argument_types.push(wrapper_return_type.ptr_type(AddressSpace::Generic).into()); diff --git a/compiler/gen_llvm/src/llvm/refcounting.rs b/compiler/gen_llvm/src/llvm/refcounting.rs index de55b101ff..0a956f8bce 100644 --- a/compiler/gen_llvm/src/llvm/refcounting.rs +++ b/compiler/gen_llvm/src/llvm/refcounting.rs @@ -139,8 +139,10 @@ impl<'ctx> PointerToRefcount<'ctx> { let block = env.builder.get_insert_block().expect("to be in a function"); let parent = block.get_parent().unwrap(); - let modify_block = env.context.append_basic_block(parent, "inc_str_modify"); - let cont_block = env.context.append_basic_block(parent, "inc_str_cont"); + let modify_block = env + .context + .append_basic_block(parent, "inc_refcount_modify"); + let cont_block = env.context.append_basic_block(parent, "inc_refcount_cont"); env.builder .build_conditional_branch(is_static_allocation, cont_block, modify_block); @@ -1718,12 +1720,25 @@ fn modify_refcount_union_help<'a, 'ctx, 'env>( "cast_to_concrete_tag", ); - // let wrapper_struct = cast_block_of_memory_to_tag(env.builder, data_bytes, wrapper_type); - for (i, field_layout) in field_layouts.iter().enumerate() { if let Layout::RecursivePointer = field_layout { panic!("non-recursive tag unions cannot contain naked recursion pointers!"); } else if field_layout.contains_refcounted() { + // crazy hack + match field_layout { + Layout::Builtin(Builtin::Str | Builtin::List(_)) => { + use inkwell::attributes::{Attribute, AttributeLoc}; + let kind_id = Attribute::get_named_enum_kind_id("noinline"); + debug_assert!(kind_id > 0); + let enum_attr = env.context.create_enum_attribute(kind_id, 1); + + fn_val.add_attribute(AttributeLoc::Function, enum_attr); + } + _ => { + // do nothing + } + } + let field_ptr = env .builder .build_struct_gep(cast_tag_data_pointer, i as u32, "modify_tag_field") From 65a9febe7dd1e6294d34619d5dd00e151fafea68 Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 11 Nov 2021 23:38:45 +0100 Subject: [PATCH 18/19] clippy --- compiler/gen_llvm/src/llvm/refcounting.rs | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/refcounting.rs b/compiler/gen_llvm/src/llvm/refcounting.rs index 0a956f8bce..47f2ecf24c 100644 --- a/compiler/gen_llvm/src/llvm/refcounting.rs +++ b/compiler/gen_llvm/src/llvm/refcounting.rs @@ -1724,19 +1724,15 @@ fn modify_refcount_union_help<'a, 'ctx, 'env>( if let Layout::RecursivePointer = field_layout { panic!("non-recursive tag unions cannot contain naked recursion pointers!"); } else if field_layout.contains_refcounted() { - // crazy hack - match field_layout { - Layout::Builtin(Builtin::Str | Builtin::List(_)) => { - use inkwell::attributes::{Attribute, AttributeLoc}; - let kind_id = Attribute::get_named_enum_kind_id("noinline"); - debug_assert!(kind_id > 0); - let enum_attr = env.context.create_enum_attribute(kind_id, 1); + // crazy hack: inlining this function when it decrements a list or string results + // in many, many valgrind errors in `--optimize` mode. We do not know why. + if let Layout::Builtin(Builtin::Str | Builtin::List(_)) = field_layout { + use inkwell::attributes::{Attribute, AttributeLoc}; + let kind_id = Attribute::get_named_enum_kind_id("noinline"); + debug_assert!(kind_id > 0); + let enum_attr = env.context.create_enum_attribute(kind_id, 1); - fn_val.add_attribute(AttributeLoc::Function, enum_attr); - } - _ => { - // do nothing - } + fn_val.add_attribute(AttributeLoc::Function, enum_attr); } let field_ptr = env From 196538cc586be523ad8dd795c04d8fd943a34787 Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 13 Nov 2021 01:00:20 +0100 Subject: [PATCH 19/19] fix valgrind error, finally --- compiler/gen_llvm/src/llvm/bitcode.rs | 4 ++-- compiler/gen_llvm/src/llvm/build_list.rs | 24 +++++++++++++---------- compiler/gen_llvm/src/llvm/refcounting.rs | 11 ----------- 3 files changed, 16 insertions(+), 23 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/bitcode.rs b/compiler/gen_llvm/src/llvm/bitcode.rs index 2afb1c7159..159c688580 100644 --- a/compiler/gen_llvm/src/llvm/bitcode.rs +++ b/compiler/gen_llvm/src/llvm/bitcode.rs @@ -260,7 +260,7 @@ fn build_transform_caller_help<'a, 'ctx, 'env>( .build_pointer_cast( argument_ptr.into_pointer_value(), basic_type, - "cast_ptr_to_tag", + "cast_ptr_to_tag_build_transform_caller_help", ) .into() } else { @@ -409,7 +409,7 @@ fn build_rc_wrapper<'a, 'ctx, 'env>( let value = if layout.is_passed_by_reference() { env.builder - .build_pointer_cast(value_ptr, value_type, "cast_ptr_to_tag") + .build_pointer_cast(value_ptr, value_type, "cast_ptr_to_tag_build_rc_wrapper") .into() } else { let value_cast = env diff --git a/compiler/gen_llvm/src/llvm/build_list.rs b/compiler/gen_llvm/src/llvm/build_list.rs index 154295d9cd..323960353d 100644 --- a/compiler/gen_llvm/src/llvm/build_list.rs +++ b/compiler/gen_llvm/src/llvm/build_list.rs @@ -17,7 +17,7 @@ use morphic_lib::UpdateMode; use roc_builtins::bitcode; use roc_mono::layout::{Builtin, Layout, LayoutIds}; -use super::build::load_roc_value; +use super::build::{load_roc_value, store_roc_value}; pub fn pass_update_mode<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, @@ -55,9 +55,13 @@ pub fn call_bitcode_fn_returns_list<'a, 'ctx, 'env>( fn pass_element_as_opaque<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, element: BasicValueEnum<'ctx>, + layout: Layout<'a>, ) -> BasicValueEnum<'ctx> { - let element_ptr = env.builder.build_alloca(element.get_type(), "element"); - env.builder.build_store(element_ptr, element); + let element_type = basic_type_from_layout(env, &layout); + let element_ptr = env + .builder + .build_alloca(element_type, "element_to_pass_as_opaque"); + store_roc_value(env, layout, element_ptr, element); env.builder.build_bitcast( element_ptr, @@ -108,7 +112,7 @@ pub fn list_single<'a, 'ctx, 'env>( env, &[ env.alignment_intvalue(element_layout), - pass_element_as_opaque(env, element), + pass_element_as_opaque(env, element, *element_layout), layout_width(env, element_layout), ], bitcode::LIST_SINGLE, @@ -130,7 +134,7 @@ pub fn list_repeat<'a, 'ctx, 'env>( &[ list_len.into(), env.alignment_intvalue(element_layout), - pass_element_as_opaque(env, element), + pass_element_as_opaque(env, element, *element_layout), layout_width(env, element_layout), inc_element_fn.as_global_value().as_pointer_value().into(), ], @@ -250,7 +254,7 @@ pub fn list_append<'a, 'ctx, 'env>( &[ pass_list_cc(env, original_wrapper.into()), env.alignment_intvalue(element_layout), - pass_element_as_opaque(env, element), + pass_element_as_opaque(env, element, *element_layout), layout_width(env, element_layout), pass_update_mode(env, update_mode), ], @@ -270,7 +274,7 @@ pub fn list_prepend<'a, 'ctx, 'env>( &[ pass_list_cc(env, original_wrapper.into()), env.alignment_intvalue(element_layout), - pass_element_as_opaque(env, element), + pass_element_as_opaque(env, element, *element_layout), layout_width(env, element_layout), ], bitcode::LIST_PREPEND, @@ -409,7 +413,7 @@ pub fn list_set<'a, 'ctx, 'env>( &[ bytes.into(), index.into(), - pass_element_as_opaque(env, element), + pass_element_as_opaque(env, element, *element_layout), layout_width(env, element_layout), dec_element_fn.as_global_value().as_pointer_value().into(), ], @@ -422,7 +426,7 @@ pub fn list_set<'a, 'ctx, 'env>( length.into(), env.alignment_intvalue(element_layout), index.into(), - pass_element_as_opaque(env, element), + pass_element_as_opaque(env, element, *element_layout), layout_width(env, element_layout), dec_element_fn.as_global_value().as_pointer_value().into(), ], @@ -598,7 +602,7 @@ pub fn list_contains<'a, 'ctx, 'env>( env, &[ pass_list_cc(env, list), - pass_element_as_opaque(env, element), + pass_element_as_opaque(env, element, *element_layout), layout_width(env, element_layout), eq_fn, ], diff --git a/compiler/gen_llvm/src/llvm/refcounting.rs b/compiler/gen_llvm/src/llvm/refcounting.rs index 47f2ecf24c..1b6c6aa4d0 100644 --- a/compiler/gen_llvm/src/llvm/refcounting.rs +++ b/compiler/gen_llvm/src/llvm/refcounting.rs @@ -1724,17 +1724,6 @@ fn modify_refcount_union_help<'a, 'ctx, 'env>( if let Layout::RecursivePointer = field_layout { panic!("non-recursive tag unions cannot contain naked recursion pointers!"); } else if field_layout.contains_refcounted() { - // crazy hack: inlining this function when it decrements a list or string results - // in many, many valgrind errors in `--optimize` mode. We do not know why. - if let Layout::Builtin(Builtin::Str | Builtin::List(_)) = field_layout { - use inkwell::attributes::{Attribute, AttributeLoc}; - let kind_id = Attribute::get_named_enum_kind_id("noinline"); - debug_assert!(kind_id > 0); - let enum_attr = env.context.create_enum_attribute(kind_id, 1); - - fn_val.add_attribute(AttributeLoc::Function, enum_attr); - } - let field_ptr = env .builder .build_struct_gep(cast_tag_data_pointer, i as u32, "modify_tag_field")