From 34e73db42b5eb2f0254b0cf8041642585445b6ac Mon Sep 17 00:00:00 2001 From: Daniel Lubarov Date: Mon, 27 Jun 2022 16:03:56 -0700 Subject: [PATCH] Memory naming tweaks (#579) * Memory naming tweaks - Define the channel count and value limbs in a single place, so they're easy to adjust. - Standardize on "channels" which I think is more explicit, since e.g. `num_mem_ops` used to mean either the channel count or total operation count in a trace. * feedback * tweaks * fmt --- evm/src/all_stark.rs | 22 ++++++------- evm/src/cpu/columns.rs | 60 +++++++++++++++++----------------- evm/src/cpu/cpu_stark.rs | 20 +++++++----- evm/src/memory/memory_stark.rs | 13 ++++---- evm/src/memory/mod.rs | 3 ++ evm/src/memory/registers.rs | 26 +++++++-------- 6 files changed, 75 insertions(+), 69 deletions(-) diff --git a/evm/src/all_stark.rs b/evm/src/all_stark.rs index 052b9990..3975e5f4 100644 --- a/evm/src/all_stark.rs +++ b/evm/src/all_stark.rs @@ -3,7 +3,6 @@ use plonky2::field::types::Field; use plonky2::hash::hash_types::RichField; use crate::config::StarkConfig; -use crate::cpu::columns::NUM_MEMORY_OPS; use crate::cpu::cpu_stark; use crate::cpu::cpu_stark::CpuStark; use crate::cross_table_lookup::{CrossTableLookup, TableWithColumns}; @@ -11,8 +10,8 @@ use crate::keccak::keccak_stark; use crate::keccak::keccak_stark::KeccakStark; use crate::logic; use crate::logic::LogicStark; -use crate::memory::memory_stark; use crate::memory::memory_stark::MemoryStark; +use crate::memory::{memory_stark, NUM_CHANNELS}; use crate::stark::Stark; #[derive(Clone)] @@ -65,7 +64,7 @@ impl Table { #[allow(unused)] // TODO: Should be used soon. pub(crate) fn all_cross_table_lookups() -> Vec> { let mut cross_table_lookups = vec![ctl_keccak(), ctl_logic()]; - cross_table_lookups.extend((0..NUM_MEMORY_OPS).map(ctl_memory)); + cross_table_lookups.extend((0..NUM_CHANNELS).map(ctl_memory)); cross_table_lookups } @@ -133,6 +132,7 @@ mod tests { use crate::keccak::keccak_stark::{KeccakStark, NUM_INPUTS, NUM_ROUNDS}; use crate::logic::{self, LogicStark}; use crate::memory::memory_stark::{generate_random_memory_ops, MemoryStark}; + use crate::memory::NUM_CHANNELS; use crate::proof::AllProof; use crate::prover::prove; use crate::recursive_verifier::{ @@ -280,8 +280,8 @@ mod tests { for i in 0..num_memory_ops { let mem_timestamp = memory_trace[memory::registers::TIMESTAMP].values[i]; let clock = mem_timestamp; - let op = (0..4) - .filter(|&o| memory_trace[memory::registers::is_memop(o)].values[i] == F::ONE) + let op = (0..NUM_CHANNELS) + .filter(|&o| memory_trace[memory::registers::is_channel(o)].values[i] == F::ONE) .collect_vec()[0]; if mem_timestamp != last_timestamp { @@ -289,18 +289,18 @@ mod tests { last_timestamp = mem_timestamp; } - cpu_trace_rows[current_cpu_index][cpu::columns::uses_memop(op)] = F::ONE; + cpu_trace_rows[current_cpu_index][cpu::columns::mem_channel_used(op)] = F::ONE; cpu_trace_rows[current_cpu_index][cpu::columns::CLOCK] = clock; - cpu_trace_rows[current_cpu_index][cpu::columns::memop_is_read(op)] = + cpu_trace_rows[current_cpu_index][cpu::columns::mem_is_read(op)] = memory_trace[memory::registers::IS_READ].values[i]; - cpu_trace_rows[current_cpu_index][cpu::columns::memop_addr_context(op)] = + cpu_trace_rows[current_cpu_index][cpu::columns::mem_addr_context(op)] = memory_trace[memory::registers::ADDR_CONTEXT].values[i]; - cpu_trace_rows[current_cpu_index][cpu::columns::memop_addr_segment(op)] = + cpu_trace_rows[current_cpu_index][cpu::columns::mem_addr_segment(op)] = memory_trace[memory::registers::ADDR_SEGMENT].values[i]; - cpu_trace_rows[current_cpu_index][cpu::columns::memop_addr_virtual(op)] = + cpu_trace_rows[current_cpu_index][cpu::columns::mem_addr_virtual(op)] = memory_trace[memory::registers::ADDR_VIRTUAL].values[i]; for j in 0..8 { - cpu_trace_rows[current_cpu_index][cpu::columns::memop_value(op, j)] = + cpu_trace_rows[current_cpu_index][cpu::columns::mem_value(op, j)] = memory_trace[memory::registers::value_limb(j)].values[i]; } } diff --git a/evm/src/cpu/columns.rs b/evm/src/cpu/columns.rs index c603ceb6..8b65b84f 100644 --- a/evm/src/cpu/columns.rs +++ b/evm/src/cpu/columns.rs @@ -3,6 +3,8 @@ use std::ops::Range; +use crate::memory; + /// Filter. 1 if the row is part of bootstrapping the kernel code, 0 otherwise. pub const IS_BOOTSTRAP_KERNEL: usize = 0; @@ -161,47 +163,45 @@ pub const LOGIC_OUTPUT: Range = LOGIC_INPUT1.end..LOGIC_INPUT1.end + 16; pub const SIMPLE_LOGIC_DIFF: usize = LOGIC_OUTPUT.end; pub const SIMPLE_LOGIC_DIFF_INV: usize = SIMPLE_LOGIC_DIFF + 1; -pub(crate) const NUM_MEMORY_OPS: usize = 4; -pub(crate) const NUM_MEMORY_VALUE_LIMBS: usize = 8; - pub(crate) const CLOCK: usize = SIMPLE_LOGIC_DIFF_INV + 1; -// Uses_memop(i) is `F::ONE` iff this row includes a memory operation in its `i`th spot. -const USES_MEMOP_START: usize = CLOCK + 1; -pub const fn uses_memop(op: usize) -> usize { - debug_assert!(op < NUM_MEMORY_OPS); - USES_MEMOP_START + op +/// 1 if this row includes a memory operation in the `i`th channel of the memory bus, otherwise 0. +const MEM_CHANNEL_USED_START: usize = CLOCK + 1; +pub const fn mem_channel_used(channel: usize) -> usize { + debug_assert!(channel < memory::NUM_CHANNELS); + MEM_CHANNEL_USED_START + channel } -const MEMOP_ISREAD_START: usize = USES_MEMOP_START + NUM_MEMORY_OPS; -pub const fn memop_is_read(op: usize) -> usize { - debug_assert!(op < NUM_MEMORY_OPS); - MEMOP_ISREAD_START + op +const MEM_ISREAD_START: usize = MEM_CHANNEL_USED_START + memory::NUM_CHANNELS; +pub const fn mem_is_read(channel: usize) -> usize { + debug_assert!(channel < memory::NUM_CHANNELS); + MEM_ISREAD_START + channel } -const MEMOP_ADDR_CONTEXT_START: usize = MEMOP_ISREAD_START + NUM_MEMORY_OPS; -pub const fn memop_addr_context(op: usize) -> usize { - debug_assert!(op < NUM_MEMORY_OPS); - MEMOP_ADDR_CONTEXT_START + op +const MEM_ADDR_CONTEXT_START: usize = MEM_ISREAD_START + memory::NUM_CHANNELS; +pub const fn mem_addr_context(channel: usize) -> usize { + debug_assert!(channel < memory::NUM_CHANNELS); + MEM_ADDR_CONTEXT_START + channel } -const MEMOP_ADDR_SEGMENT_START: usize = MEMOP_ADDR_CONTEXT_START + NUM_MEMORY_OPS; -pub const fn memop_addr_segment(op: usize) -> usize { - debug_assert!(op < NUM_MEMORY_OPS); - MEMOP_ADDR_SEGMENT_START + op +const MEM_ADDR_SEGMENT_START: usize = MEM_ADDR_CONTEXT_START + memory::NUM_CHANNELS; +pub const fn mem_addr_segment(channel: usize) -> usize { + debug_assert!(channel < memory::NUM_CHANNELS); + MEM_ADDR_SEGMENT_START + channel } -const MEMOP_ADDR_VIRTUAL_START: usize = MEMOP_ADDR_SEGMENT_START + NUM_MEMORY_OPS; -pub const fn memop_addr_virtual(op: usize) -> usize { - debug_assert!(op < NUM_MEMORY_OPS); - MEMOP_ADDR_VIRTUAL_START + op +const MEM_ADDR_VIRTUAL_START: usize = MEM_ADDR_SEGMENT_START + memory::NUM_CHANNELS; +pub const fn mem_addr_virtual(channel: usize) -> usize { + debug_assert!(channel < memory::NUM_CHANNELS); + MEM_ADDR_VIRTUAL_START + channel } -const MEMOP_ADDR_VALUE_START: usize = MEMOP_ADDR_VIRTUAL_START + NUM_MEMORY_OPS; -pub const fn memop_value(op: usize, limb: usize) -> usize { - debug_assert!(op < NUM_MEMORY_OPS); - debug_assert!(limb < NUM_MEMORY_VALUE_LIMBS); - MEMOP_ADDR_VALUE_START + op * NUM_MEMORY_VALUE_LIMBS + limb +const MEM_ADDR_VALUE_START: usize = MEM_ADDR_VIRTUAL_START + memory::NUM_CHANNELS; +pub const fn mem_value(channel: usize, limb: usize) -> usize { + debug_assert!(channel < memory::NUM_CHANNELS); + debug_assert!(limb < memory::VALUE_LIMBS); + MEM_ADDR_VALUE_START + channel * memory::VALUE_LIMBS + limb } -pub const NUM_CPU_COLUMNS: usize = MEMOP_ADDR_VALUE_START + NUM_MEMORY_OPS * NUM_MEMORY_VALUE_LIMBS; +pub const NUM_CPU_COLUMNS: usize = + MEM_ADDR_VALUE_START + memory::NUM_CHANNELS * memory::VALUE_LIMBS; diff --git a/evm/src/cpu/cpu_stark.rs b/evm/src/cpu/cpu_stark.rs index 74e5249f..1c8522c3 100644 --- a/evm/src/cpu/cpu_stark.rs +++ b/evm/src/cpu/cpu_stark.rs @@ -9,6 +9,7 @@ use plonky2::hash::hash_types::RichField; use crate::constraint_consumer::{ConstraintConsumer, RecursiveConstraintConsumer}; use crate::cpu::{columns, decode, simple_logic}; use crate::cross_table_lookup::Column; +use crate::memory::NUM_CHANNELS; use crate::permutation::PermutationPair; use crate::stark::Stark; use crate::vars::{StarkEvaluationTargets, StarkEvaluationVars}; @@ -35,21 +36,24 @@ pub fn ctl_filter_logic() -> Column { Column::sum([columns::IS_AND, columns::IS_OR, columns::IS_XOR]) } -pub fn ctl_data_memory(op: usize) -> Vec> { +pub fn ctl_data_memory(channel: usize) -> Vec> { + debug_assert!(channel < NUM_CHANNELS); let mut cols: Vec> = Column::singles([ columns::CLOCK, - columns::memop_is_read(op), - columns::memop_addr_context(op), - columns::memop_addr_segment(op), - columns::memop_addr_virtual(op), + columns::mem_is_read(channel), + columns::mem_addr_context(channel), + columns::mem_addr_segment(channel), + columns::mem_addr_virtual(channel), ]) .collect_vec(); - cols.extend(Column::singles((0..8).map(|j| columns::memop_value(op, j)))); + cols.extend(Column::singles( + (0..8).map(|j| columns::mem_value(channel, j)), + )); cols } -pub fn ctl_filter_memory(op: usize) -> Column { - Column::single(columns::uses_memop(op)) +pub fn ctl_filter_memory(channel: usize) -> Column { + Column::single(columns::mem_channel_used(channel)) } #[derive(Copy, Clone)] diff --git a/evm/src/memory/memory_stark.rs b/evm/src/memory/memory_stark.rs index e98a0379..1c7c6c45 100644 --- a/evm/src/memory/memory_stark.rs +++ b/evm/src/memory/memory_stark.rs @@ -11,7 +11,7 @@ use plonky2::timed; use plonky2::util::timing::TimingTree; use rand::Rng; -use super::registers::is_memop; +use super::registers::is_channel; use crate::constraint_consumer::{ConstraintConsumer, RecursiveConstraintConsumer}; use crate::cross_table_lookup::Column; use crate::lookup::{eval_lookups, eval_lookups_circuit, permuted_cols}; @@ -21,6 +21,7 @@ use crate::memory::registers::{ SEGMENT_FIRST_CHANGE, SORTED_ADDR_CONTEXT, SORTED_ADDR_SEGMENT, SORTED_ADDR_VIRTUAL, SORTED_IS_READ, SORTED_TIMESTAMP, TIMESTAMP, VIRTUAL_FIRST_CHANGE, }; +use crate::memory::NUM_CHANNELS; use crate::permutation::PermutationPair; use crate::stark::Stark; use crate::util::trace_rows_to_poly_values; @@ -35,8 +36,8 @@ pub fn ctl_data() -> Vec> { res } -pub fn ctl_filter(op: usize) -> Column { - Column::single(is_memop(op)) +pub fn ctl_filter(channel: usize) -> Column { + Column::single(is_channel(channel)) } #[derive(Copy, Clone)] @@ -68,9 +69,9 @@ pub fn generate_random_memory_ops( let mut new_writes_this_cycle = HashMap::new(); let mut has_read = false; for _ in 0..2 { - let mut channel_index = rng.gen_range(0..4); + let mut channel_index = rng.gen_range(0..NUM_CHANNELS); while used_indices.contains(&channel_index) { - channel_index = rng.gen_range(0..4); + channel_index = rng.gen_range(0..NUM_CHANNELS); } used_indices.insert(channel_index); @@ -240,7 +241,7 @@ impl, const D: usize> MemoryStark { virt, value, } = memory_ops[i]; - trace_cols[is_memop(channel_index)][i] = F::ONE; + trace_cols[is_channel(channel_index)][i] = F::ONE; trace_cols[TIMESTAMP][i] = timestamp; trace_cols[IS_READ][i] = is_read; trace_cols[ADDR_CONTEXT][i] = context; diff --git a/evm/src/memory/mod.rs b/evm/src/memory/mod.rs index 51adaf72..261b356d 100644 --- a/evm/src/memory/mod.rs +++ b/evm/src/memory/mod.rs @@ -1,2 +1,5 @@ pub mod memory_stark; pub mod registers; + +pub(crate) const NUM_CHANNELS: usize = 4; +pub(crate) const VALUE_LIMBS: usize = 8; diff --git a/evm/src/memory/registers.rs b/evm/src/memory/registers.rs index 2e458cfb..4ed7f866 100644 --- a/evm/src/memory/registers.rs +++ b/evm/src/memory/registers.rs @@ -1,7 +1,6 @@ -//! Memory unit. +//! Memory registers. -const NUM_MEMORY_OPS: usize = 4; -const NUM_MEMORY_VALUE_LIMBS: usize = 8; +use crate::memory::{NUM_CHANNELS, VALUE_LIMBS}; pub(crate) const TIMESTAMP: usize = 0; pub(crate) const IS_READ: usize = TIMESTAMP + 1; @@ -12,12 +11,12 @@ pub(crate) const ADDR_VIRTUAL: usize = ADDR_SEGMENT + 1; // Eight limbs to hold up to a 256-bit value. const VALUE_START: usize = ADDR_VIRTUAL + 1; pub(crate) const fn value_limb(i: usize) -> usize { - debug_assert!(i < NUM_MEMORY_VALUE_LIMBS); + debug_assert!(i < VALUE_LIMBS); VALUE_START + i } // Separate columns for the same memory operations, sorted by (addr, timestamp). -pub(crate) const SORTED_TIMESTAMP: usize = VALUE_START + NUM_MEMORY_VALUE_LIMBS; +pub(crate) const SORTED_TIMESTAMP: usize = VALUE_START + VALUE_LIMBS; pub(crate) const SORTED_IS_READ: usize = SORTED_TIMESTAMP + 1; pub(crate) const SORTED_ADDR_CONTEXT: usize = SORTED_IS_READ + 1; pub(crate) const SORTED_ADDR_SEGMENT: usize = SORTED_ADDR_CONTEXT + 1; @@ -25,7 +24,7 @@ pub(crate) const SORTED_ADDR_VIRTUAL: usize = SORTED_ADDR_SEGMENT + 1; const SORTED_VALUE_START: usize = SORTED_ADDR_VIRTUAL + 1; pub(crate) const fn sorted_value_limb(i: usize) -> usize { - debug_assert!(i < NUM_MEMORY_VALUE_LIMBS); + debug_assert!(i < VALUE_LIMBS); SORTED_VALUE_START + i } @@ -33,7 +32,7 @@ pub(crate) const fn sorted_value_limb(i: usize) -> usize { // columns), and the previous parts do not differ. // That is, e.g., `SEGMENT_FIRST_CHANGE` is `F::ONE` iff `SORTED_ADDR_CONTEXT` is the same in this // row and the next, but `SORTED_ADDR_SEGMENT` is not. -pub(crate) const CONTEXT_FIRST_CHANGE: usize = SORTED_VALUE_START + NUM_MEMORY_VALUE_LIMBS; +pub(crate) const CONTEXT_FIRST_CHANGE: usize = SORTED_VALUE_START + VALUE_LIMBS; pub(crate) const SEGMENT_FIRST_CHANGE: usize = CONTEXT_FIRST_CHANGE + 1; pub(crate) const VIRTUAL_FIRST_CHANGE: usize = SEGMENT_FIRST_CHANGE + 1; @@ -45,13 +44,12 @@ pub(crate) const COUNTER: usize = RANGE_CHECK + 1; pub(crate) const RANGE_CHECK_PERMUTED: usize = COUNTER + 1; pub(crate) const COUNTER_PERMUTED: usize = RANGE_CHECK_PERMUTED + 1; -// Flags to indicate if this operation corresponds to the `i`th memory op in a certain row of the -// CPU table. -const IS_MEMOP_START: usize = COUNTER_PERMUTED + 1; +// Flags to indicate if this operation came from the `i`th channel of the memory bus. +const IS_CHANNEL_START: usize = COUNTER_PERMUTED + 1; #[allow(dead_code)] -pub(crate) const fn is_memop(i: usize) -> usize { - debug_assert!(i < NUM_MEMORY_OPS); - IS_MEMOP_START + i +pub(crate) const fn is_channel(channel: usize) -> usize { + debug_assert!(channel < NUM_CHANNELS); + IS_CHANNEL_START + channel } -pub(crate) const NUM_REGISTERS: usize = IS_MEMOP_START + NUM_MEMORY_OPS; +pub(crate) const NUM_REGISTERS: usize = IS_CHANNEL_START + NUM_CHANNELS;