Always store collections as (usize, usize)

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;
+            BasicValueEnum::IntValue(load_list_len(env.builder, args[0].0.into_struct_value()))
-            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,
-            ))
         }
         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
+    let ptr_as_int = builder
-        .build_bitcast(wrapper_struct, struct_type, "cast_collection")
+        .build_extract_value(wrapper_struct, Builtin::WRAPPER_PTR, "read_list_ptr")
-        .into_struct_value();
-
-    builder
-        .build_extract_value(wrapper_struct, Builtin::WRAPPER_PTR, "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,
             ),
         };

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::Layout;
-use roc_mono::layout::{Builtin, 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.
+/// Two usize values. Could be a wrapper for a List or a Str.
-/// 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 int_type = BasicTypeEnum::IntType(ptr_int(ctx, ptr_bytes));
-    let total_bytes = ptr_bytes * num_fields;
-    let array_type = ctx.i8_type().array_type(total_bytes).as_basic_type_enum();
 
-    ctx.struct_type(&[array_type], false).into()
+    ctx.struct_type(&[int_type, int_type], false)
 }
 
-/// A length usize and a pointer to some elements.
+/// Two usize values.
-///
+pub fn collection_int_wrapper<'ctx>(ctx: &'ctx Context, ptr_bytes: u32) -> StructType<'ctx> {
-/// Could be a wrapper for a List or a Str.
+    let usize_type = BasicTypeEnum::IntType(ptr_int(ctx, ptr_bytes));
-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));
 
-    // This conditional is based on a constant, so the branch should be optimized away.
+    ctx.struct_type(&[usize_type, usize_type], false)
-    // 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<'_> {