perf(core): tune the AVX-512 Keccak-f[1600] permutation - #12844
perf(core): tune the AVX-512 Keccak-f[1600] permutation#12844benaadams wants to merge 5 commits into
Conversation
|
Claude finished @benaadams's task in 7m 40s —— View job Review:
|
| row | lane 0 | lane 1 | lane 2 | lane 3 | lane 4 |
|---|---|---|---|---|---|
| 0 | c0[0] |
c1[1] |
c2[2] |
c3[3] |
c4[4] |
| 1 | c0[3] |
c1[4] |
c2[0] |
c3[1] |
c4[2] |
| 2 | c0[1] |
c1[2] |
c2[3] |
c3[4] |
c4[0] |
| 3 | c0[4] |
c1[0] |
c2[1] |
c3[2] |
c4[3] |
| 4 | c0[2] |
c1[3] |
c2[4] |
c3[0] |
c4[1] |
Specifically checked and clean:
- Starting each chain from its own row (the register-copy saving) is sound. The old
c1Pichain beganPermuteVar8x64x2(c0, Create(3UL, 8+4, …), c1); the new one begins fromc1and foldsc0[3]in as the last step. Readingc0-c4in those later steps is safe because the batch assignment (c0 = c0Pi; c1 = c1Pi; …) was moved to after all five chains complete.masterinterleaved those assignments into thec4Pichain, which was the fragile part — the new ordering is strictly easier to reason about. - Lanes 5-7 stay inert through Theta (index vectors never map ≥5 into 0-4), Rho (rotate-by-0), Pi (all lane-0-4 indices are
<5or8 + <5) and Chi. They are also always overwritten by the overlapping stores at 0/5/10/15/20/24. - All memory accesses stay inside 25 elements for a well-formed state: the
Vector512reads at 0/5/10/15 touch 0-7, 5-12, 10-17, 15-22. LaneShift1genuinely is both Theta's[1,2,3,4,0]and Chi'spermute1— that dedup is valid.Vector512.Create(8 + 2, 1, 2, 3, 1UL, …)resolves to theulongoverload (ulong→ulongidentity beatsulong→double), so no silent lane-width surprise.- Renaming
KeccakF1600→KeccakF1600Scalarand wideningprivate→internalis safe: no other callers in the repo, andNethermind.Core.TesthasInternalsVisibleTo(InternalsVisibility.cs:6).
Findings
🔴 High — Debug.Assert guard on a public API is elided in Release
KeccakF1600Avx512F is public. Replacing _ = state[24] with Debug.Assert(state.Length == 25) means Release builds have no guard, while the body now uses MemoryMarshal.GetReference + Unsafe.Add throughout. A caller passing a short span gets 64-byte OOB writes at offsets 0/5/10/15 — silent heap corruption instead of the IndexOutOfRangeException master throws. The cost being saved is a single predictable compare against 24 rounds of AVX-512 work. Either restore a real check with a [DoesNotReturn] throw helper, or narrow the method to internal (breaking).
🟠 Medium — RoundConstantVec[round] may cost more than the vmovq it removes
KeccakHash.std.cs:37-38 → used at line 348
Per round this is a static-base load + an array bounds check the JIT can't eliminate (RoundConstantVec.Length isn't a JIT constant) + a 64-byte load from an 8-byte-aligned heap array that will frequently straddle a cache line. That's traded against one 8-byte vmovq. Suggested fix in the inline comment: hoist MemoryMarshal.GetArrayDataReference out of the loop and pass the Vector512<ulong> into Round by value.
🟠 Medium — perf-only PR with no measurements
Every claim in the description ("removes the per-round scalar broadcast", "saving a register copy per row", "kills the per-round vmovq") is a plausible-but-unverified microarchitectural assertion, and at least two of them look like they could go the other way. The repo has Benchmarks.slnx for exactly this. Please add before/after BenchmarkDotNet numbers (and ideally the relevant disasm) — for a 123/-186 diff to hot crypto code, that's what makes the tradeoffs reviewable.
🟡 Low
- Redundant
mask/ fourvpandq— provably dead given the overlapping stores; a free win the tuning pass missed. [SkipLocalsInit]onRoundis a no-op, plus a note on the spill risk if the fiveref Vector512<ulong>params ever fail to inline.- Inconsistent constant strategy — Theta/Chi indices hoisted to
static readonlyfields while Rho and all Pi indices stay inline; inlineVector512.Createof constants folds to an rodata load, a static field adds an indirection. - Index-vector readability and test coverage gaps (no canary for the write extent; differential-only, no known-answer vector).
Style / repo rules
No violations of coding-style.md that I found: no var, no LINQ, no #region, Array.Empty/collection-expression rules respected. The multi-statement lines (c0 = …; c1 = …; at 302-303, 338) are unusual for this codebase but dotnet format whitespace won't object — reviewer's call.
robustness.md §Safety asks that unsafe blocks carry a justifying comment. There's no unsafe keyword here, but the Unsafe.As-based reads/writes are the same class of hazard, and this PR removed the comments that documented the invariant (// Can straight load and over-read for start elements, // Can't over-write for last elements…). Since the guard is gone too, a short remark on the entry point stating the state.Length == 25 precondition and the over-read/overlapping-store scheme would be worth keeping.
What I could not verify
This review is static analysis only:
- No AVX-512 hardware — the runner is an AMD EPYC 7763 (Zen 3, AVX2 only), so
Avx512_permutation_matches_scalarwouldAssert.Ignorehere even if I could run it. I'm relying on the author's reported 1051/1051 on AVX-512 hardware, plus the symbolic trace above. dotnet build/dotnet test/ scripting were not permitted in this session (permission prompts denied), so I could not compile the branch or rundotnet format whitespace. If you want those executed on future runs, they need adding to--allowedTools.
· branch perf/keccak-avx512
- Restructure into a shared inlined Round helper with shared lane-index vectors for Theta/Chi and pre-broadcast round constants (removes the per-round scalar broadcast) - Start Pi permute chains from their own column, saving a register copy per row - Guard the state length with Debug.Assert, matching the scalar path, and access all lanes unchecked - Rename the portable path to KeccakF1600Scalar (internal) and add an AVX-512-vs-scalar equivalence test
03926d0 to
bc68a29
Compare
There was a problem hiding this comment.
Pull request overview
Optimizes the x64 AVX-512 Keccak-f[1600] permutation in Nethermind.Core by restructuring the permutation into an inlined round helper, sharing permute-index vectors, and pre-broadcasting round constants; also renames/exposes the scalar reference path for testing and adds an AVX-512-vs-scalar equivalence test.
Changes:
- Refactors the AVX-512 permutation into an inlined
Roundhelper and introduces shared lane-index vectors plus pre-broadcast round constants. - Renames the portable permutation implementation to
KeccakF1600Scalar(internal) and updates dispatch accordingly. - Adds an AVX-512 vs scalar permutation equivalence test.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Nethermind/Nethermind.Core/Crypto/KeccakHash.std.cs | AVX-512 Keccak-f[1600] round refactor + scalar rename + constant/vector hoisting. |
| src/Nethermind/Nethermind.Core.Test/KeccakTests.cs | Adds an AVX-512 permutation equivalence test against the scalar reference. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
The method skips bounds checks under a Debug.Assert length contract, so restrict callers to the assembly and friend test assemblies, matching KeccakF1600Scalar.
- Lanes 5-7 of the over-read row loads never reach result lanes 0-4 (Theta/Rho/Pi/Chi map lanes 0-4 only from lanes 0-4) and the stores overwrite them, so the vpandq masking was dead - Pass the pre-broadcast round constant into Round via GetArrayDataReference, eliminating the round + 1 bounds check Tier-1 body shrinks 1562 -> 1528 bytes with no range checks left.
|
@claude re-review |
|
Claude finished @benaadams's task in 3m 30s —— View job Re-review:
|
| # | Sev | Finding | Status |
|---|---|---|---|
| 1 | 🔴 High | Debug.Assert guard on a public API |
Resolved — d1e1404 narrows to internal |
| 2 | 🟠 Med | RoundConstantVec[round] indirection |
Resolved — hoisted in 04c2649; disasm rebuts the rest |
| 3 | 🟠 Med | Perf-only PR with no measurements | Answered — Tier-1 disasm + EXPB run |
| 4 | 🟡 Low | Dead mask / four vpandq |
Applied in 04c2649 |
| 5 | 🟡 Low | [SkipLocalsInit] no-op / inline risk |
Answered — both call sites confirmed inlined |
| 6 | 🟡 Low | Inconsistent constant strategy | Answered — both forms hit JIT rodata; my premise was wrong |
| 7 | 🟡 Low | Test coverage gaps | Deferred with rationale |
On #1: internal is the right call. Confirmed the only two call sites are the KeccakF dispatch (KeccakHash.std.cs:45) and the test via InternalsVisibleTo, both passing exactly 25 lanes, so the unchecked body is unreachable from outside the assembly and Debug.Assert is now a defensible contract check rather than a hole.
On #6: your disasm shows ThetaRot4/LaneShift1/LaneShift2 loading from [reloc @RWD…] exactly like the inline Creates — the frozen-object folding I claimed wouldn't happen does happen. My analysis was wrong there; the mixed strategy costs nothing.
Re-verification of 04c2649 (mask removal)
This is the commit that can silently corrupt hashes, so I re-derived the invariant rather than taking the passing tests as proof. It holds. Lanes 5-7 are a closed set — nothing in a round moves them into lanes 0-4:
- Theta —
ThetaRot4,LaneShift1,LaneShift2at result positions 0-4 select source lanes{4,0,1,2,3},{1,2,3,4,0},{2,3,4,0,1}; all< 5. - Rho —
RotateLeftVariableis element-wise. - Pi — checked all 20
PermuteVar8x64x2index vectors (lines 315-338); every entry at positions 0-4 isnor8 + nwithn < 5. - Chi —
LaneShift1/LaneShift2again. - Iota —
Vector512.CreateScalar(rc)zeroes lanes 1-7, so garbage inc0's upper lanes cannot leak into lane 0.
And the store sequence (lines 286-291) is exhaustive and correctly ordered — each garbage triple is overwritten by the next row's store:
| store | writes | garbage left | fixed by |
|---|---|---|---|
c0 @0 |
s[0..7] | s[5..7] | c1 store |
c1 @5 |
s[5..12] | s[10..12] | c2 store |
c2 @10 |
s[10..17] | s[15..17] | c3 store |
c3 @15 |
s[15..22] | s[20..22] | c4 v256 store |
c4 v256 @20 + scalar @24 |
s[20..24] | — | — |
Max write extent is s[22], max read extent s[24] — both inside 25, so with the Length == 25 contract there is no OOB access in either direction. Since all five rows are loaded before any store, the overlap is safe.
Also checked the hoist itself: MemoryMarshal.GetArrayDataReference yields a GC-tracked interior byref, so it survives a relocation across the 24-round loop — no pinning needed.
New finding
🟡 Low — ROUNDS / table-length coupling is now unguarded
Unsafe.Add(ref roundConstants, round + 1) drops the array bounds check that was latent protection against ROUNDS and RoundConstantVec.Length drifting apart (and against ROUNDS becoming odd). Both assumptions hold today. The asymmetry is the concern: the scalar path still indexes RoundConstants[round] and would throw, while this path would silently read past the array and emit a wrong-but-well-formed digest. Debug.Assert(RoundConstantVec.Length >= ROUNDS && ROUNDS % 2 == 0) next to the existing assert closes it for free.
Also informational, no action needed: public → internal is technically a surface-breaking change to Nethermind.Core while the PR's "Breaking change" box is unticked. For a raw permutation primitive with no in-repo external callers that's the right trade, just noting the checkbox mismatch.
Benchmarks
The EXPB run is neutral across all three payload sets — fusaka -1.43% AVG, realblocks +0.28%, superblocks +0.03%, all inside run-to-run noise. That is the expected shape: Keccak is a thin slice of end-to-end block processing, so EXPB can't confirm the micro-optimization, but it does confirm no regression, which combined with the Tier-1 disasm is enough for a change this size.
Correction to my previous review
I said the runner was "an AMD EPYC 7763 (Zen 3, AVX2 only)" and that the equivalence test would Assert.Ignore here. That was wrong — this runner is an AMD EPYC 9V74 (Zen 5) and /proc/cpuinfo reports the AVX-512 flags, so Avx512_permutation_matches_scalar would genuinely execute in this environment. It matters because it means CI on these runners does cover the AVX-512 path, rather than silently skipping it.
What I still could not verify
dotnet build and dotnet test remain outside --allowedTools (both denied this session), so this is again static analysis only — no compile, no test execution, no dotnet format whitespace. Given the hardware above, adding those two to the allowlist would let a future run actually execute the differential test rather than reason about it.
· branch perf/keccak-avx512
Unsafe.Add dropped the latent bounds check tying RoundConstantVec to the loop, so assert the length and evenness next to the state assert. Bounding the loop by RoundConstantVec.Length instead was measured with JitAsm and rejected: the non-const bound stops the JIT hoisting the 28 vector constants, re-loading all of them from rodata every iteration (loop grows 143 -> 165 instructions per 2 rounds).
|
@claude re-review, note PR description has been updated |
|
Claude finished @benaadams's task in 5m 0s —— View job Re-review (3rd pass):
|
| # | Sev | Finding | Status |
|---|---|---|---|
| 1 | 🔴 High | Debug.Assert guard on a public API |
Resolved — internal (d1e1404) |
| 2 | 🟠 Med | RoundConstantVec[round] indirection |
Resolved — hoisted (04c2649) |
| 3 | 🟠 Med | No measurements | Answered — Tier-1 disasm table now in the description + EXPB run |
| 4 | 🟡 Low | Dead mask / vpandq |
Applied (04c2649) |
| 5 | 🟡 Low | [SkipLocalsInit] no-op |
Answered — both call sites inline |
| 6 | 🟡 Low | Constant strategy | Answered — my premise was wrong |
| 7 | 🟡 Low | ROUNDS / table coupling |
Applied (6e309b0) |
6e309b0 review
Debug.Assert(RoundConstantVec.Length == ROUNDS && ROUNDS % 2 == 0) — correct and sufficient. ROUNDS % 2 == 0 folds to true at compile time for today's value, but it still fires at runtime in Debug if someone sets ROUNDS = 25, so it isn't inert as a drift guard. Length == ROUNDS (rather than my suggested >=) is the stronger and better choice here since the table is derived from RoundConstants — they should track exactly.
The rejection of the RoundConstantVec.Length loop bound is well-argued and matches the pre-existing // Use constant for loop so Jit expects to loop comment on master (KeccakHash.std.cs:280 there). Recording the why in a comment rather than silently keeping ROUNDS is the right call — that's exactly the kind of non-obvious constraint AGENTS.md wants a comment for.
Independent re-derivation of the permutation
This pass I checked the code against FIPS-202 directly rather than only diffing against master, since a diff-only argument can't catch a bug the restructure inherited. All five steps check out:
- Theta —
parity = TernaryLogic(TernaryLogic(c0,c1,c2,0x96), c3, c4, 0x96);0x96is 3-input XOR, soparity[x] = C[x].ThetaRot4 = (4,0,1,2,3)givesC[x-1];LaneShift1 = (1,2,3,4,0)rotl-1 givesrot(C[x+1],1).D[x] = C[x−1] ⊕ rot(C[x+1],1)✓ - Rho — offsets per row (
c_yholdsA[0..4][y]):
c0: 0,1,62,28,27·c1: 36,44,6,55,20·c2: 3,10,43,25,39·c3: 41,45,15,21,8·c4: 18,2,61,56,14— exact match to the FIPS-202r[x][y]table ✓ - Pi — re-derived all 20
vpermt2qchains (res[i] = idx[i] < 8 ? a[idx[i]] : b[idx[i]−8]):
row0=(c0[0],c1[1],c2[2],c3[3],c4[4]),row1=(c0[3],c1[4],c2[0],c3[1],c4[2]),row2=(c0[1],c1[2],c2[3],c3[4],c4[0]),row3=(c0[4],c1[0],c2[1],c3[2],c4[3]),row4=(c0[2],c1[3],c2[4],c3[0],c4[1])✓
Each chain's intermediate lanes survive to the end — verified position-by-position, including thec4Pichain whereCreate(8+2, 1, 2, 3, 1UL, …)seeds pos4 fromc4[1]and never touches it again. - Chi —
TernaryLogic(a, a[x+1], a[x+2], 0xD2). Truth table forA ⊕ (¬B ∧ C)is1101 0010=0xD2✓ - Iota —
Vector512.CreateScalar(rc)zeroes lanes 1-7, so the unmasked garbage inc0[5..7]can't leak into lane 0 ✓
Memory extent, exhaustively: max read index is 24 (c3 load reads s[15..22], c4 reads s[20..23] + s[24]), max write index is 24. Nothing touches memory outside the 25-lane state in either direction — the "over-read" in the new comment is over-read of neighbouring rows, never past the array. So the mask removal is safe under a weaker precondition than I assumed last pass.
Overload resolution on the mixed-literal Create calls — re-checked the corner case Vector512.Create(8 + 2, 1, 2, 3, 1UL, 5, 6, 7) where the first arg is an int literal. Create(long,…) is inapplicable (ulong→long has no implicit conversion); between Create(ulong,…) and Create(double,…), ulong is the better conversion target (§12.6.4.7: ulong→double exists, double→ulong does not), so the ulong overload wins for every argument. No silent Vector512<double> reinterpretation.
Debug.Assert(state.Length == 25) — contract verified end-to-end
Traced all five KeccakF call sites (KeccakHash.cs:109,129,217,242,294) to their state buffers:
KeccakHash.cs:86—stackalloc ulong[STATE_SIZE / sizeof(ulong)]= 25Pool.RentState()(KeccakHash.cs:501) —new ulong[STATE_SIZE / sizeof(ulong)]= 25, and the return path (ReturnState) re-enqueues the same arrays, so the thread-static queue can only ever hold length-25 arrays. Notably this is a hand-rolled queue, notArrayPool.Rent, so there's no over-sized-buffer path that would makeLength == 25false.
So the exact-equality assert is satisfiable by construction. (The scalar path already carried this assert on master, so it was implicitly proven — but the AVX path previously accepted Length >= 25, so it was worth confirming rather than assuming.)
Description accuracy
Every bullet matches the code as merged, and the stat line (+131/−189) matches git diff origin/master...HEAD. The disasm table is internally consistent with 6e309b0's commit message (143 instructions/2 rounds is both the "after" figure in the table and the baseline in the rejected-alternative measurement). Two things the description doesn't mention, both harmless: the new RoundConstantVec.Length assert, and the const-loop-bound rationale comment.
The "Breaking change" checkbox is still unticked despite public → internal on KeccakF1600Avx512F. As noted last pass, for a raw permutation primitive with no in-repo external callers this is the right trade — just flagging that the checkbox drives the automatic PR labelling, so if you want a breaking change label it won't appear.
Informational — new coupling between the test and the .std build flavour
KeccakTests.cs:198-199 now references KeccakF1600Scalar / KeccakF1600Avx512F directly. Both live only in KeccakHash.std.cs, which src/Nethermind/Directory.Build.targets:5-10 excludes when EnableZkEvm=true, while KeccakTests.cs itself is always compiled. So dotnet build -p:EnableZkEvm=true on Nethermind.Core.Test would now fail where it previously wouldn't.
I checked and no CI workflow does this — nethermind-tests.yml's zkevmTest is an EEST fixture type, unrelated to the EnableZkEvm build property, which is only used for the ZisK guest build (the runner, not the test projects). So there is nothing to fix; noting it only because it's a new constraint that didn't exist before this PR.
Benchmarks
EXPB is neutral across all three payload sets (fusaka −1.43%, realblocks +0.28%, superblocks +0.03% AVG) — inside noise, as expected for a permutation that's a thin slice of block processing. Combined with the Tier-1 disasm table in the description (1724→1528 bytes, 247→222 instructions, zero range checks), that's adequate evidence for a change of this size: the micro-level win is measured, and the macro level confirms no regression.
What I could not verify
Static analysis only again — dotnet build and dotnet test were both denied this session (permission prompt on dotnet build src/Nethermind/Nethermind.Core/Nethermind.Core.csproj -c Release), so no compile, no test execution, no dotnet format whitespace. Since these runners do report AVX-512, adding those to --allowedTools would let a future pass actually execute Avx512_permutation_matches_scalar rather than reason about it.
· branch perf/keccak-avx512
EXPB Benchmark ComparisonRun: View workflow run superblocksScenario: Client Processing (SSE)
K6 TTFB
realblocksScenario: Client Processing (SSE)
K6 TTFB
fusakaScenario: Client Processing (SSE)
K6 TTFB
|
|
@benaadams any actual benchmarks on avx-512 hardware (Zen 5) you can post? |
- Take the Keccak-f[1600] state as `ref ulong` in the internal AVX-512 and scalar permutations; the KeccakF dispatch extracts the ref once, so call sites stop materializing a Span for every permutation call - Replace the stackalloc state in ComputeHash with an [InlineArray] struct local: localloc pinned the method at Tier0-FullOpts (no tiering, dynamic PGO, or inlining) and added GS-cookie and stack-probe overhead per call; as a struct local it tiers to Tier1 and inlines into ValueKeccak.Compute, folding the round size, padding index, and output copy for 32-byte outputs - Add [SkipLocalsInit] to the hot wrappers (ValueKeccak.Compute, InternalCompute, ComputeHash, Update, UpdateFinalTo, GenerateValueHash), removing frame zero-init that Unsafe.SkipInit alone does not skip - Drop `checked` from GetRoundSize: the entry guard bounds the output length to [1, 200], so the arithmetic cannot overflow Verified with DOTNET_JitDisasm at Tier-1: ComputeHash now reaches Tier1 (was permanently Tier0-FullOpts) with an rsp frame, no GS cookie check, no stack probe, and no overflow branches; permutation call sites pass the state pointer directly with no span stores. Wall-clock is unchanged within noise for 20-532 byte inputs (the permutation dominates at ~240 ns per block). Tests: KeccakTests 1051/1051 on x64 with AVX-512, including the AVX-512-vs-scalar equivalence test; Nethermind.Core also builds with -p:EnableZkEvm=true against the unchanged zkevm KeccakF partial.
|
@LukaszRozmej measured on a Ryzen 9 9950X (Zen 5, full AVX-512), .NET 10.0.11, Windows 11, BenchmarkDotNet 0.15.8 (DefaultJob, MemoryDiagnoser):
Zero allocations on both sides; every delta is inside this box's 2-4% run-to-run noise floor, in both directions - so no wall-clock win claimed. A single f[1600] permutation costs ~243 ns on Zen 5 either way and dominates everything below the 136-byte rate. The permutation is latency-bound on the serial Theta-Rho-Pi-Chi-Iota dependency chain, and what this PR removes (the per-round constant rebuild, the lane masks, the bounds check) sits off that critical path where Zen 5's execution width was already hiding it - hence the gains land in the codegen table in the description (smaller body and frame, zero range checks), not in time. A port-narrower AVX-512 implementation might convert some of it into time; not measurable on this machine. Also pushed a follow-up commit cleaning up the outer methods ( |
Changes
KeccakF1600Avx512Finto a shared inlinedRoundhelper with shared lane-index vectors for Theta/Chi and pre-broadcast round constants (removes the per-round scalar broadcast)Round, eliminating the last bounds checkKeccakF1600Avx512Finternal(it skips bounds checks under a documented 25-lane contract; theKeccakFdispatch and tests are the only callers) and rename the portable path toKeccakF1600Scalar(internal)ref ulongin the internal AVX-512 and scalar entry points; theKeccakF(Span<ulong>)dispatch extracts the ref once (the ZK_EVM partial is untouched), so call sites stop materializing aSpanfor every permutation callstackallocstate inComputeHashwith an[InlineArray]struct local: localloc pinned the method permanently at Tier0-FullOpts (no tiering, no dynamic PGO, ineligible for inlining) and added a GS-cookie check and stack probe per call[SkipLocalsInit]to the hot wrappers (ValueKeccak.Compute,InternalCompute,ComputeHash,Update,UpdateFinalTo,GenerateValueHash) -Unsafe.SkipInitalone does not skip the frame zero-initcheckedfromGetRoundSize: the entry guard bounds the output length to [1, 200], so the arithmetic cannot overflowTypes of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?
Notes on testing
Avx512_permutation_matches_scalarchecks the AVX-512 path against the scalar reference over 64 states (all-zero, all-ones, pseudo-random lanes) and executes for real on AVX-512 hardware; it now drives theref ulongentry points directlyKeccakTestssuite: 1051/1051 passed on x64 with AVX-512, re-verified after each codegen cleanup and again after the outer-method commitValueKeccak.Computeover the same input produces byte-identical hashes on a master build and this branchNethermind.Corealso builds with-p:EnableZkEvm=trueagainst the unchanged zkevmKeccakFpartialDocumentation
Requires documentation update
Requires explanation in Release Notes
Remarks
Measured Tier-1 codegen, same machine (x64/AVX-512), same .NET 10 runtime,
DOTNET_JitDisasmafter driving the method to Tier-1 throughValueKeccak.Compute:KeccakF1600Avx512Fvmovq+vpinsrq/vinserti64x4+vxorpsrebuildvpxordwith a memory operandRNGCHKFAILblocksThe core vector work is unchanged (per 2 rounds: 40 permute-pair ops, 24
vpermq, 14vpternlogq, 10vprolvqon both sides); the savings are the per-round constant rebuild, the masking, and the checks. BothRoundcall sites inline into a straight-line vector loop with all five state rows resident in zmm registers - no calls remain in the Tier-1 body. The frozenRoundConstantVeclength folds to an immediate and thestatic readonlyindex vectors fold into JIT rodata exactly like inlineVector512.Createconstants.The outer-method commit was verified the same way: the
stackallochad keptComputeHashat Tier0-FullOpts for the life of the process (localloc blocks tiering, dynamic PGO, and inlining). As an[InlineArray]struct local it reaches Tier-1 and inlines intoValueKeccak.Compute, where the constant 32-byte output folds the round size, eliminates the second padding store's bounds check, specializes the 136-byte absorb loop, and selects the vector output copy statically. The GS-cookie check, stack probe, andcheckedoverflow branches are gone, and permutation call sites pass the state pointer directly instead of building aSpanper call.Wall-clock is unchanged within run-to-run noise (Ryzen 9 9950X / Zen 5, .NET 10.0.11, BenchmarkDotNet 0.15.8,
ValueKeccak.Computeover random inputs, zero allocations on both sides): a single f[1600] permutation costs ~243 ns either way and dominates every input below the 136-byte rate. The permutation is latency-bound on the serial Theta-Rho-Pi-Chi-Iota round chain; the removed work sat off that critical path, so the improvement is code size, frame size, and check elimination rather than time on this microarchitecture.Extracted from #12843, which additionally carries the Arm SHA-3 permutation and the .NET 11 preview bump it requires; this PR is the x64-only part and builds on the current SDK.