path: root/src/board_representation/MIGRATION.md

# 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 <filename>` (`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.