Repository navigation
ORE v2 (4/n): NEON + AVX2 SIMD backends - #81
Merged
Merged
Conversation
coderdan
force-pushed
the
feat/ore-v2-simd
branch
from
June 12, 2026 15:23
b045f36 to
dc14ed9
Compare
This was referenced Jun 12, 2026
coderdan
added a commit
that referenced
this pull request
Jun 16, 2026
…view) Code-review follow-ups on the SIMD PR: - lsb_mask_256: promote the blocks.len()==256 / out.len()==32 debug_asserts to real assert_eq!. The NEON path gathers `blocks` via raw pointers assuming exactly 256 blocks, so a shorter slice would read out of bounds (UB) in a release build; the assert enforces the unsafe precondition at the boundary rather than relying on the single caller's length check. - Replace the blanket #[allow(unreachable_code)] on both dispatchers' scalar tails with #[cfg(not(target_arch = "aarch64"))], and drop the now-redundant aarch64 return. Paths are compile-time mutually exclusive, so a future bad early-return surfaces as a real diagnostic instead of being silently masked. - Scope scalar::gt_mask_xor_256 to cfg(any(not(aarch64), test)) so it isn't dead code on aarch64 (only reached via the non-aarch64 dispatcher or tests); no allow needed. scalar::lsb_mask stays always-compiled (hash.rs uses it directly for non-256 inputs). AVX2 pre-merge test coverage gap tracked in #87. No wire change (compat vectors byte-identical).
coderdan
force-pushed
the
feat/ore-v2-simd
branch
from
June 16, 2026 10:02
de430fe to
763222d
Compare
coderdan
force-pushed
the
feat/ore-v2-simd
branch
from
October 8, 2026 08:36
763222d to
951c7cb
Compare
coderdan
marked this pull request as ready for review
October 8, 2026 08:50
coderdan
force-pushed
the
feat/ore-v2-simd
branch
from
October 8, 2026 09:05
951c7cb to
d0eba34
Compare
coderdan
force-pushed
the
feat/ore-v2-simd
branch
from
October 8, 2026 09:52
d0eba34 to
642cce3
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Release validation can silently truncate masks, and the new bit setter cannot clear previously set bits.
3 open findings
What changed in this PR
Adds NEON and AVX2 acceleration for right-ciphertext encoding while retaining scalar fallbacks.
Changes:
- Adds SIMD indicator-mask and hash-LSB kernels.
- Dispatches by architecture and CPU capabilities.
- Adds randomized backend-equivalence tests.
| File | Description |
|---|---|
primitives/simd.rs |
Implements SIMD kernels, dispatch, fallbacks, and tests. |
primitives/hash.rs |
Uses vectorized LSB packing for 256 blocks. |
primitives/prp.rs |
Uses vectorized indicator-mask construction. |
primitives.rs |
Registers SIMD module and updates contracts. |
scheme/width.rs |
Adds width-generic bit-setting support. |
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
coderdan
added a commit
that referenced
this pull request
Oct 9, 2026
…view) Code-review follow-ups on the SIMD PR: - lsb_mask_256: promote the blocks.len()==256 / out.len()==32 debug_asserts to real assert_eq!. The NEON path gathers `blocks` via raw pointers assuming exactly 256 blocks, so a shorter slice would read out of bounds (UB) in a release build; the assert enforces the unsafe precondition at the boundary rather than relying on the single caller's length check. - Replace the blanket #[allow(unreachable_code)] on both dispatchers' scalar tails with #[cfg(not(target_arch = "aarch64"))], and drop the now-redundant aarch64 return. Paths are compile-time mutually exclusive, so a future bad early-return surfaces as a real diagnostic instead of being silently masked. - Scope scalar::gt_mask_xor_256 to cfg(any(not(aarch64), test)) so it isn't dead code on aarch64 (only reached via the non-aarch64 dispatcher or tests); no allow needed. scalar::lsb_mask stays always-compiled (hash.rs uses it directly for non-256 inputs). AVX2 pre-merge test coverage gap tracked in #87. No wire change (compat vectors byte-identical).
coderdan
added a commit
that referenced
this pull request
Oct 9, 2026
Review of #81: - Removing pack_bits_lsb_first (68ec6d0) dropped its release-mode length check. hash_all_into, scalar::lsb_mask and gt_mask_xor_256 now assert their output length in release too, so a mismatch fails the same way on every backend instead of the scalar path silently truncating. - RightBitVec::set_bit promised assignment but forwarded to an OR-only setter. It had no callers outside tests, so it is removed from the trait; the inherent RightBlock32::set_bit now overwrites the bit, matching its doc. Its only production caller wrote 0/1 into zeroed blocks, where OR and assign agree, so no ciphertext byte changes.
coderdan
force-pushed
the
feat/ore-v2-simd
branch
from
October 9, 2026 09:31
861fc03 to
c2a9591
Compare
coderdan
added a commit
that referenced
this pull request
Oct 9, 2026
e164730 (13 June) predates the #81 self-review commit 951c7cb (16 June). When this branch was rebased over it, e164730's version of simd.rs won, silently undoing two of 951c7cb's fixes: - lsb_mask_256's length checks went back to debug_assert. The NEON path gathers blocks through raw pointers assuming exactly 256 of them, so a shorter slice reads out of bounds in a release build: UB reachable from a safe fn. Today's callers check the length first, but the fn was unsound. They are real asserts again. - The scalar fallbacks went back behind #[allow(unreachable_code)], which Rust 1.78 does not honour on a statement, so clippy failed on aarch64. Every dispatcher, including the new gt_mask_xor_64, now ends in the NEON call on aarch64 and puts the AVX2 check and scalar fallback under cfg(not(aarch64)); scalar::gt_mask_xor is compiled only where it is used, as 951c7cb had it. Bit6 also drops its explicit DOMAIN, now derived from BITS.
coderdan
force-pushed
the
feat/ore-v2-simd
branch
from
October 12, 2026 00:54
c2a9591 to
9121b00
Compare
Adds primitives::simd with the two vectorisable kernels from the right- ciphertext encoder (v2 plan §3), wired into indicator_mask_xor and hash_all_into: - gt_mask_xor_256: bytewise greater-than against a broadcast value packed to a bitmask. NEON (vcgtq_u8 + and-with-bitpos + vaddv_u8 pack; the shrn trick benched slower on M1 for this exact bit layout) and AVX2 (vpcmpgtb with 0x80 bias for unsigned semantics + vpmovmskb). ~30x over the pre-PR-3 per-bit shape, ~4x over the scalar bulk form in isolation. - lsb_mask_256: stride-16 gather of AES-output LSBs. NEON via ld1q_x4 + constant-index vqtbl4q (the textbook ld4+uzp1 tree benched ~25% slower on M1); scalar elsewhere (AVX2 gains nothing without AVX-512 VBMI). Dispatch: compile-time on aarch64 (NEON is baseline), cached runtime detection on x86_64, length-generic scalar fallback everywhere else. All paths branch-free with fixed trip counts and constant tbl indices (constant-time discipline documented in the module). Equivalence pinned three ways: dispatched-vs-scalar randomized tests in the module, mask-vs-per-bit-invert quickcheck in prp.rs, and the PR 1 byte vectors end-to-end (which on this machine exercise the NEON path). Per-u64 gain on M1 Max is ~0.5% over PR 3 (PRP construction dominates at Bit8); these kernels are sized for the Bit6 scheme where per-block AES work shrinks 4x. Microarch caveat: pack/gather strategy choices are M1-tuned; re-bench on Neoverse before locking server-ARM dispatch. Part of the ORE v2 program (docs/plans/2026-06-12-ore-v2-architecture.md, PR 4).
…view) Code-review follow-ups on the SIMD PR: - lsb_mask_256: promote the blocks.len()==256 / out.len()==32 debug_asserts to real assert_eq!. The NEON path gathers `blocks` via raw pointers assuming exactly 256 blocks, so a shorter slice would read out of bounds (UB) in a release build; the assert enforces the unsafe precondition at the boundary rather than relying on the single caller's length check. - Replace the blanket #[allow(unreachable_code)] on both dispatchers' scalar tails with #[cfg(not(target_arch = "aarch64"))], and drop the now-redundant aarch64 return. Paths are compile-time mutually exclusive, so a future bad early-return surfaces as a real diagnostic instead of being silently masked. - Scope scalar::gt_mask_xor_256 to cfg(any(not(aarch64), test)) so it isn't dead code on aarch64 (only reached via the non-aarch64 dispatcher or tests); no allow needed. scalar::lsb_mask stays always-compiled (hash.rs uses it directly for non-256 inputs). AVX2 pre-merge test coverage gap tracked in #87. No wire change (compat vectors byte-identical).
Review of #81: - Removing pack_bits_lsb_first (68ec6d0) dropped its release-mode length check. hash_all_into, scalar::lsb_mask and gt_mask_xor_256 now assert their output length in release too, so a mismatch fails the same way on every backend instead of the scalar path silently truncating. - RightBitVec::set_bit promised assignment but forwarded to an OR-only setter. It had no callers outside tests, so it is removed from the trait; the inherent RightBlock32::set_bit now overwrites the bit, matching its doc. Its only production caller wrote 0/1 into zeroed blocks, where OR and assign agree, so no ciphertext byte changes.
coderdan
force-pushed
the
feat/ore-v2-simd
branch
from
October 12, 2026 01:05
9121b00 to
773d717
Compare
freshtonic
approved these changes
Oct 12, 2026
freshtonic
left a comment
freshtonic
left a comment
Contributor
There was a problem hiding this comment.
Approved after the round-2 security re-review. The round-1 findings for this PR are fixed or have no remaining blocking items.
🤖 Generated with Claude Code
coderdan
added a commit
that referenced
this pull request
Oct 12, 2026
e164730 (13 June) predates the #81 self-review commit 951c7cb (16 June). When this branch was rebased over it, e164730's version of simd.rs won, silently undoing two of 951c7cb's fixes: - lsb_mask_256's length checks went back to debug_assert. The NEON path gathers blocks through raw pointers assuming exactly 256 of them, so a shorter slice reads out of bounds in a release build: UB reachable from a safe fn. Today's callers check the length first, but the fn was unsound. They are real asserts again. - The scalar fallbacks went back behind #[allow(unreachable_code)], which Rust 1.78 does not honour on a statement, so clippy failed on aarch64. Every dispatcher, including the new gt_mask_xor_64, now ends in the NEON call on aarch64 and puts the AVX2 check and scalar fallback under cfg(not(aarch64)); scalar::gt_mask_xor is compiled only where it is used, as 951c7cb had it. Bit6 also drops its explicit DOMAIN, now derived from BITS.
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.
Stacked on #80. Plan §3 (
docs/plans/2026-06-12-ore-v2-architecture.md).What
primitives::simdwith the two data-parallel kernels of right-ciphertext encoding, selected by a spike benchmarked on M1 Max (kernels verified byte-identical over 10k randomized inputs before lifting):table[j] > x→ bitmask)vcgtq_u8+ bitpos-AND +vaddv_u8(8.3 ns/256 lanes)vpcmpgtb(0x80-biased) +vpmovmskbld1q_x4+ constant-indexvqtbl4q(41.8 ns/256)Dispatch: compile-time NEON on aarch64, runtime-detected AVX2 on x86_64 (
is_x86_feature_detected!, cached by std), scalar elsewhere. No cargo feature — one build, tests cover the dispatched and scalar paths plus the per-bit reference.Constant-time
Every path has fixed trip counts, no data-dependent branches, and constant
tblindices; documented as a module invariant.Honest numbers
~0.5% over PR 3 at Bit8 on M1 Max — PRP construction now dominates that scheme (see #80). These kernels are sized for Bit6 (PR 5), where per-block AES work drops 4× and the 64-lane indicator variant (2.7 ns) carries the encoder. Landing them separately keeps PR 5 reviewable.
Verification notes for review
gt_mask_dispatched_matches_scalartest exercises it). It could not be run locally (aarch64 host, no rustup x86_64 std).shrnandld4+uzp1lost on M1, and the Neoverse re-bench caveat) preserved in the commit message and/tmp/ore-neon-spike/RESULTS.md.