From 62564d72fc729f7000b6574cd11400a51224e1f5 Mon Sep 17 00:00:00 2001
From: Emma Turner <emma.turner@trili.tech>
Date: Fri, 13 Sep 2024 16:56:59 +0100
Subject: [PATCH] RISC-V: instruction cache returns references

This avoids unneccessary copying of the instruction bytes, since running instructions also takes a reference, resulting
in a performance gain.
---
 .../src/machine_state/instruction_cache.rs    | 30 +++++++++----------
 src/riscv/lib/src/state_backend/region.rs     | 16 ++++++++++
 2 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/src/riscv/lib/src/machine_state/instruction_cache.rs b/src/riscv/lib/src/machine_state/instruction_cache.rs
index 8e7194a32875..abde39fdc12b 100644
--- a/src/riscv/lib/src/machine_state/instruction_cache.rs
+++ b/src/riscv/lib/src/machine_state/instruction_cache.rs
@@ -39,8 +39,8 @@ use crate::machine_state::bus::Address;
 use crate::parser::instruction::{Instr, InstrCacheable};
 use crate::parser::{parse, parse_compressed_instruction, parse_uncompressed_instruction};
 use crate::state_backend::{
-    self, AllocatedOf, Atom, Cell, CellRead, CellWrite, Choreographer, Elem, Layout, LazyCell,
-    ManagerAlloc, ManagerBase, ManagerRead, ManagerReadWrite, ManagerWrite, Many, Placed, Ref,
+    self, AllocatedOf, Atom, Cell, CellWrite, Choreographer, Elem, Layout, LazyCell, ManagerAlloc,
+    ManagerBase, ManagerRead, ManagerReadWrite, ManagerWrite, Many, Placed, Ref,
 };
 use std::convert::Infallible;
 
@@ -346,7 +346,7 @@ impl<ICL: InstructionCacheLayout, M: ManagerBase> InstructionCache<ICL, M> {
 
     /// Fetch the cached instruction at the given address, if an entry exists.
     #[inline(always)]
-    pub fn fetch_instr(&self, phys_addr: Address) -> Option<InstrCacheable>
+    pub fn fetch_instr(&mut self, phys_addr: Address) -> Option<&InstrCacheable>
     where
         M: ManagerRead,
     {
@@ -354,12 +354,12 @@ impl<ICL: InstructionCacheLayout, M: ManagerBase> InstructionCache<ICL, M> {
             return None;
         }
 
-        let cached = ICL::entry(&self.entries, phys_addr);
+        let cached = ICL::entry_mut(&mut self.entries, phys_addr);
 
-        if cached.fence_counter.read() == self.fence_counter.read()
-            && cached.phys_addr.read() == phys_addr
+        if cached.phys_addr.read() == phys_addr
+            && cached.fence_counter.read() == self.fence_counter.read()
         {
-            let (instr, _) = cached.instr.read();
+            let instr = &cached.instr.read_ref().0;
             Some(instr)
         } else {
             None
@@ -437,7 +437,7 @@ mod tests {
     // devices).
     backend_test!(test_fetch_before_cache_misses, F, {
         let mut backend = create_backend!(TestInstructionCacheLayout, F);
-        let state = create_state!(
+        let mut state = create_state!(
             InstructionCache,
             TestInstructionCacheLayout,
             F,
@@ -475,12 +475,12 @@ mod tests {
 
         // Compressed instruction can be cached
         state.cache_compressed(phys_addr, compressed_bytes);
-        assert_eq!(Some(compressed), state.fetch_instr(phys_addr));
+        assert_eq!(Some(&compressed), state.fetch_instr(phys_addr));
 
         // Caching an uncompressed instruction across page boundary should fail - the old
         // uncompressed instruction is still there.
         state.cache_uncompressed(phys_addr, uncompressed_bytes);
-        assert_eq!(Some(compressed), state.fetch_instr(phys_addr));
+        assert_eq!(Some(&compressed), state.fetch_instr(phys_addr));
     });
 
     // Ensure
@@ -516,18 +516,18 @@ mod tests {
             );
 
             state.cache_compressed(phys_addr_compressed, compressed_bytes);
-            assert_eq!(Some(compressed), state.fetch_instr(phys_addr_compressed));
+            assert_eq!(Some(&compressed), state.fetch_instr(phys_addr_compressed));
 
             state.cache_uncompressed(phys_addr_uncompressed, uncompressed_bytes);
             assert_eq!(
-                Some(uncompressed),
+                Some(&uncompressed),
                 state.fetch_instr(phys_addr_uncompressed)
             );
         }
 
         // Rebind state
         {
-            let state = create_state!(
+            let mut state = create_state!(
                 InstructionCache,
                 TestInstructionCacheLayout,
                 F,
@@ -535,9 +535,9 @@ mod tests {
                 TestInstructionCacheLayout
             );
 
-            assert_eq!(Some(compressed), state.fetch_instr(phys_addr_compressed));
+            assert_eq!(Some(&compressed), state.fetch_instr(phys_addr_compressed));
             assert_eq!(
-                Some(uncompressed),
+                Some(&uncompressed),
                 state.fetch_instr(phys_addr_uncompressed)
             );
         }
diff --git a/src/riscv/lib/src/state_backend/region.rs b/src/riscv/lib/src/state_backend/region.rs
index 464f1add842e..ed724d9253a6 100644
--- a/src/riscv/lib/src/state_backend/region.rs
+++ b/src/riscv/lib/src/state_backend/region.rs
@@ -119,6 +119,22 @@ impl<E: Elem, T, M: ManagerBase> LazyCell<E, T, M> {
     pub fn struct_ref(&self) -> AllocatedOf<Atom<E>, Ref<'_, M>> {
         self.inner.struct_ref()
     }
+
+    /// Read the in-memory value as a reference, rather than copying as
+    /// [`CellRead::read`] does.
+    pub fn read_ref(&mut self) -> &T
+    where
+        M: ManagerRead,
+        T: From<E>,
+    {
+        match self.value.get_mut() {
+            Some(v) => v,
+            n @ None => {
+                *n = Some(self.inner.read().into());
+                n.as_ref().unwrap()
+            }
+        }
+    }
 }
 
 /// A cell that support reading only.
-- 
GitLab