From 4a3855e25d41ca26cd967112fbf080fe98f215c0 Mon Sep 17 00:00:00 2001 From: Alex Hansen Date: Fri, 5 Nov 2021 14:27:38 -0700 Subject: [PATCH] Conditionally use MCPI instead of SW for struct field writing (#371) * conditionally use MCPI instead of SW for struct field writing * fix retd struct test --- Cargo.lock | 2 +- .../src/asm_generation/expression/structs.rs | 55 ++++++++++++++++--- test/src/e2e_vm_tests/mod.rs | 5 +- .../b512_struct_alignment/Forc.toml | 9 +++ .../b512_struct_alignment/src/main.sw | 41 ++++++++++++++ 5 files changed, 102 insertions(+), 10 deletions(-) create mode 100644 test/src/e2e_vm_tests/test_programs/b512_struct_alignment/Forc.toml create mode 100644 test/src/e2e_vm_tests/test_programs/b512_struct_alignment/src/main.sw diff --git a/Cargo.lock b/Cargo.lock index 715a3182a0e..9ebd4a07979 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1339,7 +1339,7 @@ dependencies = [ [[package]] name = "fuel-vm" version = "0.1.0" -source = "git+ssh://git@github.com/FuelLabs/fuel-vm.git#409ce6241aa1a323c0ce44e8b3c7d484a55cda69" +source = "git+ssh://git@github.com/FuelLabs/fuel-vm.git#694997f472b592127417bb22efa662191aa0b162" dependencies = [ "fuel-asm", "fuel-storage", diff --git a/core_lang/src/asm_generation/expression/structs.rs b/core_lang/src/asm_generation/expression/structs.rs index aff8f3adb63..79d68d76367 100644 --- a/core_lang/src/asm_generation/expression/structs.rs +++ b/core_lang/src/asm_generation/expression/structs.rs @@ -1,7 +1,9 @@ //! This module contains the logic for struct layout in memory and instantiation. use crate::{ asm_generation::{convert_expression_to_asm, AsmNamespace, RegisterSequencer}, - asm_lang::{ConstantRegister, Op, VirtualImmediate12, VirtualImmediate24, VirtualRegister}, + asm_lang::{ + ConstantRegister, Op, VirtualImmediate12, VirtualImmediate24, VirtualOp, VirtualRegister, + }, error::*, semantic_analysis::ast_node::TypedStructExpressionField, type_engine::{look_up_type_id, resolve_type, TypeId}, @@ -219,6 +221,7 @@ pub(crate) fn convert_struct_expression_to_asm<'sc>( } // step 3 + // `offset` is in words let mut offset = 0; for TypedStructExpressionField { name, value } in fields { // evaluate the expression @@ -243,12 +246,50 @@ pub(crate) fn convert_struct_expression_to_asm<'sc>( errors ); asm_buf.append(&mut field_instantiation); - asm_buf.push(Op::write_register_to_memory( - struct_beginning_pointer.clone(), - return_register, - VirtualImmediate12::new_unchecked(offset, "the whole struct is less than 12 bits so every individual field should be as well."), - name.span.clone(), - )); + // if the value is less than one word in size, we write it via the SW opcode. + // Otherwise, use MCPI to copy the contiguous memory + let type_size = match look_up_type_id(value.return_type).stack_size_of(&name.span) { + Ok(o) => o, + Err(e) => { + errors.push(e.into()); + return err(warnings, errors); + } + }; + if type_size > 1 { + // copy the struct beginning pointer and add the offset to it + let address_to_write_to = register_sequencer.next(); + // load the address via ADDI + asm_buf.push(Op { + opcode: either::Either::Left(VirtualOp::ADDI( + address_to_write_to.clone(), + struct_beginning_pointer.clone(), + VirtualImmediate12::new_unchecked(offset * 8, "struct size is too large"), + )), + owning_span: Some(value.span.clone()), + comment: format!( + "prep struct field reg (size {} for field {})", + type_size, name.primary_name + ), + }); + + // copy the data + asm_buf.push(Op { + opcode: either::Either::Left(VirtualOp::MCPI( + address_to_write_to, + return_register, + VirtualImmediate12::new_unchecked(type_size * 8, "struct cannot be this big"), + )), + owning_span: Some(value.span.clone()), + comment: format!("cp type size {} for field {}", type_size, name.primary_name), + }); + } else { + asm_buf.push(Op::write_register_to_memory( + struct_beginning_pointer.clone(), + return_register, + VirtualImmediate12::new_unchecked(offset, "the whole struct is less than 12 bits so every individual field should be as well."), + name.span.clone(), + )); + } // TODO: if the struct needs multiple allocations, this offset could exceed the size of the // immediate value allowed in SW. In that case, we need to shift `struct_beginning_pointer` // to the max offset and start the offset back from 0. This is only for structs in excess diff --git a/test/src/e2e_vm_tests/mod.rs b/test/src/e2e_vm_tests/mod.rs index 9273cd33219..379cb4a4683 100644 --- a/test/src/e2e_vm_tests/mod.rs +++ b/test/src/e2e_vm_tests/mod.rs @@ -36,8 +36,8 @@ pub fn run(filter_regex: Option) { ( "retd_struct", ProgramState::ReturnData(Bytes32::from([ - 139, 216, 67, 1, 172, 74, 189, 183, 82, 11, 99, 241, 23, 111, 195, 89, 208, 127, - 16, 95, 247, 254, 168, 151, 227, 225, 199, 179, 50, 80, 63, 175, + 251, 57, 24, 241, 63, 94, 17, 102, 252, 182, 8, 110, 140, 105, 102, 105, 138, 202, + 155, 39, 97, 32, 94, 129, 141, 144, 190, 142, 33, 32, 33, 75, ])), ), ("op_precedence", ProgramState::Return(0)), @@ -53,6 +53,7 @@ pub fn run(filter_regex: Option) { ("const_decl", ProgramState::Return(100)), ("const_decl_in_library", ProgramState::Return(1)), // true ("aliased_imports", ProgramState::Return(42)), + ("b512_struct_alignment", ProgramState::Return(1)), // true ]; project_names.into_iter().for_each(|(name, res)| { diff --git a/test/src/e2e_vm_tests/test_programs/b512_struct_alignment/Forc.toml b/test/src/e2e_vm_tests/test_programs/b512_struct_alignment/Forc.toml new file mode 100644 index 00000000000..9f4b2125515 --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/b512_struct_alignment/Forc.toml @@ -0,0 +1,9 @@ +[project] +author = "Alexander Hansen" +license = "MIT" +name = "b512_panic_test" +entry = "main.sw" + +[dependencies] +std = { path = "../../../../../stdlib" } + diff --git a/test/src/e2e_vm_tests/test_programs/b512_struct_alignment/src/main.sw b/test/src/e2e_vm_tests/test_programs/b512_struct_alignment/src/main.sw new file mode 100644 index 00000000000..5b4ee66b137 --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/b512_struct_alignment/src/main.sw @@ -0,0 +1,41 @@ +script; + +// Stores two b256s in contiguous memory. +// Guaranteed to be contiguous for things like ECR. +struct B512 { + hi: b256, + lo: b256, +} + +// temp +pub fn build_from_b256s(hi: b256, lo: b256) -> B512 { + let hi = asm(r1: hi, rhi) { + move rhi sp; // move stack pointer to rhi + cfei i32; // extend call frame by 32 bytes to allocate more memory. now $rhi is pointing to blank, uninitialized (but allocated) memory + // addi r5 zero i32; + mcpi rhi r1 i32; + rhi: b256 + }; + + let lo = asm(r1: lo, rlo) { + move rlo sp; + cfei i32; + // now $rlo is pointing to blank memory that we can use + mcpi rlo r1 i32; + rlo: b256 + }; + + B512 { + hi: hi, + lo: lo + } +} + +fn main() -> bool { + let hi_bits: b256 = 0x7777777777777777777777777777777777777777777777777777777777777777; + let lo_bits: b256 = 0x5555555555555555555555555555555555555555555555555555555555555555; + + let b: B512 = build_from_b256s(hi_bits, lo_bits); + + b.lo == lo_bits && b.hi == hi_bits +}