Skip to content

Merge HalfKA and Threats accumulators#6890

Closed
anematode wants to merge 3 commits into
official-stockfish:masterfrom
anematode:merge-accumulators
Closed

Merge HalfKA and Threats accumulators#6890
anematode wants to merge 3 commits into
official-stockfish:masterfrom
anematode:merge-accumulators

Conversation

@anematode

Copy link
Copy Markdown
Member

Passed STC (https://tests.stockfishchess.org/tests/view/6a2893ad7c758d82accea129):
LLR: 3.20 (-2.94,2.94) <0.00,2.00>
Total: 23328 W: 6145 L: 5838 D: 11345
Ptnml(0-2): 50, 2463, 6346, 2740, 65

Instead of repeatedly doing the sum HalfKA + threats at the end, it's profitable to simply store one accumulator per side that combines them. This also avoids an extra/load store of an accumulator, and halves the cache footprint of the accumulators.

For full refreshes, we always compute both halfka and threats simultaneously. Any threat full refresh is always a halfka refresh because it occurs when the king crosses the center line, while halfka refreshes are required for ANY king move, so we don't need a separate detection path for threats.

I get about a 2.5% speedup locally with this, but I'd appreciate other ppl's measurements.

No functional change

@github-actions

Copy link
Copy Markdown

(execution 27248272859 / attempt 1)

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 644a7127-5ba3-47d4-902d-084bd347c993

📥 Commits

Reviewing files that changed from the base of the PR and between 6867a8f and b50e231.

📒 Files selected for processing (1)
  • src/nnue/nnue_accumulator.h
💤 Files with no reviewable changes (1)
  • src/nnue/nnue_accumulator.h

📝 Walkthrough

Walkthrough

This PR unifies NNUE accumulator handling by replacing templated PSQ/threat states with a single non-templated AccumulatorState containing DirtyPiece and DirtyThreats. AccumulatorStack APIs (latest/mut_latest/evaluate_side/find_last_usable_accumulator/forward/backward incremental updates) and storage are converted to non-templated, Color-keyed forms. A new apply_combined implements SIMD/scalar combined PSQ+threat delta application. update_accumulator_refresh_cache now refreshes PSQ and threat contributions together and the separate threat-only refresh function was removed. FeatureTransformer::transform() reads psqtAccumulation from the unified latest accumulator and omits threatAccumulation additions.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@anematode anematode force-pushed the merge-accumulators branch from ded8e9a to 5385e3a Compare June 10, 2026 03:25
@github-actions

Copy link
Copy Markdown

(execution 27251005329 / attempt 1)

@github-actions

Copy link
Copy Markdown

(execution 27251205882 / attempt 1)

@github-actions

Copy link
Copy Markdown

(execution 27253689613 / attempt 1)

@vondele

vondele commented Jun 10, 2026

Copy link
Copy Markdown
Member
==== master ====
==== Bench: 3493826 ====
1 Nodes/second : 294691755
2 Nodes/second : 294351857
3 Nodes/second : 296083491
Average (over 3):  295042367
==== merge-accumulators ====
==== Bench: 3493826 ====
1 Nodes/second : 308522687
2 Nodes/second : 307923326
3 Nodes/second : 308319161
Average (over 3):  308255058

@mstembera

Copy link
Copy Markdown
Contributor

I wonder if this makes a larger L1 like say 1280 more promising?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants