ab_riscv_interpreter/

basic.rs

//! Basic implementations of various interpreter traits

// Only available on 64-bit hosts. The position of the fetcher in the decoded stream is a host
// pointer, which a relative branch moves by its `i32` offset times the decoded stream bytes per
// guest byte. On a 32-bit host that product doesn't always fit into `isize`, so the pointer wraps
// around, and a far branch can land on a wrong instruction. Clamping the move avoids that, but a
// refused target is kept in the position for the cold continuation, since threaded dispatch passes
// nothing but the two pointers of the fetcher to it, so a clamped target is reported with a wrong
// address. Supporting 32-bit hosts needs room for the real target somewhere else.
#[cfg(all(feature = "alloc", target_pointer_width = "64"))]
mod eager_instruction_fetcher;
#[cfg(test)]
mod tests;

#[cfg(all(feature = "alloc", target_pointer_width = "64"))]
pub use crate::basic::eager_instruction_fetcher::{
    BasicEagerDecodeError, BasicEagerInstructionFetcher, BasicEagerInstructions,
};
use crate::zawrs::WrsHandler;
use crate::zifencei::FenceIHandler;
use crate::{
    Address, BasicInt, ExecutableInstruction, ExecutionError, ExecutionResult,
    FetchInstructionResult, InstructionFetcher, InstructionReg, PackedAddress, ProgramCounter,
    RegisterFile, Rs1Rs2OperandValues, Rs1Rs2Operands, SystemInstructionHandler, VirtualMemory,
    VirtualMemoryError,
};
use ab_riscv_primitives::prelude::*;
#[cfg(feature = "alloc")]
use alloc::boxed::Box;
use core::hint::{assert_unchecked, cold_path};
use core::ops::ControlFlow;
use replace_with::replace_with_or_abort_and_return;

/// Offset of a register in [`BasicRegisters`], which is always below [`Self::COUNT`]
#[derive(Debug, Clone, Copy)]
#[derive_const(PartialEq, Eq)]
#[repr(transparent)]
pub struct RegisterOffset(u8);

const impl From<RegisterOffset> for usize {
    #[inline(always)]
    fn from(offset: RegisterOffset) -> Self {
        // SAFETY: The field is private to this module and only set by the constructor, which
        // rejects offsets that are not below `RegisterOffset::COUNT`
        unsafe {
            assert_unchecked(usize::from(offset.0) < RegisterOffset::COUNT);
        }
        Self::from(offset.0)
    }
}

impl RegisterOffset {
    /// The number of distinct offsets, the maximum number of general purpose registers
    pub type const COUNT: usize = 32;

    /// Create a new instance.
    ///
    /// Returns `None` if `offset` is not below [`Self::COUNT`].
    #[inline(always)]
    pub const fn new(offset: u8) -> Option<Self> {
        if usize::from(offset) < Self::COUNT {
            Some(Self(offset))
        } else {
            None
        }
    }
}

/// Basic general purpose register to be used with [`BasicRegisters`]
pub const trait BasicRegister
where
    Self: [const] Register,
{
    /// Offset in a set of registers.
    ///
    /// Distinct registers must have distinct offsets for correct behavior, but it is not a safety
    /// requirement.
    fn offset(self) -> RegisterOffset;
}

const impl<Type> BasicRegister for EReg<Type>
where
    Self: [const] Register,
{
    #[inline(always)]
    fn offset(self) -> RegisterOffset {
        let offset = match self {
            Self::Zero => 0,
            Self::Ra => 1,
            Self::Sp => 2,
            Self::Gp => 3,
            Self::Tp => 4,
            Self::T0 => 5,
            Self::T1 => 6,
            Self::T2 => 7,
            Self::S0 => 8,
            Self::S1 => 9,
            Self::A0 => 10,
            Self::A1 => 11,
            Self::A2 => 12,
            Self::A3 => 13,
            Self::A4 => 14,
            Self::A5 => 15,
        };
        RegisterOffset::new(offset).expect("All offsets above are below 16; qed")
    }
}

