From ae7b3c9559ea9e5e60477552909eb62af17417b5 Mon Sep 17 00:00:00 2001 From: Neelay Sant Date: Thu, 6 Mar 2025 12:22:45 +0000 Subject: [PATCH 1/2] RISC-V: Adding test framework in JIT --- src/riscv/lib/src/jit.rs | 728 ++++++++++++++++----------------------- 1 file changed, 292 insertions(+), 436 deletions(-) diff --git a/src/riscv/lib/src/jit.rs b/src/riscv/lib/src/jit.rs index 9add82453e64..333d48894cd5 100644 --- a/src/riscv/lib/src/jit.rs +++ b/src/riscv/lib/src/jit.rs @@ -272,6 +272,8 @@ impl Default for JIT { #[cfg(test)] mod tests { + use Instruction as I; + use super::*; use crate::backend_test; use crate::create_state; @@ -284,10 +286,10 @@ mod tests { use crate::machine_state::block_cache::bcall::InterpretedBlockBuilder; use crate::machine_state::memory::M4K; use crate::machine_state::memory::MemoryConfig; - use crate::parser::instruction::InstrWidth; use crate::parser::instruction::InstrWidth::*; use crate::state_backend::FnManagerIdent; use crate::state_backend::ManagerRead; + use crate::state_backend::test_helpers::TestBackendFactory; use crate::state_backend::test_helpers::assert_eq_struct; fn instructions(block: &Interpreted) -> Vec @@ -298,550 +300,404 @@ mod tests { instr.iter().map(|cell| cell.read_stored()).collect() } - // Simplified variant of the `Cached` structure in the block cache. - backend_test!(test_cnop, F, { - use Instruction as I; + type SetupHook = dyn Fn(&mut MachineCoreState::Manager>); + type AssertHook = dyn Fn(&MachineCoreState::Manager>); - // Arrange - let scenarios: &[&[I]] = &[ - &[I::new_nop(Compressed)], - &[I::new_nop(Compressed), I::new_nop(Uncompressed)], - &[ - I::new_nop(Uncompressed), - I::new_nop(Compressed), - I::new_nop(Uncompressed), - ], - ]; + struct Scenario<'a, F: TestBackendFactory> { + initial_pc: Option, + expected_steps: Option, + instructions: Vec, + setup_hook: &'a SetupHook, + assert_hook: &'a AssertHook, + } - let mut jit = JIT::::new().unwrap(); - let mut interpreted_bb = InterpretedBlockBuilder; + impl<'a, F: TestBackendFactory> Scenario<'a, F> { + fn simple(instructions: &[Instruction]) -> Self { + Scenario { + initial_pc: None, + expected_steps: None, + instructions: instructions.to_vec(), + setup_hook: &|_| {}, + assert_hook: &|_| {}, + } + } - for scenario in scenarios { + /// Run a test scenario over both the Interpreted & JIT modes of compilation, + /// to ensure they behave identically. + fn run( + &self, + jit: &mut JIT, + interpreted_bb: &mut InterpretedBlockBuilder, + ) { + // Create the states for the interpreted and jitted runs. let mut interpreted = create_state!(MachineCoreState, MachineCoreStateLayout, F, M4K); let mut jitted = create_state!(MachineCoreState, MachineCoreStateLayout, F, M4K); - let mut block = create_state!(Interpreted, BlockLayout, F, M4K); - let hash = super::Hash::blake2b_hash(scenario).unwrap(); + let hash = super::Hash::blake2b_hash(&self.instructions).unwrap(); + // Create the block of instructions. + let mut block = create_state!(Interpreted, BlockLayout, F, M4K); block.start_block(); - for instr in scenario.iter() { + for instr in self.instructions.iter() { block.push_instr(*instr); } + // Run the setup hooks. + (self.setup_hook)(&mut interpreted); + (self.setup_hook)(&mut jitted); + + // initialise starting parameters: pc, steps + let initial_pc = self.initial_pc.unwrap_or_default(); let mut interpreted_steps = 0; let mut jitted_steps = 0; - - let initial_pc = 0; interpreted.hart.pc.write(initial_pc); jitted.hart.pc.write(initial_pc); - // Act + // Create the JIT function. let fun = jit .compile(&hash, instructions(&block).as_slice()) - .expect("Compilation of CNop should succeed"); + .expect("Compilation of block should succeed."); + // Run the block in both interpreted and jitted mode. let interpreted_res = unsafe { // SAFETY: interpreted blocks are always callable - block.callable(&mut interpreted_bb) + block.callable(interpreted_bb) } .unwrap() .run_block(&mut interpreted, initial_pc, &mut interpreted_steps); let jitted_res = unsafe { - // # Safety - the jit is not dropped until after we - // exit the for loop + // # Safety - the block builder is alive for at least + // the duration of the `run` function. fun.call(&mut jitted, initial_pc, &mut jitted_steps) }; - // Assert + // Assert state equality. assert_eq!(jitted_res, interpreted_res); assert_eq!( interpreted_steps, jitted_steps, "Interpreted mode ran for {interpreted_steps}, compared to jit-mode of {jitted_steps}" ); - - assert_eq!(interpreted_steps, scenario.len()); - assert_eq_struct( &interpreted.struct_ref::(), &jitted.struct_ref::(), ); - } - }); - backend_test!(test_cmv, F, { - use Instruction as I; - - use crate::machine_state::registers::NonZeroXRegister::*; - - // Arrange - let scenarios: &[&[I]] = &[ - &[I::new_li(x1, 1, Compressed), I::new_mv(x2, x1, Compressed)], - &[ - I::new_li(x1, 1, Uncompressed), - I::new_mv(x2, x1, Uncompressed), - ], - &[ - I::new_li(x1, 1, Uncompressed), - I::new_mv(x2, x1, Compressed), - I::new_mv(x3, x2, Uncompressed), - ], - ]; - - let mut jit = JIT::::new().unwrap(); - let mut interpreted_bb = InterpretedBlockBuilder; + // Only check steps against one state, as we know both interpreted/jit steps are equal. + let expected_steps = self.expected_steps.unwrap_or(self.instructions.len()); + assert_eq!( + interpreted_steps, expected_steps, + "Scenario ran for {interpreted_steps} steps, but expected {expected_steps}" + ); - for scenario in scenarios { - let mut interpreted = - create_state!(MachineCoreState, MachineCoreStateLayout, F, M4K); - let mut jitted = create_state!(MachineCoreState, MachineCoreStateLayout, F, M4K); - let mut block = create_state!(Interpreted, BlockLayout, F, M4K); + // Run the assert hooks. Since we have already verified that the states are equal, + // we can run the assert hooks on just the interpreted state. + (self.assert_hook)(&interpreted); + } + } - let hash = super::Hash::blake2b_hash(scenario).unwrap(); + /// A builder for creating scenarios. + struct ScenarioBuilder<'a, F: TestBackendFactory> { + initial_pc: Option, + expected_steps: Option, + instructions: Vec, + setup_hook: &'a SetupHook, + assert_hook: &'a AssertHook, + } - block.start_block(); - for instr in scenario.iter() { - block.push_instr(*instr); + impl<'a, F: TestBackendFactory> Default for ScenarioBuilder<'a, F> { + fn default() -> Self { + ScenarioBuilder { + initial_pc: None, + expected_steps: None, + instructions: Vec::new(), + setup_hook: &|_| {}, + assert_hook: &|_| {}, } + } + } - let mut interpreted_steps = 0; - let mut jitted_steps = 0; + impl<'a, F: TestBackendFactory> ScenarioBuilder<'a, F> { + fn set_instructions(mut self, instructions: &[Instruction]) -> Self { + self.instructions = instructions.to_vec(); + self + } - let initial_pc = 0; - interpreted.hart.pc.write(initial_pc); - jitted.hart.pc.write(initial_pc); + fn set_initial_pc(mut self, initial_pc: u64) -> Self { + self.initial_pc = Some(initial_pc); + self + } - // Act - let fun = jit - .compile(&hash, instructions(&block).as_slice()) - .expect("Compilation should succeed"); + fn set_expected_steps(mut self, expected_steps: usize) -> Self { + self.expected_steps = Some(expected_steps); + self + } - let interpreted_res = unsafe { - // SAFETY: interpreted blocks are always callable - block.callable(&mut interpreted_bb) - } - .unwrap() - .run_block(&mut interpreted, initial_pc, &mut interpreted_steps); - let jitted_res = unsafe { - // # Safety - the jit is not dropped until after we - // exit the for loop - fun.call(&mut jitted, initial_pc, &mut jitted_steps) - }; + fn set_assert_hook(mut self, assert_hook: &'a AssertHook) -> Self { + self.assert_hook = assert_hook; + self + } - // Assert - assert_eq!(jitted_res, interpreted_res); - assert_eq!( - interpreted_steps, jitted_steps, - "Interpreted mode ran for {interpreted_steps}, compared to jit-mode of {jitted_steps}" - ); + fn build(self) -> Scenario<'a, F> { + Scenario { + initial_pc: self.initial_pc, + expected_steps: self.expected_steps, + instructions: self.instructions, + setup_hook: self.setup_hook, + assert_hook: self.assert_hook, + } + } + } - assert_eq!(interpreted_steps, scenario.len()); + backend_test!(test_cnop, F, { + let scenarios: &[Scenario<'_, F>] = &[ + Scenario::simple(&[I::new_nop(Compressed)]), + Scenario::simple(&[I::new_nop(Compressed), I::new_nop(Uncompressed)]), + Scenario::simple(&[ + I::new_nop(Uncompressed), + I::new_nop(Compressed), + I::new_nop(Uncompressed), + ]), + ]; - // every scenario expects x2 to be 1 after - assert_eq!(interpreted.hart.xregisters.read_nz(x2), 1); - assert_eq!(jitted.hart.xregisters.read_nz(x2), 1); + let mut jit = JIT::::new().unwrap(); + let mut interpreted_bb = InterpretedBlockBuilder; - assert_eq_struct( - &interpreted.struct_ref::(), - &jitted.struct_ref::(), - ); + for scenario in scenarios { + scenario.run(&mut jit, &mut interpreted_bb); } }); - backend_test!(test_add, F, { - use Instruction as I; - + backend_test!(test_cmv, F, { use crate::machine_state::registers::NonZeroXRegister::*; - // Arrange + let assert_x2_is_one = |core: &MachineCoreState| { + assert_eq!(core.hart.xregisters.read_nz(x2), 1); + }; - // calculate fibonacci(4) == 5 - let scenario: &[I] = &[ - I::new_li(x1, 1, Uncompressed), - I::new_add(x2, x2, x1, Compressed), - I::new_add(x1, x1, x2, Uncompressed), - I::new_add(x2, x2, x1, Uncompressed), - I::new_add(x1, x1, x2, Compressed), + // Arrange + let scenarios: &[Scenario<'_, F>] = &[ + ScenarioBuilder::default() + .set_instructions(&[I::new_li(x1, 1, Compressed), I::new_mv(x2, x1, Compressed)]) + .set_assert_hook(&assert_x2_is_one) + .build(), + ScenarioBuilder::default() + .set_instructions(&[ + I::new_li(x1, 1, Uncompressed), + I::new_mv(x2, x1, Uncompressed), + ]) + .set_assert_hook(&assert_x2_is_one) + .build(), + ScenarioBuilder::default() + .set_instructions(&[ + I::new_li(x1, 1, Compressed), + I::new_mv(x2, x1, Compressed), + I::new_mv(x3, x2, Compressed), + ]) + .set_assert_hook(&assert_x2_is_one) + .build(), ]; let mut jit = JIT::::new().unwrap(); let mut interpreted_bb = InterpretedBlockBuilder; - let mut interpreted = create_state!(MachineCoreState, MachineCoreStateLayout, F, M4K); - let mut jitted = create_state!(MachineCoreState, MachineCoreStateLayout, F, M4K); - let mut block = create_state!(Interpreted, BlockLayout, F, M4K); - - let hash = super::Hash::blake2b_hash(scenario).unwrap(); - - block.start_block(); - for instr in scenario.iter() { - block.push_instr(*instr); + for scenario in scenarios { + scenario.run(&mut jit, &mut interpreted_bb); } + }); - let mut interpreted_steps = 0; - let mut jitted_steps = 0; - - let initial_pc = 0; - interpreted.hart.pc.write(initial_pc); - jitted.hart.pc.write(initial_pc); - - // Act - let fun = jit - .compile(&hash, instructions(&block).as_slice()) - .expect("Compilation should succeed"); + backend_test!(test_add, F, { + use crate::machine_state::registers::NonZeroXRegister::*; - let interpreted_res = unsafe { - // SAFETY: interpreted blocks are always callable - block.callable(&mut interpreted_bb) - } - .unwrap() - .run_block(&mut interpreted, initial_pc, &mut interpreted_steps); - let jitted_res = unsafe { - // # Safety - the jit is not dropped until after we - // exit the block - fun.call(&mut jitted, initial_pc, &mut jitted_steps) + let assert_x1_is_five = |core: &MachineCoreState| { + assert_eq!(core.hart.xregisters.read_nz(x1), 5); }; - // Assert - assert_eq!(jitted_res, interpreted_res); - assert_eq!( - interpreted_steps, jitted_steps, - "Interpreted mode ran for {interpreted_steps}, compared to jit-mode of {jitted_steps}" - ); - - assert_eq!(interpreted_steps, scenario.len()); + let scenario: Scenario<'_, F> = ScenarioBuilder::default() + .set_instructions(&[ + I::new_li(x1, 1, Uncompressed), + I::new_add(x2, x2, x1, Compressed), + I::new_add(x1, x1, x2, Uncompressed), + I::new_add(x2, x2, x1, Uncompressed), + I::new_add(x1, x1, x2, Compressed), + ]) + .set_assert_hook(&assert_x1_is_five) + .build(); - // have got to the fibonacci number 5 - assert_eq!(interpreted.hart.xregisters.read_nz(x1), 5); - assert_eq!(jitted.hart.xregisters.read_nz(x1), 5); + let mut jit = JIT::::new().unwrap(); + let mut interpreted_bb = InterpretedBlockBuilder; - assert_eq_struct( - &interpreted.struct_ref::(), - &jitted.struct_ref::(), - ); + scenario.run(&mut jit, &mut interpreted_bb); }); backend_test!(test_and, F, { - use Instruction as I; - use crate::machine_state::registers::NonZeroXRegister::*; - // Arrange - let scenarios: &[&[I]] = &[ - // Bitwise and with all ones is self. - &[ - I::new_li(x1, 13872, Uncompressed), - I::new_li(x3, !0, Compressed), - I::new_and(x2, x1, x3, Compressed), - ], - // Bitwise and with itself is self. - &[ - I::new_li(x1, 49666, Uncompressed), - I::new_and(x2, x1, x1, Compressed), - ], - // Bitwise and with 0 is 0. - &[ - I::new_li(x1, 0, Uncompressed), - I::new_li(x3, 540921, Compressed), - I::new_and(x2, x1, x3, Compressed), - ], + let assert_x1_and_x2_equal = |core: &MachineCoreState| { + assert_eq!( + core.hart.xregisters.read_nz(x1), + core.hart.xregisters.read_nz(x2) + ); + }; + + let scenarios: &[Scenario<'_, F>] = &[ + ScenarioBuilder::default() + // Bitwise and with all ones is self. + .set_instructions(&[ + I::new_li(x1, 13872, Uncompressed), + I::new_li(x3, !0, Compressed), + I::new_and(x2, x1, x3, Compressed), + ]) + .set_assert_hook(&assert_x1_and_x2_equal) + .build(), + ScenarioBuilder::default() + // Bitwise and with itself is self. + .set_instructions(&[ + I::new_li(x1, 49666, Uncompressed), + I::new_and(x2, x1, x1, Compressed), + ]) + .set_assert_hook(&assert_x1_and_x2_equal) + .build(), + ScenarioBuilder::default() + // Bitwise and with 0 is 0. + .set_instructions(&[ + I::new_li(x1, 0, Uncompressed), + I::new_li(x3, 540921, Compressed), + I::new_and(x2, x1, x3, Compressed), + ]) + .set_assert_hook(&assert_x1_and_x2_equal) + .build(), ]; let mut jit = JIT::::new().unwrap(); let mut interpreted_bb = InterpretedBlockBuilder; for scenario in scenarios { - let mut interpreted = - create_state!(MachineCoreState, MachineCoreStateLayout, F, M4K); - let mut jitted = create_state!(MachineCoreState, MachineCoreStateLayout, F, M4K); - let mut block = create_state!(Interpreted, BlockLayout, F, M4K); - let hash = super::Hash::blake2b_hash(scenario).unwrap(); - - block.start_block(); - for instr in scenario.iter() { - block.push_instr(*instr); - } - - let mut interpreted_steps = 0; - let mut jitted_steps = 0; - - let initial_pc = 0; - interpreted.hart.pc.write(initial_pc); - jitted.hart.pc.write(initial_pc); - - // Act - let fun = jit - .compile(&hash, instructions(&block).as_slice()) - .expect("Compilation should succeed"); - - let interpreted_res = unsafe { - // SAFETY: interpreted blocks are always callable - block.callable(&mut interpreted_bb) - } - .unwrap() - .run_block(&mut interpreted, initial_pc, &mut interpreted_steps); - let jitted_res = unsafe { - // # Safety - the jit is not dropped until after we - // exit the for loop - fun.call(&mut jitted, initial_pc, &mut jitted_steps) - }; - - // Assert - assert_eq!(jitted_res, interpreted_res); - assert_eq!( - interpreted_steps, jitted_steps, - "Interpreted mode ran for {interpreted_steps}, compared to jit-mode of {jitted_steps}" - ); - - assert_eq!(interpreted_steps, scenario.len()); - - // every scenario expects x1 and x2 to be equal. - assert_eq!( - interpreted.hart.xregisters.read_nz(x1), - interpreted.hart.xregisters.read_nz(x2) - ); - assert_eq!( - jitted.hart.xregisters.read_nz(x1), - jitted.hart.xregisters.read_nz(x2) - ); - - assert_eq_struct( - &interpreted.struct_ref::(), - &jitted.struct_ref::(), - ); + scenario.run(&mut jit, &mut interpreted_bb); } }); backend_test!(test_or, F, { - use Instruction as I; - use crate::machine_state::registers::NonZeroXRegister::*; - // Arrange - let scenarios: &[&[I]] = &[ + let assert_x1_and_x2_equal = |core: &MachineCoreState| { + assert_eq!( + core.hart.xregisters.read_nz(x1), + core.hart.xregisters.read_nz(x2) + ); + }; + + let scenarios: &[Scenario<'_, F>] = &[ // Bitwise or with all ones is all-ones. - &[ - I::new_li(x1, !0, Uncompressed), - I::new_li(x3, 13872, Compressed), - I::new_or(x2, x1, x3, Compressed), - ], + ScenarioBuilder::default() + .set_instructions(&[ + I::new_li(x1, !0, Uncompressed), + I::new_li(x3, 13872, Compressed), + I::new_or(x2, x1, x3, Compressed), + ]) + .set_assert_hook(&assert_x1_and_x2_equal) + .build(), // Bitwise or with itself is self. - &[ - I::new_li(x1, 49666, Uncompressed), - I::new_or(x2, x1, x1, Compressed), - ], + ScenarioBuilder::default() + .set_instructions(&[ + I::new_li(x1, 49666, Uncompressed), + I::new_or(x2, x1, x1, Compressed), + ]) + .set_assert_hook(&assert_x1_and_x2_equal) + .build(), // Bitwise or with 0 is self. - &[ - I::new_li(x1, 540921, Uncompressed), - I::new_li(x3, 0, Compressed), - I::new_or(x2, x1, x3, Compressed), - ], + ScenarioBuilder::default() + .set_instructions(&[ + I::new_li(x1, 540921, Uncompressed), + I::new_li(x3, 0, Compressed), + I::new_or(x2, x1, x3, Compressed), + ]) + .set_assert_hook(&assert_x1_and_x2_equal) + .build(), ]; let mut jit = JIT::::new().unwrap(); let mut interpreted_bb = InterpretedBlockBuilder; for scenario in scenarios { - let mut interpreted = - create_state!(MachineCoreState, MachineCoreStateLayout, F, M4K); - let mut jitted = create_state!(MachineCoreState, MachineCoreStateLayout, F, M4K); - let mut block = create_state!(Interpreted, BlockLayout, F, M4K); - let hash = super::Hash::blake2b_hash(scenario).unwrap(); - - block.start_block(); - for instr in scenario.iter() { - block.push_instr(*instr); - } - - let mut interpreted_steps = 0; - let mut jitted_steps = 0; - - let initial_pc = 0; - interpreted.hart.pc.write(initial_pc); - jitted.hart.pc.write(initial_pc); - - // Act - let fun = jit - .compile(&hash, instructions(&block).as_slice()) - .expect("Compilation should succeed"); - - let interpreted_res = unsafe { - // SAFETY: interpreted blocks are always callable - block.callable(&mut interpreted_bb) - } - .unwrap() - .run_block(&mut interpreted, initial_pc, &mut interpreted_steps); - let jitted_res = unsafe { - // # Safety - the jit is not dropped until after we - // exit the for loop - fun.call(&mut jitted, initial_pc, &mut jitted_steps) - }; - - // Assert - assert_eq!(jitted_res, interpreted_res); - assert_eq!( - interpreted_steps, jitted_steps, - "Interpreted mode ran for {interpreted_steps}, compared to jit-mode of {jitted_steps}" - ); - - assert_eq!(interpreted_steps, scenario.len()); - - // every scenario expects x1 and x2 to be equal. - assert_eq!( - interpreted.hart.xregisters.read_nz(x1), - interpreted.hart.xregisters.read_nz(x2) - ); - assert_eq!( - jitted.hart.xregisters.read_nz(x1), - jitted.hart.xregisters.read_nz(x2) - ); - - assert_eq_struct( - &interpreted.struct_ref::(), - &jitted.struct_ref::(), - ); + scenario.run(&mut jit, &mut interpreted_bb); } }); - struct Scenario { - initial_pc: u64, - expected_pc: u64, - instructions: Vec, - } - backend_test!(test_j, F, { - use Instruction as I; - - use crate::machine_state::registers::NonZeroXRegister::*; - - // recreate scenarios array but as a vec of scenario structs - let scenarios: Vec = vec![ - // Jumping to the next instruction should exit the block - Scenario { - initial_pc: 0, - expected_pc: 6, - instructions: vec![ - I::new_nop(InstrWidth::Compressed), - I::new_nop(InstrWidth::Compressed), - I::new_j(2, InstrWidth::Compressed), - ], - }, - // Jump past 0 - in both worlds we should wrap around. - Scenario { - initial_pc: 0, - expected_pc: u64::MAX - 3, - instructions: vec![ - I::new_nop(InstrWidth::Compressed), - I::new_nop(InstrWidth::Compressed), - I::new_j(-8, InstrWidth::Compressed), - ], - }, - // Jump past u64::MAX - in both worlds we should wrap around. - Scenario { - initial_pc: (i64::MAX - 5) as u64, - expected_pc: 1, - instructions: vec![ - I::new_nop(InstrWidth::Uncompressed), - I::new_nop(InstrWidth::Uncompressed), - I::new_j(i64::MAX, InstrWidth::Uncompressed), - ], - }, - // jump by nothing - Scenario { - initial_pc: 0, - expected_pc: 4, - instructions: vec![ - I::new_nop(InstrWidth::Compressed), - I::new_nop(InstrWidth::Compressed), - I::new_j(0, InstrWidth::Compressed), - ], - }, - // jumping to start of the block should exit the block in both interpreted and jitted world - Scenario { - initial_pc: 0, - expected_pc: 0, - instructions: vec![ - I::new_nop(InstrWidth::Compressed), - I::new_nop(InstrWidth::Compressed), - I::new_j(-4, InstrWidth::Compressed), - ], - }, + let scenarios: &[Scenario<'_, F>] = &[ + ScenarioBuilder::default() + // Jumping to the next instruction should exit the block + .set_instructions(&[ + I::new_nop(Compressed), + I::new_nop(Compressed), + I::new_j(2, Compressed), + ]) + .set_assert_hook(&|core: &MachineCoreState| { + assert_eq!(core.hart.pc.read(), 6); + }) + .set_expected_steps(3) + .build(), + ScenarioBuilder::default() + // Jump past 0 - in both worlds we should wrap around. + .set_instructions(&[I::new_j(-4, Compressed)]) + .set_assert_hook(&|core: &MachineCoreState| { + assert_eq!(core.hart.pc.read(), u64::MAX - 3); + }) + .set_expected_steps(1) + .build(), + ScenarioBuilder::default() + // Jump past u64::MAX - in both worlds we should wrap around but not + // execute functions past the end of the block (the jump). + .set_instructions(&[ + I::new_nop(Uncompressed), + I::new_nop(Uncompressed), + I::new_j(i64::MAX, Uncompressed), + I::new_nop(Compressed), + I::new_nop(Uncompressed), + ]) + .set_initial_pc((i64::MAX - 5) as u64) + .set_assert_hook(&|core: &MachineCoreState| { + assert_eq!(core.hart.pc.read(), 1); + }) + .set_expected_steps(3) + .build(), + ScenarioBuilder::default() + // jump by nothing + .set_instructions(&[ + I::new_nop(Compressed), + I::new_j(0, Compressed), + I::new_nop(Uncompressed), + ]) + .set_assert_hook(&|core: &MachineCoreState| { + assert_eq!(core.hart.pc.read(), 2); + }) + .set_expected_steps(2) + .build(), + ScenarioBuilder::default() + // jumping to start of the block should exit the block in both interpreted and jitted world + .set_instructions(&[ + I::new_nop(Compressed), + I::new_nop(Compressed), + I::new_j(-4, Compressed), + I::new_nop(Uncompressed), + ]) + .set_assert_hook(&|core: &MachineCoreState| { + assert_eq!(core.hart.pc.read(), 0); + }) + .set_expected_steps(3) + .build(), ]; let mut jit = JIT::::new().unwrap(); let mut interpreted_bb = InterpretedBlockBuilder; for scenario in scenarios { - let mut interpreted = - create_state!(MachineCoreState, MachineCoreStateLayout, F, M4K); - let mut jitted = create_state!(MachineCoreState, MachineCoreStateLayout, F, M4K); - let mut block = create_state!(Interpreted, BlockLayout, F, M4K); - let hash = super::Hash::blake2b_hash(&scenario.instructions).unwrap(); - - block.start_block(); - for instr in scenario.instructions.iter() { - block.push_instr(*instr); - } - - let mut interpreted_steps = 0; - let mut jitted_steps = 0; - - interpreted.hart.pc.write(scenario.initial_pc); - jitted.hart.pc.write(scenario.initial_pc); - - // Act - let fun = jit - .compile(&hash, instructions(&block).as_slice()) - .expect("Compilation should succeed"); - - let interpreted_res = unsafe { - // SAFETY: interpreted blocks are always callable - block.callable(&mut interpreted_bb) - } - .unwrap() - .run_block( - &mut interpreted, - scenario.initial_pc, - &mut interpreted_steps, - ); - let jitted_res = unsafe { - // # Safety - the jit is not dropped until after we - // exit the for loop - fun.call(&mut jitted, scenario.initial_pc, &mut jitted_steps) - }; - - // Assert - assert_eq!(jitted_res, interpreted_res); - assert_eq!( - interpreted_steps, jitted_steps, - "Interpreted mode ran for {interpreted_steps}, compared to jit-mode of {jitted_steps}" - ); - - assert_eq!(interpreted_steps, scenario.instructions.len()); - - // every scenario expects x1 and x2 to be equal. - assert_eq!( - interpreted.hart.xregisters.read_nz(x1), - interpreted.hart.xregisters.read_nz(x2) - ); - assert_eq!( - jitted.hart.xregisters.read_nz(x1), - jitted.hart.xregisters.read_nz(x2) - ); - - assert_eq!(interpreted.hart.pc.read(), scenario.expected_pc); - assert_eq!(jitted.hart.pc.read(), scenario.expected_pc); - - assert_eq_struct( - &interpreted.struct_ref::(), - &jitted.struct_ref::(), - ); + scenario.run(&mut jit, &mut interpreted_bb); } }); backend_test!(test_jit_recovers_from_compilation_failure, F, { - use Instruction as I; - use crate::machine_state::registers::NonZeroXRegister::*; // Arrange -- GitLab From 125df0579d105a29fa92eb472894ef88c6938bf0 Mon Sep 17 00:00:00 2001 From: Neelay Sant Date: Wed, 5 Mar 2025 12:57:31 +0000 Subject: [PATCH 2/2] RISC-V: Organise existing interpreter run files --- src/riscv/lib/src/interpreter.rs | 4 +- src/riscv/lib/src/interpreter/branching.rs | 3 +- src/riscv/lib/src/interpreter/c.rs | 78 ------------------ src/riscv/lib/src/interpreter/i.rs | 59 -------------- src/riscv/lib/src/interpreter/integer.rs | 79 ++++++++++++++++++- src/riscv/lib/src/interpreter/load_store.rs | 6 ++ src/riscv/lib/src/interpreter/rv32i.rs | 2 +- .../lib/src/machine_state/instruction.rs | 8 +- 8 files changed, 91 insertions(+), 148 deletions(-) delete mode 100644 src/riscv/lib/src/interpreter/c.rs delete mode 100644 src/riscv/lib/src/interpreter/i.rs diff --git a/src/riscv/lib/src/interpreter.rs b/src/riscv/lib/src/interpreter.rs index b15e2f3c19da..a4ed29936189 100644 --- a/src/riscv/lib/src/interpreter.rs +++ b/src/riscv/lib/src/interpreter.rs @@ -1,14 +1,12 @@ -// SPDX-FileCopyrightText: 2023-2024 TriliTech +// SPDX-FileCopyrightText: 2023-2025 TriliTech // SPDX-FileCopyrightText: 2024 Nomadic Labs // // SPDX-License-Identifier: MIT mod atomics; pub mod branching; -pub mod c; mod common_memory; pub mod float; -pub mod i; pub mod integer; pub mod load_store; pub mod rv32a; diff --git a/src/riscv/lib/src/interpreter/branching.rs b/src/riscv/lib/src/interpreter/branching.rs index a7834d7dd795..28e0dafc098b 100644 --- a/src/riscv/lib/src/interpreter/branching.rs +++ b/src/riscv/lib/src/interpreter/branching.rs @@ -2,7 +2,8 @@ // // SPDX-License-Identifier: MIT -//! Implementation of internal branching opcodes. +//! Implementation of branching and jumping instructions for RISC-V over the ICB. +// TODO: RV-520: Update remaining 'jump' handlers in the file to work over the ICB. use crate::instruction_context::ICB; use crate::machine_state::hart_state::HartState; diff --git a/src/riscv/lib/src/interpreter/c.rs b/src/riscv/lib/src/interpreter/c.rs deleted file mode 100644 index ceee0bbfdac2..000000000000 --- a/src/riscv/lib/src/interpreter/c.rs +++ /dev/null @@ -1,78 +0,0 @@ -// SPDX-FileCopyrightText: 2025 TriliTech -// -// SPDX-License-Identifier: MIT - -//! Implementation of Risc-V's 32 & 64 bit C extensions for RISC-V -//! -//! Chapter 16,5 - Unprivileged spec - -use crate::instruction_context::ICB; -use crate::machine_state::registers::NonZeroXRegister; - -/// Copies the value in register `rs2` into register `rd_rs1`. -/// -/// Relevant RISC-V opcodes: -/// - C.MV -/// - ADD -/// - SUB -/// - OR -/// - XOR -/// - SLL -/// - SRL -/// - SRA -pub fn run_mv(icb: &mut impl ICB, rd_rs1: NonZeroXRegister, rs2: NonZeroXRegister) { - let rs2_val = icb.xregister_read_nz(rs2); - icb.xregister_write_nz(rd_rs1, rs2_val) -} - -/// Does nothing. -/// -/// Relevant RISC-V opcodes: -/// - C.NOP -/// - ADDI -/// - C.ADDI4SPN -/// - C.ANDI -/// - C.SRLI -/// - C.SRAI -/// - C.AND -/// - C.OR -/// - C.XOR -/// - BNE -/// - C.BNEZ -/// - C.SUB -pub fn run_nop(_icb: &mut impl ICB) {} - -#[cfg(test)] -mod tests { - use super::*; - use crate::backend_test; - use crate::create_state; - use crate::interpreter::i::run_add; - use crate::machine_state::MachineCoreState; - use crate::machine_state::MachineCoreStateLayout; - use crate::machine_state::memory::M4K; - use crate::machine_state::registers::nz; - - backend_test!(test_add_mv, F, { - let imm_rs1_res = [ - (0_i64, 0_u64, 0_u64), - (0, 0xFFF0_0420, 0xFFF0_0420), - (-1, 0, 0xFFFF_FFFF_FFFF_FFFF), - (1_000_000, -123_000_987_i64 as u64, -122_000_987_i64 as u64), - (1_000_000, 123_000_987, 124_000_987), - (-1, -321_000_000_000_i64 as u64, -321_000_000_001_i64 as u64), - ]; - - for (imm, rs1, res) in imm_rs1_res { - let mut state = create_state!(MachineCoreState, MachineCoreStateLayout, F, M4K); - - state.hart.xregisters.write_nz(nz::a3, rs1); - state.hart.xregisters.write_nz(nz::a4, imm as u64); - - run_add(&mut state, nz::a3, nz::a4, nz::a3); - assert_eq!(state.hart.xregisters.read_nz(nz::a3), res); - run_mv(&mut state, nz::a4, nz::a3); - assert_eq!(state.hart.xregisters.read_nz(nz::a4), res); - } - }); -} diff --git a/src/riscv/lib/src/interpreter/i.rs b/src/riscv/lib/src/interpreter/i.rs deleted file mode 100644 index 2d978230eeb0..000000000000 --- a/src/riscv/lib/src/interpreter/i.rs +++ /dev/null @@ -1,59 +0,0 @@ -// SPDX-FileCopyrightText: 2025 TriliTech -// -// SPDX-License-Identifier: MIT - -//! Implementation of Risc-V's 32 & 64 bit I extensions for RISC-V -//! -//! Chapter 5,1 - Unprivileged spec - -use crate::instruction_context::ICB; -use crate::machine_state::registers::NonZeroXRegister; - -/// Perform `val(rs1) + val(rs2)` and store the result in `rd` -/// -/// Relevant RISC-V opcodes: -/// - ADD -/// - C.ADD -pub fn run_add( - icb: &mut impl ICB, - rs1: NonZeroXRegister, - rs2: NonZeroXRegister, - rd: NonZeroXRegister, -) { - let lhs = icb.xregister_read_nz(rs1); - let rhs = icb.xregister_read_nz(rs2); - // Wrapped addition in two's complement behaves the same for signed and unsigned - let result = icb.xvalue_wrapping_add(lhs, rhs); - icb.xregister_write_nz(rd, result) -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::backend_test; - use crate::create_state; - use crate::machine_state::MachineCoreState; - use crate::machine_state::MachineCoreStateLayout; - use crate::machine_state::memory::M4K; - use crate::machine_state::registers::nz; - backend_test!(test_add, F, { - let imm_rs1_res = [ - (0_i64, 0_u64, 0_u64), - (0, 0xFFF0_0420, 0xFFF0_0420), - (-1, 0, 0xFFFF_FFFF_FFFF_FFFF), - (1_000_000, -123_000_987_i64 as u64, -122_000_987_i64 as u64), - (1_000_000, 123_000_987, 124_000_987), - (-1, -321_000_000_000_i64 as u64, -321_000_000_001_i64 as u64), - ]; - - for (imm, rs1, res) in imm_rs1_res { - let mut state = create_state!(MachineCoreState, MachineCoreStateLayout, F, M4K); - - state.hart.xregisters.write_nz(nz::a0, rs1); - state.hart.xregisters.write_nz(nz::t0, imm as u64); - - run_add(&mut state, nz::a0, nz::t0, nz::a0); - assert_eq!(state.hart.xregisters.read_nz(nz::a0), res); - } - }); -} diff --git a/src/riscv/lib/src/interpreter/integer.rs b/src/riscv/lib/src/interpreter/integer.rs index fd3196c52be8..c52f4863b288 100644 --- a/src/riscv/lib/src/interpreter/integer.rs +++ b/src/riscv/lib/src/interpreter/integer.rs @@ -2,7 +2,8 @@ // // SPDX-License-Identifier: MIT -//! Implementation of internal opcodes appropriate for JIT compilation. +//! Implementation of integer arithmetic operations for RISC-V over the ICB. +// TODO: RV 519: Update remaining 'neg' handler in the file to work over the ICB use crate::instruction_context::ICB; use crate::machine_state::registers::NonZeroXRegister; @@ -23,6 +24,57 @@ where } } +/// Copies the value in register `rs2` into register `rd_rs1`. +/// +/// Relevant RISC-V opcodes: +/// - C.MV +/// - ADD +/// - SUB +/// - OR +/// - XOR +/// - SLL +/// - SRL +/// - SRA +pub fn run_mv(icb: &mut impl ICB, rd_rs1: NonZeroXRegister, rs2: NonZeroXRegister) { + let rs2_val = icb.xregister_read_nz(rs2); + icb.xregister_write_nz(rd_rs1, rs2_val) +} + +/// Does nothing. +/// +/// Relevant RISC-V opcodes: +/// - C.NOP +/// - ADDI +/// - C.ADDI4SPN +/// - C.ANDI +/// - C.SRLI +/// - C.SRAI +/// - C.AND +/// - C.OR +/// - C.XOR +/// - BNE +/// - C.BNEZ +/// - C.SUB +pub fn run_nop(_icb: &mut impl ICB) {} + +/// Perform `val(rs1) + val(rs2)` and store the result in `rd` +/// +/// Relevant RISC-V opcodes: +/// - ADD +/// - C.ADD +pub fn run_add( + icb: &mut impl ICB, + rs1: NonZeroXRegister, + rs2: NonZeroXRegister, + rd: NonZeroXRegister, +) { + let lhs = icb.xregister_read_nz(rs1); + let rhs = icb.xregister_read_nz(rs2); + // Wrapped addition in two's complement behaves the same for signed and unsigned + let result = icb.xvalue_wrapping_add(lhs, rhs); + icb.xregister_write_nz(rd, result) +} + /// Saves in `rd` the bitwise AND between the value in `rs1` and `rs2` /// /// Relevant RISC-V opcodes: @@ -63,6 +115,8 @@ pub fn run_or( mod tests { use crate::backend_test; use crate::create_state; + use crate::interpreter::integer::run_add; + use crate::interpreter::integer::run_mv; use crate::machine_state::MachineCoreState; use crate::machine_state::MachineCoreStateLayout; use crate::machine_state::memory::M4K; @@ -84,4 +138,27 @@ mod tests { assert_eq!(state.hart.xregisters.read_nz(rd), res); } }); + + backend_test!(test_add_mv, F, { + let imm_rs1_res = [ + (0_i64, 0_u64, 0_u64), + (0, 0xFFF0_0420, 0xFFF0_0420), + (-1, 0, 0xFFFF_FFFF_FFFF_FFFF), + (1_000_000, -123_000_987_i64 as u64, -122_000_987_i64 as u64), + (1_000_000, 123_000_987, 124_000_987), + (-1, -321_000_000_000_i64 as u64, -321_000_000_001_i64 as u64), + ]; + + for (imm, rs1, res) in imm_rs1_res { + let mut state = create_state!(MachineCoreState, MachineCoreStateLayout, F, M4K); + + state.hart.xregisters.write_nz(nz::a3, rs1); + state.hart.xregisters.write_nz(nz::a4, imm as u64); + + run_add(&mut state, nz::a3, nz::a4, nz::a3); + assert_eq!(state.hart.xregisters.read_nz(nz::a3), res); + run_mv(&mut state, nz::a4, nz::a3); + assert_eq!(state.hart.xregisters.read_nz(nz::a4), res); + } + }); } diff --git a/src/riscv/lib/src/interpreter/load_store.rs b/src/riscv/lib/src/interpreter/load_store.rs index 0fb67195a130..f045aa2b29fe 100644 --- a/src/riscv/lib/src/interpreter/load_store.rs +++ b/src/riscv/lib/src/interpreter/load_store.rs @@ -1,3 +1,9 @@ +// SPDX-FileCopyrightText: 2025 TriliTech +// +// SPDX-License-Identifier: MIT + +//! Implementation of load and store instructions for RISC-V over the ICB. + use crate::instruction_context::ICB; use crate::machine_state::registers::NonZeroXRegister; diff --git a/src/riscv/lib/src/interpreter/rv32i.rs b/src/riscv/lib/src/interpreter/rv32i.rs index 7f87a50a2b4b..a70efbe9dd48 100644 --- a/src/riscv/lib/src/interpreter/rv32i.rs +++ b/src/riscv/lib/src/interpreter/rv32i.rs @@ -422,7 +422,7 @@ mod tests { use crate::backend_test; use crate::create_state; - use crate::interpreter::i::run_add; + use crate::interpreter::integer::run_add; use crate::interpreter::integer::run_and; use crate::interpreter::integer::run_or; use crate::machine_state::MachineCoreState; diff --git a/src/riscv/lib/src/machine_state/instruction.rs b/src/riscv/lib/src/machine_state/instruction.rs index faf5920160b4..fba9741343ec 100644 --- a/src/riscv/lib/src/machine_state/instruction.rs +++ b/src/riscv/lib/src/machine_state/instruction.rs @@ -39,8 +39,6 @@ use crate::instruction_context::ICB; use crate::instruction_context::IcbFnResult; use crate::instruction_context::IcbLoweringFn; use crate::interpreter::branching; -use crate::interpreter::c; -use crate::interpreter::i; use crate::interpreter::integer; use crate::interpreter::load_store; use crate::machine_state::ProgramCounterUpdate::Next; @@ -1184,7 +1182,7 @@ macro_rules! impl_f_r_type { impl Args { // RV64I R-type instructions - impl_r_type!(i::run_add, run_add, non_zero); + impl_r_type!(integer::run_add, run_add, non_zero); impl_r_type!(run_sub, non_zero); impl_r_type!(run_xor, non_zero); impl_r_type!(integer::run_and, run_and, non_zero); @@ -1403,7 +1401,7 @@ impl Args { impl_csr_imm_type!(run_csrrci); // RV32C compressed instructions - impl_cr_nz_type!(c::run_mv, run_mv); + impl_cr_nz_type!(integer::run_mv, run_mv); impl_cb_type!(run_beqz); impl_cb_type!(run_bnez); impl_cb_type!(run_bltz); @@ -1460,7 +1458,7 @@ impl Args { } fn run_nop(&self, icb: &mut I) -> IcbFnResult { - c::run_nop(icb); + integer::run_nop(icb); let pcu = ProgramCounterUpdate::Next(self.width); icb.ok(pcu) } -- GitLab