diff --git a/Cargo.lock b/Cargo.lock index 6bace12f3e..a537330d71 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -833,6 +833,7 @@ dependencies = [ "indoc", "inkwell", "inlinable_string", + "libc", "maplit", "pretty_assertions", "quickcheck", diff --git a/compiler/gen/Cargo.toml b/compiler/gen/Cargo.toml index 72cb231524..c6fc276305 100644 --- a/compiler/gen/Cargo.toml +++ b/compiler/gen/Cargo.toml @@ -54,3 +54,4 @@ quickcheck = "0.8" quickcheck_macros = "0.8" tokio = { version = "0.2", features = ["blocking", "fs", "sync", "rt-threaded"] } bumpalo = { version = "3.2", features = ["collections"] } +libc = "0.2" diff --git a/compiler/gen/src/crane/build.rs b/compiler/gen/src/crane/build.rs index 4727f9e65e..91baa2ad4f 100644 --- a/compiler/gen/src/crane/build.rs +++ b/compiler/gen/src/crane/build.rs @@ -351,6 +351,7 @@ pub fn build_expr<'a, B: Backend>( let bytes_len = str_literal.len() + 1/* TODO drop the +1 when we have structs and this is no longer a NUL-terminated CString.*/; let size = builder.ins().iconst(types::I64, bytes_len as i64); let ptr = call_malloc(env, module, builder, size); + // TODO check if malloc returned null; if so, runtime error for OOM! let mem_flags = MemFlags::new(); // Store the bytes from the string literal in the array @@ -389,6 +390,8 @@ pub fn build_expr<'a, B: Backend>( let bytes_len = elem_bytes as usize * elems.len(); let size = builder.ins().iconst(types::I64, bytes_len as i64); let elems_ptr = call_malloc(env, module, builder, size); + + // TODO check if malloc returned null; if so, runtime error for OOM! let mem_flags = MemFlags::new(); // Copy the elements from the literal into the array @@ -1044,6 +1047,8 @@ fn clone_list( // Allocate space for the new array that we'll copy into. let new_elems_ptr = call_malloc(env, module, builder, size); + // TODO check if malloc returned null; if so, runtime error for OOM! + // Either memcpy or deep clone the array elements if elem_layout.safe_to_memcpy() { // Copy the bytes from the original array into the new diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index 7ed0e04cbd..a10c2df0f3 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -208,20 +208,23 @@ pub fn build_expr<'a, 'ctx, 'env>( } else { let ctx = env.context; let builder = env.builder; - let bytes_len = str_literal.len() + 1/* TODO drop the +1 when we have structs and this is no longer a NUL-terminated CString.*/; + let str_len = str_literal.len() + 1/* TODO drop the +1 when we have structs and this is no longer a NUL-terminated CString.*/; let byte_type = ctx.i8_type(); let nul_terminator = byte_type.const_zero(); - let len = ctx.i32_type().const_int(bytes_len as u64, false); + let len_val = ctx.i32_type().const_int(str_len as u64, false); let ptr = env .builder - .build_array_malloc(ctx.i8_type(), len, "str_ptr") + .build_array_malloc(ctx.i8_type(), len_val, "str_ptr") .unwrap(); + // TODO check if malloc returned null; if so, runtime error for OOM! + // Copy the bytes from the string literal into the array for (index, byte) in str_literal.bytes().enumerate() { - let index = ctx.i32_type().const_int(index as u64, false); - let elem_ptr = unsafe { builder.build_gep(ptr, &[index], "byte") }; + let index_val = ctx.i32_type().const_int(index as u64, false); + let elem_ptr = + unsafe { builder.build_in_bounds_gep(ptr, &[index_val], "byte") }; builder.build_store(elem_ptr, byte_type.const_int(byte as u64, false)); } @@ -229,8 +232,9 @@ pub fn build_expr<'a, 'ctx, 'env>( // Add a NUL terminator at the end. // TODO: Instead of NUL-terminating, return a struct // with the pointer and also the length and capacity. - let index = ctx.i32_type().const_int(bytes_len as u64 - 1, false); - let elem_ptr = unsafe { builder.build_gep(ptr, &[index], "nul_terminator") }; + let index_val = ctx.i32_type().const_int(str_len as u64 - 1, false); + let elem_ptr = + unsafe { builder.build_in_bounds_gep(ptr, &[index_val], "nul_terminator") }; builder.build_store(elem_ptr, nul_terminator); @@ -260,13 +264,15 @@ pub fn build_expr<'a, 'ctx, 'env>( env.builder .build_array_malloc(elem_type, len, "create_list_ptr") .unwrap() + + // TODO check if malloc returned null; if so, runtime error for OOM! }; // Copy the elements from the list literal into the array for (index, elem) in elems.iter().enumerate() { - let offset = ctx.i32_type().const_int(elem_bytes * index as u64, false); - let elem_ptr = unsafe { builder.build_gep(ptr, &[offset], "elem") }; - + let index_val = ctx.i32_type().const_int(index as u64, false); + let elem_ptr = + unsafe { builder.build_in_bounds_gep(ptr, &[index_val], "index") }; let val = build_expr(env, &scope, parent, &elem, procs); builder.build_store(elem_ptr, val); @@ -1065,18 +1071,13 @@ fn call_with_args<'a, 'ctx, 'env>( // TODO here, check to see if the requested index exceeds the length of the array. match list_layout { - Layout::Builtin(Builtin::List(elem_layout)) => { + Layout::Builtin(Builtin::List(_)) => { // Load the pointer to the array data let array_data_ptr = load_list_ptr(builder, wrapper_struct); - let elem_bytes = elem_layout.stack_size(env.ptr_bytes) as u64; - let elem_size = env.ptr_int().const_int(elem_bytes, false); - - // Calculate the offset at runtime by multiplying the index by the size of an element. - let offset_bytes = builder.build_int_mul(elem_index, elem_size, "mul_offset"); // We already checked the bounds earlier. let elem_ptr = unsafe { - builder.build_in_bounds_gep(array_data_ptr, &[offset_bytes], "elem") + builder.build_in_bounds_gep(array_data_ptr, &[elem_index], "elem") }; builder.build_load(elem_ptr, "List.get") @@ -1111,16 +1112,12 @@ fn call_with_args<'a, 'ctx, 'env>( ) }; - let elem_bytes = elem_layout.stack_size(env.ptr_bytes) as u64; - let elem_size = env.ptr_int().const_int(elem_bytes, false); - // Calculate the offset at runtime by multiplying the index by the size of an element. let elem_index = args[1].0.into_int_value(); - let offset_bytes = builder.build_int_mul(elem_index, elem_size, "mul_offset"); // We already checked the bounds earlier. let elem_ptr = - unsafe { builder.build_in_bounds_gep(array_data_ptr, &[offset_bytes], "elem") }; + unsafe { builder.build_in_bounds_gep(array_data_ptr, &[elem_index], "elem") }; // Mutate the array in-place. builder.build_store(elem_ptr, elem); @@ -1135,7 +1132,7 @@ fn call_with_args<'a, 'ctx, 'env>( let wrapper_struct = args[0].0.into_struct_value(); let elem_index = args[1].0.into_int_value(); - let (elem, elem_layout) = args[2]; + let (elem, _elem_layout) = args[2]; // Load the usize length let _list_len = load_list_len(builder, wrapper_struct); @@ -1146,15 +1143,9 @@ fn call_with_args<'a, 'ctx, 'env>( // Load the pointer to the elements let array_data_ptr = load_list_ptr(builder, wrapper_struct); - let elem_bytes = elem_layout.stack_size(env.ptr_bytes) as u64; - let elem_size = env.context.i64_type().const_int(elem_bytes, false); - - // Calculate the offset at runtime by multiplying the index by the size of an element. - let offset_bytes = builder.build_int_mul(elem_index, elem_size, "mul_offset"); - // We already checked the bounds earlier. let elem_ptr = - unsafe { builder.build_in_bounds_gep(array_data_ptr, &[offset_bytes], "elem") }; + unsafe { builder.build_in_bounds_gep(array_data_ptr, &[elem_index], "elem") }; // Mutate the array in-place. builder.build_store(elem_ptr, elem); @@ -1229,6 +1220,8 @@ fn clone_list<'a, 'ctx, 'env>( .build_array_malloc(elem_type, list_len, "list_ptr") .unwrap(); + // TODO check if malloc returned null; if so, runtime error for OOM! + // Either memcpy or deep clone the array elements if elem_layout.safe_to_memcpy() { // Copy the bytes from the original array into the new diff --git a/compiler/gen/src/llvm/convert.rs b/compiler/gen/src/llvm/convert.rs index 57916f8a77..bbbc117c53 100644 --- a/compiler/gen/src/llvm/convert.rs +++ b/compiler/gen/src/llvm/convert.rs @@ -151,7 +151,14 @@ pub fn collection_wrapper<'ctx>( let ptr_type_enum = BasicTypeEnum::PointerType(ptr_type); let len_type = BasicTypeEnum::IntType(ptr_int(ctx, ptr_bytes)); - ctx.struct_type(&[ptr_type_enum, len_type], false) + // 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) + } } pub fn ptr_int(ctx: &Context, ptr_bytes: u32) -> IntType<'_> { diff --git a/compiler/gen/tests/test_gen.rs b/compiler/gen/tests/test_gen.rs index 617c94ce8c..ea7ede9851 100644 --- a/compiler/gen/tests/test_gen.rs +++ b/compiler/gen/tests/test_gen.rs @@ -5,6 +5,7 @@ extern crate indoc; extern crate bumpalo; extern crate inkwell; +extern crate libc; extern crate roc_gen; mod helpers; @@ -541,6 +542,11 @@ mod test_gen { assert_llvm_evals_to!("[]", &[], &'static [i64], |x| x); } + #[test] + fn int_list_literal() { + assert_llvm_evals_to!("[ 12, 9, 6, 3 ]", &[12, 9, 6, 3], &'static [i64], |x| x); + } + #[test] fn head_int_list() { assert_evals_to!("List.getUnsafe [ 12, 9, 6, 3 ] 0", 12, i64); diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index 939062c832..e6e312cb76 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -143,10 +143,12 @@ impl<'a> Builtin<'a> { pub const SET_WORDS: u32 = Builtin::MAP_WORDS; // Set is an alias for Map with {} for value pub const LIST_WORDS: u32 = 2; - /// Layout of collection wrapper - a struct of (pointer, length, capacity) + /// Layout of collection wrapper for List and Str - a struct of (pointre, length). + /// + /// We choose this layout (with pointer first) because it's how + /// Rust slices are laid out, meaning we can cast to/from them for free. pub const WRAPPER_PTR: u32 = 0; pub const WRAPPER_LEN: u32 = 1; - pub const WRAPPER_CAPACITY: u32 = 2; pub fn stack_size(&self, pointer_size: u32) -> u32 { use Builtin::*;