const impl<Type> BasicRegister for Reg<Type>
where
    Self: [const] Register,
{
    #[inline(always)]
    fn offset(self) -> RegisterOffset {
        let offset = match self {
            Self::Zero => 0,
            Self::Ra => 1,
            Self::Sp => 2,
            Self::Gp => 3,
            Self::Tp => 4,
            Self::T0 => 5,
            Self::T1 => 6,
            Self::T2 => 7,
            Self::S0 => 8,
            Self::S1 => 9,
            Self::A0 => 10,
            Self::A1 => 11,
            Self::A2 => 12,
            Self::A3 => 13,
            Self::A4 => 14,
            Self::A5 => 15,
            Self::A6 => 16,
            Self::A7 => 17,
            Self::S2 => 18,
            Self::S3 => 19,
            Self::S4 => 20,
            Self::S5 => 21,
            Self::S6 => 22,
            Self::S7 => 23,
            Self::S8 => 24,
            Self::S9 => 25,
            Self::S10 => 26,
            Self::S11 => 27,
            Self::T3 => 28,
            Self::T4 => 29,
            Self::T5 => 30,
            Self::T6 => 31,
        };
        RegisterOffset::new(offset).expect("All offsets above are below 32; qed")
    }
}

/// A basic set of RISC-V GPRs (General Purpose Registers).
///
/// `ZEROSTORE` generic determines whether to zero `x0` register on write instead of checking
/// register index on read. `match` loop usually performs better with branching, while threaded
/// execution benefits from zeroing.
///
/// There is a slot for every possible [`RegisterOffset`], even with fewer registers (like with the
/// E extension), so that any offset is valid and no bounds checks are needed.
#[derive(Debug, Clone, Copy)]
#[repr(align(16))]
pub struct BasicRegisters<Reg, const ZEROSTORE: bool = false>
where
    Reg: BasicRegister,
{
    regs: [Reg::Type; RegisterOffset::COUNT],
}

impl<Reg, const ZEROSTORE: bool> Default for BasicRegisters<Reg, ZEROSTORE>
where
    Reg: BasicRegister,
{
    #[inline(always)]
    fn default() -> Self {
        Self {
            regs: [Reg::Type::default(); _],
        }
    }
}

const impl<Reg, const ZEROSTORE: bool> RegisterFile<Reg> for BasicRegisters<Reg, ZEROSTORE>
where
    Reg: [const] BasicRegister,
{
    #[inline(always)]
    #[cfg_attr(feature = "no-panic", no_panic_const::no_panic(const))]
    fn read(&self, reg: Reg) -> Reg::Type {
        if reg == Reg::ZERO && !ZEROSTORE {
            // Always zero
            return Reg::Type::default();
        }

        *self
            .regs
            .get(usize::from(reg.offset()))
            .expect("There is a register for every offset; qed")
    }

    #[inline(always)]
    #[cfg_attr(feature = "no-panic", no_panic_const::no_panic(const))]
    fn write(&mut self, reg: Reg, value: Reg::Type) {
        *self
            .regs
            .get_mut(usize::from(reg.offset()))
            .expect("There is a register for every offset; qed") = value;
        if ZEROSTORE {
            *self
                .regs
                .get_mut(usize::from(Reg::ZERO.offset()))
                .expect("There is a register for every offset; qed") = Reg::Type::default();
        }
    }
}

/// Basic interpreter state.
///
/// This is a simple container, which is not required to be used, is helpful for storing the whole
/// state related to the interpreter together.
#[derive(Debug)]
pub struct BasicInterpreterState<Regs, Env, Memory, IF> {
    /// General purpose registers
    pub regs: Regs,
    /// Execution environment.
    ///
    /// Extensions might use this to place additional constraints on `Env` to require additional
    /// registers, handlers (like [`SystemInstructionHandler`]) or other resources. If no such
    /// extension is used, `()` can be used as a placeholder.
    pub env: Env,
    /// Memory
    pub memory: Memory,
    /// Instruction fetcher
    pub instruction_fetcher: IF,
}

