diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index 1dcbba58c8..c0eb7054a4 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -1,5 +1,5 @@ use crate::llvm::convert::{ - basic_type_from_layout, collection, collection_wrapper, get_fn_type, get_ptr_type, ptr_int, + basic_type_from_layout, collection, get_fn_type, get_ptr_type, ptr_int, }; use bumpalo::collections::Vec; use bumpalo::Bump; @@ -8,7 +8,7 @@ use inkwell::context::Context; use inkwell::memory_buffer::MemoryBuffer; use inkwell::module::{Linkage, Module}; use inkwell::passes::{PassManager, PassManagerBuilder}; -use inkwell::types::{BasicTypeEnum, FunctionType, IntType, StructType}; +use inkwell::types::{BasicTypeEnum, FunctionType, IntType, PointerType, StructType}; use inkwell::values::BasicValueEnum::{self, *}; use inkwell::values::{FunctionValue, IntValue, PointerValue, StructValue}; use inkwell::AddressSpace; @@ -391,9 +391,10 @@ pub fn build_expr<'a, 'ctx, 'env>( builder.build_store(elem_ptr, val); } - let ptr_val = BasicValueEnum::PointerValue(ptr); let ptr_bytes = env.ptr_bytes; - let struct_type = collection_wrapper(ctx, ptr.get_type(), ptr_bytes); + let int_type = ptr_int(ctx, ptr_bytes); + let ptr_as_int = builder.build_ptr_to_int(ptr, int_type, "list_cast_ptr"); + let struct_type = collection(ctx, ptr_bytes); let len = BasicValueEnum::IntValue(env.ptr_int().const_int(len_u64, false)); let mut struct_val; @@ -401,7 +402,7 @@ pub fn build_expr<'a, 'ctx, 'env>( struct_val = builder .build_insert_value( struct_type.get_undef(), - ptr_val, + ptr_as_int, Builtin::WRAPPER_PTR, "insert_ptr", ) @@ -673,7 +674,7 @@ fn cast_basic_basic<'ctx>( let to_type_pointer = builder .build_bitcast( argument_pointer, - to_type.ptr_type(AddressSpace::Generic), + to_type.ptr_type(inkwell::AddressSpace::Generic), "", ) .into_pointer_value(); @@ -1109,28 +1110,14 @@ fn call_with_args<'a, 'ctx, 'env>( Symbol::LIST_LEN => { debug_assert!(args.len() == 1); - let ctx = env.context; - let elem_type = basic_type_from_layout(env.arena, ctx, elem_layout, env.ptr_bytes); - let ptr_type = get_ptr_type(&elem_type, AddressSpace::Generic); - let wrapper_type = collection_wrapper(ctx, ptr_type, env.ptr_bytes); - - BasicValueEnum::IntValue(load_list_len( - env.builder, - args[0].0.into_struct_value(), - wrapper_type, - )) + BasicValueEnum::IntValue(load_list_len(env.builder, args[0].0.into_struct_value())) } Symbol::LIST_IS_EMPTY => { debug_assert!(args.len() == 1); let list_struct = args[0].0.into_struct_value(); - - let ctx = env.context; - let elem_type = basic_type_from_layout(env.arena, ctx, elem_layout, env.ptr_bytes); - let ptr_type = get_ptr_type(&elem_type, AddressSpace::Generic); - let wrapper_type = collection_wrapper(ctx, ptr_type, env.ptr_bytes); let builder = env.builder; - let list_len = load_list_len(builder, list_struct, wrapper_type); + let list_len = load_list_len(builder, list_struct); let zero = env.ptr_int().const_zero(); let answer = builder.build_int_compare(IntPredicate::EQ, list_len, zero, "is_zero"); @@ -1226,10 +1213,8 @@ fn call_with_args<'a, 'ctx, 'env>( let elem_type = basic_type_from_layout(env.arena, ctx, elem_layout, env.ptr_bytes); let ptr_type = get_ptr_type(&elem_type, AddressSpace::Generic); - let wrapper_type = collection_wrapper(ctx, ptr_type, env.ptr_bytes); - // Load the pointer to the array data - let array_data_ptr = load_list_ptr(builder, wrapper_struct, wrapper_type); + let array_data_ptr = load_list_ptr(builder, wrapper_struct, ptr_type); // Assume the bounds have already been checked earlier // (e.g. by List.get or List.first, which wrap List.#getUnsafe) @@ -1317,12 +1302,7 @@ fn call_intrinsic<'a, 'ctx, 'env>( fn load_list_len<'ctx>( builder: &Builder<'ctx>, wrapper_struct: StructValue<'ctx>, - struct_type: StructType<'ctx>, ) -> IntValue<'ctx> { - let wrapper_struct = builder - .build_bitcast(wrapper_struct, struct_type, "cast_collection") - .into_struct_value(); - builder .build_extract_value(wrapper_struct, Builtin::WRAPPER_LEN, "list_len") .unwrap() @@ -1332,16 +1312,14 @@ fn load_list_len<'ctx>( fn load_list_ptr<'ctx>( builder: &Builder<'ctx>, wrapper_struct: StructValue<'ctx>, - struct_type: StructType<'ctx>, + ptr_type: PointerType<'ctx>, ) -> PointerValue<'ctx> { - let wrapper_struct = builder - .build_bitcast(wrapper_struct, struct_type, "cast_collection") - .into_struct_value(); - - builder - .build_extract_value(wrapper_struct, Builtin::WRAPPER_PTR, "list_ptr") + let ptr_as_int = builder + .build_extract_value(wrapper_struct, Builtin::WRAPPER_PTR, "read_list_ptr") .unwrap() - .into_pointer_value() + .into_int_value(); + + builder.build_int_to_ptr(ptr_as_int, ptr_type, "list_cast_ptr") } fn clone_nonempty_list<'a, 'ctx, 'env>( @@ -1367,6 +1345,8 @@ fn clone_nonempty_list<'a, 'ctx, 'env>( let clone_ptr = builder .build_array_malloc(elem_type, list_len, "list_ptr") .unwrap(); + let int_type = ptr_int(ctx, ptr_bytes); + let ptr_as_int = builder.build_ptr_to_int(clone_ptr, int_type, "list_cast_ptr"); // TODO check if malloc returned null; if so, runtime error for OOM! @@ -1382,14 +1362,14 @@ fn clone_nonempty_list<'a, 'ctx, 'env>( } // Create a fresh wrapper struct for the newly populated array - let struct_type = collection_wrapper(ctx, clone_ptr.get_type(), env.ptr_bytes); + let struct_type = collection(ctx, env.ptr_bytes); let mut struct_val; // Store the pointer struct_val = builder .build_insert_value( struct_type.get_undef(), - clone_ptr, + ptr_as_int, Builtin::WRAPPER_PTR, "insert_ptr", ) @@ -1442,13 +1422,8 @@ fn list_set<'a, 'ctx, 'env>( let original_wrapper = args[0].0.into_struct_value(); let elem_index = args[1].0.into_int_value(); - let ctx = env.context; - let elem_type = basic_type_from_layout(env.arena, ctx, elem_layout, env.ptr_bytes); - let ptr_type = get_ptr_type(&elem_type, AddressSpace::Generic); - let wrapper_type = collection_wrapper(ctx, ptr_type, env.ptr_bytes); - // Load the usize length from the wrapper. We need it for bounds checking. - let list_len = load_list_len(builder, original_wrapper, wrapper_type); + let list_len = load_list_len(builder, original_wrapper); // Bounds check: only proceed if index < length. // Otherwise, return the list unaltered. @@ -1460,16 +1435,15 @@ fn list_set<'a, 'ctx, 'env>( let ctx = env.context; let elem_type = basic_type_from_layout(env.arena, ctx, elem_layout, env.ptr_bytes); let ptr_type = get_ptr_type(&elem_type, AddressSpace::Generic); - let wrapper_type = collection_wrapper(ctx, ptr_type, env.ptr_bytes); let (new_wrapper, array_data_ptr) = match in_place { InPlace::InPlace => ( original_wrapper, - load_list_ptr(builder, original_wrapper, wrapper_type), + load_list_ptr(builder, original_wrapper, ptr_type), ), InPlace::Clone => clone_nonempty_list( env, list_len, - load_list_ptr(builder, original_wrapper, wrapper_type), + load_list_ptr(builder, original_wrapper, ptr_type), elem_layout, ), }; diff --git a/compiler/gen/src/llvm/convert.rs b/compiler/gen/src/llvm/convert.rs index d0bfd12ce9..ee42ce08ff 100644 --- a/compiler/gen/src/llvm/convert.rs +++ b/compiler/gen/src/llvm/convert.rs @@ -4,8 +4,7 @@ use inkwell::context::Context; use inkwell::types::BasicTypeEnum::{self, *}; use inkwell::types::{ArrayType, BasicType, FunctionType, IntType, PointerType, StructType}; use inkwell::AddressSpace; - -use roc_mono::layout::{Builtin, Layout}; +use roc_mono::layout::Layout; /// TODO could this be added to Inkwell itself as a method on BasicValueEnum? pub fn get_ptr_type<'ctx>( @@ -138,35 +137,28 @@ pub fn basic_type_from_layout<'ctx>( } } -/// A length usize and a pointer to some elements. -/// Could be a wrapper for a List or a Str. +/// Two usize values. Could be a wrapper for a List or a Str. +/// +/// It would be nicer if we could store this as a tuple containing one usize +/// and one pointer. However, if we do that, we run into a problem with the +/// empty list: it doesn't know what pointer type it should initailize to, +/// so it can only create an empty (usize, usize) struct. +/// +/// This way, we always initialize it to (usize, usize), and then if there's +/// actually a pointer, we use build_int_to_ptr and build_ptr_to_int to convert +/// the field when necessary. (It's not allowed to cast the entire struct from +/// (usize, usize) to (usize, ptr) or vice versa.) pub fn collection(ctx: &Context, ptr_bytes: u32) -> StructType<'_> { - let num_fields = 2; - let total_bytes = ptr_bytes * num_fields; - let array_type = ctx.i8_type().array_type(total_bytes).as_basic_type_enum(); + let int_type = BasicTypeEnum::IntType(ptr_int(ctx, ptr_bytes)); - ctx.struct_type(&[array_type], false).into() + ctx.struct_type(&[int_type, int_type], false) } -/// A length usize and a pointer to some elements. -/// -/// Could be a wrapper for a List or a Str. -pub fn collection_wrapper<'ctx>( - ctx: &'ctx Context, - ptr_type: PointerType<'ctx>, - ptr_bytes: u32, -) -> StructType<'ctx> { - let ptr_type_enum = BasicTypeEnum::PointerType(ptr_type); - let len_type = BasicTypeEnum::IntType(ptr_int(ctx, ptr_bytes)); +/// Two usize values. +pub fn collection_int_wrapper<'ctx>(ctx: &'ctx Context, ptr_bytes: u32) -> StructType<'ctx> { + let usize_type = BasicTypeEnum::IntType(ptr_int(ctx, ptr_bytes)); - // This conditional is based on a constant, so the branch should be optimized away. - // The reason for keeping the conditional here is so we can flip the order - // of the fields (by changing the constants) without breaking this code. - if Builtin::WRAPPER_PTR == 0 { - ctx.struct_type(&[ptr_type_enum, len_type], false) - } else { - ctx.struct_type(&[len_type, ptr_type_enum], false) - } + ctx.struct_type(&[usize_type, usize_type], false) } pub fn ptr_int(ctx: &Context, ptr_bytes: u32) -> IntType<'_> {