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

@@ -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<F: Field>() -> Vec<CrossTableLookup<F>> {
     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];
             }
         }

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<usize> = 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;

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<F: Field>() -> Column<F> {
     Column::sum([columns::IS_AND, columns::IS_OR, columns::IS_XOR])
 }
 
-pub fn ctl_data_memory<F: Field>(op: usize) -> Vec<Column<F>> {
+pub fn ctl_data_memory<F: Field>(channel: usize) -> Vec<Column<F>> {
+    debug_assert!(channel < NUM_CHANNELS);
     let mut cols: Vec<Column<F>> = 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<F: Field>(op: usize) -> Column<F> {
-    Column::single(columns::uses_memop(op))
+pub fn ctl_filter_memory<F: Field>(channel: usize) -> Column<F> {
+    Column::single(columns::mem_channel_used(channel))
 }
 
 #[derive(Copy, Clone)]

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<F: Field>() -> Vec<Column<F>> {
     res
 }
 
-pub fn ctl_filter<F: Field>(op: usize) -> Column<F> {
-    Column::single(is_memop(op))
+pub fn ctl_filter<F: Field>(channel: usize) -> Column<F> {
+    Column::single(is_channel(channel))
 }
 
 #[derive(Copy, Clone)]
@@ -68,9 +69,9 @@ pub fn generate_random_memory_ops<F: RichField, R: Rng>(
         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<F: RichField + Extendable<D>, const D: usize> MemoryStark<F, D> {
                 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;

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;

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;