impl<Regs, Env, Memory, IF> BasicInterpreterState<Regs, Env, Memory, IF> {
    /// Execute the program with a given basic interpreter state.
    ///
    /// The implementation is designed to be efficient with little left to optimize further. Though
    /// it is still possible to improve performance by applying additional constraints on the
    /// program.
    ///
    /// When execution fails, the program counter stays where the failure left it:
    /// * after a branch whose target was refused, at that target, so calling this again fails the
    ///   same way
    /// * after an instruction that failed, or a jump whose target [`ProgramCounter::set_pc()`]
    ///   refused, right after that instruction, so calling this again continues with the next one
    /// * after a failed fetch, wherever the instruction fetcher left it, which for
    ///   [`BasicInstructionFetcher`] is the address it couldn't fetch from
    // TODO: It might be impractical to support `no-panic` here directly in a general case, but it
    //  should be possible to do so for small extensions to verify the workflow
    //
    // Make sure each handler starts on a cache line boundary
    #[rustc_align(64)]
    pub fn execute<I>(&mut self) -> Result<(), ExecutionError<Address<I>>>
    where
        Regs: RegisterFile<InstructionReg<I>>,
        I: ExecutableInstruction<Regs, Env, Memory, IF>,
        Memory: VirtualMemory,
        IF: InstructionFetcher<I, Memory> + ProgramCounter<Address<I>, Memory>,
    {
        replace_with_or_abort_and_return(
            &mut self.instruction_fetcher,
            #[inline(always)]
            |mut instruction_fetcher| {
                // The last execution may have stopped where there is nothing to fetch
                let pc = instruction_fetcher.get_pc();
                match instruction_fetcher.set_pc(&self.memory, pc) {
                    Ok(ControlFlow::Continue(())) => {}
                    Ok(ControlFlow::Break(())) => {
                        cold_path();
                        return (Ok(()), instruction_fetcher);
                    }
                    Err(error) => {
                        cold_path();
                        return (Err(error), instruction_fetcher);
                    }
                }

                loop {
                    // SAFETY: `set_pc()` above returned `Continue`, and the loop only comes back
                    // here after fetching returned `Continue`, after an instruction continued, or
                    // after its branch or jump was accepted
                    let fetched = unsafe { instruction_fetcher.fetch_instruction(&self.memory) };
                    let instruction = match fetched {
                        FetchInstructionResult::Instruction(instruction) => instruction,
                        FetchInstructionResult::Continue => {
                            cold_path();
                            continue;
                        }
                        FetchInstructionResult::Break => {
                            cold_path();
                            break;
                        }
                        FetchInstructionResult::Err(error) => {
                            cold_path();
                            return (Err(error), instruction_fetcher);
                        }
                    };

                    let Rs1Rs2Operands { rs1, rs2 } = instruction.get_rs1_rs2_operands();
                    let rs1rs2_values = Rs1Rs2OperandValues {
                        rs1_value: self.regs.read(rs1),
                        rs2_value: self.regs.read(rs2),
                    };

                    let outcome = instruction.execute(
                        rs1rs2_values,
                        &mut self.regs,
                        &mut self.env,
                        &mut self.memory,
                        &instruction_fetcher,
                    );

                    let control_flow =
                        match outcome {
                            ExecutionResult::Continue { rd, value } => {
                                self.regs.write(rd, value);
                                continue;
                            }
                            ExecutionResult::ContinueNoWrite => {
                                continue;
                            }
                            ExecutionResult::Branch { offset } => instruction_fetcher
                                .set_pc_relative(&self.memory, instruction.size(), offset),
                            ExecutionResult::Jump { target } => {
                                instruction_fetcher.set_pc(&self.memory, target)
                            }
                            ExecutionResult::Break => {
                                cold_path();
                                break;
                            }
                            ExecutionResult::Err(error) => {
                                cold_path();
                                return (Err(error), instruction_fetcher);
                            }
                        };

                    match control_flow {
                        Ok(ControlFlow::Continue(())) => {}
                        Ok(ControlFlow::Break(())) => {
                            cold_path();
                            break;
                        }
                        Err(error) => {
                            cold_path();
                            return (Err(error), instruction_fetcher);
                        }
                    }
                }

                (Ok(()), instruction_fetcher)
            },
        )
    }
}

