From 56dc06a139c5c820ada0f76473ff06a5bca24ad8 Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Wed, 9 Mar 2022 14:07:52 +0100 Subject: [PATCH 1/7] fixed the previous formatting change when inside longer pipeline --- compiler/fmt/src/expr.rs | 8 ++++++-- compiler/fmt/tests/test_fmt.rs | 15 ++++++++++++++- examples/benchmarks/AStar.roc | 14 +++++++------- examples/interactive/echo.roc | 8 ++++---- 4 files changed, 31 insertions(+), 14 deletions(-) diff --git a/compiler/fmt/src/expr.rs b/compiler/fmt/src/expr.rs index f5cad3cc34..b54ff826b2 100644 --- a/compiler/fmt/src/expr.rs +++ b/compiler/fmt/src/expr.rs @@ -117,6 +117,7 @@ impl<'a> Formattable for Expr<'a> { ) { use self::Expr::*; + //dbg!(self); let format_newlines = newlines == Newlines::Yes; let apply_needs_parens = parens == Parens::InApply; @@ -453,14 +454,17 @@ fn fmt_bin_ops<'a, 'buf>( || (&loc_right_side.value).is_multiline() || lefts.iter().any(|(expr, _)| expr.value.is_multiline()); + let mut curr_indent = indent; + for (loc_left_side, loc_bin_op) in lefts { let bin_op = loc_bin_op.value; - loc_left_side.format_with_options(buf, apply_needs_parens, Newlines::No, indent); + loc_left_side.format_with_options(buf, apply_needs_parens, Newlines::No, curr_indent); if is_multiline { buf.newline(); - buf.indent(indent + INDENT); + curr_indent = indent + INDENT; + buf.indent(curr_indent); } else { buf.spaces(1); } diff --git a/compiler/fmt/tests/test_fmt.rs b/compiler/fmt/tests/test_fmt.rs index 72034e879e..c5bde106a7 100644 --- a/compiler/fmt/tests/test_fmt.rs +++ b/compiler/fmt/tests/test_fmt.rs @@ -2595,7 +2595,7 @@ mod test_fmt { } #[test] - fn pipline_apply_lambda() { + fn pipline_apply_lambda_1() { expr_formats_same(indoc!( r#" shout @@ -2606,6 +2606,19 @@ mod test_fmt { )); } + #[test] + fn pipline_apply_lambda_2() { + expr_formats_same(indoc!( + r#" + shout + |> List.map + xs + (\i -> i) + |> List.join + "# + )); + } + // MODULES #[test] diff --git a/examples/benchmarks/AStar.roc b/examples/benchmarks/AStar.roc index f4bd5ed452..4f5a8e0fa1 100644 --- a/examples/benchmarks/AStar.roc +++ b/examples/benchmarks/AStar.roc @@ -25,14 +25,14 @@ cheapestOpen = \costFn, model -> model.openSet |> Set.toList |> List.keepOks - (\position -> - when Dict.get model.costs position is - Err _ -> - Err {} + (\position -> + when Dict.get model.costs position is + Err _ -> + Err {} - Ok cost -> - Ok { cost: cost + costFn position, position } - ) + Ok cost -> + Ok { cost: cost + costFn position, position } + ) |> Quicksort.sortBy .cost |> List.first |> Result.map .position diff --git a/examples/interactive/echo.roc b/examples/interactive/echo.roc index 4eefae59f5..84ca8c668d 100644 --- a/examples/interactive/echo.roc +++ b/examples/interactive/echo.roc @@ -23,11 +23,11 @@ echo = \shout -> shout |> Str.toUtf8 |> List.mapWithIndex - (\_, i -> - length = (List.len (Str.toUtf8 shout) - i) - phrase = (List.split (Str.toUtf8 shout) length).before + (\_, i -> + length = (List.len (Str.toUtf8 shout) - i) + phrase = (List.split (Str.toUtf8 shout) length).before - List.concat (silence (if i == 0 then 2 * length else length)) phrase) + List.concat (silence (if i == 0 then 2 * length else length)) phrase) |> List.join |> Str.fromUtf8 |> Result.withDefault "" From 5d15166bb589e436a40c3d7720c954d8db6b35e9 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 9 Mar 2022 16:33:36 +0100 Subject: [PATCH 2/7] rename --- compiler/gen_llvm/src/llvm/build.rs | 4 +- compiler/gen_llvm/src/llvm/build_hash.rs | 24 +++---- compiler/gen_llvm/src/llvm/compare.rs | 4 +- compiler/gen_llvm/src/llvm/convert.rs | 86 ++++++++++++----------- compiler/gen_llvm/src/llvm/refcounting.rs | 4 +- 5 files changed, 60 insertions(+), 62 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index c043622f08..e2424b5396 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -24,7 +24,7 @@ use crate::llvm::build_str::{ }; use crate::llvm::compare::{generic_eq, generic_neq}; use crate::llvm::convert::{ - self, basic_type_from_builtin, basic_type_from_layout, basic_type_from_layout_1, + self, basic_type_from_builtin, basic_type_from_layout, basic_type_from_layout_tag_as_alloca, block_of_memory_slices, }; use crate::llvm::refcounting::{ @@ -4225,7 +4225,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_1(env, layout); + let arg_type = basic_type_from_layout_tag_as_alloca(env, layout); arg_basic_types.push(arg_type); } diff --git a/compiler/gen_llvm/src/llvm/build_hash.rs b/compiler/gen_llvm/src/llvm/build_hash.rs index aa89583c77..da9f07588f 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::{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 crate::llvm::convert::{basic_type_from_layout, basic_type_from_layout_tag_as_alloca}; use bumpalo::collections::Vec; use inkwell::values::{ BasicValue, BasicValueEnum, FunctionValue, IntValue, PointerValue, StructValue, @@ -13,6 +13,8 @@ use roc_builtins::bitcode; use roc_module::symbol::Symbol; use roc_mono::layout::{Builtin, Layout, LayoutIds, UnionLayout}; +use super::convert::stack_basic_type_from_union_layout; + #[derive(Clone, Debug)] enum WhenRecursive<'a> { Unreachable, @@ -68,9 +70,7 @@ fn build_hash_layout<'a, 'ctx, 'env>( when_recursive, ), - Layout::Union(union_layout) => { - build_hash_tag(env, layout_ids, layout, union_layout, seed, val) - } + Layout::Union(union_layout) => build_hash_tag(env, layout_ids, union_layout, seed, val), Layout::RecursivePointer => match when_recursive { WhenRecursive::Unreachable => { @@ -87,14 +87,7 @@ fn build_hash_layout<'a, 'ctx, 'env>( .build_bitcast(val, bt, "i64_to_opaque") .into_pointer_value(); - build_hash_tag( - env, - layout_ids, - &layout, - &union_layout, - seed, - field_cast.into(), - ) + build_hash_tag(env, layout_ids, &union_layout, seed, field_cast.into()) } }, } @@ -308,7 +301,6 @@ fn hash_struct<'a, 'ctx, 'env>( fn build_hash_tag<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, layout_ids: &mut LayoutIds<'a>, - layout: &Layout<'a>, union_layout: &UnionLayout<'a>, seed: IntValue<'ctx>, value: BasicValueEnum<'ctx>, @@ -318,7 +310,7 @@ fn build_hash_tag<'a, 'ctx, 'env>( let symbol = Symbol::GENERIC_HASH; let fn_name = layout_ids - .get(symbol, layout) + .get(symbol, &Layout::Union(*union_layout)) .to_symbol_string(symbol, &env.interns); let function = match env.module.get_function(fn_name.as_str()) { @@ -326,7 +318,7 @@ fn build_hash_tag<'a, 'ctx, 'env>( None => { let seed_type = env.context.i64_type(); - let arg_type = basic_type_from_layout_1(env, layout); + let arg_type = stack_basic_type_from_union_layout(env, union_layout); let function_value = crate::llvm::refcounting::build_header_help( env, @@ -805,7 +797,7 @@ fn hash_ptr_to_struct<'a, 'ctx, 'env>( ) -> IntValue<'ctx> { use inkwell::types::BasicType; - let wrapper_type = basic_type_from_layout_1(env, &Layout::Union(*union_layout)); + let wrapper_type = basic_type_from_layout_tag_as_alloca(env, &Layout::Union(*union_layout)); // cast the opaque pointer to a pointer of the correct shape let wrapper_ptr = env diff --git a/compiler/gen_llvm/src/llvm/compare.rs b/compiler/gen_llvm/src/llvm/compare.rs index 9f8a0928a6..372f3e174f 100644 --- a/compiler/gen_llvm/src/llvm/compare.rs +++ b/compiler/gen_llvm/src/llvm/compare.rs @@ -2,7 +2,7 @@ use crate::llvm::bitcode::call_bitcode_fn; 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, basic_type_from_layout_1}; +use crate::llvm::convert::{basic_type_from_layout, basic_type_from_layout_tag_as_alloca}; use bumpalo::collections::Vec; use inkwell::types::BasicType; use inkwell::values::{ @@ -780,7 +780,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_1(env, tag_layout); + let arg_type = basic_type_from_layout_tag_as_alloca(env, tag_layout); let function_value = crate::llvm::refcounting::build_header_help( env, diff --git a/compiler/gen_llvm/src/llvm/convert.rs b/compiler/gen_llvm/src/llvm/convert.rs index 8d516e539e..b1fbb80ed1 100644 --- a/compiler/gen_llvm/src/llvm/convert.rs +++ b/compiler/gen_llvm/src/llvm/convert.rs @@ -82,7 +82,50 @@ pub fn basic_type_from_layout<'a, 'ctx, 'env>( } } -pub fn basic_type_from_layout_1<'a, 'ctx, 'env>( +pub fn stack_basic_type_from_union_layout<'a, 'ctx, 'env>( + env: &crate::llvm::build::Env<'a, 'ctx, 'env>, + union_layout: &UnionLayout<'_>, +) -> BasicTypeEnum<'ctx> { + use UnionLayout::*; + + let tag_id_type = basic_type_from_layout(env, &union_layout.tag_id_layout()); + + match union_layout { + NonRecursive(tags) => { + let data = block_of_memory_slices(env.context, tags, env.target_info); + 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.target_info); + + if union_layout.stores_tag_id_as_data(env.target_info) { + 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.target_info); + block.ptr_type(AddressSpace::Generic).into() + } + NonNullableUnwrapped(fields) => { + let block = block_of_memory_slices(env.context, &[fields], env.target_info); + block.ptr_type(AddressSpace::Generic).into() + } + } +} + +/// If the layout is a layout of a non-recursive tag, +/// give not the representation of this tag value but a pointer type to that value. +pub fn basic_type_from_layout_tag_as_alloca<'a, 'ctx, 'env>( env: &crate::llvm::build::Env<'a, 'ctx, 'env>, layout: &Layout<'_>, ) -> BasicTypeEnum<'ctx> { @@ -94,46 +137,9 @@ pub fn basic_type_from_layout_1<'a, 'ctx, 'env>( .. } => 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(env, &union_layout.tag_id_layout()); - - match union_layout { - NonRecursive(tags) => { - let data = block_of_memory_slices(env.context, tags, env.target_info); - 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.target_info); - - if union_layout.stores_tag_id_as_data(env.target_info) { - 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.target_info); - block.ptr_type(AddressSpace::Generic).into() - } - NonNullableUnwrapped(fields) => { - let block = block_of_memory_slices(env.context, &[fields], env.target_info); - block.ptr_type(AddressSpace::Generic).into() - } - } + basic_type_from_layout_tag_as_alloca(env, &lambda_set.runtime_representation()) } + Union(union_layout) => stack_basic_type_from_union_layout(env, union_layout), RecursivePointer => { // TODO make this dynamic env.context diff --git a/compiler/gen_llvm/src/llvm/refcounting.rs b/compiler/gen_llvm/src/llvm/refcounting.rs index da820087bf..2518b7ee4b 100644 --- a/compiler/gen_llvm/src/llvm/refcounting.rs +++ b/compiler/gen_llvm/src/llvm/refcounting.rs @@ -5,7 +5,7 @@ use crate::llvm::build::{ 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}; +use crate::llvm::convert::{basic_type_from_layout, basic_type_from_layout_tag_as_alloca}; use bumpalo::collections::Vec; use inkwell::basic_block::BasicBlock; use inkwell::context::Context; @@ -1635,7 +1635,7 @@ 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_1(env, &layout); + let basic_type = basic_type_from_layout_tag_as_alloca(env, &layout); let function_value = build_header(env, basic_type, mode, &fn_name); modify_refcount_union_help( From 29e053abf3591545d864f004acfffd6f9116c00f Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 9 Mar 2022 19:01:03 +0100 Subject: [PATCH 3/7] clarify basic_type_from_layout function --- compiler/gen_llvm/src/llvm/build.rs | 4 +- compiler/gen_llvm/src/llvm/build_hash.rs | 8 +- compiler/gen_llvm/src/llvm/compare.rs | 12 +- compiler/gen_llvm/src/llvm/convert.rs | 157 +++++++++------------- compiler/gen_llvm/src/llvm/refcounting.rs | 9 +- 5 files changed, 77 insertions(+), 113 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index e2424b5396..81881bc834 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -24,7 +24,7 @@ use crate::llvm::build_str::{ }; use crate::llvm::compare::{generic_eq, generic_neq}; use crate::llvm::convert::{ - self, basic_type_from_builtin, basic_type_from_layout, basic_type_from_layout_tag_as_alloca, + self, argument_type_from_layout, basic_type_from_builtin, basic_type_from_layout, block_of_memory_slices, }; use crate::llvm::refcounting::{ @@ -4225,7 +4225,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_tag_as_alloca(env, layout); + let arg_type = argument_type_from_layout(env, layout); arg_basic_types.push(arg_type); } diff --git a/compiler/gen_llvm/src/llvm/build_hash.rs b/compiler/gen_llvm/src/llvm/build_hash.rs index da9f07588f..6b23798e5b 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::{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_tag_as_alloca}; +use crate::llvm::convert::basic_type_from_layout; use bumpalo::collections::Vec; use inkwell::values::{ BasicValue, BasicValueEnum, FunctionValue, IntValue, PointerValue, StructValue, @@ -13,7 +13,7 @@ use roc_builtins::bitcode; use roc_module::symbol::Symbol; use roc_mono::layout::{Builtin, Layout, LayoutIds, UnionLayout}; -use super::convert::stack_basic_type_from_union_layout; +use super::convert::argument_type_from_union_layout; #[derive(Clone, Debug)] enum WhenRecursive<'a> { @@ -318,7 +318,7 @@ fn build_hash_tag<'a, 'ctx, 'env>( None => { let seed_type = env.context.i64_type(); - let arg_type = stack_basic_type_from_union_layout(env, union_layout); + let arg_type = argument_type_from_union_layout(env, union_layout); let function_value = crate::llvm::refcounting::build_header_help( env, @@ -797,7 +797,7 @@ fn hash_ptr_to_struct<'a, 'ctx, 'env>( ) -> IntValue<'ctx> { use inkwell::types::BasicType; - let wrapper_type = basic_type_from_layout_tag_as_alloca(env, &Layout::Union(*union_layout)); + let wrapper_type = argument_type_from_union_layout(env, union_layout); // cast the opaque pointer to a pointer of the correct shape let wrapper_ptr = env diff --git a/compiler/gen_llvm/src/llvm/compare.rs b/compiler/gen_llvm/src/llvm/compare.rs index 372f3e174f..5d2e3d4bc6 100644 --- a/compiler/gen_llvm/src/llvm/compare.rs +++ b/compiler/gen_llvm/src/llvm/compare.rs @@ -2,7 +2,7 @@ use crate::llvm::bitcode::call_bitcode_fn; 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, basic_type_from_layout_tag_as_alloca}; +use crate::llvm::convert::basic_type_from_layout; use bumpalo::collections::Vec; use inkwell::types::BasicType; use inkwell::values::{ @@ -15,6 +15,7 @@ use roc_module::symbol::Symbol; use roc_mono::layout::{Builtin, Layout, LayoutIds, UnionLayout}; use super::build::load_roc_value; +use super::convert::argument_type_from_union_layout; #[derive(Clone, Debug)] enum WhenRecursive<'a> { @@ -176,7 +177,6 @@ fn build_eq<'a, 'ctx, 'env>( env, layout_ids, when_recursive, - lhs_layout, union_layout, lhs_val, rhs_val, @@ -207,7 +207,6 @@ fn build_eq<'a, 'ctx, 'env>( env, layout_ids, WhenRecursive::Loop(union_layout), - &layout, &union_layout, field1_cast.into(), field2_cast.into(), @@ -350,7 +349,6 @@ fn build_neq<'a, 'ctx, 'env>( env, layout_ids, when_recursive, - lhs_layout, union_layout, lhs_val, rhs_val, @@ -764,7 +762,6 @@ fn build_tag_eq<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, layout_ids: &mut LayoutIds<'a>, when_recursive: WhenRecursive<'a>, - tag_layout: &Layout<'a>, union_layout: &UnionLayout<'a>, tag1: BasicValueEnum<'ctx>, tag2: BasicValueEnum<'ctx>, @@ -772,15 +769,16 @@ fn build_tag_eq<'a, 'ctx, 'env>( let block = env.builder.get_insert_block().expect("to be in a function"); let di_location = env.builder.get_current_debug_location().unwrap(); + let tag_layout = Layout::Union(*union_layout); let symbol = Symbol::GENERIC_EQ; let fn_name = layout_ids - .get(symbol, tag_layout) + .get(symbol, &tag_layout) .to_symbol_string(symbol, &env.interns); let function = match env.module.get_function(fn_name.as_str()) { Some(function_value) => function_value, None => { - let arg_type = basic_type_from_layout_tag_as_alloca(env, tag_layout); + let arg_type = argument_type_from_union_layout(env, union_layout); let function_value = crate::llvm::refcounting::build_header_help( env, diff --git a/compiler/gen_llvm/src/llvm/convert.rs b/compiler/gen_llvm/src/llvm/convert.rs index b1fbb80ed1..58282d4cf5 100644 --- a/compiler/gen_llvm/src/llvm/convert.rs +++ b/compiler/gen_llvm/src/llvm/convert.rs @@ -1,3 +1,4 @@ +use crate::llvm::build::Env; use bumpalo::collections::Vec; use inkwell::context::Context; use inkwell::types::{BasicType, BasicTypeEnum, FloatType, IntType, StructType}; @@ -7,7 +8,7 @@ use roc_mono::layout::{Builtin, Layout, UnionLayout}; use roc_target::TargetInfo; fn basic_type_from_record<'a, 'ctx, 'env>( - env: &crate::llvm::build::Env<'a, 'ctx, 'env>, + env: &Env<'a, 'ctx, 'env>, fields: &[Layout<'_>], ) -> BasicTypeEnum<'ctx> { let mut field_types = Vec::with_capacity_in(fields.len(), env.arena); @@ -22,7 +23,7 @@ fn basic_type_from_record<'a, 'ctx, 'env>( } pub fn basic_type_from_layout<'a, 'ctx, 'env>( - env: &crate::llvm::build::Env<'a, 'ctx, 'env>, + env: &Env<'a, 'ctx, 'env>, layout: &Layout<'_>, ) -> BasicTypeEnum<'ctx> { use Layout::*; @@ -33,57 +34,19 @@ pub fn basic_type_from_layout<'a, 'ctx, 'env>( .. } => basic_type_from_record(env, sorted_fields), LambdaSet(lambda_set) => basic_type_from_layout(env, &lambda_set.runtime_representation()), - Union(union_layout) => { - use UnionLayout::*; - - let tag_id_type = basic_type_from_layout(env, &union_layout.tag_id_layout()); - - match union_layout { - NonRecursive(tags) => { - let data = block_of_memory_slices(env.context, tags, env.target_info); - - env.context.struct_type(&[data, tag_id_type], false).into() - } - Recursive(tags) - | NullableWrapped { - other_tags: tags, .. - } => { - let data = block_of_memory_slices(env.context, tags, env.target_info); - - if union_layout.stores_tag_id_as_data(env.target_info) { - 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.target_info); - block.ptr_type(AddressSpace::Generic).into() - } - NonNullableUnwrapped(fields) => { - let block = block_of_memory_slices(env.context, &[fields], env.target_info); - block.ptr_type(AddressSpace::Generic).into() - } - } - } - RecursivePointer => { - // TODO make this dynamic - env.context - .i64_type() - .ptr_type(AddressSpace::Generic) - .as_basic_type_enum() - } + Union(union_layout) => basic_type_from_union_layout(env, union_layout), + RecursivePointer => env + .context + .i64_type() + .ptr_type(AddressSpace::Generic) + .as_basic_type_enum(), Builtin(builtin) => basic_type_from_builtin(env, builtin), } } -pub fn stack_basic_type_from_union_layout<'a, 'ctx, 'env>( - env: &crate::llvm::build::Env<'a, 'ctx, 'env>, +pub fn basic_type_from_union_layout<'a, 'ctx, 'env>( + env: &Env<'a, 'ctx, 'env>, union_layout: &UnionLayout<'_>, ) -> BasicTypeEnum<'ctx> { use UnionLayout::*; @@ -93,9 +56,8 @@ pub fn stack_basic_type_from_union_layout<'a, 'ctx, 'env>( match union_layout { NonRecursive(tags) => { let data = block_of_memory_slices(env.context, tags, env.target_info); - let struct_type = env.context.struct_type(&[data, tag_id_type], false); - struct_type.ptr_type(AddressSpace::Generic).into() + env.context.struct_type(&[data, tag_id_type], false).into() } Recursive(tags) | NullableWrapped { @@ -123,37 +85,8 @@ pub fn stack_basic_type_from_union_layout<'a, 'ctx, 'env>( } } -/// If the layout is a layout of a non-recursive tag, -/// give not the representation of this tag value but a pointer type to that value. -pub fn basic_type_from_layout_tag_as_alloca<'a, 'ctx, 'env>( - env: &crate::llvm::build::Env<'a, 'ctx, 'env>, - layout: &Layout<'_>, -) -> BasicTypeEnum<'ctx> { - use Layout::*; - - match layout { - Struct { - field_layouts: sorted_fields, - .. - } => basic_type_from_record(env, sorted_fields), - LambdaSet(lambda_set) => { - basic_type_from_layout_tag_as_alloca(env, &lambda_set.runtime_representation()) - } - Union(union_layout) => stack_basic_type_from_union_layout(env, union_layout), - 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>, + env: &Env<'a, 'ctx, 'env>, builtin: &Builtin<'_>, ) -> BasicTypeEnum<'ctx> { use Builtin::*; @@ -172,8 +105,48 @@ pub fn basic_type_from_builtin<'a, 'ctx, 'env>( } } +/// Turn a layout into a BasicType that we use in LLVM function arguments. +/// +/// This makes it possible to pass values as something different from how they are typically stored. +/// Current differences +/// +/// - tag unions are passed by-reference. That means that +/// * `f : [ Some I64, None ] -> I64` is typed `{ { i64, i8 }, i64 }* -> i64` +/// * `f : { x : [ Some I64, None ] } -> I64 is typed `{ { { i64, i8 }, i64 } } -> i64` +/// +/// Ideas exist to have (bigger than 2 register) records also be passed by-reference, but this +/// is not currently implemented +pub fn argument_type_from_layout<'a, 'ctx, 'env>( + env: &Env<'a, 'ctx, 'env>, + layout: &Layout<'_>, +) -> BasicTypeEnum<'ctx> { + use Layout::*; + + match layout { + LambdaSet(lambda_set) => { + argument_type_from_layout(env, &lambda_set.runtime_representation()) + } + Union(union_layout) => argument_type_from_union_layout(env, union_layout), + other => basic_type_from_layout(env, other), + } +} + +/// Non-recursive tag unions are stored on the stack, but passed by-reference +pub fn argument_type_from_union_layout<'a, 'ctx, 'env>( + env: &Env<'a, 'ctx, 'env>, + union_layout: &UnionLayout<'_>, +) -> BasicTypeEnum<'ctx> { + let heap_type = basic_type_from_union_layout(env, union_layout); + + if let UnionLayout::NonRecursive(_) = union_layout { + heap_type.ptr_type(AddressSpace::Generic).into() + } else { + heap_type + } +} + pub fn int_type_from_int_width<'a, 'ctx, 'env>( - env: &crate::llvm::build::Env<'a, 'ctx, 'env>, + env: &Env<'a, 'ctx, 'env>, int_width: IntWidth, ) -> IntType<'ctx> { use IntWidth::*; @@ -188,7 +161,7 @@ pub fn int_type_from_int_width<'a, 'ctx, 'env>( } pub fn float_type_from_float_width<'a, 'ctx, 'env>( - env: &crate::llvm::build::Env<'a, 'ctx, 'env>, + env: &Env<'a, 'ctx, 'env>, float_width: FloatWidth, ) -> FloatType<'ctx> { use FloatWidth::*; @@ -273,33 +246,23 @@ pub fn str_list_int(ctx: &Context, target_info: TargetInfo) -> IntType<'_> { } } -pub fn zig_dict_type<'a, 'ctx, 'env>( - env: &crate::llvm::build::Env<'a, 'ctx, 'env>, -) -> StructType<'ctx> { +pub fn zig_dict_type<'a, 'ctx, 'env>(env: &Env<'a, 'ctx, 'env>) -> StructType<'ctx> { env.module.get_struct_type("dict.RocDict").unwrap() } -pub fn zig_list_type<'a, 'ctx, 'env>( - env: &crate::llvm::build::Env<'a, 'ctx, 'env>, -) -> StructType<'ctx> { +pub fn zig_list_type<'a, 'ctx, 'env>(env: &Env<'a, 'ctx, 'env>) -> StructType<'ctx> { env.module.get_struct_type("list.RocList").unwrap() } -pub fn zig_str_type<'a, 'ctx, 'env>( - env: &crate::llvm::build::Env<'a, 'ctx, 'env>, -) -> StructType<'ctx> { +pub fn zig_str_type<'a, 'ctx, 'env>(env: &Env<'a, 'ctx, 'env>) -> StructType<'ctx> { env.module.get_struct_type("str.RocStr").unwrap() } -pub fn zig_has_tag_id_type<'a, 'ctx, 'env>( - env: &crate::llvm::build::Env<'a, 'ctx, 'env>, -) -> StructType<'ctx> { +pub fn zig_has_tag_id_type<'a, 'ctx, 'env>(env: &Env<'a, 'ctx, 'env>) -> StructType<'ctx> { env.module.get_struct_type("list.HasTagId").unwrap() } -pub fn zig_with_overflow_roc_dec<'a, 'ctx, 'env>( - env: &crate::llvm::build::Env<'a, 'ctx, 'env>, -) -> StructType<'ctx> { +pub fn zig_with_overflow_roc_dec<'a, 'ctx, 'env>(env: &Env<'a, 'ctx, 'env>) -> StructType<'ctx> { env.module .get_struct_type("utils.WithOverflow(dec.RocDec)") .unwrap() diff --git a/compiler/gen_llvm/src/llvm/refcounting.rs b/compiler/gen_llvm/src/llvm/refcounting.rs index 2518b7ee4b..27da0fd7c1 100644 --- a/compiler/gen_llvm/src/llvm/refcounting.rs +++ b/compiler/gen_llvm/src/llvm/refcounting.rs @@ -5,7 +5,7 @@ use crate::llvm::build::{ 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_tag_as_alloca}; +use crate::llvm::convert::basic_type_from_layout; use bumpalo::collections::Vec; use inkwell::basic_block::BasicBlock; use inkwell::context::Context; @@ -20,6 +20,8 @@ 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; + /// "Infinite" reference count, for static values /// Ref counts are encoded as negative numbers where isize::MIN represents 1 pub const REFCOUNT_MAX: usize = 0_usize; @@ -1618,7 +1620,8 @@ fn modify_refcount_union<'a, 'ctx, 'env>( when_recursive: &WhenRecursive<'a>, fields: &'a [&'a [Layout<'a>]], ) -> FunctionValue<'ctx> { - let layout = Layout::Union(UnionLayout::NonRecursive(fields)); + let union_layout = UnionLayout::NonRecursive(fields); + let layout = Layout::Union(union_layout); let block = env.builder.get_insert_block().expect("to be in a function"); let di_location = env.builder.get_current_debug_location().unwrap(); @@ -1635,7 +1638,7 @@ 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_tag_as_alloca(env, &layout); + let basic_type = argument_type_from_union_layout(env, &union_layout); let function_value = build_header(env, basic_type, mode, &fn_name); modify_refcount_union_help( From de6349fbbd39e5f187bbae0b05f1ffe7270a4e71 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 9 Mar 2022 22:54:56 +0100 Subject: [PATCH 4/7] don't double-alloca non-recursive tags --- compiler/gen_llvm/src/llvm/build.rs | 1 + compiler/gen_llvm/src/llvm/build_list.rs | 25 ++++++++++++++++++------ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index c043622f08..577fd54956 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -2581,6 +2581,7 @@ pub fn build_exp_stmt<'a, 'ctx, 'env>( let align_bytes = layout.alignment_bytes(env.target_info); if align_bytes > 0 { + debug_assert!(value.is_pointer_value(), "{:?}\n{:?}", value, layout); let value_ptr = value.into_pointer_value(); // We can only do this if the function itself writes data into this diff --git a/compiler/gen_llvm/src/llvm/build_list.rs b/compiler/gen_llvm/src/llvm/build_list.rs index dca6e49466..0f7a601e7f 100644 --- a/compiler/gen_llvm/src/llvm/build_list.rs +++ b/compiler/gen_llvm/src/llvm/build_list.rs @@ -43,7 +43,7 @@ fn pass_element_as_opaque<'a, 'ctx, 'env>( env.builder.build_bitcast( element_ptr, env.context.i8_type().ptr_type(AddressSpace::Generic), - "to_opaque", + "pass_element_as_opaque", ) } @@ -75,7 +75,7 @@ pub fn pass_as_opaque<'a, 'ctx, 'env>( env.builder.build_bitcast( ptr, env.context.i8_type().ptr_type(AddressSpace::Generic), - "to_opaque", + "pass_as_opaque", ) } @@ -407,10 +407,19 @@ pub fn list_walk_generic<'a, 'ctx, 'env>( ListWalk::WalkBackwardsUntil => todo!(), }; - let default_ptr = builder.build_alloca(default.get_type(), "default_ptr"); - env.builder.build_store(default_ptr, default); + let default_ptr = if default_layout.is_passed_by_reference() { + debug_assert!(default.is_pointer_value()); + default.into_pointer_value() + } else { + let default_ptr = builder.build_alloca(default.get_type(), "default_ptr"); + env.builder.build_store(default_ptr, default); + default_ptr + }; - let result_ptr = env.builder.build_alloca(default.get_type(), "result"); + let result_ptr = { + let basic_type = basic_type_from_layout(env, default_layout); + env.builder.build_alloca(basic_type, "result") + }; match variant { ListWalk::Walk | ListWalk::WalkBackwards => { @@ -467,7 +476,11 @@ pub fn list_walk_generic<'a, 'ctx, 'env>( } } - env.builder.build_load(result_ptr, "load_result") + if default_layout.is_passed_by_reference() { + result_ptr.into() + } else { + env.builder.build_load(result_ptr, "load_result") + } } /// List.range : Int a, Int a -> List (Int a) From 98b2ed5d60a10fd0586114af9bb980e4a257c3b7 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 9 Mar 2022 23:07:24 +0100 Subject: [PATCH 5/7] add test --- compiler/test_gen/src/gen_list.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/compiler/test_gen/src/gen_list.rs b/compiler/test_gen/src/gen_list.rs index 41635ea927..dd870a6192 100644 --- a/compiler/test_gen/src/gen_list.rs +++ b/compiler/test_gen/src/gen_list.rs @@ -798,6 +798,34 @@ fn list_walk_until_sum() { ); } +#[test] +#[cfg(any(feature = "gen-llvm"))] +fn list_walk_imlements_position() { + assert_evals_to!( + r#" + Option a : [ Some a, None ] + + find : List a, a -> Option Nat + find = \list, needle -> + findHelp list needle + |> .v + + findHelp = \list, needle -> + List.walkUntil list { n: 0, v: None } \{ n, v }, element -> + if element == needle then + Stop { n, v: Some n } + else + Continue { n: n + 1, v } + + when find [ 1, 2, 3 ] 3 is + None -> 0 + Some v -> v + "#, + 2, + i64 + ); +} + #[test] #[cfg(any(feature = "gen-llvm"))] fn list_walk_until_even_prefix_sum() { From 381ec9c01bb1e2410a53837b9b47dce961cac785 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 9 Mar 2022 23:10:07 +0100 Subject: [PATCH 6/7] make type conversion work on 32-bit architectures --- compiler/gen_llvm/src/llvm/convert.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/convert.rs b/compiler/gen_llvm/src/llvm/convert.rs index 58282d4cf5..1bf3fd585b 100644 --- a/compiler/gen_llvm/src/llvm/convert.rs +++ b/compiler/gen_llvm/src/llvm/convert.rs @@ -36,8 +36,7 @@ pub fn basic_type_from_layout<'a, 'ctx, 'env>( LambdaSet(lambda_set) => basic_type_from_layout(env, &lambda_set.runtime_representation()), Union(union_layout) => basic_type_from_union_layout(env, union_layout), RecursivePointer => env - .context - .i64_type() + .ptr_int() .ptr_type(AddressSpace::Generic) .as_basic_type_enum(), From f8ca0694e5ff4afd587bac2646af94cfba2a9282 Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 10 Mar 2022 00:04:34 +0100 Subject: [PATCH 7/7] Revert "make type conversion work on 32-bit architectures" This reverts commit 381ec9c01bb1e2410a53837b9b47dce961cac785. --- compiler/gen_llvm/src/llvm/convert.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/gen_llvm/src/llvm/convert.rs b/compiler/gen_llvm/src/llvm/convert.rs index 1bf3fd585b..58282d4cf5 100644 --- a/compiler/gen_llvm/src/llvm/convert.rs +++ b/compiler/gen_llvm/src/llvm/convert.rs @@ -36,7 +36,8 @@ pub fn basic_type_from_layout<'a, 'ctx, 'env>( LambdaSet(lambda_set) => basic_type_from_layout(env, &lambda_set.runtime_representation()), Union(union_layout) => basic_type_from_union_layout(env, union_layout), RecursivePointer => env - .ptr_int() + .context + .i64_type() .ptr_type(AddressSpace::Generic) .as_basic_type_enum(),