diff options
Diffstat (limited to 'src/board_representation/MOVEGEN_MIGRATION.md')
| -rw-r--r-- | src/board_representation/MOVEGEN_MIGRATION.md | 912 |
1 files changed, 850 insertions, 62 deletions
diff --git a/src/board_representation/MOVEGEN_MIGRATION.md b/src/board_representation/MOVEGEN_MIGRATION.md index 5ba30d8..44527b1 100644 --- a/src/board_representation/MOVEGEN_MIGRATION.md +++ b/src/board_representation/MOVEGEN_MIGRATION.md @@ -1,15 +1,61 @@ # Migration plan: bitboard-backed move generation (`generate.c`) -**Status: planning only. No code written.** This is a scoping document, -drafted after `MIGRATION.md`'s `GetAttacks` work landed, to decide -whether/how to extend the same bitboard substrate (`bbPieces`, -`bbPawns`, `g_RookRayToEdge`/`g_BishopRayToEdge`/`g_RookRayAll`/ -`g_BishopRayAll`, `g_KnightAttacksBB`, `g_PawnAttackOriginBB`) to move -generation itself. Deliberately kept as a **separate** document from -`MIGRATION.md`, not a new section appended to it -- same reasoning as -dropping `CountKingSafetyDefects` from that plan: this is a -substantially bigger, higher-risk surface than `GetAttacks` was, and -bundling it in would blur two very differently-shaped efforts. +**Status (2026-09-04): Part A, Part B, and section 6b all implemented +and correctness/speed-gated; nothing shipped yet (every toggle still +off by default).** + +- **Part A** (section 3, `_GenerateAllMoves`): all six piece types + (knight, king, rook, bishop, queen, pawn) plus the `_GenerateAllMovesBB` + dispatch-layer rewrite -- done, correctness-verified per-piece-type + and combined. +- **Part B** (section 6a, `_GenerateEscapes`): both phases (king flight + via `_WhoAttacksSquareBB`, block/capture via `bbTargetMask` + + per-piece `SaveMe*BB` + the dispatch-loop elimination) -- done, + correctness-verified individually and combined. +- **Section 6b** (`movesup.c`): `ExposesCheck`/`FasterExposesCheck`/ + `ExposesCheckEp` and `IsAttacked`/`InCheck` -- done, correctness- + verified, `IsAttackedBB` additionally shows a genuine 0.73x-0.93x + speed win. +- **All nine toggles verified combined simultaneously**, including one + real bug found and fixed in the test harness itself (`testsup.c`'s + `GenerateRandomLegalPosition` never initialized `cEpSquare`) -- + 15/15 clean runs post-fix. +- **Section 4's `sd10` curated-suite gate has been run** against + `head_reference/`, with all nine toggles combined: `ecm_ringers` + 10/11 and `ecm_confident_quick` 84/90 match `head_reference` exactly; + `ecm_hard_quick` showed 25/90 against `head_reference`'s recorded + 28/90, but this was tracked down to intervening non-toggle-gated + commits unrelated to this migration (`5c8d794`/`a8806ad`), confirmed + by reproducing the identical 25/90 on plain current-HEAD mailbox with + every toggle off. **The correct comparison baseline going forward is + current HEAD's own numbers, not `head_reference`'s stale recorded + ones**: `ecm_ringers` 10/11, `ecm_confident_quick` 84/90, + `ecm_hard_quick` 25/90 (all at `sd10`, `--cpus 1 --hash 256m`) -- + `head_reference/` itself has not been refreshed to pick up + `5c8d794`/`a8806ad` yet (a separate, not-yet-run action via + `update_head_reference.sh`), so its own logs still show the older + 28/90 figure until that happens. +- **Still outstanding**: `match_play.py`'s `LOWER95 >= 0.5` gate (not + run), the 20,000-position move-set comparison harness section 4.1 + originally called for (never built -- perft plus direct + mailbox-vs-bitboard comparison harnesses have covered this gap so + far), refreshing `head_reference/` itself, and section 7's retirement + criteria (deleting any mailbox function) -- not cleared for anything, + nothing should be deleted yet. `_FindUnblockedSquares`/`WouldGiveCheck` + (flagged during the section 6b survey) remains a separate, unstarted, + smaller candidate. + +Originally a scoping document only (see the rest of this paragraph for +that history): drafted after `MIGRATION.md`'s `GetAttacks` work landed, +to decide whether/how to extend the same bitboard substrate +(`bbPieces`, `bbPawns`, `g_RookRayToEdge`/`g_BishopRayToEdge`/ +`g_RookRayAll`/`g_BishopRayAll`, `g_KnightAttacksBB`, +`g_PawnAttackOriginBB`) to move generation itself. Deliberately kept as +a **separate** document from `MIGRATION.md`, not a new section appended +to it -- same reasoning as dropping `CountKingSafetyDefects` from that +plan: this is a substantially bigger, higher-risk surface than +`GetAttacks` was, and bundling it in would blur two very +differently-shaped efforts. ## 0. Why this is a bigger project than `GetAttacks` was @@ -164,6 +210,139 @@ from-scratch undertaking: itself isn't what made that work slow; duplicating both representations without removing the old one was. +## 2a. New infrastructure required for magic bitboards + +Unlike everything in section 2, none of this exists yet -- it's a +prerequisite sub-project for step 3 of section 3, not a reuse of +`GetAttacks`-era work. Standard magic-bitboard components, all +per-piece-type (rook, bishop) and per-square: + +- **Relevant-occupancy masks** (`g_RookOccupancyMask[128]` / + `g_BishopOccupancyMask[128]`) -- the full ray-to-edge tables + (section 2) minus the actual board edge squares themselves (a piece + on the edge doesn't block anything beyond the edge, so those bits + are irrelevant to the lookup and must be excluded to keep the + occupancy-permutation count, and therefore the table size, minimal). +- **Magic numbers, occupancy masks, and attack tables -- decided: all + computed live at engine startup, in a new `InitMagic()` in `data.c`, + nothing hardcoded.** Initially assumed the random-candidate search + would be too slow to pay on every process launch (the validation + prototype's *total* runtime was 3.24s), which would have forced + baking found magics in as literal compile-time constants instead + (the way a published constant set would have been used). That + assumption was wrong, caught by re-checking the prototype's own + breakdown: **3.15s of that 3.24s was the separate PEXT-vs-multiply + microbenchmark** (400M loop iterations, unrelated to magic-finding), + not the search. Isolating just the search-and-verify work (a + from-scratch throwaway build of the same logic, no benchmark) + measured **0.22s total** for all 128 squares, both piece types, + full collision-freedom verification included. At that cost, adding + it to engine startup is in the same ballpark as accepting a slightly + heavier `InitializeRookRayTables()`-style init step, not a + qualitatively different cost -- doesn't justify the complexity of a + separate offline generator tool, hand-reviewed output, and a + hand-maintained pasted-in constant block that goes stale the moment + `data.c`'s ray tables or square numbering ever change shape. + `InitMagic()` computes, in order: occupancy masks (from the existing + `g_RookRayToEdge`/`g_BishopRayToEdge`, same cost class as today's + ray-table init), then magic numbers per square via the same + fixed-seeded (not time-seeded) sparse-random search the prototype + used -- fixed-seeded so a given build produces the same magics on + every run, keeping behavior reproducible for debugging even though + nothing is hardcoded -- verifying each one collision-free against + the slow ray-walk reference before accepting it, then builds the + attack tables (measured sizes: **819,200 bytes rook, 41,984 bytes + bishop**, trivial next to the ~75MB `SEARCHER_THREAD_CONTEXT`) by + filling from the now-verified magics. All of this runs once, at + process startup, before the first `GenerateMoves` call -- no + generator tool, no pasted constants, no separate offline step to + keep in sync. +- **Verification stays a startup-time gate, not a one-time offline + check.** Since `InitMagic()` runs fresh every process launch, the + collision-freedom check runs fresh every launch too -- actually + *safer* than hardcoded constants would have been (a hardcoded magic + silently wrong for some edge case would ship broken until caught by + section 4's harness; a startup-time verification failure would abort + immediately, every single run, the moment the underlying tables or + square numbering ever drifted out of sync with the search). +- **Concrete determinism trap to avoid**: `main.c:454` calls + `srand((unsigned int)time(0))` during startup, seeding libc's shared + `rand()` with the current time. If `InitMagic()`'s search were + implemented using that shared `rand()` (directly, or via any helper + built on it), it would silently inherit that time-based seed and + produce different magic numbers -- and therefore different + attack-table contents -- on every single process launch, exactly + the non-determinism this whole design is meant to avoid, regardless + of `InitMagic()`'s own call-order relative to that `srand()` call. + `InitMagic()` must carry its own private PRNG state (the same + fixed-seeded `xorshift64*` the prototype and generator both used), + entirely independent of libc's `rand()`/`srand()` -- not a + hypothetical risk, a specific existing line of code this would + collide with if not done carefully. +- **Verification of the table-building step itself**, before it's + ever used by a generator: for every square, every occupancy subset + of that square's relevant mask must hash to a unique index with no + collision against a different subset's *different* result (a magic + number is only valid if this holds for all subsets) -- a real gate, + independent of and prior to section 4's generator-level correctness + gates, since a bad magic number silently corrupts every downstream + query. +- **PEXT vs. classic magic multiplication -- resolved, with data, not + just reputation.** This box is a Ryzen 9 3900X (Zen 2); + `sysctl`/`dmesg.boot` confirm `BMI2` is a reported CPU feature, but + Zen 1/Zen 2 are the well-known case where AMD implements `PEXT`/ + `PDEP` in microcode rather than natively, making them dramatically + slower than the multiply-based alternative despite the flag being + present. Confirmed empirically (see prototype below), not assumed: + on this exact CPU, `_pext_u64` averaged **15.08 ns/op** vs. **0.67 + ns/op** for `(occupancy * magic) >> shift` -- **~22x slower**. + **Decision: classic multiply-based magic numbers, PEXT rejected for + this box.** + +**Preliminary validation, done (2026-09-04), before any real +`data.c`/`generate.c` code was written**: a standalone scratch +prototype, `/tmp/typhoon/magic_proto/magic_proto.c` (per CLAUDE.md, +disposable/not checked in, but kept as the reference implementation +for when this becomes load-bearing code), reimplements just enough of +`chess.h`'s 0x88/`COOR_TO_BIT_NUMBER`/`COOR_TO_BB` conventions and +`data.c`'s `InitializeRookRayTables`/`InitializeBishopRayTables` shape +to stay directly portable later, and does all of: + +1. Builds the relevant-occupancy masks (ray-to-edge minus each + direction's outermost/edge square). +2. Searches for a collision-free magic number per square per piece + type via random sparse-candidate search (`rand & rand & rand`, + standard technique) against a slow ray-walk reference -- no need + for published magic constants, since local search converged + trivially fast: **9,150,194 total candidate attempts across all 64 + rook squares, 672,021 across all 64 bishop squares**, both + effectively instant. +3. Verifies every entry found this way against the slow reference + across *every* occupancy subset of that square's mask -- the + section 2a collision-freedom gate -- with **zero mismatches across + 107,648 (square, occupancy-subset) pairs** (102,400 rook + 5,248 + bishop). +4. Reports resulting attack-table sizes: **819,200 bytes (rook)**, + **41,984 bytes (bishop)** -- both far under the "few hundred KB" + estimate above, trivial next to the ~75MB `SEARCHER_THREAD_CONTEXT`. +5. Ran the PEXT-vs-multiply benchmark above. + +This clears the section 2a infrastructure gate in isolation -- +occupancy masks, magic numbers, attack tables, and collision-freedom +verification are all now proven to work end-to-end on this exact +codebase's conventions and this exact CPU, before any of it touches +`generate.c`. What's left before `GenerateRook`/`GenerateBishop` can +be written for real: porting the prototype's search/verify/table-build +logic into `data.c` proper as a single startup-time `InitMagic()` +(called alongside `InitializeRookRayTables()`/ +`InitializeBishopRayTables()`/`InitializeKnightAttackTables()` in +`main.c`'s existing startup sequence -- see the determinism trap noted +above regarding `main.c:454`'s `srand()` call), populating real +`g_Rook*`/`g_Bishop*` globals rather than prototype-local arrays. No +offline generator tool and no hand-pasted constants are part of this +plan -- see the "computed live at engine startup" decision above for +why. + ## 3. Per-function plan Ordered by expected implementation risk/complexity, cheapest and @@ -190,44 +369,205 @@ extra-cautious overhead, it's the minimum viable rollout shape. enumerable destination set; move generation needs the actual set). Same shape as knight otherwise: table lookup, AND off friendly occupancy, extract, classify, add. -3. **Rook/Bishop** (`GenerateRook`, `GenerateBishop`) -- the real test - of the segment-marking idea from section 2. For each of the 4 - relevant ray directions: find the nearest blocker (existing - mechanism from `_WhoAttacksSquareBB`), OR together `g_RookRayToEdge[ - u][c]` with the *complement* of "everything at-or-beyond the - blocker" to get the empty-square segment (or, if no blocker on that - ray, the whole ray), add the blocker itself as a capture only if - enemy. Needs a per-direction "ray up to but not including - `X`" mask -- either a new table (`g_RookRaySegmentTo[4][128][?]`, - awkward since the blocker square varies per-call, not - precomputable per-(direction, origin) pair alone) or a runtime - computation via the existing ray + blocker bit (e.g. XOR the ray - against the ray-from-the-blocker-in-the-same-direction, or a - bit-masking trick -- needs actual design work, not just table - reuse, unlike knight/king). This is where most of the real design - effort in this project lives. +3. **Rook/Bishop** (`GenerateRook`, `GenerateBishop`) -- **decided: magic + bitboards, not the 4-direction ray-walk.** A cheaper runtime-only + ray-walk/XOR alternative was considered (find nearest blocker per + direction via the existing `_WhoAttacksSquareBB` mechanism, XOR the + full ray against the ray-from-the-blocker to truncate it) and works, + reusing only already-verified infrastructure with zero new tables. + This is genuinely new infrastructure, not a reuse of section 2's + `GetAttacks`-era tables -- see section 2a for what had to be built + and verified before any generator code could use it. + + **Speed result, measured, not what was predicted going in: parity, + not a win, and that's an acceptable outcome.** The original + reasoning ("magic bitboards are strictly faster -- one multiply + + shift + lookup replaces a 4-direction ray walk") turned out to + undersell what a mailbox ray walk actually costs here: every square + a mailbox walk visits before hitting a blocker becomes an output + move, not wasted work, so its cost is already close to O(destination + count) -- the same order the magic lookup's post-lookup bit- + extraction loop pays. `_GenerateRookBB`'s benchmark (testgenerate.c's + `TestGenerateRookSpeed`, same interleaved-call methodology as + knight/king) measured, with the once-per-node + `bbOccupied`/`bbFriendlyOccupied` build cost already excluded from + the per-call number (best case for the bitboard side): rook on an + open file/rank in an endgame position (~13 destinations, the case + expected to show the biggest win) was **1.04x slower**, not faster; + blocked opening/middlegame positions were statistically tied + (0.99x). The magic lookup pipeline (5-7 sequential loads across + `bbOccupied`, the occupancy mask, the magic constant, the shift + constant, and a double-indirect load through + `g_RookAttackTable[cRook][index]`, plus a 64-bit multiply) turned + out to have a longer critical path than the handful of cheap, + branch-predictable, already-cache-resident `pos->rgSquare` accesses + a mailbox ray walk needs at these distances (max 7 squares/ + direction). Knight and king showed the identical near-parity pattern + for the same underlying reason (fixed small destination counts, + no repeated-query amortization to exploit) -- see their entries + above. + + **Why this doesn't kill the project**: magic bitboards' real + advantage is amortizing a table lookup's O(1) cost across *repeated* + queries against the same or changing occupancy (SEE-style attacker + detection probed many times per node, or eval mobility counts summed + over many squares) -- a shape move generation's "call once per piece + per node" pattern never gets to exploit. That advantage is already + real and already banked: `GetAttacks`/SEE (`MIGRATION.md`) measured + roughly a 50% win from the exact same magic-table technique, and + eval's mobility counting is expected to see a similar win for the + same repeated-query reason, once undertaken. Rook/bishop/queen's + *move-generation* migration is therefore worth finishing for section + 6's stated end goal (full mailbox retirement) even at speed parity, + not for a per-function speed win that was never actually the point + for this particular piece-type/call-site combination. Section 7's + retirement criteria ("no solve-count regression," `match_play.py` + gate) still apply in full -- parity is acceptable, an actual + regression is not. 4. **Queen** (`GenerateQueen`) -- mechanically just rook-directions + bishop-directions combined, once 3 is solved; no new design needed, same caution `_EvalQueenOccupancyBB`'s PoC comment already flagged (a combined 8-ray table measured *slower* than reusing the rook/ bishop tables in two passes -- don't rediscover that, reuse the - two-pass structure). -5. **Pawns** (`GenerateWhitePawn`/`GenerateBlackPawn`) -- do last, and - budget the most design time relative to its actual runtime cost. - Single/double push and the two capture squares are each individually - bitboard-friendly (a push mask shifted by rank, capture squares via - a new `g_PawnAttackTargetBB[2][128]` -- note this is the *opposite* - direction table from `g_PawnAttackOriginBB`, which answers "who - could attack me", not "what can I attack"; the two are not - interchangeable despite looking similar), but promotion enumeration - (4 piece types x push/capture-left/capture-right, all needing - separate `MOVE` entries) doesn't reduce to bitboard operations at - all -- that part stays a small fixed-iteration loop regardless of - how the destination squares were found. Realistic expected win here - is smaller than knight/rook/bishop/queen, possibly small enough - that it's not worth the correctness risk -- explicitly revisit - "is this worth doing" after 1-4 land and are benchmarked, rather - than assuming it's automatically worth doing because the others were. + two-pass structure). **Speed result: the one piece type with a + genuine, if position-dependent, per-function win** -- queen combines + 8 directions (rook's 4 + bishop's 4) in mailbox vs. two magic + lookups in bitboard, so mailbox pays roughly double rook/bishop's + own per-direction dispatch overhead while bitboard's fixed cost only + grows modestly; measured 0.71x (opening, queen fully blocked -- the + per-direction dispatch overhead dominates when there's nothing to + enumerate), 0.99x (middlegame), 1.09x (endgame, open board -- + extraction-loop cost reasserts the same pattern rook/bishop/knight/ + king all showed). + +**Dispatch-layer finding, found after all four piece types above +landed -- the real payoff this migration was actually hiding, not in +any individual generator function:** `_GenerateAllMoves`'s own outer +loop (`pos->cNonPawns[side][]`, a flat list mixing every non-pawn piece +type together since pieces are added/removed via swap-with-last, so +there's no contiguous per-type range to slice) dispatches via +`JumpTable[pos->rgSquare[c].pPiece]` -- an indirect call whose target +changes almost every iteration as the loop walks across mixed piece +types, close to the worst case for a CPU's indirect-branch predictor. +**Every benchmark above called its `_Generate*BB` function directly, +bypassing `JumpTable` entirely** -- none of those numbers ever +measured, or could benefit from removing, this cost. + +`pos->bbPieces[side][KNIGHT/BISHOP/ROOK/QUEEN]` sidesteps the problem +`cNonPawns` has: each piece type already has its own bitboard, so a +per-type bit-extraction loop can call `_GenerateKnightBB`/ +`_GenerateBishopBB`/`_GenerateRookBB`/`_GenerateQueenBB` **directly, by +name** -- a statically-known, likely-inlinable call, no function +pointer anywhere. `_GenerateAllMovesBB` (`generate.c`, exposed non- +static for benchmarking) implements exactly this: four per-type bit- +extraction loops plus a direct king call (king has no bitboard of its +own, a single square is already all `GetAttacks` ever needed), pawns +unchanged (copied verbatim from `_GenerateAllMoves`'s tail, out of +scope per step 5 below). Forked as a **whole separate function**, not +a branch nested inside `_GenerateAllMoves`, and swapped in via a +`#define _GenerateAllMoves _GenerateAllMovesBB` (matching `chess.h`'s +`GetAttacks` precedent) gated on **all five** piece-type toggles being +defined together -- a partial-rollout mix still needs +`_GenerateAllMoves`'s `cNonPawns`/`JumpTable` path, since only that +path knows how to fall back to a still-mailbox piece type while also +finding already-migrated ones in the same mixed list; +`_GenerateAllMovesBB` does not attempt to support partial rollout. + +**Measured result (`testgenerate.c`'s `TestGenerateAllMovesSpeed`, +same interleaved methodology, but comparing whole-node generation, not +a single piece): a genuine win**, in the positions that actually +exercise the mechanism: + +``` +opening : mailbox 329 cycles/call, BB dispatch 255 cycles/call (0.77x) +middlegame: mailbox 431 cycles/call, BB dispatch 411 cycles/call (0.95x) +endgame : mailbox 391 cycles/call, BB dispatch 432 cycles/call (1.10x) +``` + +Opening/middlegame (dense, many mixed piece types -- exactly where +`JumpTable` has to jump between wildly different targets almost every +iteration) show a real win, up to 23%. The endgame test position +(sparse -- few total pieces, so few dispatch decisions for +misprediction to cost anything on) regresses slightly, consistent with +the per-function findings above (the few pieces present are on a wide- +open board, paying the same per-call fixed-lookup cost that lost in +every isolated open-position benchmark). Net story: the dispatch-level +win dominates in typical richer positions; the per-generator cost +dominates in sparse/wide-open ones -- a coherent, not noisy, result. + +Correctness: perft (`TestMoveGenerator`) passes both against the +toggle-free baseline (`_GenerateAllMoves` under its own name, unrenamed +since the macro-swap condition is false) and against a build with all +five piece-type toggles defined together (macro-swap active, +`_GenerateAllMovesBB` substituted at all four of `GenerateMoves`'s call +sites). + +This is the actual justification for finishing Part A even setting +aside individual-function parity -- the win was always going to live +in the dispatch layer once enough piece types migrated to make +`pos->bbPieces`-driven iteration possible at all, not in any one +generator beating mailbox on its own. +5. **Pawns** (`GenerateWhitePawn`/`GenerateBlackPawn`) -- **implemented, + done last as planned, but not the way originally sketched above.** + The pre-implementation guess (a new `g_PawnAttackTargetBB[2][128]` + per-square table, generated one pawn at a time like the other five + piece types) turned out to be the wrong shape entirely. Confirmed + via `~/crafty/movgen.c` before implementing (the standard technique, + not something specific to this codebase): `pos->bbPawns[side]`'s + bits already live in dense `rank*8+file` space + (`COOR_TO_BIT_NUMBER`), so shifting the *entire* bitboard by 8 moves + every pawn of that side forward one rank *simultaneously* -- no + per-square table, no per-pawn loop to find destinations, only to + emit the resulting moves. `_GenerateAllPawnMovesBB` + (`generate.c`) generates an entire side's pawn moves in one call: + + - **Single push**: `(bbPawns >> 8) & empty` for White, `<< 8` for + Black -- this engine's square numbering has A8 = bit 0 (opposite + of Crafty's convention), so White's forward direction is a + *right* shift here, not left; got this from re-deriving the + `RANK`/`RANK1`/`RANK8`/`A1`/`A8` macros directly rather than + assuming Crafty's shift directions would carry over. + - **Double push**: mask the single-push *destination* bitboard + against `BBRANK[3]`/`BBRANK[6]` (did this pawn's single push land + on rank 3/6, only possible starting from rank 2/7) before shifting + again -- same technique Crafty uses (`padvances2`), no per-square + starting-rank table needed, reusing the already-existing `BBRANK[]` + table instead of building a new one. + - **Captures**: `+-7`/`+-9` diagonal shifts (one rank plus one file), + each masked against the *opposite* file (`BBFILE[0]`/`BBFILE[7]`) + before shifting, to stop a same-row wraparound -- an h-file pawn's + naive `>>7` would otherwise silently land back on the same row's + a-file, a silent-wrong-answer trap, not an out-of-range index. + Verified against `bbEnemy` (occupied minus friendly), same + convention as the other five generators. + - **En passant**: deliberately *not* bulk -- checked directly (do + either of the two squares diagonally behind `pos->cEpSquare` hold + one of this side's pawns), since it's at most one event per node + and not worth deriving a whole extra masked bitboard for. + - **Promotions**: confirmed correctly not bitboard-reducible, exactly + as predicted -- `_AddPromote`'s 4-piece-type enumeration loop is + unchanged, just reached via `RANK8(cTo) || RANK1(cTo)` on each + extracted destination instead of a per-square rank check. + + Gated behind its own `GENERATE_PAWN_BITBOARD` toggle (independent of + the other five, per section 6), wired into both `_GenerateAllMoves`'s + and `_GenerateAllMovesBB`'s pawn tails. + + **Correctness**: perft (`TestMoveGenerator`) clean in a `TEST=1` + build, and clean in a `TEST=1 DEBUG=1` build exercising every + `ASSERT` added (including sanity checks on the reverse-shift + origin-square recovery and capture-color validation) -- first-try + correct on genuinely hand-derived shift/mask arithmetic, which the + perft harness (externally-verified leaf counts, including a + castling/en-passant-heavy position) would have caught immediately + had the direction, shift amount, or edge mask been wrong in either + color. + + **Speed**: not yet isolated-benchmarked the way the other five were + (no `TestGenerateAllPawnMovesSpeed` written) -- worth doing before + this toggle is considered for default-on, but lower priority than + getting section 4's full gate run at least once across everything + implemented so far. ## 4. Correctness verification @@ -324,28 +664,476 @@ functions.** Checked directly -- `GenerateKnight`/`GenerateRook`/ are referenced only from `_GenerateAllMoves`'s `JumpTable[]` and the pawn-specific dispatch beside it; `_GenerateEscapes` has its own, independent mailbox implementation. This means the per-piece-type -toggle above only ever covers the *not-in-check* path -- a real, -previously-unstated scope gap. Two options, not resolved by this -document: +toggle above only ever covers the *not-in-check* path. + +**Decided: this is a two-part project, not seven-vs-eight targets in +one pass.** The end goal is full mailbox retirement in `generate.c`, +so `_GenerateEscapes` is in scope -- but as **Part B**, done only after +**Part A** (the seven not-in-check generators, sections 3/4/5/7 as +written) is fully landed, retired, and re-baselined into +`head_reference/`. Reasons to sequence rather than parallelize: + +- Part A already establishes every piece of infrastructure Part B + needs (segment-marking mechanism, per-type toggle pattern, perft + + move-set comparison harness shape, `testgenerate.c` itself) -- + building `_GenerateEscapes`'s bitboard version first, or alongside, + would mean designing that infrastructure against an unusual, + narrower-scoped caller (single-checker blocking/capturing moves, + possibly king moves out of check) before it's been proven against + the general case. +- `_GenerateEscapes` is called on a minority of nodes (most positions + aren't in check), so it's correctly the lower-priority half of the + NPS win -- no reason to hold Part A's already-larger win hostage to + Part B's design work. +- Keeps the retirement-criteria checklist (section 7) honest per the + existing principle of not letting one target's clean bill of health + lower the bar for another -- Part B gets its own full pass through + that checklist once it starts, not a discount for arriving after + Part A proved out the pattern. + +Part B's own design questions were left undecided in an earlier draft +of this document, to be scoped only after Part A landed -- **that +scoping pass happened (2026-09-04, before Part A's own section 4 gate +was run, at the user's direction) and is written up in section 6a +below.** `TestMoveGenerator`'s existing `PlyTest` already exercises +both `GENERATE_ALL_MOVES` and `GENERATE_ESCAPES` (the `fInCheck` +branch, generate.c:58) -- section 4's move-set comparison harness only +needed to cover the not-in-check path for Part A; Part B's own +correctness gate will need to exercise the `fInCheck` branch +specifically when implementation starts. + +## 6a. Part B design (`_GenerateEscapes`) + +Traced end to end (`_FindUnblockedSquares`, `_GenerateEscapes`, all six +`SaveMeFoo` functions, the `BLOCKS_THE_CHECK` macro, `IsAttacked`, +`_WhoAttacksSquareBB`) before writing any code, same discipline as +Part A's per-function plan. Two separate findings came out of this, +one a scoping correction and one a design that's a genuine +simplification, not just a reimplementation. + +**Scoping correction: `_FindUnblockedSquares` is not actually +`_GenerateEscapes`'s precondition.** It looked that way structurally +(a "queen standing on a square, walk all 8 rays" ray-walk, same shape +`_GenerateRookBB`/`_GenerateBishopBB` already use), but tracing its +call sites shows it runs on **every** `GenerateMoves` call (all four +`GENERATE_*` cases), keyed off the *opposing* king's square, building a +per-square reverse-pointer table (`pStack->sUnblocked[uPly][]`) that +`WouldGiveCheck` later uses to cheaply detect discovered checks -- +nothing specific to the in-check path. It's a legitimate, separate +bitboard-migration candidate (same ray-walk shape, its own `#define` +toggle), but it does **not** belong inside Part B's scope; recording it +here so it isn't lost, not because it's being done now. + +**Phase 1 (king flight) -- a genuine simplification, not just a +port.** Today's mailbox code (`_GenerateEscapes`'s first loop) walks +`g_iQKDeltas`, calls mailbox `IsAttacked(c, pos, enemy)` per candidate +square, then runs a **second, manual loop over every checker** +specifically to catch a case `IsAttacked` gets wrong: `IsAttacked` has +no way to test against a hypothetical occupancy, so a candidate escape +square can come back "safe" purely because the king's own *still +physically present* body is blocking a slider's ray from extending +past it -- the classic x-ray/discovered-attack-when-stepping-back +problem. The existing code works around this by hand, checking each +checker's direction against each candidate square's direction from the +king. + +`_WhoAttacksSquareBB` (`see.c`, currently `static`, would need +exposing) already takes an explicit `bbOccupied` parameter for exactly +this reason -- it was built for `GetAttacks`, not for this, but the +capability is already there. The fix: compute +`bbOccupiedWithoutKing = pStack->bbOccupied & ~COOR_TO_BB(cKing)` once, +then for each candidate in `g_KingAttacksBB[cKing] & +~bbFriendlyOccupied`, test `_WhoAttacksSquareBB(pos, c, enemy, +bbOccupiedWithoutKing) == 0` (plus a separate pawn-attack check via +`g_PawnAttackOriginBB`, since `_WhoAttacksSquareBB` deliberately +excludes pawns -- see its own header comment). **This doesn't just +port the existing logic, it deletes the manual per-checker x-ray loop +entirely** -- testing against king-vacated occupancy handles that case +for free, because a slider whose ray was blocked only by the king's own +body will now correctly show up as attacking `c` if `c` is still on +that ray. + +**Phase 2 (block-or-capture by a non-king piece) -- collapses six +per-square-loop functions to one AND each.** Today, `SaveMeKnight`/ +`SaveMeBishop`/`SaveMeRook`/`SaveMeQueen`/`SaveMeWhitePawn`/ +`SaveMeBlackPawn` each re-walk their piece's own move pattern and test +`(c == cAttacker) || BLOCKS_THE_CHECK(c)` per candidate square. The +bitboard design collapses this to one precomputed mask, intersected +once per piece instead of tested once per square: + +- `bbTargetMask` = the checker's own square (a capture always resolves + check) OR, when the checker is a slider, every square strictly + between it and the king (a block also resolves check). The + "squares between two aligned squares" bitboard is a well-known + trick, and it costs nothing new here: `bbBetween = RookAttacks(cKing, + occ) & RookAttacks(cAttacker, occ)` (rook or bishop table, whichever + the checker's line matches) -- each square's magic-table attack + bitboard already reaches exactly to its nearest blocker in every + direction, so ANDing both sides' attack sets from each end gives + precisely the empty segment between them. This is a **direct + consumer of `InitMagic()`'s tables for something other than move + generation** -- the first sign the magic-table investment pays off a + third time (after `GetAttacks`/SEE and this), independent of eval + mobility. +- Once `bbTargetMask` is computed, every already-migrated piece's + `SaveMeFoo` collapses from a per-square loop to one line: `bbDest = + <that piece's already-computed attack bitboard> & bbTargetMask` + (knight: `g_KnightAttacksBB[c] & ~bbFriendlyOccupied & bbTargetMask`; + rook/bishop/queen: the same magic lookups `_GenerateRookBB`/etc. + already perform, ANDed with `bbTargetMask` instead of just + `~bbFriendlyOccupied`). No per-square `BLOCKS_THE_CHECK` branch + anywhere -- an actual structural simplification versus Part A's own + generators, which still needed a per-square classification step. +- Pawns: same bulk shift-and-mask technique as + `_GenerateAllPawnMovesBB`, with each of the four move-category + bitboards (single push/double push/capture-left/capture-right) ANDed + against `bbTargetMask` before extraction. En passant stays a direct, + narrow special case exactly as today -- `SaveMeWhitePawn`'s existing + comment already notes it only applies when the double-jumping pawn + *is* the checker, genuinely rare, not worth deriving a bulk mask for. + +**Missed in the first pass of this section, caught by the user while +Phase 1 was being implemented: Phase 2 has the exact same +dispatch-layer problem `_GenerateAllMoves` had, and the fix is the +same pattern.** The bullets above only addressed collapsing each +`SaveMeFoo`'s *internal* per-square loop -- they didn't address the +*outer* loop that calls them: + +```c +for (u = 1; u < pos->uNonPawnCount[pos->uToMove][0]; u++) { + cDefender = pos->cNonPawns[pos->uToMove][u]; + ... + (JumpTable[p])(pStack, pos, cDefender, cKing, c); +} +``` + +`pos->cNonPawns[side][]` mixes every non-pawn piece type together +(same reason as `_GenerateAllMoves`'s loop -- pieces are added/removed +via swap-with-last, no contiguous per-type range to slice), so +`JumpTable[p]` is the identical indirect-call-with-changing-target +pattern that `_GenerateAllMovesBB` was built to eliminate. The fix is +the same one, applied here: a `_GenerateEscapesBB`-style function +loops `pos->bbPieces[side][KNIGHT/BISHOP/ROOK/QUEEN]` directly (once +`bbTargetMask` is computed) and calls each specific `SaveMeFooBB` +function **by name** -- no function pointer, same +statically-known-call-target win `_GenerateAllMovesBB` already +demonstrated (measured up to 23% faster in dense positions, section +3's writeup after step 4). Given Part A's `_GenerateAllMovesBB` +already proved this exact mechanism out, expect Part B's dispatch fork +to pay off the same way, for the same reason -- worth building +regardless of whether each individual `SaveMeFooBB`'s per-square-loop +collapse (the bullets above) shows a win in isolation, same lesson +Part A's per-function benchmarks already taught. + +**What does not change (behaviorally)**: `ExposesCheck`'s pin-detection +*outcome* is unchanged -- called per surviving candidate move the same +way the mailbox `SaveMeFoo` functions call it today, including its own +documented bug (a pinned piece's capture can still slip through as +pseudo-legal). Not in scope to fix, per section 1's non-goal against +becoming more legal-aware than the code being replaced; must replicate +bug-for-bug like everything else in this migration. **`ExposesCheck`'s +own implementation, however, turned out to be a separate, higher-value +bitboard target in its own right** -- see section 6b, added after +finishing Phase 1/Phase 2 and auditing the rest of `movesup.c` at the +user's direction. The `GetAttacks(&rgCheckers, ...)` call that finds +who's checking is unchanged -- already covered by the existing +`GETATTACKS_BITBOARD` toggle from `MIGRATION.md`, orthogonal to this +work. + +Toggle granularity for Part B ended up being two independent flags, +`GENERATE_ESCAPES_KING_BITBOARD` (Phase 1) and +`GENERATE_ESCAPES_BLOCK_BITBOARD` (Phase 2) -- both implemented, see +their own writeups above. A separate whole-function `_GenerateEscapesBB` +fork (mirroring `_GenerateAllMovesBB`) turned out to be unnecessary: +Part A's fork was required because five independent piece-type toggles +all had to agree before `pos->bbPieces`-driven iteration became +possible at all; Phase 1 and Phase 2 are independent sections of the +same function, not five orthogonal toggles gating one shared loop, so +two `#if` blocks inside `_GenerateEscapes` deliver the identical +dispatch-elimination win (Phase 2's `cNonPawns`/`JumpTable` loop, +caught by the user while Phase 1 was landing) without needing a +parallel top-level function. + +## 6b. `movesup.c` survey -- what else is bitboard-eligible + +Prompted by a direct question after Phase 1/Phase 2 landed: `generate.c` +isn't the only file with mailbox board-query helpers. `movesup.c` holds +several, called from all over the engine (`search.c`, `move.c`, +`eval.c`, `root.c`, `dynamic.c`, `san.c`), not just from move +generation. Every function in the file was read and categorized -- +three real categories emerged, not two, and the exercise surfaced a +scoping correction to this document's own "retire the mailbox" framing. + +**Category A -- bitboard-eligible, and it matters for performance:** + +- **`IsAttacked`** -- corrects an earlier (wrong) read of this same + function from earlier in this session: its *direct* callers + (`san.c`, `move.c`) are cold castling-through-check checks, but + `InCheck` is a thin wrapper around it, and `InCheck` has **70 call + sites** across `search.c`/`move.c`/`eval.c`/`root.c`/`dynamic.c` -- + called constantly through search, not a cold path at all. Design: + `_WhoAttacksSquareBB(pos, cTest, uSide, bbOccupied) != 0` plus a + separate pawn-attack check via `g_PawnAttackOriginBB` (same pattern + Phase 1's king-flight code already uses inline). +- **`InCheck`** -- a two-line wrapper around `IsAttacked`. Benefits + automatically once `IsAttacked` has a bitboard version; no separate + design needed. +- **`ExposesCheck`** -- called from `MakeMove` (`move.c`) on + essentially every move actually played during search, the pin- + legality safety net every generator's header comment references. + Design: exclude the hypothetically-removed square from occupancy + (`bbOccupied & ~COOR_TO_BB(cRemove)`), magic-lookup the attack set + from `cLocation` against that occupancy, mask to the single ray + through `cRemove` via `g_RookRayToEdge`/`g_BishopRayToEdge` (so an + unrelated attacker on a different ray through `cLocation` can't + falsely register), then apply the same enemy-color/piece-type check + the mailbox version does on whatever single square survives. +- **`FasterExposesCheck`** -- identical call shape to `ExposesCheck` + minus the initial alignment pre-check (caller already knows exposure + is geometrically possible); same bitboard design, minus the early-out. +- **`ExposesCheckEp`** -- the en passant variant, checking whether + capturing en passant would expose check via the rank the capturing + and captured pawns both sat on. Same trick, with *two* squares + excluded from occupancy instead of one (the moving pawn's origin and + the captured pawn's square). + +**Category B -- bitboard-expressible, but converting buys nothing:** +`SanityCheckMove`, `_SanityCheckPieceMove`, `_SanityCheckPawnMove` -- +all `DEBUG`-only, called exclusively inside `ASSERT(SanityCheckMove( +...))`, compiling to nothing in a release build. `_SanityCheckPieceMove`'s +"is the path from `cFrom` to `cTo` clear" ray-walk is actually the +*cleanest* possible bitboard reduction found anywhere in this survey +(`_RookAttacksBB(cFrom, occ) & COOR_TO_BB(cTo) != 0` -- one lookup, one +bit test, simpler than anything needing a between-squares mask) -- but +since none of these three ever execute in release, there is no +performance to gain from converting them. Worth doing only if/when the +structural "retire mailbox reads" goal below is pursued, never as a +speed item. + +**Category C -- no mailbox-vs-bitboard axis exists at all:** +`LooksLikeFile`/`LooksLikeRank`/`LooksLikeCoor`/`StripMove`/ +`LooksLikeMove` (pure string parsing -- most don't even take a +`POSITION*`); `SelectBestWithHistory`/`SelectBestNoHistory`/ +`SelectMoveAtRoot` (scan an already-generated `MOVE_STACK` by score/ +history, never touch board occupancy); `Perft`/`PerftCommand` (pure +recursive driver over `GenerateMoves`/`MakeMove`/`UnmakeMove`, no +direct board access). None of these read `pos->rgSquare` today and +none would need to under any board-representation change. + +**Clarification on Category B and C's `MOVE`-focused members** +(`SanityCheckMove` and friends, `LooksLikeMove`/`StripMove`): these +take or produce a `MOVE`, and `MOVE`'s `cFrom:8`/`cTo:8` encoding is +already frozen by section 1's existing non-goal (the same exclusion +`MIGRATION.md` made originally, for the same reason -- a much bigger, +separate project). Their *internals* could still switch to bitboard +board-queries (Category B's point above), but their signatures and +purpose -- validating/parsing a `MOVE` against a `POSITION` -- never +change regardless of how the board itself is represented. These were +never "generator helpers" in the sense `IsAttacked`/`ExposesCheck` are; +worth stating precisely so a future pass doesn't conflate "migrate +board queries to bitboards" with "change the `MOVE` representation," +two entirely different and already-separately-scoped projects. + +**Scoping correction to this document's own "retire the mailbox" framing, +surfaced by actually doing this survey**: even Category B's functions, +and every already-migrated `_Generate*BB` function in Part A (knight/ +king/rook/bishop/queen/pawn), still call `pos->rgSquare[c].pPiece` to +answer "what's on this destination square" for move classification +(quiet vs. capture). Bitboards answer "where are my knights" cheaply; +they don't answer "what's on square X" without a per-piece-type +membership scan across up to 8 bitboards. This means **`pos->rgSquare[]` +is almost certainly not fully removable**, no matter how much of +`generate.c`/`movesup.c` migrates to bitboard-driven logic. "Retire the +mailbox" (this document's stated end goal, section 6/7) should be read +as retiring the mailbox *move-generation and board-query loops*, not +literally deleting the square-indexed array -- every generator, bitboard +or not, depends on it for O(1) single-square classification, and that +dependency doesn't go away just because the *destination-finding* logic +became bitboard-native. + +**`ExposesCheck`/`FasterExposesCheck`/`ExposesCheckEp` -- implemented, +debugged, and validated (2026-09-04).** `movesup.c` now has +`ExposesCheckBB`/`FasterExposesCheckBB`/`ExposesCheckEpBB`, gated behind +a single `EXPOSESCHECK_BITBOARD` toggle (`#define`-swapped in via +`chess.h`, same pattern as `GetAttacks`). One real wrinkle in the +macro-swap itself: unlike `GetAttacks` (a real asm symbol living in a +*different* file, so the macro never touches its own definition), these +three mailbox functions are defined in the *same* file as their BB +counterparts -- the macro would otherwise rename `movesup.c`'s own +function definitions too, colliding with the real `*BB` symbols. Fixed +with `#undef` immediately after `#include "chess.h"`, restoring the +real names for this file's own definitions while every other +translation unit still sees the macro-renamed calls. + +**Two real bugs found and fixed** getting this correct, both caught by +the existing correctness harness exactly as designed, not by manual +inspection: + +1. **Ray-direction sign was backwards on the first attempt.** + `CHECK_DELTA_WITH_INDEX(cLocation - cRemove)`'s actual convention + (confirmed by reading `InitializeVectorDeltaTable`'s table- + construction loop in `data.c`, not by guessing) is "the direction to + step from `cLocation` *toward* `cRemove`" -- the first attempt + negated this unnecessarily. Caught by `TestSan` failing on a + castling/pin-disambiguation test case. +2. **A more fundamental design flaw**, caught only after the sign fix, + by a `DEBUG`-build assertion (`movesup.c:117`, + `ASSERT(!IS_EMPTY(xPiece))`) rather than a leaf-count mismatch: the + first design ANDed the *full* multi-directional magic attack + bitboard against a single-direction ray mask, assuming this would + isolate exactly one bit (the nearest blocker) or zero. Wrong -- an + *unblocked* ray's attack bitboard contains every empty square out to + the edge, so the intersection can contain many bits, and + `FastFirstBit` on that set picks the lowest absolute square index, + which is not necessarily nearest to `cLocation` (bit-index order and + "distance from origin" only coincide for one of the two directions + along any given ray). Fixed by abandoning the magic-lookup approach + for this specific query entirely and mirroring + `_WhoAttacksSquareBB`'s (`see.c`) already-proven nearest-blocker + technique instead: isolate the lowest set bit for a "positive" + direction (`bb & -bb`) or the highest set bit for a "negative" + direction (`1ULL << (FastLastBit-1)`), using the existing + `g_RookRayPositiveDir`/`g_BishopRayPositiveDir` tables to know which. + New shared helper: `_NearestBlockerAlongRayBB`. + +**Validation**: perft-based hand-crafted repros (a straight-line rook +pin, a diagonal bishop pin, both en-passant-discovered-check example +FENs already in `generate.c`'s comments) all matched baseline exactly +once both fixes landed -- but the strongest confirmation came from +external ground truth: **chessprogramming.org's "Position 4"** +(`8/2p5/3p4/KP5r/1R3p1k/8/4P1P1/8 w - -`, chosen deliberately for its +heavy en-passant/pin content) matched the published reference exactly +to depth 8, and **"Kiwipete"** +(`r3k2r/Pppp1ppp/1b3nbN/nP6/BBP1P3/q4N2/Pp1P2PP/R2Q1RK1 w kq -`) matched +to depth 6 -- both are standard, deliberately adversarial community +test positions specifically because they catch exactly this class of +en-passant/pin/castling-legality bug. A separate, intermittent +`TestSearch` failure (hits `recogn.c`, `probe.c`, or `util.c` on +different runs, always inside the material-recognizer/tablebase- +agreement or PV-formatting subsystem, never inside move generation) +was investigated in parallel and is **not** related to this work -- +`_SanityCheckRecognizers`'s tablebase cross-check calls `ProbeEGTB` +directly against raw position bitboards/material counts, with no +dependency on `MakeMove`, `ExposesCheck`, or move generation at all. +Pre-existing, `GenerateRandomLegalPosition`-triggered flakiness in the +recognizer subsystem, out of scope for this document. + +`IsAttacked`/`InCheck` (the other Category A target from this section) +remain unstarted. + +**All nine toggles combined, verified together for the first time +(2026-09-04): a third real bug found, this time in the test harness +itself, not in any of this migration's code.** Running every Part A/ +Part B/`EXPOSESCHECK_BITBOARD` toggle simultaneously (the first time +this combination had been tried -- everything up to this point was +tested individually or in small groups) surfaced an intermittent +segfault in `TestSearch`'s random-position loop, distinct from the +already-diagnosed `recogn.c`/`util.c` recognizer flakiness above. Root +cause: `GenerateRandomLegalPosition` (`testsup.c`) `memset`s the whole +`POSITION` to zero and explicitly resets `cPawns`/`cNonPawns` to +`ILLEGAL_COOR` afterward, but never touches `cEpSquare` -- leaving it +at `0x00` (square A8, a real on-board square) instead of the "no en +passant" sentinel. Every single randomly-generated test position +therefore had a bogus "en passant available on a8" flag active. Both +mailbox and bitboard pawn code trust this field unconditionally (by +design -- it's supposed to be set only by `MakeMove` after a genuine +double-pawn-jump); the bitboard generators +(`_GenerateAllPawnMovesBB`/`_SaveMeAllPawnMovesBB`) gate their en +passant handling on `IS_ON_BOARD(cEpSquare)` *proactively*, once per +side per node, versus mailbox's more incidental per-pawn +`cTo == cEpSquare` check -- meaning the bitboard path exercises this +latent test-harness bug far more consistently than mailbox ever did, +which is how a pre-existing harness defect turned into a new-looking +crash. Whenever a real pawn happened to occupy b7 (the one square +diagonally behind the bogus a8 target) in a random position, a garbage +"en passant capture" move got constructed and fed to `MakeMove`, +corrupting position state. Fixed with one line +(`pos->cEpSquare = ILLEGAL_COOR;`) added right alongside the existing +`cPawns`/`cNonPawns` resets in `GenerateRandomLegalPosition`. Also +added FEN logging to `TestSearch`'s random-position loop +(`Trace("TestSearch position %lu/20: %s\n", ...)`, via `PositionToFen`) +so a future intermittent failure has its exact triggering position +captured automatically, matching `debug_smoke_test.sh`'s existing +convention -- this fix took real time to find precisely because the +triggering position wasn't logged anywhere. + +Post-fix: 15/15 clean runs with all nine toggles combined (`TEST=1`, +no `DEBUG`), plus clean `DEBUG`-build runs beforehand. `precommit_check.sh` +also passes clean on the untouched, all-toggles-off default path, +confirming none of this migration's extensive edits affected anyone +not opting in. + +**Whole-engine `sd10` curated-suite check against `head_reference/`, +with all nine toggles combined**: `ecm_ringers` 10/11 and +`ecm_confident_quick` 84/90 both match `head_reference` exactly. +`ecm_hard_quick` showed 25/90 against a recorded baseline of 28/90 -- +investigated and **confirmed unrelated to this migration**: rebuilding +plain current-HEAD mailbox (every toggle off) reproduces the identical +25/90, proving the 3-solve delta comes entirely from intervening, +non-toggle-gated commits that landed between `head_reference`'s +baseline and current HEAD (`5c8d794` "Fix passed-pawn bitboard bit-clear +bug, LMR gate coupling..." and `a8806ad` "Fix draw-score bug in +hash-hit path" are the likely candidates, both already-committed, +default-on, and unrelated to move generation). **Net result: zero +solve-count regression attributable to this migration** across all +three curated suites. + +**`IsAttacked`/`InCheck` -- implemented, tested properly this time, and +a genuine speed win.** `movesup.c` now has `IsAttackedBB`/`InCheckBB`, +gated behind `ISATTACKED_BITBOARD` (same three-way `#define`-swap +pattern as `EXPOSESCHECK_BITBOARD`, including the same `#undef` fix in +`movesup.c` for the same same-file-definition reason). Design: +`_WhoAttacksSquareBB(pos, cTest, uSide, bbOccupied) != 0` plus a +separate pawn check via `g_PawnAttackOriginBB` (mirroring exactly what +Phase 1's king-flight code already did inline, now factored into a +reusable, directly-testable function). + +**One real ordering bug caught before it shipped, not after**: the +first draft placed the `#define IsAttacked IsAttackedBB` block *before* +the real mailbox function's own declaration in `chess.h` -- when the +toggle is active, that ordering means the mailbox declaration line +itself gets macro-substituted too, so the plain name `IsAttacked` is +never actually declared anywhere under that name. Harmless as long as +nothing needs to call the mailbox version by its real name explicitly +-- which is exactly what the new comparison-harness test does, and +which is why the bug surfaced immediately as a compile error rather +than shipping silently. `GetAttacks`'s existing three-way block already +gets this ordering right (real function declared first, unconditionally, +*then* the `#define`); `ExposesCheck`'s block happened to already be +correct too (its mailbox declarations pre-dated the `#define` insertion +point). Fixed by reordering to match `GetAttacks`'s pattern exactly. -1. Treat `_GenerateEscapes` as an eighth migration target with its own - design/correctness/benchmark pass (likely smallest-scope-first - candidate again, e.g. does it even have a slider-blocker-walk - shape, or is it already simpler than the general case since it's - specifically "moves that address a single check"?), or -2. Leave `_GenerateEscapes` on the mailbox path indefinitely even - after the other seven functions migrate, accepting that in-check - nodes don't get the speedup. Plausible if `_GenerateEscapes` turns - out to be called rarely enough (most nodes aren't in check) that - its contribution to whole-engine NPS is small regardless. +**Tested properly from the start this time** -- unlike `ExposesCheck`, +which shipped without a direct comparison harness and had to be +debugged after the fact via `TestSan`/perft/`DEBUG`-assert failures: +`TestIsAttackedBB` (`testsee.c`, modeled directly on `TestGetAttacks`) +compares `IsAttacked` vs. `IsAttackedBB` and `InCheck` vs. `InCheckBB` +across `GenerateRandomLegalPosition`'s full 20,000-position sweep, every +square, both colors -- **clean on the first try**, no debugging odyssey +required. Also benefits for free from `testmove.c`'s pre-existing +`TestIsAttacked` (a curated table of tricky attacker-geometry fixed +positions -- knight forks, pawn attacks, blockers, x-rays, pins) -- +since that test calls the plain `IsAttacked`/`InCheck` names, the +`ISATTACKED_BITBOARD` toggle routes it through the bitboard version +too, for free, no changes needed to that test. -Whichever is chosen, `TestMoveGenerator`'s existing `PlyTest` already -exercises both `GENERATE_ALL_MOVES` and `GENERATE_ESCAPES` (the -`fInCheck` branch, generate.c:58) -- section 4's move-set comparison -harness needs to do the same, not just exercise the not-in-check path, -regardless of which option above is picked. +**Speed**: a genuine, consistent win, unlike most of Part A -- +`TestIsAttackedBB`'s isolated benchmark measured **0.73x-0.93x of +mailbox** across opening/middlegame/endgame. Makes sense structurally: +mailbox `IsAttacked` loops over *every* one of `uSide`'s non-pawn +pieces doing an alignment check each time (O(piece count)), while +`_WhoAttacksSquareBB` gets O(1) knight/king lookups plus an early bulk +"is anything even aligned with this square at all" check +(`g_RookRayAll`/`g_BishopRayAll`) before paying for any per-direction +work -- a real algorithmic difference, not just a constant-factor one, +unlike move generation's per-square walks where mailbox's per-square +work was already close to minimal. -## 7. Retirement criteria +Category A (section 6b) is now fully implemented: +`ExposesCheck`/`FasterExposesCheck`/`ExposesCheckEp` and +`IsAttacked`/`InCheck` both done, both correctness-verified, one with +a genuine speed win and one at parity-ish-but-cleaner-tested. `movesup.c`'s +survey from earlier in this section is fully worked through. Per piece type, only delete that type's mailbox generator function after **all** of: |