From 6eadbd97b6f494710b4b939acc48d59233f2315b Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 11 Dec 2024 11:24:13 -0500 Subject: [PATCH] fix: fix memory tests --- air/src/trace/chiplets/memory.rs | 4 +- air/src/trace/chiplets/mod.rs | 2 +- miden/src/cli/debug/executor.rs | 9 +- miden/src/repl/mod.rs | 19 +- miden/tests/integration/exec_iters.rs | 50 +- processor/src/chiplets/memory/mod.rs | 29 +- processor/src/chiplets/memory/segment.rs | 93 ++-- processor/src/chiplets/memory/tests.rs | 442 +++++++++++++----- processor/src/chiplets/mod.rs | 13 +- processor/src/debug.rs | 17 +- processor/src/errors.rs | 2 + processor/src/host/debug.rs | 60 ++- processor/src/lib.rs | 11 +- processor/src/operations/io_ops.rs | 22 +- .../operations/sys_ops/sys_event_handlers.rs | 19 +- stdlib/tests/mem/mod.rs | 21 +- test-utils/src/lib.rs | 25 +- 17 files changed, 529 insertions(+), 309 deletions(-) diff --git a/air/src/trace/chiplets/memory.rs b/air/src/trace/chiplets/memory.rs index 1c0aa80f97..486f01de0d 100644 --- a/air/src/trace/chiplets/memory.rs +++ b/air/src/trace/chiplets/memory.rs @@ -60,9 +60,9 @@ pub const ELEMENT_OR_WORD_COL_IDX: usize = READ_WRITE_COL_IDX + 1; /// Column to hold the context ID of the current memory context. pub const CTX_COL_IDX: usize = ELEMENT_OR_WORD_COL_IDX + 1; /// Column to hold the memory address. -pub const ADDR_COL_IDX: usize = CTX_COL_IDX + 1; +pub const BATCH_COL_IDX: usize = CTX_COL_IDX + 1; /// Column to hold the first bit of the index of the address in the batch. -pub const IDX0_COL_IDX: usize = ADDR_COL_IDX + 1; +pub const IDX0_COL_IDX: usize = BATCH_COL_IDX + 1; /// Column to hold the second bit of the index of the address in the batch. pub const IDX1_COL_IDX: usize = IDX0_COL_IDX + 1; /// Column for the clock cycle in which the memory operation occurred. diff --git a/air/src/trace/chiplets/mod.rs b/air/src/trace/chiplets/mod.rs index d892c67e36..91a82b19b1 100644 --- a/air/src/trace/chiplets/mod.rs +++ b/air/src/trace/chiplets/mod.rs @@ -92,7 +92,7 @@ pub const MEMORY_SELECTORS_COL_IDX: usize = MEMORY_TRACE_OFFSET; /// The index within the main trace of the column containing the memory context. pub const MEMORY_CTX_COL_IDX: usize = MEMORY_TRACE_OFFSET + memory::CTX_COL_IDX; /// The index within the main trace of the column containing the memory address. -pub const MEMORY_ADDR_COL_IDX: usize = MEMORY_TRACE_OFFSET + memory::ADDR_COL_IDX; +pub const MEMORY_ADDR_COL_IDX: usize = MEMORY_TRACE_OFFSET + memory::BATCH_COL_IDX; /// The index within the main trace of the column containing the clock cycle of the memory /// access. pub const MEMORY_CLK_COL_IDX: usize = MEMORY_TRACE_OFFSET + memory::CLK_COL_IDX; diff --git a/miden/src/cli/debug/executor.rs b/miden/src/cli/debug/executor.rs index d2a145fe03..c755f3ead0 100644 --- a/miden/src/cli/debug/executor.rs +++ b/miden/src/cli/debug/executor.rs @@ -154,7 +154,7 @@ impl DebugExecutor { /// print all memory entries. pub fn print_memory(&self) { - for (address, mem) in self.vm_state.memory.iter() { + for &(address, mem) in self.vm_state.memory.iter() { Self::print_memory_data(address, mem) } } @@ -167,7 +167,7 @@ impl DebugExecutor { }); match entry { - Some(mem) => Self::print_memory_data(&address, mem), + Some(&mem) => Self::print_memory_data(address, mem), None => println!("memory at address '{address}' not found"), } } @@ -176,9 +176,8 @@ impl DebugExecutor { // -------------------------------------------------------------------------------------------- /// print memory data. - fn print_memory_data(address: &u64, memory: &[Felt]) { - let mem_int = memory.iter().map(|&x| x.as_int()).collect::>(); - println!("{address} {mem_int:?}"); + fn print_memory_data(address: u64, mem_value: Felt) { + println!("{address} {mem_value:?}"); } /// print help message diff --git a/miden/src/repl/mod.rs b/miden/src/repl/mod.rs index 84dfe8df47..1ad8e23e3c 100644 --- a/miden/src/repl/mod.rs +++ b/miden/src/repl/mod.rs @@ -1,7 +1,7 @@ use std::{collections::BTreeSet, path::PathBuf}; use assembly::{Assembler, Library}; -use miden_vm::{math::Felt, DefaultHost, StackInputs, Word}; +use miden_vm::{math::Felt, DefaultHost, StackInputs}; use processor::ContextId; use rustyline::{error::ReadlineError, DefaultEditor}; use stdlib::StdLibrary; @@ -171,7 +171,7 @@ pub fn start_repl(library_paths: &Vec, use_stdlib: bool) { let mut should_print_stack = false; // state of the entire memory at the latest clock cycle. - let mut memory: Vec<(u64, Word)> = Vec::new(); + let mut memory: Vec<(u64, Felt)> = Vec::new(); // initializing readline. let mut rl = DefaultEditor::new().expect("Readline couldn't be initialized"); @@ -224,9 +224,9 @@ pub fn start_repl(library_paths: &Vec, use_stdlib: bool) { println!("The memory has not been initialized yet"); continue; } - for (addr, mem) in &memory { + for &(addr, mem) in &memory { // prints out the address and memory value at that address. - print_mem_address(*addr, mem); + print_mem_address(addr, mem); } } else if line.len() > 6 && &line[..5] == "!mem[" { // if user wants to see the state of a particular address in a memory, the input @@ -238,8 +238,8 @@ pub fn start_repl(library_paths: &Vec, use_stdlib: bool) { // extracts the address from user input. match read_mem_address(&line) { Ok(addr) => { - for (i, memory_value) in &memory { - if *i == addr { + for &(i, memory_value) in &memory { + if i == addr { // prints the address and memory value at that address. print_mem_address(addr, memory_value); // sets the flag to true as the address has been initialized. @@ -305,7 +305,7 @@ pub fn start_repl(library_paths: &Vec, use_stdlib: bool) { fn execute( program: String, provided_libraries: &[Library], -) -> Result<(Vec<(u64, Word)>, Vec), String> { +) -> Result<(Vec<(u64, Felt)>, Vec), String> { // compile program let mut assembler = Assembler::default(); @@ -404,7 +404,6 @@ fn print_stack(stack: Vec) { /// Accepts and returns a memory at an address by converting its register into integer /// from Felt. -fn print_mem_address(addr: u64, mem: &Word) { - let mem_int = mem.iter().map(|&x| x.as_int()).collect::>(); - println!("{} {:?}", addr, mem_int) +fn print_mem_address(addr: u64, mem_value: Felt) { + println!("{addr} {mem_value}") } diff --git a/miden/tests/integration/exec_iters.rs b/miden/tests/integration/exec_iters.rs index dd9b4f1208..4f05bd82ee 100644 --- a/miden/tests/integration/exec_iters.rs +++ b/miden/tests/integration/exec_iters.rs @@ -5,6 +5,7 @@ use vm_core::{debuginfo::Location, AssemblyOp, Operation}; // EXEC ITER TESTS // ================================================================= /// TODO: Reenable (and fix) after we stabilized the assembler +/// Note: expect the memory values to be very wrong. #[test] #[ignore] fn test_exec_iter() { @@ -18,7 +19,8 @@ fn test_exec_iter() { let traces = test.execute_iter(); let fmp = Felt::new(2u64.pow(30)); let next_fmp = fmp + ONE; - let mem = vec![(1_u64, slice_to_word(&[13, 14, 15, 16]))]; + // TODO: double check this value + let mem = vec![(1_u64, Felt::from(13_u32))]; let mem_storew1_loc = Some(Location { path: path.clone(), start: 33.into(), @@ -309,10 +311,7 @@ fn test_exec_iter() { )), stack: [17, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0, 0, 0].to_elements(), fmp: next_fmp, - memory: vec![ - (1_u64, slice_to_word(&[13, 14, 15, 16])), - (2u64.pow(30) + 1, slice_to_word(&[17, 0, 0, 0])), - ], + memory: vec![(1_u64, 13_u32.into()), (2u64.pow(30) + 1, 17_u32.into())], }, VmState { clk: RowIndex::from(19), @@ -330,10 +329,7 @@ fn test_exec_iter() { )), stack: [12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0, 0, 0, 0, 0].to_elements(), fmp: next_fmp, - memory: vec![ - (1_u64, slice_to_word(&[13, 14, 15, 16])), - (2u64.pow(30) + 1, slice_to_word(&[17, 0, 0, 0])), - ], + memory: vec![(1_u64, 13_u32.into()), (2u64.pow(30) + 1, 17_u32.into())], }, VmState { clk: RowIndex::from(20), @@ -343,10 +339,7 @@ fn test_exec_iter() { stack: [18446744069414584320, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0, 0, 0] .to_elements(), fmp: next_fmp, - memory: vec![ - (1_u64, slice_to_word(&[13, 14, 15, 16])), - (2u64.pow(30) + 1, slice_to_word(&[17, 0, 0, 0])), - ], + memory: vec![(1_u64, 13_u32.into()), (2u64.pow(30) + 1, 17_u32.into())], }, VmState { clk: RowIndex::from(21), @@ -355,10 +348,7 @@ fn test_exec_iter() { asmop: None, stack: [12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0, 0, 0, 0].to_elements(), fmp, - memory: vec![ - (1_u64, slice_to_word(&[13, 14, 15, 16])), - (2u64.pow(30) + 1, slice_to_word(&[17, 0, 0, 0])), - ], + memory: vec![(1_u64, 13_u32.into()), (2u64.pow(30) + 1, 17_u32.into())], }, VmState { clk: RowIndex::from(22), @@ -367,10 +357,7 @@ fn test_exec_iter() { asmop: None, stack: [12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0, 0, 0, 0].to_elements(), fmp, - memory: vec![ - (1_u64, slice_to_word(&[13, 14, 15, 16])), - (2u64.pow(30) + 1, slice_to_word(&[17, 0, 0, 0])), - ], + memory: vec![(1_u64, 13_u32.into()), (2u64.pow(30) + 1, 17_u32.into())], }, VmState { clk: RowIndex::from(23), @@ -379,10 +366,7 @@ fn test_exec_iter() { asmop: None, stack: [12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0, 0, 0, 0].to_elements(), fmp, - memory: vec![ - (1_u64, slice_to_word(&[13, 14, 15, 16])), - (2u64.pow(30) + 1, slice_to_word(&[17, 0, 0, 0])), - ], + memory: vec![(1_u64, 13_u32.into()), (2u64.pow(30) + 1, 17_u32.into())], }, VmState { clk: RowIndex::from(24), @@ -391,10 +375,7 @@ fn test_exec_iter() { asmop: None, stack: [12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0, 0, 0, 0].to_elements(), fmp, - memory: vec![ - (1_u64, slice_to_word(&[13, 14, 15, 16])), - (2u64.pow(30) + 1, slice_to_word(&[17, 0, 0, 0])), - ], + memory: vec![(1_u64, 13_u32.into()), (2u64.pow(30) + 1, 17_u32.into())], }, ]; for (expected, t) in expected_states.iter().zip(traces) { @@ -402,14 +383,3 @@ fn test_exec_iter() { assert_eq!(*expected, *state); } } - -// HELPER FUNCTIONS -// ================================================================= -fn slice_to_word(values: &[i32]) -> [Felt; 4] { - [ - Felt::new(values[0] as u64), - Felt::new(values[1] as u64), - Felt::new(values[2] as u64), - Felt::new(values[3] as u64), - ] -} diff --git a/processor/src/chiplets/memory/mod.rs b/processor/src/chiplets/memory/mod.rs index 0203655d7e..08ca700d31 100644 --- a/processor/src/chiplets/memory/mod.rs +++ b/processor/src/chiplets/memory/mod.rs @@ -2,7 +2,7 @@ use alloc::{collections::BTreeMap, vec::Vec}; use miden_air::{ trace::chiplets::memory::{ - ADDR_COL_IDX, CLK_COL_IDX, CTX_COL_IDX, D0_COL_IDX, D1_COL_IDX, D_INV_COL_IDX, + BATCH_COL_IDX, CLK_COL_IDX, CTX_COL_IDX, D0_COL_IDX, D1_COL_IDX, D_INV_COL_IDX, ELEMENT_OR_WORD_COL_IDX, FLAG_SAME_BATCH_AND_CONTEXT, IDX0_COL_IDX, IDX1_COL_IDX, MEMORY_ACCESS_ELEMENT, MEMORY_ACCESS_WORD, MEMORY_READ, MEMORY_WRITE, READ_WRITE_COL_IDX, V_COL_RANGE, @@ -109,26 +109,27 @@ impl Memory { /// /// Unlike read() which modifies the memory access trace, this method returns the value at the /// specified address (if one exists) without altering the memory access trace. - pub fn get_value(&self, ctx: ContextId, addr: Felt) -> Option { + pub fn get_value(&self, ctx: ContextId, addr: Felt) -> Option { match self.trace.get(&ctx) { - // TODO(plafer): fix + // TODO(plafer): change `addr` to u32 Some(segment) => segment.get_value(addr.try_into().unwrap()), None => None, } } - /// Returns the word at the specified context/address which should be used as the "old value" - /// for a write request. It will be the previously stored value, if one exists, or - /// initialized memory. - pub fn get_old_value(&self, ctx: ContextId, addr: Felt) -> Word { - // get the stored word or return [0, 0, 0, 0], since the memory is initialized with zeros - self.get_value(ctx, addr).unwrap_or(INIT_MEM_VALUE) + // TODO(plafer): is it ok to panic when addr is not aligned? + pub fn get_word(&self, ctx: ContextId, addr: Felt) -> Option { + match self.trace.get(&ctx) { + // TODO(plafer): change `addr` to u32 + Some(segment) => segment.get_word(addr.try_into().unwrap()), + None => None, + } } /// Returns the entire memory state for the specified execution context at the specified cycle. /// The state is returned as a vector of (address, value) tuples, and includes addresses which /// have been accessed at least once. - pub fn get_state_at(&self, ctx: ContextId, clk: RowIndex) -> Vec<(u64, Word)> { + pub fn get_state_at(&self, ctx: ContextId, clk: RowIndex) -> Vec<(u64, Felt)> { if clk == 0 { return vec![]; } @@ -338,7 +339,7 @@ impl Memory { }, }; trace.set(row, CTX_COL_IDX, ctx); - trace.set(row, ADDR_COL_IDX, felt_addr); + trace.set(row, BATCH_COL_IDX, felt_addr); trace.set(row, IDX0_COL_IDX, idx0); trace.set(row, IDX1_COL_IDX, idx1); trace.set(row, CLK_COL_IDX, clk); @@ -396,9 +397,9 @@ impl Memory { // TEST HELPERS // -------------------------------------------------------------------------------------------- - /// Returns current size of the memory (in words) across all contexts. + /// Returns the number of batches that were accessed at least once across all contexts. #[cfg(test)] - pub fn size(&self) -> usize { - self.trace.iter().fold(0, |acc, (_, s)| acc + s.size()) + pub fn num_accessed_batches(&self) -> usize { + self.trace.iter().fold(0, |acc, (_, s)| acc + s.num_accessed_batches()) } } diff --git a/processor/src/chiplets/memory/segment.rs b/processor/src/chiplets/memory/segment.rs index cb68b0a273..ae027c75db 100644 --- a/processor/src/chiplets/memory/segment.rs +++ b/processor/src/chiplets/memory/segment.rs @@ -24,21 +24,37 @@ impl MemorySegmentTrace { // PUBLIC ACCESSORS // -------------------------------------------------------------------------------------------- - /// Returns a word located at the specified address, or None if the address hasn't been + /// Returns the element located at the specified address, or None if the address hasn't been /// accessed previously. /// /// Unlike read() which modifies the memory access trace, this method returns the value at the /// specified address (if one exists) without altering the memory access trace. - pub fn get_value(&self, addr: u32) -> Option { - match self.0.get(&addr) { + pub fn get_value(&self, addr: u32) -> Option { + let (batch, addr_idx_in_word) = addr_to_batch_and_idx(addr); + + match self.0.get(&batch) { + Some(addr_trace) => { + addr_trace.last().map(|access| access.batch()[addr_idx_in_word as usize]) + }, + None => None, + } + } + + // TODO(plafer): is it ok to panic? + pub fn get_word(&self, addr: u32) -> Option { + assert!(addr % WORD_SIZE as u32 == 0, "unaligned word access: {addr}"); + + let batch = addr / WORD_SIZE as u32; + + match self.0.get(&batch) { Some(addr_trace) => addr_trace.last().map(|access| access.batch()), None => None, } } /// Returns the entire memory state at the beginning of the specified cycle. - pub fn get_state_at(&self, clk: RowIndex) -> Vec<(u64, Word)> { - let mut result: Vec<(u64, Word)> = Vec::new(); + pub fn get_state_at(&self, clk: RowIndex) -> Vec<(u64, Felt)> { + let mut result: Vec<(u64, Felt)> = Vec::new(); if clk == 0 { return result; @@ -51,13 +67,29 @@ impl MemorySegmentTrace { for (&addr, addr_trace) in self.0.iter() { match addr_trace.binary_search_by(|access| access.clk().as_int().cmp(&search_clk)) { - Ok(i) => result.push((addr.into(), addr_trace[i].batch())), + Ok(i) => { + let batch = addr_trace[i - 1].batch(); + let addr: u64 = addr.into(); + result.extend([ + (addr, batch[0]), + (addr + 1, batch[1]), + (addr + 2, batch[2]), + (addr + 3, batch[3]), + ]); + }, Err(i) => { // Binary search finds the index of the data with the specified clock cycle. // Decrement the index to get the trace from the previously accessed clock // cycle to insert into the results. if i > 0 { - result.push((addr.into(), addr_trace[i - 1].batch())); + let batch = addr_trace[i - 1].batch(); + let addr: u64 = addr.into(); + result.extend([ + (addr, batch[0]), + (addr + 1, batch[1]), + (addr + 2, batch[2]), + (addr + 3, batch[3]), + ]); } }, } @@ -77,21 +109,12 @@ impl MemorySegmentTrace { /// # Errors /// - Returns an error if the same address is accessed more than once in the same clock cycle. pub fn read(&mut self, ctx: ContextId, addr: u32, clk: Felt) -> Result { - let addr_idx_in_word = addr % WORD_SIZE as u32; - let batch = { - let addr_first_ele = addr - addr_idx_in_word; + let (batch, addr_idx_in_word) = addr_to_batch_and_idx(addr); - addr_first_ele / 4 - }; + let batch_values = + self.read_batch(ctx, batch, clk, MemoryAccessType::Element { addr_idx_in_word })?; - let batch = self.read_batch( - ctx, - batch, - clk, - MemoryAccessType::Element { addr_idx_in_word: addr_idx_in_word as u8 }, - )?; - - Ok(batch[addr_idx_in_word as usize]) + Ok(batch_values[addr_idx_in_word as usize]) } /// Returns a word located in memory starting at the specified address, which must be word @@ -128,11 +151,7 @@ impl MemorySegmentTrace { clk: Felt, value: Felt, ) -> Result<(), ExecutionError> { - let addr_idx_in_word = addr % WORD_SIZE as u32; - let batch = { - let addr_first_ele = addr - addr_idx_in_word; - addr_first_ele / 4 - }; + let (batch, addr_idx_in_word) = addr_to_batch_and_idx(addr); match self.0.entry(batch) { Entry::Vacant(vacant_entry) => { @@ -147,7 +166,7 @@ impl MemorySegmentTrace { let access = MemorySegmentAccess::new( clk, MemoryOperation::Write, - MemoryAccessType::Element { addr_idx_in_word: addr_idx_in_word as u8 }, + MemoryAccessType::Element { addr_idx_in_word }, batch, ); vacant_entry.insert(vec![access]); @@ -281,9 +300,9 @@ impl MemorySegmentTrace { } } - /// Returns current size (in words) of this memory segment. + /// Returns the number of batches that were accessed at least once. #[cfg(test)] - pub fn size(&self) -> usize { + pub fn num_accessed_batches(&self) -> usize { self.0.len() } } @@ -303,6 +322,16 @@ pub enum MemoryAccessType { Word, } +impl MemoryAccessType { + pub fn element(addr_idx_in_word: u8) -> Self { + Self::Element { addr_idx_in_word } + } + + pub fn word() -> Self { + Self::Word + } +} + /// A single memory access representing the specified memory operation with the specified value at /// the specified clock cycle. #[derive(Copy, Debug, Clone)] @@ -341,3 +370,11 @@ impl MemorySegmentAccess { self.batch } } + +/// HELPERS +/// ================================================================================================ + +pub fn addr_to_batch_and_idx(addr: u32) -> (u32, u8) { + let idx = addr % WORD_SIZE as u32; + ((addr - idx) / WORD_SIZE as u32, idx as u8) +} diff --git a/processor/src/chiplets/memory/tests.rs b/processor/src/chiplets/memory/tests.rs index 66233a355d..61b9dbcc85 100644 --- a/processor/src/chiplets/memory/tests.rs +++ b/processor/src/chiplets/memory/tests.rs @@ -2,23 +2,26 @@ use alloc::vec::Vec; use miden_air::{ trace::chiplets::memory::{ - Selectors, MEMORY_COPY_READ, MEMORY_INIT_READ, MEMORY_WRITE_SELECTOR, + ELEMENT_OR_WORD_COL_IDX, FLAG_SAME_BATCH_AND_CONTEXT, IDX0_COL_IDX, IDX1_COL_IDX, + MEMORY_ACCESS_ELEMENT, MEMORY_ACCESS_WORD, MEMORY_READ, MEMORY_WRITE, READ_WRITE_COL_IDX, TRACE_WIDTH as MEMORY_TRACE_WIDTH, }, RowIndex, }; -use vm_core::Word; +use vm_core::{Word, WORD_SIZE}; use super::{ - super::ZERO, Felt, FieldElement, Memory, TraceFragment, ADDR_COL_IDX, CLK_COL_IDX, CTX_COL_IDX, - D0_COL_IDX, D1_COL_IDX, D_INV_COL_IDX, EMPTY_WORD, ONE, V_COL_RANGE, + super::ZERO, + segment::{MemoryAccessType, MemoryOperation}, + Felt, FieldElement, Memory, TraceFragment, BATCH_COL_IDX, CLK_COL_IDX, CTX_COL_IDX, D0_COL_IDX, + D1_COL_IDX, D_INV_COL_IDX, EMPTY_WORD, ONE, V_COL_RANGE, }; use crate::ContextId; #[test] fn mem_init() { let mem = Memory::default(); - assert_eq!(0, mem.size()); + assert_eq!(0, mem.num_accessed_batches()); assert_eq!(0, mem.trace_len()); } @@ -31,48 +34,77 @@ fn mem_read() { let addr0 = ZERO; let value = mem.read(ContextId::root(), addr0, 1.into()).unwrap(); assert_eq!(ZERO, value); - assert_eq!(1, mem.size()); + assert_eq!(1, mem.num_accessed_batches()); assert_eq!(1, mem.trace_len()); // read a value from address 3; clk = 2 let addr3 = Felt::from(3_u32); let value = mem.read(ContextId::root(), addr3, 2.into()).unwrap(); assert_eq!(ZERO, value); - assert_eq!(2, mem.size()); + assert_eq!(1, mem.num_accessed_batches()); assert_eq!(2, mem.trace_len()); // read a value from address 0 again; clk = 3 let value = mem.read(ContextId::root(), addr0, 3.into()).unwrap(); assert_eq!(ZERO, value); - assert_eq!(2, mem.size()); + assert_eq!(1, mem.num_accessed_batches()); assert_eq!(3, mem.trace_len()); // read a value from address 2; clk = 4 let addr2 = Felt::from(2_u32); let value = mem.read(ContextId::root(), addr2, 4.into()).unwrap(); assert_eq!(ZERO, value); - assert_eq!(3, mem.size()); + assert_eq!(1, mem.num_accessed_batches()); assert_eq!(4, mem.trace_len()); - // check generated trace and memory data provided to the ChipletsBus; rows should be sorted by - // address and then clock cycle + // check generated trace and memory data provided to the ChipletsBus; rows should be sorted only + // by clock cycle, since they all access the same batch let trace = build_trace(mem, 4); - // address 0 + // clk 1 let mut prev_row = [ZERO; MEMORY_TRACE_WIDTH]; - let memory_access = MemoryAccess::new(ContextId::root(), addr0, 1.into(), EMPTY_WORD); - prev_row = verify_memory_access(&trace, 0, MEMORY_INIT_READ, &memory_access, prev_row); - - let memory_access = MemoryAccess::new(ContextId::root(), addr0, 3.into(), EMPTY_WORD); - prev_row = verify_memory_access(&trace, 1, MEMORY_COPY_READ, &memory_access, prev_row); - - // address 2 - let memory_access = MemoryAccess::new(ContextId::root(), addr2, 4.into(), EMPTY_WORD); - prev_row = verify_memory_access(&trace, 2, MEMORY_INIT_READ, &memory_access, prev_row); - - // address 3 - let memory_access = MemoryAccess::new(ContextId::root(), addr3, 2.into(), EMPTY_WORD); - verify_memory_access(&trace, 3, MEMORY_INIT_READ, &memory_access, prev_row); + let memory_access = MemoryAccess::new( + MemoryOperation::Read, + MemoryAccessType::element(0), + ContextId::root(), + addr0, + 1.into(), + EMPTY_WORD, + ); + prev_row = verify_memory_access(&trace, 0, memory_access, prev_row); + + // clk 2 + let memory_access = MemoryAccess::new( + MemoryOperation::Read, + MemoryAccessType::element(3), + ContextId::root(), + addr3, + 2.into(), + EMPTY_WORD, + ); + prev_row = verify_memory_access(&trace, 1, memory_access, prev_row); + + // clk 3 + let memory_access = MemoryAccess::new( + MemoryOperation::Read, + MemoryAccessType::element(0), + ContextId::root(), + addr0, + 3.into(), + EMPTY_WORD, + ); + prev_row = verify_memory_access(&trace, 2, memory_access, prev_row); + + // clk 4 + let memory_access = MemoryAccess::new( + MemoryOperation::Read, + MemoryAccessType::element(2), + ContextId::root(), + addr2, + 4.into(), + EMPTY_WORD, + ); + verify_memory_access(&trace, 3, memory_access, prev_row); } // TODO(plafer): add checks for write word, at boundaries, and not at boundaries. @@ -84,33 +116,33 @@ fn mem_write() { // write a value into address 0; clk = 1 let addr0 = ZERO; - let value1 = [ONE, ZERO, ZERO, ZERO]; - mem.write_word(ContextId::root(), addr0, 1.into(), value1).unwrap(); - assert_eq!(value1, mem.get_value(ContextId::root(), addr0).unwrap()); - assert_eq!(1, mem.size()); + let word1 = [ONE, ZERO, ZERO, ZERO]; + mem.write_word(ContextId::root(), addr0, 1.into(), word1).unwrap(); + assert_eq!(word1, mem.get_word(ContextId::root(), addr0).unwrap()); + assert_eq!(1, mem.num_accessed_batches()); assert_eq!(1, mem.trace_len()); // write a value into address 2; clk = 2 let addr2 = Felt::from(2_u32); - let value5 = [Felt::new(5), ZERO, ZERO, ZERO]; - mem.write_word(ContextId::root(), addr2, 2.into(), value5).unwrap(); + let value5 = Felt::new(5); + mem.write(ContextId::root(), addr2, 2.into(), value5).unwrap(); assert_eq!(value5, mem.get_value(ContextId::root(), addr2).unwrap()); - assert_eq!(2, mem.size()); + assert_eq!(2, mem.num_accessed_batches()); assert_eq!(2, mem.trace_len()); // write a value into address 1; clk = 3 let addr1 = Felt::from(1_u32); - let value7 = [Felt::new(7), ZERO, ZERO, ZERO]; - mem.write_word(ContextId::root(), addr1, 3.into(), value7).unwrap(); - assert_eq!(value7, mem.get_value(ContextId::root(), addr1).unwrap()); - assert_eq!(3, mem.size()); + let word7 = [Felt::new(7), ZERO, ZERO, ZERO]; + mem.write_word(ContextId::root(), addr1, 3.into(), word7).unwrap(); + assert_eq!(word7, mem.get_word(ContextId::root(), addr1).unwrap()); + assert_eq!(3, mem.num_accessed_batches()); assert_eq!(3, mem.trace_len()); // write a value into address 0; clk = 4 - let value9 = [Felt::new(9), ZERO, ZERO, ZERO]; - mem.write_word(ContextId::root(), addr0, 4.into(), value9).unwrap(); - assert_eq!(value7, mem.get_value(ContextId::root(), addr1).unwrap()); - assert_eq!(3, mem.size()); + let word9 = [Felt::new(9), ZERO, ZERO, ZERO]; + mem.write_word(ContextId::root(), addr0, 4.into(), word9).unwrap(); + assert_eq!(word7, mem.get_word(ContextId::root(), addr1).unwrap()); + assert_eq!(3, mem.num_accessed_batches()); assert_eq!(4, mem.trace_len()); // check generated trace and memory data provided to the ChipletsBus; rows should be sorted by @@ -119,19 +151,47 @@ fn mem_write() { // address 0 let mut prev_row = [ZERO; MEMORY_TRACE_WIDTH]; - let memory_access = MemoryAccess::new(ContextId::root(), addr0, 1.into(), value1); - prev_row = verify_memory_access(&trace, 0, MEMORY_WRITE_SELECTOR, &memory_access, prev_row); - - let memory_access = MemoryAccess::new(ContextId::root(), addr0, 4.into(), value9); - prev_row = verify_memory_access(&trace, 1, MEMORY_WRITE_SELECTOR, &memory_access, prev_row); + let memory_access = MemoryAccess::new( + MemoryOperation::Write, + MemoryAccessType::Word, + ContextId::root(), + addr0, + 1.into(), + word1, + ); + prev_row = verify_memory_access(&trace, 0, memory_access, prev_row); + + let memory_access = MemoryAccess::new( + MemoryOperation::Write, + MemoryAccessType::Word, + ContextId::root(), + addr0, + 4.into(), + word9, + ); + prev_row = verify_memory_access(&trace, 1, memory_access, prev_row); // address 1 - let memory_access = MemoryAccess::new(ContextId::root(), addr1, 3.into(), value7); - prev_row = verify_memory_access(&trace, 2, MEMORY_WRITE_SELECTOR, &memory_access, prev_row); + let memory_access = MemoryAccess::new( + MemoryOperation::Write, + MemoryAccessType::Word, + ContextId::root(), + addr1, + 3.into(), + word7, + ); + prev_row = verify_memory_access(&trace, 2, memory_access, prev_row); // address 2 - let memory_access = MemoryAccess::new(ContextId::root(), addr2, 2.into(), value5); - verify_memory_access(&trace, 3, MEMORY_WRITE_SELECTOR, &memory_access, prev_row); + let memory_access = MemoryAccess::new( + MemoryOperation::Write, + MemoryAccessType::Word, + ContextId::root(), + addr2, + 2.into(), + [ZERO, ZERO, value5, ZERO], + ); + verify_memory_access(&trace, 3, memory_access, prev_row); } #[test] @@ -177,70 +237,133 @@ fn mem_write_read() { // address 2 let mut prev_row = [ZERO; MEMORY_TRACE_WIDTH]; - let memory_access = MemoryAccess::new(ContextId::root(), addr2, 2.into(), value4); - prev_row = verify_memory_access(&trace, 0, MEMORY_WRITE_SELECTOR, &memory_access, prev_row); - - let memory_access = MemoryAccess::new(ContextId::root(), addr2, 5.into(), value4); - prev_row = verify_memory_access(&trace, 1, MEMORY_COPY_READ, &memory_access, prev_row); - - let memory_access = MemoryAccess::new(ContextId::root(), addr2, 6.into(), value7); - prev_row = verify_memory_access(&trace, 2, MEMORY_WRITE_SELECTOR, &memory_access, prev_row); - - let memory_access = MemoryAccess::new(ContextId::root(), addr2, 8.into(), value7); - prev_row = verify_memory_access(&trace, 3, MEMORY_COPY_READ, &memory_access, prev_row); + let memory_access = MemoryAccess::new( + MemoryOperation::Write, + MemoryAccessType::Word, + ContextId::root(), + addr2, + 2.into(), + value4, + ); + prev_row = verify_memory_access(&trace, 0, memory_access, prev_row); + + let memory_access = MemoryAccess::new( + MemoryOperation::Read, + MemoryAccessType::element(2), + ContextId::root(), + addr2, + 5.into(), + value4, + ); + prev_row = verify_memory_access(&trace, 1, memory_access, prev_row); + + let memory_access = MemoryAccess::new( + MemoryOperation::Write, + MemoryAccessType::Word, + ContextId::root(), + addr2, + 6.into(), + value7, + ); + prev_row = verify_memory_access(&trace, 2, memory_access, prev_row); + + let memory_access = MemoryAccess::new( + MemoryOperation::Read, + MemoryAccessType::element(2), + ContextId::root(), + addr2, + 8.into(), + value7, + ); + prev_row = verify_memory_access(&trace, 3, memory_access, prev_row); // address 5 - let memory_access = MemoryAccess::new(ContextId::root(), addr5, 1.into(), value1); - prev_row = verify_memory_access(&trace, 4, MEMORY_WRITE_SELECTOR, &memory_access, prev_row); - - let memory_access = MemoryAccess::new(ContextId::root(), addr5, 3.into(), value1); - prev_row = verify_memory_access(&trace, 5, MEMORY_COPY_READ, &memory_access, prev_row); - - let memory_access = MemoryAccess::new(ContextId::root(), addr5, 4.into(), value2); - prev_row = verify_memory_access(&trace, 6, MEMORY_WRITE_SELECTOR, &memory_access, prev_row); - - let memory_access = MemoryAccess::new(ContextId::root(), addr5, 7.into(), value2); - prev_row = verify_memory_access(&trace, 7, MEMORY_COPY_READ, &memory_access, prev_row); - - let memory_access = MemoryAccess::new(ContextId::root(), addr5, 9.into(), value2); - verify_memory_access(&trace, 8, MEMORY_COPY_READ, &memory_access, prev_row); + let memory_access = MemoryAccess::new( + MemoryOperation::Write, + MemoryAccessType::Word, + ContextId::root(), + addr5, + 1.into(), + value1, + ); + prev_row = verify_memory_access(&trace, 4, memory_access, prev_row); + + let memory_access = MemoryAccess::new( + MemoryOperation::Read, + MemoryAccessType::element(1), + ContextId::root(), + addr5, + 3.into(), + value1, + ); + prev_row = verify_memory_access(&trace, 5, memory_access, prev_row); + + let memory_access = MemoryAccess::new( + MemoryOperation::Write, + MemoryAccessType::Word, + ContextId::root(), + addr5, + 4.into(), + value2, + ); + prev_row = verify_memory_access(&trace, 6, memory_access, prev_row); + + let memory_access = MemoryAccess::new( + MemoryOperation::Read, + MemoryAccessType::element(1), + ContextId::root(), + addr5, + 7.into(), + value2, + ); + prev_row = verify_memory_access(&trace, 7, memory_access, prev_row); + + let memory_access = MemoryAccess::new( + MemoryOperation::Read, + MemoryAccessType::element(1), + ContextId::root(), + addr5, + 9.into(), + value2, + ); + verify_memory_access(&trace, 8, memory_access, prev_row); } #[test] fn mem_multi_context() { let mut mem = Memory::default(); - // write a value into ctx = ContextId::root(), addr = 0; clk = 1 + // write a value into ctx = 0, addr = 0; clk = 1 let value1 = [ONE, ZERO, ZERO, ZERO]; mem.write_word(ContextId::root(), ZERO, 1.into(), value1).unwrap(); - assert_eq!(value1, mem.get_value(ContextId::root(), ZERO).unwrap()); - assert_eq!(1, mem.size()); + assert_eq!(value1, mem.get_word(ContextId::root(), ZERO).unwrap()); + assert_eq!(1, mem.num_accessed_batches()); assert_eq!(1, mem.trace_len()); // write a value into ctx = 3, addr = 1; clk = 4 let value2 = [ZERO, ONE, ZERO, ZERO]; mem.write_word(3.into(), ONE, 4.into(), value2).unwrap(); - assert_eq!(value2, mem.get_value(3.into(), ONE).unwrap()); - assert_eq!(2, mem.size()); + assert_eq!(value2, mem.get_word(3.into(), ONE).unwrap()); + assert_eq!(2, mem.num_accessed_batches()); assert_eq!(2, mem.trace_len()); // read a value from ctx = 3, addr = 1; clk = 6 let value = mem.read_word(3.into(), ONE, 6.into()).unwrap(); assert_eq!(value2, value); - assert_eq!(2, mem.size()); + assert_eq!(2, mem.num_accessed_batches()); assert_eq!(3, mem.trace_len()); // write a value into ctx = 3, addr = 0; clk = 7 let value3 = [ZERO, ZERO, ONE, ZERO]; mem.write_word(3.into(), ZERO, 7.into(), value3).unwrap(); - assert_eq!(value3, mem.get_value(3.into(), ZERO).unwrap()); - assert_eq!(3, mem.size()); + assert_eq!(value3, mem.get_word(3.into(), ZERO).unwrap()); + assert_eq!(3, mem.num_accessed_batches()); assert_eq!(4, mem.trace_len()); // read a value from ctx = 0, addr = 0; clk = 9 let value = mem.read_word(ContextId::root(), ZERO, 9.into()).unwrap(); assert_eq!(value1, value); - assert_eq!(3, mem.size()); + assert_eq!(3, mem.num_accessed_batches()); assert_eq!(5, mem.trace_len()); // check generated trace and memory data provided to the ChipletsBus; rows should be sorted by @@ -249,22 +372,57 @@ fn mem_multi_context() { // ctx = 0, addr = 0 let mut prev_row = [ZERO; MEMORY_TRACE_WIDTH]; - let memory_access = MemoryAccess::new(ContextId::root(), ZERO, 1.into(), value1); - prev_row = verify_memory_access(&trace, 0, MEMORY_WRITE_SELECTOR, &memory_access, prev_row); - - let memory_access = MemoryAccess::new(ContextId::root(), ZERO, 9.into(), value1); - prev_row = verify_memory_access(&trace, 1, MEMORY_COPY_READ, &memory_access, prev_row); + let memory_access = MemoryAccess::new( + MemoryOperation::Write, + MemoryAccessType::Word, + ContextId::root(), + ZERO, + 1.into(), + value1, + ); + prev_row = verify_memory_access(&trace, 0, memory_access, prev_row); + + let memory_access = MemoryAccess::new( + MemoryOperation::Read, + MemoryAccessType::Word, + ContextId::root(), + ZERO, + 9.into(), + value1, + ); + prev_row = verify_memory_access(&trace, 1, memory_access, prev_row); // ctx = 3, addr = 0 - let memory_access = MemoryAccess::new(3.into(), ZERO, 7.into(), value3); - prev_row = verify_memory_access(&trace, 2, MEMORY_WRITE_SELECTOR, &memory_access, prev_row); + let memory_access = MemoryAccess::new( + MemoryOperation::Write, + MemoryAccessType::Word, + 3.into(), + ZERO, + 7.into(), + value3, + ); + prev_row = verify_memory_access(&trace, 2, memory_access, prev_row); // ctx = 3, addr = 1 - let memory_access = MemoryAccess::new(3.into(), ONE, 4.into(), value2); - prev_row = verify_memory_access(&trace, 3, MEMORY_WRITE_SELECTOR, &memory_access, prev_row); - - let memory_access = MemoryAccess::new(3.into(), ONE, 6.into(), value2); - verify_memory_access(&trace, 4, MEMORY_COPY_READ, &memory_access, prev_row); + let memory_access = MemoryAccess::new( + MemoryOperation::Write, + MemoryAccessType::Word, + 3.into(), + ONE, + 4.into(), + value2, + ); + prev_row = verify_memory_access(&trace, 3, memory_access, prev_row); + + let memory_access = MemoryAccess::new( + MemoryOperation::Read, + MemoryAccessType::Word, + 3.into(), + ONE, + 6.into(), + value2, + ); + verify_memory_access(&trace, 4, memory_access, prev_row); } #[test] @@ -273,18 +431,18 @@ fn mem_get_state_at() { // Write 1 into (ctx = 0, addr = 5) at clk = 1. // This means that mem[5] = 1 at the beginning of clk = 2 - let value1 = [ONE, ZERO, ZERO, ZERO]; - mem.write_word(ContextId::root(), Felt::from(5_u32), 1.into(), value1).unwrap(); + let value1 = ONE; + mem.write(ContextId::root(), Felt::from(5_u32), 1.into(), value1).unwrap(); // Write 4 into (ctx = 0, addr = 2) at clk = 2. // This means that mem[2] = 4 at the beginning of clk = 3 - let value4 = [Felt::new(4), ZERO, ZERO, ZERO]; - mem.write_word(ContextId::root(), Felt::from(2_u32), 2.into(), value4).unwrap(); + let value4 = Felt::new(4); + mem.write(ContextId::root(), Felt::from(2_u32), 2.into(), value4).unwrap(); // write 7 into (ctx = 3, addr = 3) at clk = 4 // This means that mem[3] = 7 at the beginning of clk = 4 - let value7 = [Felt::new(7), ZERO, ZERO, ZERO]; - mem.write_word(3.into(), Felt::from(3_u32), 4.into(), value7).unwrap(); + let value7 = Felt::new(7); + mem.write(3.into(), Felt::from(3_u32), 4.into(), value7).unwrap(); // Check memory state at clk = 2 assert_eq!(mem.get_state_at(ContextId::root(), 2.into()), vec![(5, value1)]); @@ -308,15 +466,36 @@ fn mem_get_state_at() { /// Contains data representing a memory access. pub struct MemoryAccess { + operation: MemoryOperation, + access_type: MemoryAccessType, ctx: ContextId, addr: Felt, clk: Felt, - word: [Felt; 4], + batch_values: [Felt; 4], } impl MemoryAccess { - pub fn new(ctx: ContextId, addr: Felt, clk: RowIndex, word: Word) -> Self { - Self { ctx, addr, clk: Felt::from(clk), word } + pub fn new( + operation: MemoryOperation, + access_type: MemoryAccessType, + ctx: ContextId, + addr: Felt, + clk: RowIndex, + batch_values: Word, + ) -> Self { + if let MemoryAccessType::Element { addr_idx_in_word } = access_type { + let addr: u32 = addr.try_into().unwrap(); + assert_eq!(addr_idx_in_word as u32, addr % WORD_SIZE as u32); + } + + Self { + operation, + access_type, + ctx, + addr, + clk: Felt::from(clk), + batch_values, + } } } @@ -338,18 +517,46 @@ fn read_trace_row(trace: &[Vec], step: usize) -> [Felt; MEMORY_TRACE_WIDTH } fn build_trace_row( - memory_access: &MemoryAccess, - op_selectors: Selectors, + memory_access: MemoryAccess, prev_row: [Felt; MEMORY_TRACE_WIDTH], ) -> [Felt; MEMORY_TRACE_WIDTH] { - let MemoryAccess { ctx, addr, clk, word: new_val } = *memory_access; + let MemoryAccess { + operation, + access_type, + ctx, + addr, + clk, + batch_values: new_val, + } = memory_access; + + let (batch, idx1, idx0) = { + let addr: u32 = addr.try_into().unwrap(); + let remainder = addr % WORD_SIZE as u32; + let batch: Felt = Felt::from((addr - remainder) / WORD_SIZE as u32); + + match remainder { + 0 => (batch, ZERO, ZERO), + 1 => (batch, ZERO, ONE), + 2 => (batch, ONE, ZERO), + 3 => (batch, ONE, ONE), + _ => unreachable!(), + } + }; let mut row = [ZERO; MEMORY_TRACE_WIDTH]; - row[0] = op_selectors[0]; - row[1] = op_selectors[1]; + row[READ_WRITE_COL_IDX] = match operation { + MemoryOperation::Read => MEMORY_READ, + MemoryOperation::Write => MEMORY_WRITE, + }; + row[ELEMENT_OR_WORD_COL_IDX] = match access_type { + MemoryAccessType::Element { .. } => MEMORY_ACCESS_ELEMENT, + MemoryAccessType::Word => MEMORY_ACCESS_WORD, + }; row[CTX_COL_IDX] = ctx.into(); - row[ADDR_COL_IDX] = addr; + row[BATCH_COL_IDX] = batch; + row[IDX0_COL_IDX] = idx0; + row[IDX1_COL_IDX] = idx1; row[CLK_COL_IDX] = clk; row[V_COL_RANGE.start] = new_val[0]; row[V_COL_RANGE.start + 1] = new_val[1]; @@ -359,8 +566,8 @@ fn build_trace_row( if prev_row != [ZERO; MEMORY_TRACE_WIDTH] { let delta = if row[CTX_COL_IDX] != prev_row[CTX_COL_IDX] { row[CTX_COL_IDX] - prev_row[CTX_COL_IDX] - } else if row[ADDR_COL_IDX] != prev_row[ADDR_COL_IDX] { - row[ADDR_COL_IDX] - prev_row[ADDR_COL_IDX] + } else if row[BATCH_COL_IDX] != prev_row[BATCH_COL_IDX] { + row[BATCH_COL_IDX] - prev_row[BATCH_COL_IDX] } else { row[CLK_COL_IDX] - prev_row[CLK_COL_IDX] - ONE }; @@ -371,17 +578,22 @@ fn build_trace_row( row[D_INV_COL_IDX] = delta.inv(); } + if row[BATCH_COL_IDX] == prev_row[BATCH_COL_IDX] && row[CTX_COL_IDX] == prev_row[CTX_COL_IDX] { + row[FLAG_SAME_BATCH_AND_CONTEXT] = ONE; + } else { + row[FLAG_SAME_BATCH_AND_CONTEXT] = ZERO; + } + row } fn verify_memory_access( trace: &[Vec], row: u32, - op_selectors: Selectors, - memory_access: &MemoryAccess, + mem_access: MemoryAccess, prev_row: [Felt; MEMORY_TRACE_WIDTH], ) -> [Felt; MEMORY_TRACE_WIDTH] { - let expected_row = build_trace_row(memory_access, op_selectors, prev_row); + let expected_row = build_trace_row(mem_access, prev_row); assert_eq!(expected_row, read_trace_row(trace, row as usize)); expected_row diff --git a/processor/src/chiplets/mod.rs b/processor/src/chiplets/mod.rs index 2cc4742e39..f1c309a4d5 100644 --- a/processor/src/chiplets/mod.rs +++ b/processor/src/chiplets/mod.rs @@ -302,21 +302,28 @@ impl Chiplets { /// /// Unlike mem_read() which modifies the memory access trace, this method returns the value at /// the specified address (if one exists) without altering the memory access trace. - pub fn get_mem_value(&self, ctx: ContextId, addr: u32) -> Option { + pub fn get_mem_value(&self, ctx: ContextId, addr: u32) -> Option { self.memory.get_value(ctx, Felt::from(addr)) } + /// Returns the word starting at the specified context/address. Unlike mem_read() + /// which modifies the memory access trace, this method returns the batch starting at the + /// specified address (if one exists) without altering the memory access trace. + pub fn get_mem_word(&self, ctx: ContextId, addr: u32) -> Option { + self.memory.get_word(ctx, Felt::from(addr)) + } + /// Returns the entire memory state for the specified execution context at the specified cycle. /// The state is returned as a vector of (address, value) tuples, and includes addresses which /// have been accessed at least once. - pub fn get_mem_state_at(&self, ctx: ContextId, clk: RowIndex) -> Vec<(u64, Word)> { + pub fn get_mem_state_at(&self, ctx: ContextId, clk: RowIndex) -> Vec<(u64, Felt)> { self.memory.get_state_at(ctx, clk) } /// Returns current size of the memory (in words) across all execution contexts. #[cfg(test)] pub fn get_mem_size(&self) -> usize { - self.memory.size() + self.memory.num_accessed_batches() } // KERNEL ROM ACCESSORS diff --git a/processor/src/debug.rs b/processor/src/debug.rs index ab908b5609..0a670a7b0b 100644 --- a/processor/src/debug.rs +++ b/processor/src/debug.rs @@ -5,7 +5,7 @@ use alloc::{ use core::fmt; use miden_air::RowIndex; -use vm_core::{AssemblyOp, Operation, StackOutputs, Word}; +use vm_core::{AssemblyOp, Operation, StackOutputs}; use crate::{ range::RangeChecker, system::ContextId, Chiplets, ChipletsLengths, Decoder, ExecutionError, @@ -21,17 +21,15 @@ pub struct VmState { pub asmop: Option, pub fmp: Felt, pub stack: Vec, - pub memory: Vec<(u64, Word)>, + pub memory: Vec<(u64, Felt)>, } impl fmt::Display for VmState { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let stack: Vec = self.stack.iter().map(|x| x.as_int()).collect(); - let memory: Vec<(u64, [u64; 4])> = - self.memory.iter().map(|x| (x.0, word_to_ints(&x.1))).collect(); write!( f, - "clk={}{}{}, fmp={}, stack={stack:?}, memory={memory:?}", + "clk={}{}{}, fmp={}, stack={stack:?}, memory={:?}", self.clk, match self.op { Some(op) => format!(", op={op}"), @@ -41,7 +39,8 @@ impl fmt::Display for VmState { Some(op) => format!(", {op}"), None => "".to_string(), }, - self.fmp + self.fmp, + self.memory ) } } @@ -245,12 +244,6 @@ impl Iterator for VmStateIterator { } } -// HELPER FUNCTIONS -// ================================================================================================ -fn word_to_ints(word: &Word) -> [u64; 4] { - [word[0].as_int(), word[1].as_int(), word[2].as_int(), word[3].as_int()] -} - /// Contains assembly instruction and operation index in the sequence corresponding to the specified /// AsmOp decorator. This index starts from 1 instead of 0. #[derive(Clone, Debug, Eq, PartialEq)] diff --git a/processor/src/errors.rs b/processor/src/errors.rs index dc9783235a..fc0f88adeb 100644 --- a/processor/src/errors.rs +++ b/processor/src/errors.rs @@ -156,6 +156,8 @@ pub enum Ext2InttError { InputSizeTooBig(u64), #[error("address of the first input must be smaller than 2^32, but was {0}")] InputStartAddressTooBig(u64), + #[error("address of the first input is not word aligned: {0}")] + InputStartNotWordAligned(u64), #[error("output size ({0}) cannot be greater than the input size ({1})")] OutputSizeTooBig(usize, usize), #[error("output size must be greater than 0")] diff --git a/processor/src/host/debug.rs b/processor/src/host/debug.rs index 6812d5bc65..bb4c7ad46d 100644 --- a/processor/src/host/debug.rs +++ b/processor/src/host/debug.rs @@ -2,7 +2,7 @@ use alloc::vec::Vec; use std::{print, println}; use miden_air::RowIndex; -use vm_core::{DebugOptions, Word}; +use vm_core::{DebugOptions, Felt}; use super::ProcessState; use crate::system::ContextId; @@ -74,19 +74,22 @@ impl Printer { /// Prints the whole memory state at the cycle `clk` in context `ctx`. fn print_mem_all(&self, process: ProcessState) { let mem = process.get_mem_state(self.ctx); - let padding = - mem.iter().fold(0, |max, value| word_elem_max_len(Some(value.1)).max(max)) as usize; + let ele_width = mem + .iter() + .map(|(_addr, value)| element_printed_width(Some(*value))) + .max() + .unwrap_or(0) as usize; println!("Memory state before step {} for the context {}:", self.clk, self.ctx); // print the main part of the memory (wihtout the last value) for (addr, value) in mem.iter().take(mem.len() - 1) { - print_mem_address(*addr as u32, Some(*value), false, false, padding); + print_mem_address(*addr as u32, Some(*value), false, false, ele_width); } // print the last memory value if let Some((addr, value)) = mem.last() { - print_mem_address(*addr as u32, Some(*value), true, false, padding); + print_mem_address(*addr as u32, Some(*value), true, false, ele_width); } } @@ -150,18 +153,21 @@ impl Printer { /// /// If `is_local` is true, the output addresses are formatted as decimal values, otherwise as hex /// strings. -fn print_interval(mem_interval: Vec<(u32, Option)>, is_local: bool) { - let padding = - mem_interval.iter().fold(0, |max, value| word_elem_max_len(value.1).max(max)) as usize; +fn print_interval(mem_interval: Vec<(u32, Option)>, is_local: bool) { + let ele_width = mem_interval + .iter() + .map(|(_addr, value)| element_printed_width(*value)) + .max() + .unwrap_or(0) as usize; // print the main part of the memory (wihtout the last value) - for (addr, value) in mem_interval.iter().take(mem_interval.len() - 1) { - print_mem_address(*addr, *value, false, is_local, padding) + for (addr, mem_value) in mem_interval.iter().take(mem_interval.len() - 1) { + print_mem_address(*addr, *mem_value, false, is_local, ele_width) } // print the last memory value if let Some((addr, value)) = mem_interval.last() { - print_mem_address(*addr, *value, true, is_local, padding); + print_mem_address(*addr, *value, true, is_local, ele_width); } } @@ -171,27 +177,26 @@ fn print_interval(mem_interval: Vec<(u32, Option)>, is_local: bool) { /// string. fn print_mem_address( addr: u32, - value: Option, + mem_value: Option, is_last: bool, is_local: bool, - padding: usize, + ele_width: usize, ) { - if let Some(value) = value { + if let Some(value) = mem_value { if is_last { if is_local { print!("└── {addr:>5}: "); } else { print!("└── {addr:#010x}: "); } - print_word(value, padding); - println!(); + println!("{:>width$}\n", value.as_int(), width = ele_width); } else { if is_local { print!("├── {addr:>5}: "); } else { print!("├── {addr:#010x}: "); } - print_word(value, padding); + println!("{:>width$}", value.as_int(), width = ele_width); } } else if is_last { if is_local { @@ -206,23 +211,10 @@ fn print_mem_address( } } -/// Prints the provided Word with specified padding. -fn print_word(value: Word, padding: usize) { - println!( - "[{:>width$}, {:>width$}, {:>width$}, {:>width$}]", - value[0].as_int(), - value[1].as_int(), - value[2].as_int(), - value[3].as_int(), - width = padding - ) -} - -/// Returns the maximum length among the word elements. -fn word_elem_max_len(word: Option) -> u32 { - if let Some(word) = word { - word.iter() - .fold(0, |max, value| (value.as_int().checked_ilog10().unwrap_or(1) + 1).max(max)) +/// Returns the number of digits required to print the provided element. +fn element_printed_width(element: Option) -> u32 { + if let Some(element) = element { + element.as_int().checked_ilog10().unwrap_or(1) + 1 } else { 0 } diff --git a/processor/src/lib.rs b/processor/src/lib.rs index 2f186fd728..d107174359 100644 --- a/processor/src/lib.rs +++ b/processor/src/lib.rs @@ -676,18 +676,23 @@ impl ProcessState<'_> { self.stack.get_state_at(self.system.clk()) } - /// Returns a word located at the specified context/address, or None if the address hasn't + /// Returns the element located at the specified context/address, or None if the address hasn't /// been accessed previously. - pub fn get_mem_value(&self, ctx: ContextId, addr: u32) -> Option { + pub fn get_mem_value(&self, ctx: ContextId, addr: u32) -> Option { self.chiplets.get_mem_value(ctx, addr) } + /// Returns the batch of elements starting at the specified context/address. + pub fn get_mem_word(&self, ctx: ContextId, addr: u32) -> Option { + self.chiplets.get_mem_word(ctx, addr) + } + /// Returns the entire memory state for the specified execution context at the current clock /// cycle. /// /// The state is returned as a vector of (address, value) tuples, and includes addresses which /// have been accessed at least once. - pub fn get_mem_state(&self, ctx: ContextId) -> Vec<(u64, Word)> { + pub fn get_mem_state(&self, ctx: ContextId) -> Vec<(u64, Felt)> { self.chiplets.get_mem_state_at(ctx, self.system.clk()) } } diff --git a/processor/src/operations/io_ops.rs b/processor/src/operations/io_ops.rs index 5c90eec905..38d748a8a2 100644 --- a/processor/src/operations/io_ops.rs +++ b/processor/src/operations/io_ops.rs @@ -316,7 +316,7 @@ mod tests { // check memory state assert_eq!(1, process.chiplets.get_mem_size()); - assert_eq!(word, process.chiplets.get_mem_value(ContextId::root(), 1).unwrap()); + assert_eq!(word, process.chiplets.get_mem_word(ContextId::root(), 1).unwrap()); // --- calling MLOADW with address greater than u32::MAX leads to an error ---------------- process.execute_op(Operation::Push(Felt::new(u64::MAX / 2)), &mut host).unwrap(); @@ -346,7 +346,7 @@ mod tests { // check memory state assert_eq!(1, process.chiplets.get_mem_size()); - assert_eq!(word, process.chiplets.get_mem_value(ContextId::root(), 2).unwrap()); + assert_eq!(word, process.chiplets.get_mem_word(ContextId::root(), 2).unwrap()); // --- calling MLOAD with address greater than u32::MAX leads to an error ----------------- process.execute_op(Operation::Push(Felt::new(u64::MAX / 2)), &mut host).unwrap(); @@ -372,8 +372,8 @@ mod tests { // check memory state assert_eq!(2, process.chiplets.get_mem_size()); - assert_eq!(word1_felts, process.chiplets.get_mem_value(ContextId::root(), 1).unwrap()); - assert_eq!(word2_felts, process.chiplets.get_mem_value(ContextId::root(), 2).unwrap()); + assert_eq!(word1_felts, process.chiplets.get_mem_word(ContextId::root(), 1).unwrap()); + assert_eq!(word2_felts, process.chiplets.get_mem_word(ContextId::root(), 2).unwrap()); // clear the stack for _ in 0..8 { @@ -420,7 +420,7 @@ mod tests { // check memory state assert_eq!(1, process.chiplets.get_mem_size()); - assert_eq!(word1, process.chiplets.get_mem_value(ContextId::root(), 0).unwrap()); + assert_eq!(word1, process.chiplets.get_mem_word(ContextId::root(), 0).unwrap()); // push the second word onto the stack and save it at address 3 let word2 = [2, 4, 6, 8].to_elements().try_into().unwrap(); @@ -432,8 +432,8 @@ mod tests { // check memory state assert_eq!(2, process.chiplets.get_mem_size()); - assert_eq!(word1, process.chiplets.get_mem_value(ContextId::root(), 0).unwrap()); - assert_eq!(word2, process.chiplets.get_mem_value(ContextId::root(), 3).unwrap()); + assert_eq!(word1, process.chiplets.get_mem_word(ContextId::root(), 0).unwrap()); + assert_eq!(word2, process.chiplets.get_mem_word(ContextId::root(), 3).unwrap()); // --- calling MSTOREW with address greater than u32::MAX leads to an error ---------------- process.execute_op(Operation::Push(Felt::new(u64::MAX / 2)), &mut host).unwrap(); @@ -462,7 +462,7 @@ mod tests { // check memory state let mem_0 = [element, ZERO, ZERO, ZERO]; assert_eq!(1, process.chiplets.get_mem_size()); - assert_eq!(mem_0, process.chiplets.get_mem_value(ContextId::root(), 0).unwrap()); + assert_eq!(mem_0, process.chiplets.get_mem_word(ContextId::root(), 0).unwrap()); // push the word onto the stack and save it at address 2 let word_2 = [1, 3, 5, 7].to_elements().try_into().unwrap(); @@ -479,7 +479,7 @@ mod tests { // check memory state to make sure the other 3 elements were not affected let mem_2 = [element, Felt::new(3), Felt::new(5), Felt::new(7)]; assert_eq!(2, process.chiplets.get_mem_size()); - assert_eq!(mem_2, process.chiplets.get_mem_value(ContextId::root(), 2).unwrap()); + assert_eq!(mem_2, process.chiplets.get_mem_word(ContextId::root(), 2).unwrap()); // --- calling MSTORE with address greater than u32::MAX leads to an error ---------------- process.execute_op(Operation::Push(Felt::new(u64::MAX / 2)), &mut host).unwrap(); @@ -522,8 +522,8 @@ mod tests { // check memory state contains the words from the advice stack assert_eq!(2, process.chiplets.get_mem_size()); - assert_eq!(word1_felts, process.chiplets.get_mem_value(ContextId::root(), 1).unwrap()); - assert_eq!(word2_felts, process.chiplets.get_mem_value(ContextId::root(), 2).unwrap()); + assert_eq!(word1_felts, process.chiplets.get_mem_word(ContextId::root(), 1).unwrap()); + assert_eq!(word2_felts, process.chiplets.get_mem_word(ContextId::root(), 2).unwrap()); // the first 8 values should be the values from the advice stack. the next 4 values should // remain unchanged, and the address should be incremented by 2 (i.e., 1 -> 3). diff --git a/processor/src/operations/sys_ops/sys_event_handlers.rs b/processor/src/operations/sys_ops/sys_event_handlers.rs index 13e4a796be..a869472612 100644 --- a/processor/src/operations/sys_ops/sys_event_handlers.rs +++ b/processor/src/operations/sys_ops/sys_event_handlers.rs @@ -6,7 +6,7 @@ use vm_core::{ merkle::{EmptySubtreeRoots, Smt, SMT_DEPTH}, }, sys_events::SystemEvent, - Felt, FieldElement, SignatureKind, Word, EMPTY_WORD, WORD_SIZE, ZERO, + Felt, FieldElement, SignatureKind, Word, WORD_SIZE, ZERO, }; use winter_prover::math::fft; @@ -91,8 +91,8 @@ pub fn insert_mem_values_into_adv_map( let mut values = Vec::with_capacity(((end_addr - start_addr) as usize) * WORD_SIZE); for addr in start_addr..end_addr { - let mem_value = process.get_mem_value(ctx, addr).unwrap_or(EMPTY_WORD); - values.extend_from_slice(&mem_value); + let mem_value = process.get_mem_value(ctx, addr).unwrap_or(ZERO); + values.push(mem_value); } let key = process.get_stack_word(0); @@ -403,8 +403,8 @@ pub fn push_ext2_inv_result( /// Returns an error if: /// - `input_size` less than or equal to 1, or is not a power of 2. /// - `output_size` is 0 or is greater than the `input_size`. -/// - `input_ptr` is greater than 2^32. -/// - `input_ptr + input_size / 2` is greater than 2^32. +/// - `input_ptr` is greater than 2^32, or is not aligned on a word boundary. +/// - `input_ptr + input_size * 2` is greater than 2^32. pub fn push_ext2_intt_result( advice_provider: &mut impl AdviceProvider, process: ProcessState, @@ -422,11 +422,14 @@ pub fn push_ext2_intt_result( if input_start_ptr >= u32::MAX as u64 { return Err(Ext2InttError::InputStartAddressTooBig(input_start_ptr).into()); } + if input_start_ptr % WORD_SIZE as u64 != 0 { + return Err(Ext2InttError::InputStartNotWordAligned(input_start_ptr).into()); + } if input_size > u32::MAX as usize { return Err(Ext2InttError::InputSizeTooBig(input_size as u64).into()); } - let input_end_ptr = input_start_ptr + (input_size / 2) as u64; + let input_end_ptr = input_start_ptr + (input_size * 2) as u64; if input_end_ptr > u32::MAX as u64 { return Err(Ext2InttError::InputEndAddressTooBig(input_end_ptr).into()); } @@ -439,9 +442,9 @@ pub fn push_ext2_intt_result( } let mut poly = Vec::with_capacity(input_size); - for addr in (input_start_ptr as u32)..(input_end_ptr as u32) { + for addr in ((input_start_ptr as u32)..(input_end_ptr as u32)).step_by(4) { let word = process - .get_mem_value(process.ctx(), addr) + .get_mem_word(process.ctx(), addr) .ok_or(Ext2InttError::UninitializedMemoryAddress(addr))?; poly.push(QuadFelt::new(word[0], word[1])); diff --git a/stdlib/tests/mem/mod.rs b/stdlib/tests/mem/mod.rs index 8cfe9e4226..337679d211 100644 --- a/stdlib/tests/mem/mod.rs +++ b/stdlib/tests/mem/mod.rs @@ -37,54 +37,55 @@ fn test_memcopy() { Process::new(program.kernel().clone(), StackInputs::default(), ExecutionOptions::default()); process.execute(&program, &mut host).unwrap(); + // TODO(plafer): this will fail due to addresses being too close to each other assert_eq!( - process.chiplets.get_mem_value(ContextId::root(), 1000), + process.chiplets.get_mem_word(ContextId::root(), 1000), Some([ZERO, ZERO, ZERO, ONE]), "Address 1000" ); assert_eq!( - process.chiplets.get_mem_value(ContextId::root(), 1001), + process.chiplets.get_mem_word(ContextId::root(), 1001), Some([ZERO, ZERO, ONE, ZERO]), "Address 1001" ); assert_eq!( - process.chiplets.get_mem_value(ContextId::root(), 1002), + process.chiplets.get_mem_word(ContextId::root(), 1002), Some([ZERO, ZERO, ONE, ONE]), "Address 1002" ); assert_eq!( - process.chiplets.get_mem_value(ContextId::root(), 1003), + process.chiplets.get_mem_word(ContextId::root(), 1003), Some([ZERO, ONE, ZERO, ZERO]), "Address 1003" ); assert_eq!( - process.chiplets.get_mem_value(ContextId::root(), 1004), + process.chiplets.get_mem_word(ContextId::root(), 1004), Some([ZERO, ONE, ZERO, ONE]), "Address 1004" ); assert_eq!( - process.chiplets.get_mem_value(ContextId::root(), 2000), + process.chiplets.get_mem_word(ContextId::root(), 2000), Some([ZERO, ZERO, ZERO, ONE]), "Address 2000" ); assert_eq!( - process.chiplets.get_mem_value(ContextId::root(), 2001), + process.chiplets.get_mem_word(ContextId::root(), 2001), Some([ZERO, ZERO, ONE, ZERO]), "Address 2001" ); assert_eq!( - process.chiplets.get_mem_value(ContextId::root(), 2002), + process.chiplets.get_mem_word(ContextId::root(), 2002), Some([ZERO, ZERO, ONE, ONE]), "Address 2002" ); assert_eq!( - process.chiplets.get_mem_value(ContextId::root(), 2003), + process.chiplets.get_mem_word(ContextId::root(), 2003), Some([ZERO, ONE, ZERO, ZERO]), "Address 2003" ); assert_eq!( - process.chiplets.get_mem_value(ContextId::root(), 2004), + process.chiplets.get_mem_word(ContextId::root(), 2004), Some([ZERO, ONE, ZERO, ONE]), "Address 2004" ); diff --git a/test-utils/src/lib.rs b/test-utils/src/lib.rs index 2d048db53f..9006f3407f 100644 --- a/test-utils/src/lib.rs +++ b/test-utils/src/lib.rs @@ -23,6 +23,7 @@ pub use processor::{ }; #[cfg(not(target_family = "wasm"))] use proptest::prelude::{Arbitrary, Strategy}; +use prover::utils::range; pub use prover::{prove, MemAdviceProvider, MerkleTreeVC, ProvingOptions}; pub use test_case::test_case; pub use verifier::{verify, AcceptableOptions, VerifierError}; @@ -221,7 +222,7 @@ impl Test { pub fn expect_stack_and_memory( &self, final_stack: &[u64], - mut mem_start_addr: u32, + mem_start_addr: u32, expected_mem: &[u64], ) { // compile the program @@ -243,21 +244,19 @@ impl Test { process.execute(&program, &mut host).unwrap(); // validate the memory state - for data in expected_mem.chunks(WORD_SIZE) { - // Main memory is zeroed by default, use zeros as a fallback when unwrap to make testing - // easier - let mem_state = process - .chiplets - .get_mem_value(ContextId::root(), mem_start_addr) - .unwrap_or(EMPTY_WORD); - - let mem_state = felt_slice_to_ints(&mem_state); + for (addr, mem_value) in + (range(mem_start_addr as usize, expected_mem.len())).zip(expected_mem.iter()) + { + let mem_state = + process.chiplets.get_mem_value(ContextId::root(), addr as u32).unwrap_or(ZERO); assert_eq!( - data, mem_state, + *mem_value, + mem_state.as_int(), "Expected memory [{}] => {:?}, found {:?}", - mem_start_addr, data, mem_state + addr, + mem_value, + mem_state ); - mem_start_addr += 1; } // validate the stack states