Skip to content

Feat/fext accelerator#818

Open
jotabulacios wants to merge 40 commits into
mainfrom
feat/fext-accelerator
Open

Feat/fext accelerator#818
jotabulacios wants to merge 40 commits into
mainfrom
feat/fext-accelerator

Conversation

@jotabulacios

@jotabulacios jotabulacios commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Motivation

Recursion — the STARK verifier running as an in-VM guest — spends most of its
arithmetic in the VM's native degree-3 Goldilocks extension field
Fp3 = Fp[x]/(x³ − 2) (DEEP composition, FRI folding, constraint evaluation). As
plain RISC-V, one Fp3 multiply-add expands to a dozen-plus field mul/adds plus
the surrounding load/store traffic, all charged to the CPU, MEMW and LT tables.

This PR adds the FEXT precompile (spec PR #809, spec/fext): a·b + c over Fp3
in a single ecall, backed by a small degree-2 AIR — the Fp3 analog of the existing
KECCAK/ECSM accelerators

@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown

Benchmark Results for modified programs 🚀

Command Mean [ms] Min [ms] Max [ms] Relative
head ecsm 3.3 ± 0.0 3.3 3.4 1.00
Command Mean [ms] Min [ms] Max [ms] Relative
head hashmap 135.7 ± 5.3 129.0 144.5 1.00
Command Mean [ms] Min [ms] Max [ms] Relative
head keccak 130.8 ± 3.4 125.1 136.8 1.00
Command Mean [ms] Min [ms] Max [ms] Relative
head syscall_commit 97.8 ± 1.2 95.5 99.2 1.00

@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown

Benchmark Results for unmodified programs 🚀

Command Mean [ms] Min [ms] Max [ms] Relative
base binary_search 59.6 ± 0.7 58.7 60.6 1.00
head binary_search 63.4 ± 1.2 62.0 66.3 1.06 ± 0.02
Command Mean [ms] Min [ms] Max [ms] Relative
base bitwise_ops 58.5 ± 0.2 58.2 58.9 1.00
head bitwise_ops 62.2 ± 1.1 61.1 64.0 1.06 ± 0.02
Command Mean [ms] Min [ms] Max [ms] Relative
base fibonacci_26 63.4 ± 0.7 62.7 64.7 1.00
head fibonacci_26 68.0 ± 0.9 66.9 69.6 1.07 ± 0.02
Command Mean [ms] Min [ms] Max [ms] Relative
base matrix_multiply 65.4 ± 0.6 64.8 67.0 1.00
head matrix_multiply 69.3 ± 1.4 67.2 71.7 1.06 ± 0.02
Command Mean [ms] Min [ms] Max [ms] Relative
base modular_exp 60.4 ± 1.5 58.6 63.1 1.00
head modular_exp 62.5 ± 0.7 61.4 63.9 1.04 ± 0.03
Command Mean [ms] Min [ms] Max [ms] Relative
base quicksort 63.5 ± 0.5 62.6 64.0 1.00
head quicksort 67.7 ± 0.5 66.8 68.5 1.07 ± 0.01
Command Mean [ms] Min [ms] Max [ms] Relative
base sieve 65.4 ± 0.6 64.7 66.2 1.00
head sieve 69.5 ± 0.5 69.1 70.8 1.06 ± 0.01
Command Mean [ms] Min [ms] Max [ms] Relative
base sum_array 74.1 ± 1.1 73.3 76.8 1.00
head sum_array 78.3 ± 0.6 77.6 79.2 1.06 ± 0.02

@jotabulacios
jotabulacios marked this pull request as ready for review July 16, 2026 19:00
@jotabulacios
jotabulacios force-pushed the feat/fext-accelerator branch from 2f8ac34 to 3bf6794 Compare July 17, 2026 12:20
@jotabulacios

Copy link
Copy Markdown
Collaborator Author

/ai-review

@github-actions

Copy link
Copy Markdown

Codex Code Review

  • Low — prover/src/lib.rs:447: Error::FextInContinuation is now unreachable because this PR adds continuation support and removes all constructors. Remove the obsolete variant and its Display arm.

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown

AI Review

PR #818 · 49 changed files

Warning: the diff was truncated before review.

Findings

Status Sev Location Finding Found by
confirmed medium prover/src/auto_storage.rs:107 auto_storage peak-RAM estimate omits the four FEXT tables kimi
openrouter/moonshotai/kimi-k2.7-code
confirmed low executor/programs/rust/fext_baseline/src/main.rs fext_baseline Rust program is dead code added by the PR moonmath
zro/minimax-m3

Status column reflects the verdict from the verifier: deepseek-verifier (openrouter/deepseek/deepseek-v4-pro).

AI-002: auto_storage peak-RAM estimate omits the four FEXT tables
  • Status: confirmed
  • Severity: medium
  • Location: prover/src/auto_storage.rs:107
  • Found by: kimi:openrouter/moonshotai/kimi-k2.7-code
  • Verified by: deepseek-verifier:openrouter/deepseek/deepseek-v4-pro
  • Rejected by: -

Claim

table_specs builds the per-table memory estimate used by peak_bytes/decide, but it has no entries for fext_load, fext_fma, fext_store or fext_page. Even if count_table_lengths were updated, these fixed-size tables would still be ignored, so disk-spill decisions for FEXT-heavy proofs will underestimate peak RAM.

Evidence

The loop in count_table_lengths (lines 4133-4216) counts ECALL Commit and Keccak/ECSM, but never checks cpu_op.ecall_fext_load/fext_fma/fext_store. It also never calls any FEXT-specific LT/bitwise collectors. The comment at line 4077 says it 'Must stay in sync with Traces::from_elf_and_logs', but the FEXT additions in collect_ops_from_cpu/build_traces are not mirrored here.

Suggested fix

Mirror the FEXT accounting from the trace builder in count_table_lengths: add FEXT ecall counts, add their register reads to memw_register_count, add their LT and bitwise lookup counts to lt_count, and account for FEXT_PAGE/FEXT_L2G addr-LT lookups when present.

AI-007: `fext_baseline` Rust program is dead code added by the PR
  • Status: confirmed
  • Severity: low
  • Location: executor/programs/rust/fext_baseline/src/main.rs
  • Found by: moonmath:zro/minimax-m3
  • Verified by: deepseek-verifier:openrouter/deepseek/deepseek-v4-pro
  • Rejected by: -

Claim

A new Rust program (fext_baseline + Cargo.toml + Cargo.lock) is added but is not referenced by any test, benchmark, or runtime code path.

Evidence

The only references to fext_baseline in the PR are its own files and a self-referential doc comment ("Compared against fext_bench.s"). The new bench in prover/benches/vm_prover_benchmark.rs uses asm_elf_bytes("fext_bench") (the asm version) and never references fext_baseline. The Makefile's RUST_PROGRAMS_DIR=./executor/programs/rust glob picks it up and builds program_artifacts/rust/fext_baseline.elf, but no test or bench reads that artifact, so the program is built but never executed in the test/bench pipeline.

Suggested fix

Either delete the fext_baseline directory entirely (the asm version is what the bench actually exercises) or wire it into vm_prover_benchmark.rs so the produced .elf is actually benchmarked alongside fext_bench.

Reviewer Lanes

Lane Model Prompt Status Findings
glm openrouter/z-ai/glm-5.2 general success 0
kimi openrouter/moonshotai/kimi-k2.7-code general success 2
minimax minimax/MiniMax-M3 general error: opencode failed (provider/auth/runtime error) and no findings were submitted 0
moonmath zro/minimax-m3 general success 2
nemotron openrouter/nvidia/nemotron-3-ultra-550b-a55b general success 4

Verification Lanes

Lane Model Status Confirmed Rejected Uncertain
deepseek-verifier openrouter/deepseek/deepseek-v4-pro success 2 5 0

Native Codex and Claude reviews run separately and post their own comments. They are not included in this structured provenance report.

Discarded candidates (5) — rejected by the verifier
  • Guest field-storage handle collision risk in Fp3Fma implementation (crypto/crypto/src/field_ext.rs:98, found by nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — The hardcoded handles at 0xFFFF_0000_0000_0000 are used ONLY by the in-VM verifier's own Fp3Fma implementation (behind #[cfg(target_arch = "riscv64")]). Field-storage is per-program — the verifier runs as the guest program itself after the guest program's execution is complete. There is no external guest program running concurrently that could collide with these addresses. The comment acknowledges this is a convention, but the collision risk is purely hypothetical — no practical attack vector exists because the verifier is the sole program executing at the time these handles are used.
  • Missing max_rows configuration for new FEXT tables (prover/src/tables/mod.rs:127, found by nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — MaxRowsConfig is only for SPLIT tables that can be chunked across multiple sub-proofs: CPU, MEMW, MEMW_A, DVRM, MUL, LT, SHIFT, LOAD, BRANCH, MEMW_R, EQ, BYTEWISE, STORE, CPU32. The FEXT tables (like BITWISE, DECODE, HALT, KECCAK, REGISTER, ECSM, ECDAS) are FIXED tables — each appears exactly once in VmAirs, is not split, and does not need a max_rows entry. The finding incorrectly assumes FEXT tables are split like CPU or MEMW.
  • Fp3Fma trait bound on FieldExtension may limit future field extensions (crypto/stark/src/verifier.rs:12, found by nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — On non-riscv64 (host), there is a blanket impl impl<E: IsField> Fp3Fma for E that provides the default software implementation for ANY field extension. So the + Fp3Fma bound does NOT break future field extensions — they get the blanket impl automatically with the software default. On riscv64 (guest), only Fp3 (Goldilocks) has the accelerated impl, but the guest only runs with Goldilocks. The bound is not restrictive.
  • fext_bench.s comment claims distinct addresses per iteration but reuses the same 4 addresses (executor/programs/asm/fext_bench.s:31, found by moonmath:zro/minimax-m3) — The comment 'distinct addresses satisfy the accelerator's per-op guard' correctly describes the pairwise distinctness of the 4 addresses within each FEXT_FMA call (1≠2≠3≠4). The addresses do NOT change per iteration, but the comment does not claim they do — it just explains why all 4 addresses are distinct from each other. 'Fresh timestamp' is also accurate: each ecall gets a new timestamp. There is no misleading content.
  • FEXT_FMA pairwise distinctness check uses O(n^2) comparison (executor/src/vm/instruction/execution.rs:475, found by nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — The nested-loop comparison over 4 elements produces exactly 6 comparisons (not 16 as the finding claims), which is trivially negligible. The code is clear, correct, and the O(n²) notation is irrelevant at n=4 — rewriting this as a hand-unrolled set of 6 comparisons would be less readable for zero performance gain. This is not a real issue.

Raw lane outputs, candidates, final issues, and model metrics are uploaded as workflow artifacts.

@jotabulacios

Copy link
Copy Markdown
Collaborator Author

/bench-verify

@github-actions

Copy link
Copy Markdown

Benchmark started on the bench server. The verifier bench takes ~5 min; the recursion-guest cycle comparison then adds guest builds — a few minutes when cached, up to ~1h on a cold run. The bench server is occupied until it finishes.

@github-actions

Copy link
Copy Markdown

Verifier benchmark — bd80ef4255 vs main (20 pairs)

=== Verify ABBA result ===

Metric main PR Δ
Verify time (per-side) 3.479s 3.657s +5.10% 🔴
Proof size 204.30 MiB 205.52 MiB +0.60% 🔴

Per-side (⚠️ PR REJECTS the baseline's valid proof — likely a VERIFY REGRESSION, not a format change): A/B/B/A cancels machine drift but not proof-specific variance — read the Verify-time Δ as approximate.

  pairs: 20   mean A (PR): 3.657s   mean B (main): 3.479s
  [parametric] paired-t   mean +5.10%   sd 0.86%   se 0.19%
               95% CI: [+4.70%, +5.51%]   (t df=19 = 2.093)
  [robust]     median +5.06%   Wilcoxon W+=210 W-=0  p(exact)=1.9e-06  (z=+3.90)

  run-to-run jitter:    A CV 0.67%   B CV 0.39%        (lower = steadier)
  within-session drift: -0.20% over the run, 1st->2nd half +0.01%

🔴 REAL REGRESSION — PR verifies ~5.10% slower (paired-t and Wilcoxon agree).

Drift-free interleaved A/B/B/A measurement. - = PR faster. Trust the verdict when paired-t and Wilcoxon agree.


Recursion guest cycles (main vs PR)

⚠️ Recursion cycle bench did not complete (does not affect the verifier verdict above). Last log lines:

   preset=min  work=/tmp/recursion_cycles_run  sysroot=/home/app/.lambda-vm-sysroot
==> Building MEASURE_CLI (cli, release) from 3ea4f9165f ...
==> [baseline] Adding worktree /tmp/recursion_cycles_run/wt_3ea4f916 @ 3ea4f916
Preparing worktree (detached HEAD 3ea4f916)
==> [baseline] make compile-recursion-elfs @ 3ea4f916 (this can take 10-20 min the first time) ...
==> [baseline] guest ELF: recursion-min.elf
==> [baseline] dumping recursion input blob (cargo test test_dump_recursion_input) ...
==> [baseline] blob: 284904 bytes -> /tmp/recursion_cycles_run/blob_3ea4f916_min.bin
==> [baseline] measuring: /tmp/recursion_cycles_run/measure_cli execute recursion-min.elf --private-input <blob> --cycles
==> [baseline] cycles=41715803 keccak=3025  (execute wall-time 1s)
==> [PR] Adding worktree /tmp/recursion_cycles_run/wt_bd80ef42 @ bd80ef42
Preparing worktree (detached HEAD bd80ef42)
==> [PR] make compile-recursion-elfs @ bd80ef42 (this can take 10-20 min the first time) ...
==> [PR] guest ELF: recursion-min.elf
==> [PR] dumping recursion input blob (cargo test test_dump_recursion_input) ...
==> [PR] blob: 296416 bytes -> /tmp/recursion_cycles_run/blob_bd80ef42_min.bin
==> [PR] measuring: /tmp/recursion_cycles_run/measure_cli execute recursion-min.elf --private-input <blob> --cycles
ERROR: [PR] MEASURE_CLI execute failed for bd80ef4255ef1f249f05d627fc03ede2a9d620d0 (bd80ef42). Tail of stderr:
Reading private input file...
Execution failed: ExecutionError(UnknownSyscall(18446744073709551596))

@jotabulacios

Copy link
Copy Markdown
Collaborator Author

/ai-review

@github-actions

Copy link
Copy Markdown

Codex Code Review

  • Mediumfield_ext.rs:99: Scratch handles are only conventionally reserved. Public FEXT syscalls allow guest code to write H_ZERO or other scratch handles, corrupting later verifier arithmetic. Enforce a reserved namespace or explicitly initialize every scratch handle before use.

  • Lowfield_ext.rs:43: The ProdAcc API and guest implementation are unused outside tests. Despite comments presenting it as a verifier optimization, no production call site invokes it. Remove it or wire it into the verifier; currently it adds substantial unexercised stateful complexity.

Comment thread prover/src/tests/ood_window_ir_tests.rs
@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review: FEXT accelerator (a·b + c over the native Fp3)

Reviewed the full diff (~5.7k lines): executor syscalls + field-storage, the FEXT_LOAD/FEXT_FMA/FEXT_STORE/FEXT_PAGE chips, the sorted-keys uniqueness argument, the continuation carry (FEXT_LOCAL_TO_GLOBAL + GLOBAL_FIELD_MEMORY), and the guest verifier Fp3Fma routing. This is a large but carefully structured and unusually well-tested PR — I found no blocking correctness or soundness issues. Notes below.

Soundness properties I checked and believe hold

  • Single-timestamp field accesses in FEXT_FMA. The executor's pairwise-distinct-address guard (FextOperandOverlap) is a completeness requirement, not the soundness lever. A forged trace with out==a would need a second consume of a token at timestamp t for the same cell, which violates the old_ts < ts ALU-LT on that access — so the memory argument can't balance. Good that this is spelled out in the comment.
  • Read-back canonicality (FEXT_STORE). The Memory read only pins lo + 2^32·hi ≡ V (mod p); the added half-word decomposition (IsHalfword + recompose constraints) + coeff < p ALU-LT correctly close the V+p alias hole. The adversarial V+p test is the right guard.
  • Page/global uniqueness. The sorted-keys argument (strict addr increase per domain, domain +1/+2 on change, IsHalfword-pinned limbs) prevents duplicate/omitted cells; completeness is forced by the bus dangling on omission. Mirrors the existing RAM L2G/global_memory pattern.
  • next_row_columns() contract. The new next-row-read mechanism in constraints/builder.rs + lookup.rs is genuinely soundness-critical (an undeclared next-row read is pruned to zero, silently corrupting transition eval). FEXT_PAGE declares it and is covered by all_table_windows_match_captured_ir. See the one inline note about extending that test to the two continuation aggregation tables.
  • Continuation carry. field_image/field_provenance carried exactly like RAM; the interim FextInContinuation guard was correctly removed only once prove_global/verify_global consume the GlobalFieldMemory bus and the FEXT_L2G roots are commitment-bound (verify_fext_l2g_commitment_binding).

Minor / low

  • crypto/stark/src/verifier.rs: #[inline(always)] on verify_rounds_2_to_4 landed in the same commit that reverted the experimental [profile.release] block. Worth confirming it's an intended perf hint (guest verifier) rather than debug residue.
  • On riscv64 only Degree3GoldilocksExtensionField implements Fp3Fma, so the in-guest verifier is constrained to the Fp3 extension. Intended (recursion), just flagging the compile-time coupling.

Strong adversarial test coverage (tampered field-final value, non-canonical store, forged domain/next_addr, wrong FMA output, prover determinism, differential tests against the math-crate oracle for the exact guest syscall choreography). Nice work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants