From 74efdb5adf8ef28f84690d872b70d2f9a7805fe1 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Tue, 10 Dec 2024 08:06:47 -0500 Subject: [PATCH] feat: update memory chiplet to be element-addressable --- air/src/constraints/chiplets/memory/mod.rs | 6 +- air/src/constraints/chiplets/memory/tests.rs | 13 +- air/src/trace/chiplets/memory.rs | 41 +- air/src/trace/chiplets/mod.rs | 2 +- air/src/trace/mod.rs | 2 +- assembly/src/assembler/instruction/mem_ops.rs | 9 +- assembly/src/assembler/mod.rs | 14 +- assembly/src/tests.rs | 11 +- miden/src/cli/debug/executor.rs | 9 +- miden/src/repl/mod.rs | 21 +- miden/tests/integration/exec_iters.rs | 50 +- processor/src/chiplets/memory/mod.rs | 213 ++++-- processor/src/chiplets/memory/segment.rs | 285 ++++++-- processor/src/chiplets/memory/tests.rs | 660 ++++++++++++------ processor/src/chiplets/mod.rs | 123 +--- processor/src/chiplets/tests.rs | 3 +- processor/src/debug.rs | 21 +- processor/src/decoder/mod.rs | 10 +- processor/src/decoder/tests.rs | 10 +- processor/src/errors.rs | 8 + processor/src/host/debug.rs | 60 +- processor/src/lib.rs | 18 +- processor/src/operations/comb_ops.rs | 12 +- processor/src/operations/io_ops.rs | 347 +++++---- .../operations/sys_ops/sys_event_handlers.rs | 19 +- processor/src/trace/tests/chiplets/memory.rs | 10 +- stdlib/tests/mem/mod.rs | 21 +- test-utils/src/lib.rs | 24 +- 28 files changed, 1252 insertions(+), 770 deletions(-) diff --git a/air/src/constraints/chiplets/memory/mod.rs b/air/src/constraints/chiplets/memory/mod.rs index 1f1289be8e..709fe0a42f 100644 --- a/air/src/constraints/chiplets/memory/mod.rs +++ b/air/src/constraints/chiplets/memory/mod.rs @@ -5,7 +5,7 @@ use winter_air::TransitionConstraintDegree; use super::{EvaluationFrame, FieldElement}; use crate::{ trace::chiplets::{ - memory::NUM_ELEMENTS, MEMORY_ADDR_COL_IDX, MEMORY_CLK_COL_IDX, MEMORY_CTX_COL_IDX, + memory::NUM_ELEMENTS_IN_BATCH, MEMORY_ADDR_COL_IDX, MEMORY_CLK_COL_IDX, MEMORY_CTX_COL_IDX, MEMORY_D0_COL_IDX, MEMORY_D1_COL_IDX, MEMORY_D_INV_COL_IDX, MEMORY_TRACE_OFFSET, MEMORY_V_COL_RANGE, }, @@ -152,13 +152,13 @@ fn enforce_values( let mut index = 0; // initialize memory to zero when reading from new context and address pair. - for i in 0..NUM_ELEMENTS { + for i in 0..NUM_ELEMENTS_IN_BATCH { result[index] = memory_flag * frame.init_read_flag() * frame.v(i); index += 1; } // copy previous values when reading memory that was previously accessed. - for i in 0..NUM_ELEMENTS { + for i in 0..NUM_ELEMENTS_IN_BATCH { result[index] = memory_flag * frame.copy_read_flag() * (frame.v_next(i) - frame.v(i)); index += 1; } diff --git a/air/src/constraints/chiplets/memory/tests.rs b/air/src/constraints/chiplets/memory/tests.rs index cdb34b117d..077a2ef89b 100644 --- a/air/src/constraints/chiplets/memory/tests.rs +++ b/air/src/constraints/chiplets/memory/tests.rs @@ -4,13 +4,14 @@ use rand_utils::rand_value; use super::{ EvaluationFrame, MEMORY_ADDR_COL_IDX, MEMORY_CLK_COL_IDX, MEMORY_CTX_COL_IDX, - MEMORY_D0_COL_IDX, MEMORY_D1_COL_IDX, MEMORY_D_INV_COL_IDX, MEMORY_V_COL_RANGE, NUM_ELEMENTS, + MEMORY_D0_COL_IDX, MEMORY_D1_COL_IDX, MEMORY_D_INV_COL_IDX, MEMORY_V_COL_RANGE, + NUM_ELEMENTS_IN_BATCH, }; use crate::{ chiplets::memory, trace::{ chiplets::{ - memory::{Selectors, MEMORY_COPY_READ, MEMORY_INIT_READ, MEMORY_WRITE}, + memory::{Selectors, MEMORY_COPY_READ, MEMORY_INIT_READ, MEMORY_WRITE_SELECTOR}, MEMORY_TRACE_OFFSET, }, TRACE_WIDTH, @@ -30,7 +31,7 @@ fn test_memory_write() { // Write to a new context. let result = get_constraint_evaluation( - MEMORY_WRITE, + MEMORY_WRITE_SELECTOR, MemoryTestDeltaType::Context, &old_values, &new_values, @@ -39,7 +40,7 @@ fn test_memory_write() { // Write to a new address in the same context. let result = get_constraint_evaluation( - MEMORY_WRITE, + MEMORY_WRITE_SELECTOR, MemoryTestDeltaType::Address, &old_values, &new_values, @@ -48,7 +49,7 @@ fn test_memory_write() { // Write to the same context and address at a new clock cycle. let result = get_constraint_evaluation( - MEMORY_WRITE, + MEMORY_WRITE_SELECTOR, MemoryTestDeltaType::Clock, &old_values, &new_values, @@ -160,7 +161,7 @@ fn get_test_frame( next[MEMORY_CLK_COL_IDX] = Felt::new(delta_row[2]); // Set the old and new values. - for idx in 0..NUM_ELEMENTS { + for idx in 0..NUM_ELEMENTS_IN_BATCH { let old_value = Felt::new(old_values[idx] as u64); // Add a write for the old values to the current row. current[MEMORY_V_COL_RANGE.start + idx] = old_value; diff --git a/air/src/trace/chiplets/memory.rs b/air/src/trace/chiplets/memory.rs index 6e531d30d1..7f62ea5be3 100644 --- a/air/src/trace/chiplets/memory.rs +++ b/air/src/trace/chiplets/memory.rs @@ -4,8 +4,9 @@ use super::{create_range, Felt, Range, ONE, ZERO}; // ================================================================================================ /// Number of columns needed to record an execution trace of the memory chiplet. -pub const TRACE_WIDTH: usize = 12; +pub const TRACE_WIDTH: usize = 15; +// TODO(plafer): get rid of all "selector" constants /// Number of selector columns in the trace. pub const NUM_SELECTORS: usize = 2; @@ -15,8 +16,6 @@ pub const NUM_SELECTORS: usize = 2; /// read / write) is to be applied at a specific row of the memory execution trace. pub type Selectors = [Felt; NUM_SELECTORS]; -// --- OPERATION SELECTORS ------------------------------------------------------------------------ - /// Specifies an operation that initializes new memory and then reads it. pub const MEMORY_INIT_READ: Selectors = [ONE, ZERO]; @@ -24,7 +23,20 @@ pub const MEMORY_INIT_READ: Selectors = [ONE, ZERO]; pub const MEMORY_COPY_READ: Selectors = [ONE, ONE]; /// Specifies a memory write operation. -pub const MEMORY_WRITE: Selectors = [ZERO, ZERO]; +pub const MEMORY_WRITE_SELECTOR: Selectors = [ZERO, ZERO]; + +// --- OPERATION SELECTORS ------------------------------------------------------------------------ + +/// Specifies the value of the `READ_WRITE` column when the operation is a write. +pub const MEMORY_WRITE: Felt = ZERO; +/// Specifies the value of the `READ_WRITE` column when the operation is a read. +pub const MEMORY_READ: Felt = ONE; +/// Specifies the value of the `ELEMENT_OR_WORD` column when the operation is over an element. +pub const MEMORY_ACCESS_ELEMENT: Felt = ZERO; +/// Specifies the value of the `ELEMENT_OR_WORD` column when the operation is over a word. +pub const MEMORY_ACCESS_WORD: Felt = ONE; + +// TODO(plafer): figure out the new labels /// Unique label computed as 1 plus the full chiplet selector with the bits reversed. /// mem_read selector=[1, 1, 0, 1], rev(selector)=[1, 0, 1, 1], +1=[1, 1, 0, 0] @@ -37,17 +49,25 @@ pub const MEMORY_WRITE_LABEL: u8 = 0b0100; // --- COLUMN ACCESSOR INDICES WITHIN THE CHIPLET ------------------------------------------------- /// The number of elements accessible in one read or write memory access. -pub const NUM_ELEMENTS: usize = 4; +pub const NUM_ELEMENTS_IN_BATCH: usize = 4; +/// Column to hold the whether the operation is a read or write. +pub const READ_WRITE_COL_IDX: usize = 0; +/// Column to hold the whether the operation was over an element or a word. +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 = NUM_SELECTORS; +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 = 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. -pub const CLK_COL_IDX: usize = ADDR_COL_IDX + 1; +pub const CLK_COL_IDX: usize = IDX1_COL_IDX + 1; /// Columns to hold the values stored at a given memory context, address, and clock cycle after /// the memory operation. When reading from a new address, these are initialized to zero. -pub const V_COL_RANGE: Range = create_range(CLK_COL_IDX + 1, NUM_ELEMENTS); +pub const V_COL_RANGE: Range = create_range(CLK_COL_IDX + 1, NUM_ELEMENTS_IN_BATCH); /// Column for the lower 16-bits of the delta between two consecutive context IDs, addresses, or /// clock cycles. pub const D0_COL_IDX: usize = V_COL_RANGE.end; @@ -57,3 +77,6 @@ pub const D1_COL_IDX: usize = D0_COL_IDX + 1; /// Column for the inverse of the delta between two consecutive context IDs, addresses, or clock /// cycles, used to enforce that changes are correctly constrained. pub const D_INV_COL_IDX: usize = D1_COL_IDX + 1; +/// Column to hold the flag indicating whether the current memory operation is in the same batch and +/// same context as the previous operation. +pub const FLAG_SAME_BATCH_AND_CONTEXT: usize = D_INV_COL_IDX + 1; 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/air/src/trace/mod.rs b/air/src/trace/mod.rs index 62c7a910f5..c4eef3e810 100644 --- a/air/src/trace/mod.rs +++ b/air/src/trace/mod.rs @@ -51,7 +51,7 @@ pub const RANGE_CHECK_TRACE_RANGE: Range = // Chiplets trace pub const CHIPLETS_OFFSET: usize = RANGE_CHECK_TRACE_RANGE.end; -pub const CHIPLETS_WIDTH: usize = 17; +pub const CHIPLETS_WIDTH: usize = 18; pub const CHIPLETS_RANGE: Range = range(CHIPLETS_OFFSET, CHIPLETS_WIDTH); pub const TRACE_WIDTH: usize = CHIPLETS_OFFSET + CHIPLETS_WIDTH; diff --git a/assembly/src/assembler/instruction/mem_ops.rs b/assembly/src/assembler/instruction/mem_ops.rs index cdfbdc79c5..a20a424988 100644 --- a/assembly/src/assembler/instruction/mem_ops.rs +++ b/assembly/src/assembler/instruction/mem_ops.rs @@ -1,6 +1,6 @@ use alloc::string::ToString; -use vm_core::{Felt, Operation::*}; +use vm_core::{Felt, Operation::*, WORD_SIZE}; use super::{push_felt, push_u32_value, validate_param, BasicBlockBuilder}; use crate::{assembler::ProcedureContext, diagnostics::Report, AssemblyError}; @@ -111,7 +111,7 @@ pub fn mem_write_imm( /// Returns an error if index is greater than the number of procedure locals. pub fn local_to_absolute_addr( block_builder: &mut BasicBlockBuilder, - index: u16, + index_of_local: u16, num_proc_locals: u16, ) -> Result<(), AssemblyError> { if num_proc_locals == 0 { @@ -125,9 +125,10 @@ pub fn local_to_absolute_addr( } let max = num_proc_locals - 1; - validate_param(index, 0..=max)?; + validate_param(index_of_local, 0..=max)?; - push_felt(block_builder, -Felt::from(max - index)); + let fmp_offset_of_local = (max - index_of_local) * WORD_SIZE as u16; + push_felt(block_builder, -Felt::from(fmp_offset_of_local)); block_builder.push_op(FmpAdd); Ok(()) diff --git a/assembly/src/assembler/mod.rs b/assembly/src/assembler/mod.rs index 2ddc94e31a..01804d83fa 100644 --- a/assembly/src/assembler/mod.rs +++ b/assembly/src/assembler/mod.rs @@ -7,7 +7,7 @@ use vm_core::{ crypto::hash::RpoDigest, debuginfo::SourceSpan, mast::{DecoratorId, MastNodeId}, - DecoratorList, Felt, Kernel, Operation, Program, + DecoratorList, Felt, Kernel, Operation, Program, WORD_SIZE, }; use crate::{ @@ -574,12 +574,14 @@ impl Assembler { let proc_body_id = if num_locals > 0 { // for procedures with locals, we need to update fmp register before and after the // procedure body is executed. specifically: - // - to allocate procedure locals we need to increment fmp by the number of locals - // - to deallocate procedure locals we need to decrement it by the same amount - let num_locals = Felt::from(num_locals); + // - to allocate procedure locals we need to increment fmp by 4 times the number of + // locals + // - to deallocate procedure locals we need to decrement it by the same amount We leave + // 4 elements between locals to properly support reading and writing words to locals. + let locals_frame = Felt::from(num_locals * WORD_SIZE as u16); let wrapper = BodyWrapper { - prologue: vec![Operation::Push(num_locals), Operation::FmpUpdate], - epilogue: vec![Operation::Push(-num_locals), Operation::FmpUpdate], + prologue: vec![Operation::Push(locals_frame), Operation::FmpUpdate], + epilogue: vec![Operation::Push(-locals_frame), Operation::FmpUpdate], }; self.compile_body(proc.iter(), &mut proc_ctx, Some(wrapper), mast_forest_builder)? } else { diff --git a/assembly/src/tests.rs b/assembly/src/tests.rs index 0cc537e3a2..f9d578e5bd 100644 --- a/assembly/src/tests.rs +++ b/assembly/src/tests.rs @@ -1800,18 +1800,19 @@ fn program_with_proc_locals() -> TestResult { mul \ end \ begin \ - push.4 push.3 push.2 \ + push.10 push.9 push.8 \ exec.foo \ end" ); let program = context.assemble(source)?; + // Note: 18446744069414584317 == -4 (mod 2^64 - 2^32 + 1) let expected = "\ begin basic_block + push(10) + push(9) + push(8) push(4) - push(3) - push(2) - push(1) fmpupdate pad fmpadd @@ -1822,7 +1823,7 @@ begin fmpadd mload mul - push(18446744069414584320) + push(18446744069414584317) fmpupdate end end"; 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..53cf3cecdb 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(); @@ -329,7 +329,7 @@ fn execute( } // loads the memory at the latest clock cycle. - let mem_state = chiplets.get_mem_state_at(ContextId::root(), system.clk()); + let mem_state = chiplets.memory().get_state_at(ContextId::root(), system.clk()); // loads the stack along with the overflow values at the latest clock cycle. let stack_state = stack.get_state_at(system.clk()); @@ -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 99f78d0a62..0b03df31fb 100644 --- a/processor/src/chiplets/memory/mod.rs +++ b/processor/src/chiplets/memory/mod.rs @@ -2,10 +2,14 @@ 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, V_COL_RANGE, + 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, }, RowIndex, }; +use vm_core::{WORD_SIZE, ZERO}; use super::{ utils::{split_element_u32_into_u16, split_u32_into_u16}, @@ -14,7 +18,7 @@ use super::{ use crate::{system::ContextId, ExecutionError}; mod segment; -use segment::MemorySegmentTrace; +use segment::{MemoryOperation, MemorySegmentTrace}; #[cfg(test)] mod tests; @@ -34,46 +38,49 @@ const INIT_MEM_VALUE: Word = EMPTY_WORD; /// building an execution trace of all memory accesses. /// /// The memory is comprised of one or more segments, each segment accessible from a specific -/// execution context. The root (kernel) context has context ID 0, and all additional contexts -/// have increasing IDs. Within each segment, the memory is word-addressable. That is, four field -/// elements are located at each memory address, and we can read and write elements to/from memory -/// in batches of four. +/// execution context. The root (kernel) context has context ID 0, and all additional contexts have +/// increasing IDs. Within each segment, the memory is element-addressable, even though the trace +/// tracks batches of four elements for optimization purposes. That is, a single field element is +/// located at each memory address, and we can read and write elements to/from memory either +/// individually or in batches of four. /// -/// Memory for a a given address is always initialized to zeros. That is, reading from an address -/// before writing to it will return four ZERO elements. +/// Memory for a given address is always initialized to zero. That is, reading from an address +/// before writing to it will return ZERO. /// /// ## Execution trace /// The layout of the memory access trace is shown below. /// -/// s0 s1 ctx addr clk v0 v1 v2 v3 d0 d1 d_inv -/// ├────┴────┴────┴──────┴─────┴────┴────┴────┴────┴────┴────┴───────┤ +/// rw ew ctx batch idx0 idx1 clk v0 v1 v2 v3 d0 d1 d_inv f_scb +/// ├────┴────┴────┴───────┴──────┴──────┴────┴────┴────┴────┴────┴────┴────┴───────┴───────┤ /// /// In the above, the meaning of the columns is as follows: -/// - `s0` is a selector column used to identify whether the memory access is a read or a write. A -/// value of ZERO indicates a write, and ONE indicates a read. -/// - `s1` is a selector column used to identify whether the memory access is a read of an existing -/// memory value or not (i.e., this context/addr combination already existed and is being read). A -/// value of ONE indicates a read of existing memory, meaning the previous value must be copied. +/// - `rw` is a selector column used to identify whether the memory operation is a read or a write. +/// - `ew` is a selector column used to identify whether the memory operation is over an element or +/// a word. /// - `ctx` contains execution context ID. Values in this column must increase monotonically but /// there can be gaps between two consecutive context IDs of up to 2^32. Also, two consecutive /// values can be the same. -/// - `addr` contains memory address. Values in this column must increase monotonically for a given -/// context but there can be gaps between two consecutive values of up to 2^32. Also, two -/// consecutive values can be the same. -/// - `clk` contains clock cycle at which a memory operation happened. Values in this column must -/// increase monotonically for a given context and memory address but there can be gaps between -/// two consecutive values of up to 2^32. -/// - Columns `v0`, `v1`, `v2`, `v3` contain field elements stored at a given context/address/clock +/// - `batch` contains the the index of the batch of addresses, which is the address of the first +/// element in the batch. For example, the value of `batch` for the batch of addresses 40, 41, 42, +/// and 43 is 40. Note then that the first address of a batch *must* be divisible by 4. Values in +/// this column must increase monotonically for a given context but there can be gaps between two +/// consecutive values of up to 2^32. Also, two consecutive values can be the same. +/// - `clk` contains the clock cycle at which a memory operation happened. Values in this column +/// must increase monotonically for a given context and batch but there can be gaps between two +/// consecutive values of up to 2^32. +/// - Columns `v0`, `v1`, `v2`, `v3` contain field elements stored at a given context/batch/clock /// cycle after the memory operation. /// - Columns `d0` and `d1` contain lower and upper 16 bits of the delta between two consecutive -/// context IDs, addresses, or clock cycles. Specifically: +/// context IDs, batches, or clock cycles. Specifically: /// - When the context changes, these columns contain (`new_ctx` - `old_ctx`). -/// - When the context remains the same but the address changes, these columns contain (`new_addr` -/// - `old-addr`). -/// - When both the context and the address remain the same, these columns contain (`new_clk` - +/// - When the context remains the same but the batch changes, these columns contain (`new_batch` +/// - `old_batch`). +/// - When both the context and the batch remain the same, these columns contain (`new_clk` - /// `old_clk` - 1). -/// - `d_inv` contains the inverse of the delta between two consecutive context IDs, addresses, or +/// - `d_inv` contains the inverse of the delta between two consecutive context IDs, batches, or /// clock cycles computed as described above. +/// - `f_scb` is a flag indicating whether the context and the batch are the same as in the next +/// row. /// /// For the first row of the trace, values in `d0`, `d1`, and `d_inv` are set to zeros. #[derive(Debug, Default)] @@ -101,25 +108,31 @@ 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: u32) -> Option { + pub fn get_value(&self, ctx: ContextId, addr: u32) -> Option { match self.trace.get(&ctx) { Some(segment) => segment.get_value(addr), 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: u32) -> 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) + /// Returns the word located in memory starting at the specified address, which must be word + /// aligned. + /// + /// # Errors + /// - Returns an error if `addr` is not word aligned. + pub fn get_word(&self, ctx: ContextId, addr: u32) -> Result, ExecutionError> { + match self.trace.get(&ctx) { + Some(segment) => segment + .get_word(addr) + .map_err(|_| ExecutionError::UnalignedMemoryWordAccess { addr, ctx }), + None => Ok(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![]; } @@ -130,41 +143,111 @@ impl Memory { } } - // STATE ACCESSORS AND MUTATORS + // STATE MUTATORS // -------------------------------------------------------------------------------------------- + /// Returns the field element located in memory at the specified context/address. + /// + /// If the specified address hasn't been previously written to, ZERO is returned. This + /// effectively implies that memory is initialized to ZERO. + /// + /// # Errors + /// - Returns an error if the address is equal or greater than 2^32. + /// - 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: Felt, + clk: RowIndex, + ) -> Result { + let addr: u32 = addr + .as_int() + .try_into() + .map_err(|_| ExecutionError::MemoryAddressOutOfBounds(addr.as_int()))?; + self.num_trace_rows += 1; + self.trace.entry(ctx).or_default().read(ctx, addr, Felt::from(clk)) + } + /// Returns a word located in memory at the specified context/address. /// /// If the specified address hasn't been previously written to, four ZERO elements are /// returned. This effectively implies that memory is initialized to ZERO. /// /// # Errors + /// - Returns an error if the address is equal or greater than 2^32. + /// - Returns an error if the address is not aligned to a word boundary. /// - Returns an error if the same address is accessed more than once in the same clock cycle. - pub fn read( + pub fn read_word( &mut self, ctx: ContextId, - addr: u32, + addr: Felt, clk: RowIndex, ) -> Result { + let addr: u32 = addr + .as_int() + .try_into() + .map_err(|_| ExecutionError::MemoryAddressOutOfBounds(addr.as_int()))?; + if addr % WORD_SIZE as u32 != 0 { + return Err(ExecutionError::MemoryUnalignedWordAccess { + addr, + ctx, + clk: Felt::from(clk), + }); + } + self.num_trace_rows += 1; - self.trace.entry(ctx).or_default().read(ctx, addr, Felt::from(clk)) + self.trace.entry(ctx).or_default().read_word(ctx, addr, Felt::from(clk)) } - /// Writes the provided word at the specified context/address. + /// Writes the provided field element at the specified context/address. /// /// # Errors + /// - Returns an error if the address is equal or greater than 2^32. /// - Returns an error if the same address is accessed more than once in the same clock cycle. pub fn write( &mut self, ctx: ContextId, - addr: u32, + addr: Felt, clk: RowIndex, - value: Word, + value: Felt, ) -> Result<(), ExecutionError> { + let addr: u32 = addr + .as_int() + .try_into() + .map_err(|_| ExecutionError::MemoryAddressOutOfBounds(addr.as_int()))?; self.num_trace_rows += 1; self.trace.entry(ctx).or_default().write(ctx, addr, Felt::from(clk), value) } + /// Writes the provided word at the specified context/address. + /// + /// # Errors + /// - Returns an error if the address is equal or greater than 2^32. + /// - Returns an error if the address is not aligned to a word boundary. + /// - Returns an error if the same address is accessed more than once in the same clock cycle. + pub fn write_word( + &mut self, + ctx: ContextId, + addr: Felt, + clk: RowIndex, + value: Word, + ) -> Result<(), ExecutionError> { + let addr: u32 = addr + .as_int() + .try_into() + .map_err(|_| ExecutionError::MemoryAddressOutOfBounds(addr.as_int()))?; + if addr % WORD_SIZE as u32 != 0 { + return Err(ExecutionError::MemoryUnalignedWordAccess { + addr, + ctx, + clk: Felt::from(clk), + }); + } + + self.num_trace_rows += 1; + self.trace.entry(ctx).or_default().write_word(ctx, addr, Felt::from(clk), value) + } + // EXECUTION TRACE GENERATION // -------------------------------------------------------------------------------------------- @@ -224,7 +307,7 @@ impl Memory { }; // iterate through addresses in ascending order, and write trace row for each memory access - // into the trace. we expect the trace to be 14 columns wide. + // into the trace. we expect the trace to be 15 columns wide. let mut row: RowIndex = 0.into(); for (ctx, segment) in self.trace { @@ -235,13 +318,35 @@ impl Memory { let felt_addr = Felt::from(addr); for memory_access in addr_trace { let clk = memory_access.clk(); - let value = memory_access.value(); + let value = memory_access.batch(); - let selectors = memory_access.op_selectors(); - trace.set(row, 0, selectors[0]); - trace.set(row, 1, selectors[1]); + match memory_access.operation() { + MemoryOperation::Read => trace.set(row, READ_WRITE_COL_IDX, MEMORY_READ), + MemoryOperation::Write => trace.set(row, READ_WRITE_COL_IDX, MEMORY_WRITE), + } + let (idx1, idx0) = match memory_access.access_type() { + segment::MemoryAccessType::Element { + addr_idx_in_batch: addr_idx_in_word, + } => { + trace.set(row, ELEMENT_OR_WORD_COL_IDX, MEMORY_ACCESS_ELEMENT); + + match addr_idx_in_word { + 0 => (ZERO, ZERO), + 1 => (ZERO, ONE), + 2 => (ONE, ZERO), + 3 => (ONE, ONE), + _ => panic!("invalid address index in word: {addr_idx_in_word}"), + } + }, + segment::MemoryAccessType::Word => { + trace.set(row, ELEMENT_OR_WORD_COL_IDX, MEMORY_ACCESS_WORD); + (ZERO, ZERO) + }, + }; 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); for (idx, col) in V_COL_RANGE.enumerate() { trace.set(row, col, value[idx]); @@ -262,6 +367,12 @@ impl Memory { // TODO: switch to batch inversion to improve efficiency. trace.set(row, D_INV_COL_IDX, delta.inv()); + if prev_ctx == ctx && prev_addr == felt_addr { + trace.set(row, FLAG_SAME_BATCH_AND_CONTEXT, ONE); + } else { + trace.set(row, FLAG_SAME_BATCH_AND_CONTEXT, ZERO); + }; + // update values for the next iteration of the loop prev_ctx = ctx; prev_addr = felt_addr; @@ -291,9 +402,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 4957286564..dd9b014217 100644 --- a/processor/src/chiplets/memory/segment.rs +++ b/processor/src/chiplets/memory/segment.rs @@ -3,10 +3,8 @@ use alloc::{ vec::Vec, }; -use miden_air::{ - trace::chiplets::memory::{Selectors, MEMORY_COPY_READ, MEMORY_INIT_READ, MEMORY_WRITE}, - RowIndex, -}; +use miden_air::RowIndex; +use vm_core::WORD_SIZE; use super::{Felt, Word, INIT_MEM_VALUE}; use crate::{ContextId, ExecutionError}; @@ -26,21 +24,43 @@ 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) { - Some(addr_trace) => addr_trace.last().map(|access| access.value()), + 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, } } + /// Returns the word located in memory starting at the specified address, which must be word + /// aligned. + /// + /// # Errors + /// - Returns an error if `addr` is not word aligned. + pub fn get_word(&self, addr: u32) -> Result, ()> { + if addr % WORD_SIZE as u32 != 0 { + return Err(()); + } + + let (batch, _) = addr_to_batch_and_idx(addr); + + match self.0.get(&batch) { + Some(addr_trace) => Ok(addr_trace.last().map(|access| access.batch())), + None => Ok(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; @@ -53,13 +73,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].value())), + Ok(i) => { + let batch = addr_trace[i].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].value())); + 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]), + ]); } }, } @@ -71,62 +107,136 @@ impl MemorySegmentTrace { // STATE MUTATORS // -------------------------------------------------------------------------------------------- - /// Returns a word located in memory at the specified address. The memory access is assumed - /// to happen at the provided clock cycle. + /// Returns the element located at the specified address. The memory access is assumed to happen + /// at the provided clock cycle. + /// + /// If the element at the specified address hasn't been previously written to, ZERO is returned. + /// + /// # 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 (batch, addr_idx_in_word) = addr_to_batch_and_idx(addr); + + let batch_values = self.read_batch( + ctx, + batch, + clk, + MemoryAccessType::Element { addr_idx_in_batch: addr_idx_in_word }, + )?; + + Ok(batch_values[addr_idx_in_word as usize]) + } + + /// Returns a word located in memory starting at the specified address, which must be word + /// aligned. The memory access is assumed to happen at the provided clock cycle. + /// + /// If the word starting at the specified address hasn't been previously written to, four ZERO + /// elements are returned. This effectively implies that memory is initialized to ZERO. + /// + /// # Errors + /// - Returns an error if the same address is accessed more than once in the same clock cycle. + pub fn read_word( + &mut self, + ctx: ContextId, + addr: u32, + clk: Felt, + ) -> Result { + debug_assert!(addr % 4 == 0, "unaligned word access: {addr}"); + + let (batch, _) = addr_to_batch_and_idx(addr); + self.read_batch(ctx, batch, clk, MemoryAccessType::Word) + } + + /// Writes the element located at the specified address. The memory access is assumed to happen + /// at the provided clock cycle. /// - /// If the specified address hasn't been previously written to, four ZERO elements are - /// returned. This effectively implies that memory is initialized to ZERO. + /// If the element at the specified address hasn't been previously written to, ZERO is returned. /// /// # 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 { - // look up the previous value in the appropriate address trace and add (clk, prev_value) - // to it; if this is the first time we access this address, create address trace for it - // with entry (clk, [ZERO, 4]). in both cases, return the last value in the address trace. - match self.0.entry(addr) { + pub fn write( + &mut self, + ctx: ContextId, + addr: u32, + clk: Felt, + value: Felt, + ) -> Result<(), ExecutionError> { + let (batch, addr_idx_in_word) = addr_to_batch_and_idx(addr); + + match self.0.entry(batch) { Entry::Vacant(vacant_entry) => { - let access = - MemorySegmentAccess::new(clk, MemoryOperation::InitRead, INIT_MEM_VALUE); + // If this is the first access to the ctx/batch pair, then all values in the batch + // are initialized to 0, except for the address being written. + let batch = { + let mut batch = Word::default(); + batch[addr_idx_in_word as usize] = value; + batch + }; + + let access = MemorySegmentAccess::new( + clk, + MemoryOperation::Write, + MemoryAccessType::Element { addr_idx_in_batch: addr_idx_in_word }, + batch, + ); vacant_entry.insert(vec![access]); - Ok(INIT_MEM_VALUE) + Ok(()) }, Entry::Occupied(mut occupied_entry) => { + // If the ctx/batch pair has been accessed before, then the values in the batch are + // the same as the previous access, except for the address being written. let addr_trace = occupied_entry.get_mut(); if addr_trace.last().expect("empty address trace").clk() == clk { Err(ExecutionError::DuplicateMemoryAccess { ctx, addr, clk }) } else { - let last_value = addr_trace.last().expect("empty address trace").value(); - let access = - MemorySegmentAccess::new(clk, MemoryOperation::CopyRead, last_value); + let batch = { + let mut last_batch = + addr_trace.last().expect("empty address trace").batch(); + last_batch[addr_idx_in_word as usize] = value; + + last_batch + }; + + let access = MemorySegmentAccess::new( + clk, + MemoryOperation::Write, + MemoryAccessType::Element { addr_idx_in_batch: addr_idx_in_word }, + batch, + ); addr_trace.push(access); - Ok(last_value) + Ok(()) } }, } } - /// Writes the provided word at the specified address. The memory access is assumed to happen - /// at the provided clock cycle. + /// Writes the provided word starting at the specified address. The memory access is assumed to + /// happen at the provided clock cycle. /// /// # Errors /// - Returns an error if the same address is accessed more than once in the same clock cycle. - pub fn write( + pub fn write_word( &mut self, ctx: ContextId, addr: u32, clk: Felt, - value: Word, + word: Word, ) -> Result<(), ExecutionError> { - // add a memory access to the appropriate address trace; if this is the first time - // we access this address, initialize address trace. - let access = MemorySegmentAccess::new(clk, MemoryOperation::Write, value); - match self.0.entry(addr) { + debug_assert!(addr % 4 == 0, "unaligned memory access: {addr}"); + + let (batch, _) = addr_to_batch_and_idx(addr); + + let access = + MemorySegmentAccess::new(clk, MemoryOperation::Write, MemoryAccessType::Word, word); + match self.0.entry(batch) { Entry::Vacant(vacant_entry) => { + // All values in the batch are set to the word being written. vacant_entry.insert(vec![access]); Ok(()) }, Entry::Occupied(mut occupied_entry) => { + // All values in the batch are set to the word being written. let addr_trace = occupied_entry.get_mut(); if addr_trace.last().expect("empty address trace").clk() == clk { Err(ExecutionError::DuplicateMemoryAccess { ctx, addr, clk }) @@ -154,9 +264,55 @@ impl MemorySegmentTrace { // HELPER FUNCTIONS // -------------------------------------------------------------------------------------------- - /// Returns current size (in words) of this memory segment. + /// Records a read operation on the specified batch at the specified clock cycle. + /// + /// The access type either specifies the element in batch that was read, or that the entire word + /// was read. + fn read_batch( + &mut self, + ctx: ContextId, + batch: u32, + clk: Felt, + access_type: MemoryAccessType, + ) -> Result { + match self.0.entry(batch) { + Entry::Vacant(vacant_entry) => { + // If this is the first access to the ctx/batch pair, then all values in the batch + // are initialized to 0. + let access = MemorySegmentAccess::new( + clk, + MemoryOperation::Read, + access_type, + INIT_MEM_VALUE, + ); + vacant_entry.insert(vec![access]); + Ok(INIT_MEM_VALUE) + }, + Entry::Occupied(mut occupied_entry) => { + // If the ctx/batch pair has been accessed before, then the values in the batch are + // the same as the previous access. + let addr_trace = occupied_entry.get_mut(); + if addr_trace.last().expect("empty address trace").clk() == clk { + Err(ExecutionError::DuplicateMemoryAccess { ctx, addr: batch, clk }) + } else { + let last_batch = addr_trace.last().expect("empty address trace").batch(); + let access = MemorySegmentAccess::new( + clk, + MemoryOperation::Read, + access_type, + last_batch, + ); + addr_trace.push(access); + + Ok(last_batch) + } + }, + } + } + + /// 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() } } @@ -166,23 +322,29 @@ impl MemorySegmentTrace { #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum MemoryOperation { - InitRead, - CopyRead, + Read, Write, } +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum MemoryAccessType { + Element { addr_idx_in_batch: u8 }, + Word, +} + /// A single memory access representing the specified memory operation with the specified value at /// the specified clock cycle. #[derive(Copy, Debug, Clone)] pub struct MemorySegmentAccess { clk: Felt, - op: MemoryOperation, - value: Word, + operation: MemoryOperation, + access_type: MemoryAccessType, + batch: Word, } impl MemorySegmentAccess { - fn new(clk: Felt, op: MemoryOperation, value: Word) -> Self { - Self { clk, op, value } + fn new(clk: Felt, op: MemoryOperation, access_type: MemoryAccessType, batch: Word) -> Self { + Self { clk, operation: op, access_type, batch } } /// Returns the clock cycle at which this memory access happened. @@ -190,17 +352,32 @@ impl MemorySegmentAccess { self.clk } - /// Returns the selector values matching the operation used in this memory access. - pub(super) fn op_selectors(&self) -> Selectors { - match self.op { - MemoryOperation::InitRead => MEMORY_INIT_READ, - MemoryOperation::CopyRead => MEMORY_COPY_READ, - MemoryOperation::Write => MEMORY_WRITE, - } + /// Returns the operation associated with this memory access. + pub(super) fn operation(&self) -> MemoryOperation { + self.operation + } + + /// Returns the access type associated with this memory access. + pub(super) fn access_type(&self) -> MemoryAccessType { + self.access_type } - /// Returns the word value for this memory access. - pub(super) fn value(&self) -> Word { - self.value + /// Returns the batch associated with this memory access. + /// + /// For example, if the memory access is an element read of address 42, the batch will contain + /// the values of addresses 40, 41, 42, and 43. + pub(super) fn batch(&self) -> Word { + self.batch } } + +// HELPERS +// ================================================================================================ + +/// Splits an address into two components: +/// 1. a batch, which is the closest value to `addr` that is both smaller and word aligned, and +/// 2. the index within the batch which `addr` represents. +pub fn addr_to_batch_and_idx(addr: u32) -> (u32, u8) { + let idx = addr % WORD_SIZE as u32; + (addr - idx, idx as u8) +} diff --git a/processor/src/chiplets/memory/tests.rs b/processor/src/chiplets/memory/tests.rs index 5c169507f5..56cbce5850 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, + 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::{assert_matches, 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; +use crate::{ContextId, ExecutionError}; #[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()); } @@ -27,51 +30,98 @@ fn mem_read() { let mut mem = Memory::default(); // read a value from address 0; clk = 1 - let addr0 = 0; + let addr0 = ZERO; let value = mem.read(ContextId::root(), addr0, 1.into()).unwrap(); - assert_eq!(EMPTY_WORD, value); - assert_eq!(1, mem.size()); + assert_eq!(ZERO, value); + assert_eq!(1, mem.num_accessed_batches()); assert_eq!(1, mem.trace_len()); // read a value from address 3; clk = 2 - let addr3 = 3; + let addr3 = Felt::from(3_u32); let value = mem.read(ContextId::root(), addr3, 2.into()).unwrap(); - assert_eq!(EMPTY_WORD, value); - assert_eq!(2, mem.size()); + assert_eq!(ZERO, value); + 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!(EMPTY_WORD, value); - assert_eq!(2, mem.size()); + assert_eq!(ZERO, value); + assert_eq!(1, mem.num_accessed_batches()); assert_eq!(3, mem.trace_len()); // read a value from address 2; clk = 4 - let addr2 = 2; + let addr2 = Felt::from(2_u32); let value = mem.read(ContextId::root(), addr2, 4.into()).unwrap(); - assert_eq!(EMPTY_WORD, value); - assert_eq!(3, mem.size()); + assert_eq!(ZERO, value); + 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); + let memory_access = MemoryAccess::new( + MemoryOperation::Read, + MemoryAccessType::Element { addr_idx_in_batch: 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 { addr_idx_in_batch: 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 { addr_idx_in_batch: 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 { addr_idx_in_batch: 2 }, + ContextId::root(), + addr2, + 4.into(), + EMPTY_WORD, + ); + verify_memory_access(&trace, 3, 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); +/// Tests that writing a word to an address that is not aligned with the word boundary results in an +/// error. +#[test] +fn mem_read_word_unaligned() { + let mut mem = Memory::default(); - // 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); + // write a value into address 0; clk = 1 + let addr = ONE; + let clk = 1.into(); + let ctx = ContextId::root(); + let ret = mem.read_word(ctx, addr, clk); + + assert_matches!( + ret, + Err(ExecutionError::MemoryUnalignedWordAccess { addr: _, ctx: _, clk: _ }) + ); } #[test] @@ -79,224 +129,330 @@ fn mem_write() { let mut mem = Memory::default(); // write a value into address 0; clk = 1 - let addr0 = 0; - let value1 = [ONE, ZERO, ZERO, ZERO]; - mem.write(ContextId::root(), addr0, 1.into(), value1).unwrap(); - assert_eq!(value1, mem.get_value(ContextId::root(), addr0).unwrap()); - assert_eq!(1, mem.size()); + let addr0 = 0_u32; + let word1 = [ONE, ZERO, ZERO, ZERO]; + mem.write_word(ContextId::root(), addr0.into(), 1.into(), word1).unwrap(); + assert_eq!(word1, mem.get_word(ContextId::root(), addr0).unwrap().unwrap()); + assert_eq!(1, mem.num_accessed_batches()); assert_eq!(1, mem.trace_len()); // write a value into address 2; clk = 2 - let addr2 = 2; - let value5 = [Felt::new(5), ZERO, ZERO, ZERO]; - mem.write(ContextId::root(), addr2, 2.into(), value5).unwrap(); + let addr2 = 2_u32; + let value5 = Felt::new(5); + mem.write(ContextId::root(), addr2.into(), 2.into(), value5).unwrap(); assert_eq!(value5, mem.get_value(ContextId::root(), addr2).unwrap()); - assert_eq!(2, mem.size()); + assert_eq!(1, mem.num_accessed_batches()); assert_eq!(2, mem.trace_len()); // write a value into address 1; clk = 3 - let addr1 = 1; - let value7 = [Felt::new(7), ZERO, ZERO, ZERO]; - mem.write(ContextId::root(), addr1, 3.into(), value7).unwrap(); + let addr1 = 1_u32; + let value7 = Felt::new(7); + mem.write(ContextId::root(), addr1.into(), 3.into(), value7).unwrap(); assert_eq!(value7, mem.get_value(ContextId::root(), addr1).unwrap()); - assert_eq!(3, mem.size()); + assert_eq!(1, 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(ContextId::root(), addr0, 4.into(), value9).unwrap(); - assert_eq!(value7, mem.get_value(ContextId::root(), addr1).unwrap()); - assert_eq!(3, mem.size()); + // write a value into address 3; clk = 4 + let addr3 = 3_u32; + let value9 = Felt::new(9); + mem.write(ContextId::root(), addr3.into(), 4.into(), value9).unwrap(); + assert_eq!(value9, mem.get_value(ContextId::root(), addr3).unwrap()); + assert_eq!(1, mem.num_accessed_batches()); assert_eq!(4, mem.trace_len()); + // write a word into address 4; clk = 5 + let addr4 = 4_u32; + let word1234 = [ONE, 2_u32.into(), 3_u32.into(), 4_u32.into()]; + mem.write_word(ContextId::root(), addr4.into(), 5.into(), word1234).unwrap(); + assert_eq!(word1234, mem.get_word(ContextId::root(), addr4).unwrap().unwrap()); + assert_eq!(2, mem.num_accessed_batches()); + assert_eq!(5, mem.trace_len()); + + // write a word into address 0; clk = 6 + let word5678: [Felt; 4] = [5_u32.into(), 6_u32.into(), 7_u32.into(), 8_u32.into()]; + mem.write_word(ContextId::root(), addr0.into(), 6.into(), word5678).unwrap(); + assert_eq!(word5678, mem.get_word(ContextId::root(), addr0).unwrap().unwrap()); + assert_eq!(2, mem.num_accessed_batches()); + assert_eq!(6, mem.trace_len()); + // check generated trace and memory data provided to the ChipletsBus; rows should be sorted by // address and then clock cycle - let trace = build_trace(mem, 4); + let trace = build_trace(mem, 6); - // address 0 + // batch 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, &memory_access, prev_row); - - let memory_access = MemoryAccess::new(ContextId::root(), addr0, 4.into(), value9); - prev_row = verify_memory_access(&trace, 1, MEMORY_WRITE, &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, &memory_access, prev_row); - - // address 2 - let memory_access = MemoryAccess::new(ContextId::root(), addr2, 2.into(), value5); - verify_memory_access(&trace, 3, MEMORY_WRITE, &memory_access, prev_row); + let memory_access = MemoryAccess::new( + MemoryOperation::Write, + MemoryAccessType::Word, + ContextId::root(), + addr0.into(), + 1.into(), + word1, + ); + prev_row = verify_memory_access(&trace, 0, memory_access, prev_row); + + let memory_access = MemoryAccess::new( + MemoryOperation::Write, + MemoryAccessType::Element { addr_idx_in_batch: 2 }, + ContextId::root(), + addr2.into(), + 2.into(), + [ONE, ZERO, value5, ZERO], + ); + prev_row = verify_memory_access(&trace, 1, memory_access, prev_row); + + let memory_access = MemoryAccess::new( + MemoryOperation::Write, + MemoryAccessType::Element { addr_idx_in_batch: 1 }, + ContextId::root(), + addr1.into(), + 3.into(), + [ONE, value7, value5, ZERO], + ); + prev_row = verify_memory_access(&trace, 2, memory_access, prev_row); + + let memory_access = MemoryAccess::new( + MemoryOperation::Write, + MemoryAccessType::Element { addr_idx_in_batch: 3 }, + ContextId::root(), + addr3.into(), + 4.into(), + [ONE, value7, value5, value9], + ); + prev_row = verify_memory_access(&trace, 3, memory_access, prev_row); + + let memory_access = MemoryAccess::new( + MemoryOperation::Write, + MemoryAccessType::Word, + ContextId::root(), + addr0.into(), + 6.into(), + word5678, + ); + prev_row = verify_memory_access(&trace, 4, memory_access, prev_row); + + // batch 1 + let memory_access = MemoryAccess::new( + MemoryOperation::Write, + MemoryAccessType::Word, + ContextId::root(), + addr4.into(), + 5.into(), + word1234, + ); + verify_memory_access(&trace, 5, memory_access, prev_row); } +/// Tests that writing a word to an address that is not aligned with the word boundary results in an +/// error. #[test] -fn mem_write_read() { +fn mem_write_word_unaligned() { let mut mem = Memory::default(); - // write 1 into address 5; clk = 1 - let addr5 = 5; - let value1 = [ONE, ZERO, ZERO, ZERO]; - mem.write(ContextId::root(), addr5, 1.into(), value1).unwrap(); - - // write 4 into address 2; clk = 2 - let addr2 = 2; - let value4 = [Felt::new(4), ZERO, ZERO, ZERO]; - mem.write(ContextId::root(), addr2, 2.into(), value4).unwrap(); - - // read a value from address 5; clk = 3 - mem.read(ContextId::root(), addr5, 3.into()).unwrap(); - - // write 2 into address 5; clk = 4 - let value2 = [Felt::new(2), ZERO, ZERO, ZERO]; - mem.write(ContextId::root(), addr5, 4.into(), value2).unwrap(); - - // read a value from address 2; clk = 5 - mem.read(ContextId::root(), addr2, 5.into()).unwrap(); - - // write 7 into address 2; clk = 6 - let value7 = [Felt::new(7), ZERO, ZERO, ZERO]; - mem.write(ContextId::root(), addr2, 6.into(), value7).unwrap(); - - // read a value from address 5; clk = 7 - mem.read(ContextId::root(), addr5, 7.into()).unwrap(); - - // read a value from address 2; clk = 8 - mem.read(ContextId::root(), addr2, 8.into()).unwrap(); - - // read a value from address 5; clk = 9 - mem.read(ContextId::root(), addr5, 9.into()).unwrap(); - - // check generated trace and memory data provided to the ChipletsBus; rows should be sorted by - // address and then clock cycle - let trace = build_trace(mem, 9); - - // 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, &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, &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); - - // address 5 - let memory_access = MemoryAccess::new(ContextId::root(), addr5, 1.into(), value1); - prev_row = verify_memory_access(&trace, 4, MEMORY_WRITE, &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, &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); + // write a value into address 0; clk = 1 + let addr = ONE; + let word1 = [ONE, ZERO, ZERO, ZERO]; + let clk = 1.into(); + let ctx = ContextId::root(); + let ret = mem.write_word(ctx, addr, clk, word1); + + assert_matches!( + ret, + Err(ExecutionError::MemoryUnalignedWordAccess { addr: _, ctx: _, clk: _ }) + ); } +/// Tests that values written are properly read back. #[test] -fn mem_multi_context() { +fn mem_write_read() { let mut mem = Memory::default(); - - // write a value into ctx = ContextId::root(), addr = 0; clk = 1 - let value1 = [ONE, ZERO, ZERO, ZERO]; - mem.write(ContextId::root(), 0, 1.into(), value1).unwrap(); - assert_eq!(value1, mem.get_value(ContextId::root(), 0).unwrap()); - assert_eq!(1, mem.size()); - assert_eq!(1, mem.trace_len()); - - // write a value into ctx = 3, addr = 1; clk = 4 - let value2 = [ZERO, ONE, ZERO, ZERO]; - mem.write(3.into(), 1, 4.into(), value2).unwrap(); - assert_eq!(value2, mem.get_value(3.into(), 1).unwrap()); - assert_eq!(2, mem.size()); - assert_eq!(2, mem.trace_len()); - - // read a value from ctx = 3, addr = 1; clk = 6 - let value = mem.read(3.into(), 1, 6.into()).unwrap(); - assert_eq!(value2, value); - assert_eq!(2, mem.size()); - assert_eq!(3, mem.trace_len()); - - // write a value into ctx = 3, addr = 0; clk = 7 - let value3 = [ZERO, ZERO, ONE, ZERO]; - mem.write(3.into(), 0, 7.into(), value3).unwrap(); - assert_eq!(value3, mem.get_value(3.into(), 0).unwrap()); - assert_eq!(3, mem.size()); - assert_eq!(4, mem.trace_len()); - - // read a value from ctx = 0, addr = 0; clk = 9 - let value = mem.read(ContextId::root(), 0, 9.into()).unwrap(); - assert_eq!(value1, value); - assert_eq!(3, mem.size()); - assert_eq!(5, mem.trace_len()); + let mut clk: RowIndex = 1.into(); + + // write [1,2,3,4] starting at address 0; clk = 1 + let word1234 = [ONE, 2_u32.into(), 3_u32.into(), 4_u32.into()]; + mem.write_word(ContextId::root(), ZERO, clk, word1234).unwrap(); + clk += 1; + + // read individual values from addresses 3,2,1,0; clk = 2,3,4,5 + let value_read = mem.read(ContextId::root(), 3_u32.into(), clk).unwrap(); + assert_eq!(value_read, 4_u32.into()); + clk += 1; + let value_read = mem.read(ContextId::root(), 2_u32.into(), clk).unwrap(); + assert_eq!(value_read, 3_u32.into()); + clk += 1; + let value_read = mem.read(ContextId::root(), 1_u32.into(), clk).unwrap(); + assert_eq!(value_read, 2_u32.into()); + clk += 1; + let value_read = mem.read(ContextId::root(), ZERO, clk).unwrap(); + assert_eq!(value_read, 1_u32.into()); + clk += 1; + + // read word from address 0; clk = 6 + let word_read = mem.read_word(ContextId::root(), ZERO, clk).unwrap(); + assert_eq!(word_read, word1234); + clk += 1; + + // write 42 into address 2; clk = 7 + mem.write(ContextId::root(), 2_u32.into(), clk, 42_u32.into()).unwrap(); + clk += 1; + + // read element from address 2; clk = 8 + let value_read = mem.read(ContextId::root(), 2_u32.into(), clk).unwrap(); + assert_eq!(value_read, 42_u32.into()); + clk += 1; + + // read word from address 0; clk = 9 + let word_read = mem.read_word(ContextId::root(), ZERO, clk).unwrap(); + assert_eq!(word_read, [ONE, 2_u32.into(), 42_u32.into(), 4_u32.into()]); + clk += 1; // check generated trace and memory data provided to the ChipletsBus; rows should be sorted by // address and then clock cycle - let trace = build_trace(mem, 5); + let trace = build_trace(mem, 9); + let mut clk: RowIndex = 1.into(); - // ctx = 0, addr = 0 + // address 2 let mut prev_row = [ZERO; MEMORY_TRACE_WIDTH]; - let memory_access = MemoryAccess::new(ContextId::root(), 0, 1.into(), value1); - prev_row = verify_memory_access(&trace, 0, MEMORY_WRITE, &memory_access, prev_row); - - let memory_access = MemoryAccess::new(ContextId::root(), 0, 9.into(), value1); - prev_row = verify_memory_access(&trace, 1, MEMORY_COPY_READ, &memory_access, prev_row); - - // ctx = 3, addr = 0 - let memory_access = MemoryAccess::new(3.into(), 0, 7.into(), value3); - prev_row = verify_memory_access(&trace, 2, MEMORY_WRITE, &memory_access, prev_row); - - // ctx = 3, addr = 1 - let memory_access = MemoryAccess::new(3.into(), 1, 4.into(), value2); - prev_row = verify_memory_access(&trace, 3, MEMORY_WRITE, &memory_access, prev_row); - - let memory_access = MemoryAccess::new(3.into(), 1, 6.into(), value2); - verify_memory_access(&trace, 4, MEMORY_COPY_READ, &memory_access, prev_row); + let memory_access = MemoryAccess::new( + MemoryOperation::Write, + MemoryAccessType::Word, + ContextId::root(), + ZERO, + clk, + word1234, + ); + prev_row = verify_memory_access(&trace, 0, memory_access, prev_row); + clk += 1; + + let memory_access = MemoryAccess::new( + MemoryOperation::Read, + MemoryAccessType::Element { addr_idx_in_batch: 3 }, + ContextId::root(), + 3_u32.into(), + clk, + word1234, + ); + prev_row = verify_memory_access(&trace, 1, memory_access, prev_row); + clk += 1; + + let memory_access = MemoryAccess::new( + MemoryOperation::Read, + MemoryAccessType::Element { addr_idx_in_batch: 2 }, + ContextId::root(), + 2_u32.into(), + clk, + word1234, + ); + prev_row = verify_memory_access(&trace, 2, memory_access, prev_row); + clk += 1; + + let memory_access = MemoryAccess::new( + MemoryOperation::Read, + MemoryAccessType::Element { addr_idx_in_batch: 1 }, + ContextId::root(), + 1_u32.into(), + clk, + word1234, + ); + prev_row = verify_memory_access(&trace, 3, memory_access, prev_row); + clk += 1; + + let memory_access = MemoryAccess::new( + MemoryOperation::Read, + MemoryAccessType::Element { addr_idx_in_batch: 0 }, + ContextId::root(), + ZERO, + clk, + word1234, + ); + prev_row = verify_memory_access(&trace, 4, memory_access, prev_row); + clk += 1; + + let memory_access = MemoryAccess::new( + MemoryOperation::Read, + MemoryAccessType::Word, + ContextId::root(), + ZERO, + clk, + word1234, + ); + prev_row = verify_memory_access(&trace, 5, memory_access, prev_row); + clk += 1; + + let memory_access = MemoryAccess::new( + MemoryOperation::Write, + MemoryAccessType::Element { addr_idx_in_batch: 2 }, + ContextId::root(), + 2_u32.into(), + clk, + [ONE, 2_u32.into(), 42_u32.into(), 4_u32.into()], + ); + prev_row = verify_memory_access(&trace, 6, memory_access, prev_row); + clk += 1; + + let memory_access = MemoryAccess::new( + MemoryOperation::Read, + MemoryAccessType::Element { addr_idx_in_batch: 2 }, + ContextId::root(), + 2_u32.into(), + clk, + [ONE, 2_u32.into(), 42_u32.into(), 4_u32.into()], + ); + prev_row = verify_memory_access(&trace, 7, memory_access, prev_row); + clk += 1; + + let memory_access = MemoryAccess::new( + MemoryOperation::Read, + MemoryAccessType::Word, + ContextId::root(), + ZERO, + clk, + [ONE, 2_u32.into(), 42_u32.into(), 4_u32.into()], + ); + verify_memory_access(&trace, 8, memory_access, prev_row); } #[test] fn mem_get_state_at() { let mut mem = Memory::default(); - // 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(ContextId::root(), 5, 1.into(), value1).unwrap(); + let addr_start: u32 = 40_u32; - // 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(ContextId::root(), 2, 2.into(), value4).unwrap(); + // Write word starting at (ctx = 0, addr = 40) at clk = 1. + // This means that mem[40..43] is set at the beginning of clk = 2 + let word1234 = [ONE, 2_u32.into(), 3_u32.into(), 4_u32.into()]; + mem.write_word(ContextId::root(), addr_start.into(), 1.into(), word1234) + .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(3.into(), 3, 4.into(), value7).unwrap(); + let word4567: [Felt; 4] = [4_u32.into(), 5_u32.into(), 6_u32.into(), 7_u32.into()]; + mem.write_word(ContextId::root(), addr_start.into(), 2.into(), word4567) + .unwrap(); // Check memory state at clk = 2 - assert_eq!(mem.get_state_at(ContextId::root(), 2.into()), vec![(5, value1)]); - assert_eq!(mem.get_state_at(3.into(), 2.into()), vec![]); + let clk: RowIndex = 2.into(); + assert_eq!( + mem.get_state_at(ContextId::root(), clk), + vec![ + (addr_start.into(), word1234[0]), + (u64::from(addr_start) + 1_u64, word1234[1]), + (u64::from(addr_start) + 2_u64, word1234[2]), + (u64::from(addr_start) + 3_u64, word1234[3]) + ] + ); + assert_eq!(mem.get_state_at(3.into(), clk), vec![]); // Check memory state at clk = 3 - assert_eq!(mem.get_state_at(ContextId::root(), 3.into()), vec![(2, value4), (5, value1)]); - assert_eq!(mem.get_state_at(3.into(), 3.into()), vec![]); - - // Check memory state at clk = 4 - assert_eq!(mem.get_state_at(ContextId::root(), 4.into()), vec![(2, value4), (5, value1)]); - assert_eq!(mem.get_state_at(3.into(), 4.into()), vec![]); - - // Check memory state at clk = 5 - assert_eq!(mem.get_state_at(ContextId::root(), 5.into()), vec![(2, value4), (5, value1)]); - assert_eq!(mem.get_state_at(3.into(), 5.into()), vec![(3, value7)]); + let clk: RowIndex = 3.into(); + assert_eq!( + mem.get_state_at(ContextId::root(), clk), + vec![ + (addr_start.into(), word4567[0]), + (u64::from(addr_start) + 1_u64, word4567[1]), + (u64::from(addr_start) + 2_u64, word4567[2]), + (u64::from(addr_start) + 3_u64, word4567[3]) + ] + ); + assert_eq!(mem.get_state_at(3.into(), clk), vec![]); } // HELPER STRUCT & FUNCTIONS @@ -304,19 +460,35 @@ 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: u32, clk: RowIndex, word: Word) -> Self { + 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_batch: 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: Felt::from(addr), + addr, clk: Felt::from(clk), - word, + batch_values, } } } @@ -339,29 +511,57 @@ 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, + } = memory_access; + + let (batch, idx1, idx0) = { + let addr: u32 = addr.try_into().unwrap(); + let remainder = addr % WORD_SIZE as u32; + let batch = Felt::from(addr - remainder); + + 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]; - row[V_COL_RANGE.start + 2] = new_val[2]; - row[V_COL_RANGE.start + 3] = new_val[3]; + row[V_COL_RANGE.start] = batch_values[0]; + row[V_COL_RANGE.start + 1] = batch_values[1]; + row[V_COL_RANGE.start + 2] = batch_values[2]; + row[V_COL_RANGE.start + 3] = batch_values[3]; 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 }; @@ -372,18 +572,24 @@ 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); - assert_eq!(expected_row, read_trace_row(trace, row as usize)); + let expected_row = build_trace_row(mem_access, prev_row); + let actual_row = read_trace_row(trace, row as usize); + assert_eq!(expected_row, actual_row); expected_row } diff --git a/processor/src/chiplets/mod.rs b/processor/src/chiplets/mod.rs index d2c8c13aae..c36dda1945 100644 --- a/processor/src/chiplets/mod.rs +++ b/processor/src/chiplets/mod.rs @@ -10,7 +10,6 @@ use super::{ crypto::MerklePath, utils, ChipletsTrace, ExecutionError, Felt, FieldElement, RangeChecker, TraceFragment, Word, CHIPLETS_WIDTH, EMPTY_WORD, ONE, ZERO, }; -use crate::system::ContextId; mod bitwise; use bitwise::Bitwise; @@ -44,7 +43,8 @@ mod tests; /// * Hasher segment: contains the trace and selector for the hasher chiplet. This segment fills the /// first rows of the trace up to the length of the hasher `trace_len`. /// - column 0: selector column with values set to ZERO -/// - columns 1-17: execution trace of hash chiplet +/// - columns 1-16: execution trace of hash chiplet +/// - column 17: unused column padded with ZERO /// * Bitwise segment: contains the trace and selectors for the bitwise chiplet. This segment begins /// at the end of the hasher segment and fills the next rows of the trace for the `trace_len` of /// the bitwise chiplet. @@ -52,13 +52,12 @@ mod tests; /// - column 1: selector column with values set to ZERO /// - columns 2-14: execution trace of bitwise chiplet /// - columns 15-17: unused columns padded with ZERO -/// * Memory segment: contains the trace and selectors for the memory chiplet * This segment begins +/// * Memory segment: contains the trace and selectors for the memory chiplet. This segment begins /// at the end of the bitwise segment and fills the next rows of the trace for the `trace_len` of /// the memory chiplet. /// - column 0-1: selector columns with values set to ONE /// - column 2: selector column with values set to ZERO -/// - columns 3-14: execution trace of memory chiplet -/// - columns 15-17: unused column padded with ZERO +/// - columns 3-17: execution trace of memory chiplet /// * Kernel ROM segment: contains the trace and selectors for the kernel ROM chiplet * This segment /// begins at the end of the memory segment and fills the next rows of the trace for the /// `trace_len` of the kernel ROM chiplet. @@ -89,11 +88,11 @@ mod tests; /// | . | . | selectors | |-------------| /// | . | 0 | | |-------------| /// | . +---+---+-----------------------------------------------+-------------+ -/// | . | 1 | 0 | | |-------------| -/// | . | . | . | Memory chiplet | Memory chiplet |-------------| -/// | . | . | . | internal | 12 columns |-- Padding --| -/// | . | . | . | selectors | constraint degree 9 |-------------| -/// | . | . | 0 | | |-------------| +/// | . | 1 | 0 | |-------------| +/// | . | . | . | Memory chiplet |-------------| +/// | . | . | . | 15 columns |-- Padding --| +/// | . | . | . | constraint degree 9 |-------------| +/// | . | . | 0 | |-------------| /// | . + . |---+---+-------------------------------------------+-------------+ /// | . | . | 1 | 0 | | |-------------| /// | . | . | . | . | Kernel ROM | Kernel ROM chiplet |-------------| @@ -286,91 +285,14 @@ impl Chiplets { // MEMORY CHIPLET ACCESSORS // -------------------------------------------------------------------------------------------- - /// Returns a word located in memory at the specified context/address while recording the - /// memory access in the memory trace. - /// - /// If the specified address hasn't been previously written to, four ZERO elements are - /// returned. This effectively implies that memory is initialized to ZERO. - pub fn read_mem(&mut self, ctx: ContextId, addr: u32) -> Result { - // read the word from memory - self.memory.read(ctx, addr, self.clk) - } - - /// Returns two words read from consecutive addresses started with `addr` in the specified - /// context while recording memory accesses in the memory trace. - /// - /// If either of the accessed addresses hasn't been previously written to, ZERO elements are - /// returned. This effectively implies that memory is initialized to ZERO. - pub fn read_mem_double( - &mut self, - ctx: ContextId, - addr: u32, - ) -> Result<[Word; 2], ExecutionError> { - // read two words from memory: from addr and from addr + 1 - let addr2 = addr + 1; - Ok([self.memory.read(ctx, addr, self.clk)?, self.memory.read(ctx, addr2, self.clk)?]) - } - - /// Writes the provided word at the specified context/address. - pub fn write_mem( - &mut self, - ctx: ContextId, - addr: u32, - word: Word, - ) -> Result<(), ExecutionError> { - self.memory.write(ctx, addr, self.clk, word) - } - - /// Writes the provided element into the specified context/address leaving the remaining 3 - /// elements of the word previously stored at that address unchanged. - pub fn write_mem_element( - &mut self, - ctx: ContextId, - addr: u32, - value: Felt, - ) -> Result { - let old_word = self.memory.get_old_value(ctx, addr); - let new_word = [value, old_word[1], old_word[2], old_word[3]]; - - self.memory.write(ctx, addr, self.clk, new_word)?; - - Ok(old_word) + /// Returns a reference to the Memory chiplet. + pub fn memory(&self) -> &Memory { + &self.memory } - /// Writes the two provided words to two consecutive addresses in memory in the specified - /// context, starting at the specified address. - pub fn write_mem_double( - &mut self, - ctx: ContextId, - addr: u32, - words: [Word; 2], - ) -> Result<(), ExecutionError> { - let addr2 = addr + 1; - // write two words to memory at addr and addr + 1 - self.memory.write(ctx, addr, self.clk, words[0])?; - self.memory.write(ctx, addr2, self.clk, words[1]) - } - - /// Returns a word located at the specified context/address, or None if the address hasn't - /// been accessed previously. - /// - /// 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 { - self.memory.get_value(ctx, 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)> { - 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() + /// Returns a mutable reference to the Memory chiplet. + pub fn memory_mut(&mut self) -> &mut Memory { + &mut self.memory } // KERNEL ROM ACCESSORS @@ -469,7 +391,7 @@ impl Chiplets { // so they can be filled with the chiplet traces for (column_num, column) in trace.iter_mut().enumerate().skip(1) { match column_num { - 1 | 15..=17 => { + 1 => { // columns 1 and 15 - 17 are relevant only for the hasher hasher_fragment.push_column_slice(column, hasher.trace_len()); }, @@ -491,6 +413,19 @@ impl Chiplets { let rest = memory_fragment.push_column_slice(rest, memory.trace_len()); kernel_rom_fragment.push_column_slice(rest, kernel_rom.trace_len()); }, + 15 | 16 => { + // columns 15 and 16 are relevant only for the hasher and memory chiplets + let rest = hasher_fragment.push_column_slice(column, hasher.trace_len()); + // skip bitwise chiplet + let (_, rest) = rest.split_at_mut(bitwise.trace_len()); + memory_fragment.push_column_slice(rest, memory.trace_len()); + }, + 17 => { + // column 17 is relevant only for the memory chiplet + // skip the hasher and bitwise chiplets + let (_, rest) = column.split_at_mut(hasher.trace_len() + bitwise.trace_len()); + memory_fragment.push_column_slice(rest, memory.trace_len()); + }, _ => panic!("invalid column index"), } } diff --git a/processor/src/chiplets/tests.rs b/processor/src/chiplets/tests.rs index 89253f6916..28606b983b 100644 --- a/processor/src/chiplets/tests.rs +++ b/processor/src/chiplets/tests.rs @@ -51,8 +51,9 @@ fn bitwise_chiplet_trace() { #[test] fn memory_chiplet_trace() { // --- single memory operation with no stack manipulation ------------------------------------- + let addr = Felt::from(4_u32); let stack = [1, 2, 3, 4]; - let operations = vec![Operation::Push(Felt::new(2)), Operation::MStoreW]; + let operations = vec![Operation::Push(addr), Operation::MStoreW]; let (chiplets_trace, trace_len) = build_trace(&stack, operations, Kernel::default()); let memory_trace_len = 1; diff --git a/processor/src/debug.rs b/processor/src/debug.rs index ab908b5609..805b7f64a6 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 ) } } @@ -166,7 +165,7 @@ impl VmStateIterator { asmop, fmp: self.system.get_fmp_at(self.clk), stack: self.stack.get_state_at(self.clk), - memory: self.chiplets.get_mem_state_at(ctx, self.clk), + memory: self.chiplets.memory().get_state_at(ctx, self.clk), }); self.clk -= 1; @@ -236,7 +235,7 @@ impl Iterator for VmStateIterator { asmop, fmp: self.system.get_fmp_at(self.clk), stack: self.stack.get_state_at(self.clk), - memory: self.chiplets.get_mem_state_at(ctx, self.clk), + memory: self.chiplets.memory().get_state_at(ctx, self.clk), })); self.clk += 1; @@ -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/decoder/mod.rs b/processor/src/decoder/mod.rs index 2b7ee125ad..c7575e5b45 100644 --- a/processor/src/decoder/mod.rs +++ b/processor/src/decoder/mod.rs @@ -319,7 +319,10 @@ impl Process { let mem_addr = self.stack.get(0); // The callee hash is stored in memory, and the address is specified on the top of the // stack. - let callee_hash = self.read_mem_word(mem_addr)?; + let callee_hash = + self.chiplets + .memory_mut() + .read_word(self.system.ctx(), mem_addr, self.system.clk())?; let addr = self.chiplets.hash_control_block( EMPTY_WORD, @@ -350,7 +353,10 @@ impl Process { let mem_addr = self.stack.get(0); // The callee hash is stored in memory, and the address is specified on the top of the // stack. - let callee_hash = self.read_mem_word(mem_addr)?; + let callee_hash = + self.chiplets + .memory_mut() + .read_word(self.system.ctx(), mem_addr, self.system.clk())?; // Note: other functions end in "executing a Noop", which // 1. ensures trace capacity, diff --git a/processor/src/decoder/tests.rs b/processor/src/decoder/tests.rs index 664517b7a2..ef7c810b88 100644 --- a/processor/src/decoder/tests.rs +++ b/processor/src/decoder/tests.rs @@ -1289,14 +1289,14 @@ fn dyn_block() { // end // // begin - // # stack: [42, DIGEST] + // # stack: [40, DIGEST] // mstorew // push.42 // dynexec // end - const FOO_ROOT_NODE_ADDR: u64 = 42; - const PUSH_42_OP: Operation = Operation::Push(Felt::new(FOO_ROOT_NODE_ADDR)); + const FOO_ROOT_NODE_ADDR: u64 = 40; + const PUSH_40_OP: Operation = Operation::Push(Felt::new(FOO_ROOT_NODE_ADDR)); let mut mast_forest = MastForest::new(); @@ -1308,7 +1308,7 @@ fn dyn_block() { let mstorew_node = MastNode::new_basic_block(vec![Operation::MStoreW], None).unwrap(); let mstorew_node_id = mast_forest.add_node(mstorew_node.clone()).unwrap(); - let push_node = MastNode::new_basic_block(vec![PUSH_42_OP], None).unwrap(); + let push_node = MastNode::new_basic_block(vec![PUSH_40_OP], None).unwrap(); let push_node_id = mast_forest.add_node(push_node.clone()).unwrap(); let join_node = MastNode::new_join(mstorew_node_id, push_node_id, &mast_forest).unwrap(); @@ -1348,7 +1348,7 @@ fn dyn_block() { // starting second span let push_basic_block_addr = mstorew_basic_block_addr + EIGHT; check_op_decoding(&trace, 5, join_addr, Operation::Span, 2, 0, 0); - check_op_decoding(&trace, 6, push_basic_block_addr, PUSH_42_OP, 1, 0, 1); + check_op_decoding(&trace, 6, push_basic_block_addr, PUSH_40_OP, 1, 0, 1); check_op_decoding(&trace, 7, push_basic_block_addr, Operation::Noop, 0, 1, 1); check_op_decoding(&trace, 8, push_basic_block_addr, Operation::End, 0, 0, 0); // end inner join diff --git a/processor/src/errors.rs b/processor/src/errors.rs index 61d7319300..902b1df115 100644 --- a/processor/src/errors.rs +++ b/processor/src/errors.rs @@ -92,6 +92,10 @@ pub enum ExecutionError { NoMastForestWithProcedure { root_digest: Digest }, #[error("memory address cannot exceed 2^32 but was {0}")] MemoryAddressOutOfBounds(u64), + #[error( + "word memory access at address {addr} in context {ctx} is unaligned at clock cycle {clk}" + )] + MemoryUnalignedWordAccess { addr: u32, ctx: ContextId, clk: Felt }, #[error("merkle path verification failed for value {value} at index {index} in the Merkle tree with root {root} (error code: {err_code})", value = to_hex(Felt::elements_as_bytes(value)), root = to_hex(root.as_bytes()), @@ -129,6 +133,8 @@ pub enum ExecutionError { hex = to_hex(.0.as_bytes()) )] SyscallTargetNotInKernel(Digest), + #[error("word access at memory address {addr} in context {ctx} is unaligned")] + UnalignedMemoryWordAccess { addr: u32, ctx: ContextId }, } impl From for ExecutionError { @@ -152,6 +158,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..c1acc513a2 100644 --- a/processor/src/lib.rs +++ b/processor/src/lib.rs @@ -676,10 +676,18 @@ 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 { - self.chiplets.get_mem_value(ctx, addr) + pub fn get_mem_value(&self, ctx: ContextId, addr: u32) -> Option { + self.chiplets.memory().get_value(ctx, addr) + } + + /// Returns the batch of elements starting at the specified context/address. + /// + /// # Errors + /// - If the address is not word aligned. + pub fn get_mem_word(&self, ctx: ContextId, addr: u32) -> Result, ExecutionError> { + self.chiplets.memory().get_word(ctx, addr) } /// Returns the entire memory state for the specified execution context at the current clock @@ -687,8 +695,8 @@ impl ProcessState<'_> { /// /// 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)> { - self.chiplets.get_mem_state_at(ctx, self.system.clk()) + pub fn get_mem_state(&self, ctx: ContextId) -> Vec<(u64, Felt)> { + self.chiplets.memory().get_state_at(ctx, self.system.clk()) } } diff --git a/processor/src/operations/comb_ops.rs b/processor/src/operations/comb_ops.rs index 498026f038..73bece2e4b 100644 --- a/processor/src/operations/comb_ops.rs +++ b/processor/src/operations/comb_ops.rs @@ -125,7 +125,7 @@ impl Process { fn get_randomness(&mut self) -> Result { let ctx = self.system.ctx(); let addr = self.stack.get(14); - let word = self.chiplets.read_mem(ctx, addr.as_int() as u32)?; + let word = self.chiplets.memory_mut().read_word(ctx, addr, self.system.clk())?; let a0 = word[0]; let a1 = word[1]; @@ -136,7 +136,7 @@ impl Process { fn get_ood_values(&mut self) -> Result<[QuadFelt; 2], ExecutionError> { let ctx = self.system.ctx(); let addr = self.stack.get(13); - let word = self.chiplets.read_mem(ctx, addr.as_int() as u32)?; + let word = self.chiplets.memory_mut().read_word(ctx, addr, self.system.clk())?; Ok([QuadFelt::new(word[0], word[1]), QuadFelt::new(word[2], word[3])]) } @@ -203,9 +203,11 @@ mod tests { let tztgz = rand_array::(); process .chiplets - .write_mem( + .memory_mut() + .write_word( ctx, inputs[2].as_int().try_into().expect("Shouldn't fail by construction"), + process.system.clk(), tztgz, ) .unwrap(); @@ -213,9 +215,11 @@ mod tests { let a = rand_array::(); process .chiplets - .write_mem( + .memory_mut() + .write_word( ctx, inputs[1].as_int().try_into().expect("Shouldn't fail by construction"), + process.system.clk(), a, ) .unwrap(); diff --git a/processor/src/operations/io_ops.rs b/processor/src/operations/io_ops.rs index 96c98c0b81..b1ded47a2c 100644 --- a/processor/src/operations/io_ops.rs +++ b/processor/src/operations/io_ops.rs @@ -1,4 +1,6 @@ -use super::{ExecutionError, Felt, Operation, Process}; +use vm_core::WORD_SIZE; + +use super::{ExecutionError, Felt, Process}; use crate::{AdviceProvider, Host, Word}; // INPUT / OUTPUT OPERATIONS @@ -20,19 +22,23 @@ impl Process { // MEMORY READING AND WRITING // -------------------------------------------------------------------------------------------- - /// Loads a word (4 elements) from the specified memory address onto the stack. + /// Loads a word (4 elements) starting at the specified memory address onto the stack. /// /// The operation works as follows: /// - The memory address is popped off the stack. - /// - A word is retrieved from memory at the specified address. The memory is always initialized - /// to ZEROs, and thus, if the specified address has never been written to, four ZERO elements - /// are returned. + /// - A word is retrieved from memory starting at the specified address, which must be aligned + /// to a word boundary. The memory is always initialized to ZEROs, and thus, for any of the + /// four addresses which were not previously been written to, four ZERO elements are returned. /// - The top four elements of the stack are overwritten with values retrieved from memory. /// /// Thus, the net result of the operation is that the stack is shifted left by one item. pub(super) fn op_mloadw(&mut self) -> Result<(), ExecutionError> { // get the address from the stack and read the word from current memory context - let mut word = self.read_mem_word(self.stack.get(0))?; + let mut word = self.chiplets.memory_mut().read_word( + self.system.ctx(), + self.stack.get(0), + self.system.clk(), + )?; word.reverse(); // update the stack state @@ -44,68 +50,24 @@ impl Process { Ok(()) } - /// Loads the first element from the specified memory address onto the stack. + /// Loads the element from the specified memory address onto the stack. /// /// The operation works as follows: /// - The memory address is popped off the stack. - /// - A word is retrieved from memory at the specified address. The memory is always initialized - /// to ZEROs, and thus, if the specified address has never been written to, four ZERO elements - /// are returned. - /// - The first element of the word retrieved from memory is pushed to the top of the stack. - /// - /// The first 3 helper registers are filled with the elements of the word which were not pushed - /// to the stack. They are stored in stack order, with the last element of the word in helper - /// register 0. + /// - The element is retrieved from memory at the specified address. The memory is always + /// initialized to ZEROs, and thus, if the specified address has never been written to, the + /// ZERO element is returned. + /// - The element retrieved from memory is pushed to the top of the stack. pub(super) fn op_mload(&mut self) -> Result<(), ExecutionError> { - // get the address from the stack and read the word from memory - let mut word = self.read_mem_word(self.stack.get(0))?; - word.reverse(); + let element = self.chiplets.memory_mut().read( + self.system.ctx(), + self.stack.get(0), + self.system.clk(), + )?; - // update the stack state - self.stack.set(0, word[3]); + self.stack.set(0, element); self.stack.copy_state(1); - // write the 3 unused elements to the helpers so they're available for constraint evaluation - self.decoder.set_user_op_helpers(Operation::MLoad, &word[..3]); - - Ok(()) - } - - /// Loads two words from memory and replaces the top 8 elements of the stack with their - /// contents. - /// - /// The operation works as follows: - /// - The memory address of the first word is retrieved from 13th stack element (position 12). - /// - Two consecutive words, starting at this address, are loaded from memory. - /// - Elements of these words are written to the top 8 elements of the stack (element-wise, in - /// stack order). - /// - Memory address (in position 12) is incremented by 2. - /// - All other stack elements remain the same. - pub(super) fn op_mstream(&mut self) -> Result<(), ExecutionError> { - // get the address from position 12 on the stack - let ctx = self.system.ctx(); - let addr = Self::get_valid_address(self.stack.get(12))?; - - // load two words from memory - let words = self.chiplets.read_mem_double(ctx, addr)?; - - // replace the stack elements with the elements from memory (in stack order) - for (i, &mem_value) in words.iter().flat_map(|word| word.iter()).rev().enumerate() { - self.stack.set(i, mem_value); - } - - // copy over the next 4 elements - for i in 8..12 { - let stack_value = self.stack.get(i); - self.stack.set(i, stack_value); - } - - // increment the address by 2 - self.stack.set(12, Felt::from(addr + 2)); - - // copy over the rest of the stack - self.stack.copy_state(13); - Ok(()) } @@ -113,20 +75,21 @@ impl Process { /// /// The operation works as follows: /// - The memory address is popped off the stack. - /// - The top four stack items are saved into the specified memory address. The items are not - /// removed from the stack. + /// - The top four stack items are saved starting at the specified memory address, which must be + /// aligned on a word boundary. The items are not removed from the stack. /// /// Thus, the net result of the operation is that the stack is shifted left by one item. pub(super) fn op_mstorew(&mut self) -> Result<(), ExecutionError> { // get the address from the stack and build the word to be saved from the stack values - let ctx = self.system.ctx(); - let addr = Self::get_valid_address(self.stack.get(0))?; + let addr = self.stack.get(0); // build the word in memory order (reverse of stack order) let word = [self.stack.get(4), self.stack.get(3), self.stack.get(2), self.stack.get(1)]; - // write the word to memory and get the previous word - self.chiplets.write_mem(ctx, addr, word)?; + // write the word to memory + self.chiplets + .memory_mut() + .write_word(self.system.ctx(), addr, self.system.clk(), word)?; // reverse the order of the memory word & update the stack state for (i, &value) in word.iter().rev().enumerate() { @@ -141,28 +104,18 @@ impl Process { /// /// The operation works as follows: /// - The memory address is popped off the stack. - /// - The top stack element is saved into the first element of the word located at the specified - /// memory address. The remaining 3 elements of the word are not affected. The element is not - /// removed from the stack. + /// - The top stack element is saved at the specified memory address. The element is not removed + /// from the stack. /// /// Thus, the net result of the operation is that the stack is shifted left by one item. - /// - /// The first 3 helper registers are filled with the remaining elements of the word which were - /// previously stored in memory and not overwritten by the operation. They are stored in stack - /// order, with the last element at helper register 0. pub(super) fn op_mstore(&mut self) -> Result<(), ExecutionError> { // get the address and the value from the stack let ctx = self.system.ctx(); - let addr = Self::get_valid_address(self.stack.get(0))?; + let addr = self.stack.get(0); let value = self.stack.get(1); // write the value to the memory and get the previous word - let mut old_word = self.chiplets.write_mem_element(ctx, addr, value)?; - // put the retrieved word into stack order - old_word.reverse(); - - // write the 3 unused elements to the helpers so they're available for constraint evaluation - self.decoder.set_user_op_helpers(Operation::MStore, &old_word[..3]); + self.chiplets.memory_mut().write(ctx, addr, self.system.clk(), value)?; // update the stack state self.stack.shift_left(1); @@ -170,6 +123,51 @@ impl Process { Ok(()) } + /// Loads two words from memory and replaces the top 8 elements of the stack with their + /// contents. + /// + /// The operation works as follows: + /// - The memory address of the first word is retrieved from 13th stack element (position 12). + /// - Two consecutive words, starting at this address, are loaded from memory. + /// - Elements of these words are written to the top 8 elements of the stack (element-wise, in + /// stack order). + /// - Memory address (in position 12) is incremented by 8. + /// - All other stack elements remain the same. + pub(super) fn op_mstream(&mut self) -> Result<(), ExecutionError> { + const MEM_ADDR_STACK_IDX: usize = 12; + + let ctx = self.system.ctx(); + let clk = self.system.clk(); + let addr_first_word = self.stack.get(MEM_ADDR_STACK_IDX); + let addr_second_word = addr_first_word + Felt::from(WORD_SIZE as u32); + + // load two words from memory + let words = [ + self.chiplets.memory_mut().read_word(ctx, addr_first_word, clk)?, + self.chiplets.memory_mut().read_word(ctx, addr_second_word, clk)?, + ]; + + // replace the stack elements with the elements from memory (in stack order) + for (i, &mem_value) in words.iter().flat_map(|word| word.iter()).rev().enumerate() { + self.stack.set(i, mem_value); + } + + // copy over the next 4 elements + for i in 8..MEM_ADDR_STACK_IDX { + let stack_value = self.stack.get(i); + self.stack.set(i, stack_value); + } + + // increment the address by 8 (2 words) + self.stack + .set(MEM_ADDR_STACK_IDX, addr_first_word + Felt::from(WORD_SIZE as u32 * 2)); + + // copy over the rest of the stack + self.stack.copy_state(13); + + Ok(()) + } + /// Moves 8 elements from the advice stack to the memory, via the operand stack. /// /// The operation works as follows: @@ -178,18 +176,23 @@ impl Process { /// (position 12). /// - The two words are written to memory consecutively, starting at this address. /// - These words replace the top 8 elements of the stack (element-wise, in stack order). - /// - Memory address (in position 12) is incremented by 2. + /// - Memory address (in position 12) is incremented by 8. /// - All other stack elements remain the same. pub(super) fn op_pipe(&mut self, host: &mut impl Host) -> Result<(), ExecutionError> { + const MEM_ADDR_STACK_IDX: usize = 12; + // get the address from position 12 on the stack let ctx = self.system.ctx(); - let addr = Self::get_valid_address(self.stack.get(12))?; + let clk = self.system.clk(); + let addr_first_word = self.stack.get(MEM_ADDR_STACK_IDX); + let addr_second_word = addr_first_word + Felt::from(WORD_SIZE as u32); // pop two words from the advice stack let words = host.advice_provider_mut().pop_stack_dword(self.into())?; // write the words memory - self.chiplets.write_mem_double(ctx, addr, words)?; + self.chiplets.memory_mut().write_word(ctx, addr_first_word, clk, words[0])?; + self.chiplets.memory_mut().write_word(ctx, addr_second_word, clk, words[1])?; // replace the elements on the stack with the word elements (in stack order) for (i, &adv_value) in words.iter().flat_map(|word| word.iter()).rev().enumerate() { @@ -202,8 +205,9 @@ impl Process { self.stack.set(i, stack_value); } - // increment the address by 2 - self.stack.set(12, Felt::from(addr + 2)); + // increment the address by 8 (2 words) + self.stack + .set(MEM_ADDR_STACK_IDX, addr_first_word + Felt::from(WORD_SIZE as u32 * 2)); // copy over the rest of the stack self.stack.copy_state(13); @@ -241,30 +245,6 @@ impl Process { Ok(()) } - - // HELPER FUNCTIONS - // -------------------------------------------------------------------------------------------- - - /// Returns the memory word at address `addr` in the current context. - pub(crate) fn read_mem_word(&mut self, addr: Felt) -> Result { - let ctx = self.system.ctx(); - let mem_addr = Self::get_valid_address(addr)?; - let word_at_addr = self.chiplets.read_mem(ctx, mem_addr)?; - - Ok(word_at_addr) - } - - /// Checks that provided address is less than u32::MAX and returns it cast to u32. - /// - /// # Errors - /// Returns an error if the provided address is greater than u32::MAX. - fn get_valid_address(addr: Felt) -> Result { - let addr = addr.as_int(); - if addr > u32::MAX as u64 { - return Err(ExecutionError::MemoryAddressOutOfBounds(addr)); - } - Ok(addr as u32) - } } // TESTS @@ -316,11 +296,11 @@ mod tests { fn op_mloadw() { let mut host = DefaultHost::default(); let mut process = Process::new_dummy_with_decoder_helpers_and_empty_stack(); - assert_eq!(0, process.chiplets.get_mem_size()); + assert_eq!(0, process.chiplets.memory().num_accessed_batches()); - // push a word onto the stack and save it at address 1 + // push a word onto the stack and save it at address 4 let word = [1, 3, 5, 7].to_elements().try_into().unwrap(); - store_value(&mut process, 1, word, &mut host); + store_value(&mut process, 4, word, &mut host); // push four zeros onto the stack for _ in 0..4 { @@ -328,15 +308,18 @@ mod tests { } // push the address onto the stack and load the word - process.execute_op(Operation::Push(ONE), &mut host).unwrap(); + process.execute_op(Operation::Push(4_u32.into()), &mut host).unwrap(); process.execute_op(Operation::MLoadW, &mut host).unwrap(); let expected_stack = build_expected_stack(&[7, 5, 3, 1, 7, 5, 3, 1]); assert_eq!(expected_stack, process.stack.trace_state()); // 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!(1, process.chiplets.memory().num_accessed_batches()); + assert_eq!( + word, + process.chiplets.memory().get_word(ContextId::root(), 4).unwrap().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(); @@ -351,22 +334,25 @@ mod tests { fn op_mload() { let mut host = DefaultHost::default(); let mut process = Process::new_dummy_with_decoder_helpers_and_empty_stack(); - assert_eq!(0, process.chiplets.get_mem_size()); + assert_eq!(0, process.chiplets.memory().num_accessed_batches()); - // push a word onto the stack and save it at address 2 + // push a word onto the stack and save it at address 4 let word = [1, 3, 5, 7].to_elements().try_into().unwrap(); - store_value(&mut process, 2, word, &mut host); + store_value(&mut process, 4, word, &mut host); // push the address onto the stack and load the element - process.execute_op(Operation::Push(Felt::new(2)), &mut host).unwrap(); + process.execute_op(Operation::Push(Felt::new(4)), &mut host).unwrap(); process.execute_op(Operation::MLoad, &mut host).unwrap(); let expected_stack = build_expected_stack(&[1, 7, 5, 3, 1]); assert_eq!(expected_stack, process.stack.trace_state()); // 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!(1, process.chiplets.memory().num_accessed_batches()); + assert_eq!( + word, + process.chiplets.memory().get_word(ContextId::root(), 4).unwrap().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(); @@ -382,18 +368,24 @@ mod tests { let mut host = DefaultHost::default(); let mut process = Process::new_dummy_with_decoder_helpers_and_empty_stack(); - // save two words into memory addresses 1 and 2 + // save two words into memory addresses 4 and 8 let word1 = [30, 29, 28, 27]; let word2 = [26, 25, 24, 23]; let word1_felts: Word = word1.to_elements().try_into().unwrap(); let word2_felts: Word = word2.to_elements().try_into().unwrap(); - store_value(&mut process, 1, word1_felts, &mut host); - store_value(&mut process, 2, word2_felts, &mut host); + store_value(&mut process, 4, word1_felts, &mut host); + store_value(&mut process, 8, word2_felts, &mut host); // 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!(2, process.chiplets.memory().num_accessed_batches()); + assert_eq!( + word1_felts, + process.chiplets.memory().get_word(ContextId::root(), 4).unwrap().unwrap() + ); + assert_eq!( + word2_felts, + process.chiplets.memory().get_word(ContextId::root(), 8).unwrap().unwrap() + ); // clear the stack for _ in 0..8 { @@ -402,11 +394,11 @@ mod tests { // arrange the stack such that: // - 101 is at position 13 (to make sure it is not overwritten) - // - 1 (the address) is at position 12 + // - 4 (the address) is at position 12 // - values 1 - 12 are at positions 0 - 11. Adding the first 8 of these values to the values // stored in memory should result in 35. process.execute_op(Operation::Push(Felt::new(101)), &mut host).unwrap(); - process.execute_op(Operation::Push(ONE), &mut host).unwrap(); + process.execute_op(Operation::Push(4_u32.into()), &mut host).unwrap(); for i in 1..13 { process.execute_op(Operation::Push(Felt::new(i)), &mut host).unwrap(); } @@ -417,8 +409,20 @@ mod tests { // the first 8 values should contain the values from memory. the next 4 values should remain // unchanged, and the address should be incremented by 2 (i.e., 1 -> 3). let stack_values = [ - word2[3], word2[2], word2[1], word2[0], word1[3], word1[2], word1[1], word1[0], 4, 3, - 2, 1, 3, 101, + word2[3], + word2[2], + word2[1], + word2[0], + word1[3], + word1[2], + word1[1], + word1[0], + 4, + 3, + 2, + 1, + 4 + 8, // initial address + 2 words + 101, // rest of stack ]; let expected_stack = build_expected_stack(&stack_values); assert_eq!(expected_stack, process.stack.trace_state()); @@ -428,7 +432,7 @@ mod tests { fn op_mstorew() { let mut host = DefaultHost::default(); let mut process = Process::new_dummy_with_decoder_helpers_and_empty_stack(); - assert_eq!(0, process.chiplets.get_mem_size()); + assert_eq!(0, process.chiplets.memory().num_accessed_batches()); // push the first word onto the stack and save it at address 0 let word1 = [1, 3, 5, 7].to_elements().try_into().unwrap(); @@ -439,21 +443,30 @@ mod tests { assert_eq!(expected_stack, process.stack.trace_state()); // 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!(1, process.chiplets.memory().num_accessed_batches()); + assert_eq!( + word1, + process.chiplets.memory().get_word(ContextId::root(), 0).unwrap().unwrap() + ); - // push the second word onto the stack and save it at address 3 + // push the second word onto the stack and save it at address 4 let word2 = [2, 4, 6, 8].to_elements().try_into().unwrap(); - store_value(&mut process, 3, word2, &mut host); + store_value(&mut process, 4, word2, &mut host); // check stack state let expected_stack = build_expected_stack(&[8, 6, 4, 2, 7, 5, 3, 1]); assert_eq!(expected_stack, process.stack.trace_state()); // 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!(2, process.chiplets.memory().num_accessed_batches()); + assert_eq!( + word1, + process.chiplets.memory().get_word(ContextId::root(), 0).unwrap().unwrap() + ); + assert_eq!( + word2, + process.chiplets.memory().get_word(ContextId::root(), 4).unwrap().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(); @@ -468,7 +481,7 @@ mod tests { fn op_mstore() { let mut host = DefaultHost::default(); let mut process = Process::new_dummy_with_decoder_helpers_and_empty_stack(); - assert_eq!(0, process.chiplets.get_mem_size()); + assert_eq!(0, process.chiplets.memory().num_accessed_batches()); // push new element onto the stack and save it as first element of the word on // uninitialized memory at address 0 @@ -481,16 +494,19 @@ 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!(1, process.chiplets.memory().num_accessed_batches()); + assert_eq!( + mem_0, + process.chiplets.memory().get_word(ContextId::root(), 0).unwrap().unwrap() + ); - // push the word onto the stack and save it at address 2 + // push the word onto the stack and save it at address 4 let word_2 = [1, 3, 5, 7].to_elements().try_into().unwrap(); - store_value(&mut process, 2, word_2, &mut host); + store_value(&mut process, 4, word_2, &mut host); // push new element onto the stack and save it as first element of the word at address 2 let element = Felt::new(12); - store_element(&mut process, 2, element, &mut host); + store_element(&mut process, 4, element, &mut host); // check stack state let expected_stack = build_expected_stack(&[12, 7, 5, 3, 1, 10]); @@ -498,8 +514,11 @@ 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!(2, process.chiplets.memory().num_accessed_batches()); + assert_eq!( + mem_2, + process.chiplets.memory().get_word(ContextId::root(), 4).unwrap().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(); @@ -527,12 +546,12 @@ mod tests { // arrange the stack such that: // - 101 is at position 13 (to make sure it is not overwritten) - // - 1 (the address) is at position 12 + // - 4 (the address) is at position 12 // - values 1 - 12 are at positions 0 - 11. Replacing the first 8 of these values with the // values from the advice stack should result in 30 through 23 in stack order (with 23 at // stack[0]). process.execute_op(Operation::Push(Felt::new(101)), &mut host).unwrap(); - process.execute_op(Operation::Push(ONE), &mut host).unwrap(); + process.execute_op(Operation::Push(4_u32.into()), &mut host).unwrap(); for i in 1..13 { process.execute_op(Operation::Push(Felt::new(i)), &mut host).unwrap(); } @@ -541,15 +560,33 @@ mod tests { process.execute_op(Operation::Pipe, &mut host).unwrap(); // 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!(2, process.chiplets.memory().num_accessed_batches()); + assert_eq!( + word1_felts, + process.chiplets.memory().get_word(ContextId::root(), 4).unwrap().unwrap() + ); + assert_eq!( + word2_felts, + process.chiplets.memory().get_word(ContextId::root(), 8).unwrap().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). let stack_values = [ - word2[3], word2[2], word2[1], word2[0], word1[3], word1[2], word1[1], word1[0], 4, 3, - 2, 1, 3, 101, + word2[3], + word2[2], + word2[1], + word2[0], + word1[3], + word1[2], + word1[1], + word1[0], + 4, + 3, + 2, + 1, + 4 + 8, // initial address + 2 words + 101, // rest of stack ]; let expected_stack = build_expected_stack(&stack_values); assert_eq!(expected_stack, process.stack.trace_state()); diff --git a/processor/src/operations/sys_ops/sys_event_handlers.rs b/processor/src/operations/sys_ops/sys_event_handlers.rs index 13e4a796be..7ff647945b 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/processor/src/trace/tests/chiplets/memory.rs b/processor/src/trace/tests/chiplets/memory.rs index 5415a38def..d4617145d7 100644 --- a/processor/src/trace/tests/chiplets/memory.rs +++ b/processor/src/trace/tests/chiplets/memory.rs @@ -1,6 +1,8 @@ use miden_air::{ trace::chiplets::{ - memory::{MEMORY_READ_LABEL, MEMORY_WRITE, MEMORY_WRITE_LABEL, NUM_ELEMENTS}, + memory::{ + MEMORY_READ_LABEL, MEMORY_WRITE_LABEL, MEMORY_WRITE_SELECTOR, NUM_ELEMENTS_IN_BATCH, + }, MEMORY_ADDR_COL_IDX, MEMORY_CLK_COL_IDX, MEMORY_CTX_COL_IDX, MEMORY_SELECTORS_COL_IDX, MEMORY_V_COL_RANGE, }, @@ -156,7 +158,7 @@ fn build_expected_memory( word: Word, ) -> Felt { let mut word_value = ZERO; - for i in 0..NUM_ELEMENTS { + for i in 0..NUM_ELEMENTS_IN_BATCH { word_value += alphas[i + 5] * word[i]; } @@ -176,7 +178,7 @@ fn build_expected_memory_from_trace( // get the memory access operation let s0 = trace.main_trace.get_column(MEMORY_SELECTORS_COL_IDX)[row]; let s1 = trace.main_trace.get_column(MEMORY_SELECTORS_COL_IDX + 1)[row]; - let op_label = if s0 == MEMORY_WRITE[0] { + let op_label = if s0 == MEMORY_WRITE_SELECTOR[0] { debug_assert!(s1 == ZERO); MEMORY_WRITE_LABEL } else { @@ -189,7 +191,7 @@ fn build_expected_memory_from_trace( let clk = trace.main_trace.get_column(MEMORY_CLK_COL_IDX)[row]; // get the memory value - let mut word = [ZERO; NUM_ELEMENTS]; + let mut word = [ZERO; NUM_ELEMENTS_IN_BATCH]; for (i, element) in word.iter_mut().enumerate() { *element = trace.main_trace.get_column(MEMORY_V_COL_RANGE.start + i)[row]; } diff --git a/stdlib/tests/mem/mod.rs b/stdlib/tests/mem/mod.rs index 8cfe9e4226..773a0ade11 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.memory().get_word(ContextId::root(), 1000).unwrap(), Some([ZERO, ZERO, ZERO, ONE]), "Address 1000" ); assert_eq!( - process.chiplets.get_mem_value(ContextId::root(), 1001), + process.chiplets.memory().get_word(ContextId::root(), 1001).unwrap(), Some([ZERO, ZERO, ONE, ZERO]), "Address 1001" ); assert_eq!( - process.chiplets.get_mem_value(ContextId::root(), 1002), + process.chiplets.memory().get_word(ContextId::root(), 1002).unwrap(), Some([ZERO, ZERO, ONE, ONE]), "Address 1002" ); assert_eq!( - process.chiplets.get_mem_value(ContextId::root(), 1003), + process.chiplets.memory().get_word(ContextId::root(), 1003).unwrap(), Some([ZERO, ONE, ZERO, ZERO]), "Address 1003" ); assert_eq!( - process.chiplets.get_mem_value(ContextId::root(), 1004), + process.chiplets.memory().get_word(ContextId::root(), 1004).unwrap(), Some([ZERO, ONE, ZERO, ONE]), "Address 1004" ); assert_eq!( - process.chiplets.get_mem_value(ContextId::root(), 2000), + process.chiplets.memory().get_word(ContextId::root(), 2000).unwrap(), Some([ZERO, ZERO, ZERO, ONE]), "Address 2000" ); assert_eq!( - process.chiplets.get_mem_value(ContextId::root(), 2001), + process.chiplets.memory().get_word(ContextId::root(), 2001).unwrap(), Some([ZERO, ZERO, ONE, ZERO]), "Address 2001" ); assert_eq!( - process.chiplets.get_mem_value(ContextId::root(), 2002), + process.chiplets.memory().get_word(ContextId::root(), 2002).unwrap(), Some([ZERO, ZERO, ONE, ONE]), "Address 2002" ); assert_eq!( - process.chiplets.get_mem_value(ContextId::root(), 2003), + process.chiplets.memory().get_word(ContextId::root(), 2003).unwrap(), Some([ZERO, ONE, ZERO, ZERO]), "Address 2003" ); assert_eq!( - process.chiplets.get_mem_value(ContextId::root(), 2004), + process.chiplets.memory().get_word(ContextId::root(), 2004).unwrap(), Some([ZERO, ONE, ZERO, ONE]), "Address 2004" ); diff --git a/test-utils/src/lib.rs b/test-utils/src/lib.rs index 2d048db53f..ae5b934873 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,22 @@ 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 + 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(), mem_start_addr) - .unwrap_or(EMPTY_WORD); - - let mem_state = felt_slice_to_ints(&mem_state); + .memory() + .get_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