Move Instruction-Related Types into New Common Module (#61)

* Move Instruction-Related Types into New Common Module

* Non-Automatic Merge Fixes

* Add Traits for CPU/Registers

* Sam Comments II

* Revert "Sam Comments II" to Remove Generic Errors

This reverts commit f515ade66841d6e47403468420b318674e784eb3.

* Sam Comments II - Separate Errors Version

* Add Missing Files

* Sam Comments III + Misc. Fixes

* Sam Comments IV

* Fix Test

Cargo.toml

@@ -1,7 +1,14 @@
 [workspace]
 resolver = "2"
-members = ["runtime", "prover", "vm", "precompiles", "tests/testing-framework"]
-default-members = ["runtime", "prover", "vm"]
+members = [
+    "runtime",
+    "prover",
+    "vm",
+    "precompiles",
+    "common",
+    "tests/testing-framework",
+]
+default-members = ["runtime", "prover", "vm", "precompiles", "common"]
 
 [workspace.package]
 edition = "2021"

common/Cargo.toml

@@ -0,0 +1,17 @@
+[package]
+name = "nexus-common"
+edition.workspace = true
+version.workspace = true
+authors.workspace = true
+homepage.workspace = true
+repository.workspace = true
+keywords.workspace = true
+categories.workspace = true
+publish.workspace = true
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+rrs-lib = { git = "https://github.com/GregAC/rrs/" }
+thiserror = "1.0"
+variant_count = "1.1"

common/src/cpu/mod.rs

@@ -0,0 +1,7 @@
+mod pc;
+mod registers;
+mod traits;
+
+pub use pc::PC;
+pub use registers::Registers;
+pub use traits::{InstructionExecutor, InstructionState, Processor};

common/src/cpu/pc.rs

@@ -0,0 +1,63 @@
+#[derive(Default, Debug, Clone, Copy, PartialEq, Eq)]
+pub struct PC {
+    pub value: u32,
+}
+
+impl PC {
+    // Increment PC by 4 bytes (standard instruction length)
+    pub fn step(&mut self) {
+        self.value = self.value.wrapping_add(4);
+    }
+
+    // Branch: Add immediate value to PC
+    pub fn branch(&mut self, imm: u32) {
+        self.value = self.value.wrapping_add(sign_extension_branch(imm));
+    }
+
+    // Jump and Link: Add immediate value to PC
+    pub fn jal(&mut self, imm: u32) {
+        self.value = self.value.wrapping_add(sign_extension_jal(imm));
+    }
+
+    // Jump and Link Register: Set PC to rs1 + imm
+    pub fn jalr(&mut self, rs1: u32, imm: u32) {
+        self.value = rs1.wrapping_add(sign_extension_jalr(imm));
+    }
+}
+
+impl PartialEq<u32> for PC {
+    fn eq(&self, other: &u32) -> bool {
+        self.value == *other
+    }
+}
+
+#[inline]
+const fn sign_extension(imm: u32, bits: u32) -> u32 {
+    let mask = 1u32 << (bits - 1);
+    let value = imm & ((1u32 << bits) - 1); // Ensure we only use the specified number of bits
+    if value & mask != 0 {
+        // If the sign bit is set, extend with 1s
+        value | !((1u32 << bits) - 1)
+    } else {
+        // If the sign bit is not set, extend with 0s
+        value
+    }
+}
+
+// Sign extension for Branch (13-bit immediate)
+#[inline]
+const fn sign_extension_branch(imm: u32) -> u32 {
+    sign_extension(imm, 13)
+}
+
+// Sign extension for JAL (21-bit immediate)
+#[inline]
+const fn sign_extension_jal(imm: u32) -> u32 {
+    sign_extension(imm, 21)
+}
+
+// Sign extension for JALR (12-bit immediate)
+#[inline]
+const fn sign_extension_jalr(imm: u32) -> u32 {
+    sign_extension(imm, 12)
+}

common/src/cpu/registers.rs

@@ -0,0 +1,8 @@
+use std::{fmt::Display, ops::Index};
+
+use crate::riscv::register::Register;
+
+pub trait Registers: Index<Register, Output = u32> + Display {
+    fn read(&self, reg: Register) -> u32;
+    fn write(&mut self, reg: Register, value: u32);
+}

common/src/cpu/traits.rs

@@ -0,0 +1,96 @@
+use crate::{error::MemoryError, memory::MemoryProcessor, riscv::instruction::Instruction};
+
+use super::{pc::PC, registers::Registers};
+
+/// Interface that a CPU implementation must provide.
+pub trait Processor {
+    /// Returns an immutable reference to the CPU registers.
+    fn registers(&self) -> &impl Registers;
+
+    /// Returns a mutable reference to the CPU registers.
+    fn registers_mut(&mut self) -> &mut impl Registers;
+
+    fn pc(&self) -> &PC;
+
+    fn pc_mut(&mut self) -> &mut PC;
+}
+
+pub trait InstructionState {
+    type Result;
+
+    /// Executes the instruction's operation.
+    ///
+    /// This method performs the actual operation specified by the instruction,
+    /// such as arithmetic, logical operations, or control flow changes.
+    /// * `self` - The current instruction state.
+    fn execute(&mut self);
+
+    /// Performs memory access for load operations.
+    ///
+    /// # Arguments
+    /// * `self` - The mutable current instruction state.
+    /// * `memory` - Immutable reference to the memory subsystem.
+    ///
+    /// # Returns
+    /// A `Result` indicating the outcome of the memory access operation.
+    fn memory_read(&mut self, memory: &impl MemoryProcessor) -> Result<Self::Result, MemoryError>;
+
+    /// Performs memory access for store operations.
+    ///
+    /// # Arguments
+    /// * `self` - The immutable current instruction state.
+    /// * `memory` - Mutable reference to the memory subsystem.
+    ///
+    /// # Returns
+    /// A `Result` indicating the outcome of the memory access operation.
+    fn memory_write(&self, memory: &mut impl MemoryProcessor) -> Result<Self::Result, MemoryError>;
+
+    /// Updates the CPU state with the result of the instruction execution.
+    ///
+    /// This method writes back the results to registers or updates other CPU state as necessary.
+    ///
+    /// # Arguments
+    /// * `self` - The current instruction state.
+    /// * `cpu` - Mutable reference to the CPU state.
+    fn write_back(&self, cpu: &mut impl Processor);
+}
+
+/// Trait defining the execution stages of a RISC-V instruction.
+///
+/// This trait represents a simplified instruction cycle, excluding the fetch stage.
+/// It includes decode, execute, memory access, and write-back stages.
+pub trait InstructionExecutor {
+    type InstructionState: InstructionState;
+
+    /// Decodes the instruction and prepares operands.
+    ///
+    /// # Arguments
+    /// * `ins` - The instruction to be decoded.
+    /// * `regs` - The current state of the CPU registers.
+    ///
+    /// # Returns
+    /// An `InstructionState` containing the decoded instruction information.
+    fn decode(ins: &Instruction, regs: &impl Registers) -> Self::InstructionState;
+
+    /// Evaluates the constructed executor
+    ///
+    /// # Arguments
+    /// * `cpu` - Mutable reference to the CPU state.
+    /// * `memory` - Immutable reference to the memory subsystem.
+    /// * `ins` - The instruction to be decoded.
+    ///
+    /// # Returns
+    /// A `Result` indicating the whether the instruction was executed successfully.
+    fn evaluator(
+        cpu: &mut impl Processor,
+        memory: &mut impl MemoryProcessor,
+        ins: &Instruction,
+    ) -> Result<(), MemoryError> {
+        let mut executor: Self::InstructionState = Self::decode(ins, cpu.registers());
+        executor.memory_read(memory)?;
+        executor.execute();
+        executor.memory_write(memory)?;
+        executor.write_back(cpu);
+        Ok(())
+    }
+}

common/src/error/memory.rs

@@ -0,0 +1,20 @@
+use thiserror::Error;
+
+#[derive(Error, Debug, PartialEq)]
+pub enum MemoryError {
+    // Cannot write unaligned memory
+    #[error("Unaligned memory write: 0x{0:08X}")]
+    UnalignedMemoryWrite(u32),
+
+    // Cannot read unaligned memory
+    #[error("Unaligned memory read: 0x{0:08X}")]
+    UnalignedMemoryRead(u32),
+
+    // Invalid memory access
+    #[error("Invalid memory access: 0x{0:08X}")]
+    InvalidMemoryAccess(u32),
+
+    // Address calculation overflow
+    #[error("Address calculation overflow")]
+    AddressCalculationOverflow,
+}

common/src/error/mod.rs

@@ -0,0 +1,5 @@
+mod memory;
+mod opcode;
+
+pub use memory::MemoryError;
+pub use opcode::OpcodeError;

common/src/error/opcode.rs

@@ -0,0 +1,9 @@
+use thiserror::Error;
+
+use crate::riscv::Opcode;
+
+#[derive(Debug, Error, PartialEq)]
+pub enum OpcodeError {
+    #[error("Cannot convert non-builtin opcode to BuiltinOpcode: {0}")]
+    OpcodeNotBuiltin(Opcode),
+}

common/src/lib.rs

@@ -0,0 +1,4 @@
+pub mod cpu;
+pub mod error;
+pub mod memory;
+pub mod riscv;

common/src/memory/mod.rs

@@ -0,0 +1,2 @@
+pub mod traits;
+pub use traits::*;

vm/src/memory/trait.rs → common/src/memory/traits.rs

@@ -1,4 +1,4 @@
-use crate::error::Result;
+use crate::error::MemoryError;
 
 #[derive(Debug, Clone, Copy)]
 /// Represents the size of memory access operations.
@@ -10,7 +10,7 @@ pub enum MemAccessSize {
 }
 
 // Helper function to get shift and mask for different access sizes
-pub(crate) fn get_shift_and_mask(size: MemAccessSize, address: u32) -> (u32, u32) {
+pub fn get_shift_and_mask(size: MemAccessSize, address: u32) -> (u32, u32) {
     match size {
         MemAccessSize::Byte => ((address & 0x3) * 8, 0xff),
         MemAccessSize::HalfWord => ((address & 0x2) * 8, 0xffff),
@@ -34,7 +34,7 @@ pub trait MemoryProcessor: Default {
     /// # Returns
     ///
     /// Returns a `Result` containing the read value or an error.
-    fn read(&self, address: u32, size: MemAccessSize) -> Result<u32>;
+    fn read(&self, address: u32, size: MemAccessSize) -> Result<u32, MemoryError>;
 
     /// Writes a value to memory at the specified address.
     ///
@@ -47,11 +47,11 @@ pub trait MemoryProcessor: Default {
     /// # Returns
     ///
     /// Returns a `Result` indicating success or failure of the write operation.
-    fn write(&mut self, address: u32, size: MemAccessSize, value: u32) -> Result<u32>;
+    fn write(&mut self, address: u32, size: MemAccessSize, value: u32) -> Result<u32, MemoryError>;
 
     /// Reads multiple bytes from memory at the specified address, built on top of `read`.
-    fn read_bytes(&self, address: u32, size: usize) -> Result<Vec<u8>>;
+    fn read_bytes(&self, address: u32, size: usize) -> Result<Vec<u8>, MemoryError>;
 
     /// Writes multiple bytes to memory at the specified address, built on top of `write`.
-    fn write_bytes(&mut self, address: u32, data: &[u8]) -> Result<()>;
+    fn write_bytes(&mut self, address: u32, data: &[u8]) -> Result<(), MemoryError>;
 }