From 6c045be8a37a8eae1ea6aed250944e22af2335a1 Mon Sep 17 00:00:00 2001 From: Scott Gasch Date: Fri, 4 Sep 2026 01:27:59 -0700 Subject: Add bbPieces incremental piece-location bitboards (migration plan section 1) Board-representation migration, section 1: add POSITION.bbPieces[2][8] (per-color, per-piece-type location bitboards, indexed like the existing uNonPawnCount) as incrementally-maintained state, not a per-Eval()-call rebuild -- the structural fix for why the earlier attack-presence-bitboard work measured slower, not faster. - chess.h: bbPieces[2][8] field; extern decls for data.c's g_RookRayToEdge/g_BishopRayToEdge/g_KnightAttacksBB ray tables and their Initialize* functions (needed by the planned bitboard-backed GetAttacks/CountKingSafetyDefects primitive, section 3). - fen.c: populate bbPieces during piece placement; zeroing is free via the existing memset(p, 0, sizeof(POSITION)). - move.c: maintain bbPieces at all 6 non-pawn piece-movement functions (SlidePiece/LiftPiece/PlacePiece and their WithoutSigs siblings used by UnmakeMove) -- covers every move type: normal moves, captures, both-side castling, promotion with/without capture, en passant, and every undo. - board.c: extend VerifyPositionConsistency's existing non-pawn piece-list walk with a parallel bbPieces reconstruction-and-compare, rather than a separate bespoke check. - data.c/main.c: pulled ray-to-edge/knight-attack tables from stash (needed by section 3, not section 1 itself, but zero-risk to land now). Also, while verifying: COOR_TO_BB was a table lookup (BBSQUARE[idx]) measured ~5-7% slower than the pure-ALU shift already sitting unused in SLOWCOOR_TO_BB (whose "SLOW" name reflects a stale assumption about variable shifts never actually tested on this hardware). Switched COOR_TO_BB to the shift; fixed testbitboard.c's existing but broken (dead-code-eliminated, silently reporting "0 cycles/op") comparison benchmark for both while at it. Verified via gmake TEST=1 (including TestMakeUnmakeMove's explicit en-passant/promotion-with-capture/both-castling coverage) and debug_smoke_test.sh, both clean; release build clean and runs normally. Nothing reads bbPieces yet -- pure addition, zero behavioral risk. See board_representation/MIGRATION.md for the full plan. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Jntky4yGUTyQVaGCXms4F2 --- src/board_representation/MIGRATION.md | 403 ++++++++++++++++++++++++++++++++++ 1 file changed, 403 insertions(+) create mode 100644 src/board_representation/MIGRATION.md (limited to 'src/board_representation/MIGRATION.md') diff --git a/src/board_representation/MIGRATION.md b/src/board_representation/MIGRATION.md new file mode 100644 index 0000000..226e37e --- /dev/null +++ b/src/board_representation/MIGRATION.md @@ -0,0 +1,403 @@ +# Migration plan: bitboard-backed `GetAttacks` and `CountKingSafetyDefects` + +**Status: section 1 (and its correctness gate, section 2) implemented, +verified, uncommitted on disk. Sections 3-8 not started.** See the +per-section status notes below for specifics. + +Context: a prior pass of this session added occupancy-bitboard-driven +mobility/attack-presence bitboards (`bbPawnAttacks`, `bbMinorAttacks`, +`bbRookAttacks`, `bbQueenAttacks`, `bbKingAttacks`, plus xray variants) to +`POSITION`, rebuilt from scratch inside every `Eval()` call, additive +alongside the pre-existing per-square `rgSquare[c|8].bvAttacks` mailbox +struct. Measured via a dedicated `evalcycles` command (rdtsc-based, +compared against a `git worktree` build of pre-migration HEAD, since none +of this work was committed): the new code is **17-41% slower per `Eval()` +call**, consistently, across 5 representative positions. Root cause: pure +duplication -- every mobility function still does the original mailbox +ray-walk *and* now also writes the bitboards, so every position pays for +both representations and nothing was ever removed to pay for it. + +That result reframed the question: the win isn't "add more bitboards to +Eval," it's picking the *right* bitboards in the *right* place. This +document is the plan for that -- two functions sharing one new bitboard +primitive: + +1. **`GetAttacks`** (the SEE support routine in `see.c` / `x86.asm` / + `x64.asm`), called far more often per node than `Eval()` -- every + capture move considered in move ordering, at every node, via + `generate.c`'s 5 call sites and `searchsup.c:365`. +2. **`CountKingSafetyDefects`** (`eval.c:2408`), which turns out to be an + even more pervasive case of the same problem: it's called not just + from `Eval()` (`eval.c:7386-7387`, `eval.c:2549-2550`) but directly + from search-tree pruning/extension decisions -- `search.c:1252`, + `searchsup.c:484`, `searchsup.c:902,905` -- meaning it runs at search + nodes regardless of whether `Eval()` is even reached at that node. + +Both do the exact same thing algorithmically: walk every one of a side's +non-pawn pieces with a geometry-table lookup (`CHECK_VECTOR`) and, for +sliders, a manual mailbox ray-walk, to answer "does this piece attack (or +point at) this particular square" -- `GetAttacks`'s target is the move's +destination square, `CountKingSafetyDefects`'s target is the king's +square. O(pieces) candidates x O(ray length), every call, regardless of +how close any given piece actually is to the target square. Both are +strong candidates for the same fix, because both reduce to the same +underlying query: "which of this side's pieces attack square X." + +## -1. Current repo state: what's committed, what's stashed + +As of this writing, `HEAD` (`5c8d794`) has the keep-worthy pieces from +the abandoned attack-presence-bitboard work already committed on their +own: the `bitboard.c` passed-pawn bit-clear fix, the `search.c`/ +`searchsup.c` LMR-gate-to-caller refactor plus its matching `split.c` +fix, and *only* the `CountBits`/`CoorFromBitBoardRank8ToRank1`/ +`CoorFromBitBoardRank1ToRank8` inlining block in `eval.c` (the +`_FastFirstBit`/`_FastLastBit` pair from the same block was PoC-only -- +no production call sites -- and was left out, since keeping it would +leave an unused-function warning once the PoC code it served is gone). + +Everything else from that work -- the `bbPawnAttacks`/etc. `POSITION` +fields, all the `eval.c` reader-migration and mobility-function +production code built on them, the `RunEval{Rook,Bishop,Queen,Knight}AB` +PoC harnesses and their `command.c` entries -- is **not on disk**. It's +sitting in `git stash@{0}` ("eval bitboard reader-migration work +(measured 17-41% slower) + PoC harnesses + reusable data.c ray tables + +command.c evalcycles"), deliberately left there rather than popped back, +since most of it isn't wanted. + +Two things were wanted out of that stash for this plan, and **have now +been pulled** (done, not just planned): + +- **`data.c`** (clean, additions-only diff): `g_RookRayToEdge`/ + `g_BishopRayToEdge`/`g_KnightAttacksBB` + their `Initialize*` functions + -- pulled whole-file via `git checkout stash@{0} -- data.c`, builds + clean. +- **`main.c`**'s 3-line addition calling those three `Initialize*` + functions from the startup sequence -- pulled the same way, alongside + `data.c`. +- **`chess.h`**'s `extern` declarations for the above (7 lines: the two + ray-to-edge tables + their delta/direction arrays, `g_KnightAttacksBB`, + and the three `Initialize*` prototypes) -- hand-copied rather than + checked out, since `chess.h`'s stashed diff also carries the unwanted + `bbPawnAttacks`/etc. `POSITION` fields and PoC function prototypes. Only + the 7 wanted lines were added, next to the existing `BBADJACENT_RANKS` + externs. +- `command.c`'s `evalcycles` command was **not** pulled yet -- still + sitting in the stash, still wanted eventually (section 5 needs the same + shape for `getattacksbench`/`kingdefectsbench`), just not needed until + that section starts. Extraction instructions unchanged from before: + `git diff stash@{0}^ stash@{0} -- command.c`, take only + `EvalCyclesCommand` and its command-table entry, leave + `evalrookab`/etc. behind. + +`eval.c` was correctly left alone -- its stashed diff includes the +inlining block already committed, so pulling it would conflict. Nothing +from section 3 onward has touched `eval.c`/`see.c` yet; when it does, +use the stashed `eval.c` only as *reference* for the ray-walk pattern +(already proven correct there -- `RunEvalRookAB` et al. verified 0 +mismatches), not as something to check out. + +Also present in the stash, independent of all of the above and not +needed for this plan: a small `uNumSplits`/`uNumSplitsTerminated` +split-statistics counter/reporting addition spanning `chess.h`, +`root.c`, and `script.c`. Harmless and separable, but tangential -- +leave it in the stash unless it's wanted on its own merits later. + +## 0. Scope and non-goals + +In scope: add incrementally-maintained per-color, per-piece-type location +bitboards to `POSITION`, and a bitboard-driven "who attacks square X" +primitive that both `GetAttacks` and `CountKingSafetyDefects` consume. + +Explicitly not in scope: changing `MOVE`'s `cFrom:8`/`cTo:8` encoding or +`COOR`'s `0x88` numbering. That would shrink `g_HistoryCounters`, +`g_iPSQT`, `g_ContinuationHistory`, and the countermove table +(`COUNTER_MOVE_TABLE_SIZE`) since they're all keyed directly off raw +`COOR`/`MOVE` bits sized to `0x88`'s wasted range -- a real win if it ever +happens, but a much bigger, separate project (touches `san.c`/`ics.c` +notation, `hash.c`'s packed move encoding, `book.c`'s format, `root.c`'s +PV printing). Ruled out for this pass; noted here only so it isn't +rediscovered and reconsidered mid-migration. + +Also out of scope for this pass: touching `Eval()`'s existing +(measured-slower) attack-presence bitboards -- that's a separate +decision, addressed in section 8, not bundled with this one. Keeping +these independent means a bad result here doesn't force a revert of the +Eval work and vice versa. + +## 1. New state: `bbPieces[2][8]` -- DONE + +Implemented as planned, with one correction found while implementing: +there's no existing `NUM_PIECE_TYPES` constant, so it's sized `[2][8]` +to exactly match `uNonPawnCount[2][8]`'s existing convention (indices +0/1 unused for this field, 2..6 are `PIECE_TYPE`-indexed +KNIGHT/BISHOP/ROOK/QUEEN/KING, KING's slot always zero by construction). +Added to `POSITION` (`chess.h`) right after `uNonPawnCount`. + +Pawns excluded (`pHash->bbPawnLocations[2]` already covers them). King +excluded (`cNonPawns[color][0]`, single square, a bitboard adds nothing) +-- enforced by only ever writing indices `KNIGHT..QUEEN` (i.e. `< KING`), +never `KING` itself. + +**Maintained incrementally in `move.c`**, at all 6 non-pawn +piece-movement functions (more than the original plan's 2 sites -- +`SlidePiece`/`LiftPiece`/`PlacePiece` each turned out to have a second, +`WithoutSigs` sibling used by `UnmakeMove`, not found until reading the +actual call graph): + +- `SlidePiece` / `SlidePieceWithoutSigs` (simple non-capture move, and + its unmake counterpart -- also what castling uses, twice per call, for + king and rook) -- clear from-bit, set to-bit. +- `LiftPiece` / `LiftPieceWithoutSigs` (piece leaves the board: capture, + or the pawn side of a promotion, or unmake-removing a + promoted/just-placed piece) -- clear the bit. Both already branch on + `IS_PAWN`/`!IS_KING` for other bookkeeping, so the new line drops + straight into the existing non-pawn branch, no new conditional needed. +- `PlacePiece` / `PlacePieceWithoutSigs` (piece appears on the board: + promotion result, or unmake-restoring a captured piece) -- set the bit. + Same existing-branch placement as above. + +Confirmed via the actual call sites (`move.c:811-828`, `:960-993`, +`:1249-1339`) that every move type -- normal, capture, both-side +castling, promotion (with or without capture), en passant, and every +undo -- routes through one of these 6 functions; nothing bypasses them +with a direct `rgSquare[].pPiece` write. + +O(1) extra work per move, next to array bookkeeping already happening +there. No rebuild-from-scratch anywhere. + +**From-scratch population**: `fen.c`'s piece-placement loop (the +non-king branch that already sets `cNonPawns[uColor][uIndex]`), one line +added. Zeroing is free -- `fen.c`'s `memset(p, 0, sizeof(POSITION))` +covers the new field automatically, no separate zeroing code needed. + +## 2. Correctness gate on the *maintenance* side -- DONE, verified clean + +Implemented in `board.c`'s existing `VerifyPositionConsistency` +(`board.c:129`), not new bespoke code, as planned: a local `bbPieces[2][8]` +accumulated in the same non-pawn piece-list walk that already builds +`uNonPawnCount[2][7]` (`|= COOR_TO_BB(c)` guarded by `!IS_KING(p)`, +right next to the existing `uNonPawnCount[u][PIECE_TYPE(p)]++`), +compared against `pos->bbPieces` in a loop next to the existing count +comparison (`KNIGHT` to `< KING`, matching `move.c`'s own range +convention), and one new `szExplainations[]` entry ("bbPieces bitboard +doesn't match piece list", reason index 21). + +**Verified clean**, not just added: `gmake DEBUG=1` build has zero +warnings, and `precommit_check.sh` (`gmake TEST=1` self-test + +`debug_smoke_test.sh`) passed with the assert live. Specifically checked +that `TEST=1`'s `TestMakeUnmakeMove` (`testmove.c:454`) exercises the +edge cases that would most likely expose a missed `move.c` update site -- +en passant, promotion-with-capture, and both-side castling, each +round-tripped through make+unmake with `VerifyPositionConsistency` +firing on every `Slide`/`Lift`/`Place` call in `DEBUG` builds -- and the +`bbPieces` check stayed silent throughout. Also confirmed the release +build (`GENETIC=1 PERF_COUNTERS=1 MP=1 SIXTYFOUR=1`) compiles clean and +runs a normal search correctly (the maintenance code has no `#ifdef +DEBUG` guard, matching section 6's "always on" toggle strategy). + +Nothing reads `bbPieces` yet (confirmed: it's write-only outside the +`DEBUG` consistency check), so this remains pure addition with zero +behavioral risk, exactly as planned. **Not yet committed** -- sitting on +disk, verified, ready to commit whenever section 1 is considered a good +checkpoint. + +## 3. New `_WhoAttacksSquareBB` primitive, consumed by two PoCs -- NOT STARTED + +Everything below this point is still just the plan -- no code written yet. + +Build one shared bitboard primitive rather than two independent +implementations, since `GetAttacks` and `CountKingSafetyDefects` are the +same query against different target squares: + +- Pawns: existing 2-square delta check, unchanged (already O(1), nothing + to improve). +- Knights/king: `g_KnightAttacksBB[cSquare] & bbPieces[side][KNIGHT]` + (and the equivalent king delta check) -- O(1), reusing + `g_KnightAttacksBB` already built and verified this session. +- Sliders (bishop/rook/queen): reuse `g_RookRayToEdge`/`g_BishopRayToEdge` + + `_FastFirstBit`/`_FastLastBit`, walking outward from the target + square along each ray to the nearest blocker, testing it against + `bbPieces[side][ROOK]`/`[BISHOP]`/`[QUEEN]` (queens checked on both ray + sets). Needs `bbOccupied` (both colors, all pieces) to find the + blocker -- construct as `bbPieces[WHITE][*] | bbPieces[BLACK][*] | + pHash->bbPawnLocations[*] | king bits`, now cheap (all pre-maintained) + rather than a `cNonPawns` walk. + +Two call-site wrappers, each a PoC alongside the real thing (same pattern +as this session's earlier `_EvalRookOccupancyBB` PoC work), gated so old +and new are both callable side by side: + +- **`_GetAttacksBB`** (`see.c`): calls the primitive against the move's + destination square, populates a `SEE_LIST` exactly like `GetAttacks` + does today. No pin-detection, no en-passant handling -- matching + `GetAttacks`'s exact (deliberately approximate) semantics. Not + introducing new behavior, just recomputing the same answer differently. +- **`_CountKingSafetyDefectsBB`** (`eval.c`): calls the primitive against + the king's square, feeding the same `CHECK_VECTOR`-derived defect-count + logic `CountKingSafetyDefects` already has (`uPiecesPointingAtKing` + bookkeeping included) -- only the "which enemy pieces bear on this + square" step changes, the counting/scoring logic downstream of it + doesn't. + +`CountKingSafetyDefects` doesn't build a `SEE_LIST` at all (it just +counts and classifies), so it isn't a second consumer of `_GetAttacksBB` +directly -- it's a second consumer of the lower-level per-piece-type +bitboard scan the two share. Structure the primitive so it can return +either "the raw attacker bitboard per piece type" (what +`CountKingSafetyDefects` wants, to test `CHECK_VECTOR`-equivalent +conditions itself) or "a populated `SEE_LIST`" (what `GetAttacks` wants), +rather than forcing one call shape on both. + +## 4. Correctness verification + +Confirmed before writing this plan: `SEE()` sorts/heaps the attacker list +immediately after `GetAttacks` returns (`see.c:872-878`, +`_BuildHeap`/`_SortList`), so a bitboard-based `GetAttacks` returning +attackers in a different order than the mailbox version is fine -- +correctness only requires the *set* of attackers to match, not the +sequence. + +1. **Fix `SeeListsAreEqual` to be order-independent first** + (`testsee.c:139`) -- sort both lists by `(cLoc, pPiece)` before the + field-by-field compare, or compare as multisets. Do this as its own + tiny commit; verify it still passes against the existing + Slow-vs-asm comparison (order shouldn't change there, so this change + should be provably a no-op in isolation before it's relied on for + anything new). +2. **Extend `TestGetAttacks`** (`testsee.c:154`) to add `_GetAttacksBB` + as a third comparison against the same 20,000 random-position x + every-square x both-colors sweep already in place + (`GenerateRandomLegalPosition`). This is the primary correctness + gate -- cheap to run (`gmake TEST=1`), already wired into the batch + self-test startup path. +3. **Add an equivalent random-position test for + `_CountKingSafetyDefectsBB`**, same 20,000-position generator, both + colors, comparing its returned defect count *and* the + `uPiecesPointingAtKing` side-effect against `CountKingSafetyDefects`. + No existing harness to extend here (unlike `GetAttacks`), so this is + new test code in the `TEST=1` build, following `TestGetAttacks`'s + shape. +4. **`precommit_check.sh`** as always, for the crash/assert layer. +5. **SEE-value diffing at the position level**: since `SEE()` returns a + single integer per move (not a board eval), the equivalent of the + Eval work's "150-position score-identity diff" is a diagnostic + command (`seescores`, analogous to the batch `eval` dumps used + earlier) that computes `SEE()` for every legal capture in a batch of + ECM/random positions, run once per `GetAttacksBB` toggle state, + diffed. Catches any behavioral drift `TestGetAttacks`'s raw-list + comparison might miss once results feed into `_MinLegalPiece`'s + exchange simulation. Do the same for `CountKingSafetyDefects`: a + `kingdefects` diagnostic command dumping the defect count for both + kings across the same position batch, diffed the same way -- this one + matters more than it sounds, since the count feeds directly into + `Eval()`'s king-safety score term (`eval.c:2549-2550`), not just + pruning gates. +6. **Full-suite behavioral check is mandatory here, not optional** -- + this is the one place this migration is *riskier* than the Eval + constant work: `GetAttacks` feeds move ordering and pruning decisions + (`generate.c`'s 5 call sites, `searchsup.c:365`), and + `CountKingSafetyDefects` feeds pruning/extension decisions directly + (`search.c:1252`, `searchsup.c:484,902,905`) *in addition to* an + Eval() score term -- so even value-for-value-identical results can + shift which move gets tried first or which lines get pruned/extended, + changing node counts and occasionally search results at the margins. + Run all three curated suites (`ecm_ringers`, `ecm_confident_quick`, + `ecm_hard_quick`) at `sd10` against `head_reference/`, and don't treat + a clean `TestGetAttacks`/`_CountKingSafetyDefectsBB` unit-level pass + as sufficient sign-off by itself -- `CountKingSafetyDefects`'s direct + search-code call sites make this the higher-risk of the two changes, + despite being the simpler one to implement. +7. **`match_play.py` gate** (`LOWER95 >= 0.5`) before calling this done + -- same reasoning: this is closer to a search-behavior change than a + pure eval-magnitude change, so the existing eval-tuning gate criteria + apply. Run once with both migrations landed together (they share the + same underlying primitive, so testing them jointly is representative + of how they'll actually ship) plus, if the combined match shows a + problem, one more isolating each toggle independently to attribute it. + +## 5. Microbenchmarking + +The Eval cycle measurement earlier this session is the cautionary tale: +an isolated PoC can look plausible and still lose once integrated, +because integration costs (dual maintenance, extra indirection) aren't +visible in isolation. Two tiers, both required before wiring in: + +1. **Isolated cycles/call**, same `SystemReadTimeStampCounter` pattern + as `evalcycles`/`RunEvalRookAB`: a `getattacksbench [iterations]` + command looping old vs. new `GetAttacks` on the *current* + position/square, and a `kingdefectsbench [iterations]` command doing + the same for `CountKingSafetyDefects`, interleaved (old/new/old/new) + to cancel shared-box noise, run across a battery of positions spanning + piece density -- opening (~30 pieces), middlegame (~20), endgame + (~8) -- since the algorithmic win should scale *with* piece count, and + a flat or inverted result across that spectrum is a red flag before + proceeding further, exactly like the rook/bishop Eval win that didn't + hold up once integrated. +2. **Whole-engine cycles-per-node / NPS**, not just the isolated call: + `sd`-fixed-depth comparison against `head_reference` on the three + curated suites, using the engine's own self-reported `Searched for N + seconds, M nodes` (not `ps` sampling -- noisy on this shared box per + earlier sessions). This is the check that would have caught the Eval + regression earlier if it had been run before the reader-migration + work proceeded -- don't skip straight from isolated-cycles-looks-good + to production wiring again. Given `CountKingSafetyDefects`'s search-code + call sites, this whole-engine number is where its win (or lack of one) + will actually show up -- its isolated per-call cycle count matters + less than `GetAttacks`'s does, precisely because it's called from more + places than just move ordering. + +## 6. Dual-support / toggle strategy + +Same `#define` pattern already established for the Eval occupancy work +(`ROOK_OCCUPANCY_EVAL` et al.): `GETATTACKS_BITBOARD`, flipping which +implementation `chess.h`'s `GetAttacks` macro/prototype resolves to +(alongside the existing `CROUTINES` old/new(asm) switch -- this becomes a +three-way choice: `SlowGetAttacks` (C mailbox), asm `GetAttacks` (x86/x64 +mailbox), `_GetAttacksBB` (new)). A second, independent toggle, +`KINGSAFETY_BITBOARD`, does the same for `CountKingSafetyDefects` vs. +`_CountKingSafetyDefectsBB` -- independent so each can be measured, +gated, and (if needed) rolled back on its own, even though they share the +underlying `_WhoAttacksSquareBB` primitive. + +`bbPieces` maintenance itself is **always on** regardless of either +toggle -- it's cheap enough that gating it adds complexity for no +benefit, and other future work (see section 8) can build on it once +trusted. + +## 7. Retirement criteria for the old mailbox implementations + +Only delete `SlowGetAttacks`/asm `GetAttacks` after **all** of: + +- Extended `TestGetAttacks` clean across the 20,000-position sweep. +- Isolated cycles/call shows a consistent win across the piece-density + spectrum (not just one favorable position). +- Whole-engine `sd10` on all three curated suites shows no solve-count + regression vs. `head_reference`. +- `match_play.py` gate clears `LOWER95 >= 0.5`. +- `head_reference/` rebuilt as the new baseline once landed. + +Only delete the old `CountKingSafetyDefects` mailbox body after the same +five criteria, evaluated against *its* test/bench additions -- do not +retire it just because `GetAttacks`'s migration cleared its own bar; they +share a primitive but are independently gated per section 6, and +`CountKingSafetyDefects`'s wider call-site footprint (search-code pruning +gates, not just move ordering) makes it the one more likely to surface a +problem only visible in full search behavior, not in the unit-level +correctness test. + +One easy win to check first, independent of all this: confirm whether +`x86.asm`'s `GetAttacks` is even compiled into the `SIXTYFOUR=1` release +profile at all (`GNUmakefile`) -- if it's already dead on this 64-bit-only +box, that's a zero-risk deletion to do up front, separate from the +migration. + +## 8. What this means for the existing (measured-slower) Eval bitboards + +Not bundling a decision here, but flagging it: once `bbPieces` exists and +is trusted, it's a cheaper substrate than what the current +`bbPawnAttacks`/`bbMinorAttacks`/`bbRookAttacks`/`bbQueenAttacks`/ +`bbKingAttacks` work does today (rebuild-from-scratch per `Eval()` call). +Worth revisiting *after* this migration lands -- either reimplementing +the attack-presence bitboards on top of `bbPieces` + ray tables cheaply, +or reverting them if `bbPieces` alone doesn't end up mattering for Eval. +Separate decision, separate correctness/benchmark cycle, not this one. -- cgit v1.3