Skip to content

Commit c166661

Browse files
committed
fix: NOP/marker-0 collision and step-bucketing latch in recursion profiling
decode_step_marker required only dst==0, matching the canonical NOP (addi x0, x0, 0) as marker 0; pin src==0 and imm!=0 per the documented addi x0, x0, N convention. run_profile latched the step bucket at the highest marker ever seen, so multi_verify's per-AIR-table 3,4,5,6 repetition folded every table after the first into bucket 6. Track the latest marker instead.
1 parent 56d6cc7 commit c166661

2 files changed

Lines changed: 17 additions & 15 deletions

File tree

‎executor/src/vm/execution.rs‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -303,17 +303,18 @@ impl InstructionCache {
303303
}
304304

305305
/// Decode a `stark::profile_markers::step_marker` hit at `pc`: the marker
306-
/// convention is `addi x0, x0, N` (an `ArithImm` with `dst == 0`, `op ==
307-
/// Add`), which real code never emits spontaneously since writes to `x0` are
308-
/// always discarded. Returns the marker's `N` if `pc` decodes to one.
306+
/// convention is `addi x0, x0, N` (an `ArithImm` with `dst == 0`, `src == 0`,
307+
/// `op == Add`, `N != 0`), which real code never emits spontaneously since
308+
/// writes to `x0` are always discarded and the canonical NOP is `addi x0, x0,
309+
/// 0`. Returns the marker's `N` if `pc` decodes to one.
309310
pub fn decode_step_marker(instructions: &InstructionCache, pc: u64) -> Option<u32> {
310311
match instructions.get(pc)? {
311312
Instruction::ArithImm {
312313
dst: 0,
314+
src: 0,
313315
op: crate::vm::instruction::decoding::ArithOp::Add,
314316
imm,
315-
..
316-
} => Some(*imm as u32),
317+
} if *imm != 0 => Some(*imm as u32),
317318
_ => None,
318319
}
319320
}

‎prover/src/tests/recursion_smoke_test.rs‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -196,9 +196,10 @@ fn resolve_pc(symbols: &executor::elf::SymbolTable, pc: u64) -> String {
196196
}
197197

198198
/// Verifier sub-steps in execution order, keyed by `stark::profile_markers::STEP_*`
199-
/// value. `run_profile` buckets cycles by the highest marker observed so far
200-
/// (`decode_step_marker` — a missing marker just means that bucket stays at 0
201-
/// cycles, no substring matching or symbol table needed).
199+
/// value. `run_profile` buckets cycles by the latest marker observed so far
200+
/// (`decode_step_marker`, defaulting to bucket 0 until the first marker fires),
201+
/// so `multi_verify`'s per-table `3,4,5,6` repetition re-attributes cycles to
202+
/// the correct step on each table's `6->3` transition instead of latching at 6.
202203
const STEP_LABELS: [&str; 7] = [
203204
"0. setup (alloc init + postcard decode)",
204205
"1. airs_and_bus_balance (Elf::load/VmAirs::new preprocessed FFT+Merkle/bus balance)",
@@ -265,8 +266,10 @@ fn print_function_table(
265266
) {
266267
let mut by_function: std::collections::HashMap<String, (u64, u64)> =
267268
std::collections::HashMap::new();
268-
let mut by_function_per_step: std::collections::HashMap<u8, std::collections::HashMap<String, (u64, u64)>> =
269-
std::collections::HashMap::new();
269+
let mut by_function_per_step: std::collections::HashMap<
270+
u8,
271+
std::collections::HashMap<String, (u64, u64)>,
272+
> = std::collections::HashMap::new();
270273
let mut unique_pcs: std::collections::HashSet<u64> = std::collections::HashSet::new();
271274
for ((pc, bucket), count) in &pc_hist {
272275
unique_pcs.insert(*pc);
@@ -315,10 +318,10 @@ fn print_function_table(
315318
}
316319
}
317320

318-
/// Print the monotonic per-verifier-step cycle bucketing (`buckets[0]` = setup).
321+
/// Print the per-verifier-step cycle bucketing (`buckets[0]` = setup).
319322
fn print_step_breakdown(buckets: &[u64; 7], total_cycles: u64) {
320323
eprintln!();
321-
eprintln!(" Per-step cycle breakdown (monotonic state machine):");
324+
eprintln!(" Per-step cycle breakdown (latest-marker state machine):");
322325
eprintln!(" {:<70} {:>14} {:>7}", "bucket", "cycles", "%");
323326
for (label, cycles) in STEP_LABELS.iter().zip(buckets.iter()) {
324327
let pct = if total_cycles > 0 {
@@ -365,9 +368,7 @@ fn run_profile(
365368
|log| {
366369
let pc = log.current_pc;
367370

368-
if let Some(marker) = executor::vm::execution::decode_step_marker(&instructions, pc)
369-
&& bucket.get() < marker as u8
370-
{
371+
if let Some(marker) = executor::vm::execution::decode_step_marker(&instructions, pc) {
371372
bucket.set(marker as u8);
372373
}
373374
buckets[bucket.get() as usize] += 1;

0 commit comments

Comments
 (0)