/// Ensures [`BasicMemory`] doesn't wrap around the end of the address space
const NON_WRAPPING_MEMORY<const BASE_ADDR: u64, const SIZE: usize>: usize = {
    assert!(
        BASE_ADDR.checked_add(SIZE as u64).is_some(),
        "`BasicMemory` must not wrap around the end of the address space, or an address below its \
        base address could wrap into it"
    );
    0
};

/// Basic memory implementation.
///
/// Flat structure, no rwx protections, no alignment requirements. It uses stack, so for larger
/// allocation it'll need to be boxed (zero-initialized is fine) or a custom implementation to be
/// used.
///
/// `BASE_ADDR + SIZE` must fit into `u64`, meaning the memory region can't wrap around or end at
/// the very end of the address space, which is checked at compile time. This is what allows an
/// address below `BASE_ADDR` to be recognized by the bounds check that follows the subtraction of
/// the base address rather than by a check of its own.
///
/// This implementation is intentionally basic and correct, but not the most performant. It is
/// possible to have a more efficient implementation that skips certain checks by placing additional
/// constraints on the program.
///
/// This works for simpler cases, while a more sophisticated implementation might prevent certain
/// memory from being writable, supporting actual virtual memory with dynamically allocated memory
/// pages, etc.
#[derive(Debug, Copy, Clone)]
#[repr(align(16))]
pub struct BasicMemory<const BASE_ADDR: u64, const SIZE: usize> {
    data: [u8; SIZE],
}

const impl<const BASE_ADDR: u64, const SIZE: usize> VirtualMemory for BasicMemory<BASE_ADDR, SIZE>
where
    [(); NON_WRAPPING_MEMORY::<BASE_ADDR, SIZE>]:,
{
    #[inline(always)]
    #[cfg_attr(feature = "no-panic", no_panic_const::no_panic(const))]
    fn read<T>(&self, address: u64) -> Result<T, VirtualMemoryError>
    where
        T: BasicInt,
    {
        let offset = Self::offset(address);

        if offset.saturating_add(size_of::<T>() as u64) > self.data.len() as u64 {
            cold_path();
            return Err(VirtualMemoryError::OutOfBoundsRead { address });
        }

        // SAFETY: Only reading basic integers from initialized memory
        unsafe {
            Ok(self
                .data
                .as_ptr()
                .cast::<T>()
                .byte_add(offset as usize)
                .read_unaligned())
        }
    }

    #[inline(always)]
    #[cfg_attr(feature = "no-panic", no_panic_const::no_panic(const))]
    unsafe fn read_unchecked<T>(&self, address: u64) -> T
    where
        T: BasicInt,
    {
        // SAFETY: Guaranteed by function contract
        unsafe {
            let offset = address.unchecked_sub(BASE_ADDR) as usize;
            self.data
                .as_ptr()
                .cast::<T>()
                .byte_add(offset)
                .read_unaligned()
        }
    }

    #[inline(always)]
    #[cfg_attr(feature = "no-panic", no_panic_const::no_panic(const))]
    fn read_slice(&self, address: u64, len: u32) -> Result<&[u8], VirtualMemoryError> {
        let result = self.read_slice_inner(address, len);
        if let Ok(bytes) = result {
            // SAFETY: `read_slice_inner()` returns exactly `len` bytes on success. Asserted here
            // rather than inside so that callers can rely on it without the implementation being
            // inlined.
            unsafe {
                assert_unchecked(bytes.len() == len as usize);
            }
        }
        result
    }

    #[cfg_attr(feature = "no-panic", no_panic_const::no_panic(const))]
    fn read_slice_up_to(&self, address: u64, len: u32) -> &[u8] {
        let offset = Self::offset(address);

        if offset > self.data.len() as u64 {
            cold_path();
            return &[];
        }

        let remaining = self.data.get(offset as usize..).unwrap_or_default();
        remaining.get(..len as usize).unwrap_or(remaining)
    }

    #[inline(always)]
    #[cfg_attr(feature = "no-panic", no_panic_const::no_panic(const))]
    fn write<T>(&mut self, address: u64, value: T) -> Result<(), VirtualMemoryError>
    where
        T: BasicInt,
    {
        let offset = Self::offset(address);

        if offset.saturating_add(size_of::<T>() as u64) > self.data.len() as u64 {
            cold_path();
            return Err(VirtualMemoryError::OutOfBoundsWrite { address });
        }

        // SAFETY: Only writing basic integers to initialized memory
        unsafe {
            self.data
                .as_mut_ptr()
                .cast::<T>()
                .byte_add(offset as usize)
                .write_unaligned(value);
        }

        Ok(())
    }

    #[cfg_attr(feature = "no-panic", no_panic_const::no_panic(const))]
    fn write_slice(&mut self, address: u64, data: &[u8]) -> Result<(), VirtualMemoryError> {
        let offset = Self::offset(address);

        if offset > self.data.len() as u64 {
            cold_path();
            return Err(VirtualMemoryError::OutOfBoundsWrite { address });
        }

        let len = data.len();
        let Some(target_data) = self
            .data
            .get_mut(offset as usize..)
            .and_then(const |data| data.get_mut(..len))
        else {
            cold_path();
            return Err(VirtualMemoryError::OutOfBoundsWrite { address });
        };

        target_data.copy_from_slice(data);

        Ok(())
    }
}

