From ca5e9518d9b55c28ef48419df88446e4973342e4 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 26 May 2021 14:12:53 +0200 Subject: [PATCH 1/4] remove PhantomEmptyStruct --- cli/src/repl/eval.rs | 6 ------ compiler/gen/src/llvm/build.rs | 1 - compiler/gen/src/llvm/build_hash.rs | 5 ----- compiler/gen/src/llvm/compare.rs | 10 ---------- compiler/gen/src/llvm/convert.rs | 1 - compiler/gen/src/llvm/refcounting.rs | 2 -- compiler/mono/src/alias_analysis.rs | 1 - compiler/mono/src/layout.rs | 13 +------------ 8 files changed, 1 insertion(+), 38 deletions(-) diff --git a/cli/src/repl/eval.rs b/cli/src/repl/eval.rs index 94f51ec6cf..1f16b2dcca 100644 --- a/cli/src/repl/eval.rs +++ b/cli/src/repl/eval.rs @@ -125,12 +125,6 @@ fn jit_to_ast_help<'a>( Layout::Builtin(other) => { todo!("add support for rendering builtin {:?} to the REPL", other) } - Layout::PhantomEmptyStruct => Ok(run_jit_function!(lib, main_fn_name, &u8, |_| { - Expr::Record { - fields: &[], - final_comments: env.arena.alloc([]), - } - })), Layout::Struct(field_layouts) => { let ptr_to_ast = |ptr: *const u8| match content { Content::Structure(FlatType::Record(fields, _)) => { diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index c1a0006844..92b587acf4 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -3360,7 +3360,6 @@ pub fn build_proc<'a, 'ctx, 'env>( } Layout::Builtin(_) => {} - Layout::PhantomEmptyStruct => {} Layout::Struct(_) => {} Layout::Union(_) => {} Layout::RecursivePointer => {} diff --git a/compiler/gen/src/llvm/build_hash.rs b/compiler/gen/src/llvm/build_hash.rs index 95db536a88..9dafe0cb2f 100644 --- a/compiler/gen/src/llvm/build_hash.rs +++ b/compiler/gen/src/llvm/build_hash.rs @@ -56,11 +56,6 @@ fn build_hash_layout<'a, 'ctx, 'env>( val.into_struct_value(), ), - Layout::PhantomEmptyStruct => { - // just does nothing and returns the seed - seed - } - Layout::Union(union_layout) => { build_hash_tag(env, layout_ids, layout, union_layout, seed, val) } diff --git a/compiler/gen/src/llvm/compare.rs b/compiler/gen/src/llvm/compare.rs index 0cf32263d0..bd4381fd0c 100644 --- a/compiler/gen/src/llvm/compare.rs +++ b/compiler/gen/src/llvm/compare.rs @@ -159,11 +159,6 @@ fn build_eq<'a, 'ctx, 'env>( rhs_val, ), - Layout::PhantomEmptyStruct => { - // always equal to itself - env.context.bool_type().const_int(1, false).into() - } - Layout::RecursivePointer => match when_recursive { WhenRecursive::Unreachable => { unreachable!("recursion pointers should never be compared directly") @@ -338,11 +333,6 @@ fn build_neq<'a, 'ctx, 'env>( result.into() } - Layout::PhantomEmptyStruct => { - // always equal to itself - env.context.bool_type().const_int(1, false).into() - } - Layout::RecursivePointer => { unreachable!("recursion pointers should never be compared directly") } diff --git a/compiler/gen/src/llvm/convert.rs b/compiler/gen/src/llvm/convert.rs index 29e874c992..a88c0ce7f5 100644 --- a/compiler/gen/src/llvm/convert.rs +++ b/compiler/gen/src/llvm/convert.rs @@ -115,7 +115,6 @@ pub fn basic_type_from_layout<'a, 'ctx, 'env>( Pointer(layout) => basic_type_from_layout(env, &layout) .ptr_type(AddressSpace::Generic) .into(), - PhantomEmptyStruct => env.context.struct_type(&[], false).into(), Struct(sorted_fields) => basic_type_from_record(env, sorted_fields), Union(variant) => { use UnionLayout::*; diff --git a/compiler/gen/src/llvm/refcounting.rs b/compiler/gen/src/llvm/refcounting.rs index cf0a254163..494df692b0 100644 --- a/compiler/gen/src/llvm/refcounting.rs +++ b/compiler/gen/src/llvm/refcounting.rs @@ -724,8 +724,6 @@ fn modify_refcount_layout_build_function<'a, 'ctx, 'env>( Some(function) } - PhantomEmptyStruct => None, - Layout::RecursivePointer => match when_recursive { WhenRecursive::Unreachable => { unreachable!("recursion pointers should never be hashed directly") diff --git a/compiler/mono/src/alias_analysis.rs b/compiler/mono/src/alias_analysis.rs index ebca28d1a5..0fb36ae318 100644 --- a/compiler/mono/src/alias_analysis.rs +++ b/compiler/mono/src/alias_analysis.rs @@ -525,7 +525,6 @@ fn layout_spec(builder: &mut FuncDefBuilder, layout: &Layout) -> Result match layout { Builtin(builtin) => builtin_spec(builder, builtin), - PhantomEmptyStruct => todo!(), Struct(fields) => build_tuple_type(builder, fields), Union(union_layout) => { let variant_types = build_variant_types_help(builder, union_layout)?; diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index 37f8669418..7877894c8a 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -36,7 +36,6 @@ pub enum Layout<'a> { /// A layout that is empty (turns into the empty struct in LLVM IR /// but for our purposes, not zero-sized, so it does not get dropped from data structures /// this is important for closures that capture zero-sized values - PhantomEmptyStruct, Struct(&'a [Layout<'a>]), Union(UnionLayout<'a>), RecursivePointer, @@ -479,7 +478,6 @@ impl<'a> Layout<'a> { match self { Builtin(builtin) => builtin.safe_to_memcpy(), - PhantomEmptyStruct => true, Struct(fields) => fields .iter() .all(|field_layout| field_layout.safe_to_memcpy()), @@ -519,12 +517,7 @@ impl<'a> Layout<'a> { // For this calculation, we don't need an accurate // stack size, we just need to know whether it's zero, // so it's fine to use a pointer size of 1. - if let Layout::PhantomEmptyStruct = self { - false - } else { - // self.stack_size(1) == 0 - false - } + false } pub fn stack_size(&self, pointer_size: u32) -> u32 { @@ -532,7 +525,6 @@ impl<'a> Layout<'a> { match self { Builtin(builtin) => builtin.stack_size(pointer_size), - PhantomEmptyStruct => 0, Struct(fields) => { let mut sum = 0; @@ -596,7 +588,6 @@ impl<'a> Layout<'a> { } } Layout::Builtin(builtin) => builtin.alignment_bytes(pointer_size), - Layout::PhantomEmptyStruct => 0, Layout::RecursivePointer => pointer_size, Layout::FunctionPointer(_, _) => pointer_size, Layout::Pointer(_) => pointer_size, @@ -636,7 +627,6 @@ impl<'a> Layout<'a> { match self { Builtin(builtin) => builtin.is_refcounted(), - PhantomEmptyStruct => false, Struct(fields) => fields.iter().any(|f| f.contains_refcounted()), Union(variant) => { use UnionLayout::*; @@ -669,7 +659,6 @@ impl<'a> Layout<'a> { match self { Builtin(builtin) => builtin.to_doc(alloc, parens), - PhantomEmptyStruct => alloc.text("{}"), Struct(fields) => { let fields_doc = fields.iter().map(|x| x.to_doc(alloc, parens)); From bfe49d2188a4a56f636dffee311fc3b2c6170667 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 26 May 2021 14:18:19 +0200 Subject: [PATCH 2/4] remove Pointer --- cli/src/repl/eval.rs | 1 - compiler/gen/src/llvm/build.rs | 1 - compiler/gen/src/llvm/build_hash.rs | 4 ---- compiler/gen/src/llvm/compare.rs | 8 -------- compiler/gen/src/llvm/convert.rs | 3 --- compiler/gen/src/llvm/refcounting.rs | 2 +- compiler/mono/src/alias_analysis.rs | 1 - compiler/mono/src/layout.rs | 10 +--------- 8 files changed, 2 insertions(+), 28 deletions(-) diff --git a/cli/src/repl/eval.rs b/cli/src/repl/eval.rs index 1f16b2dcca..0c363dfa3b 100644 --- a/cli/src/repl/eval.rs +++ b/cli/src/repl/eval.rs @@ -241,7 +241,6 @@ fn jit_to_ast_help<'a>( Layout::Closure(_, _, _) => Err(ToAstProblem::FunctionLayout), Layout::FunctionPointer(_, _) => Err(ToAstProblem::FunctionLayout), - Layout::Pointer(_) => todo!("add support for rendering pointers in the REPL"), } } diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index 92b587acf4..03e96b035b 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -3363,7 +3363,6 @@ pub fn build_proc<'a, 'ctx, 'env>( Layout::Struct(_) => {} Layout::Union(_) => {} Layout::RecursivePointer => {} - Layout::Pointer(_) => {} } } } diff --git a/compiler/gen/src/llvm/build_hash.rs b/compiler/gen/src/llvm/build_hash.rs index 9dafe0cb2f..dab6eee5db 100644 --- a/compiler/gen/src/llvm/build_hash.rs +++ b/compiler/gen/src/llvm/build_hash.rs @@ -86,10 +86,6 @@ fn build_hash_layout<'a, 'ctx, 'env>( } }, - Layout::Pointer(_) => { - unreachable!("unused") - } - Layout::FunctionPointer(_, _) | Layout::Closure(_, _, _) => { unreachable!("the type system will guarantee these are never hashed") } diff --git a/compiler/gen/src/llvm/compare.rs b/compiler/gen/src/llvm/compare.rs index bd4381fd0c..62ff98d1c7 100644 --- a/compiler/gen/src/llvm/compare.rs +++ b/compiler/gen/src/llvm/compare.rs @@ -192,10 +192,6 @@ fn build_eq<'a, 'ctx, 'env>( } }, - Layout::Pointer(_) => { - unreachable!("unused") - } - Layout::FunctionPointer(_, _) | Layout::Closure(_, _, _) => { unreachable!("the type system will guarantee these are never compared") } @@ -337,10 +333,6 @@ fn build_neq<'a, 'ctx, 'env>( unreachable!("recursion pointers should never be compared directly") } - Layout::Pointer(_) => { - unreachable!("unused") - } - Layout::FunctionPointer(_, _) | Layout::Closure(_, _, _) => { unreachable!("the type system will guarantee these are never compared") } diff --git a/compiler/gen/src/llvm/convert.rs b/compiler/gen/src/llvm/convert.rs index a88c0ce7f5..5a1589b6b5 100644 --- a/compiler/gen/src/llvm/convert.rs +++ b/compiler/gen/src/llvm/convert.rs @@ -112,9 +112,6 @@ pub fn basic_type_from_layout<'a, 'ctx, 'env>( let closure_data_layout = closure_layout.runtime_representation(); basic_type_from_layout(env, &closure_data_layout) } - Pointer(layout) => basic_type_from_layout(env, &layout) - .ptr_type(AddressSpace::Generic) - .into(), Struct(sorted_fields) => basic_type_from_record(env, sorted_fields), Union(variant) => { use UnionLayout::*; diff --git a/compiler/gen/src/llvm/refcounting.rs b/compiler/gen/src/llvm/refcounting.rs index 494df692b0..05a85b0886 100644 --- a/compiler/gen/src/llvm/refcounting.rs +++ b/compiler/gen/src/llvm/refcounting.rs @@ -744,7 +744,7 @@ fn modify_refcount_layout_build_function<'a, 'ctx, 'env>( } }, - FunctionPointer(_, _) | Pointer(_) => None, + FunctionPointer(_, _) => None, } } diff --git a/compiler/mono/src/alias_analysis.rs b/compiler/mono/src/alias_analysis.rs index 0fb36ae318..4283de8ab3 100644 --- a/compiler/mono/src/alias_analysis.rs +++ b/compiler/mono/src/alias_analysis.rs @@ -533,7 +533,6 @@ fn layout_spec(builder: &mut FuncDefBuilder, layout: &Layout) -> Result RecursivePointer => todo!(), FunctionPointer(_, _) => todo!(), Closure(_, _, _) => todo!(), - Pointer(_) => todo!(), } } diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index 7877894c8a..31094cf0d0 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -42,7 +42,6 @@ pub enum Layout<'a> { /// A function. The types of its arguments, then the type of its return value. FunctionPointer(&'a [Layout<'a>], &'a Layout<'a>), Closure(&'a [Layout<'a>], LambdaSet<'a>, &'a Layout<'a>), - Pointer(&'a Layout<'a>), } impl<'a> Layout<'a> { @@ -502,10 +501,6 @@ impl<'a> Layout<'a> { true } Closure(_, closure_layout, _) => closure_layout.safe_to_memcpy(), - Pointer(_) => { - // We cannot memcpy pointers, because then we would have the same pointer in multiple places! - false - } RecursivePointer => { // We cannot memcpy pointers, because then we would have the same pointer in multiple places! false @@ -558,7 +553,6 @@ impl<'a> Layout<'a> { Closure(_, lambda_set, _) => lambda_set.stack_size(pointer_size), FunctionPointer(_, _) => pointer_size, RecursivePointer => pointer_size, - Pointer(_) => pointer_size, } } @@ -590,7 +584,6 @@ impl<'a> Layout<'a> { Layout::Builtin(builtin) => builtin.alignment_bytes(pointer_size), Layout::RecursivePointer => pointer_size, Layout::FunctionPointer(_, _) => pointer_size, - Layout::Pointer(_) => pointer_size, Layout::Closure(_, captured, _) => { pointer_size.max(captured.alignment_bytes(pointer_size)) } @@ -645,7 +638,7 @@ impl<'a> Layout<'a> { } RecursivePointer => true, Closure(_, closure_layout, _) => closure_layout.contains_refcounted(), - FunctionPointer(_, _) | Pointer(_) => false, + FunctionPointer(_, _) => false, } } @@ -691,7 +684,6 @@ impl<'a> Layout<'a> { .append(" |} -> ") .append(result.to_doc(alloc, Parens::InFunction)) } - Pointer(_) => todo!(), } } } From e2b200b84fcbeef829ca39fbc740d4834e4293a8 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 26 May 2021 14:28:56 +0200 Subject: [PATCH 3/4] simplify list layout --- cli/src/repl/eval.rs | 4 +-- compiler/gen/src/llvm/build.rs | 46 ++++++++++++++-------------- compiler/gen/src/llvm/build_hash.rs | 2 +- compiler/gen/src/llvm/build_list.rs | 18 ++++------- compiler/gen/src/llvm/compare.rs | 4 +-- compiler/gen/src/llvm/convert.rs | 2 +- compiler/gen/src/llvm/refcounting.rs | 28 ++++++++--------- compiler/mono/src/alias_analysis.rs | 2 +- compiler/mono/src/ir.rs | 13 +++----- compiler/mono/src/layout.rs | 36 ++++++---------------- 10 files changed, 62 insertions(+), 93 deletions(-) diff --git a/cli/src/repl/eval.rs b/cli/src/repl/eval.rs index 0c363dfa3b..21304feab9 100644 --- a/cli/src/repl/eval.rs +++ b/cli/src/repl/eval.rs @@ -116,7 +116,7 @@ fn jit_to_ast_help<'a>( } })) } - Layout::Builtin(Builtin::List(_, elem_layout)) => Ok(run_jit_function!( + Layout::Builtin(Builtin::List(elem_layout)) => Ok(run_jit_function!( lib, main_fn_name, (*const u8, usize), @@ -291,7 +291,7 @@ fn ptr_to_ast<'a>( items: &[], final_comments: &[], }, - Layout::Builtin(Builtin::List(_, elem_layout)) => { + Layout::Builtin(Builtin::List(elem_layout)) => { // Turn the (ptr, len) wrapper struct into actual ptr and len values. let len = unsafe { *(ptr.offset(env.ptr_bytes as isize) as *const usize) }; let ptr = unsafe { *(ptr as *const *const u8) }; diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index 03e96b035b..b78962208a 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -2180,7 +2180,7 @@ pub fn build_exp_stmt<'a, 'ctx, 'env>( let (value, layout) = load_symbol_and_layout(scope, symbol); match layout { - Layout::Builtin(Builtin::List(_, _)) => { + Layout::Builtin(Builtin::List(_)) => { debug_assert!(value.is_struct_value()); // because of how we insert DECREF for lists, we can't guarantee that @@ -3585,7 +3585,7 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( match list_layout { Layout::Builtin(Builtin::EmptyList) => default, - Layout::Builtin(Builtin::List(_, element_layout)) => { + Layout::Builtin(Builtin::List(element_layout)) => { let argument_layouts = &[**element_layout, *default_layout]; let roc_function_call = roc_function_call( @@ -3627,8 +3627,8 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( match (list_layout, return_layout) { (Layout::Builtin(Builtin::EmptyList), _) => empty_list(env), ( - Layout::Builtin(Builtin::List(_, element_layout)), - Layout::Builtin(Builtin::List(_, result_layout)), + Layout::Builtin(Builtin::List(element_layout)), + Layout::Builtin(Builtin::List(result_layout)), ) => { let argument_layouts = &[**element_layout]; @@ -3658,9 +3658,9 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( match (list1_layout, list2_layout, return_layout) { ( - Layout::Builtin(Builtin::List(_, element1_layout)), - Layout::Builtin(Builtin::List(_, element2_layout)), - Layout::Builtin(Builtin::List(_, result_layout)), + Layout::Builtin(Builtin::List(element1_layout)), + Layout::Builtin(Builtin::List(element2_layout)), + Layout::Builtin(Builtin::List(result_layout)), ) => { let argument_layouts = &[**element1_layout, **element2_layout]; @@ -3702,10 +3702,10 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( match (list1_layout, list2_layout, list3_layout, return_layout) { ( - Layout::Builtin(Builtin::List(_, element1_layout)), - Layout::Builtin(Builtin::List(_, element2_layout)), - Layout::Builtin(Builtin::List(_, element3_layout)), - Layout::Builtin(Builtin::List(_, result_layout)), + Layout::Builtin(Builtin::List(element1_layout)), + Layout::Builtin(Builtin::List(element2_layout)), + Layout::Builtin(Builtin::List(element3_layout)), + Layout::Builtin(Builtin::List(result_layout)), ) => { let argument_layouts = &[**element1_layout, **element2_layout, **element3_layout]; @@ -3752,8 +3752,8 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( match (list_layout, return_layout) { (Layout::Builtin(Builtin::EmptyList), _) => empty_list(env), ( - Layout::Builtin(Builtin::List(_, element_layout)), - Layout::Builtin(Builtin::List(_, result_layout)), + Layout::Builtin(Builtin::List(element_layout)), + Layout::Builtin(Builtin::List(result_layout)), ) => { let argument_layouts = &[Layout::Builtin(Builtin::Usize), **element_layout]; @@ -3784,7 +3784,7 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( match list_layout { Layout::Builtin(Builtin::EmptyList) => empty_list(env), - Layout::Builtin(Builtin::List(_, element_layout)) => { + Layout::Builtin(Builtin::List(element_layout)) => { let argument_layouts = &[**element_layout]; let roc_function_call = roc_function_call( @@ -3816,8 +3816,8 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( (_, Layout::Builtin(Builtin::EmptyList)) | (Layout::Builtin(Builtin::EmptyList), _) => empty_list(env), ( - Layout::Builtin(Builtin::List(_, before_layout)), - Layout::Builtin(Builtin::List(_, after_layout)), + Layout::Builtin(Builtin::List(before_layout)), + Layout::Builtin(Builtin::List(after_layout)), ) => { let argument_layouts = &[**before_layout]; @@ -3860,8 +3860,8 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( (_, Layout::Builtin(Builtin::EmptyList)) | (Layout::Builtin(Builtin::EmptyList), _) => empty_list(env), ( - Layout::Builtin(Builtin::List(_, before_layout)), - Layout::Builtin(Builtin::List(_, after_layout)), + Layout::Builtin(Builtin::List(before_layout)), + Layout::Builtin(Builtin::List(after_layout)), ) => { let argument_layouts = &[**before_layout]; @@ -3911,7 +3911,7 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( match list_layout { Layout::Builtin(Builtin::EmptyList) => empty_list(env), - Layout::Builtin(Builtin::List(_, element_layout)) => { + Layout::Builtin(Builtin::List(element_layout)) => { use crate::llvm::bitcode::build_compare_wrapper; let argument_layouts = &[**element_layout, **element_layout]; @@ -4179,7 +4179,7 @@ fn run_low_level<'a, 'ctx, 'env>( match list_layout { Layout::Builtin(Builtin::EmptyList) => empty_list(env), - Layout::Builtin(Builtin::List(_, element_layout)) => list_drop( + Layout::Builtin(Builtin::List(element_layout)) => list_drop( env, layout_ids, original_wrapper, @@ -4448,7 +4448,7 @@ fn run_low_level<'a, 'ctx, 'env>( // no elements, so nothing to remove empty_list(env) } - Layout::Builtin(Builtin::List(_, element_layout)) => list_set( + Layout::Builtin(Builtin::List(element_layout)) => list_set( env, layout_ids, list, @@ -4469,7 +4469,7 @@ fn run_low_level<'a, 'ctx, 'env>( // no elements, so nothing to remove empty_list(env) } - Layout::Builtin(Builtin::List(_, element_layout)) => list_set( + Layout::Builtin(Builtin::List(element_layout)) => list_set( env, layout_ids, list, @@ -4646,7 +4646,7 @@ fn run_low_level<'a, 'ctx, 'env>( match list_layout { Layout::Builtin(Builtin::EmptyList) => dict_empty(env), - Layout::Builtin(Builtin::List(_, key_layout)) => { + Layout::Builtin(Builtin::List(key_layout)) => { set_from_list(env, layout_ids, list, key_layout) } _ => unreachable!("invalid dict layout"), diff --git a/compiler/gen/src/llvm/build_hash.rs b/compiler/gen/src/llvm/build_hash.rs index dab6eee5db..195ff1f30e 100644 --- a/compiler/gen/src/llvm/build_hash.rs +++ b/compiler/gen/src/llvm/build_hash.rs @@ -148,7 +148,7 @@ fn hash_builtin<'a, 'ctx, 'env>( Builtin::Set(_) => { todo!("Implement Hash for Set") } - Builtin::List(_, element_layout) => build_hash_list( + Builtin::List(element_layout) => build_hash_list( env, layout_ids, layout, diff --git a/compiler/gen/src/llvm/build_list.rs b/compiler/gen/src/llvm/build_list.rs index f71e4a507a..5a7fea02c9 100644 --- a/compiler/gen/src/llvm/build_list.rs +++ b/compiler/gen/src/llvm/build_list.rs @@ -14,7 +14,7 @@ use inkwell::types::{BasicTypeEnum, PointerType}; use inkwell::values::{BasicValueEnum, FunctionValue, IntValue, PointerValue, StructValue}; use inkwell::{AddressSpace, IntPredicate}; use roc_builtins::bitcode; -use roc_mono::layout::{Builtin, InPlace, Layout, LayoutIds, MemoryMode}; +use roc_mono::layout::{Builtin, InPlace, Layout, LayoutIds}; fn list_returned_from_zig<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, @@ -196,12 +196,12 @@ pub fn list_join<'a, 'ctx, 'env>( ) -> BasicValueEnum<'ctx> { match outer_list_layout { Layout::Builtin(Builtin::EmptyList) - | Layout::Builtin(Builtin::List(_, Layout::Builtin(Builtin::EmptyList))) => { + | Layout::Builtin(Builtin::List(Layout::Builtin(Builtin::EmptyList))) => { // If the input list is empty, or if it is a list of empty lists // then simply return an empty list empty_list(env) } - Layout::Builtin(Builtin::List(_, Layout::Builtin(Builtin::List(_, element_layout)))) => { + Layout::Builtin(Builtin::List(Layout::Builtin(Builtin::List(element_layout)))) => { call_bitcode_fn_returns_list( env, &[ @@ -231,13 +231,7 @@ pub fn list_reverse<'a, 'ctx, 'env>( // this pointer will never actually be dereferenced Layout::Builtin(Builtin::Int64), ), - Layout::Builtin(Builtin::List(memory_mode, elem_layout)) => ( - match memory_mode { - MemoryMode::Unique => InPlace::InPlace, - MemoryMode::Refcounted => InPlace::Clone, - }, - *elem_layout, - ), + Layout::Builtin(Builtin::List(elem_layout)) => (InPlace::Clone, *elem_layout), _ => unreachable!("Invalid layout {:?} in List.reverse", list_layout), }; @@ -264,7 +258,7 @@ pub fn list_get_unsafe<'a, 'ctx, 'env>( let builder = env.builder; match list_layout { - Layout::Builtin(Builtin::List(_, elem_layout)) => { + Layout::Builtin(Builtin::List(elem_layout)) => { let elem_type = basic_type_from_layout(env, elem_layout); let ptr_type = get_ptr_type(&elem_type, AddressSpace::Generic); // Load the pointer to the array data @@ -861,7 +855,7 @@ pub fn list_concat<'a, 'ctx, 'env>( // then simply return an empty list empty_list(env) } - Layout::Builtin(Builtin::List(_, elem_layout)) => call_bitcode_fn_returns_list( + Layout::Builtin(Builtin::List(elem_layout)) => call_bitcode_fn_returns_list( env, &[ pass_list_as_i128(env, first_list), diff --git a/compiler/gen/src/llvm/compare.rs b/compiler/gen/src/llvm/compare.rs index 62ff98d1c7..7f9cf127dd 100644 --- a/compiler/gen/src/llvm/compare.rs +++ b/compiler/gen/src/llvm/compare.rs @@ -99,7 +99,7 @@ fn build_eq_builtin<'a, 'ctx, 'env>( Builtin::Float16 => float_cmp(FloatPredicate::OEQ, "eq_f16"), Builtin::Str => str_equal(env, lhs_val, rhs_val), - Builtin::List(_, elem) => build_list_eq( + Builtin::List(elem) => build_list_eq( env, layout_ids, &Layout::Builtin(*builtin), @@ -249,7 +249,7 @@ fn build_neq_builtin<'a, 'ctx, 'env>( result.into() } - Builtin::List(_, elem) => { + Builtin::List(elem) => { let is_equal = build_list_eq( env, layout_ids, diff --git a/compiler/gen/src/llvm/convert.rs b/compiler/gen/src/llvm/convert.rs index 5a1589b6b5..c88c12c18b 100644 --- a/compiler/gen/src/llvm/convert.rs +++ b/compiler/gen/src/llvm/convert.rs @@ -170,7 +170,7 @@ pub fn basic_type_from_builtin<'a, 'ctx, 'env>( Float16 => context.f16_type().as_basic_type_enum(), Dict(_, _) | EmptyDict => zig_dict_type(env).into(), Set(_) | EmptySet => zig_dict_type(env).into(), - List(_, _) | EmptyList => zig_list_type(env).into(), + List(_) | EmptyList => zig_list_type(env).into(), Str | EmptyStr => zig_str_type(env).into(), } } diff --git a/compiler/gen/src/llvm/refcounting.rs b/compiler/gen/src/llvm/refcounting.rs index 05a85b0886..71c309f8b4 100644 --- a/compiler/gen/src/llvm/refcounting.rs +++ b/compiler/gen/src/llvm/refcounting.rs @@ -15,7 +15,7 @@ use inkwell::values::{BasicValueEnum, FunctionValue, IntValue, PointerValue, Str use inkwell::{AddressSpace, IntPredicate}; use roc_module::symbol::Interns; use roc_module::symbol::Symbol; -use roc_mono::layout::{Builtin, Layout, LayoutIds, MemoryMode, UnionLayout}; +use roc_mono::layout::{Builtin, Layout, LayoutIds, UnionLayout}; pub const REFCOUNT_MAX: usize = 0_usize; @@ -469,21 +469,17 @@ fn modify_refcount_builtin<'a, 'ctx, 'env>( use Builtin::*; match builtin { - List(memory_mode, element_layout) => { - if let MemoryMode::Refcounted = memory_mode { - let function = modify_refcount_list( - env, - layout_ids, - mode, - when_recursive, - layout, - element_layout, - ); + List(element_layout) => { + let function = modify_refcount_list( + env, + layout_ids, + mode, + when_recursive, + layout, + element_layout, + ); - Some(function) - } else { - None - } + Some(function) } Set(element_layout) => { let key_layout = &Layout::Struct(&[]); @@ -1687,7 +1683,7 @@ pub fn refcount_offset<'a, 'ctx, 'env>(env: &Env<'a, 'ctx, 'env>, layout: &Layou let value_bytes = layout.stack_size(env.ptr_bytes) as u64; match layout { - Layout::Builtin(Builtin::List(_, _)) => env.ptr_bytes as u64, + Layout::Builtin(Builtin::List(_)) => env.ptr_bytes as u64, Layout::Builtin(Builtin::Str) => env.ptr_bytes as u64, Layout::RecursivePointer | Layout::Union(_) => env.ptr_bytes as u64, _ => (env.ptr_bytes as u64).max(value_bytes), diff --git a/compiler/mono/src/alias_analysis.rs b/compiler/mono/src/alias_analysis.rs index 4283de8ab3..958030f9f9 100644 --- a/compiler/mono/src/alias_analysis.rs +++ b/compiler/mono/src/alias_analysis.rs @@ -548,7 +548,7 @@ fn builtin_spec(builder: &mut FuncDefBuilder, builtin: &Builtin) -> Result todo!(), Dict(_, _) => todo!(), Set(_) => todo!(), - List(_, _) => { + List(_) => { // TODO should incorporate the element type into the name Ok(builder.add_named_type(MOD_LIST, TypeName(b"List"))) } diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index cbf8649aaf..7b9ab8757f 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -3,8 +3,8 @@ use self::InProgressProc::*; use crate::exhaustive::{Ctor, Guard, RenderAs, TagId}; use crate::layout::{ - Builtin, ClosureRepresentation, LambdaSet, Layout, LayoutCache, LayoutProblem, MemoryMode, - UnionLayout, WrappedVariant, TAG_SIZE, + Builtin, ClosureRepresentation, LambdaSet, Layout, LayoutCache, LayoutProblem, UnionLayout, + WrappedVariant, TAG_SIZE, }; use bumpalo::collections::Vec; use bumpalo::Bump; @@ -3364,10 +3364,7 @@ pub fn with_hole<'a>( Stmt::Let( assigned, expr, - Layout::Builtin(Builtin::List( - MemoryMode::Refcounted, - env.arena.alloc(elem_layout), - )), + Layout::Builtin(Builtin::List(env.arena.alloc(elem_layout))), hole, ) } @@ -3399,12 +3396,10 @@ pub fn with_hole<'a>( elems: arg_symbols, }; - let mode = MemoryMode::Refcounted; - let stmt = Stmt::Let( assigned, expr, - Layout::Builtin(Builtin::List(mode, env.arena.alloc(elem_layout))), + Layout::Builtin(Builtin::List(env.arena.alloc(elem_layout))), hole, ); diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index 31094cf0d0..ba6034ecab 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -48,10 +48,7 @@ impl<'a> Layout<'a> { pub fn in_place(&self) -> InPlace { match self { Layout::Builtin(Builtin::EmptyList) => InPlace::InPlace, - Layout::Builtin(Builtin::List(memory_mode, _)) => match memory_mode { - MemoryMode::Unique => InPlace::InPlace, - MemoryMode::Refcounted => InPlace::Clone, - }, + Layout::Builtin(Builtin::List(_)) => InPlace::Clone, Layout::Builtin(Builtin::EmptyStr) => InPlace::InPlace, Layout::Builtin(Builtin::Str) => InPlace::Clone, Layout::Builtin(Builtin::Int1) => InPlace::Clone, @@ -331,12 +328,6 @@ impl<'a> LambdaSet<'a> { } } -#[derive(Clone, Debug, PartialEq, Eq, Hash, Copy)] -pub enum MemoryMode { - Unique, - Refcounted, -} - #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub enum Builtin<'a> { Int128, @@ -353,7 +344,7 @@ pub enum Builtin<'a> { Str, Dict(&'a Layout<'a>, &'a Layout<'a>), Set(&'a Layout<'a>), - List(MemoryMode, &'a Layout<'a>), + List(&'a Layout<'a>), EmptyStr, EmptyList, EmptyDict, @@ -606,7 +597,7 @@ impl<'a> Layout<'a> { RecursivePointer => true, - Builtin(List(MemoryMode::Refcounted, _)) | Builtin(Str) => true, + Builtin(List(_)) | Builtin(Str) => true, _ => false, } @@ -855,7 +846,7 @@ impl<'a> Builtin<'a> { Str | EmptyStr => Builtin::STR_WORDS * pointer_size, Dict(_, _) | EmptyDict => Builtin::DICT_WORDS * pointer_size, Set(_) | EmptySet => Builtin::SET_WORDS * pointer_size, - List(_, _) | EmptyList => Builtin::LIST_WORDS * pointer_size, + List(_) | EmptyList => Builtin::LIST_WORDS * pointer_size, } } @@ -881,7 +872,7 @@ impl<'a> Builtin<'a> { Str | EmptyStr => pointer_size, Dict(_, _) | EmptyDict => pointer_size, Set(_) | EmptySet => pointer_size, - List(_, _) | EmptyList => pointer_size, + List(_) | EmptyList => pointer_size, } } @@ -891,7 +882,7 @@ impl<'a> Builtin<'a> { match self { Int128 | Int64 | Int32 | Int16 | Int8 | Int1 | Usize | Float128 | Float64 | Float32 | Float16 | EmptyStr | EmptyDict | EmptyList | EmptySet => true, - Str | Dict(_, _) | Set(_) | List(_, _) => false, + Str | Dict(_, _) | Set(_) | List(_) => false, } } @@ -902,10 +893,7 @@ impl<'a> Builtin<'a> { match self { Int128 | Int64 | Int32 | Int16 | Int8 | Int1 | Usize | Float128 | Float64 | Float32 | Float16 | EmptyStr | EmptyDict | EmptyList | EmptySet => false, - List(mode, element_layout) => match mode { - MemoryMode::Refcounted => true, - MemoryMode::Unique => element_layout.contains_refcounted(), - }, + List(_) => true, Str | Dict(_, _) | Set(_) => true, } @@ -938,7 +926,7 @@ impl<'a> Builtin<'a> { EmptySet => alloc.text("EmptySet"), Str => alloc.text("Str"), - List(_, layout) => alloc + List(layout) => alloc .text("List ") .append(layout.to_doc(alloc, Parens::InTypeParam)), Set(layout) => alloc @@ -1878,11 +1866,7 @@ pub fn list_layout_from_elem<'a>( _ => { let elem_layout = Layout::from_var(env, elem_var)?; - // This is a normal list. - Ok(Layout::Builtin(Builtin::List( - MemoryMode::Refcounted, - env.arena.alloc(elem_layout), - ))) + Ok(Layout::Builtin(Builtin::List(env.arena.alloc(elem_layout)))) } } } @@ -1948,7 +1932,7 @@ impl<'a> std::convert::TryFrom<&Layout<'a>> for ListLayout<'a> { fn try_from(value: &Layout<'a>) -> Result { match value { Layout::Builtin(Builtin::EmptyList) => Ok(ListLayout::EmptyList), - Layout::Builtin(Builtin::List(_, element)) => Ok(ListLayout::List(element)), + Layout::Builtin(Builtin::List(element)) => Ok(ListLayout::List(element)), _ => Err(()), } } From 06b2beb7caa7dfb2b622502d1a2645573a764f94 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 26 May 2021 18:31:55 +0200 Subject: [PATCH 4/4] tags have a union layout --- compiler/gen/src/llvm/build.rs | 20 ++++++------- compiler/mono/src/alias_analysis.rs | 14 ++------- compiler/mono/src/ir.rs | 44 ++++++++++++++++------------- 3 files changed, 34 insertions(+), 44 deletions(-) diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index b78962208a..c5e9cdd7c8 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -906,9 +906,7 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( arguments, tag_layout, .. - } if *union_size == 1 - && matches!(tag_layout, Layout::Union(UnionLayout::NonRecursive(_))) => - { + } if *union_size == 1 && matches!(tag_layout, UnionLayout::NonRecursive(_)) => { let it = arguments.iter(); let ctx = env.context; @@ -956,7 +954,7 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( Tag { arguments, - tag_layout: Layout::Union(UnionLayout::NonRecursive(fields)), + tag_layout: UnionLayout::NonRecursive(fields), union_size, tag_id, .. @@ -1044,7 +1042,7 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( } Tag { arguments, - tag_layout: Layout::Union(UnionLayout::Recursive(fields)), + tag_layout: UnionLayout::Recursive(fields), union_size, tag_id, .. @@ -1119,7 +1117,7 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( Tag { arguments, - tag_layout: Layout::Union(UnionLayout::NonNullableUnwrapped(fields)), + tag_layout: UnionLayout::NonNullableUnwrapped(fields), union_size, tag_id, .. @@ -1195,10 +1193,10 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( Tag { arguments, tag_layout: - Layout::Union(UnionLayout::NullableWrapped { + UnionLayout::NullableWrapped { nullable_id, other_tags: fields, - }), + }, union_size, tag_id, .. @@ -1287,11 +1285,11 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( Tag { arguments, tag_layout: - Layout::Union(UnionLayout::NullableUnwrapped { + UnionLayout::NullableUnwrapped { nullable_id, other_fields, .. - }), + }, union_size, tag_id, tag_name, @@ -1387,8 +1385,6 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( data_ptr.into() } - Tag { .. } => unreachable!("tags should have a Union or RecursiveUnion layout"), - Reset(_) => todo!(), Reuse { .. } => todo!(), diff --git a/compiler/mono/src/alias_analysis.rs b/compiler/mono/src/alias_analysis.rs index 958030f9f9..dd3590e57c 100644 --- a/compiler/mono/src/alias_analysis.rs +++ b/compiler/mono/src/alias_analysis.rs @@ -377,16 +377,6 @@ fn lowlevel_spec( } fn build_variant_types( - builder: &mut FuncDefBuilder, - layout: &Layout, -) -> Option>> { - match layout { - Layout::Union(union_layout) => Some(build_variant_types_help(builder, union_layout)), - _ => None, - } -} - -fn build_variant_types_help( builder: &mut FuncDefBuilder, union_layout: &UnionLayout, ) -> Result> { @@ -435,7 +425,7 @@ fn expr_spec( arguments, } => { let value_id = build_tuple_value(builder, env, block, arguments)?; - let variant_types = build_variant_types(builder, tag_layout).unwrap()?; + let variant_types = build_variant_types(builder, tag_layout)?; builder.add_make_union(block, &variant_types, *tag_id as u32, value_id) } Struct(fields) => build_tuple_value(builder, env, block, fields), @@ -527,7 +517,7 @@ fn layout_spec(builder: &mut FuncDefBuilder, layout: &Layout) -> Result Builtin(builtin) => builtin_spec(builder, builtin), Struct(fields) => build_tuple_type(builder, fields), Union(union_layout) => { - let variant_types = build_variant_types_help(builder, union_layout)?; + let variant_types = build_variant_types(builder, union_layout)?; builder.add_union_type(&variant_types) } RecursivePointer => todo!(), diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index 7b9ab8757f..b02f741623 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -1143,7 +1143,7 @@ pub enum Expr<'a> { Call(Call<'a>), Tag { - tag_layout: Layout<'a>, + tag_layout: UnionLayout<'a>, tag_name: TagName, tag_id: u8, union_size: u8, @@ -4128,10 +4128,15 @@ fn construct_closure_data<'a>( tag_symbols.push(tag_id_symbol); tag_symbols.extend(symbols); + let tag_layout = match lambda_set.runtime_representation() { + Layout::Union(inner) => inner, + _ => unreachable!(), + }; + let expr1 = Expr::Literal(Literal::Int(tag_id as i128)); let expr2 = Expr::Tag { tag_id, - tag_layout: lambda_set.runtime_representation(), + tag_layout, union_size, tag_name, arguments: tag_symbols.into_bump_slice(), @@ -4293,17 +4298,17 @@ fn convert_tag_union<'a>( } debug_assert!(layouts.len() > 1); - let layout = Layout::Union(UnionLayout::Recursive(layouts.into_bump_slice())); + let union_layout = UnionLayout::Recursive(layouts.into_bump_slice()); let tag = Expr::Tag { - tag_layout: layout, + tag_layout: union_layout, tag_name, tag_id: tag_id as u8, union_size, arguments: field_symbols, }; - (tag, layout) + (tag, Layout::Union(union_layout)) } NonNullableUnwrapped { fields, @@ -4321,17 +4326,17 @@ fn convert_tag_union<'a>( temp.into_bump_slice() }; - let layout = Layout::Union(UnionLayout::NonNullableUnwrapped(fields)); + let union_layout = UnionLayout::NonNullableUnwrapped(fields); let tag = Expr::Tag { - tag_layout: layout, + tag_layout: union_layout, tag_name, tag_id: tag_id as u8, union_size, arguments: field_symbols, }; - (tag, layout) + (tag, Layout::Union(union_layout)) } NonRecursive { sorted_tag_layouts } => { let tag_id_symbol = env.unique_symbol(); @@ -4353,18 +4358,17 @@ fn convert_tag_union<'a>( layouts.push(arg_layouts); } - let layout = - Layout::Union(UnionLayout::NonRecursive(layouts.into_bump_slice())); + let union_layout = UnionLayout::NonRecursive(layouts.into_bump_slice()); let tag = Expr::Tag { - tag_layout: layout, + tag_layout: union_layout, tag_name, tag_id: tag_id as u8, union_size, arguments: field_symbols, }; - (tag, layout) + (tag, Layout::Union(union_layout)) } NullableWrapped { nullable_id, @@ -4390,20 +4394,20 @@ fn convert_tag_union<'a>( layouts.push(arg_layouts); } - let layout = Layout::Union(UnionLayout::NullableWrapped { + let union_layout = UnionLayout::NullableWrapped { nullable_id, other_tags: layouts.into_bump_slice(), - }); + }; let tag = Expr::Tag { - tag_layout: layout, + tag_layout: union_layout, tag_name, tag_id: tag_id as u8, union_size, arguments: field_symbols, }; - (tag, layout) + (tag, Layout::Union(union_layout)) } NullableUnwrapped { nullable_id, @@ -4425,20 +4429,20 @@ fn convert_tag_union<'a>( temp.into_bump_slice() }; - let layout = Layout::Union(UnionLayout::NullableUnwrapped { + let union_layout = UnionLayout::NullableUnwrapped { nullable_id, other_fields, - }); + }; let tag = Expr::Tag { - tag_layout: layout, + tag_layout: union_layout, tag_name, tag_id: tag_id as u8, union_size, arguments: field_symbols, }; - (tag, layout) + (tag, Layout::Union(union_layout)) } };