Skip to content

Commit

Permalink
Merge pull request #1049 from 0xPolygonMiden/andrew-mem-addr-limit-pr…
Browse files Browse the repository at this point in the history
…ocessor

Impose limit on the memory addresses
  • Loading branch information
Fumuran authored Aug 29, 2023
2 parents d6c49ce + 824e282 commit 0d47b0e
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 77 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
- Improved handling of invalid/incomplete parameters in `StackOutputs` constructors (#1010).
- Allowed the assembler to produce programs with "phantom" calls (#1019).
- Added `TraceLenSummary` struct which holds information about traces lengths to the `ExecutionTrace` (#1029).
- Imposed the 2^32 limit for the memory addresses used in the memory chiplet (#1049).

## 0.6.1 (2023-06-29)

Expand Down
35 changes: 20 additions & 15 deletions processor/src/chiplets/memory/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ 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: u32, addr: u64) -> Option<Word> {
pub fn get_value(&self, ctx: u32, addr: u32) -> Option<Word> {
match self.trace.get(&ctx) {
Some(segment) => segment.get_value(addr),
None => None,
Expand All @@ -105,7 +105,7 @@ impl Memory {

/// 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: u32, addr: u64) -> Word {
pub fn get_old_value(&self, ctx: u32, 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)
}
Expand All @@ -131,13 +131,13 @@ 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: u32, addr: Felt, clk: u32) -> Word {
pub fn read(&mut self, ctx: u32, addr: u32, clk: u32) -> Word {
self.num_trace_rows += 1;
self.trace.entry(ctx).or_default().read(addr, Felt::from(clk))
}

/// Writes the provided word at the specified context/address.
pub fn write(&mut self, ctx: u32, addr: Felt, clk: u32, value: Word) {
pub fn write(&mut self, ctx: u32, addr: u32, clk: u32, value: Word) {
self.num_trace_rows += 1;
self.trace.entry(ctx).or_default().write(addr, Felt::from(clk), value);
}
Expand Down Expand Up @@ -170,7 +170,7 @@ impl Memory {
let delta = if prev_ctx != ctx {
(ctx - prev_ctx) as u64
} else if prev_addr != addr {
addr - prev_addr
(addr - prev_addr) as u64
} else {
clk - prev_clk - 1
};
Expand Down Expand Up @@ -214,7 +214,7 @@ impl Memory {
for (addr, addr_trace) in segment.into_inner() {
// when we start a new address, we set the previous value to all zeros. the effect of
// this is that memory is always initialized to zero.
let addr = Felt::new(addr);
let felt_addr = Felt::from(addr);
for memory_access in addr_trace {
let clk = memory_access.clk();
let value = memory_access.value();
Expand All @@ -223,7 +223,7 @@ impl Memory {
trace.set(row, 0, selectors[0]);
trace.set(row, 1, selectors[1]);
trace.set(row, CTX_COL_IDX, ctx);
trace.set(row, ADDR_COL_IDX, addr);
trace.set(row, ADDR_COL_IDX, felt_addr);
trace.set(row, CLK_COL_IDX, clk);
for (idx, col) in V_COL_RANGE.enumerate() {
trace.set(row, col, value[idx]);
Expand All @@ -232,8 +232,8 @@ impl Memory {
// compute delta as difference between context IDs, addresses, or clock cycles
let delta = if prev_ctx != ctx {
ctx - prev_ctx
} else if prev_addr != addr {
addr - prev_addr
} else if prev_addr != felt_addr {
felt_addr - prev_addr
} else {
clk - prev_clk - ONE
};
Expand All @@ -245,14 +245,19 @@ impl Memory {
trace.set(row, D_INV_COL_IDX, delta.inv());

// provide the memory access data to the chiplets bus.
let memory_lookup =
MemoryLookup::new(memory_access.op_label(), ctx, addr, clk, value);
let memory_lookup = MemoryLookup::new(
memory_access.op_label(),
ctx,
Felt::from(addr),
clk,
value,
);
chiplets_bus
.provide_memory_operation(memory_lookup, (memory_start_row + row) as u32);

// update values for the next iteration of the loop
prev_ctx = ctx;
prev_addr = addr;
prev_addr = felt_addr;
prev_clk = clk;
row += 1;
}
Expand All @@ -265,7 +270,7 @@ impl Memory {

/// Returns the context, address, and clock cycle of the first trace row, or None if the trace
/// is empty.
fn get_first_row_info(&self) -> Option<(u32, u64, Felt)> {
fn get_first_row_info(&self) -> Option<(u32, u32, Felt)> {
let (ctx, segment) = match self.trace.iter().next() {
Some((&ctx, segment)) => (ctx, segment),
None => return None,
Expand Down Expand Up @@ -311,11 +316,11 @@ impl MemoryLookup {
}
}

pub fn from_ints(label: u8, ctx: u32, addr: Felt, clk: u32, word: Word) -> Self {
pub fn from_ints(label: u8, ctx: u32, addr: u32, clk: u32, word: Word) -> Self {
Self {
label,
ctx: Felt::from(ctx),
addr,
addr: Felt::from(addr),
clk: Felt::from(clk),
word,
}
Expand Down
20 changes: 10 additions & 10 deletions processor/src/chiplets/memory/segment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use super::{BTreeMap, Felt, StarkField, Vec, Word, INIT_MEM_VALUE};
/// 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.
#[derive(Default)]
pub struct MemorySegmentTrace(BTreeMap<u64, Vec<MemorySegmentAccess>>);
pub struct MemorySegmentTrace(BTreeMap<u32, Vec<MemorySegmentAccess>>);

impl MemorySegmentTrace {
// PUBLIC ACCESSORS
Expand All @@ -25,7 +25,7 @@ impl MemorySegmentTrace {
///
/// 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: u64) -> Option<Word> {
pub fn get_value(&self, addr: u32) -> Option<Word> {
match self.0.get(&addr) {
Some(addr_trace) => addr_trace.last().map(|access| access.value()),
None => None,
Expand All @@ -47,13 +47,13 @@ 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, addr_trace[i].value())),
Ok(i) => result.push((addr.into(), addr_trace[i].value())),
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, addr_trace[i - 1].value()));
result.push((addr.into(), addr_trace[i - 1].value()));
}
}
}
Expand All @@ -70,12 +70,12 @@ 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: Felt, clk: Felt) -> Word {
pub fn read(&mut self, addr: u32, clk: Felt) -> Word {
// 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.
self.0
.entry(addr.as_int())
.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);
Expand All @@ -93,12 +93,12 @@ impl MemorySegmentTrace {

/// 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: Felt, clk: Felt, value: Word) {
pub fn write(&mut self, addr: u32, clk: Felt, value: Word) {
// 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.as_int())
.entry(addr)
.and_modify(|addr_trace| addr_trace.push(access))
.or_insert_with(|| vec![access]);
}
Expand All @@ -107,12 +107,12 @@ impl MemorySegmentTrace {
// --------------------------------------------------------------------------------------------

/// Returns a reference to the map underlying this memory segment trace.
pub(super) fn inner(&self) -> &BTreeMap<u64, Vec<MemorySegmentAccess>> {
pub(super) fn inner(&self) -> &BTreeMap<u32, Vec<MemorySegmentAccess>> {
&self.0
}

/// Returns a map underlying this memory segment trace while consuming self.
pub(super) fn into_inner(self) -> BTreeMap<u64, Vec<MemorySegmentAccess>> {
pub(super) fn into_inner(self) -> BTreeMap<u32, Vec<MemorySegmentAccess>> {
self.0
}

Expand Down
57 changes: 28 additions & 29 deletions processor/src/chiplets/memory/tests.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use super::{
super::aux_trace::{ChipletLookup, ChipletsBusRow},
ChipletsBus, Felt, FieldElement, Memory, MemoryLookup, StarkField, TraceFragment, Vec,
ADDR_COL_IDX, CLK_COL_IDX, CTX_COL_IDX, D0_COL_IDX, D1_COL_IDX, D_INV_COL_IDX, ONE,
V_COL_RANGE, ZERO,
ChipletsBus, Felt, FieldElement, Memory, MemoryLookup, TraceFragment, Vec, ADDR_COL_IDX,
CLK_COL_IDX, CTX_COL_IDX, D0_COL_IDX, D1_COL_IDX, D_INV_COL_IDX, ONE, V_COL_RANGE, ZERO,
};
use miden_air::trace::chiplets::memory::{
Selectors, MEMORY_COPY_READ, MEMORY_INIT_READ, MEMORY_READ_LABEL, MEMORY_WRITE,
Expand All @@ -21,14 +20,14 @@ fn mem_read() {
let mut mem = Memory::default();

// read a value from address 0; clk = 1
let addr0 = Felt::new(0);
let addr0 = 0;
let value = mem.read(0, addr0, 1);
assert_eq!([ZERO; 4], value);
assert_eq!(1, mem.size());
assert_eq!(1, mem.trace_len());

// read a value from address 3; clk = 2
let addr3 = Felt::new(3);
let addr3 = 3;
let value = mem.read(0, addr3, 2);
assert_eq!([ZERO; 4], value);
assert_eq!(2, mem.size());
Expand All @@ -41,7 +40,7 @@ fn mem_read() {
assert_eq!(3, mem.trace_len());

// read a value from address 2; clk = 4
let addr2 = Felt::new(2);
let addr2 = 2;
let value = mem.read(0, addr2, 4);
assert_eq!([ZERO; 4], value);
assert_eq!(3, mem.size());
Expand Down Expand Up @@ -76,33 +75,33 @@ fn mem_write() {
let mut mem = Memory::default();

// write a value into address 0; clk = 1
let addr0 = Felt::new(0);
let addr0 = 0;
let value1 = [ONE, ZERO, ZERO, ZERO];
mem.write(0, addr0, 1, value1);
assert_eq!(value1, mem.get_value(0, addr0.as_int()).unwrap());
assert_eq!(value1, mem.get_value(0, addr0).unwrap());
assert_eq!(1, mem.size());
assert_eq!(1, mem.trace_len());

// write a value into address 2; clk = 2
let addr2 = Felt::new(2);
let addr2 = 2;
let value5 = [Felt::new(5), ZERO, ZERO, ZERO];
mem.write(0, addr2, 2, value5);
assert_eq!(value5, mem.get_value(0, addr2.as_int()).unwrap());
assert_eq!(value5, mem.get_value(0, addr2).unwrap());
assert_eq!(2, mem.size());
assert_eq!(2, mem.trace_len());

// write a value into address 1; clk = 3
let addr1 = Felt::new(1);
let addr1 = 1;
let value7 = [Felt::new(7), ZERO, ZERO, ZERO];
mem.write(0, addr1, 3, value7);
assert_eq!(value7, mem.get_value(0, addr1.as_int()).unwrap());
assert_eq!(value7, mem.get_value(0, 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(0, addr0, 4, value9);
assert_eq!(value7, mem.get_value(0, addr1.as_int()).unwrap());
assert_eq!(value7, mem.get_value(0, addr1).unwrap());
assert_eq!(3, mem.size());
assert_eq!(4, mem.trace_len());

Expand Down Expand Up @@ -135,12 +134,12 @@ fn mem_write_read() {
let mut mem = Memory::default();

// write 1 into address 5; clk = 1
let addr5 = Felt::new(5);
let addr5 = 5;
let value1 = [ONE, ZERO, ZERO, ZERO];
mem.write(0, addr5, 1, value1);

// write 4 into address 2; clk = 2
let addr2 = Felt::new(2);
let addr2 = 2;
let value4 = [Felt::new(4), ZERO, ZERO, ZERO];
mem.write(0, addr2, 2, value4);

Expand Down Expand Up @@ -216,33 +215,33 @@ fn mem_multi_context() {

// write a value into ctx = 0, addr = 0; clk = 1
let value1 = [ONE, ZERO, ZERO, ZERO];
mem.write(0, ZERO, 1, value1);
mem.write(0, 0, 1, value1);
assert_eq!(value1, mem.get_value(0, 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, ONE, 4, value2);
mem.write(3, 1, 4, value2);
assert_eq!(value2, mem.get_value(3, 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, ONE, 6);
let value = mem.read(3, 1, 6);
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, ZERO, 7, value3);
mem.write(3, 0, 7, value3);
assert_eq!(value3, mem.get_value(3, 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(0, ZERO, 9);
let value = mem.read(0, 0, 9);
assert_eq!(value1, value);
assert_eq!(3, mem.size());
assert_eq!(5, mem.trace_len());
Expand All @@ -253,25 +252,25 @@ fn mem_multi_context() {

// ctx = 0, addr = 0
let mut prev_row = [ZERO; MEMORY_TRACE_WIDTH];
let memory_access = MemoryLookup::from_ints(MEMORY_WRITE_LABEL, 0, ZERO, 1, value1);
let memory_access = MemoryLookup::from_ints(MEMORY_WRITE_LABEL, 0, 0, 1, value1);
prev_row =
verify_memory_access(&trace, &chiplets_bus, 0, MEMORY_WRITE, &memory_access, prev_row);

let memory_access = MemoryLookup::from_ints(MEMORY_READ_LABEL, 0, ZERO, 9, value1);
let memory_access = MemoryLookup::from_ints(MEMORY_READ_LABEL, 0, 0, 9, value1);
prev_row =
verify_memory_access(&trace, &chiplets_bus, 1, MEMORY_COPY_READ, &memory_access, prev_row);

// ctx = 3, addr = 0
let memory_access = MemoryLookup::from_ints(MEMORY_WRITE_LABEL, 3, ZERO, 7, value3);
let memory_access = MemoryLookup::from_ints(MEMORY_WRITE_LABEL, 3, 0, 7, value3);
prev_row =
verify_memory_access(&trace, &chiplets_bus, 2, MEMORY_WRITE, &memory_access, prev_row);

// ctx = 3, addr = 1
let memory_access = MemoryLookup::from_ints(MEMORY_WRITE_LABEL, 3, ONE, 4, value2);
let memory_access = MemoryLookup::from_ints(MEMORY_WRITE_LABEL, 3, 1, 4, value2);
prev_row =
verify_memory_access(&trace, &chiplets_bus, 3, MEMORY_WRITE, &memory_access, prev_row);

let memory_access = MemoryLookup::from_ints(MEMORY_READ_LABEL, 3, ONE, 6, value2);
let memory_access = MemoryLookup::from_ints(MEMORY_READ_LABEL, 3, 1, 6, value2);
verify_memory_access(&trace, &chiplets_bus, 4, MEMORY_COPY_READ, &memory_access, prev_row);
}

Expand All @@ -282,17 +281,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(0, Felt::new(5), 1, value1);
mem.write(0, 5, 1, value1);

// 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(0, Felt::new(2), 2, value4);
mem.write(0, 2, 2, value4);

// 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, Felt::new(3), 4, value7);
mem.write(3, 3, 4, value7);

// Check memory state at clk = 2
assert_eq!(mem.get_state_at(0, 2), vec![(5, value1)]);
Expand Down Expand Up @@ -350,7 +349,7 @@ fn build_trace_row(
row[0] = op_selectors[0];
row[1] = op_selectors[1];
row[CTX_COL_IDX] = ctx;
row[ADDR_COL_IDX] = addr;
row[ADDR_COL_IDX] = Felt::from(addr);
row[CLK_COL_IDX] = clk;
row[V_COL_RANGE.start] = new_val[0];
row[V_COL_RANGE.start + 1] = new_val[1];
Expand Down
Loading

0 comments on commit 0d47b0e

Please sign in to comment.