impl<const BASE_ADDR: u64, const SIZE: usize> Default for BasicMemory<BASE_ADDR, SIZE>
where
    [(); NON_WRAPPING_MEMORY::<BASE_ADDR, SIZE>]:,
{
    #[inline(always)]
    fn default() -> Self {
        Self { data: [0; _] }
    }
}

impl<const BASE_ADDR: u64, const SIZE: usize> BasicMemory<BASE_ADDR, SIZE>
where
    [(); NON_WRAPPING_MEMORY::<BASE_ADDR, SIZE>]:,
{
    /// Offset of `address` within this memory region.
    ///
    /// Subtraction wraps rather than being checked: an address below `BASE_ADDR` produces an offset
    /// so large that the bounds check every caller does next rejects it, which is one comparison
    /// instead of two on every memory access.
    #[inline(always)]
    #[cfg_attr(feature = "no-panic", no_panic_const::no_panic)]
    const fn offset(address: u64) -> u64 {
        address.wrapping_sub(BASE_ADDR)
    }

    /// Implementation of [`VirtualMemory::read_slice()`], returns exactly `len` bytes on success
    #[inline]
    #[cfg_attr(feature = "no-panic", no_panic_const::no_panic)]
    const fn read_slice_inner(&self, address: u64, len: u32) -> Result<&[u8], VirtualMemoryError> {
        let offset = Self::offset(address);

        if offset > self.data.len() as u64 {
            cold_path();
            return Err(VirtualMemoryError::OutOfBoundsRead { address });
        }

        self.data
            .get(offset as usize..)
            .and_then(const |data| data.get(..len as usize))
            .ok_or(VirtualMemoryError::OutOfBoundsRead { address })
    }

    #[cfg(feature = "alloc")]
    pub fn new_boxed() -> Box<Self> {
        // SAFETY: Zeroed memory is a valid invariant
        unsafe { Box::<Self>::new_zeroed().assume_init() }
    }

    /// Get a mutable slice of memory.
    ///
    /// This is primarily useful for setting up the program and should not be used beyond that.
    #[cfg_attr(feature = "no-panic", no_panic_const::no_panic)]
    pub const fn get_mut_bytes(
        &mut self,
        address: u64,
        size: usize,
    ) -> Result<&mut [u8], VirtualMemoryError> {
        let Some(offset) = address.checked_sub(BASE_ADDR) else {
            cold_path();
            return Err(VirtualMemoryError::OutOfBoundsRead { address });
        };
        let offset = offset as usize;

        let Some(slice) = self
            .data
            .get_mut(offset..)
            .and_then(const |data| data.get_mut(..size))
        else {
            cold_path();
            return Err(VirtualMemoryError::OutOfBoundsRead { address });
        };

        Ok(slice)
    }
}

