Sfoglia il codice sorgente

array of structs should not be array of pointer to structs

Fixed sized arrays and structs should be stored inline rather than
storing a pointer to them. Any many cases this reduces the number of
allocation required, the amount of pointer chasing we need to do and
most importantly, makes it possible to use the same data layout for
solana system calls.

This has the following effects on the type system:

 - a variable declared as `int[2] a` is a pointer to an array of two
   elements with type Type::Array(Type::Int(256), Some(2)).
 - a structure declared as `struct S { bool f1; int[2] f2; }` is now
   a single structure and f2 is no longer a pointer to another object.
 - when `f2` of structure `S` is accessed, sema gives it the type:
   Type::Ref(Type::Array(Type::Int(256), Some(2))). This is still a
   simple pointer (as would have been without the Type::Ref. The
   Type::Ref() means is can be used as an l-value, else it would be
   un-assignable.
 - however, assigning to this type does mean copying the entire member
   rather than just a pointer.
 - in many instances, pointer chasing is no longer necessary

This is a minimal implementation which needs further optimization. For
example, this code:

	struct S { int[2] a; bool b; }

	S s = S([1,2], true);

This will allocate an [1,2] array and then copy it into the larger
struct. This allocate/copy should be avoided. The same issue appears in
many other situations:

	struct S { int[2] a; bool b; }

	function lit() returns (int[2]) {
		return [1, 2]
	}

	S s = S(lit(), true);

and

	contract C {
		struct S { int[2] a; bool b; }
		int[2] b;

		function foo() public {
			S s = S(b, true);
		}

In each instance, we should allocate a single struct and then pass the
pinter for the a member to the struct literal generation, function call,
or the codegen for reading from storage. This would avoid the unnecessary
copy.

Signed-off-by: Sean Young <sean@mess.org>
Sean Young 3 anni fa
parent
commit
11394a8c7d

+ 6 - 0
src/codegen/expression.rs

@@ -1832,6 +1832,12 @@ pub fn assign_single(
             let dest = expression(left, cfg, contract_no, func, ns, vartab, opt);
             let right = expression(right, cfg, contract_no, func, ns, vartab, opt);
 
+            let right = if !left_ty.is_contract_storage() && right.ty().is_fixed_reference_type() {
+                Expression::Load(pt::Loc::Codegen, right.ty(), Box::new(right))
+            } else {
+                right
+            };
+
             cfg.add(
                 vartab,
                 Instr::Set {

+ 47 - 47
src/emit/ethabiencoder.rs

@@ -384,7 +384,7 @@ impl<'a, 'b> EncoderBuilder<'a, 'b> {
 
                     let len = EncoderBuilder::encoded_dynamic_length(
                         elem.into(),
-                        true,
+                        !field.ty.is_fixed_reference_type(),
                         &field.ty,
                         function,
                         binary,
@@ -858,7 +858,7 @@ impl<'a, 'b> EncoderBuilder<'a, 'b> {
                 self.encode_ty(
                     binary,
                     ns,
-                    true,
+                    !elem_ty.is_fixed_reference_type(),
                     function,
                     elem_ty.deref_any(),
                     elem.into(),
@@ -1038,7 +1038,7 @@ impl<'a, 'b> EncoderBuilder<'a, 'b> {
                 self.encode_ty(
                     binary,
                     ns,
-                    true,
+                    !elem_ty.is_fixed_reference_type(),
                     function,
                     elem_ty.deref_any(),
                     elem.into(),
@@ -1246,7 +1246,7 @@ impl<'a, 'b> EncoderBuilder<'a, 'b> {
                     self.encode_ty(
                         binary,
                         ns,
-                        true,
+                        !field.ty.is_fixed_reference_type(),
                         function,
                         &field.ty,
                         elem.into(),
@@ -1355,7 +1355,7 @@ impl<'a, 'b> EncoderBuilder<'a, 'b> {
                     self.encode_ty(
                         binary,
                         ns,
-                        true,
+                        !field.ty.is_fixed_reference_type(),
                         function,
                         &field.ty,
                         elem.into(),
@@ -1800,7 +1800,7 @@ impl<'a, 'b> EncoderBuilder<'a, 'b> {
 
                 self.encode_packed_ty(
                     binary,
-                    true,
+                    !elem_ty.is_fixed_reference_type(),
                     function,
                     elem_ty.deref_any(),
                     elem.into(),
@@ -1905,7 +1905,7 @@ impl<'a, 'b> EncoderBuilder<'a, 'b> {
 
                     self.encode_packed_ty(
                         binary,
-                        true,
+                        !field.ty.is_fixed_reference_type(),
                         function,
                         &field.ty,
                         elem.into(),
@@ -2673,23 +2673,25 @@ impl EthAbiDecoder {
                 let dest;
 
                 if let Some(d) = &dim[0] {
-                    let new = binary
-                        .builder
-                        .build_call(
-                            binary.module.get_function("__malloc").unwrap(),
-                            &[size.into()],
-                            "",
-                        )
-                        .try_as_basic_value()
-                        .left()
-                        .unwrap()
-                        .into_pointer_value();
+                    dest = to.unwrap_or_else(|| {
+                        let new = binary
+                            .builder
+                            .build_call(
+                                binary.module.get_function("__malloc").unwrap(),
+                                &[size.into()],
+                                "",
+                            )
+                            .try_as_basic_value()
+                            .left()
+                            .unwrap()
+                            .into_pointer_value();
 
-                    dest = binary.builder.build_pointer_cast(
-                        new,
-                        llvm_ty.ptr_type(AddressSpace::Generic),
-                        "dest",
-                    );
+                        binary.builder.build_pointer_cast(
+                            new,
+                            llvm_ty.ptr_type(AddressSpace::Generic),
+                            "dest",
+                        )
+                    });
 
                     // if the struct has dynamic fields, read offset from dynamic section and
                     // read fields from there
@@ -2797,7 +2799,7 @@ impl EthAbiDecoder {
                     // in dynamic arrays, offsets are counted from after the array length
                     let base_offset = dataoffset;
 
-                    let llvm_elem_ty = binary.llvm_var(elem_ty.deref_any(), ns);
+                    let llvm_elem_ty = binary.llvm_field_ty(elem_ty.deref_any(), ns);
                     let elem_size = llvm_elem_ty
                         .size_of()
                         .unwrap()
@@ -2871,10 +2873,10 @@ impl EthAbiDecoder {
                             );
                         },
                     );
-                }
 
-                if let Some(to) = to {
-                    binary.builder.build_store(to, dest);
+                    if let Some(to) = to {
+                        binary.builder.build_store(to, dest);
+                    }
                 }
 
                 dest.into()
@@ -2887,23 +2889,25 @@ impl EthAbiDecoder {
                     .unwrap()
                     .const_cast(binary.context.i32_type(), false);
 
-                let new = binary
-                    .builder
-                    .build_call(
-                        binary.module.get_function("__malloc").unwrap(),
-                        &[size.into()],
-                        "",
-                    )
-                    .try_as_basic_value()
-                    .left()
-                    .unwrap()
-                    .into_pointer_value();
+                let struct_pointer = to.unwrap_or_else(|| {
+                    let new = binary
+                        .builder
+                        .build_call(
+                            binary.module.get_function("__malloc").unwrap(),
+                            &[size.into()],
+                            "",
+                        )
+                        .try_as_basic_value()
+                        .left()
+                        .unwrap()
+                        .into_pointer_value();
 
-                let struct_pointer = binary.builder.build_pointer_cast(
-                    new,
-                    llvm_ty.ptr_type(AddressSpace::Generic),
-                    &ns.structs[*n].name,
-                );
+                    binary.builder.build_pointer_cast(
+                        new,
+                        llvm_ty.ptr_type(AddressSpace::Generic),
+                        &ns.structs[*n].name,
+                    )
+                });
 
                 // if the struct has dynamic fields, read offset from dynamic section and
                 // read fields from there
@@ -2969,10 +2973,6 @@ impl EthAbiDecoder {
                     *offset = dataoffset;
                 }
 
-                if let Some(to) = to {
-                    binary.builder.build_store(to, struct_pointer);
-                }
-
                 struct_pointer.into()
             }
             ast::Type::Ref(ty) => self.decode_ty(

+ 115 - 99
src/emit/mod.rs

@@ -416,7 +416,7 @@ pub trait TargetRuntime<'a> {
     ) -> BasicValueEnum<'a> {
         match ty {
             Type::Ref(ty) => self.storage_load_slot(bin, ty, slot, slot_ptr, function, ns),
-            Type::Array(_, dim) => {
+            Type::Array(elem_ty, dim) => {
                 if let Some(d) = &dim[0] {
                     let llvm_ty = bin.llvm_type(ty.deref_any(), ns);
                     // LLVMSizeOf() produces an i64
@@ -462,6 +462,12 @@ pub trait TargetRuntime<'a> {
                             let val =
                                 self.storage_load_slot(bin, &ty, slot, slot_ptr, function, ns);
 
+                            let val = if ty.deref_memory().is_fixed_reference_type() {
+                                bin.builder.build_load(val.into_pointer_value(), "elem")
+                            } else {
+                                val
+                            };
+
                             bin.builder.build_store(elem, val);
                         },
                     );
@@ -478,10 +484,10 @@ pub trait TargetRuntime<'a> {
                         "size",
                     );
 
-                    let elem_ty = bin.llvm_var(&ty.array_elem(), ns);
+                    let llvm_elem_ty = bin.llvm_field_ty(elem_ty, ns);
 
                     let elem_size = bin.builder.build_int_truncate(
-                        elem_ty.size_of().unwrap(),
+                        llvm_elem_ty.size_of().unwrap(),
                         bin.context.i32_type(),
                         "size_of",
                     );
@@ -526,37 +532,18 @@ pub trait TargetRuntime<'a> {
                         size,
                         &mut elem_slot,
                         |elem_no: IntValue<'a>, slot: &mut IntValue<'a>| {
-                            let index = bin.builder.build_int_mul(elem_no, elem_size, "");
+                            let elem = bin.array_subscript(ty, dest, elem_no, ns);
 
-                            let entry = self.storage_load_slot(
-                                bin,
-                                &ty.array_elem(),
-                                slot,
-                                slot_ptr,
-                                function,
-                                ns,
-                            );
+                            let entry =
+                                self.storage_load_slot(bin, elem_ty, slot, slot_ptr, function, ns);
 
-                            let data = unsafe {
-                                bin.builder.build_gep(
-                                    dest,
-                                    &[
-                                        bin.context.i32_type().const_zero(),
-                                        bin.context.i32_type().const_int(2, false),
-                                        index,
-                                    ],
-                                    "data",
-                                )
+                            let entry = if elem_ty.deref_memory().is_fixed_reference_type() {
+                                bin.builder.build_load(entry.into_pointer_value(), "elem")
+                            } else {
+                                entry
                             };
 
-                            bin.builder.build_store(
-                                bin.builder.build_pointer_cast(
-                                    data,
-                                    elem_ty.ptr_type(AddressSpace::Generic),
-                                    "entry",
-                                ),
-                                entry,
-                            );
+                            bin.builder.build_store(elem, entry);
                         },
                     );
                     // load
@@ -604,6 +591,13 @@ pub trait TargetRuntime<'a> {
                         )
                     };
 
+                    let val = if field.ty.deref_memory().is_fixed_reference_type() {
+                        bin.builder
+                            .build_load(val.into_pointer_value(), field.name_as_str())
+                    } else {
+                        val
+                    };
+
                     bin.builder.build_store(elem, val);
                 }
 
@@ -731,7 +725,9 @@ pub trait TargetRuntime<'a> {
                                 )
                             };
 
-                            if ty.is_reference_type(ns) {
+                            if ty.is_reference_type(ns)
+                                && !ty.deref_memory().is_fixed_reference_type()
+                            {
                                 elem = bin.builder.build_load(elem, "").into_pointer_value();
                             }
 
@@ -761,7 +757,7 @@ pub trait TargetRuntime<'a> {
                     let slot_ty = Type::Uint(256);
 
                     // details about our array elements
-                    let elem_ty = bin.llvm_var(&ty.array_elem(), ns);
+                    let elem_ty = bin.llvm_field_ty(&ty.array_elem(), ns);
                     let elem_size = bin.builder.build_int_truncate(
                         elem_ty.size_of().unwrap(),
                         bin.context.i32_type(),
@@ -836,7 +832,9 @@ pub trait TargetRuntime<'a> {
                                 "entry",
                             );
 
-                            if ty.is_reference_type(ns) {
+                            if ty.is_reference_type(ns)
+                                && !ty.deref_memory().is_fixed_reference_type()
+                            {
                                 elem = bin.builder.build_load(elem, "").into_pointer_value();
                             }
 
@@ -894,7 +892,7 @@ pub trait TargetRuntime<'a> {
                         )
                     };
 
-                    if field.ty.is_reference_type(ns) {
+                    if field.ty.is_reference_type(ns) && !field.ty.is_fixed_reference_type() {
                         elem = bin
                             .builder
                             .build_load(elem, field.name_as_str())
@@ -1189,8 +1187,8 @@ pub trait TargetRuntime<'a> {
                     "struct_literal",
                 );
 
-                for (i, f) in exprs.iter().enumerate() {
-                    let elem = unsafe {
+                for (i, expr) in exprs.iter().enumerate() {
+                    let elemptr = unsafe {
                         bin.builder.build_gep(
                             s,
                             &[
@@ -1201,8 +1199,15 @@ pub trait TargetRuntime<'a> {
                         )
                     };
 
-                    bin.builder
-                        .build_store(elem, self.expression(bin, f, vartab, function, ns));
+                    let elem = self.expression(bin, expr, vartab, function, ns);
+
+                    let elem = if expr.ty().is_fixed_reference_type() {
+                        bin.builder.build_load(elem.into_pointer_value(), "elem")
+                    } else {
+                        elem
+                    };
+
+                    bin.builder.build_store(elemptr, elem);
                 }
 
                 s.into()
@@ -2017,7 +2022,7 @@ pub trait TargetRuntime<'a> {
 
                 let value = bin.builder.build_load(ptr, "");
 
-                if ty.is_reference_type(ns) {
+                if ty.is_reference_type(ns) && !ty.is_fixed_reference_type() {
                     // if the pointer is null, it needs to be allocated
                     let allocation_needed = bin
                         .builder
@@ -2355,7 +2360,7 @@ pub trait TargetRuntime<'a> {
                 } else if ty.is_dynamic_memory() {
                     let array = self.expression(bin, a, vartab, function, ns);
 
-                    let ty = bin.llvm_var(elem_ty, ns);
+                    let ty = bin.llvm_field_ty(elem_ty, ns);
 
                     let mut array_index = self
                         .expression(bin, i, vartab, function, ns)
@@ -2507,8 +2512,15 @@ pub trait TargetRuntime<'a> {
                     let elemptr =
                         unsafe { bin.builder.build_gep(array, &ind, &format!("elemptr{}", i)) };
 
-                    bin.builder
-                        .build_store(elemptr, self.expression(bin, expr, vartab, function, ns));
+                    let elem = self.expression(bin, expr, vartab, function, ns);
+
+                    let elem = if expr.ty().is_fixed_reference_type() {
+                        bin.builder.build_load(elem.into_pointer_value(), "elem")
+                    } else {
+                        elem
+                    };
+
+                    bin.builder.build_store(elemptr, elem);
                 }
 
                 array.into()
@@ -3338,7 +3350,7 @@ pub trait TargetRuntime<'a> {
 
             if let Some(ref cfg_phis) = cfg_bb.phis {
                 for v in cfg_phis {
-                    let ty = bin.llvm_var(&cfg.vars[v].ty, ns);
+                    let ty = bin.llvm_var_ty(&cfg.vars[v].ty, ns);
 
                     phis.insert(*v, bin.builder.build_phi(ty, &cfg.vars[v].id.name));
                 }
@@ -3615,41 +3627,19 @@ pub trait TargetRuntime<'a> {
                         array,
                         value,
                     } => {
-                        let a = w.vars[array].value.into_pointer_value();
-                        let len = unsafe {
-                            bin.builder.build_gep(
-                                a,
-                                &[
-                                    bin.context.i32_type().const_zero(),
-                                    bin.context.i32_type().const_zero(),
-                                ],
-                                "array_len",
-                            )
-                        };
-                        let a = bin.builder.build_pointer_cast(
-                            a,
-                            bin.context.i8_type().ptr_type(AddressSpace::Generic),
-                            "a",
-                        );
+                        let arr = w.vars[array].value;
+
                         let llvm_ty = bin.llvm_type(ty, ns);
 
+                        let elem_ty = ty.array_elem();
+
                         // Calculate total size for reallocation
-                        let elem_ty = match ty {
-                            Type::Array(..) => match bin.llvm_type(&ty.array_elem(), ns) {
-                                elem @ BasicTypeEnum::StructType(_) => {
-                                    // We don't store structs directly in the array, instead we store references to structs
-                                    elem.ptr_type(AddressSpace::Generic).as_basic_type_enum()
-                                }
-                                elem => elem,
-                            },
-                            Type::DynamicBytes => bin.context.i8_type().into(),
-                            _ => unreachable!(),
-                        };
-                        let elem_size = elem_ty
+                        let llvm_elem_ty = bin.llvm_field_ty(&elem_ty, ns);
+                        let elem_size = llvm_elem_ty
                             .size_of()
                             .unwrap()
                             .const_cast(bin.context.i32_type(), false);
-                        let len = bin.builder.build_load(len, "array_len").into_int_value();
+                        let len = bin.vector_len(arr);
                         let new_len = bin.builder.build_int_add(
                             len,
                             bin.context.i32_type().const_int(1, false),
@@ -3670,7 +3660,16 @@ pub trait TargetRuntime<'a> {
                             .builder
                             .build_call(
                                 bin.module.get_function("__realloc").unwrap(),
-                                &[a.into(), size.into()],
+                                &[
+                                    bin.builder
+                                        .build_pointer_cast(
+                                            arr.into_pointer_value(),
+                                            bin.context.i8_type().ptr_type(AddressSpace::Generic),
+                                            "a",
+                                        )
+                                        .into(),
+                                    size.into(),
+                                ],
                                 "",
                             )
                             .try_as_basic_value()
@@ -3699,11 +3698,17 @@ pub trait TargetRuntime<'a> {
                         let value = self.expression(bin, value, &w.vars, function, ns);
                         let elem_ptr = bin.builder.build_pointer_cast(
                             slot_ptr,
-                            elem_ty.ptr_type(AddressSpace::Generic),
+                            llvm_elem_ty.ptr_type(AddressSpace::Generic),
                             "element pointer",
                         );
+                        let value = if elem_ty.is_fixed_reference_type() {
+                            w.vars.get_mut(res).unwrap().value = elem_ptr.into();
+                            bin.builder.build_load(value.into_pointer_value(), "elem")
+                        } else {
+                            w.vars.get_mut(res).unwrap().value = value;
+                            value
+                        };
                         bin.builder.build_store(elem_ptr, value);
-                        w.vars.get_mut(res).unwrap().value = value;
 
                         // Update the len and size field of the vector struct
                         let len_ptr = unsafe {
@@ -3779,19 +3784,11 @@ pub trait TargetRuntime<'a> {
                         bin.builder.position_at_end(pop);
                         let llvm_ty = bin.llvm_type(ty, ns);
 
+                        let elem_ty = ty.array_elem();
+                        let llvm_elem_ty = bin.llvm_field_ty(&elem_ty, ns);
+
                         // Calculate total size for reallocation
-                        let elem_ty = match ty {
-                            Type::Array(..) => match bin.llvm_type(&ty.array_elem(), ns) {
-                                elem @ BasicTypeEnum::StructType(_) => {
-                                    // We don't store structs directly in the array, instead we store references to structs
-                                    elem.ptr_type(AddressSpace::Generic).as_basic_type_enum()
-                                }
-                                elem => elem,
-                            },
-                            Type::DynamicBytes => bin.context.i8_type().into(),
-                            _ => unreachable!(),
-                        };
-                        let elem_size = elem_ty
+                        let elem_size = llvm_elem_ty
                             .size_of()
                             .unwrap()
                             .const_cast(bin.context.i32_type(), false);
@@ -3824,11 +3821,15 @@ pub trait TargetRuntime<'a> {
                         };
                         let slot_ptr = bin.builder.build_pointer_cast(
                             slot_ptr,
-                            elem_ty.ptr_type(AddressSpace::Generic),
+                            llvm_elem_ty.ptr_type(AddressSpace::Generic),
                             "slot_ptr",
                         );
-                        let ret_val = bin.builder.build_load(slot_ptr, "");
-                        w.vars.get_mut(res).unwrap().value = ret_val;
+                        if elem_ty.is_fixed_reference_type() {
+                            w.vars.get_mut(res).unwrap().value = slot_ptr.into();
+                        } else {
+                            let ret_val = bin.builder.build_load(slot_ptr, "");
+                            w.vars.get_mut(res).unwrap().value = ret_val;
+                        }
 
                         // Reallocate and reassign the array pointer
                         let a = bin.builder.build_pointer_cast(
@@ -3939,13 +3940,13 @@ pub trait TargetRuntime<'a> {
                                 parms.push(if ns.target == Target::Solana {
                                     bin.build_alloca(
                                         function,
-                                        bin.llvm_var(&v.ty, ns),
+                                        bin.llvm_var_ty(&v.ty, ns),
                                         v.name_as_str(),
                                     )
                                     .into()
                                 } else {
                                     bin.builder
-                                        .build_alloca(bin.llvm_var(&v.ty, ns), v.name_as_str())
+                                        .build_alloca(bin.llvm_var_ty(&v.ty, ns), v.name_as_str())
                                         .into()
                                 });
                             }
@@ -4027,7 +4028,8 @@ pub trait TargetRuntime<'a> {
                         if !res.is_empty() {
                             for ty in returns.iter() {
                                 parms.push(
-                                    bin.build_alloca(function, bin.llvm_var(ty, ns), "").into(),
+                                    bin.build_alloca(function, bin.llvm_var_ty(ty, ns), "")
+                                        .into(),
                                 );
                             }
                         }
@@ -6159,7 +6161,7 @@ impl<'a> Binary<'a> {
         // function parameters
         let mut args = params
             .iter()
-            .map(|ty| self.llvm_var(ty, ns).into())
+            .map(|ty| self.llvm_var_ty(ty, ns).into())
             .collect::<Vec<BasicMetadataTypeEnum>>();
 
         // add return values
@@ -6247,7 +6249,7 @@ impl<'a> Binary<'a> {
     }
 
     /// Return the llvm type for a variable holding the type, not the type itself
-    fn llvm_var(&self, ty: &Type, ns: &Namespace) -> BasicTypeEnum<'a> {
+    fn llvm_var_ty(&self, ty: &Type, ns: &Namespace) -> BasicTypeEnum<'a> {
         let llvm_ty = self.llvm_type(ty, ns);
         match ty.deref_memory() {
             Type::Struct(_) | Type::Array(..) | Type::DynamicBytes | Type::String => {
@@ -6259,7 +6261,7 @@ impl<'a> Binary<'a> {
 
     /// Default empty value
     fn default_value(&self, ty: &Type, ns: &Namespace) -> BasicValueEnum<'a> {
-        let llvm_ty = self.llvm_var(ty, ns);
+        let llvm_ty = self.llvm_var_ty(ty, ns);
 
         // const_zero() on BasicTypeEnum yet. Should be coming to inkwell soon
         if llvm_ty.is_pointer_type() {
@@ -6271,6 +6273,20 @@ impl<'a> Binary<'a> {
         }
     }
 
+    /// Return the llvm type for field in struct or array
+    fn llvm_field_ty(&self, ty: &Type, ns: &Namespace) -> BasicTypeEnum<'a> {
+        let llvm_ty = self.llvm_type(ty, ns);
+        match ty.deref_memory() {
+            Type::Array(_, dim) if dim[0].is_none() => {
+                llvm_ty.ptr_type(AddressSpace::Generic).as_basic_type_enum()
+            }
+            Type::DynamicBytes | Type::String => {
+                llvm_ty.ptr_type(AddressSpace::Generic).as_basic_type_enum()
+            }
+            _ => llvm_ty,
+        }
+    }
+
     /// Return the llvm type for the resolved type.
     fn llvm_type(&self, ty: &Type, ns: &Namespace) -> BasicTypeEnum<'a> {
         if ty.builtin_struct(ns) == BuiltinStruct::AccountInfo {
@@ -6300,7 +6316,7 @@ impl<'a> Binary<'a> {
                     self.module.get_struct_type("struct.vector").unwrap().into()
                 }
                 Type::Array(base_ty, dims) => {
-                    let ty = self.llvm_var(base_ty, ns);
+                    let ty = self.llvm_field_ty(base_ty, ns);
 
                     let mut dims = dims.iter();
 
@@ -6328,7 +6344,7 @@ impl<'a> Binary<'a> {
                         &ns.structs[*n]
                             .fields
                             .iter()
-                            .map(|f| self.llvm_var(&f.ty, ns))
+                            .map(|f| self.llvm_field_ty(&f.ty, ns))
                             .collect::<Vec<BasicTypeEnum>>(),
                         false,
                     )
@@ -6573,7 +6589,7 @@ impl<'a> Binary<'a> {
                     }
                 } else {
                     let elem_ty = array_ty.array_deref();
-                    let llvm_elem_ty = self.llvm_var(&elem_ty, ns);
+                    let llvm_elem_ty = self.llvm_field_ty(&elem_ty, ns);
 
                     // dynamic length array or vector
                     let index = self.builder.build_int_mul(

+ 22 - 2
src/emit/solana.rs

@@ -2201,6 +2201,12 @@ impl<'a> TargetRuntime<'a> for SolanaTarget {
                         )
                     };
 
+                    let val = if field.ty.is_fixed_reference_type() {
+                        binary.builder.build_load(val.into_pointer_value(), "elem")
+                    } else {
+                        val
+                    };
+
                     binary.builder.build_store(elem, val);
                 }
 
@@ -2294,6 +2300,12 @@ impl<'a> TargetRuntime<'a> for SolanaTarget {
                     ns,
                 );
 
+                let val = if elem_ty.deref_memory().is_fixed_reference_type() {
+                    binary.builder.build_load(val.into_pointer_value(), "elem")
+                } else {
+                    val
+                };
+
                 binary.builder.build_store(elem, val);
 
                 offset_val = binary.builder.build_int_add(
@@ -2554,7 +2566,11 @@ impl<'a> TargetRuntime<'a> for SolanaTarget {
                 binary,
                 elem_ty.deref_any(),
                 &mut offset_val,
-                binary.builder.build_load(elem, "array_elem"),
+                if elem_ty.deref_memory().is_fixed_reference_type() {
+                    elem.into()
+                } else {
+                    binary.builder.build_load(elem, "array_elem")
+                },
                 function,
                 ns,
             );
@@ -2598,7 +2614,11 @@ impl<'a> TargetRuntime<'a> for SolanaTarget {
                     binary,
                     &field.ty,
                     &mut offset,
-                    binary.builder.build_load(elem, field.name_as_str()),
+                    if field.ty.is_fixed_reference_type() {
+                        elem.into()
+                    } else {
+                        binary.builder.build_load(elem, field.name_as_str())
+                    },
                     function,
                     ns,
                 );

+ 51 - 40
src/emit/substrate.rs

@@ -780,6 +780,14 @@ impl SubstrateTarget {
 
                     let val = self.decode_ty(binary, function, &field.ty, data, end, ns);
 
+                    let val = if field.ty.deref_memory().is_fixed_reference_type() {
+                        binary
+                            .builder
+                            .build_load(val.into_pointer_value(), field.name_as_str())
+                    } else {
+                        val
+                    };
+
                     binary.builder.build_store(elem, val);
                 }
 
@@ -832,6 +840,13 @@ impl SubstrateTarget {
                             };
 
                             let val = self.decode_ty(binary, function, &ty, data, end, ns);
+
+                            let val = if ty.deref_memory().is_fixed_reference_type() {
+                                binary.builder.build_load(val.into_pointer_value(), "elem")
+                            } else {
+                                val
+                            };
+
                             binary.builder.build_store(elem, val);
                         },
                     );
@@ -857,7 +872,7 @@ impl SubstrateTarget {
                     let len = binary.builder.build_load(len, "array.len").into_int_value();
 
                     // details about our array elements
-                    let elem_ty = binary.llvm_var(&ty.array_elem(), ns);
+                    let elem_ty = binary.llvm_field_ty(&ty.array_elem(), ns);
                     let elem_size = elem_ty
                         .size_of()
                         .unwrap()
@@ -910,6 +925,13 @@ impl SubstrateTarget {
                             let ty = ty.array_deref();
 
                             let val = self.decode_ty(binary, function, &ty, data, end, ns);
+
+                            let val = if ty.deref_memory().is_fixed_reference_type() {
+                                binary.builder.build_load(val.into_pointer_value(), "elem")
+                            } else {
+                                val
+                            };
+
                             binary.builder.build_store(elem, val);
                         },
                     );
@@ -1247,7 +1269,7 @@ impl SubstrateTarget {
                         self.encode_ty(
                             binary,
                             ns,
-                            true,
+                            !elem_ty.is_fixed_reference_type(),
                             packed,
                             function,
                             &elem_ty,
@@ -1274,7 +1296,14 @@ impl SubstrateTarget {
                     &mut null_data,
                     |_, elem_data| {
                         self.encode_ty(
-                            binary, ns, false, packed, function, &elem_ty, elem, elem_data,
+                            binary,
+                            ns,
+                            false,
+                            packed,
+                            function,
+                            elem_ty.deref_any(),
+                            elem,
+                            elem_data,
                         );
                     },
                 );
@@ -1317,15 +1346,7 @@ impl SubstrateTarget {
                         .into_pointer_value();
                 }
 
-                // details about our array elements
                 let elem_ty = ty.array_deref();
-                let llvm_elem_ty = binary.llvm_var(&elem_ty, ns);
-                let elem_size = llvm_elem_ty
-                    .into_pointer_type()
-                    .get_element_type()
-                    .size_of()
-                    .unwrap()
-                    .const_cast(binary.context.i32_type(), false);
 
                 binary.emit_loop_cond_first_with_pointer(
                     function,
@@ -1333,35 +1354,16 @@ impl SubstrateTarget {
                     len,
                     data,
                     |elem_no, data| {
-                        let index = binary.builder.build_int_mul(elem_no, elem_size, "");
-
-                        let element_start = unsafe {
-                            binary.builder.build_gep(
-                                arg.into_pointer_value(),
-                                &[
-                                    binary.context.i32_type().const_zero(),
-                                    binary.context.i32_type().const_int(2, false),
-                                    index,
-                                ],
-                                "data",
-                            )
-                        };
-
-                        let elem = binary.builder.build_pointer_cast(
-                            element_start,
-                            llvm_elem_ty.into_pointer_type(),
-                            "entry",
-                        );
-
-                        let ty = ty.array_deref();
+                        let elem =
+                            binary.array_subscript(ty, arg.into_pointer_value(), elem_no, ns);
 
                         self.encode_ty(
                             binary,
                             ns,
-                            true,
+                            !elem_ty.deref_any().is_fixed_reference_type(),
                             packed,
                             function,
-                            ty.deref_any(),
+                            elem_ty.deref_any(),
                             elem.into(),
                             data,
                         );
@@ -1409,7 +1411,7 @@ impl SubstrateTarget {
                     self.encode_ty(
                         binary,
                         ns,
-                        true,
+                        !field.ty.is_fixed_reference_type(),
                         packed,
                         function,
                         &field.ty,
@@ -1458,7 +1460,16 @@ impl SubstrateTarget {
                 *data = either_data.as_basic_value().into_pointer_value()
             }
             ast::Type::Ref(ty) => {
-                self.encode_ty(binary, ns, load, packed, function, ty, arg, data);
+                self.encode_ty(
+                    binary,
+                    ns,
+                    !ty.is_fixed_reference_type(),
+                    packed,
+                    function,
+                    ty,
+                    arg,
+                    data,
+                );
             }
             ast::Type::String | ast::Type::DynamicBytes => {
                 let arg = if load {
@@ -1640,7 +1651,7 @@ impl SubstrateTarget {
                         normal_sum,
                         self.encoded_length(
                             elem.into(),
-                            true,
+                            !field.ty.is_fixed_reference_type(),
                             packed,
                             &field.ty,
                             function,
@@ -1738,7 +1749,7 @@ impl SubstrateTarget {
                             *sum = binary.builder.build_int_add(
                                 self.encoded_length(
                                     elem.into(),
-                                    true,
+                                    !elem_ty.deref_memory().is_fixed_reference_type(),
                                     packed,
                                     &elem_ty,
                                     function,
@@ -1820,7 +1831,7 @@ impl SubstrateTarget {
                 let array_length = binary.vector_len(arg);
 
                 let elem_ty = ty.array_deref();
-                let llvm_elem_ty = binary.llvm_var(&elem_ty, ns);
+                let llvm_elem_ty = binary.llvm_field_ty(&elem_ty, ns);
 
                 if elem_ty.is_dynamic(ns) {
                     // if the array contains elements of dynamic length, we have to iterate over all of them
@@ -1861,7 +1872,7 @@ impl SubstrateTarget {
                             *sum = binary.builder.build_int_add(
                                 self.encoded_length(
                                     elem.into(),
-                                    true,
+                                    !elem_ty.deref_memory().is_fixed_reference_type(),
                                     packed,
                                     &elem_ty,
                                     function,

+ 70 - 54
src/sema/expression.rs

@@ -551,14 +551,24 @@ pub fn cast(
 
     // First of all, if we have a ref then derefence it
     if let Type::Ref(r) = from {
-        return cast(
-            loc,
-            Expression::Load(*loc, r.as_ref().clone(), Box::new(expr)),
-            to,
-            implicit,
-            ns,
-            diagnostics,
-        );
+        if r.is_fixed_reference_type() {
+            // Accessing a struct/fixed size array within an array/struct gives
+            // a Type::Ref(..) the element, this type is just a simple pointer,
+            // like it would have been without the Type::Ref(..)
+            //
+            // The Type::Ref(..) just means it can be used as an l-value and assigned
+            // a new value, unlike say, a struct literal.
+            return cast(loc, expr, &Type::Ref(r), implicit, ns, diagnostics);
+        } else {
+            return cast(
+                loc,
+                Expression::Load(*loc, r.as_ref().clone(), Box::new(expr)),
+                to,
+                implicit,
+                ns,
+                diagnostics,
+            );
+        }
     }
 
     // If it's a storage reference then load the value. The expr is the storage slot
@@ -1439,8 +1449,7 @@ pub fn expression(
 ) -> Result<Expression, ()> {
     match expr {
         pt::Expression::ArrayLiteral(loc, exprs) => {
-            let res =
-                resolve_array_literal(loc, exprs, context, ns, symtable, diagnostics, resolve_to);
+            let res = array_literal(loc, exprs, context, ns, symtable, diagnostics, resolve_to);
 
             if let Ok(exp) = &res {
                 used_variable(ns, exp, symtable);
@@ -4214,8 +4223,51 @@ fn member_access(
     let expr = expression(e, context, ns, symtable, diagnostics, resolve_to)?;
     let expr_ty = expr.ty();
 
-    // Dereference if need to. This could be struct-in-struct for
-    // example.
+    if let Type::Struct(struct_no) = expr_ty.deref_memory() {
+        if let Some((i, f)) = ns.structs[*struct_no]
+            .fields
+            .iter()
+            .enumerate()
+            .find(|f| id.name == f.1.name_as_str())
+        {
+            return if context.lvalue && f.readonly {
+                diagnostics.push(Diagnostic::error(
+                    id.loc,
+                    format!(
+                        "struct ‘{}’ field ‘{}’ is readonly",
+                        ns.structs[*struct_no], id.name
+                    ),
+                ));
+                Err(())
+            } else if f.readonly {
+                // readonly fields return the value, not a reference
+                Ok(Expression::StructMember(
+                    id.loc,
+                    f.ty.clone(),
+                    Box::new(expr),
+                    i,
+                ))
+            } else {
+                Ok(Expression::StructMember(
+                    id.loc,
+                    Type::Ref(Box::new(f.ty.clone())),
+                    Box::new(expr),
+                    i,
+                ))
+            };
+        } else {
+            diagnostics.push(Diagnostic::error(
+                id.loc,
+                format!(
+                    "struct ‘{}’ does not have a field called ‘{}’",
+                    ns.structs[*struct_no], id.name
+                ),
+            ));
+            return Err(());
+        }
+    }
+
+    // Dereference if need to
     let (expr, expr_ty) = if let Type::Ref(ty) = &expr_ty {
         (
             Expression::Load(*loc, expr_ty.clone(), Box::new(expr)),
@@ -4337,46 +4389,6 @@ fn member_access(
             }
             _ => {}
         },
-        Type::Struct(n) => {
-            if let Some((i, f)) = ns.structs[n]
-                .fields
-                .iter()
-                .enumerate()
-                .find(|f| id.name == f.1.name_as_str())
-            {
-                return if context.lvalue && f.readonly {
-                    diagnostics.push(Diagnostic::error(
-                        id.loc,
-                        format!("struct ‘{}’ field ‘{}’ is readonly", ns.structs[n], id.name),
-                    ));
-                    Err(())
-                } else if f.readonly {
-                    // readonly fields return the value, not a reference
-                    Ok(Expression::StructMember(
-                        id.loc,
-                        f.ty.clone(),
-                        Box::new(expr),
-                        i,
-                    ))
-                } else {
-                    Ok(Expression::StructMember(
-                        id.loc,
-                        Type::Ref(Box::new(f.ty.clone())),
-                        Box::new(expr),
-                        i,
-                    ))
-                };
-            } else {
-                diagnostics.push(Diagnostic::error(
-                    id.loc,
-                    format!(
-                        "struct ‘{}’ does not have a field called ‘{}’",
-                        ns.structs[n], id.name
-                    ),
-                ));
-                return Err(());
-            }
-        }
         Type::Address(_) => {
             if id.name == "balance" {
                 if ns.target.is_substrate() {
@@ -4673,7 +4685,11 @@ fn array_subscript(
                 let array = cast(
                     &array.loc(),
                     array,
-                    array_ty.deref_any(),
+                    if array_ty.deref_memory().is_fixed_reference_type() {
+                        &array_ty
+                    } else {
+                        array_ty.deref_any()
+                    },
                     true,
                     ns,
                     diagnostics,
@@ -6572,7 +6588,7 @@ pub fn cast_shift_arg(
 /// Given an parsed literal array, ensure that it is valid. All the elements in the array
 /// must of the same type. The array might be a multidimensional array; all the leaf nodes
 /// must match.
-fn resolve_array_literal(
+fn array_literal(
     loc: &pt::Loc,
     exprs: &[pt::Expression],
     context: &ExprContext,

+ 26 - 0
src/sema/types.rs

@@ -804,6 +804,31 @@ impl Type {
         }
     }
 
+    /// Is this a reference type of fixed size
+    pub fn is_fixed_reference_type(&self) -> bool {
+        match self {
+            Type::Bool => false,
+            Type::Address(_) => false,
+            Type::Int(_) => false,
+            Type::Uint(_) => false,
+            Type::Rational => false,
+            Type::Bytes(_) => false,
+            Type::Enum(_) => false,
+            Type::Struct(_) => true,
+            Type::Array(_, dims) => dims[0].is_some(),
+            Type::DynamicBytes => false,
+            Type::String => false,
+            Type::Mapping(..) => false,
+            Type::Contract(_) => false,
+            Type::Ref(_) => false,
+            Type::StorageRef(..) => false,
+            Type::InternalFunction { .. } => false,
+            Type::ExternalFunction { .. } => false,
+            Type::Slice => false,
+            _ => unreachable!("{:?}", self),
+        }
+    }
+
     /// Given an array, return the type of its elements
     #[must_use]
     pub fn array_elem(&self) -> Self {
@@ -812,6 +837,7 @@ impl Type {
                 Type::Array(ty.clone(), dim[..dim.len() - 1].to_vec())
             }
             Type::Array(ty, dim) if dim.len() == 1 => *ty.clone(),
+            Type::DynamicBytes => Type::Bytes(1),
             _ => panic!("not an array"),
         }
     }

+ 10 - 1
tests/codegen.rs

@@ -33,6 +33,8 @@ fn testcase(path: PathBuf) {
     let mut command_line: Option<String> = None;
     let mut checks = Vec::new();
     let mut fails = Vec::new();
+    let mut read_from = None;
+
     for line in reader.lines() {
         let mut line = line.unwrap();
         line = line.trim().parse().unwrap();
@@ -40,6 +42,8 @@ fn testcase(path: PathBuf) {
             assert_eq!(command_line, None);
 
             command_line = Some(String::from(args));
+        } else if let Some(check) = line.strip_prefix("// READ:") {
+            read_from = Some(check.trim().to_string());
         } else if let Some(check) = line.strip_prefix("// CHECK:") {
             checks.push(Test::Check(check.trim().to_string()));
         } else if let Some(fail) = line.strip_prefix("// FAIL:") {
@@ -69,7 +73,12 @@ fn testcase(path: PathBuf) {
     let mut current_check = 0;
     let mut current_fail = 0;
     let mut current_line = 0;
-    let lines: Vec<&str> = stdout.split('\n').chain(stderr.split('\n')).collect();
+    let contents = if let Some(file) = read_from {
+        fs::read_to_string(file).unwrap()
+    } else {
+        stdout.to_string()
+    };
+    let lines: Vec<&str> = contents.split('\n').chain(stderr.split('\n')).collect();
 
     while current_line < lines.len() {
         let line = lines[current_line];

+ 18 - 0
tests/codegen_testcases/array_of_struct.sol

@@ -0,0 +1,18 @@
+// RUN: --target solana --emit llvm-ir
+// READ: bundle.ll
+contract c {
+    struct S {
+        int8[2] f1;
+        bool f2;
+        Q[3] f3;
+    }
+
+    struct Q {
+        bool[2] f1;
+        uint64[4] f2;
+    }
+
+// BEGIN-CHECK: @"c::c::function::foo__c.S"({ [2 x i8], i1, [3 x { [2 x i1], [4 x i64] }] }* %0
+    function foo(S s) public pure {
+    }
+}

+ 18 - 22
tests/contract_testcases/solana/account_meta.dot

@@ -37,20 +37,18 @@ strict digraph "tests/contract_testcases/solana/account_meta.sol" {
 	list_37 [label="list\ntests/contract_testcases/solana/account_meta.sol:13:3-47"]
 	load_38 [label="load bool\ntests/contract_testcases/solana/account_meta.sol:13:3-47"]
 	structmember_39 [label="struct member #1 bool\ntests/contract_testcases/solana/account_meta.sol:13:19-30"]
-	load_40 [label="load struct AccountMeta\ntests/contract_testcases/solana/account_meta.sol:13:11-30"]
 	subscript [label="subscript struct AccountMeta[2]\ntests/contract_testcases/solana/account_meta.sol:13:11-18"]
-	variable_42 [label="variable: meta\nstruct AccountMeta[2]\ntests/contract_testcases/solana/account_meta.sol:13:11-15"]
+	variable_41 [label="variable: meta\nstruct AccountMeta[2]\ntests/contract_testcases/solana/account_meta.sol:13:11-15"]
 	number_literal [label="uint32 literal: 1\ntests/contract_testcases/solana/account_meta.sol:13:16-17"]
+	load_43 [label="load address\ntests/contract_testcases/solana/account_meta.sol:13:3-47"]
 	load_44 [label="load address\ntests/contract_testcases/solana/account_meta.sol:13:3-47"]
-	load_45 [label="load address\ntests/contract_testcases/solana/account_meta.sol:13:3-47"]
-	structmember_46 [label="struct member #0 address\ntests/contract_testcases/solana/account_meta.sol:13:40-46"]
-	load_47 [label="load struct AccountMeta\ntests/contract_testcases/solana/account_meta.sol:13:32-46"]
-	subscript_48 [label="subscript struct AccountMeta[2]\ntests/contract_testcases/solana/account_meta.sol:13:32-39"]
-	variable_49 [label="variable: meta\nstruct AccountMeta[2]\ntests/contract_testcases/solana/account_meta.sol:13:32-36"]
-	number_literal_50 [label="uint32 literal: 0\ntests/contract_testcases/solana/account_meta.sol:13:37-38"]
+	structmember_45 [label="struct member #0 address\ntests/contract_testcases/solana/account_meta.sol:13:40-46"]
+	subscript_46 [label="subscript struct AccountMeta[2]\ntests/contract_testcases/solana/account_meta.sol:13:32-39"]
+	variable_47 [label="variable: meta\nstruct AccountMeta[2]\ntests/contract_testcases/solana/account_meta.sol:13:32-36"]
+	number_literal_48 [label="uint32 literal: 0\ntests/contract_testcases/solana/account_meta.sol:13:37-38"]
 	diagnostic [label="found contract ‘spl’\nlevel Debug\ntests/contract_testcases/solana/account_meta.sol:1:1-14"]
-	diagnostic_53 [label="function can be declared ‘pure’\nlevel Warning\ntests/contract_testcases/solana/account_meta.sol:2:2-47"]
-	diagnostic_54 [label="function can be declared ‘pure’\nlevel Warning\ntests/contract_testcases/solana/account_meta.sol:7:2-56"]
+	diagnostic_51 [label="function can be declared ‘pure’\nlevel Warning\ntests/contract_testcases/solana/account_meta.sol:2:2-47"]
+	diagnostic_52 [label="function can be declared ‘pure’\nlevel Warning\ntests/contract_testcases/solana/account_meta.sol:7:2-56"]
 	contracts -> contract
 	contract -> foo [label="function"]
 	foo -> returns [label="returns"]
@@ -89,18 +87,16 @@ strict digraph "tests/contract_testcases/solana/account_meta.sol" {
 	return_36 -> list_37 [label="expr"]
 	list_37 -> load_38 [label="entry #0"]
 	load_38 -> structmember_39 [label="expr"]
-	structmember_39 -> load_40 [label="var"]
-	load_40 -> subscript [label="expr"]
-	subscript -> variable_42 [label="array"]
+	structmember_39 -> subscript [label="var"]
+	subscript -> variable_41 [label="array"]
 	subscript -> number_literal [label="index"]
-	list_37 -> load_44 [label="entry #1"]
-	load_44 -> load_45 [label="expr"]
-	load_45 -> structmember_46 [label="expr"]
-	structmember_46 -> load_47 [label="var"]
-	load_47 -> subscript_48 [label="expr"]
-	subscript_48 -> variable_49 [label="array"]
-	subscript_48 -> number_literal_50 [label="index"]
+	list_37 -> load_43 [label="entry #1"]
+	load_43 -> load_44 [label="expr"]
+	load_44 -> structmember_45 [label="expr"]
+	structmember_45 -> subscript_46 [label="var"]
+	subscript_46 -> variable_47 [label="array"]
+	subscript_46 -> number_literal_48 [label="index"]
 	diagnostics -> diagnostic [label="Debug"]
-	diagnostics -> diagnostic_53 [label="Warning"]
-	diagnostics -> diagnostic_54 [label="Warning"]
+	diagnostics -> diagnostic_51 [label="Warning"]
+	diagnostics -> diagnostic_52 [label="Warning"]
 }

+ 0 - 2
tests/substrate_tests/structs.rs

@@ -284,8 +284,6 @@ fn struct_in_struct() {
 
                 f.c.y = 300;
 
-                assert(m.y == 300);
-
                 return f.b;
             }
         }"##,