Skip to content

fix(prover): align CPU sign bit constraints with SIGN template spec (#435)#460

Merged
jotabulacios merged 5 commits into
mainfrom
fix/cpu-sign-bit-spec-alignment
Mar 30, 2026
Merged

fix(prover): align CPU sign bit constraints with SIGN template spec (#435)#460
jotabulacios merged 5 commits into
mainfrom
fix/cpu-sign-bit-spec-alignment

Conversation

@ColoCarletti
Copy link
Copy Markdown
Collaborator

Aligns the CPU table's sign/extension bit handling with the updated spec (PR #435),
which replaced raw MSB16/MSB8 interactions with the SIGN template.

Changes

Column renames (no index changes):

  • rv1_sign_bitrv1_ext_bit
  • arg2_sign_bitrv2_ext_bit (clarifies it derives from rv2, not arg2)
  • res_sign_bitres_ext_bit

res extension bit: MSB8 → MSB16

  • Old: MSB8[res[3]] — extracted bit 7 of byte 3 (= bit 31 of res)
  • New: MSB16[res[2] + 256*res[3]] — extracts MSB of half at bits 16-31 (= bit 31 of res)
  • Matches spec: SIGN(res::DWordHL[1], word_instr)
  • Both extract bit 31, but the new version uses MSB16 consistently for all three ext bits

Constraint split (CE57)

  • Old: single constraint (rv1_sign_bit + arg2_sign_bit + res_sign_bit) * (1 - word_instr) = 0
  • New: three separate constraints per the SIGN template:
    • (1 - word_instr) * rv1_ext_bit = 0
    • (1 - word_instr) * rv2_ext_bit = 0
    • (1 - word_instr) * res_ext_bit = 0
  • Constraint count: 64 → 66

@github-actions
Copy link
Copy Markdown

Codex Code Review

No actionable issues found in this PR diff.

Residual risk/testing gap:

  • I could not run the targeted tests in this environment because cargo/rustup failed with a read-only temp/toolchain path error (/home/runner/.rustup/tmp), so this review is static-only.

Comment thread prover/src/tables/cpu.rs Outdated
Comment thread prover/src/tables/cpu.rs Outdated
@claude
Copy link
Copy Markdown

claude Bot commented Mar 25, 2026

Overall the changes look correct and the logic is sound.

Constraint equivalence: The old combined constraint (rv1 + rv2 + res) * (1 - word_instr) = 0 and the new three separate constraints are equivalent because all three columns are binary (they're in BIT_FLAG_COLUMNS, so IS_BIT constraints apply). The split is cleaner and easier to reason about.

MSB16 vs MSB8 for res_ext_bit: Both extract the same bit (bit 31 of res). The change to MSB16 on the halfword res[2] + 256*res[3] is consistent with the other two extension bits.

Minor nits (two inline, one here):

  • test_trace_generation_sign_bits in cpu_tests.rs:289 was not renamed to test_trace_generation_ext_bits to match the rest of the rename.

@ColoCarletti
Copy link
Copy Markdown
Collaborator Author

/bench

@github-actions
Copy link
Copy Markdown

Benchmark — fib_iterative_2M (median of 3)

Table parallelism: 32 (auto = cores / 3)

Metric main PR Δ
Peak heap 113806 MB 114032 MB +226 MB (+0.2%) ⚪
Prove time 27.970s 28.199s +0.229s (+0.8%) ⚪

✅ No significant change.

✅ Low variance (time: 3.3%, heap: 1.1%)

Memory Growth

Measured with TABLE_PARALLELISM=1 (sequential) · best of 2 samples per point

Program Steps main (MB) PR (MB) Δ
fib_iterative_250k 250k 7349 7765 +416 MB (+5.7%)
fib_iterative_500k 500k 10268 10169 -99 MB (-1.0%)
fib_iterative_1M 1M 16348 16600 +252 MB (+1.5%)
fib_iterative_2M 2M 26538 26522 -16 MB (-0.1%)

Growth rate: 10813 MB / 1M steps (main: 10949, Δ: -1.2%)
Fit: R² = 0.9970 (main: 0.9981)

✅ No significant change in memory scaling.

Commit: 5824479 · Baseline: built from main · Runner: self-hosted bench

@ColoCarletti
Copy link
Copy Markdown
Collaborator Author

/bench

@MauroToscano
Copy link
Copy Markdown
Contributor

/bench 10

@jotabulacios jotabulacios added this pull request to the merge queue Mar 30, 2026
Merged via the queue into main with commit f3e1936 Mar 30, 2026
9 checks passed
@jotabulacios jotabulacios deleted the fix/cpu-sign-bit-spec-alignment branch March 30, 2026 14:12
erik-3milabs pushed a commit that referenced this pull request Apr 10, 2026
…435) (#460)

* update cpu table

* fmt

* fix comments

---------

Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com>
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.

3 participants