From 3cc96e76dc59203fcd6f8f50191283a6882f2beb Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 26 Mar 2022 14:20:59 +0100 Subject: [PATCH 01/13] preparation for using argument_type_from_layout --- compiler/gen_llvm/src/llvm/convert.rs | 9 +++++++++ compiler/gen_llvm/src/llvm/refcounting.rs | 6 +++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/convert.rs b/compiler/gen_llvm/src/llvm/convert.rs index 3b37bfa000..5f97a18f24 100644 --- a/compiler/gen_llvm/src/llvm/convert.rs +++ b/compiler/gen_llvm/src/llvm/convert.rs @@ -132,6 +132,15 @@ pub fn argument_type_from_layout<'a, 'ctx, 'env>( argument_type_from_layout(env, &lambda_set.runtime_representation()) } Union(union_layout) => argument_type_from_union_layout(env, union_layout), + Builtin(_) => { + let base = basic_type_from_layout(env, layout); + + if layout.is_passed_by_reference() { + base.ptr_type(AddressSpace::Generic).into() + } else { + base + } + } other => basic_type_from_layout(env, other), } } diff --git a/compiler/gen_llvm/src/llvm/refcounting.rs b/compiler/gen_llvm/src/llvm/refcounting.rs index 1bd968c118..0098432aa5 100644 --- a/compiler/gen_llvm/src/llvm/refcounting.rs +++ b/compiler/gen_llvm/src/llvm/refcounting.rs @@ -20,7 +20,7 @@ use roc_module::symbol::Symbol; use roc_mono::layout::{Builtin, Layout, LayoutIds, UnionLayout}; use roc_target::TargetInfo; -use super::convert::argument_type_from_union_layout; +use super::convert::{argument_type_from_layout, argument_type_from_union_layout}; /// "Infinite" reference count, for static values /// Ref counts are encoded as negative numbers where isize::MIN represents 1 @@ -687,7 +687,7 @@ fn modify_refcount_list<'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); + let basic_type = argument_type_from_layout(env, layout); let function_value = build_header(env, basic_type, mode, &fn_name); modify_refcount_list_help( @@ -824,7 +824,7 @@ fn modify_refcount_str<'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); + let basic_type = argument_type_from_layout(env, layout); let function_value = build_header(env, basic_type, mode, &fn_name); modify_refcount_str_help(env, mode, layout, function_value); From 59a2253b9a3b74b40c297b39f88b5ce76bc609a3 Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 26 Mar 2022 13:43:41 +0100 Subject: [PATCH 02/13] rework how tags are created --- compiler/gen_llvm/src/llvm/build.rs | 196 +++++++++++----------------- 1 file changed, 75 insertions(+), 121 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index b774f1a2a2..cc7d81aaf7 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -1021,7 +1021,7 @@ fn struct_pointer_from_fields<'a, 'ctx, 'env, I>( input_pointer: PointerValue<'ctx>, values: I, ) where - I: Iterator)>, + I: Iterator, BasicValueEnum<'ctx>))>, { let struct_ptr = env .builder @@ -1033,13 +1033,13 @@ fn struct_pointer_from_fields<'a, 'ctx, 'env, I>( .into_pointer_value(); // Insert field exprs into struct_val - for (index, field_val) in values { + for (index, (field_layout, field_value)) in values { let field_ptr = env .builder .build_struct_gep(struct_ptr, index as u32, "field_struct_gep") .unwrap(); - env.builder.build_store(field_ptr, field_val); + store_roc_value(env, field_layout, field_ptr, field_value); } } @@ -1415,53 +1415,11 @@ fn build_wrapped_tag<'a, 'ctx, 'env>( reuse_allocation: Option>, parent: FunctionValue<'ctx>, ) -> BasicValueEnum<'ctx> { - let ctx = env.context; let builder = env.builder; let tag_id_layout = union_layout.tag_id_layout(); - // Determine types - let num_fields = arguments.len() + 1; - let mut field_types = Vec::with_capacity_in(num_fields, env.arena); - let mut field_vals = Vec::with_capacity_in(num_fields, env.arena); - - for (field_symbol, tag_field_layout) in arguments.iter().zip(tag_field_layouts.iter()) { - let (val, _val_layout) = load_symbol_and_layout(scope, field_symbol); - - let field_type = basic_type_from_layout(env, tag_field_layout); - - field_types.push(field_type); - - if let Layout::RecursivePointer = tag_field_layout { - debug_assert!(val.is_pointer_value()); - - // we store recursive pointers as `i64*` - let ptr = env.builder.build_bitcast( - val, - ctx.i64_type().ptr_type(AddressSpace::Generic), - "cast_recursive_pointer", - ); - - field_vals.push(ptr); - } else if matches!( - tag_field_layout, - Layout::Union(UnionLayout::NonRecursive(_)) - ) { - debug_assert!(val.is_pointer_value()); - - // We store non-recursive unions without any indirection. - let reified = env - .builder - .build_load(val.into_pointer_value(), "load_non_recursive"); - - field_vals.push(reified); - } else { - // this check fails for recursive tag unions, but can be helpful while debugging - // debug_assert_eq!(tag_field_layout, val_layout); - - field_vals.push(val); - } - } + let (field_types, field_values) = build_tag_fields(env, scope, tag_field_layouts, arguments); // Create the struct_type let raw_data_ptr = allocate_tag(env, parent, reuse_allocation, union_layout, tags); @@ -1485,7 +1443,7 @@ fn build_wrapped_tag<'a, 'ctx, 'env>( env, struct_type, opaque_struct_ptr, - field_vals.into_iter().enumerate(), + field_values.into_iter().enumerate(), ); raw_data_ptr.into() @@ -1494,7 +1452,7 @@ fn build_wrapped_tag<'a, 'ctx, 'env>( env, struct_type, raw_data_ptr, - field_vals.into_iter().enumerate(), + field_values.into_iter().enumerate(), ); tag_pointer_set_tag_id(env, tag_id, raw_data_ptr).into() @@ -1537,7 +1495,63 @@ pub fn tag_alloca<'a, 'ctx, 'env>( result_alloca } -pub fn build_tag<'a, 'ctx, 'env>( +fn build_tag_field_value<'a, 'ctx, 'env>( + env: &Env<'a, 'ctx, 'env>, + value: BasicValueEnum<'ctx>, + tag_field_layout: Layout<'a>, +) -> BasicValueEnum<'ctx> { + if let Layout::RecursivePointer = tag_field_layout { + debug_assert!(value.is_pointer_value()); + + // we store recursive pointers as `i64*` + env.builder.build_bitcast( + value, + env.context.i64_type().ptr_type(AddressSpace::Generic), + "cast_recursive_pointer", + ) + } else if tag_field_layout.is_passed_by_reference() { + debug_assert!(value.is_pointer_value()); + + // NOTE: we rely on this being passed to `store_roc_value` so that + // the value is memcpy'd + value + } else { + // this check fails for recursive tag unions, but can be helpful while debugging + // debug_assert_eq!(tag_field_layout, val_layout); + + value + } +} + +fn build_tag_fields<'a, 'ctx, 'env>( + env: &Env<'a, 'ctx, 'env>, + scope: &Scope<'a, 'ctx>, + fields: &[Layout<'a>], + arguments: &[Symbol], +) -> ( + Vec<'a, BasicTypeEnum<'ctx>>, + Vec<'a, (Layout<'a>, BasicValueEnum<'ctx>)>, +) { + debug_assert_eq!(fields.len(), arguments.len()); + + let capacity = fields.len(); + let mut field_types = Vec::with_capacity_in(capacity, env.arena); + let mut field_values = Vec::with_capacity_in(capacity, env.arena); + + for (field_symbol, tag_field_layout) in arguments.iter().zip(fields.iter()) { + let field_type = basic_type_from_layout(env, tag_field_layout); + field_types.push(field_type); + + let raw_value = load_symbol(scope, field_symbol); + let field_value = build_tag_field_value(env, raw_value, *tag_field_layout); + + field_values.push((*tag_field_layout, field_value)); + } + + (field_types, field_values) +} + +fn build_tag<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, scope: &Scope<'a, 'ctx>, union_layout: &UnionLayout<'a>, @@ -1688,49 +1702,21 @@ pub fn build_tag<'a, 'ctx, 'env>( debug_assert_eq!(tag_id, 0); debug_assert_eq!(arguments.len(), fields.len()); - let ctx = env.context; - - // Determine types - let num_fields = arguments.len() + 1; - let mut field_types = Vec::with_capacity_in(num_fields, env.arena); - let mut field_vals = Vec::with_capacity_in(num_fields, env.arena); - - for (field_symbol, tag_field_layout) in arguments.iter().zip(fields.iter()) { - let val = load_symbol(scope, field_symbol); - - let field_type = basic_type_from_layout(env, tag_field_layout); - - field_types.push(field_type); - - if let Layout::RecursivePointer = tag_field_layout { - debug_assert!(val.is_pointer_value()); - - // we store recursive pointers as `i64*` - let ptr = env.builder.build_bitcast( - val, - ctx.i64_type().ptr_type(AddressSpace::Generic), - "cast_recursive_pointer", - ); - - field_vals.push(ptr); - } else { - // this check fails for recursive tag unions, but can be helpful while debugging - - field_vals.push(val); - } - } + let (field_types, field_values) = build_tag_fields(env, scope, fields, arguments); // Create the struct_type let data_ptr = reserve_with_refcount_union_as_block_of_memory(env, *union_layout, &[fields]); - let struct_type = ctx.struct_type(field_types.into_bump_slice(), false); + let struct_type = env + .context + .struct_type(field_types.into_bump_slice(), false); struct_pointer_from_fields( env, struct_type, data_ptr, - field_vals.into_iter().enumerate(), + field_values.into_iter().enumerate(), ); data_ptr.into() @@ -1754,56 +1740,22 @@ pub fn build_tag<'a, 'ctx, 'env>( debug_assert!(union_size == 2); - let ctx = env.context; - // Determine types - let num_fields = arguments.len() + 1; - let mut field_types = Vec::with_capacity_in(num_fields, env.arena); - let mut field_vals = Vec::with_capacity_in(num_fields, env.arena); - - debug_assert_eq!(arguments.len(), other_fields.len()); - - for (field_symbol, tag_field_layout) in arguments.iter().zip(other_fields.iter()) { - let val = load_symbol(scope, field_symbol); - - // Zero-sized fields have no runtime representation. - // The layout of the struct expects them to be dropped! - if !tag_field_layout.is_dropped_because_empty() { - let field_type = basic_type_from_layout(env, tag_field_layout); - - field_types.push(field_type); - - if let Layout::RecursivePointer = tag_field_layout { - debug_assert!(val.is_pointer_value()); - - // we store recursive pointers as `i64*` - let ptr = env.builder.build_bitcast( - val, - ctx.i64_type().ptr_type(AddressSpace::Generic), - "cast_recursive_pointer", - ); - - field_vals.push(ptr); - } else { - // this check fails for recursive tag unions, but can be helpful while debugging - // debug_assert_eq!(tag_field_layout, val_layout); - - field_vals.push(val); - } - } - } + let (field_types, field_values) = build_tag_fields(env, scope, other_fields, arguments); // Create the struct_type let data_ptr = allocate_tag(env, parent, reuse_allocation, union_layout, &[other_fields]); - let struct_type = ctx.struct_type(field_types.into_bump_slice(), false); + let struct_type = env + .context + .struct_type(field_types.into_bump_slice(), false); struct_pointer_from_fields( env, struct_type, data_ptr, - field_vals.into_iter().enumerate(), + field_values.into_iter().enumerate(), ); data_ptr.into() @@ -2501,6 +2453,8 @@ pub fn store_roc_value<'a, 'ctx, 'env>( value: BasicValueEnum<'ctx>, ) { if layout.is_passed_by_reference() { + debug_assert!(value.is_pointer_value()); + let align_bytes = layout.alignment_bytes(env.target_info); if align_bytes > 0 { From 972c5d78d3499079c621c1a449c9b0c9332e4c3c Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 26 Mar 2022 13:10:18 +0100 Subject: [PATCH 03/13] use roc_ helpers for box loading and storing --- compiler/gen_llvm/src/llvm/build.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index cc7d81aaf7..15df8688de 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -1138,7 +1138,7 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( layout.alignment_bytes(env.target_info), ); - env.builder.build_store(allocation, value); + store_roc_value(env, *layout, allocation, value); allocation.into() } @@ -1148,8 +1148,7 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( debug_assert!(value.is_pointer_value()); - env.builder - .build_load(value.into_pointer_value(), "load_boxed_value") + load_roc_value(env, *layout, value.into_pointer_value(), "load_boxed_value") } Reset { symbol, .. } => { From 4426f5793b2bc88d3ffb9f0c9b2c01bce6adc87d Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 26 Mar 2022 12:52:13 +0100 Subject: [PATCH 04/13] rename a function --- compiler/gen_llvm/src/llvm/build.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 15df8688de..7afe216fc2 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -1458,7 +1458,7 @@ fn build_wrapped_tag<'a, 'ctx, 'env>( } } -pub fn tag_alloca<'a, 'ctx, 'env>( +pub fn entry_block_alloca_zerofill<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, basic_type: BasicTypeEnum<'ctx>, name: &str, @@ -1572,7 +1572,7 @@ fn build_tag<'a, 'ctx, 'env>( let wrapper_type = env .context .struct_type(&[internal_type, tag_id_type.into()], false); - let result_alloca = tag_alloca(env, wrapper_type.into(), "opaque_tag"); + let result_alloca = entry_block_alloca_zerofill(env, wrapper_type.into(), "opaque_tag"); // Determine types let num_fields = arguments.len() + 1; @@ -2037,7 +2037,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.target_info); - let alloca = tag_alloca(env, field_type, "copied_tag"); + let alloca = entry_block_alloca_zerofill(env, field_type, "copied_tag"); if align_bytes > 0 { let size = env .ptr_int() @@ -2404,7 +2404,7 @@ pub fn load_roc_value<'a, 'ctx, 'env>( name: &str, ) -> BasicValueEnum<'ctx> { if layout.is_passed_by_reference() { - let alloca = tag_alloca(env, basic_type_from_layout(env, &layout), name); + let alloca = entry_block_alloca_zerofill(env, basic_type_from_layout(env, &layout), name); store_roc_value(env, layout, alloca, source.into()); @@ -2421,7 +2421,7 @@ pub fn use_roc_value<'a, 'ctx, 'env>( name: &str, ) -> BasicValueEnum<'ctx> { if layout.is_passed_by_reference() { - let alloca = tag_alloca(env, basic_type_from_layout(env, &layout), name); + let alloca = entry_block_alloca_zerofill(env, basic_type_from_layout(env, &layout), name); env.builder.build_store(alloca, source); @@ -4642,7 +4642,7 @@ pub fn call_roc_function<'a, 'ctx, 'env>( let mut arguments = Vec::from_iter_in(it, env.arena); let result_type = basic_type_from_layout(env, result_layout); - let result_alloca = tag_alloca(env, result_type, "result_value"); + let result_alloca = entry_block_alloca_zerofill(env, result_type, "result_value"); arguments.push(result_alloca.into()); From de77748dd551407b4eb5fe84fd63d6a1543f81f8 Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 26 Mar 2022 12:46:31 +0100 Subject: [PATCH 05/13] add additonal tests --- compiler/test_gen/src/gen_dict.rs | 14 +++++--- compiler/test_gen/src/gen_list.rs | 56 +++++++++++++++++++++++++++---- 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/compiler/test_gen/src/gen_dict.rs b/compiler/test_gen/src/gen_dict.rs index d02d550c28..1f0ffa7f77 100644 --- a/compiler/test_gen/src/gen_dict.rs +++ b/compiler/test_gen/src/gen_dict.rs @@ -202,7 +202,7 @@ fn values() { #[test] #[cfg(any(feature = "gen-llvm"))] -fn from_list_with_fold() { +fn from_list_with_fold_simple() { assert_evals_to!( indoc!( r#" @@ -217,7 +217,11 @@ fn from_list_with_fold() { RocList::from_slice(&[2, 3, 1]), RocList ); +} +#[test] +#[cfg(any(feature = "gen-llvm"))] +fn from_list_with_fold_reallocates() { assert_evals_to!( indoc!( r#" @@ -235,11 +239,13 @@ fn from_list_with_fold() { |> List.walk Dict.empty (\accum, value -> Dict.insert accum value value) Dict.values myDict - |> List.len "# ), - 25, - i64 + RocList::from_slice(&[ + 4, 5, 20, 0, 7, 3, 1, 21, 10, 6, 13, 9, 14, 19, 2, 15, 12, 17, 16, 18, 22, 8, 11, 24, + 23 + ]), + RocList ); } diff --git a/compiler/test_gen/src/gen_list.rs b/compiler/test_gen/src/gen_list.rs index dd870a6192..3e2c681146 100644 --- a/compiler/test_gen/src/gen_list.rs +++ b/compiler/test_gen/src/gen_list.rs @@ -2391,13 +2391,49 @@ fn list_wrap_in_tag() { #[test] #[cfg(any(feature = "gen-llvm"))] -fn list_contains() { +fn list_contains_int() { assert_evals_to!(indoc!("List.contains [1,2,3] 1"), true, bool); assert_evals_to!(indoc!("List.contains [1,2,3] 4"), false, bool); assert_evals_to!(indoc!("List.contains [] 4"), false, bool); } + +#[test] +#[cfg(any(feature = "gen-llvm"))] +fn list_contains_str() { + assert_evals_to!(indoc!(r#"List.contains ["foo", "bar"] "bar""#), true, bool); + + assert_evals_to!( + indoc!(r#"List.contains ["foo", "bar"] "spam""#), + false, + bool + ); + + assert_evals_to!(indoc!(r#"List.contains [] "spam""#), false, bool); +} + +#[test] +#[cfg(any(feature = "gen-llvm"))] +fn list_manual_range() { + assert_evals_to!( + indoc!( + r#" + range : I64, I64, List I64-> List I64 + range = \low, high, accum -> + if low < high then + range (low + 1) high (List.append accum low) + else + accum + + range 0 5 [ 42 ] + "# + ), + RocList::from_slice(&[42, 0, 1, 2, 3, 4]), + RocList + ); +} + #[test] #[cfg(any(feature = "gen-llvm"))] fn list_min() { @@ -2470,12 +2506,23 @@ fn list_product() { #[test] #[cfg(any(feature = "gen-llvm"))] -fn list_keep_oks() { +fn list_keep_void() { assert_evals_to!( "List.keepOks [] (\\x -> x)", RocList::from_slice(&[]), RocList<()> ); + + assert_evals_to!( + "List.keepErrs [] (\\x -> x)", + RocList::from_slice(&[]), + RocList<()> + ); +} + +#[test] +#[cfg(any(feature = "gen-llvm"))] +fn list_keep_oks() { assert_evals_to!( "List.keepOks [Ok {}, Ok {}] (\\x -> x)", RocList::from_slice(&[(), ()]), @@ -2501,11 +2548,6 @@ fn list_keep_oks() { #[test] #[cfg(any(feature = "gen-llvm"))] fn list_keep_errs() { - assert_evals_to!( - "List.keepErrs [] (\\x -> x)", - RocList::from_slice(&[]), - RocList<()> - ); assert_evals_to!( "List.keepErrs [Err {}, Err {}] (\\x -> x)", RocList::from_slice(&[(), ()]), From 85bf881b3b7198a49803c34c9b34236347d4471c Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 26 Mar 2022 12:36:14 +0100 Subject: [PATCH 06/13] make Dict.keys and Dict.values use list helpers for returning --- compiler/builtins/bitcode/src/dict.zig | 40 ++++++++++++++++++------ compiler/gen_llvm/src/llvm/build_dict.rs | 38 ++++------------------ 2 files changed, 37 insertions(+), 41 deletions(-) diff --git a/compiler/builtins/bitcode/src/dict.zig b/compiler/builtins/bitcode/src/dict.zig index d45008fbd2..d23cf0d11b 100644 --- a/compiler/builtins/bitcode/src/dict.zig +++ b/compiler/builtins/bitcode/src/dict.zig @@ -408,7 +408,19 @@ const Dec = fn (?[*]u8) callconv(.C) void; const Caller3 = fn (?[*]u8, ?[*]u8, ?[*]u8, ?[*]u8, ?[*]u8) callconv(.C) void; // Dict.insert : Dict k v, k, v -> Dict k v -pub fn dictInsert(input: RocDict, alignment: Alignment, key: Opaque, key_width: usize, value: Opaque, value_width: usize, hash_fn: HashFn, is_eq: EqFn, dec_key: Dec, dec_value: Dec, output: *RocDict) callconv(.C) void { +pub fn dictInsert( + input: RocDict, + alignment: Alignment, + key: Opaque, + key_width: usize, + value: Opaque, + value_width: usize, + hash_fn: HashFn, + is_eq: EqFn, + dec_key: Dec, + dec_value: Dec, + output: *RocDict, +) callconv(.C) void { var seed: u64 = INITIAL_SEED; var result = input.makeUnique(alignment, key_width, value_width); @@ -543,7 +555,13 @@ pub fn elementsRc(dict: RocDict, alignment: Alignment, key_width: usize, value_w } } -pub fn dictKeys(dict: RocDict, alignment: Alignment, key_width: usize, value_width: usize, inc_key: Inc, output: *RocList) callconv(.C) void { +pub fn dictKeys( + dict: RocDict, + alignment: Alignment, + key_width: usize, + value_width: usize, + inc_key: Inc, +) callconv(.C) RocList { const size = dict.capacity(); var length: usize = 0; @@ -558,8 +576,7 @@ pub fn dictKeys(dict: RocDict, alignment: Alignment, key_width: usize, value_wid } if (length == 0) { - output.* = RocList.empty(); - return; + return RocList.empty(); } const data_bytes = length * key_width; @@ -581,10 +598,16 @@ pub fn dictKeys(dict: RocDict, alignment: Alignment, key_width: usize, value_wid } } - output.* = RocList{ .bytes = ptr, .length = length }; + return RocList{ .bytes = ptr, .length = length }; } -pub fn dictValues(dict: RocDict, alignment: Alignment, key_width: usize, value_width: usize, inc_value: Inc, output: *RocList) callconv(.C) void { +pub fn dictValues( + dict: RocDict, + alignment: Alignment, + key_width: usize, + value_width: usize, + inc_value: Inc, +) callconv(.C) RocList { const size = dict.capacity(); var length: usize = 0; @@ -599,8 +622,7 @@ pub fn dictValues(dict: RocDict, alignment: Alignment, key_width: usize, value_w } if (length == 0) { - output.* = RocList.empty(); - return; + return RocList.empty(); } const data_bytes = length * value_width; @@ -622,7 +644,7 @@ pub fn dictValues(dict: RocDict, alignment: Alignment, key_width: usize, value_w } } - output.* = RocList{ .bytes = ptr, .length = length }; + return RocList{ .bytes = ptr, .length = length }; } fn doNothing(_: Opaque) callconv(.C) void { diff --git a/compiler/gen_llvm/src/llvm/build_dict.rs b/compiler/gen_llvm/src/llvm/build_dict.rs index b0f5aabb7f..9e9abe75d0 100644 --- a/compiler/gen_llvm/src/llvm/build_dict.rs +++ b/compiler/gen_llvm/src/llvm/build_dict.rs @@ -19,6 +19,8 @@ use roc_module::symbol::Symbol; use roc_mono::layout::{Builtin, Layout, LayoutIds}; use roc_target::TargetInfo; +use super::bitcode::call_list_bitcode_fn; + #[repr(transparent)] struct Alignment(u8); @@ -429,9 +431,7 @@ pub fn dict_keys<'a, 'ctx, 'env>( let inc_key_fn = build_inc_wrapper(env, layout_ids, key_layout); - let list_ptr = builder.build_alloca(zig_list_type(env), "list_ptr"); - - call_void_bitcode_fn( + call_list_bitcode_fn( env, &[ pass_dict_c_abi(env, dict), @@ -439,21 +439,9 @@ pub fn dict_keys<'a, 'ctx, 'env>( key_width.into(), value_width.into(), inc_key_fn.as_global_value().as_pointer_value().into(), - list_ptr.into(), ], bitcode::DICT_KEYS, - ); - - let list_ptr = env - .builder - .build_bitcast( - list_ptr, - super::convert::zig_list_type(env).ptr_type(AddressSpace::Generic), - "to_roc_list", - ) - .into_pointer_value(); - - env.builder.build_load(list_ptr, "load_keys_list") + ) } fn pass_dict_c_abi<'a, 'ctx, 'env>( @@ -687,9 +675,7 @@ pub fn dict_values<'a, 'ctx, 'env>( let inc_value_fn = build_inc_wrapper(env, layout_ids, value_layout); - let list_ptr = builder.build_alloca(zig_list_type, "list_ptr"); - - call_void_bitcode_fn( + call_list_bitcode_fn( env, &[ pass_dict_c_abi(env, dict), @@ -697,21 +683,9 @@ pub fn dict_values<'a, 'ctx, 'env>( key_width.into(), value_width.into(), inc_value_fn.as_global_value().as_pointer_value().into(), - list_ptr.into(), ], bitcode::DICT_VALUES, - ); - - let list_ptr = env - .builder - .build_bitcast( - list_ptr, - super::convert::zig_list_type(env).ptr_type(AddressSpace::Generic), - "to_roc_list", - ) - .into_pointer_value(); - - env.builder.build_load(list_ptr, "load_keys_list") + ) } #[allow(clippy::too_many_arguments)] From 49f624a27af9563c86a57c43047bae2d60ad0175 Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 26 Mar 2022 12:41:46 +0100 Subject: [PATCH 07/13] use store_roc_value when inserting into dict --- compiler/gen_llvm/src/llvm/build_dict.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build_dict.rs b/compiler/gen_llvm/src/llvm/build_dict.rs index 9e9abe75d0..352a7ee88e 100644 --- a/compiler/gen_llvm/src/llvm/build_dict.rs +++ b/compiler/gen_llvm/src/llvm/build_dict.rs @@ -20,6 +20,7 @@ use roc_mono::layout::{Builtin, Layout, LayoutIds}; use roc_target::TargetInfo; use super::bitcode::call_list_bitcode_fn; +use super::build::store_roc_value; #[repr(transparent)] struct Alignment(u8); @@ -105,11 +106,14 @@ pub fn dict_insert<'a, 'ctx, 'env>( let u8_ptr = env.context.i8_type().ptr_type(AddressSpace::Generic); - let key_ptr = builder.build_alloca(key.get_type(), "key_ptr"); - let value_ptr = builder.build_alloca(value.get_type(), "value_ptr"); + let key_type = basic_type_from_layout(env, key_layout); + let value_type = basic_type_from_layout(env, value_layout); - env.builder.build_store(key_ptr, key); - env.builder.build_store(value_ptr, value); + let key_ptr = builder.build_alloca(key_type, "key_ptr"); + let value_ptr = builder.build_alloca(value_type, "value_ptr"); + + store_roc_value(env, *key_layout, key_ptr, key); + store_roc_value(env, *value_layout, value_ptr, value); let key_width = env .ptr_int() @@ -788,7 +792,7 @@ fn build_hash_wrapper<'a, 'ctx, 'env>( let value_cast = env .builder - .build_bitcast(value_ptr, value_type, "load_opaque") + .build_bitcast(value_ptr, value_type, "cast_to_known_type") .into_pointer_value(); let val_arg = load_roc_value(env, *layout, value_cast, "load_opaque"); From e13ea550044f9288be469cc1a33a7d57bc0b264d Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 26 Mar 2022 17:56:36 +0100 Subject: [PATCH 08/13] use roc_load_value to move a value to the stack --- compiler/gen_llvm/src/llvm/refcounting.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/refcounting.rs b/compiler/gen_llvm/src/llvm/refcounting.rs index 0098432aa5..e42414279d 100644 --- a/compiler/gen_llvm/src/llvm/refcounting.rs +++ b/compiler/gen_llvm/src/llvm/refcounting.rs @@ -20,6 +20,7 @@ use roc_module::symbol::Symbol; use roc_mono::layout::{Builtin, Layout, LayoutIds, UnionLayout}; use roc_target::TargetInfo; +use super::build::load_roc_value; use super::convert::{argument_type_from_layout, argument_type_from_union_layout}; /// "Infinite" reference count, for static values @@ -1434,9 +1435,8 @@ fn build_rec_union_recursive_decrement<'a, 'ctx, 'env>( .build_struct_gep(struct_ptr, i as u32, "gep_recursive_pointer") .unwrap(); - let field = env - .builder - .build_load(elem_pointer, "decrement_struct_field"); + let field = + load_roc_value(env, *field_layout, elem_pointer, "decrement_struct_field"); deferred_nonrec.push((field, field_layout)); } From 5a3be2adf4015403483af8eb428c8e1f38968bdf Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 30 Mar 2022 19:18:16 +0200 Subject: [PATCH 09/13] give correct stack size without alignment for tags --- compiler/mono/src/layout.rs | 53 ++++++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index 5730de27fa..fcbbf15bff 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -554,6 +554,31 @@ impl<'a> UnionLayout<'a> { (size, alignment_bytes) } + + /// Very important to use this when doing a memcpy! + fn stack_size_without_alignment(&self, target_info: TargetInfo) -> u32 { + match self { + UnionLayout::NonRecursive(tags) => { + let id_layout = self.tag_id_layout(); + + let mut size = 0; + + for field_layouts in tags.iter() { + let fields = Layout::struct_no_name_order(field_layouts); + let fields_and_id = [fields, id_layout]; + + let data = Layout::struct_no_name_order(&fields_and_id); + size = size.max(data.stack_size_without_alignment(target_info)); + } + + size + } + UnionLayout::Recursive(_) + | UnionLayout::NonNullableUnwrapped(_) + | UnionLayout::NullableWrapped { .. } + | UnionLayout::NullableUnwrapped { .. } => target_info.ptr_width() as u32, + } + } } /// Custom type so we can get the numeric representation of a symbol in tests (so `#UserApp.3` @@ -1037,7 +1062,8 @@ impl<'a> Layout<'a> { (size, alignment) } - fn stack_size_without_alignment(&self, target_info: TargetInfo) -> u32 { + /// Very important to use this when doing a memcpy! + pub fn stack_size_without_alignment(&self, target_info: TargetInfo) -> u32 { use Layout::*; match self { @@ -1051,18 +1077,7 @@ impl<'a> Layout<'a> { sum } - Union(variant) => { - use UnionLayout::*; - - match variant { - NonRecursive(_) => variant.data_size_and_alignment(target_info).0, - - Recursive(_) - | NullableWrapped { .. } - | NullableUnwrapped { .. } - | NonNullableUnwrapped(_) => target_info.ptr_width() as u32, - } - } + Union(variant) => variant.stack_size_without_alignment(target_info), LambdaSet(lambda_set) => lambda_set .runtime_representation() .stack_size_without_alignment(target_info), @@ -2947,4 +2962,16 @@ mod test { assert_eq!(layout.stack_size(target_info), 1); assert_eq!(layout.alignment_bytes(target_info), 1); } + + #[test] + fn memcpy_size_result_u32_unit() { + let ok_tag = &[Layout::Builtin(Builtin::Int(IntWidth::U32))]; + let err_tag = &[Layout::UNIT]; + let tags = [ok_tag as &[_], err_tag as &[_]]; + let union_layout = UnionLayout::NonRecursive(&tags as &[_]); + let layout = Layout::Union(union_layout); + + let target_info = TargetInfo::default_x86_64(); + assert_eq!(layout.stack_size_without_alignment(target_info), 5); + } } From e94bdb0ed8e1e35e8fb0935160b571aade09ab92 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 30 Mar 2022 17:32:24 +0200 Subject: [PATCH 10/13] use a nullable pointer to store the output of strSplitInPlace --- compiler/builtins/bitcode/src/str.zig | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/compiler/builtins/bitcode/src/str.zig b/compiler/builtins/bitcode/src/str.zig index c264d94987..73ab7b771c 100644 --- a/compiler/builtins/bitcode/src/str.zig +++ b/compiler/builtins/bitcode/src/str.zig @@ -512,8 +512,12 @@ fn strFromFloatHelp(comptime T: type, float: T) RocStr { } // Str.split -pub fn strSplitInPlaceC(array: [*]RocStr, string: RocStr, delimiter: RocStr) callconv(.C) void { - return @call(.{ .modifier = always_inline }, strSplitInPlace, .{ array, string, delimiter }); +pub fn strSplitInPlaceC(opt_array: ?[*]RocStr, string: RocStr, delimiter: RocStr) callconv(.C) void { + if (opt_array) |array| { + return @call(.{ .modifier = always_inline }, strSplitInPlace, .{ array, string, delimiter }); + } else { + return; + } } fn strSplitInPlace(array: [*]RocStr, string: RocStr, delimiter: RocStr) void { From 786246f4277d04b7dae45281b7aee7b22384de91 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 30 Mar 2022 17:33:43 +0200 Subject: [PATCH 11/13] when doing a memcpy, don't take alignment into account --- compiler/gen_llvm/src/llvm/build.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 7afe216fc2..c13815bd5e 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -2567,9 +2567,10 @@ pub fn build_exp_stmt<'a, 'ctx, 'env>( if value_ptr.get_first_use().is_some() { value_ptr.replace_all_uses_with(destination); } else { - let size = env - .ptr_int() - .const_int(layout.stack_size(env.target_info) as u64, false); + let size = env.ptr_int().const_int( + layout.stack_size_without_alignment(env.target_info) as u64, + false, + ); env.builder .build_memcpy( From bd46103f0cd122c5be8acac5d833ae8968fe8d28 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 30 Mar 2022 17:34:04 +0200 Subject: [PATCH 12/13] use RocResult for some tests --- compiler/test_gen/src/gen_str.rs | 43 ++++++++------------------------ 1 file changed, 10 insertions(+), 33 deletions(-) diff --git a/compiler/test_gen/src/gen_str.rs b/compiler/test_gen/src/gen_str.rs index 633a7c56ab..c0b15c293d 100644 --- a/compiler/test_gen/src/gen_str.rs +++ b/compiler/test_gen/src/gen_str.rs @@ -13,7 +13,7 @@ use crate::helpers::dev::assert_evals_to as assert_llvm_evals_to; #[allow(unused_imports)] use indoc::indoc; #[allow(unused_imports)] -use roc_std::{RocList, RocStr}; +use roc_std::{RocList, RocResult, RocStr}; #[test] #[cfg(any(feature = "gen-llvm"))] @@ -103,18 +103,9 @@ fn str_split_str_concat_repeated() { #[cfg(any(feature = "gen-llvm"))] fn str_split_small_str_bigger_delimiter() { assert_evals_to!( - indoc!( - r#" - when - List.first - (Str.split "JJJ" "0123456789abcdefghi") - is - Ok str -> str - _ -> "" - "# - ), - RocStr::from("JJJ"), - RocStr + indoc!(r#"Str.split "JJJ" "0123456789abcdefghi""#), + RocList::from_slice(&[RocStr::from("JJJ")]), + RocList ); } @@ -1390,16 +1381,9 @@ fn str_to_i64() { #[cfg(any(feature = "gen-llvm"))] fn str_to_u64() { assert_evals_to!( - indoc!( - r#" - when Str.toU64 "1" is - Ok n -> n - Err _ -> 0 - - "# - ), - 1, - u64 + r#"Str.toU64 "1""#, + RocResult::ok(1u64), + RocResult ); } @@ -1424,16 +1408,9 @@ fn str_to_i32() { #[cfg(any(feature = "gen-llvm"))] fn str_to_u32() { assert_evals_to!( - indoc!( - r#" - when Str.toU32 "1" is - Ok n -> n - Err _ -> 0 - - "# - ), - 1, - u32 + r#"Str.toU32 "1""#, + RocResult::ok(1u32), + RocResult ); } From 108f2142bacddf8c08d4b3f7c01d07960951045f Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 30 Mar 2022 19:23:51 +0200 Subject: [PATCH 13/13] clippy --- compiler/gen_llvm/src/llvm/build_dict.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build_dict.rs b/compiler/gen_llvm/src/llvm/build_dict.rs index 352a7ee88e..d9c752fe47 100644 --- a/compiler/gen_llvm/src/llvm/build_dict.rs +++ b/compiler/gen_llvm/src/llvm/build_dict.rs @@ -7,7 +7,7 @@ use crate::llvm::build::{ Scope, }; use crate::llvm::build_list::{layout_width, pass_as_opaque}; -use crate::llvm::convert::{basic_type_from_layout, zig_dict_type, zig_list_type}; +use crate::llvm::convert::{basic_type_from_layout, zig_dict_type}; use crate::llvm::refcounting::Mode; use inkwell::attributes::{Attribute, AttributeLoc}; use inkwell::context::Context; @@ -420,8 +420,6 @@ pub fn dict_keys<'a, 'ctx, 'env>( key_layout: &Layout<'a>, value_layout: &Layout<'a>, ) -> BasicValueEnum<'ctx> { - let builder = env.builder; - let key_width = env .ptr_int() .const_int(key_layout.stack_size(env.target_info) as u64, false); @@ -662,10 +660,6 @@ pub fn dict_values<'a, 'ctx, 'env>( key_layout: &Layout<'a>, value_layout: &Layout<'a>, ) -> BasicValueEnum<'ctx> { - let builder = env.builder; - - let zig_list_type = super::convert::zig_list_type(env); - let key_width = env .ptr_int() .const_int(key_layout.stack_size(env.target_info) as u64, false);