From 22df7946eabfd21ce88500104ebb3d446684541c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Nicolas?= Date: Tue, 3 Sep 2024 14:53:26 +0200 Subject: [PATCH 01/13] emul-trace-mem: refactor Tracer --- ceno_emul/src/tracer.rs | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/ceno_emul/src/tracer.rs b/ceno_emul/src/tracer.rs index 5a1a56e86..fc4ccee96 100644 --- a/ceno_emul/src/tracer.rs +++ b/ceno_emul/src/tracer.rs @@ -1,4 +1,4 @@ -use std::fmt; +use std::{fmt, mem}; use crate::{ addr::{ByteAddr, RegIdx, WordAddr}, @@ -55,9 +55,7 @@ impl StepRecord { } #[derive(Debug, Default)] -pub struct Tracer { - record: StepRecord, - +struct StepActions { rs1_loaded: bool, rs2_loaded: bool, rd_stored: bool, @@ -65,9 +63,16 @@ pub struct Tracer { memory_stored: bool, } +#[derive(Debug, Default)] +pub struct Tracer { + record: StepRecord, + actions: StepActions, +} + impl Tracer { pub fn advance(&mut self) -> StepRecord { - let record = std::mem::take(self).record; + let record = mem::take(&mut self.record); + let actions = mem::take(&mut self.actions); self.record.cycle = record.cycle + 1; record } @@ -82,41 +87,41 @@ impl Tracer { } pub fn load_register(&mut self, idx: usize, value: u32) { - match (self.rs1_loaded, self.rs2_loaded) { + match (self.actions.rs1_loaded, self.actions.rs2_loaded) { (false, false) => { self.record.rs1 = (idx, value); - self.rs1_loaded = true; + self.actions.rs1_loaded = true; } (true, false) => { self.record.rs2 = (idx, value); - self.rs2_loaded = true; + self.actions.rs2_loaded = true; } _ => unimplemented!("Only two register reads are supported"), } } pub fn store_register(&mut self, idx: usize, value: Change) { - if !self.rd_stored { + if !self.actions.rd_stored { self.record.rd = (idx, value); - self.rd_stored = true; + self.actions.rd_stored = true; } else { unimplemented!("Only one register write is supported"); } } pub fn load_memory(&mut self, addr: WordAddr, value: u32) { - if self.memory_loaded || self.memory_stored { + if self.actions.memory_loaded || self.actions.memory_stored { unimplemented!("Only one memory load is supported"); } - self.memory_loaded = true; + self.actions.memory_loaded = true; self.record.memory_op = (addr, Change::new(value, value)); } pub fn store_memory(&mut self, addr: WordAddr, value: Change) { - if self.memory_stored { + if self.actions.memory_stored { unimplemented!("Only one memory store is supported"); } - self.memory_stored = true; + self.actions.memory_stored = true; self.record.memory_op = (addr, value); } } From fa0e3bdcc2e9b3774ee9e021fb314d73f866859d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Nicolas?= Date: Tue, 3 Sep 2024 17:13:16 +0200 Subject: [PATCH 02/13] emul-trace-mem: keep track of memory ops --- ceno_emul/src/addr.rs | 32 ++++++++++++++++++++++-- ceno_emul/src/lib.rs | 2 +- ceno_emul/src/platform.rs | 51 ++++++++++++++++++++++++++++++++------- ceno_emul/src/tracer.rs | 29 +++++++++++++++++++--- 4 files changed, 98 insertions(+), 16 deletions(-) diff --git a/ceno_emul/src/addr.rs b/ceno_emul/src/addr.rs index e409ef131..eb2447cff 100644 --- a/ceno_emul/src/addr.rs +++ b/ceno_emul/src/addr.rs @@ -18,12 +18,16 @@ use std::{fmt, ops}; pub const WORD_SIZE: usize = 4; +// Type aliases to clarify the code without wrapper types. +pub type Word = u32; +pub type VMA = u32; +pub type Cycle = u32; pub type RegIdx = usize; -#[derive(Clone, Copy, Default, PartialEq)] +#[derive(Clone, Copy, Default, PartialEq, Eq, Hash)] pub struct ByteAddr(pub u32); -#[derive(Clone, Copy, Default, PartialEq)] +#[derive(Clone, Copy, Default, PartialEq, Eq, Hash)] pub struct WordAddr(pub u32); impl From for WordAddr { @@ -38,6 +42,30 @@ impl From for ByteAddr { } } +impl From for ByteAddr { + fn from(addr: u32) -> ByteAddr { + ByteAddr(addr) + } +} + +impl From for WordAddr { + fn from(addr: u32) -> WordAddr { + ByteAddr(addr).waddr() + } +} + +impl From for u32 { + fn from(addr: ByteAddr) -> Self { + addr.0 + } +} + +impl From for u32 { + fn from(addr: WordAddr) -> Self { + addr.baddr().0 + } +} + impl ByteAddr { pub const fn waddr(self) -> WordAddr { WordAddr(self.0 / WORD_SIZE as u32) diff --git a/ceno_emul/src/lib.rs b/ceno_emul/src/lib.rs index d3085a3c6..5a48668d6 100644 --- a/ceno_emul/src/lib.rs +++ b/ceno_emul/src/lib.rs @@ -1,5 +1,5 @@ mod addr; -pub use addr::{ByteAddr, RegIdx, WordAddr}; +pub use addr::*; mod platform; pub use platform::{Platform, CENO_PLATFORM}; diff --git a/ceno_emul/src/platform.rs b/ceno_emul/src/platform.rs index 7f91cdb9c..ce07e3894 100644 --- a/ceno_emul/src/platform.rs +++ b/ceno_emul/src/platform.rs @@ -1,3 +1,5 @@ +use crate::addr::{RegIdx, WORD_SIZE}; + /// The Platform struct holds the parameters of the VM. pub struct Platform; @@ -6,11 +8,11 @@ pub const CENO_PLATFORM: Platform = Platform; impl Platform { // Virtual memory layout. - pub fn rom_start(&self) -> u32 { + pub const fn rom_start(&self) -> u32 { 0x2000_0000 } - pub fn rom_end(&self) -> u32 { + pub const fn rom_end(&self) -> u32 { 0x3000_0000 - 1 } @@ -18,11 +20,11 @@ impl Platform { (self.rom_start()..=self.rom_end()).contains(&addr) } - pub fn ram_start(&self) -> u32 { + pub const fn ram_start(&self) -> u32 { 0x8000_0000 } - pub fn ram_end(&self) -> u32 { + pub const fn ram_end(&self) -> u32 { 0xFFFF_FFFF } @@ -30,9 +32,19 @@ impl Platform { (self.ram_start()..=self.ram_end()).contains(&addr) } + /// Virtual address of a register. + pub const fn register_vma(&self, idx: RegIdx) -> u32 { + (idx * WORD_SIZE) as u32 + } + + /// Virtual address of the program counter. + pub const fn pc_vma(&self) -> u32 { + self.register_vma(32) + } + // Startup. - pub fn pc_start(&self) -> u32 { + pub const fn pc_start(&self) -> u32 { self.rom_start() } @@ -53,22 +65,43 @@ impl Platform { // Environment calls. /// Register containing the ecall function code. (x5, t0) - pub fn reg_ecall(&self) -> usize { + pub const fn reg_ecall(&self) -> RegIdx { 5 } /// Register containing the first function argument. (x10, a0) - pub fn reg_arg0(&self) -> usize { + pub const fn reg_arg0(&self) -> RegIdx { 10 } /// The code of ecall HALT. - pub fn ecall_halt(&self) -> u32 { + pub const fn ecall_halt(&self) -> u32 { 0 } /// The code of success. - pub fn code_success(&self) -> u32 { + pub const fn code_success(&self) -> u32 { 0 } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_no_overlap() { + let p = CENO_PLATFORM; + assert!(p.can_execute(p.pc_start())); + // ROM and RAM do not overlap. + assert!(!p.is_rom(p.ram_start())); + assert!(!p.is_rom(p.ram_end())); + assert!(!p.is_ram(p.rom_start())); + assert!(!p.is_ram(p.rom_end())); + // Registers do not overlap with ROM or RAM. + for reg in [p.register_vma(0), p.register_vma(31), p.pc_vma()] { + assert!(!p.is_rom(reg)); + assert!(!p.is_ram(reg)); + } + } +} diff --git a/ceno_emul/src/tracer.rs b/ceno_emul/src/tracer.rs index fc4ccee96..ff30e1c78 100644 --- a/ceno_emul/src/tracer.rs +++ b/ceno_emul/src/tracer.rs @@ -1,14 +1,15 @@ -use std::{fmt, mem}; +use std::{collections::HashMap, fmt, mem}; use crate::{ - addr::{ByteAddr, RegIdx, WordAddr}, + addr::{ByteAddr, Cycle, RegIdx, WordAddr}, rv32im::DecodedInstruction, + CENO_PLATFORM, }; /// An instruction and its context in an execution trace. That is concrete values of registers and memory. #[derive(Clone, Debug, Default)] pub struct StepRecord { - cycle: u32, + cycle: Cycle, pc: Change, insn_code: u32, @@ -67,13 +68,33 @@ struct StepActions { pub struct Tracer { record: StepRecord, actions: StepActions, + + previous_mem_op: HashMap, } impl Tracer { pub fn advance(&mut self) -> StepRecord { - let record = mem::take(&mut self.record); + // Reset and advance to the next cycle. let actions = mem::take(&mut self.actions); + let record = mem::take(&mut self.record); self.record.cycle = record.cycle + 1; + + // Track this step as the origin of its memory accesses. + let mut track_mem = |vma| self.previous_mem_op.insert(vma, record.cycle); + + if actions.rs1_loaded { + track_mem(CENO_PLATFORM.register_vma(record.rs1.0).into()); + } + if actions.rs2_loaded { + track_mem(CENO_PLATFORM.register_vma(record.rs2.0).into()); + } + if actions.rd_stored { + track_mem(CENO_PLATFORM.register_vma(record.rd.0).into()); + } + if actions.memory_loaded || actions.memory_stored { + track_mem(record.memory_op.0); + } + record } From 4c94d6274a8a2e1fcd47b8cf7664a59c50e510a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Nicolas?= Date: Tue, 3 Sep 2024 17:28:01 +0200 Subject: [PATCH 03/13] emul-trace-mem: use type aliases everywhere before adding cycle fields --- ceno_emul/src/addr.rs | 2 +- ceno_emul/src/platform.rs | 34 +++++++++++++++++++--------------- ceno_emul/src/tracer.rs | 36 ++++++++++++++++++------------------ 3 files changed, 38 insertions(+), 34 deletions(-) diff --git a/ceno_emul/src/addr.rs b/ceno_emul/src/addr.rs index eb2447cff..2f558f1de 100644 --- a/ceno_emul/src/addr.rs +++ b/ceno_emul/src/addr.rs @@ -20,7 +20,7 @@ pub const WORD_SIZE: usize = 4; // Type aliases to clarify the code without wrapper types. pub type Word = u32; -pub type VMA = u32; +pub type Addr = u32; pub type Cycle = u32; pub type RegIdx = usize; diff --git a/ceno_emul/src/platform.rs b/ceno_emul/src/platform.rs index ce07e3894..73f979127 100644 --- a/ceno_emul/src/platform.rs +++ b/ceno_emul/src/platform.rs @@ -1,6 +1,10 @@ -use crate::addr::{RegIdx, WORD_SIZE}; +use crate::addr::{Addr, RegIdx, WORD_SIZE}; /// The Platform struct holds the parameters of the VM. +/// It defines: +/// - the layout of virtual memory, +/// - special addresses, such as the initial PC, +/// - codes of environment calls. pub struct Platform; pub const CENO_PLATFORM: Platform = Platform; @@ -8,57 +12,57 @@ pub const CENO_PLATFORM: Platform = Platform; impl Platform { // Virtual memory layout. - pub const fn rom_start(&self) -> u32 { + pub const fn rom_start(&self) -> Addr { 0x2000_0000 } - pub const fn rom_end(&self) -> u32 { + pub const fn rom_end(&self) -> Addr { 0x3000_0000 - 1 } - pub fn is_rom(&self, addr: u32) -> bool { + pub fn is_rom(&self, addr: Addr) -> bool { (self.rom_start()..=self.rom_end()).contains(&addr) } - pub const fn ram_start(&self) -> u32 { + pub const fn ram_start(&self) -> Addr { 0x8000_0000 } - pub const fn ram_end(&self) -> u32 { + pub const fn ram_end(&self) -> Addr { 0xFFFF_FFFF } - pub fn is_ram(&self, addr: u32) -> bool { + pub fn is_ram(&self, addr: Addr) -> bool { (self.ram_start()..=self.ram_end()).contains(&addr) } /// Virtual address of a register. - pub const fn register_vma(&self, idx: RegIdx) -> u32 { - (idx * WORD_SIZE) as u32 + pub const fn register_vma(&self, index: RegIdx) -> Addr { + (index * WORD_SIZE) as Addr } /// Virtual address of the program counter. - pub const fn pc_vma(&self) -> u32 { + pub const fn pc_vma(&self) -> Addr { self.register_vma(32) } // Startup. - pub const fn pc_start(&self) -> u32 { + pub const fn pc_start(&self) -> Addr { self.rom_start() } // Permissions. - pub fn can_read(&self, addr: u32) -> bool { + pub fn can_read(&self, addr: Addr) -> bool { self.is_rom(addr) || self.is_ram(addr) } - pub fn can_write(&self, addr: u32) -> bool { + pub fn can_write(&self, addr: Addr) -> bool { self.is_ram(addr) } - pub fn can_execute(&self, addr: u32) -> bool { + pub fn can_execute(&self, addr: Addr) -> bool { self.is_rom(addr) } @@ -99,7 +103,7 @@ mod tests { assert!(!p.is_ram(p.rom_start())); assert!(!p.is_ram(p.rom_end())); // Registers do not overlap with ROM or RAM. - for reg in [p.register_vma(0), p.register_vma(31), p.pc_vma()] { + for reg in [p.pc_vma(), p.register_vma(0), p.register_vma(31)] { assert!(!p.is_rom(reg)); assert!(!p.is_ram(reg)); } diff --git a/ceno_emul/src/tracer.rs b/ceno_emul/src/tracer.rs index ff30e1c78..15123eb71 100644 --- a/ceno_emul/src/tracer.rs +++ b/ceno_emul/src/tracer.rs @@ -1,7 +1,7 @@ use std::{collections::HashMap, fmt, mem}; use crate::{ - addr::{ByteAddr, Cycle, RegIdx, WordAddr}, + addr::{ByteAddr, Cycle, RegIdx, Word, WordAddr}, rv32im::DecodedInstruction, CENO_PLATFORM, }; @@ -9,20 +9,20 @@ use crate::{ /// An instruction and its context in an execution trace. That is concrete values of registers and memory. #[derive(Clone, Debug, Default)] pub struct StepRecord { - cycle: Cycle, + cycle: Cycle, // TODO: start from 1. pc: Change, - insn_code: u32, + insn_code: Word, - rs1: (RegIdx, u32), - rs2: (RegIdx, u32), + rs1: (RegIdx, Word), + rs2: (RegIdx, Word), - rd: (RegIdx, Change), + rd: (RegIdx, Change), - memory_op: (WordAddr, Change), + memory_op: (WordAddr, Change), } impl StepRecord { - pub fn cycle(&self) -> u32 { + pub fn cycle(&self) -> Cycle { self.cycle } @@ -30,7 +30,7 @@ impl StepRecord { self.pc } - pub fn insn_code(&self) -> u32 { + pub fn insn_code(&self) -> Word { self.insn_code } @@ -38,19 +38,19 @@ impl StepRecord { DecodedInstruction::new(self.insn_code) } - pub fn rs1(&self) -> (RegIdx, u32) { + pub fn rs1(&self) -> (RegIdx, Word) { self.rs1 } - pub fn rs2(&self) -> (RegIdx, u32) { + pub fn rs2(&self) -> (RegIdx, Word) { self.rs2 } - pub fn rd(&self) -> (RegIdx, Change) { + pub fn rd(&self) -> (RegIdx, Change) { self.rd } - pub fn memory_op(&self) -> (WordAddr, Change) { + pub fn memory_op(&self) -> (WordAddr, Change) { self.memory_op } } @@ -102,12 +102,12 @@ impl Tracer { self.record.pc.after = pc; } - pub fn fetch(&mut self, pc: WordAddr, value: u32) { + pub fn fetch(&mut self, pc: WordAddr, value: Word) { self.record.pc.before = pc.baddr(); self.record.insn_code = value; } - pub fn load_register(&mut self, idx: usize, value: u32) { + pub fn load_register(&mut self, idx: RegIdx, value: Word) { match (self.actions.rs1_loaded, self.actions.rs2_loaded) { (false, false) => { self.record.rs1 = (idx, value); @@ -121,7 +121,7 @@ impl Tracer { } } - pub fn store_register(&mut self, idx: usize, value: Change) { + pub fn store_register(&mut self, idx: RegIdx, value: Change) { if !self.actions.rd_stored { self.record.rd = (idx, value); self.actions.rd_stored = true; @@ -130,7 +130,7 @@ impl Tracer { } } - pub fn load_memory(&mut self, addr: WordAddr, value: u32) { + pub fn load_memory(&mut self, addr: WordAddr, value: Word) { if self.actions.memory_loaded || self.actions.memory_stored { unimplemented!("Only one memory load is supported"); } @@ -138,7 +138,7 @@ impl Tracer { self.record.memory_op = (addr, Change::new(value, value)); } - pub fn store_memory(&mut self, addr: WordAddr, value: Change) { + pub fn store_memory(&mut self, addr: WordAddr, value: Change) { if self.actions.memory_stored { unimplemented!("Only one memory store is supported"); } From 9a7585289ff631896e6a90f7fd3568ae2c0cd7a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Nicolas?= Date: Tue, 3 Sep 2024 19:52:36 +0200 Subject: [PATCH 04/13] emul-trace-mem: record the cycle of the previous operation --- ceno_emul/src/tracer.rs | 61 +++++++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 20 deletions(-) diff --git a/ceno_emul/src/tracer.rs b/ceno_emul/src/tracer.rs index 15123eb71..2ca1da8ea 100644 --- a/ceno_emul/src/tracer.rs +++ b/ceno_emul/src/tracer.rs @@ -3,22 +3,22 @@ use std::{collections::HashMap, fmt, mem}; use crate::{ addr::{ByteAddr, Cycle, RegIdx, Word, WordAddr}, rv32im::DecodedInstruction, - CENO_PLATFORM, + Addr, CENO_PLATFORM, }; /// An instruction and its context in an execution trace. That is concrete values of registers and memory. #[derive(Clone, Debug, Default)] pub struct StepRecord { - cycle: Cycle, // TODO: start from 1. + cycle: Cycle, pc: Change, insn_code: Word, - rs1: (RegIdx, Word), - rs2: (RegIdx, Word), + rs1: (RegIdx, Word, Cycle), + rs2: (RegIdx, Word, Cycle), - rd: (RegIdx, Change), + rd: (RegIdx, Change, Cycle), - memory_op: (WordAddr, Change), + memory_op: (WordAddr, Change, Cycle), } impl StepRecord { @@ -38,19 +38,19 @@ impl StepRecord { DecodedInstruction::new(self.insn_code) } - pub fn rs1(&self) -> (RegIdx, Word) { + pub fn rs1(&self) -> (RegIdx, Word, Cycle) { self.rs1 } - pub fn rs2(&self) -> (RegIdx, Word) { + pub fn rs2(&self) -> (RegIdx, Word, Cycle) { self.rs2 } - pub fn rd(&self) -> (RegIdx, Change) { + pub fn rd(&self) -> (RegIdx, Change, Cycle) { self.rd } - pub fn memory_op(&self) -> (WordAddr, Change) { + pub fn memory_op(&self) -> (WordAddr, Change, Cycle) { self.memory_op } } @@ -69,10 +69,16 @@ pub struct Tracer { record: StepRecord, actions: StepActions, - previous_mem_op: HashMap, + previous_mem_op: HashMap, } impl Tracer { + pub fn new() -> Tracer { + let mut t = Tracer::default(); + t.record.cycle = 1; + t + } + pub fn advance(&mut self) -> StepRecord { // Reset and advance to the next cycle. let actions = mem::take(&mut self.actions); @@ -83,16 +89,16 @@ impl Tracer { let mut track_mem = |vma| self.previous_mem_op.insert(vma, record.cycle); if actions.rs1_loaded { - track_mem(CENO_PLATFORM.register_vma(record.rs1.0).into()); + track_mem(CENO_PLATFORM.register_vma(record.rs1.0)); } if actions.rs2_loaded { - track_mem(CENO_PLATFORM.register_vma(record.rs2.0).into()); + track_mem(CENO_PLATFORM.register_vma(record.rs2.0)); } if actions.rd_stored { - track_mem(CENO_PLATFORM.register_vma(record.rd.0).into()); + track_mem(CENO_PLATFORM.register_vma(record.rd.0)); } if actions.memory_loaded || actions.memory_stored { - track_mem(record.memory_op.0); + track_mem(record.memory_op.0.into()); } record @@ -108,13 +114,16 @@ impl Tracer { } pub fn load_register(&mut self, idx: RegIdx, value: Word) { + let vma = CENO_PLATFORM.register_vma(idx); + let prev_cycle = self.previous_mem_op(vma); + match (self.actions.rs1_loaded, self.actions.rs2_loaded) { (false, false) => { - self.record.rs1 = (idx, value); + self.record.rs1 = (idx, value, prev_cycle); self.actions.rs1_loaded = true; } (true, false) => { - self.record.rs2 = (idx, value); + self.record.rs2 = (idx, value, prev_cycle); self.actions.rs2_loaded = true; } _ => unimplemented!("Only two register reads are supported"), @@ -123,8 +132,12 @@ impl Tracer { pub fn store_register(&mut self, idx: RegIdx, value: Change) { if !self.actions.rd_stored { - self.record.rd = (idx, value); self.actions.rd_stored = true; + + let vma = CENO_PLATFORM.register_vma(idx); + let prev_cycle = self.previous_mem_op(vma); + + self.record.rd = (idx, value, prev_cycle); } else { unimplemented!("Only one register write is supported"); } @@ -135,7 +148,9 @@ impl Tracer { unimplemented!("Only one memory load is supported"); } self.actions.memory_loaded = true; - self.record.memory_op = (addr, Change::new(value, value)); + + let prev_cycle = self.previous_mem_op(addr.into()); + self.record.memory_op = (addr, Change::new(value, value), prev_cycle); } pub fn store_memory(&mut self, addr: WordAddr, value: Change) { @@ -143,7 +158,13 @@ impl Tracer { unimplemented!("Only one memory store is supported"); } self.actions.memory_stored = true; - self.record.memory_op = (addr, value); + + let prev_cycle = self.previous_mem_op(addr.into()); + self.record.memory_op = (addr, value, prev_cycle); + } + + fn previous_mem_op(&self, addr: Addr) -> Cycle { + *self.previous_mem_op.get(&addr).unwrap_or(&0) } } From 3705de513ac9fc4bdbb61a2733fb7f51af093e3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Nicolas?= Date: Tue, 3 Sep 2024 21:39:56 +0200 Subject: [PATCH 05/13] emul-trace-mem: doc --- ceno_emul/src/tracer.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/ceno_emul/src/tracer.rs b/ceno_emul/src/tracer.rs index 2ca1da8ea..573ea453f 100644 --- a/ceno_emul/src/tracer.rs +++ b/ceno_emul/src/tracer.rs @@ -7,6 +7,12 @@ use crate::{ }; /// An instruction and its context in an execution trace. That is concrete values of registers and memory. +/// +/// - Registers are assigned a VMA (virtual memory address, u32). This way they can be unified with the RAM check. +/// - It is possible that the `rs1 / rs2 / rd` **be the same**. Then, they point to the **same previous cycle**. The circuits need to handle this case. +/// - Any of `rs1 / rs2 / rd` may be `x0`. The trace handles this like any register, including the value that was _supposed_ to be stored. The circuits must handle this case, either by storing 0 or by skipping x0 operations. +/// - `cycle = 0` means initialization; that is all the special startup logic we are going to have. The RISC-V program starts at `cycle = 1`. +/// - We assume that the PC was written at `cycle - 1` so we don’t store this. #[derive(Clone, Debug, Default)] pub struct StepRecord { cycle: Cycle, @@ -69,7 +75,7 @@ pub struct Tracer { record: StepRecord, actions: StepActions, - previous_mem_op: HashMap, + latest_accesses: HashMap, } impl Tracer { @@ -86,7 +92,7 @@ impl Tracer { self.record.cycle = record.cycle + 1; // Track this step as the origin of its memory accesses. - let mut track_mem = |vma| self.previous_mem_op.insert(vma, record.cycle); + let mut track_mem = |vma| self.latest_accesses.insert(vma, record.cycle); if actions.rs1_loaded { track_mem(CENO_PLATFORM.register_vma(record.rs1.0)); @@ -115,7 +121,7 @@ impl Tracer { pub fn load_register(&mut self, idx: RegIdx, value: Word) { let vma = CENO_PLATFORM.register_vma(idx); - let prev_cycle = self.previous_mem_op(vma); + let prev_cycle = self.latest_accesses(vma); match (self.actions.rs1_loaded, self.actions.rs2_loaded) { (false, false) => { @@ -135,7 +141,7 @@ impl Tracer { self.actions.rd_stored = true; let vma = CENO_PLATFORM.register_vma(idx); - let prev_cycle = self.previous_mem_op(vma); + let prev_cycle = self.latest_accesses(vma); self.record.rd = (idx, value, prev_cycle); } else { @@ -149,7 +155,7 @@ impl Tracer { } self.actions.memory_loaded = true; - let prev_cycle = self.previous_mem_op(addr.into()); + let prev_cycle = self.latest_accesses(addr.into()); self.record.memory_op = (addr, Change::new(value, value), prev_cycle); } @@ -159,12 +165,12 @@ impl Tracer { } self.actions.memory_stored = true; - let prev_cycle = self.previous_mem_op(addr.into()); + let prev_cycle = self.latest_accesses(addr.into()); self.record.memory_op = (addr, value, prev_cycle); } - fn previous_mem_op(&self, addr: Addr) -> Cycle { - *self.previous_mem_op.get(&addr).unwrap_or(&0) + fn latest_accesses(&self, addr: Addr) -> Cycle { + *self.latest_accesses.get(&addr).unwrap_or(&0) } } From 0f50c2da1bc0c7ead2086c51b4c013f7f476e5f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Nicolas?= Date: Tue, 3 Sep 2024 22:17:17 +0200 Subject: [PATCH 06/13] emul-trace-mem: refactor using Option --- ceno_emul/src/rv32im.rs | 5 ++- ceno_emul/src/tracer.rs | 76 +++++++++++++++------------------------ ceno_emul/src/vm_state.rs | 10 +++--- 3 files changed, 38 insertions(+), 53 deletions(-) diff --git a/ceno_emul/src/rv32im.rs b/ceno_emul/src/rv32im.rs index 0e69877e8..efcde7bc1 100644 --- a/ceno_emul/src/rv32im.rs +++ b/ceno_emul/src/rv32im.rs @@ -53,6 +53,9 @@ pub trait EmuContext { // Store to memory fn store_memory(&mut self, addr: WordAddr, data: u32) -> Result<()>; + // Get the value of a memory word without side-effects. + fn peek_memory(&self, addr: WordAddr) -> u32; + // Load from memory, in the context of instruction fetching. // Only called after check_insn_load returns true. fn fetch(&mut self, pc: WordAddr) -> Result { @@ -625,7 +628,7 @@ impl Emulator { if !ctx.check_data_store(addr) { return ctx.trap(TrapCause::StoreAccessFault); } - let mut data = ctx.load_memory(addr.waddr())?; + let mut data = ctx.peek_memory(addr.waddr()); match kind { InsnKind::SB => { data ^= data & (0xff << shift); diff --git a/ceno_emul/src/tracer.rs b/ceno_emul/src/tracer.rs index 573ea453f..496179afa 100644 --- a/ceno_emul/src/tracer.rs +++ b/ceno_emul/src/tracer.rs @@ -19,12 +19,12 @@ pub struct StepRecord { pc: Change, insn_code: Word, - rs1: (RegIdx, Word, Cycle), - rs2: (RegIdx, Word, Cycle), + rs1: Option<(RegIdx, Word, Cycle)>, + rs2: Option<(RegIdx, Word, Cycle)>, - rd: (RegIdx, Change, Cycle), + rd: Option<(RegIdx, Change, Cycle)>, - memory_op: (WordAddr, Change, Cycle), + memory_op: Option<(WordAddr, Change, Cycle)>, } impl StepRecord { @@ -44,36 +44,26 @@ impl StepRecord { DecodedInstruction::new(self.insn_code) } - pub fn rs1(&self) -> (RegIdx, Word, Cycle) { + pub fn rs1(&self) -> Option<(RegIdx, Word, Cycle)> { self.rs1 } - pub fn rs2(&self) -> (RegIdx, Word, Cycle) { + pub fn rs2(&self) -> Option<(RegIdx, Word, Cycle)> { self.rs2 } - pub fn rd(&self) -> (RegIdx, Change, Cycle) { + pub fn rd(&self) -> Option<(RegIdx, Change, Cycle)> { self.rd } - pub fn memory_op(&self) -> (WordAddr, Change, Cycle) { + pub fn memory_op(&self) -> Option<(WordAddr, Change, Cycle)> { self.memory_op } } -#[derive(Debug, Default)] -struct StepActions { - rs1_loaded: bool, - rs2_loaded: bool, - rd_stored: bool, - memory_loaded: bool, - memory_stored: bool, -} - #[derive(Debug, Default)] pub struct Tracer { record: StepRecord, - actions: StepActions, latest_accesses: HashMap, } @@ -87,24 +77,23 @@ impl Tracer { pub fn advance(&mut self) -> StepRecord { // Reset and advance to the next cycle. - let actions = mem::take(&mut self.actions); let record = mem::take(&mut self.record); self.record.cycle = record.cycle + 1; // Track this step as the origin of its memory accesses. let mut track_mem = |vma| self.latest_accesses.insert(vma, record.cycle); - if actions.rs1_loaded { - track_mem(CENO_PLATFORM.register_vma(record.rs1.0)); + if let Some((idx, _, _)) = record.rs1 { + track_mem(CENO_PLATFORM.register_vma(idx)); } - if actions.rs2_loaded { - track_mem(CENO_PLATFORM.register_vma(record.rs2.0)); + if let Some((idx, _, _)) = record.rs2 { + track_mem(CENO_PLATFORM.register_vma(idx)); } - if actions.rd_stored { - track_mem(CENO_PLATFORM.register_vma(record.rd.0)); + if let Some((idx, _, _)) = record.rd { + track_mem(CENO_PLATFORM.register_vma(idx)); } - if actions.memory_loaded || actions.memory_stored { - track_mem(record.memory_op.0.into()); + if let Some((addr, _, _)) = record.memory_op { + track_mem(addr.into()); } record @@ -123,50 +112,43 @@ impl Tracer { let vma = CENO_PLATFORM.register_vma(idx); let prev_cycle = self.latest_accesses(vma); - match (self.actions.rs1_loaded, self.actions.rs2_loaded) { - (false, false) => { - self.record.rs1 = (idx, value, prev_cycle); - self.actions.rs1_loaded = true; + match (self.record.rs1, self.record.rs2) { + (None, None) => { + self.record.rs1 = Some((idx, value, prev_cycle)); } - (true, false) => { - self.record.rs2 = (idx, value, prev_cycle); - self.actions.rs2_loaded = true; + (Some(_), None) => { + self.record.rs2 = Some((idx, value, prev_cycle)); } _ => unimplemented!("Only two register reads are supported"), } } pub fn store_register(&mut self, idx: RegIdx, value: Change) { - if !self.actions.rd_stored { - self.actions.rd_stored = true; - + if self.record.rd.is_none() { let vma = CENO_PLATFORM.register_vma(idx); let prev_cycle = self.latest_accesses(vma); - - self.record.rd = (idx, value, prev_cycle); + self.record.rd = Some((idx, value, prev_cycle)); } else { unimplemented!("Only one register write is supported"); } } pub fn load_memory(&mut self, addr: WordAddr, value: Word) { - if self.actions.memory_loaded || self.actions.memory_stored { - unimplemented!("Only one memory load is supported"); + if self.record.memory_op.is_some() { + unimplemented!("Only one memory access is supported"); } - self.actions.memory_loaded = true; let prev_cycle = self.latest_accesses(addr.into()); - self.record.memory_op = (addr, Change::new(value, value), prev_cycle); + self.record.memory_op = Some((addr, Change::new(value, value), prev_cycle)); } pub fn store_memory(&mut self, addr: WordAddr, value: Change) { - if self.actions.memory_stored { - unimplemented!("Only one memory store is supported"); + if self.record.memory_op.is_some() { + unimplemented!("Only one memory access is supported"); } - self.actions.memory_stored = true; let prev_cycle = self.latest_accesses(addr.into()); - self.record.memory_op = (addr, value, prev_cycle); + self.record.memory_op = Some((addr, value, prev_cycle)); } fn latest_accesses(&self, addr: Addr) -> Cycle { diff --git a/ceno_emul/src/vm_state.rs b/ceno_emul/src/vm_state.rs index 3996f2ad3..72aa3a564 100644 --- a/ceno_emul/src/vm_state.rs +++ b/ceno_emul/src/vm_state.rs @@ -48,11 +48,6 @@ impl VMState { self.registers[idx] } - /// Get the value of a memory word without side-effects. - pub fn peek_memory(&self, addr: WordAddr) -> u32 { - *self.memory.get(&addr.0).unwrap_or(&0) - } - /// Set a word in memory without side-effects. pub fn init_memory(&mut self, addr: WordAddr, value: u32) { self.memory.insert(addr.0, value); @@ -146,6 +141,11 @@ impl EmuContext for VMState { Ok(()) } + /// Get the value of a memory word without side-effects. + fn peek_memory(&self, addr: WordAddr) -> u32 { + *self.memory.get(&addr.0).unwrap_or(&0) + } + fn fetch(&mut self, pc: WordAddr) -> Result { let value = self.peek_memory(pc); self.tracer.fetch(pc, value); From f2e62700a2296093f80ced56e31bd65b9631861b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Nicolas?= Date: Tue, 3 Sep 2024 22:50:47 +0200 Subject: [PATCH 07/13] emul-trace-mem: Unify address type --- ceno_emul/src/addr.rs | 2 +- ceno_emul/src/tracer.rs | 52 +++++++++++++++++++-------------------- ceno_emul/src/vm_state.rs | 8 +++--- 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/ceno_emul/src/addr.rs b/ceno_emul/src/addr.rs index 2f558f1de..f41d004e2 100644 --- a/ceno_emul/src/addr.rs +++ b/ceno_emul/src/addr.rs @@ -28,7 +28,7 @@ pub type RegIdx = usize; pub struct ByteAddr(pub u32); #[derive(Clone, Copy, Default, PartialEq, Eq, Hash)] -pub struct WordAddr(pub u32); +pub struct WordAddr(u32); impl From for WordAddr { fn from(addr: ByteAddr) -> Self { diff --git a/ceno_emul/src/tracer.rs b/ceno_emul/src/tracer.rs index 496179afa..9ea4255f3 100644 --- a/ceno_emul/src/tracer.rs +++ b/ceno_emul/src/tracer.rs @@ -3,7 +3,7 @@ use std::{collections::HashMap, fmt, mem}; use crate::{ addr::{ByteAddr, Cycle, RegIdx, Word, WordAddr}, rv32im::DecodedInstruction, - Addr, CENO_PLATFORM, + CENO_PLATFORM, }; /// An instruction and its context in an execution trace. That is concrete values of registers and memory. @@ -19,10 +19,10 @@ pub struct StepRecord { pc: Change, insn_code: Word, - rs1: Option<(RegIdx, Word, Cycle)>, - rs2: Option<(RegIdx, Word, Cycle)>, + rs1: Option<(WordAddr, Word, Cycle)>, + rs2: Option<(WordAddr, Word, Cycle)>, - rd: Option<(RegIdx, Change, Cycle)>, + rd: Option<(WordAddr, Change, Cycle)>, memory_op: Option<(WordAddr, Change, Cycle)>, } @@ -44,15 +44,15 @@ impl StepRecord { DecodedInstruction::new(self.insn_code) } - pub fn rs1(&self) -> Option<(RegIdx, Word, Cycle)> { + pub fn rs1(&self) -> Option<(WordAddr, Word, Cycle)> { self.rs1 } - pub fn rs2(&self) -> Option<(RegIdx, Word, Cycle)> { + pub fn rs2(&self) -> Option<(WordAddr, Word, Cycle)> { self.rs2 } - pub fn rd(&self) -> Option<(RegIdx, Change, Cycle)> { + pub fn rd(&self) -> Option<(WordAddr, Change, Cycle)> { self.rd } @@ -65,7 +65,7 @@ impl StepRecord { pub struct Tracer { record: StepRecord, - latest_accesses: HashMap, + latest_accesses: HashMap, } impl Tracer { @@ -81,19 +81,19 @@ impl Tracer { self.record.cycle = record.cycle + 1; // Track this step as the origin of its memory accesses. - let mut track_mem = |vma| self.latest_accesses.insert(vma, record.cycle); + let mut track_mem = |addr| self.latest_accesses.insert(addr, record.cycle); - if let Some((idx, _, _)) = record.rs1 { - track_mem(CENO_PLATFORM.register_vma(idx)); + if let Some((addr, _, _)) = record.rs1 { + track_mem(addr); } - if let Some((idx, _, _)) = record.rs2 { - track_mem(CENO_PLATFORM.register_vma(idx)); + if let Some((addr, _, _)) = record.rs2 { + track_mem(addr); } - if let Some((idx, _, _)) = record.rd { - track_mem(CENO_PLATFORM.register_vma(idx)); + if let Some((addr, _, _)) = record.rd { + track_mem(addr); } if let Some((addr, _, _)) = record.memory_op { - track_mem(addr.into()); + track_mem(addr); } record @@ -109,15 +109,15 @@ impl Tracer { } pub fn load_register(&mut self, idx: RegIdx, value: Word) { - let vma = CENO_PLATFORM.register_vma(idx); - let prev_cycle = self.latest_accesses(vma); + let addr = CENO_PLATFORM.register_vma(idx).into(); + let prev_cycle = self.latest_accesses(addr); match (self.record.rs1, self.record.rs2) { (None, None) => { - self.record.rs1 = Some((idx, value, prev_cycle)); + self.record.rs1 = Some((addr, value, prev_cycle)); } (Some(_), None) => { - self.record.rs2 = Some((idx, value, prev_cycle)); + self.record.rs2 = Some((addr, value, prev_cycle)); } _ => unimplemented!("Only two register reads are supported"), } @@ -125,9 +125,9 @@ impl Tracer { pub fn store_register(&mut self, idx: RegIdx, value: Change) { if self.record.rd.is_none() { - let vma = CENO_PLATFORM.register_vma(idx); - let prev_cycle = self.latest_accesses(vma); - self.record.rd = Some((idx, value, prev_cycle)); + let addr = CENO_PLATFORM.register_vma(idx).into(); + let prev_cycle = self.latest_accesses(addr); + self.record.rd = Some((addr, value, prev_cycle)); } else { unimplemented!("Only one register write is supported"); } @@ -138,7 +138,7 @@ impl Tracer { unimplemented!("Only one memory access is supported"); } - let prev_cycle = self.latest_accesses(addr.into()); + let prev_cycle = self.latest_accesses(addr); self.record.memory_op = Some((addr, Change::new(value, value), prev_cycle)); } @@ -147,11 +147,11 @@ impl Tracer { unimplemented!("Only one memory access is supported"); } - let prev_cycle = self.latest_accesses(addr.into()); + let prev_cycle = self.latest_accesses(addr); self.record.memory_op = Some((addr, value, prev_cycle)); } - fn latest_accesses(&self, addr: Addr) -> Cycle { + fn latest_accesses(&self, addr: WordAddr) -> Cycle { *self.latest_accesses.get(&addr).unwrap_or(&0) } } diff --git a/ceno_emul/src/vm_state.rs b/ceno_emul/src/vm_state.rs index 72aa3a564..d9d1c22aa 100644 --- a/ceno_emul/src/vm_state.rs +++ b/ceno_emul/src/vm_state.rs @@ -15,7 +15,7 @@ pub struct VMState { platform: Platform, pc: u32, /// Map a word-address (addr/4) to a word. - memory: HashMap, + memory: HashMap, registers: [u32; 32], // Termination. succeeded: bool, @@ -50,7 +50,7 @@ impl VMState { /// Set a word in memory without side-effects. pub fn init_memory(&mut self, addr: WordAddr, value: u32) { - self.memory.insert(addr.0, value); + self.memory.insert(addr, value); } pub fn iter_until_success(&mut self) -> impl Iterator> + '_ { @@ -137,13 +137,13 @@ impl EmuContext for VMState { fn store_memory(&mut self, addr: WordAddr, after: u32) -> Result<()> { let before = self.peek_memory(addr); self.tracer.store_memory(addr, Change { after, before }); - self.memory.insert(addr.0, after); + self.memory.insert(addr, after); Ok(()) } /// Get the value of a memory word without side-effects. fn peek_memory(&self, addr: WordAddr) -> u32 { - *self.memory.get(&addr.0).unwrap_or(&0) + *self.memory.get(&addr).unwrap_or(&0) } fn fetch(&mut self, pc: WordAddr) -> Result { From 22a9ab88d55fb980ad3a2116a085de47e37a6159 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Nicolas?= Date: Tue, 3 Sep 2024 23:19:21 +0200 Subject: [PATCH 08/13] emul-trace-mem: refactor with a MemOp type --- ceno_emul/src/lib.rs | 2 +- ceno_emul/src/tracer.rs | 89 +++++++++++++++++++++++++++-------------- 2 files changed, 61 insertions(+), 30 deletions(-) diff --git a/ceno_emul/src/lib.rs b/ceno_emul/src/lib.rs index 5a48668d6..8c7aaadb6 100644 --- a/ceno_emul/src/lib.rs +++ b/ceno_emul/src/lib.rs @@ -5,7 +5,7 @@ mod platform; pub use platform::{Platform, CENO_PLATFORM}; mod tracer; -pub use tracer::{Change, StepRecord}; +pub use tracer::{Change, MemOp, ReadOp, StepRecord, WriteOp}; mod vm_state; pub use vm_state::VMState; diff --git a/ceno_emul/src/tracer.rs b/ceno_emul/src/tracer.rs index 9ea4255f3..f566c39fc 100644 --- a/ceno_emul/src/tracer.rs +++ b/ceno_emul/src/tracer.rs @@ -19,14 +19,28 @@ pub struct StepRecord { pc: Change, insn_code: Word, - rs1: Option<(WordAddr, Word, Cycle)>, - rs2: Option<(WordAddr, Word, Cycle)>, + rs1: Option, + rs2: Option, - rd: Option<(WordAddr, Change, Cycle)>, + rd: Option, - memory_op: Option<(WordAddr, Change, Cycle)>, + memory_op: Option, } +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct MemOp { + /// Virtual Memory Address. + /// For registers, get it from `CENO_PLATFORM.register_vma(idx)`. + pub addr: WordAddr, + /// The Word read, or the Change to be written. + pub value: T, + /// The cycle when this memory address was last accessed before this operation. + pub previous_cycle: Cycle, +} + +pub type ReadOp = MemOp; +pub type WriteOp = MemOp>; + impl StepRecord { pub fn cycle(&self) -> Cycle { self.cycle @@ -44,20 +58,20 @@ impl StepRecord { DecodedInstruction::new(self.insn_code) } - pub fn rs1(&self) -> Option<(WordAddr, Word, Cycle)> { - self.rs1 + pub fn rs1(&self) -> Option { + self.rs1.clone() } - pub fn rs2(&self) -> Option<(WordAddr, Word, Cycle)> { - self.rs2 + pub fn rs2(&self) -> Option { + self.rs2.clone() } - pub fn rd(&self) -> Option<(WordAddr, Change, Cycle)> { - self.rd + pub fn rd(&self) -> Option { + self.rd.clone() } - pub fn memory_op(&self) -> Option<(WordAddr, Change, Cycle)> { - self.memory_op + pub fn memory_op(&self) -> Option { + self.memory_op.clone() } } @@ -83,16 +97,16 @@ impl Tracer { // Track this step as the origin of its memory accesses. let mut track_mem = |addr| self.latest_accesses.insert(addr, record.cycle); - if let Some((addr, _, _)) = record.rs1 { + if let Some(ReadOp { addr, .. }) = record.rs1 { track_mem(addr); } - if let Some((addr, _, _)) = record.rs2 { + if let Some(ReadOp { addr, .. }) = record.rs2 { track_mem(addr); } - if let Some((addr, _, _)) = record.rd { + if let Some(WriteOp { addr, .. }) = record.rd { track_mem(addr); } - if let Some((addr, _, _)) = record.memory_op { + if let Some(WriteOp { addr, .. }) = record.memory_op { track_mem(addr); } @@ -110,27 +124,38 @@ impl Tracer { pub fn load_register(&mut self, idx: RegIdx, value: Word) { let addr = CENO_PLATFORM.register_vma(idx).into(); - let prev_cycle = self.latest_accesses(addr); + let previous_cycle = self.latest_accesses(addr); - match (self.record.rs1, self.record.rs2) { + match (&self.record.rs1, &self.record.rs2) { (None, None) => { - self.record.rs1 = Some((addr, value, prev_cycle)); + self.record.rs1 = Some(ReadOp { + addr, + value, + previous_cycle, + }); } (Some(_), None) => { - self.record.rs2 = Some((addr, value, prev_cycle)); + self.record.rs2 = Some(ReadOp { + addr, + value, + previous_cycle, + }); } _ => unimplemented!("Only two register reads are supported"), } } pub fn store_register(&mut self, idx: RegIdx, value: Change) { - if self.record.rd.is_none() { - let addr = CENO_PLATFORM.register_vma(idx).into(); - let prev_cycle = self.latest_accesses(addr); - self.record.rd = Some((addr, value, prev_cycle)); - } else { + if self.record.rd.is_some() { unimplemented!("Only one register write is supported"); } + + let addr = CENO_PLATFORM.register_vma(idx).into(); + self.record.rd = Some(WriteOp { + addr, + value, + previous_cycle: self.latest_accesses(addr), + }); } pub fn load_memory(&mut self, addr: WordAddr, value: Word) { @@ -138,8 +163,11 @@ impl Tracer { unimplemented!("Only one memory access is supported"); } - let prev_cycle = self.latest_accesses(addr); - self.record.memory_op = Some((addr, Change::new(value, value), prev_cycle)); + self.record.memory_op = Some(WriteOp { + addr, + value: Change::new(value, value), + previous_cycle: self.latest_accesses(addr), + }); } pub fn store_memory(&mut self, addr: WordAddr, value: Change) { @@ -147,8 +175,11 @@ impl Tracer { unimplemented!("Only one memory access is supported"); } - let prev_cycle = self.latest_accesses(addr); - self.record.memory_op = Some((addr, value, prev_cycle)); + self.record.memory_op = Some(WriteOp { + addr, + value, + previous_cycle: self.latest_accesses(addr), + }); } fn latest_accesses(&self, addr: WordAddr) -> Cycle { From 4c76c87fd1622e7f0b37f59c08105011430db426 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Nicolas?= Date: Wed, 4 Sep 2024 11:14:57 +0200 Subject: [PATCH 09/13] emul-trace-mem: split instructions over subcycles --- ceno_emul/src/addr.rs | 2 +- ceno_emul/src/tracer.rs | 90 +++++++++++++++++++-------------------- ceno_emul/src/vm_state.rs | 2 +- 3 files changed, 47 insertions(+), 47 deletions(-) diff --git a/ceno_emul/src/addr.rs b/ceno_emul/src/addr.rs index f41d004e2..0a8de8ad9 100644 --- a/ceno_emul/src/addr.rs +++ b/ceno_emul/src/addr.rs @@ -21,7 +21,7 @@ pub const WORD_SIZE: usize = 4; // Type aliases to clarify the code without wrapper types. pub type Word = u32; pub type Addr = u32; -pub type Cycle = u32; +pub type Cycle = u64; pub type RegIdx = usize; #[derive(Clone, Copy, Default, PartialEq, Eq, Hash)] diff --git a/ceno_emul/src/tracer.rs b/ceno_emul/src/tracer.rs index f566c39fc..84c79d464 100644 --- a/ceno_emul/src/tracer.rs +++ b/ceno_emul/src/tracer.rs @@ -8,11 +8,15 @@ use crate::{ /// An instruction and its context in an execution trace. That is concrete values of registers and memory. /// -/// - Registers are assigned a VMA (virtual memory address, u32). This way they can be unified with the RAM check. -/// - It is possible that the `rs1 / rs2 / rd` **be the same**. Then, they point to the **same previous cycle**. The circuits need to handle this case. -/// - Any of `rs1 / rs2 / rd` may be `x0`. The trace handles this like any register, including the value that was _supposed_ to be stored. The circuits must handle this case, either by storing 0 or by skipping x0 operations. -/// - `cycle = 0` means initialization; that is all the special startup logic we are going to have. The RISC-V program starts at `cycle = 1`. -/// - We assume that the PC was written at `cycle - 1` so we don’t store this. +/// - Each instruction is divided into 4 subcycles with the operations on: rs1, rs2, rd, memory. Each op is assigned a unique `cycle + subcycle`. +/// +/// - `cycle = 0` means initialization; that is all the special startup logic we are going to have. The RISC-V program starts at `cycle = 4` and each instruction increments `cycle += 4`. +/// +/// - Registers are assigned a VMA (virtual memory address, u32). This way they can be unified with other kinds of memory ops. +/// +/// - Any of `rs1 / rs2 / rd` **may be `x0`**. The trace handles this like any register, including the value that was _supposed_ to be stored. The circuits must handle this case: either **store `0` or skip `x0` operations**. +/// +/// - Any pair of `rs1 / rs2 / rd` **may be the same**. Then, one op will point to the other op in the same instruction but a different subcycle. The circuits may follow the operations **without special handling** of repeated registers. #[derive(Clone, Debug, Default)] pub struct StepRecord { cycle: Cycle, @@ -75,7 +79,7 @@ impl StepRecord { } } -#[derive(Debug, Default)] +#[derive(Debug)] pub struct Tracer { record: StepRecord, @@ -83,34 +87,33 @@ pub struct Tracer { } impl Tracer { + pub const SUBCYCLE_RS1: Cycle = 0; + pub const SUBCYCLE_RS2: Cycle = 1; + pub const SUBCYCLE_RD: Cycle = 2; + pub const SUBCYCLE_MEM: Cycle = 3; + pub const SUBCYCLES_PER_INSN: Cycle = 4; + pub fn new() -> Tracer { - let mut t = Tracer::default(); - t.record.cycle = 1; - t + Tracer { + record: StepRecord { + cycle: Self::SUBCYCLES_PER_INSN, + ..StepRecord::default() + }, + latest_accesses: HashMap::new(), + } } pub fn advance(&mut self) -> StepRecord { // Reset and advance to the next cycle. - let record = mem::take(&mut self.record); - self.record.cycle = record.cycle + 1; - - // Track this step as the origin of its memory accesses. - let mut track_mem = |addr| self.latest_accesses.insert(addr, record.cycle); - - if let Some(ReadOp { addr, .. }) = record.rs1 { - track_mem(addr); - } - if let Some(ReadOp { addr, .. }) = record.rs2 { - track_mem(addr); - } - if let Some(WriteOp { addr, .. }) = record.rd { - track_mem(addr); - } - if let Some(WriteOp { addr, .. }) = record.memory_op { - track_mem(addr); - } - - record + let next_cycle = self.record.cycle + Self::SUBCYCLES_PER_INSN; + let complete_step = mem::replace( + &mut self.record, + StepRecord { + cycle: next_cycle, + ..StepRecord::default() + }, + ); + complete_step } pub fn store_pc(&mut self, pc: ByteAddr) { @@ -124,21 +127,20 @@ impl Tracer { pub fn load_register(&mut self, idx: RegIdx, value: Word) { let addr = CENO_PLATFORM.register_vma(idx).into(); - let previous_cycle = self.latest_accesses(addr); match (&self.record.rs1, &self.record.rs2) { (None, None) => { self.record.rs1 = Some(ReadOp { addr, value, - previous_cycle, + previous_cycle: self.track_access(addr, Self::SUBCYCLE_RS1), }); } (Some(_), None) => { self.record.rs2 = Some(ReadOp { addr, value, - previous_cycle, + previous_cycle: self.track_access(addr, Self::SUBCYCLE_RS2), }); } _ => unimplemented!("Only two register reads are supported"), @@ -154,20 +156,12 @@ impl Tracer { self.record.rd = Some(WriteOp { addr, value, - previous_cycle: self.latest_accesses(addr), + previous_cycle: self.track_access(addr, Self::SUBCYCLE_RD), }); } pub fn load_memory(&mut self, addr: WordAddr, value: Word) { - if self.record.memory_op.is_some() { - unimplemented!("Only one memory access is supported"); - } - - self.record.memory_op = Some(WriteOp { - addr, - value: Change::new(value, value), - previous_cycle: self.latest_accesses(addr), - }); + self.store_memory(addr, Change::new(value, value)); } pub fn store_memory(&mut self, addr: WordAddr, value: Change) { @@ -178,12 +172,18 @@ impl Tracer { self.record.memory_op = Some(WriteOp { addr, value, - previous_cycle: self.latest_accesses(addr), + previous_cycle: self.track_access(addr, Self::SUBCYCLE_MEM), }); } - fn latest_accesses(&self, addr: WordAddr) -> Cycle { - *self.latest_accesses.get(&addr).unwrap_or(&0) + /// - Return the cycle when an address was last accessed. + /// - Return 0 if this is the first access. + /// - Record the current instruction as the origin of the latest access. + /// - Accesses within the same instruction are distinguished by `subcycle ∈ [0, 3]`. + fn track_access(&mut self, addr: WordAddr, subcycle: Cycle) -> Cycle { + self.latest_accesses + .insert(addr, self.record.cycle + subcycle) + .unwrap_or(0) } } diff --git a/ceno_emul/src/vm_state.rs b/ceno_emul/src/vm_state.rs index d9d1c22aa..f7a00d0ea 100644 --- a/ceno_emul/src/vm_state.rs +++ b/ceno_emul/src/vm_state.rs @@ -31,7 +31,7 @@ impl VMState { memory: HashMap::new(), registers: [0; 32], succeeded: false, - tracer: Default::default(), + tracer: Tracer::new(), } } From 15a949a547f70f0891975e49f5b1b86594ec3b11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Nicolas?= Date: Wed, 4 Sep 2024 11:42:41 +0200 Subject: [PATCH 10/13] emul-trace-mem: use Word alias more --- ceno_emul/src/lib.rs | 2 +- ceno_emul/src/rv32im.rs | 17 ++++++++++------- ceno_emul/src/vm_state.rs | 32 ++++++++++++++++---------------- ceno_emul/tests/test_vm_trace.rs | 2 +- 4 files changed, 28 insertions(+), 25 deletions(-) diff --git a/ceno_emul/src/lib.rs b/ceno_emul/src/lib.rs index 8c7aaadb6..9db5d318a 100644 --- a/ceno_emul/src/lib.rs +++ b/ceno_emul/src/lib.rs @@ -11,4 +11,4 @@ mod vm_state; pub use vm_state::VMState; mod rv32im; -pub use rv32im::{DecodedInstruction, InsnCategory, InsnKind}; +pub use rv32im::{DecodedInstruction, EmuContext, InsnCategory, InsnKind}; diff --git a/ceno_emul/src/rv32im.rs b/ceno_emul/src/rv32im.rs index efcde7bc1..28704ea39 100644 --- a/ceno_emul/src/rv32im.rs +++ b/ceno_emul/src/rv32im.rs @@ -17,7 +17,7 @@ use anyhow::{anyhow, Result}; use std::sync::OnceLock; -use super::addr::{ByteAddr, WordAddr, WORD_SIZE}; +use super::addr::{ByteAddr, RegIdx, Word, WordAddr, WORD_SIZE}; pub trait EmuContext { // Handle environment call @@ -42,23 +42,26 @@ pub trait EmuContext { fn set_pc(&mut self, addr: ByteAddr); // Load from a register - fn load_register(&mut self, idx: usize) -> Result; + fn load_register(&mut self, idx: RegIdx) -> Result; // Store to a register - fn store_register(&mut self, idx: usize, data: u32) -> Result<()>; + fn store_register(&mut self, idx: RegIdx, data: Word) -> Result<()>; // Load from memory - fn load_memory(&mut self, addr: WordAddr) -> Result; + fn load_memory(&mut self, addr: WordAddr) -> Result; // Store to memory - fn store_memory(&mut self, addr: WordAddr, data: u32) -> Result<()>; + fn store_memory(&mut self, addr: WordAddr, data: Word) -> Result<()>; + + // Get the value of a register without side-effects. + fn peek_register(&self, idx: RegIdx) -> Word; // Get the value of a memory word without side-effects. - fn peek_memory(&self, addr: WordAddr) -> u32; + fn peek_memory(&self, addr: WordAddr) -> Word; // Load from memory, in the context of instruction fetching. // Only called after check_insn_load returns true. - fn fetch(&mut self, pc: WordAddr) -> Result { + fn fetch(&mut self, pc: WordAddr) -> Result { self.load_memory(pc) } diff --git a/ceno_emul/src/vm_state.rs b/ceno_emul/src/vm_state.rs index f7a00d0ea..f8c187969 100644 --- a/ceno_emul/src/vm_state.rs +++ b/ceno_emul/src/vm_state.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use super::rv32im::EmuContext; use crate::{ - addr::{ByteAddr, WordAddr}, + addr::{ByteAddr, RegIdx, Word, WordAddr}, platform::Platform, rv32im::{DecodedInstruction, Emulator, Instruction, TrapCause}, tracer::{Change, StepRecord, Tracer}, @@ -13,10 +13,10 @@ use std::iter::from_fn; /// An implementation of the machine state and of the side-effects of operations. pub struct VMState { platform: Platform, - pc: u32, + pc: Word, /// Map a word-address (addr/4) to a word. - memory: HashMap, - registers: [u32; 32], + memory: HashMap, + registers: [Word; 32], // Termination. succeeded: bool, tracer: Tracer, @@ -43,13 +43,8 @@ impl VMState { &mut self.tracer } - /// Get the value of a register without side-effects. - pub fn peek_register(&self, idx: usize) -> u32 { - self.registers[idx] - } - /// Set a word in memory without side-effects. - pub fn init_memory(&mut self, addr: WordAddr, value: u32) { + pub fn init_memory(&mut self, addr: WordAddr, value: Word) { self.memory.insert(addr, value); } @@ -111,13 +106,13 @@ impl EmuContext for VMState { } /// Load a register and record this operation. - fn load_register(&mut self, idx: usize) -> Result { + fn load_register(&mut self, idx: RegIdx) -> Result { self.tracer.load_register(idx, self.peek_register(idx)); Ok(self.peek_register(idx)) } /// Store a register and record this operation. - fn store_register(&mut self, idx: usize, after: u32) -> Result<()> { + fn store_register(&mut self, idx: RegIdx, after: Word) -> Result<()> { if idx != 0 { let before = self.peek_register(idx); self.tracer.store_register(idx, Change { before, after }); @@ -127,26 +122,31 @@ impl EmuContext for VMState { } /// Load a memory word and record this operation. - fn load_memory(&mut self, addr: WordAddr) -> Result { + fn load_memory(&mut self, addr: WordAddr) -> Result { let value = self.peek_memory(addr); self.tracer.load_memory(addr, value); Ok(value) } /// Store a memory word and record this operation. - fn store_memory(&mut self, addr: WordAddr, after: u32) -> Result<()> { + fn store_memory(&mut self, addr: WordAddr, after: Word) -> Result<()> { let before = self.peek_memory(addr); self.tracer.store_memory(addr, Change { after, before }); self.memory.insert(addr, after); Ok(()) } + /// Get the value of a register without side-effects. + fn peek_register(&self, idx: RegIdx) -> Word { + self.registers[idx] + } + /// Get the value of a memory word without side-effects. - fn peek_memory(&self, addr: WordAddr) -> u32 { + fn peek_memory(&self, addr: WordAddr) -> Word { *self.memory.get(&addr).unwrap_or(&0) } - fn fetch(&mut self, pc: WordAddr) -> Result { + fn fetch(&mut self, pc: WordAddr) -> Result { let value = self.peek_memory(pc); self.tracer.fetch(pc, value); Ok(value) diff --git a/ceno_emul/tests/test_vm_trace.rs b/ceno_emul/tests/test_vm_trace.rs index c71964ab5..505a090a7 100644 --- a/ceno_emul/tests/test_vm_trace.rs +++ b/ceno_emul/tests/test_vm_trace.rs @@ -1,5 +1,5 @@ use anyhow::Result; -use ceno_emul::{ByteAddr, InsnKind, StepRecord, VMState, CENO_PLATFORM}; +use ceno_emul::{ByteAddr, EmuContext, InsnKind, StepRecord, VMState, CENO_PLATFORM}; #[test] fn test_vm_trace() -> Result<()> { From 1ab7bce350ec1b7540c5236e33bf149e4e19ed7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Nicolas?= Date: Wed, 4 Sep 2024 12:57:15 +0200 Subject: [PATCH 11/13] emul-trace-mem: load registers only when necessary --- ceno_emul/src/platform.rs | 5 +- ceno_emul/src/rv32im.rs | 203 +++++++++++++++++++++----------------- 2 files changed, 114 insertions(+), 94 deletions(-) diff --git a/ceno_emul/src/platform.rs b/ceno_emul/src/platform.rs index 73f979127..71241adee 100644 --- a/ceno_emul/src/platform.rs +++ b/ceno_emul/src/platform.rs @@ -1,4 +1,4 @@ -use crate::addr::{Addr, RegIdx, WORD_SIZE}; +use crate::addr::{Addr, RegIdx}; /// The Platform struct holds the parameters of the VM. /// It defines: @@ -38,7 +38,8 @@ impl Platform { /// Virtual address of a register. pub const fn register_vma(&self, index: RegIdx) -> Addr { - (index * WORD_SIZE) as Addr + // Register VMAs are aligned, cannot be confused with indices, and readable in hex. + (index << 8) as Addr } /// Virtual address of the program counter. diff --git a/ceno_emul/src/rv32im.rs b/ceno_emul/src/rv32im.rs index 28704ea39..99c2035d4 100644 --- a/ceno_emul/src/rv32im.rs +++ b/ceno_emul/src/rv32im.rs @@ -449,11 +449,11 @@ impl Emulator { kind: InsnKind, decoded: &DecodedInstruction, ) -> Result { + use InsnKind::*; + let pc = ctx.get_pc(); let mut new_pc = pc + WORD_SIZE; let mut rd = decoded.rd; - let rs1 = ctx.load_register(decoded.rs1 as usize)?; - let rs2 = ctx.load_register(decoded.rs2 as usize)?; let imm_i = decoded.imm_i(); let mut br_cond = |cond| -> u32 { rd = 0; @@ -463,100 +463,119 @@ impl Emulator { 0 }; let out = match kind { - InsnKind::ADD => rs1.wrapping_add(rs2), - InsnKind::SUB => rs1.wrapping_sub(rs2), - InsnKind::XOR => rs1 ^ rs2, - InsnKind::OR => rs1 | rs2, - InsnKind::AND => rs1 & rs2, - InsnKind::SLL => rs1 << (rs2 & 0x1f), - InsnKind::SRL => rs1 >> (rs2 & 0x1f), - InsnKind::SRA => ((rs1 as i32) >> (rs2 & 0x1f)) as u32, - InsnKind::SLT => { - if (rs1 as i32) < (rs2 as i32) { - 1 - } else { - 0 - } - } - InsnKind::SLTU => { - if rs1 < rs2 { - 1 - } else { - 0 - } - } - InsnKind::ADDI => rs1.wrapping_add(imm_i), - InsnKind::XORI => rs1 ^ imm_i, - InsnKind::ORI => rs1 | imm_i, - InsnKind::ANDI => rs1 & imm_i, - InsnKind::SLLI => rs1 << (imm_i & 0x1f), - InsnKind::SRLI => rs1 >> (imm_i & 0x1f), - InsnKind::SRAI => ((rs1 as i32) >> (imm_i & 0x1f)) as u32, - InsnKind::SLTI => { - if (rs1 as i32) < (imm_i as i32) { - 1 - } else { - 0 - } - } - InsnKind::SLTIU => { - if rs1 < imm_i { - 1 - } else { - 0 - } - } - InsnKind::BEQ => br_cond(rs1 == rs2), - InsnKind::BNE => br_cond(rs1 != rs2), - InsnKind::BLT => br_cond((rs1 as i32) < (rs2 as i32)), - InsnKind::BGE => br_cond((rs1 as i32) >= (rs2 as i32)), - InsnKind::BLTU => br_cond(rs1 < rs2), - InsnKind::BGEU => br_cond(rs1 >= rs2), - InsnKind::JAL => { + // Instructions that do not read rs1 nor rs2. + JAL => { new_pc = pc.wrapping_add(decoded.imm_j()); (pc + WORD_SIZE).0 } - InsnKind::JALR => { - new_pc = ByteAddr(rs1.wrapping_add(imm_i) & 0xfffffffe); - (pc + WORD_SIZE).0 - } - InsnKind::LUI => decoded.imm_u(), - InsnKind::AUIPC => (pc.wrapping_add(decoded.imm_u())).0, - InsnKind::MUL => rs1.wrapping_mul(rs2), - InsnKind::MULH => { - (sign_extend_u32(rs1).wrapping_mul(sign_extend_u32(rs2)) >> 32) as u32 - } - InsnKind::MULHSU => (sign_extend_u32(rs1).wrapping_mul(rs2 as i64) >> 32) as u32, - InsnKind::MULHU => (((rs1 as u64).wrapping_mul(rs2 as u64)) >> 32) as u32, - InsnKind::DIV => { - if rs2 == 0 { - u32::MAX - } else { - ((rs1 as i32).wrapping_div(rs2 as i32)) as u32 - } - } - InsnKind::DIVU => { - if rs2 == 0 { - u32::MAX - } else { - rs1 / rs2 + LUI => decoded.imm_u(), + AUIPC => (pc.wrapping_add(decoded.imm_u())).0, + + _ => { + // Instructions that read rs1 but not rs2. + let rs1 = ctx.load_register(decoded.rs1 as usize)?; + + match kind { + ADDI => rs1.wrapping_add(imm_i), + XORI => rs1 ^ imm_i, + ORI => rs1 | imm_i, + ANDI => rs1 & imm_i, + SLLI => rs1 << (imm_i & 0x1f), + SRLI => rs1 >> (imm_i & 0x1f), + SRAI => ((rs1 as i32) >> (imm_i & 0x1f)) as u32, + SLTI => { + if (rs1 as i32) < (imm_i as i32) { + 1 + } else { + 0 + } + } + SLTIU => { + if rs1 < imm_i { + 1 + } else { + 0 + } + } + JALR => { + new_pc = ByteAddr(rs1.wrapping_add(imm_i) & 0xfffffffe); + (pc + WORD_SIZE).0 + } + + _ => { + // Instructions that use rs1 and rs2. + let rs2 = ctx.load_register(decoded.rs2 as usize)?; + + match kind { + ADD => rs1.wrapping_add(rs2), + SUB => rs1.wrapping_sub(rs2), + XOR => rs1 ^ rs2, + OR => rs1 | rs2, + AND => rs1 & rs2, + SLL => rs1 << (rs2 & 0x1f), + SRL => rs1 >> (rs2 & 0x1f), + SRA => ((rs1 as i32) >> (rs2 & 0x1f)) as u32, + SLT => { + if (rs1 as i32) < (rs2 as i32) { + 1 + } else { + 0 + } + } + SLTU => { + if rs1 < rs2 { + 1 + } else { + 0 + } + } + BEQ => br_cond(rs1 == rs2), + BNE => br_cond(rs1 != rs2), + BLT => br_cond((rs1 as i32) < (rs2 as i32)), + BGE => br_cond((rs1 as i32) >= (rs2 as i32)), + BLTU => br_cond(rs1 < rs2), + BGEU => br_cond(rs1 >= rs2), + MUL => rs1.wrapping_mul(rs2), + MULH => { + (sign_extend_u32(rs1).wrapping_mul(sign_extend_u32(rs2)) >> 32) + as u32 + } + MULHSU => (sign_extend_u32(rs1).wrapping_mul(rs2 as i64) >> 32) as u32, + MULHU => (((rs1 as u64).wrapping_mul(rs2 as u64)) >> 32) as u32, + DIV => { + if rs2 == 0 { + u32::MAX + } else { + ((rs1 as i32).wrapping_div(rs2 as i32)) as u32 + } + } + DIVU => { + if rs2 == 0 { + u32::MAX + } else { + rs1 / rs2 + } + } + REM => { + if rs2 == 0 { + rs1 + } else { + ((rs1 as i32).wrapping_rem(rs2 as i32)) as u32 + } + } + REMU => { + if rs2 == 0 { + rs1 + } else { + rs1 % rs2 + } + } + + _ => unreachable!("Illegal compute instruction: {:?}", kind), + } + } } } - InsnKind::REM => { - if rs2 == 0 { - rs1 - } else { - ((rs1 as i32).wrapping_rem(rs2 as i32)) as u32 - } - } - InsnKind::REMU => { - if rs2 == 0 { - rs1 - } else { - rs1 % rs2 - } - } - _ => unreachable!(), }; if !new_pc.is_aligned() { return ctx.trap(TrapCause::InstructionAddressMisaligned); @@ -573,7 +592,7 @@ impl Emulator { decoded: &DecodedInstruction, ) -> Result { let rs1 = ctx.load_register(decoded.rs1 as usize)?; - let _rs2 = ctx.load_register(decoded.rs2 as usize)?; + // LOAD instructions do not read rs2. let addr = ByteAddr(rs1.wrapping_add(decoded.imm_i())); if !ctx.check_data_load(addr) { return ctx.trap(TrapCause::LoadAccessFault(addr)); From 6fe1a502476c41c8901fa6c2ea9c18f23f6839e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Nicolas?= Date: Wed, 4 Sep 2024 14:23:56 +0200 Subject: [PATCH 12/13] emul-trace-mem: test final accesses --- ceno_emul/src/lib.rs | 2 +- ceno_emul/src/tracer.rs | 5 ++++ ceno_emul/src/vm_state.rs | 4 +-- ceno_emul/tests/test_vm_trace.rs | 46 ++++++++++++++++++++++++++++++-- 4 files changed, 52 insertions(+), 5 deletions(-) diff --git a/ceno_emul/src/lib.rs b/ceno_emul/src/lib.rs index 9db5d318a..8c59c467b 100644 --- a/ceno_emul/src/lib.rs +++ b/ceno_emul/src/lib.rs @@ -5,7 +5,7 @@ mod platform; pub use platform::{Platform, CENO_PLATFORM}; mod tracer; -pub use tracer::{Change, MemOp, ReadOp, StepRecord, WriteOp}; +pub use tracer::{Change, MemOp, ReadOp, StepRecord, Tracer, WriteOp}; mod vm_state; pub use vm_state::VMState; diff --git a/ceno_emul/src/tracer.rs b/ceno_emul/src/tracer.rs index 84c79d464..e3b0f8866 100644 --- a/ceno_emul/src/tracer.rs +++ b/ceno_emul/src/tracer.rs @@ -185,6 +185,11 @@ impl Tracer { .insert(addr, self.record.cycle + subcycle) .unwrap_or(0) } + + /// Return all the addresses that were accessed and the cycle when they were last accessed. + pub fn final_accesses(&self) -> &HashMap { + &self.latest_accesses + } } #[derive(Copy, Clone, Default)] diff --git a/ceno_emul/src/vm_state.rs b/ceno_emul/src/vm_state.rs index f8c187969..0da8f2b03 100644 --- a/ceno_emul/src/vm_state.rs +++ b/ceno_emul/src/vm_state.rs @@ -70,8 +70,8 @@ impl EmuContext for VMState { // Expect an ecall to indicate a successful exit: // function HALT with argument SUCCESS. fn ecall(&mut self) -> Result { - let function = 0; // self.load_register(self.platform.reg_ecall())?; - let argument = 0; // self.load_register(self.platform.reg_arg0())?; + let function = self.load_register(self.platform.reg_ecall())?; + let argument = self.load_register(self.platform.reg_arg0())?; if function == self.platform.ecall_halt() && argument == self.platform.code_success() { self.succeeded = true; Ok(true) diff --git a/ceno_emul/tests/test_vm_trace.rs b/ceno_emul/tests/test_vm_trace.rs index 505a090a7..8f8051230 100644 --- a/ceno_emul/tests/test_vm_trace.rs +++ b/ceno_emul/tests/test_vm_trace.rs @@ -1,5 +1,9 @@ use anyhow::Result; -use ceno_emul::{ByteAddr, EmuContext, InsnKind, StepRecord, VMState, CENO_PLATFORM}; +use std::collections::HashMap; + +use ceno_emul::{ + ByteAddr, Cycle, EmuContext, InsnKind, StepRecord, Tracer, VMState, WordAddr, CENO_PLATFORM, +}; #[test] fn test_vm_trace() -> Result<()> { @@ -11,7 +15,6 @@ fn test_vm_trace() -> Result<()> { } let steps = run(&mut ctx)?; - let ctx = ctx; let (x1, x2, x3) = expected_fibonacci_20(); assert_eq!(ctx.peek_register(1), x1); @@ -24,6 +27,11 @@ fn test_vm_trace() -> Result<()> { .collect(); assert_eq!(ops, expected_ops_fibonacci_20()); + assert_eq!( + ctx.tracer().final_accesses(), + &expected_final_accesses_fibonacci_20() + ); + Ok(()) } @@ -39,6 +47,7 @@ fn run(state: &mut VMState) -> Result> { state.iter_until_success().collect() } +/// Example in RISC-V bytecode and assembly. const PROGRAM_FIBONACCI_20: [u32; 7] = [ // x1 = 10; // x3 = 1; @@ -61,6 +70,7 @@ const PROGRAM_FIBONACCI_20: [u32; 7] = [ 0b_000000000000_00000_000_00000_1110011, ]; +/// Rust version of the example. Reconstruct the output. fn expected_fibonacci_20() -> (u32, u32, u32) { let mut x1 = 10; let mut x2 = 0; // Even. @@ -80,6 +90,7 @@ fn expected_fibonacci_20() -> (u32, u32, u32) { (x1, x2, x3) } +/// Reconstruct the sequence of opcodes. fn expected_ops_fibonacci_20() -> Vec { use InsnKind::*; let mut ops = vec![ADDI, ADDI]; @@ -89,3 +100,34 @@ fn expected_ops_fibonacci_20() -> Vec { ops.push(EANY); ops } + +/// Reconstruct the last access of each register. +fn expected_final_accesses_fibonacci_20() -> HashMap { + let mut accesses = HashMap::new(); + let x = |i| WordAddr::from(CENO_PLATFORM.register_vma(i)); + const C: Cycle = Tracer::SUBCYCLES_PER_INSN; + + let mut cycle = C; // First cycle. + cycle += 2 * C; // Set x1 and x3. + for _ in 0..9 { + // Loop except the last iteration. + cycle += 4 * C; // ADDI, ADD, ADD, BNE. + } + cycle += 2 * C; // Last iteration ADDI and ADD. + + // Last ADD. + accesses.insert(x(2), cycle + Tracer::SUBCYCLE_RS1); + accesses.insert(x(3), cycle + Tracer::SUBCYCLE_RD); + cycle += C; + + // Last BNE. + accesses.insert(x(1), cycle + Tracer::SUBCYCLE_RS1); + accesses.insert(x(0), cycle + Tracer::SUBCYCLE_RS2); + cycle += C; + + // Now at the final ECALL cycle. + accesses.insert(x(CENO_PLATFORM.reg_ecall()), cycle + Tracer::SUBCYCLE_RS1); + accesses.insert(x(CENO_PLATFORM.reg_arg0()), cycle + Tracer::SUBCYCLE_RS2); + + accesses +} From 7ad6056b739a8d08f95d38756393e4a79005366d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Nicolas?= Date: Wed, 4 Sep 2024 15:50:50 +0200 Subject: [PATCH 13/13] emul-trace-mem: clippy --- ceno_emul/src/tracer.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/ceno_emul/src/tracer.rs b/ceno_emul/src/tracer.rs index e3b0f8866..f252ca348 100644 --- a/ceno_emul/src/tracer.rs +++ b/ceno_emul/src/tracer.rs @@ -86,6 +86,12 @@ pub struct Tracer { latest_accesses: HashMap, } +impl Default for Tracer { + fn default() -> Self { + Self::new() + } +} + impl Tracer { pub const SUBCYCLE_RS1: Cycle = 0; pub const SUBCYCLE_RS2: Cycle = 1; @@ -103,17 +109,16 @@ impl Tracer { } } + /// Return the completed step and advance to the next cycle. pub fn advance(&mut self) -> StepRecord { - // Reset and advance to the next cycle. let next_cycle = self.record.cycle + Self::SUBCYCLES_PER_INSN; - let complete_step = mem::replace( + mem::replace( &mut self.record, StepRecord { cycle: next_cycle, ..StepRecord::default() }, - ); - complete_step + ) } pub fn store_pc(&mut self, pc: ByteAddr) {