From ccd2e0ecf445eb9649e48c75ce22be5fb5a1fb6c Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 24 Nov 2020 22:01:57 +0100 Subject: [PATCH] alignment in single element tag unions --- compiler/gen/src/llvm/build.rs | 21 ++++++++++++++++++--- compiler/gen/tests/gen_records.rs | 1 + compiler/gen/tests/gen_tags.rs | 11 +++++++++++ compiler/mono/src/ir.rs | 31 +++++++++++++++++++++++++++---- compiler/mono/src/layout.rs | 9 +++++++++ 5 files changed, 66 insertions(+), 7 deletions(-) diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index dbd06bd64c..7d90e65de2 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -735,7 +735,12 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( // Insert field exprs into struct_val for (index, field_val) in field_vals.into_iter().enumerate() { struct_val = builder - .build_insert_value(struct_val, field_val, index as u32, "insert_field") + .build_insert_value( + struct_val, + field_val, + index as u32, + "insert_record_field", + ) .unwrap(); } @@ -785,7 +790,12 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( // Insert field exprs into struct_val for (index, field_val) in field_vals.into_iter().enumerate() { struct_val = builder - .build_insert_value(struct_val, field_val, index as u32, "insert_field") + .build_insert_value( + struct_val, + field_val, + index as u32, + "insert_single_tag_field", + ) .unwrap(); } @@ -848,7 +858,12 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( // Insert field exprs into struct_val for (index, field_val) in field_vals.into_iter().enumerate() { struct_val = builder - .build_insert_value(struct_val, field_val, index as u32, "insert_field") + .build_insert_value( + struct_val, + field_val, + index as u32, + "insert_multi_tag_field", + ) .unwrap(); } diff --git a/compiler/gen/tests/gen_records.rs b/compiler/gen/tests/gen_records.rs index 00b561fe5e..ba202c9bc7 100644 --- a/compiler/gen/tests/gen_records.rs +++ b/compiler/gen/tests/gen_records.rs @@ -853,6 +853,7 @@ mod gen_records { (i64, bool, u8) ); } + #[test] fn blue_and_present() { assert_evals_to!( diff --git a/compiler/gen/tests/gen_tags.rs b/compiler/gen/tests/gen_tags.rs index 7e317015b9..6a89b496c6 100644 --- a/compiler/gen/tests/gen_tags.rs +++ b/compiler/gen/tests/gen_tags.rs @@ -758,4 +758,15 @@ mod gen_tags { i64 ); } + + #[test] + fn alignment_in_single_tag() { + assert_evals_to!(indoc!("Three (1 == 1) 32"), (32i64, true), (i64, bool)); + + assert_evals_to!( + indoc!("Three (1 == 1) (if True then Red else if True then Green else Blue) 32"), + (32i64, true, 2u8), + (i64, bool, u8) + ); + } } diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index 86d15daac4..5576eb6c56 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -2384,7 +2384,7 @@ pub fn with_hole<'a>( Tag { variant_var, name: tag_name, - arguments: args, + arguments: mut args, .. } => { use crate::layout::UnionVariant::*; @@ -2421,11 +2421,34 @@ pub fn with_hole<'a>( } Unwrapped(field_layouts) => { + let mut field_symbols_temp = + Vec::with_capacity_in(field_layouts.len(), env.arena); + + for (var, arg) in args.drain(..) { + // Layout will unpack this unwrapped tack if it only has one (non-zero-sized) field + let layout = layout_cache + .from_var(env.arena, var, env.subs) + .unwrap_or_else(|err| { + panic!("TODO turn fn_var into a RuntimeError {:?}", err) + }); + + let alignment = layout.alignment_bytes(8); + + let symbol = possible_reuse_symbol(env, procs, &arg.value); + field_symbols_temp.push(( + alignment, + symbol, + ((var, arg), &*env.arena.alloc(symbol)), + )); + } + field_symbols_temp.sort_by(|a, b| b.0.cmp(&a.0)); + let mut field_symbols = Vec::with_capacity_in(field_layouts.len(), env.arena); - for (_, arg) in args.iter() { - field_symbols.push(possible_reuse_symbol(env, procs, &arg.value)); + for (_, symbol, _) in field_symbols_temp.iter() { + field_symbols.push(*symbol); } + let field_symbols = field_symbols.into_bump_slice(); // Layout will unpack this unwrapped tack if it only has one (non-zero-sized) field @@ -2438,7 +2461,7 @@ pub fn with_hole<'a>( // even though this was originally a Tag, we treat it as a Struct from now on let stmt = Stmt::Let(assigned, Expr::Struct(field_symbols), layout, hole); - let iter = args.into_iter().rev().zip(field_symbols.iter().rev()); + let iter = field_symbols_temp.into_iter().map(|(_, _, data)| data); assign_to_symbols(env, procs, layout_cache, iter, stmt) } Wrapped(sorted_tag_layouts) => { diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index 0da6001200..be9409796b 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -1047,6 +1047,15 @@ pub fn union_sorted_tags_help<'a>( } } + layouts.sort_by(|layout1, layout2| { + let ptr_bytes = 8; + + let size1 = layout1.alignment_bytes(ptr_bytes); + let size2 = layout2.alignment_bytes(ptr_bytes); + + size2.cmp(&size1) + }); + if layouts.is_empty() { if contains_zero_sized { UnionVariant::UnitWithArguments