Repository navigation
Conversation
(cherry picked from commit e7e1e43958802e67c0d1417ab114f964a3a9b0b7)
…er gathers from mul_index8 gathers a table point's x at +0 and y at +32. Nothing fixed that layout, so a change in ark's Affine would have read the wrong words without a compile error; assert it, as ifma_normalize does. The production window (LANE_PRECOMP_WINDOW_SIZE) was only checked by the ignored bench; add it to lane_mul_index_matches_scalar. words_to_limbs now comes from the shared IFMA module, so the copy here goes. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
On a host with AVX-512 IFMA the shared committer is now built at window 7, where one mul_index is 37 additions instead of 24. The trie update went to mul_index_batch with that change, but msm and msm_dense still multiplied one scalar at a time, so proof creation (the IPA rounds, the dense polynomial commitment) and the node-polynomial cache's admission check would each have slowed by about half on those hosts. Send each thread's terms through mul_index_batch there, in tasks of whole lane groups. Hosts without lanes keep the scalar loop unchanged. The IPA's blinding term was a lone mul_index per side per round; make it one more term of the round's msm. Not measured: needs a host with AVX-512 IFMA (salt/benches/proof.rs). Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Issuing the trie update's multiplications per chunk added seven lines above create_node_aligned_chunks, moving its four pinned loop sites without changing them (trie.rs 905/911/914/916 -> 912/918/921/923). Same code, same mutants, so the existing justifications still hold. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Mutation testing - PASSMutation score: 100.0% (1/1 viable mutants killed)
No unsuppressed survivors or timeouts remain. |
Performance Benchmark ComparisonCompared Detailed Comparison
|
Mutation testing - PASSMutation score: 100.0% (6/6 viable mutants killed)
No unsuppressed survivors or timeouts remain. |
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ports PR #4 of
suryuhh/salt-optimizations(commite7e1e43): the committer's fixed-base multiplications, eight per vector register, over smaller tables.Committer::mul_index_batchis new. On a host with AVX-512 IFMA it runs eight multiplications per register; elsewhere it mapsmul_index.Hosts without IFMA keep window 11 and the scalar path.
Stacked on the compact storage PR (
liquan/perf/compact-witness-storage). Last of the four.Changes on top of the external commit
msmandmsm_denserun in lanes too. At window 7 onemul_indexis 37 additions instead of 24. The external change moved the trie update to the batch, butCommitter::msmandmsm_densestill multiplied one scalar at a time. Proof creation (the IPA rounds, the dense polynomial commitment) and the node-polynomial cache's admission check go through those two, so each would have slowed by about half on IFMA hosts. Each thread's terms now go throughmul_index_batchthere, in tasks of whole lane groups. Hosts without lanes keep the scalar loop as it was.msm. It was a lonemul_indexper side per round.G ++ [Q]tables of perf: faster witness proofs and proof checks #152. The only cherry-pick conflict:DEFAULT_CRS.committer(platform::precomp_window_size()).mul_index8gathersxat +0 andyat +32 of anEdwardsAffine; a different layout now fails to compile.trie.rsare re-pinned (905/911/914/916 to 912/918/921/923). The code at those sites did not change.Testing
Passing on this head, on an Apple Silicon machine:
cargo fmt --all -- --check;cargo clippy --all-targets -- -D warnings, native and--target x86_64-unknown-linux-gnucargo test; withtest-bucket-resize(CI's settings); the random stress test;--no-default-featureswith and withouttest-bucket-resizecargo check -p salt --target riscv64imac-unknown-none-elf --no-default-featurescargo check --all-targets --target i686-unknown-linux-gnustateless-validatormain(c46a541):cargo check --workspace --all-targetswith itssaltdependency patched to this head.New tests run on every host:
mul_index_batchagainstmul_indexat windows 11 and 7, the batched sum against the scalar sum and the reference MSM, and the task size.create_with_precomp_matches_createcovers the blinding term.Not run:
msmabove. No such CPU or emulator on the machine used; compiled and linted forx86_64only. On a host without lanes the new tests only exercise the scalar fallback ofmul_index_batch. TheTest Suite (AVX-512 IFMA)job is their first real execution.perf-regressionrunsupdate 10000 KVson a runner with IFMA, so the trie update number arrives with this PR.salt/benches/proof.rs(witness creation) is not covered by it and needs a run on a host with IFMA againstmain.mega-rethagainst this head.Merging
Please keep this trailer in the squash message:
🤖 Generated with Claude Code