# Migration plan: bitboard-backed `GetAttacks` **Scope note (post-section-3): `CountKingSafetyDefects` has been removed from this plan.** It was originally included as a second consumer of the same shared primitive, but turned out not to fit -- see the "Why `CountKingSafetyDefects` was dropped" section right after this one for the full reasoning. Its code is untouched and left exactly as it was; if it's ever revisited, that should be a new, separate migration document, not a resumption of this one -- the two functions no longer have enough in common (beyond both reading `bbPieces`) to justify sharing a plan, a toggle strategy, or a retirement checklist. Every section below has been edited to drop `CountKingSafetyDefects` references accordingly; where useful, dropped content is preserved inline as a note rather than deleted, so a future session designing the separate migration doesn't have to rediscover it from git history. **Status: sections 1-3 (`bbPieces`/`bbPawns`-backed `_WhoAttacksSquareBB`/`_GetAttacksBB`), all of section 4, and section 6 (the `GETATTACKS_BITBOARD` toggle) implemented, verified, and committed -- including a whole-engine `sd10` check across all three curated suites (zero solve-count regression, +8.38% aggregate NPS) and a `seescores` SEE-value diff across all 747 captures in those suites (byte-identical, zero diff). Only `match_play.py` (section 4 item 6 / section 7) remains unmet before full retirement of the old mailbox implementation, deliberately deferred for now. Section 5's whole-engine item and section 7's remaining items are otherwise done; section 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, focused on one function: - **`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`. It does a manual mailbox ray-walk over every one of a side's non-pawn pieces (with a `CHECK_VECTOR` geometry-table lookup to quickly reject misaligned ones) to answer "does this piece attack the move's destination square" -- O(pieces) candidates x O(ray length) per call, regardless of how close any given piece actually is to the target square. A strong candidate for a bitboard-backed rewrite because it reduces to a simple, well-defined query: "which of this side's pieces attack square X." ## Why `CountKingSafetyDefects` was dropped `CountKingSafetyDefects` (`eval.c:2408`) was originally included here too, on the assumption that it does the same kind of ray-walk as `GetAttacks` against a different target square (the king's, instead of a move's destination) -- 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`), making it if anything a *more* pervasive case of the same problem. That assumption turned out to be wrong once the actual function body (`eval.c:2325`) was read carefully while implementing `GetAttacks`'s half of section 3: `CountKingSafetyDefects` does **no ray-walk and no blocker/occlusion check at all**. It's a `CHECK_VECTOR_WITH_INDEX` table lookup testing whether each enemy piece's *type* geometrically aligns with one of three squares near the king (`cKing-1, cKing, cKing+1`), regardless of what's in between -- a deliberately approximate proximity/alignment heuristic, not a true "does this piece attack this square" query. That mismatch matters for two reasons this plan's shared-primitive premise depended on: 1. **No shared code to share.** `_WhoAttacksSquareBB` (built for `GetAttacks`) is blocker-aware by construction -- that's the whole point of a ray-walk-replacement primitive. Feeding `CountKingSafetyDefects` a blocker-aware result would make it *more accurate* than today, not a value-identical reimplementation. A correct bitboard version of `CountKingSafetyDefects` would need to stay unblocked (just reformulate the existing `cNonPawns` array walk as a `bbPieces` bit walk), which shares `bbPieces` as a data source but not `_WhoAttacksSquareBB` as a function -- there is no longer a single primitive both functions consume, so this plan's core framing ("two functions sharing one new bitboard primitive") doesn't hold. 2. **Different, harder-to-satisfy correctness/gating shape.** Because the two functions would no longer share an implementation, testing them "jointly" (as section 4 originally proposed) stops being representative of anything -- each needs its own correctness sweep, its own benchmark, its own curated-suite/`match_play.py` gate, on its own schedule. And because `CountKingSafetyDefects` feeds an `Eval()` score term in addition to pruning gates, any *intentional* change to its approximation (e.g. making it blocker-aware on purpose, as a real improvement rather than an accidental one) is an eval-tuning-shaped change, not a reimplementation -- a materially different, riskier project than this one. Given that, bundling it into this plan added risk and complexity to the `GetAttacks` work for no shared benefit. It's out of scope now; `eval.c`'s `CountKingSafetyDefects` is untouched. A future migration for it should start from scratch with its actual behavior (the unblocked proximity heuristic above) as the documented baseline, not from this document's original "shares a primitive with `GetAttacks`" premise. ## -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 `GetAttacks` consumes. Explicitly not in scope (see "Why `CountKingSafetyDefects` was dropped" above): reimplementing `CountKingSafetyDefects`. It was in scope in an earlier version of this document; removed once implementing `GetAttacks`'s half revealed the two functions don't actually share enough to justify one plan. 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 -- DONE, verified correct, verified faster than asm Implemented in `see.c`: `_WhoAttacksSquareBB(pos, cSquare, uSide, bbOccupied)` returns a bitboard of every uSide knight/bishop/rook/ queen/king attacking `cSquare` (pawns handled separately, see below), and `_GetAttacksBB` wraps it into a `SEE_LIST`-populating PoC with the same signature/semantics as `SlowGetAttacks`/`GetAttacks`. Not yet wired into the `GetAttacks` macro (section 6's toggle) -- still a side-by-side PoC, called only from the test/bench harness described below. Diverged from the original plan in a few ways, all discovered by measuring rather than guessing: - **`bbOccupied` construction dropped the `pHash->bbPawnLocations` idea.** That field lives behind `PawnHashLookup(SEARCHER_THREAD_CONTEXT *)`, but `GetAttacks`'s real call sites (`generate.c` x5, `see.c`, `searchsup.c`) only ever have a `POSITION *`, matching `_GetAttacksBB`'s signature -- threading a `ctx` through everywhere just for this wasn't worth it. Added `POSITION.bbPawns[2]` instead: plain, incrementally-maintained state (identical pattern to `bbPieces`, same 6 `move.c` sites plus `fen.c`'s from-scratch population), independent of the pawn hash. This turned out to be the single biggest win in the whole section -- see benchmarks below. - **A precomputed `g_PawnAttackOriginBB[2][128]` table** (data.c, `InitializePawnAttackOriginTable`, called from `main.c`'s startup sequence) replaced the original plan's "existing 2-square delta check, unchanged" for pawns. That plan text assumed the delta check was already optimal since it's O(1) either way -- true asymptotically, but the *fixed* per-call instruction count still mattered: `g_PawnAttackOriginBB[uSide][cSquare] & pos->bbPawns[uSide]` is one table lookup + one AND, entirely in bit-space (no `cSquare + delta` arithmetic, no `IS_ON_BOARD` branch, no mailbox load), vs. the original's add/mask/load/compare done twice. Measured a further ~2-4% win from this on top of the `bbPawns` change, small but real and consistent across all three density buckets tested. - **Per-direction and per-side-group early-outs**, not just "does uSide have any rook/queen at all": `bbRookSliders & g_RookRayAll[ cSquare]` (a new startup-built table, all 4 `g_RookRayToEdge` directions pre-ORed per square) gates entry to the direction loop at all; inside it, each direction is skipped via `bbRay & bbRookSliders` before touching `bbOccupied` or calling `FastFirstBit`/`FastLastBit` (same idea as bishop). This is the bitboard equivalent of what `CHECK_VECTOR` does per-piece in the mailbox version. - **`FastFirstBit`/`FastLastBit`**: `static inline` compiler-builtin (`__builtin_ctzll`/`__builtin_clzll`) wrappers, moved to `chess.h` (not `bitboard.c` -- a non-`static` C99 `inline` split across a .c/.h pair is a link-time trap with no out-of-line instantiation anywhere; caught this by testing the link, not by inspection). Same bsf/bsr instruction the asm `FirstBit`/`LastBit` already use (confirmed by reading the compiled asm and `x64.asm`'s `FirstBit` directly) -- the entire benefit is skipping `CDECL` call/return overhead by inlining, not a smarter bit-trick. - **Blocker isolation stays in bit-space**: the slider loop isolates the nearest blocker as a bitboard bit directly (`bb & -bb` for the positive-direction/lowest-bit case, no `FastFirstBit` call needed at all; `1ULL << (FastLastBit(bb)-1)` for the negative/highest-bit case) and tests/ORs it directly against other bitboards, instead of round-tripping through `BIT_NUMBER_TO_COOR`/`COOR_TO_BB` to do the same test. `COOR_TO_BB`/`BIT_NUMBER_TO_COOR` are themselves cheap (~1 cycle, no memory access -- confirmed via `testbitboard.c`'s existing benchmark), so this wasn't about macro cost, just removing otherwise-pointless conversions between two representations that never needed to leave bit-space in the first place. - **One deliberately-reverted attempt, kept as a comment**: replacing the 4-direction "check ray, skip if empty" loop with code that derived the *specific* needed direction(s) directly from the aligned slider bits (via `FastFirstBit` + rank/file-nibble comparison, no table lookup) measured *slower* (~1.07-1.09x asm vs. ~0.96-1.00x for the simple loop) -- the bit-scan-to-avoid-a-cheap-branch traded a cheap AND+continue for a costlier extraction+branch. Reverted; the comment in `see.c` explains why, so it isn't rediscovered and retried the same way. **Benchmark** (new `TestGetAttacks` addition in `testsee.c`, interleaved `GetAttacks`(asm)/`SlowGetAttacks`/`_GetAttacksBB`, 200k calls each across opening/middlegame/endgame positions, `SystemReadTimeStampCounter`-based, same methodology as `testbitboard.c`'s existing benchmark): | stage | vs. asm `GetAttacks`, final | |---|---| | opening | **0.53x** (i.e. ~1.9x faster) | | middlegame | **0.55x** | | endgame | **0.89x** | The `bbPawns`/`_BuildOccupiedBB` change alone (before the pawn-origin table) accounted for most of the opening/middlegame jump -- from ~0.96-1.00x to ~0.55-0.57x -- since it replaced an up-to-16-iteration loop over `cPawns[2][8]` with two ORs, and opening/middlegame positions are exactly where that loop was longest. Endgame barely moved (fewer pawns = the old loop was already cheap there), which is the expected shape for that change and a useful cross-check that the win is coming from where the theory says it should. **Correctness**: `SeeListsAreEqual` (testsee.c) made order-independent first (sorts a scratch copy of each list by `(cLoc, pPiece)` before comparing -- `SEE()` sorts/heaps the list immediately after `GetAttacks` returns, so order was never semantically significant, just untested as such before). `TestGetAttacks` extended to run `_GetAttacksBB` as a third comparison against the same 20,000-random-position x every-square x both-colors sweep already exercising `SlowGetAttacks`/asm `GetAttacks` -- clean, `gmake TEST=1`. This is section 4's items 1-2, done here rather than deferred, since it was the only way to get real confidence in the optimization work above rather than trusting cycle counts on a possibly-wrong implementation. **Two real bugs found and fixed while getting the correctness sweep to actually run (not just to pass)**, both pre-existing, both unrelated to `GetAttacks` itself: 1. **`GenerateRandomLegalPosition` (testsup.c)** -- used by `TestGetAttacks`'s 20,000-position sweep -- builds positions via direct field writes (`cNonPawns[]`, `cPawns[]`, etc.), bypassing `PlacePiece`/`SlidePawn` entirely, so it never maintained `bbPieces` (already true as of section 1, latent since that commit) or the new `bbPawns`. Its own legality gate, `VerifyPositionConsistency(pos, TRUE)`, checks `bbPieces`/`bbPawns` against the piece list and silently returns `FALSE` (no panic -- `fContinue=TRUE`) for any position with a non-pawn officer or any pawn on the board, so the generator's outer retry loop would almost always discard the position and retry -- not a deterministic hang, but a large, variable slowdown (sometimes fast by luck, sometimes ran for minutes) that looked exactly like a new infinite loop the first time it was hit. Root-caused by instrumenting `VerifyPositionConsistency`'s failure path directly (temporary `fprintf` of `uReason`) rather than guessing -- confirmed reason 21 (`bbPieces` mismatch), not the new reason 22 (`bbPawns` mismatch). Fixed by adding the same one-line-per-site `bbPieces`/`bbPawns` maintenance `fen.c` already had, to `testsup.c`'s two hand-placement sites. 2. **A pre-existing, non-deterministic assertion** (`util.c:1093`, inside `FinishPVTailFromHash`'s PV-tail formatting, part of the original 2016 commit, untouched by this or any prior session's diff) surfaced once during `precommit_check.sh`'s DEBUG smoke test on an unlucky random ECM sample. Confirmed independently of this session's work: replaying the exact saved failing sample (`--cpus 4 --hash 64m`, matching `debug_smoke_test.sh`'s invocation) against a stashed-clean `HEAD` reproduced the same crash on 1 of 3 runs, and passed clean on the other 2 -- an `MP=1` (4-thread) race in PV printing, unrelated to `GetAttacks`/`bbPieces`/`bbPawns`. **Not fixed here** -- flagged for a future session, since fixing it is unrelated scope to this migration and risks masking whether a real regression exists if conflated with this work's own testing. (`CountKingSafetyDefects` was originally planned as a second consumer of this primitive -- see "Why `CountKingSafetyDefects` was dropped" near the top of this document for why that's no longer part of this plan.) ## 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. **DONE**, folded into section 3's own work above -- also fixed a latent `bbPieces`/`bbPawns` maintenance gap in `GenerateRandomLegalPosition` found while running this. 3. **`precommit_check.sh`** as always, for the crash/assert layer. **DONE**, both without and with section 6's toggle (`GETATTACKS_BITBOARD=1`) -- clean in both configurations, the latter run confirming `_GetAttacksBB` live end-to-end in real search, not just the isolated harness. 4. **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. **DONE.** `seescores ` (`command.c`, registered alongside `script`/`sd`) reads `setboard` lines from an EPD file (the same convention `tests/ecm*.ep_` already use), generates legal moves per position, and prints `(FEN, SAN move, SEE value)` for every capture. Run against all three curated suites (747 captures total: 51 + 351 + 345), once per binary from section 4 item 5's same-commit asm-vs-`GETATTACKS_BITBOARD=1` A/B build -- output **byte-identical, zero diff**, on all three. Confirms `_GetAttacksBB` is correct not just at the attacker-list level (`TestGetAttacks`) but all the way through `SEE()`/`_MinLegalPiece`'s exchange simulation to the final score every move-ordering decision actually uses. 5. **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 directly (`generate.c`'s 5 call sites, `searchsup.c:365`), 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. **DONE** -- run all three curated suites at `sd10`, but *not* against the checked-in `head_reference/` binary: that binary predates this branch's sections 1-6 by a dozen-plus unrelated commits, so diffing against it would conflate this change with everything else on the branch. Instead, isolated the one variable that matters: same commit, built twice (`gmake clean && gmake -j5 GENETIC=1 PERF_COUNTERS=1 MP=1 SIXTYFOUR=1` with and without `GETATTACKS_BITBOARD=1`), same `--cpus 1 --hash 256m`, `sd 10`, `book name /nonexistent.book.bin` invocation as `head_reference/`'s own convention. Results: | suite | solves (asm -> BB) | nodes (asm -> BB) | NPS (asm -> BB) | |---|---|---|---| | `ecm_ringers` (11) | 10 -> 10 | 37,611,927 -> 37,909,197 (+0.79%) | 1,178,986 -> 1,270,436 (+7.76%) | | `ecm_confident_quick` (90) | 83 -> 83 | 429,387,924 -> 430,930,314 (+0.36%) | 1,201,203 -> 1,300,133 (+8.24%) | | `ecm_hard_quick` (90) | 25 -> 25 | 459,712,049 -> 465,403,216 (+1.24%) | 1,131,433 -> 1,228,991 (+8.63%) | **Solve counts bit-identical on all three suites -- zero regression.** Node counts up slightly (+0.36% to +1.24%), exactly the "value-identical but not order-identical" effect warned about above -- `_GetAttacksBB` returns the same attacker *set* as asm `GetAttacks` but not necessarily the same order, so `SEE()`/move- ordering tie-breaking can shift which lines get searched first at the margins. Didn't cost a single solve here. Aggregate (total nodes / total script time across all three suites, not an average-of-averages): baseline 926,711,900 nodes / 795.7s = 1,164,842 nps; bitboard 934,242,727 nodes / 740.0s = 1,262,490 nps -- **+8.38% overall wall-clock NPS**, comfortably absorbing the extra node count. Whole-engine result confirms section 3's isolated cycles/call benchmark wasn't an artifact of measuring the wrong thing -- the speedup shows up in real search throughput, not just in the isolated per-call harness. 6. **`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. Not started yet, deliberately deferred -- item 5's zero solve regression + throughput win is convincing enough on its own for now; this gate is still required before full retirement (section 7), just not being chased immediately. ## 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`: interleaved (old/new/old/new, to cancel shared-box noise) comparison 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. **DONE**, folded into `TestGetAttacks`'s own benchmark addition rather than a separate `getattacksbench` command -- see section 3 for the results (0.53x asm opening, 0.55x middlegame, 0.89x endgame). Consistent win across the spectrum, no red flag. 2. **Whole-engine cycles-per-node / NPS**, not just the isolated call: `sd`-fixed-depth comparison on the three curated suites, using the engine's own self-reported node counts/NPS (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. **DONE**, folded into section 4 item 5's same-commit A/B run (see there for the numbers and why `head_reference/` itself wasn't the comparison point) -- +8.38% aggregate wall-clock NPS across all three suites, confirming the isolated-cycles-per-call win (section 3) actually shows up end to end, not just in the harness. ## 6. Dual-support / toggle strategy -- DONE Same `#define` pattern already established for the Eval occupancy work (`ROOK_OCCUPANCY_EVAL` et al.): `GETATTACKS_BITBOARD` (`GNUmakefile` build flag, `-DGETATTACKS_BITBOARD`), flipping which implementation `chess.h`'s `GetAttacks` macro resolves to -- a three-way choice alongside the existing `CROUTINES` switch: `_GetAttacksBB` (new, bitboard) if `GETATTACKS_BITBOARD` is defined, else `SlowGetAttacks` (C mailbox) if `CROUTINES` is defined, else the real asm `GetAttacks` (x86/x64 mailbox, the default). `_GetAttacksBB` is now reachable from every real `GetAttacks` call site (`generate.c`'s check-detection call, `see.c`'s `SEE()`, `searchsup.c`) when the flag is set, not just the test/bench harness. **Found and fixed while wiring this up**: `testsee.c`'s own `TestGetAttacks`/benchmark code calls the identifier `GetAttacks` to mean "the real asm/CROUTINES baseline" -- but once the macro can resolve to `_GetAttacksBB`, those same calls would silently become `_GetAttacksBB` compared against itself, turning the correctness sweep and the "asm vs. new" benchmark into false-positive no-ops. Fixed with a local `#undef GetAttacks` right after `#include "chess.h"` in `testsee.c`, so the harness always reaches the true baseline implementation regardless of which implementation is live in production -- the harness's job is validating the new implementation against a fixed reference, not testing the engine's current configuration against itself. **Verified with the toggle live**: `gmake TEST=1 GETATTACKS_BITBOARD=1` -- self-test suite passes (including the corrected benchmark, still reporting real asm vs. `_GetAttacksBB` correctly, same ~0.52-0.92x numbers as section 3), and a real `Search()` call in the self-test suite completed normally with `_GetAttacksBB` live in `SEE()`/move generation's check-detection path, not just the isolated harness. `precommit_check.sh GETATTACKS_BITBOARD=1` (env-var propagation into `gmake` verified directly by grepping the resulting build logs for `-DGETATTACKS_BITBOARD`) -- both the `TEST=1` self-test and `DEBUG=1` smoke test (10 random ECM positions, `sd 4`) pass clean with the bitboard implementation live end-to-end. Default build (no flag) confirmed unaffected -- `GetAttacks` still resolves to the real asm function unless the flag is explicitly passed. `bbPieces`/`bbPawns` maintenance itself is **always on** regardless of this 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 implementation Only delete `SlowGetAttacks`/asm `GetAttacks` after **all** of: - Extended `TestGetAttacks` clean across the 20,000-position sweep. **DONE.** - Isolated cycles/call shows a consistent win across the piece-density spectrum (not just one favorable position). **DONE.** - Whole-engine `sd10` on all three curated suites shows no solve-count regression. **DONE** -- bit-identical solve counts on all three (10/11, 83/90, 25/90), same-commit asm-vs-`GETATTACKS_BITBOARD=1` A/B rather than vs. a now-stale `head_reference/` (see section 4 item 5 for why and the full numbers) -- also +8.38% aggregate wall-clock NPS. - `match_play.py` gate clears `LOWER95 >= 0.5`. Not started -- deliberately deferred for now, still required before retirement. - `head_reference/` rebuilt as the new baseline once landed. 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.