diff --git a/CHANGELOG.md b/CHANGELOG.md index eacc99f7e5..60bcf819a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1012,6 +1012,10 @@ ids.remainder.d2 = remainder_split[2] ``` +* BREAKING CHANGE: optimization for instruction decoding [#942](https://github.com/lambdaclass/cairo-rs/pull/942): + * Avoids copying immediate arguments to the `Instruction` structure, as they get inferred from the offset anyway + * Breaking: removal of the field `Instruction::imm` + * Add missing `\n` character in traceback string [#997](https://github.com/lambdaclass/cairo-rs/pull/997) * BugFix: Add missing `\n` character after traceback lines when the filename is missing ("Unknown Location") diff --git a/src/types/instruction.rs b/src/types/instruction.rs index cd74eaa35e..c8bbd822a7 100644 --- a/src/types/instruction.rs +++ b/src/types/instruction.rs @@ -15,7 +15,6 @@ pub struct Instruction { pub off0: isize, pub off1: isize, pub off2: isize, - pub imm: Option, pub dst_register: Register, pub op0_register: Register, pub op1_addr: Op1Addr, @@ -75,20 +74,20 @@ pub enum Opcode { impl Instruction { pub fn size(&self) -> usize { - match self.imm { - Some(_) => 2, - None => 1, + match self.op1_addr { + Op1Addr::Imm => 2, + _ => 1, } } } -// Returns True if the given instruction looks like a call instruction. -pub(crate) fn is_call_instruction(encoded_instruction: &Felt252, imm: Option<&Felt252>) -> bool { - let encoded_i64_instruction: i64 = match encoded_instruction.to_i64() { +// Returns True if the given instruction looks like a call instruction +pub(crate) fn is_call_instruction(encoded_instruction: &Felt252) -> bool { + let encoded_i64_instruction = match encoded_instruction.to_u64() { Some(num) => num, None => return false, }; - let instruction = match decode_instruction(encoded_i64_instruction, imm) { + let instruction = match decode_instruction(encoded_i64_instruction) { Ok(inst) => inst, Err(_) => return false, }; @@ -110,27 +109,28 @@ mod tests { #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn is_call_instruction_true() { let encoded_instruction = Felt252::new(1226245742482522112_i64); - assert!(is_call_instruction( - &encoded_instruction, - Some(&Felt252::new(2)) - )); + assert!(is_call_instruction(&encoded_instruction)); } + #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn is_call_instruction_false() { let encoded_instruction = Felt252::new(4612671187288031229_i64); - assert!(!is_call_instruction(&encoded_instruction, None)); + assert!(!is_call_instruction(&encoded_instruction)); + } + + #[test] + #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] + fn is_call_instruction_invalid() { + let encoded_instruction = Felt252::new(1u64 << 63); + assert!(!is_call_instruction(&encoded_instruction)); } #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn instruction_size() { let encoded_instruction = Felt252::new(1226245742482522112_i64); - let instruction = decode_instruction( - encoded_instruction.to_i64().unwrap(), - Some(&Felt252::new(2)), - ) - .unwrap(); + let instruction = decode_instruction(encoded_instruction.to_u64().unwrap()).unwrap(); assert_eq!(instruction.size(), 2); } } diff --git a/src/utils.rs b/src/utils.rs index 210f70840d..9b222fd2bc 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -53,6 +53,7 @@ pub fn from_relocatable_to_indexes(relocatable: Relocatable) -> (usize, usize) { pub mod test_utils { use crate::types::exec_scope::ExecutionScopes; use crate::types::relocatable::MaybeRelocatable; + use crate::vm::trace::trace_entry::TraceEntry; #[macro_export] macro_rules! bigint { @@ -354,23 +355,13 @@ pub mod test_utils { } pub(crate) use non_continuous_ids_data; - macro_rules! trace_check { - ( $trace: expr, [ $( ($off_pc:expr, $off_ap:expr, $off_fp:expr) ),+ ] ) => { - let mut index = -1; - $( - index += 1; - assert_eq!( - $trace[index as usize], - TraceEntry { - pc: $off_pc, - ap: $off_ap, - fp: $off_fp, - } - ); - )* - }; + #[track_caller] + pub(crate) fn trace_check(actual: &[TraceEntry], expected: &[(usize, usize, usize)]) { + assert_eq!(actual.len(), expected.len()); + for (entry, expected) in actual.iter().zip(expected.iter()) { + assert_eq!(&(entry.pc, entry.ap, entry.fp), expected); + } } - pub(crate) use trace_check; macro_rules! exec_scopes_ref { () => { @@ -669,7 +660,7 @@ mod test { fp: 7, }, ]; - trace_check!(trace, [(2, 7, 1), (5, 1, 0), (9, 5, 7)]); + trace_check(&trace, &[(2, 7, 1), (5, 1, 0), (9, 5, 7)]); } #[test] diff --git a/src/vm/context/run_context.rs b/src/vm/context/run_context.rs index f1cb810a99..4867a154de 100644 --- a/src/vm/context/run_context.rs +++ b/src/vm/context/run_context.rs @@ -118,7 +118,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::AP, op0_register: Register::FP, op1_addr: Op1Addr::AP, @@ -147,7 +146,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -177,7 +175,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::AP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -206,7 +203,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::FP, op1_addr: Op1Addr::AP, @@ -235,7 +231,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::FP, @@ -264,7 +259,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -293,7 +287,6 @@ mod tests { off0: 1, off1: 2, off2: 1, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::Imm, @@ -322,7 +315,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::Imm, @@ -354,7 +346,6 @@ mod tests { off0: 1, off1: 2, off2: 1, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::Op0, @@ -385,7 +376,6 @@ mod tests { off0: 1, off1: 2, off2: 1, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::Op0, @@ -418,7 +408,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::Op0, diff --git a/src/vm/decoding/decoder.rs b/src/vm/decoding/decoder.rs index 77364db19a..35a98e7812 100644 --- a/src/vm/decoding/decoder.rs +++ b/src/vm/decoding/decoder.rs @@ -4,37 +4,38 @@ use crate::{ }, vm::errors::vm_errors::VirtualMachineError, }; -use felt::Felt252; // 0| opcode|ap_update|pc_update|res_logic|op1_src|op0_reg|dst_reg // 15|14 13 12| 11 10| 9 8 7| 6 5|4 3 2| 1| 0 /// Decodes an instruction. The encoding is little endian, so flags go from bit 63 to 48. -pub fn decode_instruction( - encoded_instr: i64, - mut imm: Option<&Felt252>, -) -> Result { - const DST_REG_MASK: i64 = 0x0001; - const DST_REG_OFF: i64 = 0; - const OP0_REG_MASK: i64 = 0x0002; - const OP0_REG_OFF: i64 = 1; - const OP1_SRC_MASK: i64 = 0x001C; - const OP1_SRC_OFF: i64 = 2; - const RES_LOGIC_MASK: i64 = 0x0060; - const RES_LOGIC_OFF: i64 = 5; - const PC_UPDATE_MASK: i64 = 0x0380; - const PC_UPDATE_OFF: i64 = 7; - const AP_UPDATE_MASK: i64 = 0x0C00; - const AP_UPDATE_OFF: i64 = 10; - const OPCODE_MASK: i64 = 0x7000; - const OPCODE_OFF: i64 = 12; +pub fn decode_instruction(encoded_instr: u64) -> Result { + const HIGH_BIT: u64 = 1u64 << 63; + const DST_REG_MASK: u64 = 0x0001; + const DST_REG_OFF: u64 = 0; + const OP0_REG_MASK: u64 = 0x0002; + const OP0_REG_OFF: u64 = 1; + const OP1_SRC_MASK: u64 = 0x001C; + const OP1_SRC_OFF: u64 = 2; + const RES_LOGIC_MASK: u64 = 0x0060; + const RES_LOGIC_OFF: u64 = 5; + const PC_UPDATE_MASK: u64 = 0x0380; + const PC_UPDATE_OFF: u64 = 7; + const AP_UPDATE_MASK: u64 = 0x0C00; + const AP_UPDATE_OFF: u64 = 10; + const OPCODE_MASK: u64 = 0x7000; + const OPCODE_OFF: u64 = 12; // Flags start on the 48th bit. - const FLAGS_OFFSET: i64 = 48; - const OFF0_OFF: i64 = 0; - const OFF1_OFF: i64 = 16; - const OFF2_OFF: i64 = 32; - const OFFX_MASK: i64 = 0xFFFF; + const FLAGS_OFFSET: u64 = 48; + const OFF0_OFF: u64 = 0; + const OFF1_OFF: u64 = 16; + const OFF2_OFF: u64 = 32; + const OFFX_MASK: u64 = 0xFFFF; + + if encoded_instr & HIGH_BIT != 0 { + return Err(VirtualMachineError::InstructionNonZeroHighBit); + } // Grab offsets and convert them from little endian format. let off0 = decode_offset(encoded_instr >> OFF0_OFF & OFFX_MASK); @@ -73,14 +74,6 @@ pub fn decode_instruction( _ => return Err(VirtualMachineError::InvalidOp1Reg(op1_src_num)), }; - if op1_addr == Op1Addr::Imm { - if imm.is_none() { - return Err(VirtualMachineError::NoImm); - } - } else { - imm = None - } - let pc_update = match pc_update_num { 0 => PcUpdate::Regular, 1 => PcUpdate::Jump, @@ -123,7 +116,6 @@ pub fn decode_instruction( off0, off1, off2, - imm: imm.cloned(), dst_register, op0_register, op1_addr, @@ -135,7 +127,7 @@ pub fn decode_instruction( }) } -fn decode_offset(offset: i64) -> isize { +fn decode_offset(offset: u64) -> isize { let vectorized_offset: [u8; 8] = offset.to_le_bytes(); let offset_16b_encoded = u16::from_le_bytes([vectorized_offset[0], vectorized_offset[1]]); let complement_const = 0x8000u16; @@ -152,10 +144,20 @@ mod decoder_test { #[cfg(target_arch = "wasm32")] use wasm_bindgen_test::*; + #[test] + #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] + fn non_zero_high_bit() { + let error = decode_instruction(0x94A7800080008000); + assert_eq!( + error.unwrap_err().to_string(), + "Instruction MSB should be 0", + ) + } + #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn invalid_op1_reg() { - let error = decode_instruction(0x294F800080008000, None); + let error = decode_instruction(0x294F800080008000); assert_matches!(error, Err(VirtualMachineError::InvalidOp1Reg(3))); assert_eq!( error.unwrap_err().to_string(), @@ -166,7 +168,7 @@ mod decoder_test { #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn invalid_pc_update() { - let error = decode_instruction(0x29A8800080008000, None); + let error = decode_instruction(0x29A8800080008000); assert_matches!(error, Err(VirtualMachineError::InvalidPcUpdate(3))); assert_eq!(error.unwrap_err().to_string(), "Invalid pc_update value: 3") } @@ -174,7 +176,7 @@ mod decoder_test { #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn invalid_res_logic() { - let error = decode_instruction(0x2968800080008000, None); + let error = decode_instruction(0x2968800080008000); assert_matches!(error, Err(VirtualMachineError::InvalidRes(3))); assert_eq!(error.unwrap_err().to_string(), "Invalid res value: 3") } @@ -182,7 +184,7 @@ mod decoder_test { #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn invalid_opcode() { - let error = decode_instruction(0x3948800080008000, None); + let error = decode_instruction(0x3948800080008000); assert_matches!(error, Err(VirtualMachineError::InvalidOpcode(3))); assert_eq!(error.unwrap_err().to_string(), "Invalid opcode value: 3") } @@ -190,20 +192,11 @@ mod decoder_test { #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn invalid_ap_update() { - let error = decode_instruction(0x2D48800080008000, None); + let error = decode_instruction(0x2D48800080008000); assert_matches!(error, Err(VirtualMachineError::InvalidApUpdate(3))); assert_eq!(error.unwrap_err().to_string(), "Invalid ap_update value: 3") } - #[test] - #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] - fn decode_no_immediate_given() { - assert_matches!( - decode_instruction(0x14A7800080008000, None), - Err(VirtualMachineError::NoImm) - ); - } - #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn decode_flags_call_add_jmp_add_imm_fp_fp() { @@ -212,7 +205,7 @@ mod decoder_test { // | CALL| ADD| JUMP| ADD| IMM| FP| FP // 0 0 0 1 0 1 0 0 1 0 1 0 0 1 1 1 // 0001 0100 1010 0111 = 0x14A7; offx = 0 - let inst = decode_instruction(0x14A7800080008000, Some(&Felt252::new(7))).unwrap(); + let inst = decode_instruction(0x14A7800080008000).unwrap(); assert_matches!(inst.dst_register, Register::FP); assert_matches!(inst.op0_register, Register::FP); assert_matches!(inst.op1_addr, Op1Addr::Imm); @@ -231,7 +224,7 @@ mod decoder_test { // | RET| ADD1| JUMP_REL| MUL| FP| AP| AP // 0 0 1 0 1 0 0 1 0 1 0 0 1 0 0 0 // 0010 1001 0100 1000 = 0x2948; offx = 0 - let inst = decode_instruction(0x2948800080008000, None).unwrap(); + let inst = decode_instruction(0x2948800080008000).unwrap(); assert_matches!(inst.dst_register, Register::AP); assert_matches!(inst.op0_register, Register::AP); assert_matches!(inst.op1_addr, Op1Addr::FP); @@ -250,7 +243,7 @@ mod decoder_test { // |ASSRT_EQ| ADD| JNZ| MUL| AP| AP| AP // 0 1 0 0 1 0 1 0 0 1 0 1 0 0 0 0 // 0100 1010 0101 0000 = 0x4A50; offx = 0 - let inst = decode_instruction(0x4A50800080008000, None).unwrap(); + let inst = decode_instruction(0x4A50800080008000).unwrap(); assert_matches!(inst.dst_register, Register::AP); assert_matches!(inst.op0_register, Register::AP); assert_matches!(inst.op1_addr, Op1Addr::AP); @@ -269,7 +262,7 @@ mod decoder_test { // |ASSRT_EQ| ADD2| JNZ|UNCONSTRD| OP0| AP| AP // 0 1 0 0 0 0 1 0 0 0 0 0 0 0 0 0 // 0100 0010 0000 0000 = 0x4200; offx = 0 - let inst = decode_instruction(0x4200800080008000, None).unwrap(); + let inst = decode_instruction(0x4200800080008000).unwrap(); assert_matches!(inst.dst_register, Register::AP); assert_matches!(inst.op0_register, Register::AP); assert_matches!(inst.op1_addr, Op1Addr::Op0); @@ -288,7 +281,7 @@ mod decoder_test { // | NOP| REGULAR| REGULAR| OP1| OP0| AP| AP // 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 // 0000 0000 0000 0000 = 0x0000; offx = 0 - let inst = decode_instruction(0x0000800080008000, None).unwrap(); + let inst = decode_instruction(0x0000800080008000).unwrap(); assert_matches!(inst.dst_register, Register::AP); assert_matches!(inst.op0_register, Register::AP); assert_matches!(inst.op1_addr, Op1Addr::Op0); @@ -307,7 +300,7 @@ mod decoder_test { // | NOP| REGULAR| REGULAR| OP1| OP0| AP| AP // 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 // 0000 0000 0000 0000 = 0x0000; offx = 0 - let inst = decode_instruction(0x0000800180007FFF, None).unwrap(); + let inst = decode_instruction(0x0000800180007FFF).unwrap(); assert_eq!(inst.off0, -1); assert_eq!(inst.off1, 0); assert_eq!(inst.off2, 1); diff --git a/src/vm/errors/vm_errors.rs b/src/vm/errors/vm_errors.rs index 61fac19daf..432d1a3d1c 100644 --- a/src/vm/errors/vm_errors.rs +++ b/src/vm/errors/vm_errors.rs @@ -19,18 +19,20 @@ use felt::Felt252; #[derive(Debug, Error)] pub enum VirtualMachineError { + #[error("Instruction MSB should be 0")] + InstructionNonZeroHighBit, #[error("Instruction should be an int")] InvalidInstructionEncoding, #[error("Invalid op1_register value: {0}")] - InvalidOp1Reg(i64), + InvalidOp1Reg(u64), #[error("In immediate mode, off2 should be 1")] ImmShouldBe1, #[error("op0 must be known in double dereference")] UnknownOp0, #[error("Invalid ap_update value: {0}")] - InvalidApUpdate(i64), + InvalidApUpdate(u64), #[error("Invalid pc_update value: {0}")] - InvalidPcUpdate(i64), + InvalidPcUpdate(u64), #[error("Res.UNCONSTRAINED cannot be used with ApUpdate.ADD")] UnconstrainedResAdd, #[error("Res.UNCONSTRAINED cannot be used with PcUpdate.JUMP")] @@ -56,9 +58,9 @@ pub enum VirtualMachineError { #[error("Couldn't get or load dst")] NoDst, #[error("Invalid res value: {0}")] - InvalidRes(i64), + InvalidRes(u64), #[error("Invalid opcode value: {0}")] - InvalidOpcode(i64), + InvalidOpcode(u64), #[error("This is not implemented")] NotImplemented, #[error("Inconsistent auto-deduction for builtin {0}, expected {1}, got {2:?}")] diff --git a/src/vm/runners/cairo_runner.rs b/src/vm/runners/cairo_runner.rs index 1f031354e8..c783b3181b 100644 --- a/src/vm/runners/cairo_runner.rs +++ b/src/vm/runners/cairo_runner.rs @@ -1886,9 +1886,9 @@ mod tests { //Check each TraceEntry in trace let trace = vm.trace.unwrap(); assert_eq!(trace.len(), 5); - trace_check!( - trace, - [(3, 2, 2), (5, 3, 2), (0, 5, 5), (2, 6, 5), (7, 6, 2)] + trace_check( + &trace, + &[(3, 2, 2), (5, 3, 2), (0, 5, 5), (2, 6, 5), (7, 6, 2)], ); } @@ -1962,9 +1962,9 @@ mod tests { //Check each TraceEntry in trace let trace = vm.trace.unwrap(); assert_eq!(trace.len(), 10); - trace_check!( - trace, - [ + trace_check( + &trace, + &[ (8, 3, 3), (9, 4, 3), (11, 5, 3), @@ -1974,8 +1974,8 @@ mod tests { (4, 9, 7), (5, 9, 7), (7, 10, 7), - (13, 10, 3) - ] + (13, 10, 3), + ], ); //Check the range_check builtin segment assert_eq!(vm.builtin_runners[0].name(), RANGE_CHECK_BUILTIN_NAME); @@ -2078,9 +2078,9 @@ mod tests { //Check each TraceEntry in trace let trace = vm.trace.unwrap(); assert_eq!(trace.len(), 12); - trace_check!( - trace, - [ + trace_check( + &trace, + &[ (4, 3, 3), (5, 4, 3), (7, 5, 3), @@ -2092,8 +2092,8 @@ mod tests { (0, 11, 11), (1, 11, 11), (3, 12, 11), - (13, 12, 3) - ] + (13, 12, 3), + ], ); //Check that the output to be printed is correct assert_eq!(vm.builtin_runners[0].name(), OUTPUT_BUILTIN_NAME); @@ -2216,9 +2216,9 @@ mod tests { //Check each TraceEntry in trace let trace = vm.trace.unwrap(); assert_eq!(trace.len(), 18); - trace_check!( - trace, - [ + trace_check( + &trace, + &[ (13, 4, 4), (14, 5, 4), (16, 6, 4), @@ -2236,8 +2236,8 @@ mod tests { (1, 16, 16), (3, 17, 16), (22, 17, 4), - (23, 18, 4) - ] + (23, 18, 4), + ], ); //Check the range_check builtin segment assert_eq!(vm.builtin_runners[1].name(), RANGE_CHECK_BUILTIN_NAME); diff --git a/src/vm/trace/mod.rs b/src/vm/trace/mod.rs index c10b521ac9..504da1938f 100644 --- a/src/vm/trace/mod.rs +++ b/src/vm/trace/mod.rs @@ -2,12 +2,9 @@ use core::ops::Shl; use self::trace_entry::TraceEntry; use super::{ - decoding::decoder::decode_instruction, - errors::{memory_errors::MemoryError, vm_errors::VirtualMachineError}, + decoding::decoder::decode_instruction, errors::vm_errors::VirtualMachineError, vm_memory::memory::Memory, }; -use crate::stdlib::borrow::Cow; -use crate::types::relocatable::{MaybeRelocatable, Relocatable}; use num_traits::ToPrimitive; pub mod trace_entry; @@ -21,20 +18,11 @@ pub fn get_perm_range_check_limits( .iter() .try_fold(None, |offsets: Option<(isize, isize)>, trace| { let instruction = memory.get_integer((0, trace.pc).into())?; - let immediate = memory.get::(&(0, trace.pc + 1).into()); - let instruction = instruction - .to_i64() + .to_u64() .ok_or(VirtualMachineError::InvalidInstructionEncoding)?; - let immediate = immediate - .map(|x| match x { - Cow::Borrowed(MaybeRelocatable::Int(value)) => Ok(value.clone()), - Cow::Owned(MaybeRelocatable::Int(value)) => Ok(value), - _ => Err(MemoryError::ExpectedInteger((0, trace.pc + 1).into())), - }) - .transpose()?; - let decoded_instruction = decode_instruction(instruction, immediate.as_ref())?; + let decoded_instruction = decode_instruction(instruction)?; let off0 = decoded_instruction.off0 + 1_isize.shl(OFFSET_BITS - 1); let off1 = decoded_instruction.off1 + 1_isize.shl(OFFSET_BITS - 1); let off2 = decoded_instruction.off2 + 1_isize.shl(OFFSET_BITS - 1); diff --git a/src/vm/vm_core.rs b/src/vm/vm_core.rs index b9376bbd36..c49591b157 100644 --- a/src/vm/vm_core.rs +++ b/src/vm/vm_core.rs @@ -117,20 +117,6 @@ impl VirtualMachine { self.segments.compute_effective_sizes(); } - ///Returns the encoded instruction (the value at pc) and the immediate value (the value at pc + 1, if it exists in the memory). - fn get_instruction_encoding( - &self, - ) -> Result<(Cow, Option>), VirtualMachineError> { - let encoding_ref = match self.segments.memory.get(&self.run_context.pc) { - Some(Cow::Owned(MaybeRelocatable::Int(encoding))) => Cow::Owned(encoding), - Some(Cow::Borrowed(MaybeRelocatable::Int(encoding))) => Cow::Borrowed(encoding), - _ => return Err(VirtualMachineError::InvalidInstructionEncoding), - }; - - let imm_addr = (self.run_context.pc + 1_i32)?; - Ok((encoding_ref, self.segments.memory.get(&imm_addr))) - } - fn update_fp( &mut self, instruction: &Instruction, @@ -425,18 +411,13 @@ impl VirtualMachine { } fn decode_current_instruction(&self) -> Result { - let (instruction_ref, imm) = self.get_instruction_encoding()?; - match instruction_ref.to_i64() { - Some(instruction) => { - if let Some(MaybeRelocatable::Int(imm_ref)) = imm.as_ref().map(|x| x.as_ref()) { - let decoded_instruction = decode_instruction(instruction, Some(imm_ref))?; - return Ok(decoded_instruction); - } - let decoded_instruction = decode_instruction(instruction, None)?; - Ok(decoded_instruction) - } - None => Err(VirtualMachineError::InvalidInstructionEncoding), - } + let instruction = self + .segments + .memory + .get_integer(self.run_context.pc)? + .to_u64() + .ok_or(VirtualMachineError::InvalidInstructionEncoding)?; + decode_instruction(instruction) } pub fn step_hint( @@ -724,7 +705,7 @@ impl VirtualMachine { .map(|r| self.segments.memory.get_integer(r)) { Some(Ok(instruction1)) => { - match is_call_instruction(&instruction1, None) { + match is_call_instruction(&instruction1) { true => (ret_pc - 1).unwrap(), // This unwrap wont fail as it is checked before false => { match (ret_pc - 2) @@ -732,7 +713,7 @@ impl VirtualMachine { .map(|r| self.segments.memory.get_integer(r)) { Some(Ok(instruction0)) => { - match is_call_instruction(&instruction0, Some(&instruction1)) { + match is_call_instruction(&instruction0) { true => (ret_pc - 2).unwrap(), // This unwrap wont fail as it is checked before false => break, } @@ -1130,44 +1111,6 @@ mod tests { #[cfg(target_arch = "wasm32")] use wasm_bindgen_test::*; - #[test] - #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] - fn get_instruction_encoding_successful_without_imm() { - let mut vm = vm!(); - vm.segments = segments![((0, 0), 5)]; - assert_eq!((Felt252::new(5), None), { - let value = vm.get_instruction_encoding().unwrap(); - (value.0.into_owned(), value.1) - }); - } - - #[test] - #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] - fn get_instruction_encoding_successful_with_imm() { - let mut vm = vm!(); - - vm.segments = segments![((0, 0), 5), ((0, 1), 6)]; - - let (num, imm) = vm - .get_instruction_encoding() - .expect("Unexpected error on get_instruction_encoding"); - assert_eq!(num.as_ref(), &Felt252::new(5)); - assert_eq!( - imm.map(Cow::into_owned), - Some(MaybeRelocatable::Int(Felt252::new(6))) - ); - } - - #[test] - #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] - fn get_instruction_encoding_unsuccesful() { - let vm = vm!(); - assert_matches!( - vm.get_instruction_encoding(), - Err(VirtualMachineError::InvalidInstructionEncoding) - ); - } - #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn update_fp_ap_plus2() { @@ -1175,7 +1118,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1209,7 +1151,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1243,7 +1184,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1277,7 +1217,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1312,7 +1251,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1349,7 +1287,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1385,7 +1322,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1422,7 +1358,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1459,7 +1394,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1496,7 +1430,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1530,10 +1463,9 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: Some(Felt252::new(5)), dst_register: Register::FP, op0_register: Register::AP, - op1_addr: Op1Addr::AP, + op1_addr: Op1Addr::Imm, res: Res::Add, pc_update: PcUpdate::Regular, ap_update: ApUpdate::Regular, @@ -1564,7 +1496,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1598,7 +1529,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1634,7 +1564,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1669,7 +1598,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1702,7 +1630,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1734,7 +1661,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1768,7 +1694,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1802,7 +1727,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1841,7 +1765,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1892,7 +1815,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1921,7 +1843,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1954,7 +1875,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -1982,7 +1902,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2015,7 +1934,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2045,7 +1963,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2075,7 +1992,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2106,7 +2022,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2134,7 +2049,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2166,7 +2080,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2193,7 +2106,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2225,7 +2137,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2255,7 +2166,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2284,7 +2194,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2315,7 +2224,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2345,7 +2253,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2375,7 +2282,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2405,7 +2311,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2433,7 +2338,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2461,7 +2365,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2488,7 +2391,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2511,7 +2413,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2537,7 +2438,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2560,7 +2460,6 @@ mod tests { off0: 0, off1: 1, off2: 2, - imm: None, dst_register: Register::AP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2621,7 +2520,6 @@ mod tests { off0: 0, off1: 1, off2: 2, - imm: None, dst_register: Register::FP, op0_register: Register::FP, op1_addr: Op1Addr::FP, @@ -2681,7 +2579,6 @@ mod tests { off0: 1, off1: 1, off2: 1, - imm: Some(Felt252::new(4)), dst_register: Register::AP, op0_register: Register::AP, op1_addr: Op1Addr::Imm, @@ -2735,7 +2632,6 @@ mod tests { off0: 2, off1: 0, off2: 0, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2761,7 +2657,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2792,7 +2687,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2829,7 +2723,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2865,7 +2758,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2902,7 +2794,6 @@ mod tests { off0: 1, off1: 2, off2: 3, - imm: None, dst_register: Register::FP, op0_register: Register::AP, op1_addr: Op1Addr::AP, @@ -2972,7 +2863,7 @@ mod tests { Ok(()) ); let trace = vm.trace.unwrap(); - trace_check!(trace, [(0, 2, 2)]); + trace_check(&trace, &[(0, 2, 2)]); assert_eq!(vm.run_context.pc, Relocatable::from((3, 0))); assert_eq!(vm.run_context.ap, 2); @@ -3064,9 +2955,9 @@ mod tests { //Check each TraceEntry in trace let trace = vm.trace.unwrap(); assert_eq!(trace.len(), 5); - trace_check!( - trace, - [(3, 2, 2), (5, 3, 2), (0, 5, 5), (2, 6, 5), (7, 6, 2)] + trace_check( + &trace, + &[(3, 2, 2), (5, 3, 2), (0, 5, 5), (2, 6, 5), (7, 6, 2)], ); //Check that the following addresses have been accessed: // Addresses have been copied from python execution: @@ -3257,7 +3148,6 @@ mod tests { off0: 0, off1: -5, off2: 2, - imm: None, dst_register: Register::AP, op0_register: Register::FP, op1_addr: Op1Addr::Op0, @@ -3346,7 +3236,6 @@ mod tests { off0: 0, off1: -5, off2: 2, - imm: None, dst_register: Register::AP, op0_register: Register::FP, op1_addr: Op1Addr::Op0, @@ -3763,16 +3652,16 @@ mod tests { } //Compare trace let trace = vm.trace.unwrap(); - trace_check!( - trace, - [ + trace_check( + &trace, + &[ (3, 2, 2), (0, 4, 4), (2, 5, 4), (5, 5, 2), (7, 6, 2), - (8, 6, 2) - ] + (8, 6, 2), + ], ); //Compare final register values