Skip to content

perf: deferred-reduction accumulation in accumulate_numerators - #1460

Closed
odinfree wants to merge 1 commit into
starkware-libs:devfrom
odinfree:perf-deferred-reduction-quotients
Closed

odinfree wants to merge 1 commit into
starkware-libs:devfrom
odinfree:perf-deferred-reduction-quotients

Conversation

@odinfree

Copy link
Copy Markdown

What

crates/stwo/src/prover/backend/simd/quotients.rs, accumulate_numerators_on_subdomain: the per-term acc += c*val - b loop paid a full QM31 multiply + reduce + add-min per packed element per term. This PR accumulates the exact 62-bit M31 products in i64x8 lanes and reduces once per 64-element chunk instead of per term (deferred reduction / lazy-accumulator style), and hoists the per-term b subtraction to one scalar sum per chunk.

Mechanics: even/odd transmute splits of each packed u32x16 into u64x8 products (the same primitive mul_doubled_simd uses); a lazy signed Mersenne-31 fold x mod P = (x >> 31) + (x & (2^31-1)) every 2 terms — exact for signed values with arithmetic shift — keeps accumulators below 2^63; one exact signed final reduction per chunk per coordinate. All portable std::simd: no cfg, no target_feature, no std::arch.

Why

Measured on the stage (100 columns × 2^20, blowup 1, Apple Silicon, 8 rayon threads, --release, isolated paired A/B): 20.5–21.2 → 10.3–11.7 ms medians (~1.9–2.0x).

Bit-exactness

  • M31 products are < 2^62 → exact in i64 lanes; signed two's-complement accumulators handle subtraction terms.
  • The lazy fold is exact mod P for any signed x; the final reduction equals the per-term reductions by associativity.
  • test_simd_and_cpu_numerators_are_consistent passes unchanged; full suite 267/267 with and without the parallel feature; clippy -D warnings and fmt clean.

Scope

Applies to independent-term accumulation sums (this loop; numerator accumulation generally). It does not apply to dependency chains (butterflies, fold chains, batch-inversion prefix/suffix products). Developed while profiling the quotient path; the same structure can serve future accumulation-heavy kernels.

The per-term acc += c*val - b loop paid a full QM31 multiply + reduce +
add-min per packed element. Accumulate the exact 62-bit M31 products in
i64x8 lanes instead (even/odd transmute splits, the same primitive
mul_doubled_simd uses), with a lazy signed Mersenne-31 fold every 2 terms
(arithmetic shift keeps the sign; exact for any signed x) and one exact
signed reduction per chunk per coordinate. The per-term b subtraction is
hoisted to one scalar sum per chunk.

All portable std::simd: no cfg/target_feature/std::arch. Bit-exact:
products are exact in 64-bit lanes, folds are exact mod P, and the final
reduction equals the per-term reductions by associativity.

Measured (Apple Silicon, 8 rayon threads, release, isolated paired A/B at
100 cols x 2^20, blowup 1): 20.5-21.2 -> 10.3-11.7 ms medians (~1.9-2.0x
on the stage). CPU-consistency test passes unchanged; 267/267 suite,
both feature configs; clippy -D warnings and fmt clean.
@cursor

cursor Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches core prover quotient SIMD math with custom modular reduction; correctness relies on lazy-fold bounds and chunk-wise algebra matching the old per-term path.

Overview
Reworks SIMD quotient numerator accumulation in accumulate_numerators_on_subdomain so each 64-element chunk sums c * val in wide i64x8 lanes and reduces to M31 once per chunk, instead of doing a full PackedSecureField multiply-add-minus per column term.

The new WideSecureAcc path widens packed base-field lanes via even/odd u64x8 splits, applies periodic lazy signed Mersenne-31 folds to keep accumulators in range, then finishes with exact signed reduction into four packed coordinates. The per-term b subtraction is hoisted to a single b_sum per chunk, applied after reduction. Implementation stays on portable std::simd (no arch-specific intrinsics).

Behavior is intended to remain bit-exact versus the prior SIMD loop (same algebra, deferred reduction); existing SIMD vs CPU consistency tests are unchanged.

Reviewed by Cursor Bugbot for commit 903ca8a. Bugbot is set up for automated code reviews on this repo. Configure here.

@reviewable-StarkWare

Copy link
Copy Markdown

This change is Reviewable

@odinfree

Copy link
Copy Markdown
Author

The coverage check failure is a Codecov upload flake, not a code issue: the uploader failed with HTTP 429 (rate limit) while fetching the storage URL (see the job log: 'Rate limit reached. Please upload with the Codecov repository upload token'). All test/clippy/doc/format checks pass. A rerun of that job when the rate limit clears should turn it green — I don't have rerun rights on the upstream repo.

@odinfree

Copy link
Copy Markdown
Author

Withdrawing for the time being — the author needs to run an internal review before this is considered for submission. Will resubmit (or a maintainer can pick the idea up) after that review. Thanks for the CI run.

@odinfree odinfree closed this Sep 21, 2026
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