From 2355bbf8153d227e53e39f4ee0bfa8b561ba0784 Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 24 Jun 2021 20:25:06 +0200 Subject: [PATCH 1/4] updates --- compiler/gen_llvm/src/llvm/bitcode.rs | 13 +- compiler/gen_llvm/src/llvm/build.rs | 208 +++++++++++--------------- 2 files changed, 94 insertions(+), 127 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/bitcode.rs b/compiler/gen_llvm/src/llvm/bitcode.rs index 57b0d923cd..214bfaab72 100644 --- a/compiler/gen_llvm/src/llvm/bitcode.rs +++ b/compiler/gen_llvm/src/llvm/bitcode.rs @@ -1,8 +1,6 @@ /// Helpers for interacting with the zig that generates bitcode use crate::debug_info_init; -use crate::llvm::build::{ - struct_from_fields, Env, C_CALL_CONV, FAST_CALL_CONV, TAG_DATA_INDEX, TAG_ID_INDEX, -}; +use crate::llvm::build::{struct_from_fields, Env, C_CALL_CONV, FAST_CALL_CONV}; use crate::llvm::convert::basic_type_from_layout; use crate::llvm::refcounting::{ decrement_refcount_layout, increment_n_refcount_layout, increment_refcount_layout, @@ -138,9 +136,12 @@ fn build_has_tag_id_help<'a, 'ctx, 'env>( let input = env.builder.build_load(argument_cast, "get_value"); - let tag_value = - crate::llvm::build::get_tag_id(env, function_value, &union_layout, input) - .into_int_value(); + let tag_value = crate::llvm::build::extract_tag_discriminant( + env, + function_value, + union_layout, + input, + ); let actual_tag_id = env.builder diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 3ec7749504..c8d91374c5 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -1569,77 +1569,8 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( structure, union_layout, } => { - let builder = env.builder; - - // cast the argument bytes into the desired shape for this tag - let (argument, _structure_layout) = load_symbol_and_layout(scope, structure); - - match union_layout { - UnionLayout::NonRecursive(_) => { - let pointer = builder.build_alloca(argument.get_type(), "get_type"); - builder.build_store(pointer, argument); - let tag_id_pointer = builder.build_bitcast( - pointer, - env.context.i64_type().ptr_type(AddressSpace::Generic), - "tag_id_pointer", - ); - builder.build_load(tag_id_pointer.into_pointer_value(), "load_tag_id") - } - UnionLayout::Recursive(_) => { - let pointer = argument.into_pointer_value(); - let tag_id_pointer = builder.build_bitcast( - pointer, - env.context.i64_type().ptr_type(AddressSpace::Generic), - "tag_id_pointer", - ); - builder.build_load(tag_id_pointer.into_pointer_value(), "load_tag_id") - } - UnionLayout::NonNullableUnwrapped(_) => env.context.i64_type().const_zero().into(), - UnionLayout::NullableWrapped { nullable_id, .. } => { - let argument_ptr = argument.into_pointer_value(); - let is_null = env.builder.build_is_null(argument_ptr, "is_null"); - - let ctx = env.context; - let then_block = ctx.append_basic_block(parent, "then"); - let else_block = ctx.append_basic_block(parent, "else"); - let cont_block = ctx.append_basic_block(parent, "cont"); - - let result = builder.build_alloca(ctx.i64_type(), "result"); - - env.builder - .build_conditional_branch(is_null, then_block, else_block); - - { - env.builder.position_at_end(then_block); - let tag_id = ctx.i64_type().const_int(*nullable_id as u64, false); - env.builder.build_store(result, tag_id); - env.builder.build_unconditional_branch(cont_block); - } - - { - env.builder.position_at_end(else_block); - let tag_id = extract_tag_discriminant_ptr(env, argument_ptr); - env.builder.build_store(result, tag_id); - env.builder.build_unconditional_branch(cont_block); - } - - env.builder.position_at_end(cont_block); - - env.builder.build_load(result, "load_result") - } - UnionLayout::NullableUnwrapped { nullable_id, .. } => { - let argument_ptr = argument.into_pointer_value(); - let is_null = env.builder.build_is_null(argument_ptr, "is_null"); - - let ctx = env.context; - - let then_value = ctx.i64_type().const_int(*nullable_id as u64, false); - let else_value = ctx.i64_type().const_int(!*nullable_id as u64, false); - - env.builder - .build_select(is_null, then_value, else_value, "select_tag_id") - } - } + let value = load_symbol(scope, structure); + extract_tag_discriminant(env, parent, *union_layout, value).into() } } } @@ -2436,6 +2367,89 @@ pub fn complex_bitcast<'ctx>( } } +pub fn extract_tag_discriminant<'a, 'ctx, 'env>( + env: &Env<'a, 'ctx, 'env>, + parent: FunctionValue<'ctx>, + union_layout: UnionLayout<'a>, + cond_value: BasicValueEnum<'ctx>, +) -> IntValue<'ctx> { + let builder = env.builder; + + match union_layout { + UnionLayout::NonRecursive(_) => { + let pointer = builder.build_alloca(cond_value.get_type(), "get_type"); + builder.build_store(pointer, cond_value); + let tag_id_pointer = builder.build_bitcast( + pointer, + env.context.i64_type().ptr_type(AddressSpace::Generic), + "tag_id_pointer", + ); + builder + .build_load(tag_id_pointer.into_pointer_value(), "load_tag_id") + .into_int_value() + } + UnionLayout::Recursive(_) => { + let pointer = cond_value.into_pointer_value(); + let tag_id_pointer = builder.build_bitcast( + pointer, + env.context.i64_type().ptr_type(AddressSpace::Generic), + "tag_id_pointer", + ); + builder + .build_load(tag_id_pointer.into_pointer_value(), "load_tag_id") + .into_int_value() + } + UnionLayout::NonNullableUnwrapped(_) => env.context.i64_type().const_zero(), + UnionLayout::NullableWrapped { nullable_id, .. } => { + let argument_ptr = cond_value.into_pointer_value(); + let is_null = env.builder.build_is_null(argument_ptr, "is_null"); + + let ctx = env.context; + let then_block = ctx.append_basic_block(parent, "then"); + let else_block = ctx.append_basic_block(parent, "else"); + let cont_block = ctx.append_basic_block(parent, "cont"); + + let result = builder.build_alloca(ctx.i64_type(), "result"); + + env.builder + .build_conditional_branch(is_null, then_block, else_block); + + { + env.builder.position_at_end(then_block); + let tag_id = ctx.i64_type().const_int(nullable_id as u64, false); + env.builder.build_store(result, tag_id); + env.builder.build_unconditional_branch(cont_block); + } + + { + env.builder.position_at_end(else_block); + let tag_id = extract_tag_discriminant_ptr(env, argument_ptr); + env.builder.build_store(result, tag_id); + env.builder.build_unconditional_branch(cont_block); + } + + env.builder.position_at_end(cont_block); + + env.builder + .build_load(result, "load_result") + .into_int_value() + } + UnionLayout::NullableUnwrapped { nullable_id, .. } => { + let argument_ptr = cond_value.into_pointer_value(); + let is_null = env.builder.build_is_null(argument_ptr, "is_null"); + + let ctx = env.context; + + let then_value = ctx.i64_type().const_int(nullable_id as u64, false); + let else_value = ctx.i64_type().const_int(!nullable_id as u64, false); + + env.builder + .build_select(is_null, then_value, else_value, "select_tag_id") + .into_int_value() + } + } +} + fn extract_tag_discriminant_struct<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, from_value: StructValue<'ctx>, @@ -2548,57 +2562,9 @@ fn build_switch_ir<'a, 'ctx, 'env>( .into_int_value() } Layout::Union(variant) => { - use UnionLayout::*; + cond_layout = Layout::Builtin(Builtin::Int64); - match variant { - NonRecursive(_) => { - // we match on the discriminant, not the whole Tag - cond_layout = Layout::Builtin(Builtin::Int64); - let full_cond = cond_value.into_struct_value(); - - extract_tag_discriminant_struct(env, full_cond) - } - Recursive(_) => { - // we match on the discriminant, not the whole Tag - cond_layout = Layout::Builtin(Builtin::Int64); - - debug_assert!(cond_value.is_pointer_value()); - extract_tag_discriminant_ptr(env, cond_value.into_pointer_value()) - } - NonNullableUnwrapped(_) => unreachable!("there is no tag to switch on"), - NullableWrapped { nullable_id, .. } => { - // we match on the discriminant, not the whole Tag - cond_layout = Layout::Builtin(Builtin::Int64); - let full_cond_ptr = cond_value.into_pointer_value(); - - let comparison: IntValue = - env.builder.build_is_null(full_cond_ptr, "is_null_cond"); - - let when_null = || { - env.context - .i64_type() - .const_int(nullable_id as u64, false) - .into() - }; - let when_not_null = || extract_tag_discriminant_ptr(env, full_cond_ptr).into(); - - crate::llvm::build_list::build_basic_phi2( - env, - parent, - comparison, - when_null, - when_not_null, - BasicTypeEnum::IntType(env.context.i64_type()), - ) - .into_int_value() - } - NullableUnwrapped { .. } => { - // there are only two options, so we do a `tag_id == 0` check and branch on that - unreachable!( - "we never switch on the tag id directly for NullableUnwrapped unions" - ) - } - } + extract_tag_discriminant(env, parent, variant, cond_value) } Layout::Builtin(_) => cond_value.into_int_value(), other => todo!("Build switch value from layout: {:?}", other), From a05d8b52c0d2a819fc74a23affdbe40657359975 Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 24 Jun 2021 20:31:14 +0200 Subject: [PATCH 2/4] make things compile --- compiler/gen_llvm/src/llvm/bitcode.rs | 32 +++++++--------------- compiler/gen_llvm/src/llvm/build.rs | 38 ++++++++++++++++++--------- compiler/gen_llvm/src/llvm/convert.rs | 6 +++++ 3 files changed, 42 insertions(+), 34 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/bitcode.rs b/compiler/gen_llvm/src/llvm/bitcode.rs index 214bfaab72..6dfb4ab929 100644 --- a/compiler/gen_llvm/src/llvm/bitcode.rs +++ b/compiler/gen_llvm/src/llvm/bitcode.rs @@ -134,30 +134,18 @@ fn build_has_tag_id_help<'a, 'ctx, 'env>( ) .into_pointer_value(); - let input = env.builder.build_load(argument_cast, "get_value"); + let tag_value = env.builder.build_load(argument_cast, "get_value"); - let tag_value = crate::llvm::build::extract_tag_discriminant( - env, - function_value, - union_layout, - input, - ); + let actual_tag_id = { + let tag_id_i64 = crate::llvm::build::extract_tag_discriminant( + env, + function_value, + union_layout, + tag_value, + ); - let actual_tag_id = env.builder - .build_int_cast(tag_value, env.context.i16_type(), "to_i16"); - - let data_ptr = { - let index = env - .context - .i64_type() - .const_int(TAG_DATA_INDEX as u64, false); - let ptr = unsafe { - env.builder - .build_in_bounds_gep(argument_cast, &[index], "get_tag_id") - }; - - env.builder.build_bitcast(ptr, i8_ptr_type, "to_opaque") + .build_int_cast(tag_id_i64, env.context.i16_type(), "to_i16") }; let answer = env.builder.build_int_compare( @@ -167,7 +155,7 @@ fn build_has_tag_id_help<'a, 'ctx, 'env>( "compare", ); - let field_vals = [(0, answer.into()), (1, data_ptr)]; + let field_vals = [(0, answer.into()), (1, *tag_value_ptr)]; let output = struct_from_fields(env, output_type, field_vals.iter().copied()); diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index c8d91374c5..46cdb61c02 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -996,19 +996,10 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( // Create the struct_type let struct_type = ctx.struct_type(field_types.into_bump_slice(), false); - let mut struct_val = struct_type.const_zero().into(); // 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_multi_tag_field", - ) - .unwrap(); - } + let struct_val = + struct_from_fields(env, struct_type, field_vals.into_iter().enumerate()); // How we create tag values // @@ -1036,7 +1027,7 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( let internal_type = basic_type_from_layout(env, &tag_layout); - cast_tag_to_block_of_memory(builder, struct_val.into_struct_value(), internal_type) + cast_tag_to_block_of_memory(builder, struct_val, internal_type) } Tag { arguments, @@ -1575,6 +1566,29 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( } } +pub fn struct_from_fields<'a, 'ctx, 'env, I>( + env: &Env<'a, 'ctx, 'env>, + struct_type: StructType<'ctx>, + values: I, +) -> StructValue<'ctx> +where + I: Iterator)>, +{ + let mut struct_value = struct_type.const_zero().into(); + + // Insert field exprs into struct_val + for (index, field_val) in values { + let index: u32 = index as u32; + + struct_value = env + .builder + .build_insert_value(struct_value, field_val, index, "insert_record_field") + .unwrap(); + } + + struct_value.into_struct_value() +} + fn lookup_at_index_ptr<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, field_layouts: &[Layout<'_>], diff --git a/compiler/gen_llvm/src/llvm/convert.rs b/compiler/gen_llvm/src/llvm/convert.rs index 27b9c82356..4360570d84 100644 --- a/compiler/gen_llvm/src/llvm/convert.rs +++ b/compiler/gen_llvm/src/llvm/convert.rs @@ -180,3 +180,9 @@ pub fn zig_str_type<'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> { + env.module.get_struct_type("list.HasTagId").unwrap() +} From 8d9f5b078e596d2f0f0fcf4ceb4e2a1657102084 Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 24 Jun 2021 21:11:25 +0200 Subject: [PATCH 3/4] fix ffi --- compiler/builtins/bitcode/src/list.zig | 16 +++++++++++----- compiler/gen_llvm/src/llvm/build.rs | 21 --------------------- 2 files changed, 11 insertions(+), 26 deletions(-) diff --git a/compiler/builtins/bitcode/src/list.zig b/compiler/builtins/bitcode/src/list.zig index b91df8b1a7..b1a32c3c61 100644 --- a/compiler/builtins/bitcode/src/list.zig +++ b/compiler/builtins/bitcode/src/list.zig @@ -409,9 +409,11 @@ pub fn listKeepOks( has_tag_id: HasTagId, dec_result: Dec, ) callconv(.C) RocList { + const good_constructor: u16 = 1; + return listKeepResult( list, - RocResult.isOk, + good_constructor, caller, data, inc_n_data, @@ -438,9 +440,11 @@ pub fn listKeepErrs( has_tag_id: HasTagId, dec_result: Dec, ) callconv(.C) RocList { + const good_constructor: u16 = 0; + return listKeepResult( list, - RocResult.isErr, + good_constructor, caller, data, inc_n_data, @@ -456,7 +460,7 @@ pub fn listKeepErrs( pub fn listKeepResult( list: RocList, - is_good_constructor: fn (RocResult) bool, + good_constructor: u16, caller: Caller1, data: Opaque, inc_n_data: IncN, @@ -485,9 +489,11 @@ pub fn listKeepResult( const before_element = source_ptr + (i * before_width); caller(data, before_element, temporary); - const foo = has_tag_id(1, temporary); + const foo = has_tag_id(good_constructor, temporary); if (foo.matched) { - @memcpy(target_ptr + (kept * after_width), foo.data orelse unreachable, after_width); + // drop the tag id + const contents = (foo.data orelse unreachable) + @sizeOf(i64); + @memcpy(target_ptr + (kept * after_width), contents, after_width); kept += 1; } else { dec_result(temporary); diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 46cdb61c02..e1054e6948 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -2464,27 +2464,6 @@ pub fn extract_tag_discriminant<'a, 'ctx, 'env>( } } -fn extract_tag_discriminant_struct<'a, 'ctx, 'env>( - env: &Env<'a, 'ctx, 'env>, - from_value: StructValue<'ctx>, -) -> IntValue<'ctx> { - let struct_type = env - .context - .struct_type(&[env.context.i64_type().into()], false); - - let struct_value = complex_bitcast_struct_struct( - env.builder, - from_value, - struct_type, - "extract_tag_discriminant_struct", - ); - - env.builder - .build_extract_value(struct_value, 0, "") - .expect("desired field did not decode") - .into_int_value() -} - fn extract_tag_discriminant_ptr<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, from_value: PointerValue<'ctx>, From fc4f04dbd8c14ec4c2831a405fda6d3f608a25e5 Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 24 Jun 2021 21:20:50 +0200 Subject: [PATCH 4/4] clean up variable names --- compiler/builtins/bitcode/src/list.zig | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/compiler/builtins/bitcode/src/list.zig b/compiler/builtins/bitcode/src/list.zig index b1a32c3c61..134d4ba0d5 100644 --- a/compiler/builtins/bitcode/src/list.zig +++ b/compiler/builtins/bitcode/src/list.zig @@ -489,10 +489,14 @@ pub fn listKeepResult( const before_element = source_ptr + (i * before_width); caller(data, before_element, temporary); - const foo = has_tag_id(good_constructor, temporary); - if (foo.matched) { + // a record { matched: bool, data: ?[*]u8 } + // for now, that data pointer is just the input `temporary` pointer + // this will change in the future to only return a pointer to the + // payload of the tag + const answer = has_tag_id(good_constructor, temporary); + if (answer.matched) { // drop the tag id - const contents = (foo.data orelse unreachable) + @sizeOf(i64); + const contents = (answer.data orelse unreachable) + @sizeOf(i64); @memcpy(target_ptr + (kept * after_width), contents, after_width); kept += 1; } else {