From d2b0ecdd04d2d7771841e165e5ff220fde17af65 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 19 May 2021 16:07:50 +0200 Subject: [PATCH] fix List.map2 --- compiler/builtins/bitcode/src/list.zig | 71 ++++++++++++-------------- compiler/builtins/bitcode/src/str.zig | 2 +- compiler/gen/src/llvm/build.rs | 36 ++++++++----- compiler/gen/src/llvm/build_list.rs | 34 +++--------- compiler/load/src/file.rs | 4 +- compiler/mono/src/inc_dec.rs | 18 +++---- 6 files changed, 77 insertions(+), 88 deletions(-) diff --git a/compiler/builtins/bitcode/src/list.zig b/compiler/builtins/bitcode/src/list.zig index c9a058e653..befae8585b 100644 --- a/compiler/builtins/bitcode/src/list.zig +++ b/compiler/builtins/bitcode/src/list.zig @@ -242,9 +242,41 @@ pub fn listMapWithIndex( } } -pub fn listMap2(list1: RocList, list2: RocList, transform: Opaque, caller: Caller2, alignment: usize, a_width: usize, b_width: usize, c_width: usize, dec_a: Dec, dec_b: Dec) callconv(.C) RocList { +fn decrementTail(list: RocList, start_index: usize, element_width: usize, dec: Dec) void { + if (list.bytes) |source| { + var i = start_index; + while (i < list.len()) : (i += 1) { + const element = source + i * element_width; + dec(element); + } + } +} + +pub fn listMap2( + list1: RocList, + list2: RocList, + caller: Caller2, + data: Opaque, + inc_n_data: IncN, + data_is_owned: bool, + alignment: usize, + a_width: usize, + b_width: usize, + c_width: usize, + dec_a: Dec, + dec_b: Dec, +) callconv(.C) RocList { const output_length = std.math.min(list1.len(), list2.len()); + // if the lists don't have equal length, we must consume the remaining elements + // In this case we consume by (recursively) decrementing the elements + decrementTail(list1, output_length, a_width, dec_a); + decrementTail(list2, output_length, b_width, dec_b); + + if (data_is_owned) { + inc_n_data(data, output_length); + } + if (list1.bytes) |source_a| { if (list2.bytes) |source_b| { const output = RocList.allocate(std.heap.c_allocator, alignment, output_length, c_width); @@ -255,49 +287,14 @@ pub fn listMap2(list1: RocList, list2: RocList, transform: Opaque, caller: Calle const element_a = source_a + i * a_width; const element_b = source_b + i * b_width; const target = target_ptr + i * c_width; - caller(transform, element_a, element_b, target); + caller(data, element_a, element_b, target); } - // if the lists don't have equal length, we must consume the remaining elements - // In this case we consume by (recursively) decrementing the elements - if (list1.len() > output_length) { - while (i < list1.len()) : (i += 1) { - const element_a = source_a + i * a_width; - dec_a(element_a); - } - } else if (list2.len() > output_length) { - while (i < list2.len()) : (i += 1) { - const element_b = source_b + i * b_width; - dec_b(element_b); - } - } - - utils.decref(std.heap.c_allocator, alignment, list1.bytes, list1.len() * a_width); - utils.decref(std.heap.c_allocator, alignment, list2.bytes, list2.len() * b_width); - return output; } else { - // consume list1 elements (we know there is at least one because the list1.bytes pointer is non-null - var i: usize = 0; - while (i < list1.len()) : (i += 1) { - const element_a = source_a + i * a_width; - dec_a(element_a); - } - utils.decref(std.heap.c_allocator, alignment, list1.bytes, list1.len() * a_width); - return RocList.empty(); } } else { - // consume list2 elements (if any) - if (list2.bytes) |source_b| { - var i: usize = 0; - while (i < list2.len()) : (i += 1) { - const element_b = source_b + i * b_width; - dec_b(element_b); - } - utils.decref(std.heap.c_allocator, alignment, list2.bytes, list2.len() * b_width); - } - return RocList.empty(); } } diff --git a/compiler/builtins/bitcode/src/str.zig b/compiler/builtins/bitcode/src/str.zig index f7005642d9..ac75340589 100644 --- a/compiler/builtins/bitcode/src/str.zig +++ b/compiler/builtins/bitcode/src/str.zig @@ -930,7 +930,7 @@ fn strConcat(allocator: *Allocator, result_in_place: InPlace, arg1: RocStr, arg2 return RocStr.clone(allocator, result_in_place, arg2); } else if (arg2.isEmpty()) { // the first argument is owned, so we can return it without cloning - return RocStr.clone(allocator, result_in_place, arg1); + return arg1; } else { const combined_length = arg1.len() + arg2.len(); diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index 1796103b0c..908c54b62b 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -3743,18 +3743,30 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( ( Layout::Builtin(Builtin::List(_, element1_layout)), Layout::Builtin(Builtin::List(_, element2_layout)), - ) => list_map2( - env, - layout_ids, - function, - function_layout, - closure, - *closure_layout, - list1, - list2, - element1_layout, - element2_layout, - ), + ) => { + let argument_layouts = &[**element1_layout, **element2_layout]; + + let roc_function_call = roc_function_call( + env, + layout_ids, + function, + closure, + *closure_layout, + function_owns_closure_data, + argument_layouts, + ); + + list_map2( + env, + layout_ids, + roc_function_call, + list1, + list2, + element1_layout, + element2_layout, + return_layout, + ) + } (Layout::Builtin(Builtin::EmptyList), _) | (_, Layout::Builtin(Builtin::EmptyList)) => empty_list(env), _ => unreachable!("invalid list layout"), diff --git a/compiler/gen/src/llvm/build_list.rs b/compiler/gen/src/llvm/build_list.rs index cdd4dd160d..9a0417240a 100644 --- a/compiler/gen/src/llvm/build_list.rs +++ b/compiler/gen/src/llvm/build_list.rs @@ -886,35 +886,13 @@ pub fn list_map<'a, 'ctx, 'env>( pub fn list_map2<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, layout_ids: &mut LayoutIds<'a>, - transform: FunctionValue<'ctx>, - transform_layout: Layout<'a>, - closure_data: BasicValueEnum<'ctx>, - closure_data_layout: Layout<'a>, + roc_function_call: RocFunctionCall<'ctx>, list1: BasicValueEnum<'ctx>, list2: BasicValueEnum<'ctx>, element1_layout: &Layout<'a>, element2_layout: &Layout<'a>, + return_layout: &Layout<'a>, ) -> BasicValueEnum<'ctx> { - let builder = env.builder; - - let return_layout = match transform_layout { - Layout::FunctionPointer(_, ret) => ret, - Layout::Closure(_, _, ret) => ret, - _ => unreachable!("not a callable layout"), - }; - - let closure_data_ptr = builder.build_alloca(closure_data.get_type(), "closure_data_ptr"); - env.builder.build_store(closure_data_ptr, closure_data); - - let stepper_caller = build_transform_caller_new( - env, - transform, - closure_data_layout, - &[*element1_layout, *element2_layout], - ) - .as_global_value() - .as_pointer_value(); - let a_width = env .ptr_int() .const_int(element1_layout.stack_size(env.ptr_bytes) as u64, false); @@ -935,9 +913,11 @@ pub fn list_map2<'a, 'ctx, 'env>( &[ pass_list_as_i128(env, list1), pass_list_as_i128(env, list2), - pass_as_opaque(env, closure_data_ptr), - stepper_caller.into(), - alignment_intvalue(env, &transform_layout), + roc_function_call.caller.into(), + pass_as_opaque(env, roc_function_call.data), + roc_function_call.inc_n_data.into(), + roc_function_call.data_is_owned.into(), + alignment_intvalue(env, return_layout), a_width.into(), b_width.into(), c_width.into(), diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index 816d57ad17..ce6f0098ae 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -2048,6 +2048,8 @@ fn update<'a>( && state.dependencies.solved_all() && state.goal_phase == Phase::MakeSpecializations { + Proc::insert_refcount_operations(arena, &mut state.procedures); + // display the mono IR of the module, for debug purposes if roc_mono::ir::PRETTY_PRINT_IR_SYMBOLS { let procs_string = state @@ -2067,8 +2069,6 @@ fn update<'a>( // &mut state.procedures, // ); - Proc::insert_refcount_operations(arena, &mut state.procedures); - state.constrained_ident_ids.insert(module_id, ident_ids); for (module_id, requested) in external_specializations_requested { diff --git a/compiler/mono/src/inc_dec.rs b/compiler/mono/src/inc_dec.rs index 3ae307ee52..eb8ed6002c 100644 --- a/compiler/mono/src/inc_dec.rs +++ b/compiler/mono/src/inc_dec.rs @@ -459,14 +459,14 @@ impl<'a> Context<'a> { macro_rules! create_call { ($borrows:expr) => { Expr::Call(crate::ir::Call { - call_type: if $borrows { - call_type - } else { + call_type: if let Some(OWNED) = $borrows.map(|p| p.borrow) { HigherOrderLowLevel { op: *op, closure_layout: *closure_layout, function_owns_closure_data: true, } + } else { + call_type }, arguments, }) @@ -505,7 +505,7 @@ impl<'a> Context<'a> { // if the list is owned, then all elements have been consumed, but not the list itself let b = decref_if_owned!(function_ps[0].borrow, arguments[0], b); - let v = create_call!(function_ps[1].borrow); + let v = create_call!(function_ps.get(1)); &*self.arena.alloc(Stmt::Let(z, v, l, b)) } @@ -526,7 +526,7 @@ impl<'a> Context<'a> { let b = decref_if_owned!(function_ps[1].borrow, arguments[0], b); - let v = create_call!(function_ps[2].borrow); + let v = create_call!(function_ps.get(2)); &*self.arena.alloc(Stmt::Let(z, v, l, b)) } @@ -537,8 +537,8 @@ impl<'a> Context<'a> { match self.param_map.get_symbol(arguments[2], *closure_layout) { Some(function_ps) => { let borrows = [ + function_ps[0].borrow, function_ps[1].borrow, - function_ps[2].borrow, FUNCTION, CLOSURE_DATA, ]; @@ -550,10 +550,10 @@ impl<'a> Context<'a> { b_live_vars, ); - let b = decref_if_owned!(function_ps[1].borrow, arguments[0], b); - let b = decref_if_owned!(function_ps[2].borrow, arguments[1], b); + let b = decref_if_owned!(function_ps[0].borrow, arguments[0], b); + let b = decref_if_owned!(function_ps[1].borrow, arguments[1], b); - let v = create_call!(function_ps[2].borrow); + let v = create_call!(function_ps.get(2)); &*self.arena.alloc(Stmt::Let(z, v, l, b)) }