Skip to content

perf(gpu): run DECODE (num_parts=1) DEEP/FRI on device - #946

Open
ColoCarletti wants to merge 4 commits into
mainfrom
decode-deep-fri-gpu
Open

perf(gpu): run DECODE (num_parts=1) DEEP/FRI on device#946
ColoCarletti wants to merge 4 commits into
mainfrom
decode-deep-fri-gpu

Conversation

@ColoCarletti

Copy link
Copy Markdown
Collaborator

Extends the device-resident composition-parts path to num_parts==1 so the DECODE table's DEEP/FRI runs on GPU instead of host.

For a degree-1 composition, H is already the single part on the LDE coset — no decompose, no re-extension, just an interleaved→slab de-interleave (new comp_h_to_slabs_ext3 kernel, no NTT). The R2 commit, R3 OOD, R4 DEEP, FRI and openings already read the part count from the handle, so no downstream changes are needed; the host fallback is preserved.

Validation (ethrex 100tx, RTX 5090):

  • Wall: 13.13s -> 12.18s (-7.25%, ABBA x3)
  • Cross-verify OK; math-cuda + stark cuda suites green
  • Unit parity: de-interleave bit-exact, evaluate_dev(keep) == host bit-for-bit

@ColoCarletti

Copy link
Copy Markdown
Collaborator Author

/bench-gpu

@ColoCarletti

Copy link
Copy Markdown
Collaborator Author

/ai-review

@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown

GPU Benchmark (ABBA) — a9aead5753 vs main (14 pairs)

RTX 5090 · AMD Ryzen 9 9950X 16-Core Processor (32 threads) · Vast.ai datacenter @ $0.9347222222222221/hr · prover/cuda · ethrex real block, continuations · drift-free A/B/B/A

=== ABBA paired result  (improvement: - = PR faster) ===
  pairs: 14   mean A (PR): 71.796s   mean B (base): 74.506s

  [parametric] paired-t   mean -3.55%   sd 4.69%   se 1.25%
               95% CI: [-6.26%, -0.85%]   (t df=13 = 2.16)
  [robust]     median -4.53%   Wilcoxon W+=14 W-=91  p(exact)=0.0134  (z=-2.39)

  --- server stability (this run; compare across servers) ---
  run-to-run jitter:    A CV 7.20%   B CV 6.93%        (lower = steadier)
  within-session drift: +6.97% over the run, 1st->2nd half +4.27%
    (jitter -> Tier-1 cached gate floor; drift -> whether the cached baseline can be trusted)

  VERDICT: REAL IMPROVEMENT - PR faster by ~3.55% (t-CI and Wilcoxon agree)

  raw pairs: /tmp/abba_run/pairs.csv

- = PR faster. Trust the verdict when paired-t and Wilcoxon agree.

@github-actions

Copy link
Copy Markdown

Codex Code Review

No issues found in the PR changes.

@claude

claude Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Review: perf(gpu): run DECODE (num_parts=1) DEEP/FRI on device

Reviewed the kernel, the math-cuda wrapper, the prover dispatch, and every downstream consumer of GpuLdeExt3 (build_comp_poly_tree_from_slabs_dev, try_barycentric_ext3_on_ext3_handle, deep_composition_ext3, gather_rows_ext3_on_device, download_ext3_columns, materialize_composition_parts_host). No Critical or High safety issues. The design holds up:

  • comp_h_to_slabs_ext3 is a pure grid-strided transpose writing all 3 * num_rows u64, so the unsafe { stream.alloc } (no zero-init) is sound — the contrast with the zero-padded d=2 slab layout is correctly called out in the comment.
  • Slab ordering out[k*num_rows + i] matches the handle contract (c*3 + k) * lde_size at m = 1, and matches decompose_d2_ext3's [h0.abc, h1.abc] layout.
  • Cross-stream handoff mirrors the d=2 path: kernel enqueued on h.stream, PooledEvent recorded, consumers wait_ready_on device-side. GpuCompH::buf's stream-ordered free is ordered behind the kernel on the same stream.
  • Every downstream consumer already reads the part count from handle.m / lde_composition_poly_evaluations.len(), so m = 1 needs no changes — confirmed, and num_parts == 1 already exercised the GPU comp-poly tree before this PR.
  • The host fallbacks (downloaded_hvec![h], and the else if number_of_parts == 1 arm) are value-equivalent to evaluator.evaluate(...).

Findings

Medium — device_only_for gate not extended; the want_host == false branch is dead. device_only_gate is still reached only when composition_poly_degree_bound(n) / n == 2 (prover.rs:1101), and host_trace_empty is set only when that gate held at R1. So for a num_parts == 1 table want_host is always true: the vec![Vec::new()] placeholder in try_deinterleave_comp_h_dev is unreachable, DECODE still does the full H D2H every proof, and the gate's comments ("the device-resident R2 path only exists for the d=2 quotient decomposition", prover.rs:1094 and :1101) are now wrong. Inline detail on the branch.

