Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Memory handling of 2 reads at the same address in the same clock cycle #1561

Merged
merged 1 commit into from
Nov 18, 2024
Merged

Memory handling of 2 reads at the same address in the same clock cycle #1561

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
- Fixed the construction of the chiplets virtual table (#1514) (#1556)
- Fixed the construction of the chiplets bus (#1516) (#1525)
- Decorators are now allowed in empty basic blocks (#1466)
- Return an error if an instruction performs 2 memory accesses at the same memory address in the same cycle (#1561)

## 0.10.6 (2024-09-12) - `miden-processor` crate only

Expand Down
24 changes: 24 additions & 0 deletions miden/tests/integration/operations/io_ops/mem_ops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
use prover::ExecutionError;
use test_utils::expect_exec_error;

use super::{apply_permutation, build_op_test, build_test, Felt, ToElements, TRUNCATE_STACK_PROC};

// LOADING SINGLE ELEMENT ONTO THE STACK (MLOAD)
Expand Down Expand Up @@ -228,3 +231,24 @@ fn read_after_write() {
let test = build_op_test!("mem_storew.0 dropw mem_loadw.0", &[1, 2, 3, 4, 5, 6, 7, 8]);
test.expect_stack(&[8, 7, 6, 5]);
}

// MISC
// ================================================================================================

/// Ensures that the processor returns an error when 2 memory operations occur in the same context,
/// at the same address, and in the same clock cycle (which is what RCOMBBASE does when `stack[13] =
/// stack[14] = 0`).
#[test]
fn mem_reads_same_clock_cycle() {
let asm_op = "begin rcomb_base end";

let test = build_test!(asm_op);
expect_exec_error!(
test,
ExecutionError::DuplicateMemoryAccess {
ctx: 0_u32.into(),
addr: 0,
clk: 1_u32.into()
}
);
}
27 changes: 22 additions & 5 deletions processor/src/chiplets/memory/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use super::{
utils::{split_element_u32_into_u16, split_u32_into_u16},
Felt, FieldElement, RangeChecker, TraceFragment, Word, EMPTY_WORD, ONE,
};
use crate::system::ContextId;
use crate::{system::ContextId, ExecutionError};

mod segment;
use segment::MemorySegmentTrace;
Expand Down Expand Up @@ -137,15 +137,32 @@ impl Memory {
///
/// 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(&mut self, ctx: ContextId, addr: u32, clk: RowIndex) -> Word {
///
/// # 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: RowIndex,
) -> Result<Word, ExecutionError> {
self.num_trace_rows += 1;
self.trace.entry(ctx).or_default().read(addr, Felt::from(clk))
self.trace.entry(ctx).or_default().read(ctx, addr, Felt::from(clk))
}

/// Writes the provided word at the specified context/address.
pub fn write(&mut self, ctx: ContextId, addr: u32, clk: RowIndex, value: Word) {
///
/// # Errors
/// - 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,
clk: RowIndex,
value: Word,
) -> Result<(), ExecutionError> {
self.num_trace_rows += 1;
self.trace.entry(ctx).or_default().write(addr, Felt::from(clk), value);
self.trace.entry(ctx).or_default().write(ctx, addr, Felt::from(clk), value)
}

// EXECUTION TRACE GENERATION
Expand Down
73 changes: 53 additions & 20 deletions processor/src/chiplets/memory/segment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
use alloc::{collections::BTreeMap, vec::Vec};
use alloc::{
collections::{btree_map::Entry, BTreeMap},
vec::Vec,
};

use miden_air::{
trace::chiplets::memory::{Selectors, MEMORY_COPY_READ, MEMORY_INIT_READ, MEMORY_WRITE},
RowIndex,
};

use super::{Felt, Word, INIT_MEM_VALUE};
use crate::{ContextId, ExecutionError};

// MEMORY SEGMENT TRACE
// ================================================================================================
Expand Down Expand Up @@ -72,37 +76,66 @@ impl MemorySegmentTrace {
///
/// 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(&mut self, addr: u32, clk: Felt) -> Word {
///
/// # 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<Word, ExecutionError> {
// look up the previous value in the appropriate address trace and add (clk, prev_value)
plafer marked this conversation as resolved.
Show resolved Hide resolved
// 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.
self.0
.entry(addr)
.and_modify(|addr_trace| {
let last_value = addr_trace.last().expect("empty address trace").value();
let access = MemorySegmentAccess::new(clk, MemoryOperation::CopyRead, last_value);
addr_trace.push(access);
})
.or_insert_with(|| {
match self.0.entry(addr) {
Entry::Vacant(vacant_entry) => {
let access =
MemorySegmentAccess::new(clk, MemoryOperation::InitRead, INIT_MEM_VALUE);
vec![access]
})
.last()
.expect("empty address trace")
.value()
vacant_entry.insert(vec![access]);
Ok(INIT_MEM_VALUE)
},
Entry::Occupied(mut occupied_entry) => {
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);
addr_trace.push(access);

Ok(last_value)
}
},
}
}

/// Writes the provided word at the specified address. The memory access is assumed to happen
/// at the provided clock cycle.
pub fn write(&mut self, addr: u32, clk: Felt, value: Word) {
///
/// # Errors
/// - 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,
clk: Felt,
value: 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);
self.0
.entry(addr)
.and_modify(|addr_trace| addr_trace.push(access))
.or_insert_with(|| vec![access]);
match self.0.entry(addr) {
Entry::Vacant(vacant_entry) => {
vacant_entry.insert(vec![access]);
Ok(())
},
Entry::Occupied(mut occupied_entry) => {
let addr_trace = occupied_entry.get_mut();
if addr_trace.last().expect("empty address trace").clk() == clk {
Err(ExecutionError::DuplicateMemoryAccess { ctx, addr, clk })
} else {
addr_trace.push(access);
Ok(())
}
},
}
}

// INNER VALUE ACCESSORS
Expand Down
50 changes: 25 additions & 25 deletions processor/src/chiplets/memory/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,27 +28,27 @@ fn mem_read() {

// read a value from address 0; clk = 1
let addr0 = 0;
let value = mem.read(ContextId::root(), addr0, 1.into());
let value = mem.read(ContextId::root(), addr0, 1.into()).unwrap();
assert_eq!(EMPTY_WORD, value);
assert_eq!(1, mem.size());
assert_eq!(1, mem.trace_len());

// read a value from address 3; clk = 2
let addr3 = 3;
let value = mem.read(ContextId::root(), addr3, 2.into());
let value = mem.read(ContextId::root(), addr3, 2.into()).unwrap();
assert_eq!(EMPTY_WORD, value);
assert_eq!(2, mem.size());
assert_eq!(2, mem.trace_len());

// read a value from address 0 again; clk = 3
let value = mem.read(ContextId::root(), addr0, 3.into());
let value = mem.read(ContextId::root(), addr0, 3.into()).unwrap();
assert_eq!(EMPTY_WORD, value);
assert_eq!(2, mem.size());
assert_eq!(3, mem.trace_len());

// read a value from address 2; clk = 4
let addr2 = 2;
let value = mem.read(ContextId::root(), addr2, 4.into());
let value = mem.read(ContextId::root(), addr2, 4.into()).unwrap();
assert_eq!(EMPTY_WORD, value);
assert_eq!(3, mem.size());
assert_eq!(4, mem.trace_len());
Expand Down Expand Up @@ -81,30 +81,30 @@ fn mem_write() {
// 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);
mem.write(ContextId::root(), addr0, 1.into(), value1).unwrap();
assert_eq!(value1, mem.get_value(ContextId::root(), addr0).unwrap());
assert_eq!(1, mem.size());
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);
mem.write(ContextId::root(), addr2, 2.into(), value5).unwrap();
assert_eq!(value5, mem.get_value(ContextId::root(), addr2).unwrap());
assert_eq!(2, mem.size());
assert_eq!(2, mem.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);
mem.write(ContextId::root(), addr1, 3.into(), value7).unwrap();
assert_eq!(value7, mem.get_value(ContextId::root(), addr1).unwrap());
assert_eq!(3, mem.size());
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);
mem.write(ContextId::root(), addr0, 4.into(), value9).unwrap();
assert_eq!(value7, mem.get_value(ContextId::root(), addr1).unwrap());
assert_eq!(3, mem.size());
assert_eq!(4, mem.trace_len());
Expand Down Expand Up @@ -137,35 +137,35 @@ fn mem_write_read() {
// write 1 into address 5; clk = 1
let addr5 = 5;
let value1 = [ONE, ZERO, ZERO, ZERO];
mem.write(ContextId::root(), addr5, 1.into(), value1);
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);
mem.write(ContextId::root(), addr2, 2.into(), value4).unwrap();

// read a value from address 5; clk = 3
mem.read(ContextId::root(), addr5, 3.into());
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);
mem.write(ContextId::root(), addr5, 4.into(), value2).unwrap();

// read a value from address 2; clk = 5
mem.read(ContextId::root(), addr2, 5.into());
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);
mem.write(ContextId::root(), addr2, 6.into(), value7).unwrap();

// read a value from address 5; clk = 7
mem.read(ContextId::root(), addr5, 7.into());
mem.read(ContextId::root(), addr5, 7.into()).unwrap();

// read a value from address 2; clk = 8
mem.read(ContextId::root(), addr2, 8.into());
mem.read(ContextId::root(), addr2, 8.into()).unwrap();

// read a value from address 5; clk = 9
mem.read(ContextId::root(), addr5, 9.into());
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
Expand Down Expand Up @@ -208,33 +208,33 @@ fn mem_multi_context() {

// 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);
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);
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());
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);
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());
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());
Expand Down Expand Up @@ -270,17 +270,17 @@ fn mem_get_state_at() {
// Write 1 into (ctx = 0, addr = 5) at clk = 1.
// This means that mem[5] = 1 at the beginning of clk = 2
let value1 = [ONE, ZERO, ZERO, ZERO];
mem.write(ContextId::root(), 5, 1.into(), value1);
mem.write(ContextId::root(), 5, 1.into(), value1).unwrap();

// Write 4 into (ctx = 0, addr = 2) at clk = 2.
// This means that mem[2] = 4 at the beginning of clk = 3
let value4 = [Felt::new(4), ZERO, ZERO, ZERO];
mem.write(ContextId::root(), 2, 2.into(), value4);
mem.write(ContextId::root(), 2, 2.into(), value4).unwrap();

// write 7 into (ctx = 3, addr = 3) at clk = 4
// This means that mem[3] = 7 at the beginning of clk = 4
let value7 = [Felt::new(7), ZERO, ZERO, ZERO];
mem.write(3.into(), 3, 4.into(), value7);
mem.write(3.into(), 3, 4.into(), value7).unwrap();

// Check memory state at clk = 2
assert_eq!(mem.get_state_at(ContextId::root(), 2.into()), vec![(5, value1)]);
Expand Down
Loading
Loading