From 39fd9d3261c3a4cede9c89be31f0163254c64d41 Mon Sep 17 00:00:00 2001
From: naure <naure@users.noreply.github.com>
Date: Thu, 31 Oct 2024 14:30:47 +0100
Subject: [PATCH] emul-visibility: Reduce visibility of decoder details (#513)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Aurélien Nicolas <info@nau.re>
---
 ceno_emul/src/lib.rs             |  2 +-
 ceno_emul/src/rv32im.rs          | 39 +++++++++++---------------------
 ceno_emul/tests/test_vm_trace.rs |  2 +-
 ceno_zkvm/src/scheme/tests.rs    |  2 +-
 4 files changed, 16 insertions(+), 29 deletions(-)

diff --git a/ceno_emul/src/lib.rs b/ceno_emul/src/lib.rs
index f32e462f1..628c75b7c 100644
--- a/ceno_emul/src/lib.rs
+++ b/ceno_emul/src/lib.rs
@@ -11,7 +11,7 @@ mod vm_state;
 pub use vm_state::VMState;
 
 mod rv32im;
-pub use rv32im::{DecodedInstruction, EmuContext, InsnCategory, InsnCodes, InsnKind};
+pub use rv32im::{DecodedInstruction, EmuContext, InsnCodes, InsnKind};
 
 mod elf;
 pub use elf::Program;
diff --git a/ceno_emul/src/rv32im.rs b/ceno_emul/src/rv32im.rs
index 99970f356..98dc75d14 100644
--- a/ceno_emul/src/rv32im.rs
+++ b/ceno_emul/src/rv32im.rs
@@ -101,6 +101,7 @@ pub enum TrapCause {
 pub struct DecodedInstruction {
     insn: u32,
     top_bit: u32,
+    // The bit fields of the instruction encoding, regardless of the instruction format.
     func7: u32,
     rs2: u32,
     rs1: u32,
@@ -110,7 +111,7 @@ pub struct DecodedInstruction {
 }
 
 #[derive(Clone, Copy, Debug)]
-pub enum InsnCategory {
+enum InsnCategory {
     Compute,
     Branch,
     Load,
@@ -236,11 +237,6 @@ impl DecodedInstruction {
         }
     }
 
-    /// Get the funct3 field, regardless of the instruction format.
-    pub fn funct3(&self) -> u32 {
-        self.func3
-    }
-
     /// Get the funct3 field, or zero if the instruction does not use funct3.
     pub fn funct3_or_zero(&self) -> u32 {
         match self.codes().format {
@@ -275,18 +271,13 @@ impl DecodedInstruction {
         }
     }
 
-    /// Get the funct7 field, regardless of the instruction format.
-    pub fn funct7(&self) -> u32 {
-        self.func7
-    }
-
     /// Get the decoded immediate, or 2^shift, or the funct7 field, depending on the instruction format.
     pub fn imm_or_funct7(&self) -> u32 {
         match self.codes().format {
             R => self.func7,
             I => match self.codes().kind {
                 // decode the shift as a multiplication/division by 1 << immediate
-                SLLI | SRLI | SRAI => 1 << (self.imm_i() & 0x1f),
+                SLLI | SRLI | SRAI => 1 << self.imm_shamt(),
                 _ => self.imm_i(),
             },
             S => self.imm_s(),
@@ -308,35 +299,31 @@ impl DecodedInstruction {
         }
     }
 
-    pub fn sign_bit(&self) -> u32 {
-        self.top_bit
-    }
-
     pub fn codes(&self) -> InsnCodes {
         FastDecodeTable::get().lookup(self)
     }
 
-    pub fn kind(&self) -> (InsnCategory, InsnKind) {
-        let i = FastDecodeTable::get().lookup(self);
-        (i.category, i.kind)
-    }
-
-    pub fn imm_b(&self) -> u32 {
+    fn imm_b(&self) -> u32 {
         (self.top_bit * 0xfffff000)
             | ((self.rd & 1) << 11)
             | ((self.func7 & 0x3f) << 5)
             | (self.rd & 0x1e)
     }
 
-    pub fn imm_i(&self) -> u32 {
+    fn imm_i(&self) -> u32 {
         (self.top_bit * 0xfffff000) | (self.func7 << 5) | self.rs2
     }
 
-    pub fn imm_s(&self) -> u32 {
+    /// Shift amount field of SLLI, SRLI, SRAI.
+    fn imm_shamt(&self) -> u32 {
+        self.rs2
+    }
+
+    fn imm_s(&self) -> u32 {
         (self.top_bit * 0xfffff000) | (self.func7 << 5) | self.rd
     }
 
-    pub fn imm_j(&self) -> u32 {
+    fn imm_j(&self) -> u32 {
         (self.top_bit * 0xfff00000)
             | (self.rs1 << 15)
             | (self.func3 << 12)
@@ -345,7 +332,7 @@ impl DecodedInstruction {
             | (self.rs2 & 0x1e)
     }
 
-    pub fn imm_u(&self) -> u32 {
+    fn imm_u(&self) -> u32 {
         self.insn & 0xfffff000
     }
 }
diff --git a/ceno_emul/tests/test_vm_trace.rs b/ceno_emul/tests/test_vm_trace.rs
index c01977823..3f6b00cd6 100644
--- a/ceno_emul/tests/test_vm_trace.rs
+++ b/ceno_emul/tests/test_vm_trace.rs
@@ -33,7 +33,7 @@ fn test_vm_trace() -> Result<()> {
     assert_eq!(ctx.peek_register(2), x2);
     assert_eq!(ctx.peek_register(3), x3);
 
-    let ops: Vec<InsnKind> = steps.iter().map(|step| step.insn().kind().1).collect();
+    let ops: Vec<InsnKind> = steps.iter().map(|step| step.insn().codes().kind).collect();
     assert_eq!(ops, expected_ops_fibonacci_20());
 
     assert_eq!(
diff --git a/ceno_zkvm/src/scheme/tests.rs b/ceno_zkvm/src/scheme/tests.rs
index 2a66ad599..cead8e552 100644
--- a/ceno_zkvm/src/scheme/tests.rs
+++ b/ceno_zkvm/src/scheme/tests.rs
@@ -262,7 +262,7 @@ fn test_single_add_instance_e2e() {
     let mut add_records = vec![];
     let mut halt_records = vec![];
     all_records.into_iter().for_each(|record| {
-        let kind = record.insn().kind().1;
+        let kind = record.insn().codes().kind;
         match kind {
             ADD => add_records.push(record),
             EANY => {