Low — gpu_comp_h_slabs_calls() is exported but never read. Every sibling counter is asserted in prover/tests/cuda_path_integration.rs; this one isn't, so a silent regression of the d=1 path to the host arm would go unnoticed. Add the assertion or drop the counter.

Low — comp_h_from_host_interleaved is test-only public API on math_cuda::constraint_interp. It's honestly documented as a parity helper and it does need to be pub for the integration test, so this is just a note that it widens the crate's surface for no production caller.

Low — the if number_of_parts == 1 { deinterleave } else { decompose_d2 } dispatch is duplicated verbatim in round_2_compute_composition_polynomial (prover.rs:1702) and xcheck_post_mortem (prover.rs:4299), as is the vec![host_h] vs decompose_and_extend_d2 split. A small helper would keep the two from drifting — the xcheck's whole value is being an apples-to-apples mirror of the producer.

Test coverage

comp_h_to_slabs.rs (bit-exact transpose over log 1..=14, raw non-canonical limbs) and the decode_shaped_program + evaluate_dev(keep=true) == host-drained H parity in gpu_constraint_interp.rs are both well targeted at the two things that could go wrong here. The gap is the integration level — nothing asserts the d=1 device path actually engages end to end (see the counter finding).

Comment thread crypto/stark/src/gpu_lde.rs Outdated
Comment thread crypto/stark/src/gpu_lde.rs
@github-actions

Copy link
Copy Markdown

AI Review

PR #946 · 7 changed files

Findings

No non-rejected structured findings were reported.

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 0
minimax minimax/MiniMax-M3 general error: opencode failed (provider/auth/runtime error) and no findings were submitted 0
moonmath zro/minimax-m3 general error: opencode failed (provider/auth/runtime error) and no findings were submitted 0
nemotron openrouter/nvidia/nemotron-3-ultra-550b-a55b general success 0

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

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

@ColoCarletti

Copy link
Copy Markdown
Collaborator Author

/bench-gpu

1 similar comment
@ColoCarletti

Copy link
Copy Markdown
Collaborator Author

/bench-gpu

Extend the device-resident composition-parts path to num_parts==1. For d=1, H is already the single part on the LDE coset, so deinterleave it into a 1-part GpuLdeExt3 (comp_h_to_slabs_ext3 kernel, no NTT) instead of running DEEP/FRI on host; the commit, R3 OOD, R4 DEEP, FRI and openings already read the part count from the handle. Proof-identical; host fallback preserved.
…variants

- add prover/tests/cuda_d1_path.rs + `make test-cuda-d1` (gpu_test.sh Group 3):
  lowers LAMBDA_VM_GPU_LDE_THRESHOLD so DECODE engages the d=1 device DEEP/FRI
  path end to end, asserting gpu_comp_h_slabs_calls > 0 and the proof verifies.
  Its own binary because gpu_lde_threshold() caches the env (OnceLock).
- decompose_comp_h_dev: debug_assert want_host and H rows == LDE domain size on
  the d=1 branch (the d=2 arm gets an incidental check via weights.len()).
- try_deinterleave_comp_h_dev: document that the always-drained host part feeds
  the release query-0 canary — the only e2e check on the d=1 layout.
`test-cuda-d1` set LAMBDA_VM_GPU_LDE_THRESHOLD=64 on the premise that 64 is
"the exact LDE size of fib_iterative_1M's DECODE ROM". It is 32, and the
reasoning behind the number was wrong too: DECODE's rows come from the ELF's
executable words, not from cycles.

fib_iterative_1M is 13 executable words (one 52-byte executable PT_LOAD; the
variants differ only in the `li a0, <count>` immediate, so fib_iterative_16M is
13 too). 13 + 1 CPU-padding entry = 14 -> next_power_of_two() = 16 rows ->
blowup 2 -> DECODE LDE 32. At threshold 64 that is below the gate, so DECODE
failed the R1 split-tree commit, had no gpu_main() handle, and evaluate_dev
declined - DECODE never reached the d=1 path at all.

The counter could therefore only be fed by KECCAK_RC, the only other
num_parts==1 table (a d=1 table is one with a single bus interaction), whose
fixed NUM_ROWS=32 gives LDE 64 and passes `64 < 64` by one unit. So the target,
the test name, the module docs and the assert message all named the one d=1
table guaranteed not to be exercised.

No threshold fixes this with a fib fixture: DECODE (32) sits below KECCAK_RC
(64), so <=32 engages both and 33..=64 engages only KECCAK_RC. Switch to
all_instructions_64 - 66 executable words -> 128 rows -> DECODE LDE 256 - at
threshold 128, where DECODE engages with 2x margin and KECCAK_RC declines, so a
nonzero gpu_comp_h_slabs_calls() uniquely attributes to DECODE.

