Refactor SIMD into Facade/Backend layers and consolidate CRC engine#100
Merged
Conversation
Introduce src/Simd/ with a Facade/ subfolder (arch-agnostic selection) and a Backend/ subfolder (per-hash, per-arch SIMD kernels + runtime tier selection). Each hash's core unit keeps the scalar implementation and calls the SIMD path through its facade, so the cores carry no TCpuFeatures / HASHLIB_*_ASM knowledge. For all 12 primitives (SHA-1, SHA-256, SHA-512, SHA-3, Blake2B, Blake2S, Blake3, XXH3, Adler32, Argon2, Scrypt, CRC): - Hlp<Hash>Simd (Facade) picks the backend at compile time - Hlp<Hash>X86Backend SSE2/SSSE3/AVX2/SHA-NI/PCLMUL kernels + Select - Hlp<Hash>ArmBackend NEON/SHA/PMULL kernels + Select Init-time slot selection is preserved: each core's initialization sets its function pointer once via the facade's Select, so TransformBlock pays zero per-call dispatch cost.
Engine consolidation (6 units -> 3, ~1,800 lines removed): - Delete HlpCRCStandard (wrapper TCRC16/32/64 + dead duplicate PKZIP/ Castagnoli classes), HlpCRC32Fast (whole UInt32 fast-path ecosystem incl. both Reflected32 SSE2 kernels), and HlpCRCFoldConstants (GF(2) math merged into HlpCRCCore). Factory named methods now route through the generic engine; on carry-less-multiply CPUs they already used the same kernel. - All 108 standards live in a single const table (params + aliases); replaces the 460-line case and per-class parameter duplicates. - Unified width 3..64 path: bit-serial engine deleted; MSB-first CRCs narrower than 8 bits run left-aligned at width 8 and un-shift at finalization; reflected CRCs are width-agnostic as-is. Fixes: - TransformFinal truncated widths 17-23 to 16 bits (live for CRC-17/CAN-FD); output is now sized by the constructor's HashSize ranges and the width-21 special case is gone. - Table cache was a class-level TDictionary mutated without a lock; now guarded by a critical section. - Fold state handoff uses TBinaryPrimitives.LoadUInt64/StoreUInt64 and a single UInt64 (kernels never touched the second qword). - Forward SSE2 kernels (x86_64 + i386) hoist width/mask/byte-count out of the per-16-byte-block loop. fold-by-8 kernels (128 bytes/iter for inputs >= 128B): - PCLMUL x86_64 reflected + forward gain an 8-accumulator upper path (xmm0-3 + xmm8-11) using the runtime-generated Fold_8x128 stride-1024 constants, reducing 8->4->1 into the untouched Barrett tail. New REX-prefixed pclmulqdq/pshufb encodings verified against GNU as. - PMULL aarch64 kernels regenerated with the same structure (v19-v22); all 32 new opcodes cross-checked against aarch64-linux-gnu-as. Tests: ChunkedData lengthened to 299 bytes so chunked tests cross- validate table/fold4/fold8; new fold-boundary sweep (22 lengths x 108 standards) against the byte-table reference.
* Rename kernels to their true minimum ISA and add aarch64 pure-GPR fallbacks
Naming pass: a kernel's name now states the ISA its body actually
requires, not the dispatch slot it serves - pure-GPR bodies are named
Gpr, VEX-128-only bodies Avx.
- KeccakF1600Sse2(+Absorb)_x86_64 -> KeccakF1600Gpr(+Absorb)_x86_64
- CRCFoldForwardSse2_{i386,x86_64} -> CRCFoldForwardGpr_*
- SHA1/SHA256CompressAvx2_i386 -> *CompressAvx_i386
- Blake2SCompressAvx2_{i386,x86_64} -> Blake2SCompressAvx_*
- Blake3CompressAvx2_x86_64 -> Blake3CompressAvx_x86_64
- ScryptSalsa8Avx2_{i386,x86_64} -> ScryptSalsa8Avx_*
- backend procs/wraps follow; banner comments list the dispatch tiers,
kernel headers no longer mention them
- Reflected-CRC keeps its Sse2 name (SSE2 genuinely required for the
128-bit loads and the CRC state shuttle) - headers now say so
New aarch64 fallbacks for the Crypto-Extensions-only hashes: SHA-1/
SHA-256/SHA-512 compress and Keccak-f[1600] permute + multi-block absorb
gain baseline-AArch64 GPR kernels dispatched at the NEON slot when
FEAT_SHA1/SHA256/SHA512/SHA3 are absent - previously scalar Pascal.
Bodies are the OpenSSL CRYPTOGAMS plain paths, .long-encoded and
verified byte-exact against the GNU-as reference objects.
- SHA-256/SHA-512 use sentinel-terminated K tables (K256_Gpr/K512_Gpr
with a zero terminator - the schedule loop ends on a zero K word);
SHA-1 needs no table (movz/movk round constants)
- Keccak's original round loop ends on a 256-aligned iotas table, which
a Pascal const cannot guarantee; the termination is patched to an
end-pointer compare so both kernels reuse the plain RC table
* force neon mode
* Cut NEON rotates to 2-op insert forms across the Blake/Argon2 kernels
Every 32/64-bit lane rotate in the aarch64 NEON kernels was 3-op
(ushr+shl+orr; Blake3's rot12/rot7 were even 4-op through a copy).
The 2-op insert forms (ushr+sli, shl+sri, add+sri for ror63) land the
result in a temp, so the generators now thread a register map: the
rotated row renames into the temp instead of copying back, and rows
normalize to their home registers only before writebacks and loop
back-edges. Blake3 hash4's G is restructured 26 -> 21 ops with zero
move instructions (b ping-pongs v_b -> t13 -> v_b; d lands in the
register whose message word is dead).
Instruction counts: Blake2S 632->573, Blake2B 1377->1233, Blake3
compress 714->645, Blake3 hash4 1653->1373 (-17%, the multi-chunk
throughput path), Argon2 -12 ops per round slice (x16 executed per
FillBlock). tbl-rot8 deliberately not used: official BLAKE3 NEON uses
shl+sri and there is no mask setup or extra register cost.
* Fix XXH3 SIMD kernel bugs exposed by new long-input known-answer tests
The XXH3 test vectors all stayed under 240 bytes, so the SIMD
accumulate/scramble/initSecret paths were only ever checked for
self-consistency - a wrong kernel agreed with itself and passed. Four
known-answer tests (values cross-checked against the official xxHash
implementation) now cover accumulate512 (299 B), scrambleAcc (2 KiB)
and the seeded long path (299 B with MaxUInt64 and with an asymmetric
key) on every target.
They surfaced three shipped bugs:
- aarch64 NEON Acc512/Scramble: five vector encoders carried a stray
bit in the source-register field, so every emitted uzp1/uzp2/umlal/
umlal2/mul executed with source register 2|m. Acc512 mixed the raw
secret into accumulator lanes 2-3/6-7; Scramble multiplied by
uninitialized registers. The mnemonic comments were correct - the
encoded words were not.
- i386 InitSecret (SSE2 + AVX2): the 64-bit seed materialization
pushed lo before hi, leaving the high dword at [esp], so movq loaded
a dword-swapped qword (-seed became 2^32 instead of 1).
- InitSecret's seed parameter itself: a by-value UInt64 third
parameter never rides in ecx under the i386 register convention, so
the shared Proc3 prologue handed the kernels an undefined register -
one compiler worked only by caller-codegen accident. The seed now
travels by pointer (ASeedPtr: PUInt64), the same pattern as the
Blake2 counter/flags pointers, and every kernel dereferences it on
entry ([edi] on i386, mov r8,[r8] on x86_64, ldr x2,[x2] on NEON).
* Fix big-endian custom secret in scalar XXH3 and make the acc domain explicit
XXH3_InitSecret_Scalar wrote the derived secret with native PUInt64
stores while every consumer reads the secret as little-endian wire
bytes - on big-endian targets the custom secret came out byte-swapped,
so seeded hashing of inputs over 240 bytes was wrong (caught by the
seeded long-input known-answer tests on the big-endian CI; the
unseeded ones passed since that path copies the default secret
byte-for-byte). The custom-secret stores now go through
WriteUInt64LittleEndian.
XXH3_Accumulate512_Scalar / XXH3_ScrambleAcc_Scalar were already
endian-correct (the accumulator is native value data, read and written
natively everywhere including mergeAccs), but their raw PUInt64
pointer derefs looked identical to the buggy pattern. They now use
TBinaryPrimitives.LoadUInt64/StoreUInt64 (native-order, alignment-safe)
so wire accesses (Read/WriteUInt64LittleEndian) and value accesses
(Load/StoreUInt64) are distinguishable at a glance.
* Restructure X86 backends to single-wrap kernel declarations
Each SIMD kernel wrapper was declared twice - once per architecture
section - so signature changes had to be made in two places and could
drift. Following the CryptoLib4Pascal backend style, every kernel now
has ONE declaration with per-arch {$IFDEF} selecting the prologue and
body includes (x86_64 first), and pure-Pascal wraps that were
identical across architectures are shared.
Genuinely per-arch code keeps grouped arch sections: SHA3 (different
kernel sets per arch), the SHA-1/SHA-256 ShaNi pairs (i386 is a
4-param proc with the doubled-K table, x86_64 a 5-param proc with the
byte-swap mask) and the i386 Avx / x86_64 Avx2 wrap pairs.
* enforce file format at commit time A new scripts/maintenance/check-file-format.ps1 verifies CRLF, strict UTF-8 and no BOM across the source/text tree. * Normalize source file format and unify the SIMD kernel headers Phase A - format: 74 files converted to CRLF (10 x86_64 kernel includes, 29 library units, one test unit, the scripts tree). The repo encoding rule is valid UTF-8 without BOM; a new scripts/maintenance/check-file-format.ps1 enforces CRLF + strict UTF-8 + no BOM and the pre-commit hook chains it after the duplicate-GUID check. Phase B - headers: all 107 kernel .inc headers now follow one canonical shape: title, strategy notes, "ABI (after HlpSimdProcNBegin_<arch>.inc): ..." with layout sub-lines, register map, frame/saves, the encoding one-liner, and "Reference:" always last. Typographic punctuation in the Simd tree normalized to ASCII (BOM-less sources render non-ASCII as mojibake in the IDEs). The Common/CpuFeatures prologue includes keep their structure - per- compiler adaptation is their purpose - with punctuation normalized. Every aarch64 header was changed through its generator constant, so regeneration stays byte-idempotent (asm-check passes for all 15 generated kernels). Two stale headers were corrected: Blake2B NEON now documents its d8-d15 save/restore via the shared include, and Blake3 hash4 NEON's duplicated G-temps map lines are merged. * Silence FPC's below-esp warning in the SHA-256 i386 kernels FPC flags "lea esp, [esp - $60]" with "Use of -offset(%esp), access may cause a crash or value may be lost" in the SSE2/SSSE3/AVX SHA-256 i386 kernels. The warning is a false positive - lea is address arithmetic, not a memory access; the line is the CRYPTOGAMS flag-neutral spelling of a stack allocation and nothing ever dereferences below esp. Spelled it "sub esp, $60" instead: no flag consumer lives between the allocation and the next flag-setting instruction at any of the three sites, so the forms are equivalent - and the recurring build noise is gone.
Blake2B_Compress_Scalar / Blake2S_Compress_Scalar accessed the state, IV, counter-flags and message words through raw PUInt64/PUInt32 pointer derefs. All four buffers are native value data (the caller converts the message block to native words before the call), so the accesses now go through TBinaryPrimitives.LoadUInt64/StoreUInt64 and LoadUInt32/StoreUInt32 - behavior-identical, alignment-safe, and the wire-vs-native domain is visible in the API used, consistent with the XXH3 scalar cleanup.
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
1 participant
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
Replaces the per-algorithm
*Dispatch.pasunits with a Facade/Backend SIMD architecture underHashLib/src/Simd/, consolidates CRC into a single generic engine with fold-by-8 carry-less kernels, expands x86 SIMD tier coverage (especially i386), and fixes several XXH3/CRC correctness issues found by new tests.Core hash units now call
Hlp<Hash>Simd.Select(...)once at init and hold a function pointer — noTCpuFeaturesorHASHLIB_*_ASMknowledge in the cores, and zero per-block dispatch cost.SIMD architecture
Introduces
HashLib/src/Simd/:Facade/Hlp<Hash>Simd.pasBackend/Hlp<Hash>X86Backend.pasSelectBackend/Hlp<Hash>ArmBackend.pasSelectApplies to all 12 SIMD primitives: SHA-1, SHA-256, SHA-512, SHA-3, Blake2B, Blake2S, Blake3, XXH3, Adler32, Argon2, Scrypt, and CRC fold.
Removed:
Hlp*Dispatch.pasfor every primitive above.CRC consolidation
Engine reduced from 6 units to 3 (~1,800 lines removed):
HlpCRCStandard,HlpCRC32Fast,HlpCRCFoldConstants,HlpCRCDispatchHlpCRCengine +HlpCRCCore(GF(2) fold math merged in)Bug fixes:
TransformFinalno longer truncates widths 17–23 to 16 bits (affected CRC-17/CAN-FD)TDictionary)TBinaryPrimitives.LoadUInt64/StoreUInt64fold-by-8 kernels (128 bytes/iteration for inputs ≥ 128 B):
aarch64-linux-gnu-asTests:
ChunkedDatalengthened to 299 bytes; new fold-boundary sweep (22 lengths × 108 standards) against byte-table reference.x86 SIMD expansion
Improvements
Gpr,Avx, not dispatch-slot labels); KeccakSse2→Gpron x86_64 where appropriateASeedPtr)WriteUInt64LittleEndianTooling
scripts/maintenance/check-file-format.ps1(CRLF, strict UTF-8, no BOM); chained in pre-commit.incheaders normalized to a canonical shapeBreaking changes
Hlp*Dispatch.pas,HlpCRC32Fast,HlpCRCStandard,HlpCRCFoldConstants— package/project references updated