From 1287123846cd9ac76d0d3719ace1ab2fc3e65c71 Mon Sep 17 00:00:00 2001 From: MauroFab Date: Tue, 7 Jul 2026 10:58:09 -0300 Subject: [PATCH 1/5] Make the u32 value domain a compile-time fact for MemwOperation MemwOperation::new and with_old take [u32; 8] directly and pack_register_value returns u32 limbs, so the u64->u32 narrowing debug_asserts (and the release-mode silent-truncation hazard behind them) are deleted rather than guarded: out-of-domain values are now unrepresentable. The register fast path's bare `as u32` casts, which skipped even the debug_assert, disappear for the same reason. --- prover/src/tables/memw.rs | 27 ++--------- prover/src/tables/trace_builder.rs | 77 +++++++++++++++++------------- 2 files changed, 47 insertions(+), 57 deletions(-) diff --git a/prover/src/tables/memw.rs b/prover/src/tables/memw.rs index f0f8f2df7..f4af949e1 100644 --- a/prover/src/tables/memw.rs +++ b/prover/src/tables/memw.rs @@ -131,7 +131,7 @@ impl MemwOperation { pub fn new( is_register: bool, base_address: u64, - value: [u64; 8], + value: [u32; 8], timestamp: u64, width: u8, is_read: bool, @@ -139,20 +139,7 @@ impl MemwOperation { Self { is_register, base_address, - // Callers build a [u64; 8] transiently on the stack; we store the u32 - // domain (byte / 32-bit register half) so the persisted struct is half - // the size. Values never exceed u32 (every element is a byte or a - // pack_register_value 32-bit limb). This is an always-on `assert!` (not - // `debug_assert!`): a caller passing an out-of-domain value would - // otherwise silently truncate here and produce an unsound witness in - // release builds, so the invariant is enforced in every build profile. - value: value.map(|v| { - assert!( - v <= u32::MAX as u64, - "MemwOperation value element exceeds u32: {v}" - ); - v as u32 - }), + value, timestamp, width, is_read, @@ -162,14 +149,8 @@ impl MemwOperation { } /// Set the old values (from memory model). - pub fn with_old(mut self, old: [u64; 8], old_timestamp: [u64; 8]) -> Self { - self.old = old.map(|v| { - assert!( - v <= u32::MAX as u64, - "MemwOperation old element exceeds u32: {v}" - ); - v as u32 - }); + pub fn with_old(mut self, old: [u32; 8], old_timestamp: [u64; 8]) -> Self { + self.old = old; self.old_timestamp = old_timestamp; self } diff --git a/prover/src/tables/trace_builder.rs b/prover/src/tables/trace_builder.rs index ad56ef757..4406e0c75 100644 --- a/prover/src/tables/trace_builder.rs +++ b/prover/src/tables/trace_builder.rs @@ -133,12 +133,12 @@ impl MemoryState { } /// Read multiple bytes. Returns arrays of values and timestamps. - fn read_bytes(&self, base_address: u64, count: usize) -> ([u64; 8], [u64; 8]) { - let mut values = [0u64; 8]; + fn read_bytes(&self, base_address: u64, count: usize) -> ([u32; 8], [u64; 8]) { + let mut values = [0u32; 8]; let mut timestamps = [0u64; 8]; for i in 0..count { let (val, ts) = self.read_byte(base_address.wrapping_add(i as u64)); - values[i] = val as u64; + values[i] = val as u32; timestamps[i] = ts; } (values, timestamps) @@ -313,8 +313,17 @@ fn cpu_op_to_bytes_and_signed(op: &CpuOperation) -> (usize, bool) { /// Pack a 64-bit register value into the MEMW value format. /// /// For register operations, values are packed as [lo32, hi32, 0, 0, 0, 0, 0, 0]. -fn pack_register_value(value: u64) -> [u64; 8] { - [value & 0xFFFF_FFFF, value >> 32, 0, 0, 0, 0, 0, 0] +fn pack_register_value(value: u64) -> [u32; 8] { + [ + (value & 0xFFFF_FFFF) as u32, + (value >> 32) as u32, + 0, + 0, + 0, + 0, + 0, + 0, + ] } // ============================================================================= @@ -711,9 +720,9 @@ fn collect_load_op_from_cpu( let (_old_values, old_timestamps) = memory_state.read_bytes(base_address, 8); // Extract individual bytes from loaded value - let mut value_bytes = [0u64; 8]; + let mut value_bytes = [0u32; 8]; for (j, byte) in value_bytes.iter_mut().take(byte_count).enumerate() { - *byte = (loaded_value >> (j * 8)) & 0xFF; + *byte = ((loaded_value >> (j * 8)) & 0xFF) as u32; } // Sign/zero extend the upper bytes @@ -744,7 +753,7 @@ fn collect_load_op_from_cpu( op.timestamp, byte_count as u8, signed, - res_bytes, + res_bytes.map(u64::from), ); // Collect MSB8 lookups for sign bit extraction @@ -774,9 +783,9 @@ fn collect_store_op_from_cpu(op: &CpuOperation, memory_state: &mut MemoryState) // of all 8 bytes. Must match CPU M7 which sends full rv2 as [lo32, hi32]. // Bus 16: only positions 0..byte_count participate (controlled by w2/w4/write8 // multiplicities), so extra bytes don't affect memory consistency. - let mut value_bytes = [0u64; 8]; + let mut value_bytes = [0u32; 8]; for (j, byte) in value_bytes.iter_mut().enumerate() { - *byte = (store_value >> (j * 8)) & 0xFF; + *byte = ((store_value >> (j * 8)) & 0xFF) as u32; } // The STORE chip now owns this MEMW write (the CPU sends MEMORY instead of @@ -849,10 +858,10 @@ fn collect_ecsm_ops( for (base, bytes) in [(addr_xg, &witness.x_g), (addr_k, &witness.k)] { for i in 0..4 { let addr = base.wrapping_add((8 * i) as u64); - let mut value = [0u64; 8]; + let mut value = [0u32; 8]; let mut dword = 0u64; for j in 0..8 { - value[j] = bytes[8 * i + j] as u64; + value[j] = bytes[8 * i + j] as u32; dword |= (bytes[8 * i + j] as u64) << (8 * j); } let (_old, old_ts) = memory_state.read_bytes(addr, 8); @@ -877,7 +886,7 @@ fn collect_ecsm_ops( for offset in 0..32u64 { let addr = addr_k.wrapping_add(offset); let byte = k[offset as usize]; - let value = [byte as u64, 0, 0, 0, 0, 0, 0, 0]; + let value = [byte as u32, 0, 0, 0, 0, 0, 0, 0]; let (_v, old_ts) = memory_state.read_byte(addr); memw_ops.push( MemwOperation::new(false, addr, value, t + 1, 1, true) @@ -889,10 +898,10 @@ fn collect_ecsm_ops( // xR writes at T + 2 (4 doublewords). for i in 0..4 { let addr = addr_xr.wrapping_add((8 * i) as u64); - let mut value = [0u64; 8]; + let mut value = [0u32; 8]; let mut dword = 0u64; for j in 0..8 { - value[j] = witness.x_r[8 * i + j] as u64; + value[j] = witness.x_r[8 * i + j] as u32; dword |= (witness.x_r[8 * i + j] as u64) << (8 * j); } let (old_vals, old_ts) = memory_state.read_bytes(addr, 8); @@ -949,10 +958,10 @@ fn collect_register_ops_from_cpu( // the identical MemwOperation only on the (rare) general/aligned fallback. memw_ops.push_reg_access( reg_addr, - reg_value[0] as u32, - reg_value[1] as u32, - reg_value[0] as u32, - reg_value[1] as u32, + reg_value[0], + reg_value[1], + reg_value[0], + reg_value[1], ts, old_ts, true, @@ -977,10 +986,10 @@ fn collect_register_ops_from_cpu( let ts = op.timestamp + 1; memw_ops.push_reg_access( reg_addr, - reg_value[0] as u32, - reg_value[1] as u32, - reg_value[0] as u32, - reg_value[1] as u32, + reg_value[0], + reg_value[1], + reg_value[0], + reg_value[1], ts, old_ts, true, @@ -1002,10 +1011,10 @@ fn collect_register_ops_from_cpu( let ts = op.timestamp + 2; memw_ops.push_reg_access( reg_addr, - reg_value[0] as u32, - reg_value[1] as u32, - old_value[0] as u32, - old_value[1] as u32, + reg_value[0], + reg_value[1], + old_value[0], + old_value[1], ts, old_ts, false, @@ -1249,8 +1258,8 @@ fn collect_commit_memw_ops( let new_index = old_index .checked_add(u32::try_from(count).expect("commit_count exceeds u32 range")) .expect("commit index exceeds u32 range"); - let old_value = [old_index as u64, 0, 0, 0, 0, 0, 0, 0]; - let new_value = [new_index as u64, 0, 0, 0, 0, 0, 0, 0]; + let old_value = [old_index, 0, 0, 0, 0, 0, 0, 0]; + let new_value = [new_index, 0, 0, 0, 0, 0, 0, 0]; let old_timestamps = [old_ts, 0, 0, 0, 0, 0, 0, 0]; let memw_op = MemwOperation::new( true, @@ -1269,7 +1278,7 @@ fn collect_commit_memw_ops( for i in 0..count { let addr = buf_addr.wrapping_add(i); let (byte_val, old_ts) = memory_state.read_byte(addr); - let value = [byte_val as u64, 0, 0, 0, 0, 0, 0, 0]; + let value = [byte_val as u32, 0, 0, 0, 0, 0, 0, 0]; let old_timestamps = [old_ts, 0, 0, 0, 0, 0, 0, 0]; let memw_op = MemwOperation::new(false, addr, value, ts, 1, true).with_old(value, old_timestamps); @@ -1377,10 +1386,10 @@ fn collect_keccak_memw_ops( .checked_add(lane_idx as u64 * 8) .expect("keccak state address range must be validated by the executor"); - let mut old_bytes = [0u64; 8]; + let mut old_bytes = [0u32; 8]; let mut old_timestamps = [0u64; 8]; for b in 0..8 { - old_bytes[b] = (in_lane >> (b * 8)) & 0xFF; + old_bytes[b] = ((in_lane >> (b * 8)) & 0xFF) as u32; let byte_addr = lane_addr .checked_add(b as u64) .expect("keccak state address range must be validated by the executor"); @@ -1388,9 +1397,9 @@ fn collect_keccak_memw_ops( old_timestamps[b] = old_ts; } - let mut value_bytes = [0u64; 8]; + let mut value_bytes = [0u32; 8]; for (b, byte) in value_bytes.iter_mut().enumerate() { - *byte = (out_lane >> (b * 8)) & 0xFF; + *byte = ((out_lane >> (b * 8)) & 0xFF) as u32; } let memw_op = MemwOperation::new(false, lane_addr, value_bytes, ts, 8, true) From 47948a26a6a8ba73ff01fb59e344082d4a7a80c7 Mon Sep 17 00:00:00 2001 From: MauroFab Date: Tue, 7 Jul 2026 11:14:00 -0300 Subject: [PATCH 2/5] Feed bitwise collectors straight into the histogram; cut reduce allocations - Collectors now take &mut BitwiseHistogram instead of returning Vec. The heavy sources count with no per-source vector at all: MEMW_R bumps one IS_HALFWORD per row (tens of millions of rows), PAGE bumps one ARE_BYTES per byte of every touched page, and CPU padding collapses to two bump_n calls instead of an O(padding_rows) vector of identical zero ops. - reduce_with replaces reduce(identity, ..) in the rayon tree-reduce, eliminating one zeroed 80 MiB identity histogram per reduce leaf. - RegRow.address shrinks to u16 (register index 0..=255), taking the largest persisted array of the walk from 40 to 32 bytes per row. --- prover/src/tables/bitwise.rs | 9 +- prover/src/tables/memw_register.rs | 32 ++++-- prover/src/tables/trace_builder.rs | 163 +++++++++++++---------------- 3 files changed, 104 insertions(+), 100 deletions(-) diff --git a/prover/src/tables/bitwise.rs b/prover/src/tables/bitwise.rs index fc4a14f8d..25d7c960d 100644 --- a/prover/src/tables/bitwise.rs +++ b/prover/src/tables/bitwise.rs @@ -517,6 +517,13 @@ impl BitwiseHistogram { /// Increment the counter for one lookup. #[inline] pub fn bump(&mut self, op: BitwiseOperation) { + self.bump_n(op, 1); + } + + /// Add `n` occurrences of one lookup in a single step (e.g. CPU padding rows, + /// which all send identical all-zero lookups). + #[inline] + pub fn bump_n(&mut self, op: BitwiseOperation, n: u64) { let idx = lookup_type_index(op.lookup_type) * NUM_ROWS + row_index(op.x, op.y, op.z); // (x, y) are u8, and row_index debug-asserts z < 16, so in debug builds a // corrupt op fails loudly here. In release an out-of-domain z would NOT @@ -524,7 +531,7 @@ impl BitwiseHistogram { // mis-count both cells — the proof then fails verification instead of the // prover crashing. What actually upholds the invariant is that every // `BitwiseOperation` constructor masks or debug-asserts z < 16. - self.counters[idx] += 1; + self.counters[idx] += n; } /// Fold a slice of lookups into the histogram. diff --git a/prover/src/tables/memw_register.rs b/prover/src/tables/memw_register.rs index d47b9744a..7e5ff7c02 100644 --- a/prover/src/tables/memw_register.rs +++ b/prover/src/tables/memw_register.rs @@ -43,7 +43,7 @@ use stark::trace::TraceTable; use stark::constraints::builder::{ConstraintBuilder, ConstraintSet}; -use super::bitwise::{BitwiseOperation, BitwiseOperationType}; +use super::bitwise::{BitwiseHistogram, BitwiseOperation, BitwiseOperationType}; use super::memw::MemwOperation; use super::types::{BusId, FE, GoldilocksExtension, GoldilocksField, VmTable}; use crate::constraints::templates::emit_is_bit; @@ -105,7 +105,9 @@ pub mod cols { /// enforced by `is_register_op`; the upper limb is TIMESTAMP_1 = timestamp>>32) #[derive(Debug, Clone, Copy)] pub(crate) struct RegRow { - address: u64, + /// Register index 0..=255 (`base_address / 2`); u16 keeps the struct at + /// 32 bytes — it is the largest persisted array of the walk. + address: u16, timestamp: u64, val0: u32, val1: u32, @@ -140,8 +142,12 @@ impl RegRow { 0, "register base_address must be even (got {reg_addr})" ); + debug_assert!( + reg_addr / 2 <= u16::MAX as u64, + "register index exceeds u16 (got base_address {reg_addr})" + ); RegRow { - address: reg_addr / 2, + address: (reg_addr / 2) as u16, timestamp, val0, val1, @@ -210,7 +216,7 @@ pub(crate) fn generate_memw_register_trace_from_rows( for (row_idx, r) in rows.iter().enumerate() { // ADDRESS = base_address / 2 (already divided in RegRow). - table.set_u64(row_idx, cols::ADDRESS, r.address); + table.set_u64(row_idx, cols::ADDRESS, r.address as u64); // Timestamp split into lo/hi 32-bit words. table.set_dword_wl(row_idx, cols::TIMESTAMP_0, r.timestamp); // Value: registers are DWordWL = 2 words. @@ -250,13 +256,17 @@ fn memw_register_is_half_lookup(ts_lo: u32, old_ts_lo: u32) -> BitwiseOperation ) } -/// IS_HALFWORD bitwise lookups for MEMW_R, computed directly from [`RegRow`]s via -/// the shared [`memw_register_is_half_lookup`] helper (the same lookup the MEMW_R -/// trace fill uses), so the multiplicities stay consistent with that table. -pub(crate) fn collect_bitwise_from_memw_register(rows: &[RegRow]) -> Vec { - rows.iter() - .map(|r| memw_register_is_half_lookup((r.timestamp & 0xFFFF_FFFF) as u32, r.old_ts_lo)) - .collect() +/// IS_HALFWORD bitwise lookups for MEMW_R, bumped straight into the histogram +/// via the shared [`memw_register_is_half_lookup`] helper (the same lookup the +/// MEMW_R trace fill uses), one per row. No intermediate op vector: register +/// rows number in the tens of millions and the histogram is the only consumer. +pub(crate) fn collect_bitwise_from_memw_register(rows: &[RegRow], hist: &mut BitwiseHistogram) { + for r in rows { + hist.bump(memw_register_is_half_lookup( + (r.timestamp & 0xFFFF_FFFF) as u32, + r.old_ts_lo, + )); + } } // ========================================================================= diff --git a/prover/src/tables/trace_builder.rs b/prover/src/tables/trace_builder.rs index 4406e0c75..5eb9cfd0f 100644 --- a/prover/src/tables/trace_builder.rs +++ b/prover/src/tables/trace_builder.rs @@ -1983,32 +1983,21 @@ fn collect_bitwise_from_branch(branch_ops: &[BranchOperation]) -> Vec Vec { - if num_padding_rows == 0 { - return Vec::new(); - } - - let mut ops = Vec::with_capacity(num_padding_rows * 14); - for _ in 0..num_padding_rows { - // The shrunk CPU sends, per row (incl. padding where all values are 0): - // 3× ARE_BYTES (rs1/rs2, rd/instruction_length, alu_flags/mem_flags) and - // 4× IS_HALF (the four `res` halves). - for _ in 0..3 { - ops.push(BitwiseOperation::byte_op( - BitwiseOperationType::AreBytes, - 0, - 0, - )); - } - for _ in 0..4 { - ops.push(BitwiseOperation::halfword( - BitwiseOperationType::IsHalf, - 0, - 0, - )); - } - } - ops +fn add_padding_byte_checks(hist: &mut bitwise::BitwiseHistogram, num_padding_rows: usize) { + // The shrunk CPU sends, per row (incl. padding where all values are 0): + // 3× ARE_BYTES (rs1/rs2, rd/instruction_length, alu_flags/mem_flags) and + // 4× IS_HALF (the four `res` halves). Padding rows all send the identical + // all-zero lookups, so this is two O(1) histogram bumps instead of an + // O(num_padding_rows) op vector. + let n = num_padding_rows as u64; + hist.bump_n( + BitwiseOperation::byte_op(BitwiseOperationType::AreBytes, 0, 0), + 3 * n, + ); + hist.bump_n( + BitwiseOperation::halfword(BitwiseOperationType::IsHalf, 0, 0), + 4 * n, + ); } /// Collects ARE_BYTES lookups from PAGE data (init and fini values). @@ -2128,11 +2117,11 @@ fn collect_bitwise_from_page( image: &I, memory_state: &MemoryState, exclude_touched: bool, -) -> Vec { + hist: &mut bitwise::BitwiseHistogram, +) { use std::collections::BTreeSet; let page_size = page::DEFAULT_PAGE_SIZE; - let mut bitwise_ops = Vec::new(); let init_page_data = build_init_page_data(image); @@ -2164,16 +2153,16 @@ fn collect_bitwise_from_page( // Get fini value (from final_state or init if never accessed) let fini = final_state.get(&addr).map_or(init, |state| state.value); - // C1+C2: ARE_BYTES[init, fini] — batched range check for both bytes - bitwise_ops.push(BitwiseOperation::byte_op( + // C1+C2: ARE_BYTES[init, fini] — batched range check for both bytes. + // Bumped straight into the histogram: this loop visits every byte of + // every touched page, and the histogram is the only consumer. + hist.bump(BitwiseOperation::byte_op( BitwiseOperationType::AreBytes, init, fini, )); } } - - bitwise_ops } // ============================================================================= @@ -3047,64 +3036,58 @@ fn build_traces( // The per-source bitwise collectors are all pure functions of their inputs and the // BITWISE multiplicities are order-independent (they ride a permutation-invariant bus), - // so we collect every source in parallel and concatenate. The result is byte-identical - // to the previous serial `.extend()` chain regardless of order. + // so we collect every source in parallel. The result is byte-identical to the previous + // serial `.extend()` chain regardless of order. // // MUL/DVRM dedup their per-unique bit-gated lookups PER CHIP INSTANCE, so pass the same // chunk size used to split them into instances so multiplicities match the per-instance // sends. MEMW_R sends IS_HALFWORD[timestamp_0 - old_timestamp_lo - 1]. PAGE does a // batched ARE_BYTES[init, fini] per row (skipped in continuation epochs, which the L2G // table owns). COMMIT sends AreBytes+IsHalfword; KECCAK_RND sends XOR/AND/ARE_BYTES/HWSL. - // Every source's bitwise lookups are collected by a pure `collect_*` function. We never - // concatenate them into one giant `Vec` (~140 M ops / ~560 MB at 10-tx - // whose only consumer is the multiplicity count). Instead each collector's transient - // per-source Vec is folded into a `BitwiseHistogram` and dropped, and the per-worker - // histograms are tree-reduced. The histogram is a commutative monoid, so the summed - // multiplicities are independent of accumulation order (the lookups ride a - // permutation-invariant bus). - - // Shared collector list: each is a pure `Fn() -> Vec` of its source. - // Order does not affect the histogram (commutative). - type Collector<'a> = Box Vec + Sync + 'a>; + // We never concatenate the lookups into one giant `Vec` (~140 M ops / + // ~560 MB at 10-tx whose only consumer is the multiplicity count). Each collector bumps + // the `BitwiseHistogram` it is handed: the heavy sources (MEMW_R one-per-row, PAGE + // one-per-byte, padding) count directly with no per-source Vec at all, and the small + // sources fold their transient `collect_*` Vec in and drop it. The histogram is a + // commutative monoid, so per-worker histograms tree-reduce to multiplicities that are + // independent of accumulation order. + type Collector<'a> = Box; let mul_chunk = max_rows.mul; let dvrm_chunk = max_rows.dvrm; let mut collectors: Vec = vec![ - Box::new(|| collect_bitwise_from_lt(<_ops)), - Box::new(|| collect_bitwise_from_mul(&mul_ops, mul_chunk)), - Box::new(|| collect_bitwise_from_dvrm(&dvrm_ops, dvrm_chunk)), - Box::new(|| collect_bitwise_from_branch(&branch_ops)), - Box::new(|| shift::collect_bitwise_from_shift(&shift_ops)), - Box::new(|| { - bytewise_ops - .iter() - .flat_map(|op| op.collect_bitwise_ops()) - .collect() + Box::new(|h| h.add_ops(&collect_bitwise_from_lt(<_ops))), + Box::new(|h| h.add_ops(&collect_bitwise_from_mul(&mul_ops, mul_chunk))), + Box::new(|h| h.add_ops(&collect_bitwise_from_dvrm(&dvrm_ops, dvrm_chunk))), + Box::new(|h| h.add_ops(&collect_bitwise_from_branch(&branch_ops))), + Box::new(|h| h.add_ops(&shift::collect_bitwise_from_shift(&shift_ops))), + Box::new(|h| { + for op in &bytewise_ops { + h.add_ops(&op.collect_bitwise_ops()); + } }), - Box::new(|| { - eq_ops - .iter() - .flat_map(|op| op.collect_bitwise_ops()) - .collect() + Box::new(|h| { + for op in &eq_ops { + h.add_ops(&op.collect_bitwise_ops()); + } }), - Box::new(|| { - store_ops - .iter() - .flat_map(|op| op.collect_bitwise_ops()) - .collect() + Box::new(|h| { + for op in &store_ops { + h.add_ops(&op.collect_bitwise_ops()); + } }), - Box::new(|| collect_bitwise_from_memw_aligned(&memw_aligned_ops)), - Box::new(|| memw_register::collect_bitwise_from_memw_register(&memw_register_rows)), - Box::new(|| collect_bitwise_from_commit(&commit_ops)), - Box::new(|| collect_bitwise_from_keccak(&keccak_ops)), - Box::new(|| collect_bitwise_from_ecsm(&ecsm_ops)), - Box::new(|| collect_bitwise_from_ecdas(&ecdas_ops)), - Box::new(|| collect_byte_check_ops_for_padding(num_padding_rows)), + Box::new(|h| h.add_ops(&collect_bitwise_from_memw_aligned(&memw_aligned_ops))), + Box::new(|h| memw_register::collect_bitwise_from_memw_register(&memw_register_rows, h)), + Box::new(|h| h.add_ops(&collect_bitwise_from_commit(&commit_ops))), + Box::new(|h| h.add_ops(&collect_bitwise_from_keccak(&keccak_ops))), + Box::new(|h| h.add_ops(&collect_bitwise_from_ecsm(&ecsm_ops))), + Box::new(|h| h.add_ops(&collect_bitwise_from_ecdas(&ecdas_ops))), + Box::new(|h| add_padding_byte_checks(h, num_padding_rows)), ]; if let Some(image) = initial_image && !l2g_memory_bookend { - collectors.push(Box::new(move || { - collect_bitwise_from_page(image, memory_state, l2g_memory_bookend) + collectors.push(Box::new(move |h| { + collect_bitwise_from_page(image, memory_state, l2g_memory_bookend, h) })); } @@ -3117,35 +3100,39 @@ fn build_traces( #[cfg(feature = "parallel")] { use rayon::prelude::*; - // Each collector produces a transient Vec of lookups that is folded into a - // dense 80 MiB histogram and dropped. To keep peak memory from scaling with - // core count, cap the number of concurrent histograms: split the collectors - // into at most `max_par` chunks (one histogram each), run the chunks in - // parallel, and process the collectors within a chunk sequentially (so at most - // one transient Vec is live per chunk). Tree-reduce the chunk histograms — - // add_ops/merge form a commutative monoid, so the result is order-independent - // and byte-identical to the sequential path. + // To keep peak memory from scaling with core count, cap the number of + // concurrent histograms: split the collectors into at most `max_par` + // chunks (one histogram each), run the chunks in parallel, and process + // the collectors within a chunk sequentially (so at most one transient + // Vec is live per chunk — the heavy collectors bump their chunk's + // histogram directly and have none). `reduce_with` pairs the chunk + // histograms directly; unlike `reduce(identity, ..)` it allocates NO + // per-leaf identity histograms (each a zeroed 80 MiB). Tree-reduce is + // valid because bump/add_ops/merge form a commutative monoid, so the + // result is order-independent and byte-identical to the sequential path. let max_par = rayon::current_num_threads().clamp(1, 8); let chunk_size = collectors.len().div_ceil(max_par).max(1); - let reduced = collectors + if let Some(reduced) = collectors .par_chunks(chunk_size) .map(|chunk| { let mut h = bitwise::BitwiseHistogram::new(); for f in chunk { - h.add_ops(&f()); + f(&mut h); } h }) - .reduce(bitwise::BitwiseHistogram::new, |mut a, b| { + .reduce_with(|mut a, b| { a.merge(&b); a - }); - base.merge(&reduced); + }) + { + base.merge(&reduced); + } } #[cfg(not(feature = "parallel"))] { for f in &collectors { - base.add_ops(&f()); + f(&mut base); } } let bitwise_histogram = base; From ee3fd85e476d0a47c3fe8da5470f37b0be9afb1c Mon Sep 17 00:00:00 2001 From: MauroFab Date: Tue, 7 Jul 2026 13:45:39 -0300 Subject: [PATCH 3/5] Rewrite trace-gen comments to stand alone for a fresh reader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bitwise-histogram comments were phrased against the pre-refactor code ("byte-identical to the previous serial .extend() chain", "instead of an O(n) op vector") — narration that is meaningless once this merges and the old path is gone. Restate them as standalone rationale: the multiplicities are order-independent (permutation-invariant bus) so parallel collection is sound, and the reduce_with note now explains why to prefer it over reduce(identity) rather than referencing a removed path. Also fix the stale add_padding_byte_checks doc (it described the pre-shrink 14-op CPU layout). --- prover/src/tables/trace_builder.rs | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/prover/src/tables/trace_builder.rs b/prover/src/tables/trace_builder.rs index 5eb9cfd0f..c77982489 100644 --- a/prover/src/tables/trace_builder.rs +++ b/prover/src/tables/trace_builder.rs @@ -1981,14 +1981,12 @@ fn collect_bitwise_from_branch(branch_ops: &[BranchOperation]) -> Vec( .map(|chunk| chunk.len().next_power_of_two().max(4) - chunk.len()) .sum(); - // The per-source bitwise collectors are all pure functions of their inputs and the + // The per-source bitwise collectors are all pure functions of their inputs, and the // BITWISE multiplicities are order-independent (they ride a permutation-invariant bus), - // so we collect every source in parallel. The result is byte-identical to the previous - // serial `.extend()` chain regardless of order. + // so every source can be collected in parallel and the per-worker histograms summed in + // any order. // // MUL/DVRM dedup their per-unique bit-gated lookups PER CHIP INSTANCE, so pass the same // chunk size used to split them into instances so multiplicities match the per-instance @@ -3106,10 +3104,11 @@ fn build_traces( // the collectors within a chunk sequentially (so at most one transient // Vec is live per chunk — the heavy collectors bump their chunk's // histogram directly and have none). `reduce_with` pairs the chunk - // histograms directly; unlike `reduce(identity, ..)` it allocates NO - // per-leaf identity histograms (each a zeroed 80 MiB). Tree-reduce is - // valid because bump/add_ops/merge form a commutative monoid, so the - // result is order-independent and byte-identical to the sequential path. + // histograms directly; `reduce(identity, ..)` would instead allocate a + // zeroed 80 MiB identity histogram per leaf, so prefer `reduce_with`. + // Tree-reduce is valid because bump/add_ops/merge form a commutative + // monoid, so the sum is independent of chunk order and matches the + // non-`parallel` fallback below. let max_par = rayon::current_num_threads().clamp(1, 8); let chunk_size = collectors.len().div_ceil(max_par).max(1); if let Some(reduced) = collectors From ef13fd1c5fe696ab867f450d6f47e0ce86dae249 Mon Sep 17 00:00:00 2001 From: MauroFab Date: Tue, 7 Jul 2026 13:54:59 -0300 Subject: [PATCH 4/5] Clean up stale trace-gen comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Repo-wide sweep of the trace-generation code for comments that narrate a change the next reader can't see, or that contradict the code: - MemwBuckets doc and collect_all_ops no longer reference "the old two-stage partition" / "byte-identical to partition-ing one combined vec" — that partition sweep no longer exists anywhere (grep '.partition(' → 0 hits). Restated as the current invariant (register-first-then-aligned, deterministic insertion order the multiplicity counts rely on). - Fix the MEMW_R ADDRESS column doc: register index range is 0-255 (x0-x31 plus the x254 commit index and x255 PC), not 0-31. --- prover/src/tables/memw_register.rs | 4 ++-- prover/src/tables/trace_builder.rs | 17 +++++++++-------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/prover/src/tables/memw_register.rs b/prover/src/tables/memw_register.rs index 7e5ff7c02..e29c29287 100644 --- a/prover/src/tables/memw_register.rs +++ b/prover/src/tables/memw_register.rs @@ -17,7 +17,7 @@ //! //! ## Column layout (10 columns) //! -//! - `ADDRESS`: Byte (register index 0-31) +//! - `ADDRESS`: Byte (register index 0-255: x0-x31, plus x254/x255) //! - `TIMESTAMP_0`: Word (low 32 bits) //! - `TIMESTAMP_1`: Word (high 32 bits) //! - `VAL_0`: Word (low 32 bits of register value) @@ -53,7 +53,7 @@ use crate::constraints::templates::emit_is_bit; // ========================================================================= pub mod cols { - /// Register index (0-31). CPU sends base_address = 2*reg_index. + /// Register index (0-255: x0-x31, plus x254/x255). CPU sends base_address = 2*reg_index. pub const ADDRESS: usize = 0; /// Timestamp low 32 bits diff --git a/prover/src/tables/trace_builder.rs b/prover/src/tables/trace_builder.rs index c77982489..2e7bd4925 100644 --- a/prover/src/tables/trace_builder.rs +++ b/prover/src/tables/trace_builder.rs @@ -389,16 +389,17 @@ fn classify_memw(op: &MemwOperation) -> MemwRoute { /// Routes each `MemwOperation` into its destination table bucket at CREATION time /// (register fast-path / aligned / general), so the walk fills the three buckets directly -/// and no separate partition pass is needed downstream. Classification order matches the old -/// two-stage partition (register first, then aligned), and push order within each bucket is -/// preserved, so the buckets are byte-identical to `partition`-ing one combined vec. +/// and no separate routing pass is needed downstream. Classification order is register +/// first, then aligned (see [`classify_memw`]), and push order within each bucket is the +/// walk's insertion order — the buckets are fully deterministic, which the per-cell +/// multiplicity counts rely on. /// /// ## Direct-to-column register fill /// /// For the register fast path we do NOT materialize a `Vec`. Ops that route /// to MEMW_R are stored as compact [`RegRow`]s (`register_rows`) and later filled directly /// into the MEMW_R columns. The `aligned` / `general` buckets hold `MemwOperation`s — an op -/// that FAILS `is_register_op` is routed there exactly as the old two-stage partition did. +/// that FAILS `is_register_op` is routed there (aligned if `is_aligned_op`, else general). #[derive(Default)] struct MemwBuckets { /// Compact register rows (filled directly into the MEMW_R columns). @@ -2827,14 +2828,14 @@ fn collect_all_ops( // register state (no zeroizing) so it can seed the next epoch. if is_final { // Route halt ops through the same classifier; they append to the end of their - // buckets, matching the old "append then partition" order. + // buckets. memw.extend_ops(collect_halt_ops(register_state)); } // The walk (`collect_ops_from_cpu`) already routed every MemwOperation into its bucket at - // creation via `MemwBuckets`, so there is no separate partition pass here — the old - // two-`partition` sweep (which moved millions of structs a second time, a bandwidth-bound - // cost) is gone. Order within each bucket matches the old stable partitions → byte-identical. + // creation via `MemwBuckets`, so there is no separate routing pass here: the ops are not + // moved a second time. Order within each bucket is the walk's insertion order, which the + // multiplicity counts depend on being deterministic. let MemwBuckets { register_rows: memw_register_rows, aligned: memw_aligned_ops, From 4799da31824e8338252ef5c88547be8a13650683 Mon Sep 17 00:00:00 2001 From: MauroFab Date: Tue, 7 Jul 2026 14:08:52 -0300 Subject: [PATCH 5/5] Fix stale CPU M7 reference in store MEMW comment The Bus-14 comment in collect_store_op_from_cpu said the packed value "must match CPU M7 which sends full rv2 as [lo32, hi32]", but M7 (the CPU's old inline store MEMW at timestamp+1) was removed in the ALU-bus migration. The store value now flows CPU -> MEMORY/MEMOP (rv2 as [lo32, hi32]) -> STORE chip -> MEMW write. Reword to describe that path; the byte-layout requirement it gestured at is unchanged. --- prover/src/tables/trace_builder.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/prover/src/tables/trace_builder.rs b/prover/src/tables/trace_builder.rs index 2e7bd4925..2d6e426e8 100644 --- a/prover/src/tables/trace_builder.rs +++ b/prover/src/tables/trace_builder.rs @@ -780,8 +780,10 @@ fn collect_store_op_from_cpu(op: &CpuOperation, memory_state: &mut MemoryState) let (old_values, old_timestamps) = memory_state.read_bytes(base_address, 8); // Pack ALL 8 bytes of store_value into value_bytes. - // Bus 14: MEMW Memory Write receiver reconstructs lo32/hi32 via linear combination - // of all 8 bytes. Must match CPU M7 which sends full rv2 as [lo32, hi32]. + // Bus 14: the MEMW Memory Write receiver reconstructs lo32/hi32 via a linear + // combination of all 8 bytes, so it must match the store value the CPU sends + // as [lo32, hi32] on the MEMORY bus (MEMOP) and that this STORE chip forwards + // as the MEMW write (the CPU no longer emits an inline store MEMW — see below). // Bus 16: only positions 0..byte_count participate (controlled by w2/w4/write8 // multiplicities), so extra bytes don't affect memory consistency. let mut value_bytes = [0u32; 8];