128 is also higher than the previous 64, so strictly fewer tables land on the
GPU-committed path: it narrows rather than widens the R4 gather_proofs_dev abort
site that gpu_lde.rs warns about for lowered thresholds.

Tighten the test's own guard while here. `thr > 0 && thr < 1<<14` passed
vacuously for any wrong value - including the 64 that caused this - so pin the
window to (KECCAK_RC_LDE, DECODE_LDE] against named constants instead.
… counts (#948)

* docs(gpu): update the group counts the new test group invalidated

Adding cuda_d1_path as Group 3 of gpu_test.sh renumbered the groups after it, but
five references still describe the old five-group layout:

- scripts/gpu_test.sh: "the prover suite (Groups 4 & 5) proves asm AND rust
  guests" is now Groups 5 & 6 - and it is the only thing explaining why the
  script builds rust guests up front, so a Group 5 failure sends the reader to
  test-cuda-fallback, which needs no rust guests.
- Makefile: a hang in Group 1 now costs Groups 2-6, not 2-5.
- gpu-tests.yml: the group enumeration and "5 test groups" both predate the new
  group; that comment is the merge-gate contract for anyone who does not open the
  shell script.
- cuda_path_integration.rs: the R2 composition-LDE comment enumerates two
  num_parts arms; there are now three, and the new one increments neither counter
  in the assertion below it (the assertion is still correct - no d=1 table here
  crosses the default threshold - so this is comment-only).

Also move the coverage note off the end of check_composition. It described
suite-wide coverage from inside a helper shared by two tests, and its claim that
the end-to-end d=1 counterpart "is not asserted ... exercised by real-program
proves (ethrex) and the GPU bench instead" was invalidated by this branch's own
second commit, which adds prover/tests/cuda_d1_path.rs. Restate it accurately on
the decode-shaped test it actually describes.

* docs(gpu): correct the d=1 composition-parts comments; share the admission gate

Four claims in the new d=1 prose do not match the code.

1. "all of which already read the part count from `handle.m`", and the same in
   decompose_comp_h_dev's doc. Only the R2 commit and the R4 openings read
   handle.m. R3's z^P exponent and R4 DEEP's gamma count read
   lde_composition_poly_evaluations.len() - the host part Vec's length - and DEEP
   merely validates the handle against it, declining on a mismatch. FRI never
   receives the handle at all. Benign today, because the d=1 arm always drains one
   host part, but the sentence is the stated reason for not touching R3/R4 and it
   credits the handle with the host Vec's authority. Replace it with the invariant
   that actually has to hold - handle.m == lde_composition_poly_evaluations.len(),
   which materialize_composition_parts_host also requires - and note the same on
   the d=2 arm's doc.

2. "it is the only end-to-end check that the device m=1 gather / DEEP / FRI layout
   is correct". The canary compares a device composition-row gather against the
   host part evals; DEEP and FRI consume separate downstream buffers and are not
   covered by it. cuda_d1_path.rs already describes the same canary correctly, as
   guarding "the composition-row gather". Narrow the claim to the in-prove gather
   check and point at proof verification for DEEP/FRI.

3. "zeroing a preprocessed table's host trace fails its commitment check".
   Preprocessed tables do go device-only: commit_main_trace takes device_only, the
   caller applies no preprocessed exclusion, and the preprocessed branch passes
   !device_only as want_host precisely to support it. The precomputed-root check
   runs against the device-built tree, so the host drain cannot reach it, and
   host_trace_empty is not set until Round1 construction - after every R1 commit.
   Nothing is zeroed either; the Vec is left empty. Restore the accurate reason
   (any other part count has no device R2 path and needs the host evaluator) and
   give d=1's real one: it always drains its single part to feed the canary, so it
   gains nothing from dropping the host trace.

4. "the degree gate below" in decompose_comp_h_dev. There is no gate below it in
   that function; the gate is device_only_for, far above.

Also drop the duplicated half of device_only_for's rationale, which restated the
d=2 sentence eight lines later and was where claim 3 lived, and correct the
"nothing to unwind" note on the d=1 download ordering: both values drop by RAII in
either order, so the ordering is about keeping the blocking D2H off the tail of the
de-interleave launch, not about unwinding.

Two small cleanups while in here:

- Hoist the admission gate the d=1 and d=2 producers had duplicated verbatim (two
  TypeId guards plus the threshold/power-of-two test) into dev_comp_parts_gate, so
  a future condition - a VRAM check, a tower widening - cannot land on only one
  arm and silently diverge them.
- Rename try_deinterleave_comp_h_dev to try_comp_h_to_slabs_dev, matching the
  kernel (comp_h_to_slabs_ext3), the math-cuda entry point (comp_h_to_slabs) and
  the counter (GPU_COMP_H_SLABS_CALLS); it was the one link in that chain that a
  grep from either end would miss. Drop the single-use `decomposed` temporary at
  the call site, which read as a borrow workaround where none is needed.
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