From 006c02daf53cae76a0c090f62c2fcedff573f253 Mon Sep 17 00:00:00 2001 From: MauroFab Date: Mon, 13 Jul 2026 18:04:51 -0300 Subject: [PATCH 1/3] feat(cli): report keccak/ecsm accelerator call counts under `execute --cycles` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cycle counter only counted retired instructions, so a keccak or ecsm accelerator call — one ecall that runs a whole permutation / scalar-mul in a single cycle — was invisible. `execute --cycles` now also prints `Keccak calls` and `Ecsm calls`, classified exactly as the prover's CpuOperation::from_log does (a7 == KECCAK/ECSM syscall number on an EcallEbreak), tallied in the memory-safe streaming loop. --- bin/cli/src/main.rs | 106 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 103 insertions(+), 3 deletions(-) diff --git a/bin/cli/src/main.rs b/bin/cli/src/main.rs index b430160fc..44908af0c 100644 --- a/bin/cli/src/main.rs +++ b/bin/cli/src/main.rs @@ -10,6 +10,8 @@ use clap::{Parser, Subcommand, ValueHint}; #[global_allocator] static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc; +use executor::vm::instruction::decoding::Instruction; +use executor::vm::instruction::execution::{ECSM_SYSCALL_NUMBER, KECCAK_SYSCALL_NUMBER}; use executor::{elf::Elf, flamegraph::FlamegraphGenerator, vm::execution::Executor}; use prover::VmProof; use stark::proof::options::GoldilocksCubicProofOptions; @@ -339,6 +341,37 @@ struct FlamegraphCliOptions { checkpoint_cycles: Option, } +/// The VM's two syscall accelerators. Each accelerator call is a single ECALL +/// instruction (one cycle) with the whole permutation / scalar-mul running +/// inside it, so invocations must be tallied separately from the cycle count. +#[derive(Clone, Copy)] +enum Accelerator { + /// keccak-f[1600] permutation. + Keccak, + /// secp256k1 scalar multiplication. + Ecsm, +} + +/// Classifies one executed instruction as an accelerator syscall invocation. +/// +/// Mirrors `CpuOperation::from_log` (prover/src/tables/cpu.rs): the prover sets +/// `ecall_keccak`/`ecall_ecsm` from `f.ecall && log.src1_val == `. +/// Here `f.ecall` is the instruction at the log's `current_pc` being +/// `EcallEbreak`, and `src1_val` carries a7 (the syscall number) on ECALL logs. +/// Keep this in sync with that classifier so the CLI's counts equal the prover's +/// chip-trigger counts by construction. (`get_private_input` is a memory-mapped +/// read, not a syscall, so it never reaches this path.) +fn accelerator_of(instruction: Option<&Instruction>, src1_val: u64) -> Option { + if !matches!(instruction, Some(Instruction::EcallEbreak)) { + return None; + } + match src1_val { + KECCAK_SYSCALL_NUMBER => Some(Accelerator::Keccak), + ECSM_SYSCALL_NUMBER => Some(Accelerator::Ecsm), + _ => None, + } +} + fn cmd_execute( elf_path: PathBuf, private_input_path: Option, @@ -370,6 +403,12 @@ fn cmd_execute( } }; + // Accelerator invocation counts, tallied only in the plain streaming path + // below (the flamegraph path drives execution inside the executor and does + // not expose per-log data). `None` means "not counted", so the accel lines + // are omitted rather than printed as misleading zeros. + let mut accel_counts: Option<(u64, u64)> = None; + let cycle_count = if let Some(ref output_path) = flamegraph.path { // Shared execute+flamegraph path (executor::flamegraph) instead of // hand-rolling the SymbolTable/Executor/drive-loop wiring here. @@ -434,6 +473,14 @@ fn cmd_execute( }; let mut cycle_count: u64 = 0; + let mut keccak_calls: u64 = 0; + let mut ecsm_calls: u64 = 0; + // Reused per chunk: `(current_pc, a7)` for logs whose a7 matches an + // accelerator syscall number. This is a cheap superset — a non-ECALL + // instruction can hold the same value in src1 — that `accelerator_of` + // confirms below, once the chunk's `&Log` borrow (tied to the executor's + // `&mut`) is released so the instruction cache can be read again. + let mut accel_candidates: Vec<(u64, u64)> = Vec::new(); loop { let logs = match executor.resume_budgeted(cycle_count, cycle_budget) { Ok(logs) => logs, @@ -442,9 +489,24 @@ fn cmd_execute( return ExitCode::FAILURE; } }; - match logs { - Some(logs) => cycle_count += logs.len() as u64, - None => break, + let Some(logs) = logs else { break }; + cycle_count += logs.len() as u64; + if cycles { + for log in logs { + if log.src1_val == KECCAK_SYSCALL_NUMBER || log.src1_val == ECSM_SYSCALL_NUMBER + { + accel_candidates.push((log.current_pc, log.src1_val)); + } + } + } + // `logs` is no longer used, so the executor's `&mut` borrow is free + // and the instruction cache can be read to confirm each candidate. + for (pc, a7) in accel_candidates.drain(..) { + match accelerator_of(executor.instructions.get(pc), a7) { + Some(Accelerator::Keccak) => keccak_calls += 1, + Some(Accelerator::Ecsm) => ecsm_calls += 1, + None => {} + } } if cycle_budget.is_some_and(|budget| cycle_count >= budget) { break; @@ -456,11 +518,18 @@ fn cmd_execute( return ExitCode::FAILURE; } + if cycles { + accel_counts = Some((keccak_calls, ecsm_calls)); + } cycle_count }; if cycles { println!("Cycles: {}", cycle_count); + if let Some((keccak_calls, ecsm_calls)) = accel_counts { + println!("Keccak calls: {}", keccak_calls); + println!("Ecsm calls: {}", ecsm_calls); + } } ExitCode::SUCCESS @@ -1011,4 +1080,35 @@ mod tests { fn continuation_epoch_size_uses_exact_power_of_two() { assert_eq!(continuation_epoch_size(20).unwrap(), 1 << 20); } + + // `accelerator_of` must match the prover's `CpuOperation::from_log`: count an + // invocation only when the instruction is an ECALL AND a7 is the accelerator + // syscall number. Covers both accelerators, the non-accelerator syscalls, a + // non-ECALL whose src1 collides with an accelerator number, and a cache miss. + #[test] + fn accelerator_of_mirrors_prover_classification() { + use executor::vm::instruction::execution::SyscallNumbers; + + let ecall = Instruction::EcallEbreak; + + assert!(matches!( + accelerator_of(Some(&ecall), KECCAK_SYSCALL_NUMBER), + Some(Accelerator::Keccak) + )); + assert!(matches!( + accelerator_of(Some(&ecall), ECSM_SYSCALL_NUMBER), + Some(Accelerator::Ecsm) + )); + + // Non-accelerator syscalls (Commit=64, Halt=93) count as neither. + assert!(accelerator_of(Some(&ecall), SyscallNumbers::Commit as u64).is_none()); + assert!(accelerator_of(Some(&ecall), SyscallNumbers::Halt as u64).is_none()); + + // A non-ECALL instruction whose src1 happens to equal an accelerator a7 + // must not count — this is the `f.ecall &&` guard the prover applies. + assert!(accelerator_of(Some(&Instruction::Fence), KECCAK_SYSCALL_NUMBER).is_none()); + + // No decoded instruction at the pc (cache miss) counts as neither. + assert!(accelerator_of(None, KECCAK_SYSCALL_NUMBER).is_none()); + } } From c2a4418a2d585e6e00a8a751cfb7620bfca08c70 Mon Sep 17 00:00:00 2001 From: MauroFab Date: Mon, 13 Jul 2026 18:35:41 -0300 Subject: [PATCH 2/3] refactor(cli): single canonical accelerator classifier + parity/e2e tests The keccak/ecsm classification behind `execute --cycles` was duplicated in three non-exhaustive spots bound only by a comment (prover `from_log`, the CLI's `accelerator_of`, and the CLI's streaming prefilter). A future third accelerator, or an edit to one CLI site but not the other, would silently miscount. - executor: add `enum Accelerator` and `SyscallNumbers::accelerator()`, an exhaustive `match self` so adding a syscall variant is a compile error here. This is now the single source of truth. - cli: delete the CLI's own `Accelerator` enum; route both `accelerator_of` and the streaming prefilter through `SyscallNumbers::accelerator()`, dropping the duplicated syscall-number constants. Extract the streaming count loop into `count_cycles_and_accelerators` so a test can drive the exact CLI path. - prover: add a parity test locking `from_log`'s `ecall_keccak`/`ecall_ecsm` bools to the canonical classifier (the prover's trace-building logic is unchanged; the test fails if it ever diverges). - cli: add an ignored e2e test asserting the keccak tally for test_keccak (1) and test_keccak_multi (3) through the real count loop. - docs: note the `Keccak calls` / `Ecsm calls` output (and the --flamegraph omission) in the README and `--cycles` clap help. Output and behavior are unchanged. --- bin/cli/README.md | 2 +- bin/cli/src/main.rs | 186 ++++++++++++++--------- executor/src/vm/instruction/execution.rs | 23 +++ prover/src/tables/cpu.rs | 41 +++++ 4 files changed, 182 insertions(+), 70 deletions(-) diff --git a/bin/cli/README.md b/bin/cli/README.md index 267fc61b7..5ef3cf40d 100644 --- a/bin/cli/README.md +++ b/bin/cli/README.md @@ -41,7 +41,7 @@ cargo run -p cli --release -- execute [--private-input ] [-- |---|---| | `--private-input ` | Pass private input bytes to the guest (read via `get_private_input()`). | | `--flamegraph ` | Generate folded-stack flamegraph output. See [Guest Program Flamegraphs](#guest-program-flamegraphs). | -| `--cycles` | Count instructions during execution and print the dynamic instruction count. | +| `--cycles` | Count instructions during execution and print the dynamic instruction count. Also reports `Keccak calls` / `Ecsm calls` (accelerator syscall invocations). Combined with `--flamegraph`, the accelerator lines are omitted (the flamegraph path exposes no per-log data). | ### Prove diff --git a/bin/cli/src/main.rs b/bin/cli/src/main.rs index 44908af0c..6438b40b0 100644 --- a/bin/cli/src/main.rs +++ b/bin/cli/src/main.rs @@ -11,7 +11,7 @@ use clap::{Parser, Subcommand, ValueHint}; #[global_allocator] static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc; use executor::vm::instruction::decoding::Instruction; -use executor::vm::instruction::execution::{ECSM_SYSCALL_NUMBER, KECCAK_SYSCALL_NUMBER}; +use executor::vm::instruction::execution::{Accelerator, SyscallNumbers}; use executor::{elf::Elf, flamegraph::FlamegraphGenerator, vm::execution::Executor}; use prover::VmProof; use stark::proof::options::GoldilocksCubicProofOptions; @@ -126,7 +126,10 @@ enum Commands { #[arg(long)] cycle_budget: Option, - /// Print the dynamic instruction (cycle) count + /// Print the dynamic instruction (cycle) count, plus `Keccak calls` / + /// `Ecsm calls` (accelerator syscall invocations). The accelerator lines + /// are omitted when combined with --flamegraph (that path has no per-log + /// data). #[arg(long)] cycles: bool, }, @@ -341,35 +344,78 @@ struct FlamegraphCliOptions { checkpoint_cycles: Option, } -/// The VM's two syscall accelerators. Each accelerator call is a single ECALL -/// instruction (one cycle) with the whole permutation / scalar-mul running -/// inside it, so invocations must be tallied separately from the cycle count. -#[derive(Clone, Copy)] -enum Accelerator { - /// keccak-f[1600] permutation. - Keccak, - /// secp256k1 scalar multiplication. - Ecsm, -} - /// Classifies one executed instruction as an accelerator syscall invocation. /// -/// Mirrors `CpuOperation::from_log` (prover/src/tables/cpu.rs): the prover sets -/// `ecall_keccak`/`ecall_ecsm` from `f.ecall && log.src1_val == `. -/// Here `f.ecall` is the instruction at the log's `current_pc` being -/// `EcallEbreak`, and `src1_val` carries a7 (the syscall number) on ECALL logs. -/// Keep this in sync with that classifier so the CLI's counts equal the prover's -/// chip-trigger counts by construction. (`get_private_input` is a memory-mapped -/// read, not a syscall, so it never reaches this path.) +/// Delegates to the executor's canonical `SyscallNumbers::accelerator()` so the +/// CLI's counts equal the prover's chip-trigger counts by construction: the +/// prover sets `ecall_keccak`/`ecall_ecsm` from `f.ecall && log.src1_val == +/// `. Here `f.ecall` is the instruction at the log's +/// `current_pc` being `EcallEbreak`, and `src1_val` carries a7 (the syscall +/// number) on ECALL logs. (`get_private_input` is a memory-mapped read, not a +/// syscall, so it never reaches this path.) fn accelerator_of(instruction: Option<&Instruction>, src1_val: u64) -> Option { if !matches!(instruction, Some(Instruction::EcallEbreak)) { return None; } - match src1_val { - KECCAK_SYSCALL_NUMBER => Some(Accelerator::Keccak), - ECSM_SYSCALL_NUMBER => Some(Accelerator::Ecsm), - _ => None, + SyscallNumbers::try_from(src1_val) + .ok() + .and_then(|s| s.accelerator()) +} + +/// Streams the program to completion (or the cycle budget), returning +/// `(cycle_count, keccak_calls, ecsm_calls)`. +/// +/// This is the counting path behind `execute --cycles`. Keeping it in one +/// function lets a test drive the exact prefilter/confirm loop the CLI uses. +/// Each accelerator call is a single ECALL (one cycle) with the whole +/// permutation / scalar-mul running inside it, so invocations are tallied +/// separately from the cycle count. The tallies are collected only when +/// `count_accelerators` is set (the plain execute path skips the work). +/// +/// The loop first prefilters logs whose `src1_val` (a7 on ECALL logs) matches an +/// accelerator syscall number — a cheap superset, since a non-ECALL instruction +/// can hold the same value in src1 — then confirms each candidate against the +/// decoded instruction once the chunk's `&Log` borrow (tied to the executor's +/// `&mut`) is released so the instruction cache can be read again. +fn count_cycles_and_accelerators( + executor: &mut Executor, + cycle_budget: Option, + count_accelerators: bool, +) -> Result<(u64, u64, u64), String> { + let mut cycle_count: u64 = 0; + let mut keccak_calls: u64 = 0; + let mut ecsm_calls: u64 = 0; + let mut accel_candidates: Vec<(u64, u64)> = Vec::new(); + loop { + let logs = executor + .resume_budgeted(cycle_count, cycle_budget) + .map_err(|e| format!("Execution failed: {e:?}"))?; + let Some(logs) = logs else { break }; + cycle_count += logs.len() as u64; + if count_accelerators { + for log in logs { + if SyscallNumbers::try_from(log.src1_val) + .map(|s| s.accelerator().is_some()) + .unwrap_or(false) + { + accel_candidates.push((log.current_pc, log.src1_val)); + } + } + } + // `logs` is no longer used, so the executor's `&mut` borrow is free and + // the instruction cache can be read to confirm each candidate. + for (pc, a7) in accel_candidates.drain(..) { + match accelerator_of(executor.instructions.get(pc), a7) { + Some(Accelerator::Keccak) => keccak_calls += 1, + Some(Accelerator::Ecsm) => ecsm_calls += 1, + None => {} + } + } + if cycle_budget.is_some_and(|budget| cycle_count >= budget) { + break; + } } + Ok((cycle_count, keccak_calls, ecsm_calls)) } fn cmd_execute( @@ -472,46 +518,14 @@ fn cmd_execute( } }; - let mut cycle_count: u64 = 0; - let mut keccak_calls: u64 = 0; - let mut ecsm_calls: u64 = 0; - // Reused per chunk: `(current_pc, a7)` for logs whose a7 matches an - // accelerator syscall number. This is a cheap superset — a non-ECALL - // instruction can hold the same value in src1 — that `accelerator_of` - // confirms below, once the chunk's `&Log` borrow (tied to the executor's - // `&mut`) is released so the instruction cache can be read again. - let mut accel_candidates: Vec<(u64, u64)> = Vec::new(); - loop { - let logs = match executor.resume_budgeted(cycle_count, cycle_budget) { - Ok(logs) => logs, + let (cycle_count, keccak_calls, ecsm_calls) = + match count_cycles_and_accelerators(&mut executor, cycle_budget, cycles) { + Ok(counts) => counts, Err(e) => { - eprintln!("Execution failed: {:?}", e); + eprintln!("{e}"); return ExitCode::FAILURE; } }; - let Some(logs) = logs else { break }; - cycle_count += logs.len() as u64; - if cycles { - for log in logs { - if log.src1_val == KECCAK_SYSCALL_NUMBER || log.src1_val == ECSM_SYSCALL_NUMBER - { - accel_candidates.push((log.current_pc, log.src1_val)); - } - } - } - // `logs` is no longer used, so the executor's `&mut` borrow is free - // and the instruction cache can be read to confirm each candidate. - for (pc, a7) in accel_candidates.drain(..) { - match accelerator_of(executor.instructions.get(pc), a7) { - Some(Accelerator::Keccak) => keccak_calls += 1, - Some(Accelerator::Ecsm) => ecsm_calls += 1, - None => {} - } - } - if cycle_budget.is_some_and(|budget| cycle_count >= budget) { - break; - } - } if let Err(e) = executor.finish() { eprintln!("Failed to finish execution: {:?}", e); @@ -1087,28 +1101,62 @@ mod tests { // non-ECALL whose src1 collides with an accelerator number, and a cache miss. #[test] fn accelerator_of_mirrors_prover_classification() { - use executor::vm::instruction::execution::SyscallNumbers; + use executor::vm::instruction::execution::{ECSM_SYSCALL_NUMBER, KECCAK_SYSCALL_NUMBER}; let ecall = Instruction::EcallEbreak; - assert!(matches!( + assert_eq!( accelerator_of(Some(&ecall), KECCAK_SYSCALL_NUMBER), Some(Accelerator::Keccak) - )); - assert!(matches!( + ); + assert_eq!( accelerator_of(Some(&ecall), ECSM_SYSCALL_NUMBER), Some(Accelerator::Ecsm) - )); + ); // Non-accelerator syscalls (Commit=64, Halt=93) count as neither. - assert!(accelerator_of(Some(&ecall), SyscallNumbers::Commit as u64).is_none()); - assert!(accelerator_of(Some(&ecall), SyscallNumbers::Halt as u64).is_none()); + assert_eq!( + accelerator_of(Some(&ecall), SyscallNumbers::Commit as u64), + None + ); + assert_eq!( + accelerator_of(Some(&ecall), SyscallNumbers::Halt as u64), + None + ); // A non-ECALL instruction whose src1 happens to equal an accelerator a7 // must not count — this is the `f.ecall &&` guard the prover applies. - assert!(accelerator_of(Some(&Instruction::Fence), KECCAK_SYSCALL_NUMBER).is_none()); + assert_eq!( + accelerator_of(Some(&Instruction::Fence), KECCAK_SYSCALL_NUMBER), + None + ); // No decoded instruction at the pc (cache miss) counts as neither. - assert!(accelerator_of(None, KECCAK_SYSCALL_NUMBER).is_none()); + assert_eq!(accelerator_of(None, KECCAK_SYSCALL_NUMBER), None); + } + + // End-to-end: drive a known keccak program through the same streaming count + // loop `execute --cycles` uses (`count_cycles_and_accelerators`) and assert + // the keccak tally. `test_keccak.s` issues exactly one keccak-f[1600] ecall + // (plus commit + halt); `test_keccak_multi.s` issues three. Locks the + // prefilter/confirm loop, not just the pure classifier. Ignored by default + // because it needs a prebuilt ELF artifact. + #[test] + #[ignore = "needs prebuilt guest ELF (make compile-programs-asm)"] + fn cycles_counts_keccak_calls_end_to_end() { + for (name, expected_keccak) in [("test_keccak", 1u64), ("test_keccak_multi", 3u64)] { + let elf_path = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("../../executor/program_artifacts/asm") + .join(format!("{name}.elf")); + let elf_data = std::fs::read(&elf_path) + .unwrap_or_else(|e| panic!("read {}: {e}", elf_path.display())); + let program = Elf::load(&elf_data).unwrap(); + let mut executor = Executor::new(&program, Vec::new()).unwrap(); + let (_cycles, keccak_calls, ecsm_calls) = + count_cycles_and_accelerators(&mut executor, None, true).unwrap(); + executor.finish().unwrap(); + assert_eq!(keccak_calls, expected_keccak, "{name} keccak calls"); + assert_eq!(ecsm_calls, 0, "{name} ecsm calls"); + } } } diff --git a/executor/src/vm/instruction/execution.rs b/executor/src/vm/instruction/execution.rs index e6ad745e5..c92c0ab88 100644 --- a/executor/src/vm/instruction/execution.rs +++ b/executor/src/vm/instruction/execution.rs @@ -50,6 +50,29 @@ impl TryFrom for SyscallNumbers { } } +/// A syscall that drives a specialized in-circuit accelerator chip. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum Accelerator { + Keccak, + Ecsm, +} + +impl SyscallNumbers { + /// The accelerator this syscall drives, if any. Exhaustive `match self`: + /// adding a `SyscallNumbers` variant is a compile error here, so a new + /// accelerator can't be silently missed by counters that consume this. + pub fn accelerator(self) -> Option { + match self { + SyscallNumbers::KeccakPermute => Some(Accelerator::Keccak), + SyscallNumbers::Ecsm => Some(Accelerator::Ecsm), + SyscallNumbers::Print + | SyscallNumbers::Panic + | SyscallNumbers::Commit + | SyscallNumbers::Halt => None, + } + } +} + /// Reads a 256-bit little-endian value as four doublewords at `addr + 8i`. fn load_u256_le(memory: &Memory, addr: u64) -> Result<[u8; 32], MemoryError> { let mut out = [0u8; 32]; diff --git a/prover/src/tables/cpu.rs b/prover/src/tables/cpu.rs index 781bb02b0..6c2691cc3 100644 --- a/prover/src/tables/cpu.rs +++ b/prover/src/tables/cpu.rs @@ -1050,3 +1050,44 @@ fn memw_register_read( ], ) } + +#[cfg(test)] +mod tests { + use super::CpuOperation; + use executor::vm::instruction::decoding::Instruction; + use executor::vm::instruction::execution::{ + Accelerator, ECSM_SYSCALL_NUMBER, KECCAK_SYSCALL_NUMBER, SyscallNumbers, + }; + use executor::vm::logs::Log; + + /// The prover's `ecall_keccak`/`ecall_ecsm` classification in `from_log` must + /// stay identical to the executor's canonical `SyscallNumbers::accelerator()`. + /// Constructs the `from_log` inputs a real ECALL produces (a `Log` whose + /// `src1_val` is a7 = the syscall number, decoded as `Instruction::EcallEbreak` + /// so `f.ecall` is set) and checks both bools against the canonical classifier. + /// If the prover ever diverges from `accelerator()`, this fails. + #[test] + fn from_log_accelerator_flags_match_canonical_classifier() { + for n in [KECCAK_SYSCALL_NUMBER, ECSM_SYSCALL_NUMBER] { + let log = Log { + current_pc: 0, + next_pc: 4, + src1_val: n, + src2_val: 0, + dst_val: 0, + }; + let op = CpuOperation::from_log_and_instruction(&log, 0, Instruction::EcallEbreak); + let accel = SyscallNumbers::try_from(n).unwrap().accelerator(); + assert_eq!( + op.ecall_keccak, + matches!(accel, Some(Accelerator::Keccak)), + "ecall_keccak mismatch for syscall {n:#x}" + ); + assert_eq!( + op.ecall_ecsm, + matches!(accel, Some(Accelerator::Ecsm)), + "ecall_ecsm mismatch for syscall {n:#x}" + ); + } + } +} From 14d9380fcc88b1f2eb6581e3125130ac5b2ce93c Mon Sep 17 00:00:00 2001 From: MauroFab Date: Mon, 13 Jul 2026 18:41:17 -0300 Subject: [PATCH 3/3] refactor(cli): narrow to canonical classifier + docs (drop added tests) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scope-down of the previous commit to just the canonical classifier, the two CLI call sites, and the docs. - Remove the prover↔executor parity test in prover/src/tables/cpu.rs (reverts that file to unchanged; the prover's from_log classification is untouched). - Remove the ignored end-to-end keccak count test in the CLI. - Revert the `count_cycles_and_accelerators` extraction back to the inline streaming loop; the prefilter still routes through `SyscallNumbers::accelerator()`, so both CLI sites remain on the single canonical classifier. Output and behavior unchanged. --- bin/cli/src/main.rs | 123 ++++++++++++--------------------------- prover/src/tables/cpu.rs | 41 ------------- 2 files changed, 38 insertions(+), 126 deletions(-) diff --git a/bin/cli/src/main.rs b/bin/cli/src/main.rs index 6438b40b0..95c3050b7 100644 --- a/bin/cli/src/main.rs +++ b/bin/cli/src/main.rs @@ -362,62 +362,6 @@ fn accelerator_of(instruction: Option<&Instruction>, src1_val: u64) -> Option, - count_accelerators: bool, -) -> Result<(u64, u64, u64), String> { - let mut cycle_count: u64 = 0; - let mut keccak_calls: u64 = 0; - let mut ecsm_calls: u64 = 0; - let mut accel_candidates: Vec<(u64, u64)> = Vec::new(); - loop { - let logs = executor - .resume_budgeted(cycle_count, cycle_budget) - .map_err(|e| format!("Execution failed: {e:?}"))?; - let Some(logs) = logs else { break }; - cycle_count += logs.len() as u64; - if count_accelerators { - for log in logs { - if SyscallNumbers::try_from(log.src1_val) - .map(|s| s.accelerator().is_some()) - .unwrap_or(false) - { - accel_candidates.push((log.current_pc, log.src1_val)); - } - } - } - // `logs` is no longer used, so the executor's `&mut` borrow is free and - // the instruction cache can be read to confirm each candidate. - for (pc, a7) in accel_candidates.drain(..) { - match accelerator_of(executor.instructions.get(pc), a7) { - Some(Accelerator::Keccak) => keccak_calls += 1, - Some(Accelerator::Ecsm) => ecsm_calls += 1, - None => {} - } - } - if cycle_budget.is_some_and(|budget| cycle_count >= budget) { - break; - } - } - Ok((cycle_count, keccak_calls, ecsm_calls)) -} - fn cmd_execute( elf_path: PathBuf, private_input_path: Option, @@ -518,14 +462,48 @@ fn cmd_execute( } }; - let (cycle_count, keccak_calls, ecsm_calls) = - match count_cycles_and_accelerators(&mut executor, cycle_budget, cycles) { - Ok(counts) => counts, + let mut cycle_count: u64 = 0; + let mut keccak_calls: u64 = 0; + let mut ecsm_calls: u64 = 0; + // Reused per chunk: `(current_pc, a7)` for logs whose a7 matches an + // accelerator syscall number. This is a cheap superset — a non-ECALL + // instruction can hold the same value in src1 — that `accelerator_of` + // confirms below, once the chunk's `&Log` borrow (tied to the executor's + // `&mut`) is released so the instruction cache can be read again. + let mut accel_candidates: Vec<(u64, u64)> = Vec::new(); + loop { + let logs = match executor.resume_budgeted(cycle_count, cycle_budget) { + Ok(logs) => logs, Err(e) => { - eprintln!("{e}"); + eprintln!("Execution failed: {:?}", e); return ExitCode::FAILURE; } }; + let Some(logs) = logs else { break }; + cycle_count += logs.len() as u64; + if cycles { + for log in logs { + if SyscallNumbers::try_from(log.src1_val) + .map(|s| s.accelerator().is_some()) + .unwrap_or(false) + { + accel_candidates.push((log.current_pc, log.src1_val)); + } + } + } + // `logs` is no longer used, so the executor's `&mut` borrow is free + // and the instruction cache can be read to confirm each candidate. + for (pc, a7) in accel_candidates.drain(..) { + match accelerator_of(executor.instructions.get(pc), a7) { + Some(Accelerator::Keccak) => keccak_calls += 1, + Some(Accelerator::Ecsm) => ecsm_calls += 1, + None => {} + } + } + if cycle_budget.is_some_and(|budget| cycle_count >= budget) { + break; + } + } if let Err(e) = executor.finish() { eprintln!("Failed to finish execution: {:?}", e); @@ -1134,29 +1112,4 @@ mod tests { // No decoded instruction at the pc (cache miss) counts as neither. assert_eq!(accelerator_of(None, KECCAK_SYSCALL_NUMBER), None); } - - // End-to-end: drive a known keccak program through the same streaming count - // loop `execute --cycles` uses (`count_cycles_and_accelerators`) and assert - // the keccak tally. `test_keccak.s` issues exactly one keccak-f[1600] ecall - // (plus commit + halt); `test_keccak_multi.s` issues three. Locks the - // prefilter/confirm loop, not just the pure classifier. Ignored by default - // because it needs a prebuilt ELF artifact. - #[test] - #[ignore = "needs prebuilt guest ELF (make compile-programs-asm)"] - fn cycles_counts_keccak_calls_end_to_end() { - for (name, expected_keccak) in [("test_keccak", 1u64), ("test_keccak_multi", 3u64)] { - let elf_path = Path::new(env!("CARGO_MANIFEST_DIR")) - .join("../../executor/program_artifacts/asm") - .join(format!("{name}.elf")); - let elf_data = std::fs::read(&elf_path) - .unwrap_or_else(|e| panic!("read {}: {e}", elf_path.display())); - let program = Elf::load(&elf_data).unwrap(); - let mut executor = Executor::new(&program, Vec::new()).unwrap(); - let (_cycles, keccak_calls, ecsm_calls) = - count_cycles_and_accelerators(&mut executor, None, true).unwrap(); - executor.finish().unwrap(); - assert_eq!(keccak_calls, expected_keccak, "{name} keccak calls"); - assert_eq!(ecsm_calls, 0, "{name} ecsm calls"); - } - } } diff --git a/prover/src/tables/cpu.rs b/prover/src/tables/cpu.rs index 6c2691cc3..781bb02b0 100644 --- a/prover/src/tables/cpu.rs +++ b/prover/src/tables/cpu.rs @@ -1050,44 +1050,3 @@ fn memw_register_read( ], ) } - -#[cfg(test)] -mod tests { - use super::CpuOperation; - use executor::vm::instruction::decoding::Instruction; - use executor::vm::instruction::execution::{ - Accelerator, ECSM_SYSCALL_NUMBER, KECCAK_SYSCALL_NUMBER, SyscallNumbers, - }; - use executor::vm::logs::Log; - - /// The prover's `ecall_keccak`/`ecall_ecsm` classification in `from_log` must - /// stay identical to the executor's canonical `SyscallNumbers::accelerator()`. - /// Constructs the `from_log` inputs a real ECALL produces (a `Log` whose - /// `src1_val` is a7 = the syscall number, decoded as `Instruction::EcallEbreak` - /// so `f.ecall` is set) and checks both bools against the canonical classifier. - /// If the prover ever diverges from `accelerator()`, this fails. - #[test] - fn from_log_accelerator_flags_match_canonical_classifier() { - for n in [KECCAK_SYSCALL_NUMBER, ECSM_SYSCALL_NUMBER] { - let log = Log { - current_pc: 0, - next_pc: 4, - src1_val: n, - src2_val: 0, - dst_val: 0, - }; - let op = CpuOperation::from_log_and_instruction(&log, 0, Instruction::EcallEbreak); - let accel = SyscallNumbers::try_from(n).unwrap().accelerator(); - assert_eq!( - op.ecall_keccak, - matches!(accel, Some(Accelerator::Keccak)), - "ecall_keccak mismatch for syscall {n:#x}" - ); - assert_eq!( - op.ecall_ecsm, - matches!(accel, Some(Accelerator::Ecsm)), - "ecall_ecsm mismatch for syscall {n:#x}" - ); - } - } -}