/// Basic instruction fetcher implementation.
///
/// This implementation is intentionally basic and correct, but not the most performant. It is
/// possible to have a more efficient implementation that skips certain checks by placing additional
/// constraints on the constructor.
///
/// Note that it loads instructions from anywhere in memory. This works, but it is likely that you
/// want to restrict this to a specific executable region of memory.
///
/// Every fetch is a bounds-checked memory read, so fetching is allowed at any program counter,
/// whatever moved it there.
#[derive(Debug, Copy, Clone)]
pub struct BasicInstructionFetcher<I>
where
    I: Instruction,
{
    return_trap_address: Address<I>,
    pc: Address<I>,
}

const impl<I, Memory> ProgramCounter<Address<I>, Memory> for BasicInstructionFetcher<I>
where
    I: [const] Instruction,
    Memory: [const] VirtualMemory,
{
    #[inline(always)]
    fn get_pc(&self) -> Address<I> {
        self.pc
    }

    #[inline(always)]
    #[cfg_attr(feature = "no-panic", no_panic_const::no_panic(const))]
    unsafe fn try_set_pc_relative(&mut self, instruction_size: u8, offset: i32) -> bool {
        let old_pc = <Self as ProgramCounter<_, Memory>>::old_pc(self, instruction_size);
        let pc = old_pc.wrapping_add_signed(offset);
        // Stored either way: on the way out it is what `failed_branch()` reports on, and until then
        // nothing else is allowed to look at it
        self.pc = pc;

        pc != self.return_trap_address && pc.as_u64().is_multiple_of(u64::from(I::ALIGNMENT))
    }

    #[cold]
    #[inline(never)]
    #[cfg_attr(feature = "no-panic", no_panic_const::no_panic(const))]
    unsafe fn failed_branch(
        &mut self,
        memory: &Memory,
    ) -> Result<ControlFlow<()>, ExecutionError<Address<I>>> {
        // The program counter holds the refused target, and `set_pc()` is what says what is wrong
        // with it
        self.set_pc(memory, self.pc)
    }

    #[inline]
    #[cfg_attr(feature = "no-panic", no_panic_const::no_panic(const))]
    fn set_pc(
        &mut self,
        _memory: &Memory,
        pc: Address<I>,
    ) -> Result<ControlFlow<()>, ExecutionError<Address<I>>> {
        if pc == self.return_trap_address {
            cold_path();
            // Execution stops at the trap, so that is the program counter `get_pc()` reports
            self.pc = pc;
            return Ok(ControlFlow::Break(()));
        }

        if !pc.as_u64().is_multiple_of(u64::from(I::ALIGNMENT)) {
            cold_path();
            return Err(ExecutionError::UnalignedInstruction {
                address: PackedAddress::new(pc),
            });
        }

        self.pc = pc;

        Ok(ControlFlow::Continue(()))
    }
}

// SAFETY: Fetching is a checked memory read with no `unsafe` code, so it is sound at any program
// counter. The program counter is a plain field that only methods taking `&mut self` write.
// `peeked_instruction()` returns `peeked` itself, which is the instruction `peek_instruction()`
// returned.
const unsafe impl<I, Memory> InstructionFetcher<I, Memory> for BasicInstructionFetcher<I>
where
    I: [const] Instruction,
    Memory: [const] VirtualMemory,
{
    type Peeked = I;

    #[inline(always)]
    #[cfg_attr(feature = "no-panic", no_panic_const::no_panic(const))]
    unsafe fn peeked_instruction<'a>(&'a self, peeked: &'a I) -> &'a I {
        peeked
    }

    #[inline]
    #[cfg_attr(feature = "no-panic", no_panic_const::no_panic(const))]
    unsafe fn peek_instruction(&mut self, memory: &Memory) -> FetchInstructionResult<I> {
        let instruction = match memory.read(self.pc.as_u64()).or_else(const |error| {
            cold_path();
            // Attempt to read a 16-bit compressed instruction
            if let Ok(instruction) = memory.read::<u16>(self.pc.as_u64())
                && (instruction & 0b11) != 0b11
            {
                return Ok(u32::from(instruction));
            }
            Err(error)
        }) {
            Ok(instruction) => instruction,
            Err(error) => {
                cold_path();
                return FetchInstructionResult::Err(ExecutionError::from(error));
            }
        };

        let Some(instruction) = I::try_decode(instruction) else {
            cold_path();
            return FetchInstructionResult::Err(ExecutionError::IllegalInstruction {
                address: PackedAddress::new(self.pc),
            });
        };
        FetchInstructionResult::Instruction(instruction)
    }

    #[inline(always)]
    #[cfg_attr(feature = "no-panic", no_panic_const::no_panic(const))]
    unsafe fn advance(&mut self, instruction_size: u8) {
        self.pc = self.pc.wrapping_add_signed(i32::from(instruction_size));
    }

    #[inline]
    #[cfg_attr(feature = "no-panic", no_panic_const::no_panic(const))]
    unsafe fn fetch_instruction(&mut self, memory: &Memory) -> FetchInstructionResult<I> {
        // SAFETY: Same requirements as for this method, which the caller upholds
        let result = unsafe { InstructionFetcher::<I, Memory>::peek_instruction(self, memory) };

        if let FetchInstructionResult::Instruction(instruction) = result {
            // SAFETY: The instruction was just peeked successfully, and this is the only place
            // that moves past it
            unsafe {
                InstructionFetcher::<I, Memory>::advance(self, instruction.size());
            }
        }

        result
    }
}

impl<I> BasicInstructionFetcher<I>
where
    I: Instruction,
{
    /// Create a new instance.
    ///
    /// `return_trap_address` is the address at which the interpreter will stop execution
    /// (gracefully).
    #[inline(always)]
    pub const fn new(return_trap_address: Address<I>, pc: Address<I>) -> Self {
        Self {
            return_trap_address,
            pc,
        }
    }
}

/// System instruction handler that results in illegal instruction for all system calls and does
/// nothing for other system instructions.
///
/// Being stateless, it can be used as the whole execution environment of a configuration that
/// needs nothing else from it.
#[derive(Debug, Default, Clone, Copy)]
pub struct IllegalEcallSystemInstructionHandler;

const impl<Reg, Regs, Memory, PC> SystemInstructionHandler<Reg, Regs, Memory, PC>
    for IllegalEcallSystemInstructionHandler
where
    Reg: [const] Register,
    PC: [const] ProgramCounter<Reg::Type, Memory>,
{
    #[cfg_attr(feature = "no-panic", no_panic_const::no_panic(const))]
    fn handle_ecall(
        &mut self,
        _regs: &mut Regs,
        _memory: &mut Memory,
        program_counter: &PC,
    ) -> ExecutionResult<Reg> {
        ExecutionResult::Err(ExecutionError::IllegalInstruction {
            address: PackedAddress::new(program_counter.old_pc(size_of::<u32>() as u8)),
        })
    }
}

const impl WrsHandler for IllegalEcallSystemInstructionHandler {}

const impl FenceIHandler for IllegalEcallSystemInstructionHandler {}