Unify HelpSearch() onto SearchOneMove (UNIFY_SEARCH.md step 4)

HelpSearch() now calls the same per-move kernel Search() uses
(searchsup.c's SearchOneMove, extracted in step 2), instead of its
own hand-rolled inline copy. This is the actual unification: helper
threads gain the real CanWeReduceOrPruneMove() gate (9 base checks +
2 move-identity exemptions) and ShouldLatePrune()'s LMP, neither of
which HelpSearch had before -- its old inline LMR-only gate was
missing the SEE-negative-capture carve-out, the mate-magnitude-alpha
check, fThreat, and both killer/countermove exemptions. It also now
gets real attack-table snapshots (bbMinorAttacksSnapshot/
bbRookAttacksSnapshot) and fThreat from g_SplitInfo (step 3's
plumbing), instead of the zeroed snapshots and missing-fThreat
degraded inputs the old inline LMR lookup used.

Per-call parameter mapping, not obvious from SearchOneMove's
search.c call site alone:
- x is chosen so (x - 1) reproduces mv's absolute index in
  ctx->sMoveStack (uBegin[ctx->uPly] + uMoveNum, computed pre-MakeMove
  while ctx->uPly is still the split ply) -- the same relationship
  search.c's own x maintains for its callers inside the kernel.
- uLegalMoves is passed PRE-increment (g_SplitInfo[u].uAlreadyDone +
  uMoveNum, not + 1): SearchOneMove itself increments it once right
  after MakeMove succeeds, so passing the already-"+1" value would
  double-count relative to what the old inline code's
  uSplitLegalMoves computed.
- fFullWindow is always FALSE (a helper thread's first pulled move is
  never this node's actual first move -- uAlreadyDone >= 1 always,
  since a split only launches after the originating thread already
  searched at least one move with the full window).
- fIsGeneratedMoveStage is always TRUE, fIsLeftoverMove is always
  TRUE -- both already true of every move HelpSearch handles, same
  as the removed inline code's own assumptions.
- MOVE_ORDERING_EVIDENCE_COLLECTION/TEST_NULL instrumentation gets
  FALSE/0/throwaway-output -- confirmed with the user these
  diagnostics are single-threaded-only by design, not a gap to fill.

Two real, DEBUG-only invariant breaks found and fixed getting this to
pass under real multithreading (both confirmed via a DEBUG=1 --cpus 4
stress run across all three curated suites, not just the fast smoke
test):

1. SearchOneMove's own post-UnmakeMove sanity ASSERT compares against
   ctx->sPlyInfo[ctx->uPly].sPosition (pi->sPosition), which Search()
   populates once at node entry -- but a helper thread never calls
   Search()/CommonSearchInit for the split-point ply itself (see
   eNodeType's own SPLIT_INFO comment), so that slot was stale
   leftover data from whatever unrelated node the thread last helped
   with. Fixed by having HelpSearch() populate it itself, once before
   the move loop, matching Search()'s own convention.

2. StartParallelSearch's post-split DEBUG assert ("fail-low implies
   every candidate was searched", split.c ~line 801) no longer holds
   universally: a helper thread can now legitimately stop pulling
   moves from a failing-low split via MOVE_KERNEL_ABORTED when
   WE_SHOULD_STOP_SEARCHING's MP definition fires for a reason
   unrelated to this split's own outcome (ThreadUnderTerminatingSplit
   -- an ANCESTOR split up this thread's call stack terminated).
   The old inline code never checked WE_SHOULD_STOP_SEARCHING
   explicitly, so it never bailed early for this reason either; this
   is a correctness improvement (stop grinding through a doomed
   split's remaining moves once told to), not a regression, so the
   assert was loosened to tolerate it rather than reverting the
   early-exit.

Verified: precommit_check.sh passes. Bit-identical node counts at
sd10/--cpus 1 unaffected (confirmed again), as expected -- the
single-threaded path never calls HelpSearch(). DEBUG=1 --cpus 4 sd10
run across all three curated suites (191 positions): clean exit, zero
assertion failures, after both fixes above.

Not yet done: the solve-count/fail-high-rate comparison at --cpus 4
against head_reference/baseline_cpus4_pre_unification.md -- a single
run was taken (71/191 vs the baseline's 81/191) but --cpus 4 is
confirmed genuinely nondeterministic (real races across splits, not
noise-free), so a one-shot diff isn't a meaningful comparison either
direction. A real comparison needs match_play.py-style repeated
trials, not done here. head_reference/ itself not yet rebuilt as the
new baseline pending that.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

1 files changed, 168 insertions, 167 deletions

diff --git a/src/split.c b/src/split.c
index fd65f2b..cf45de6 100644
--- a/src/split.c
+++ b/src/split.c
@@ -791,7 +791,25 @@ Return value:
             SystemDeferExecution(rand() % 2);
             ASSERT((g_SplitInfo[u].uNumThreadsHelping == 1) ||
                    (g_fExitProgram));
-            if (g_SplitInfo[u].iBestScore < g_SplitInfo[u].iBeta)
+            // "fail-low implies every candidate at this split was
+            // actually searched" no longer universally holds now that
+            // HelpSearch() calls SearchOneMove: a helper thread can
+            // legitimately stop pulling moves from THIS split's pool
+            // on MOVE_KERNEL_ABORTED even while this split itself is
+            // failing low on its own terms, because
+            // WE_SHOULD_STOP_SEARCHING's MP definition (chess.h) also
+            // covers ThreadUnderTerminatingSplit(ctx) -- an ANCESTOR
+            // split further up this thread's call stack can terminate
+            // (its own fail-high) independent of how this split is
+            // doing, and that's a global-enough signal to stop
+            // grinding through a doomed split's remaining moves rather
+            // than a bug. WE_SHOULD_STOP_SEARCHING is otherwise sticky
+            // for the remainder of this search, so checking it here
+            // (after every helper thread has already returned) still
+            // correctly distinguishes "we exhausted the pool normally"
+            // from "we bailed early for a legitimate reason."
+            if ((g_SplitInfo[u].iBestScore < g_SplitInfo[u].iBeta) &&
+                (!WE_SHOULD_STOP_SEARCHING))
             {
                 for (v = 0;
                      v < g_SplitInfo[u].uRemainingMoves;
@@ -1121,14 +1139,29 @@ Return value:
     ULONG uDepth;
     ULONG uMoveNum = 0;
     INT iOrigExtend;
-    INT iExtend;
     MOVE mv;
-    SCORE iMoveSee;
+    ULONG uRawDepthB;
+    ULONG uRawImproveB;
+    MOVE_KERNEL_RESULT eMoveKernelResult;
 #ifdef DEBUG
     POSITION board;
 
     memcpy(&board, &ctx->sPosition, sizeof(POSITION));
     ASSERT(PositionsAreEquivalent(&board, &g_SplitInfo[u].sSplitPosition));
+    // SearchOneMove's own post-UnmakeMove sanity ASSERT compares against
+    // ctx->sPlyInfo[ctx->uPly].sPosition (pi->sPosition), the same DEBUG-
+    // only field Search() populates once at node entry (search.c's own
+    // opening memcpy). A helper thread never calls Search()/
+    // CommonSearchInit for the split-point ply itself -- see eNodeType's
+    // own comment in SPLIT_INFO (chess.h) for why -- so that slot in
+    // THIS thread's own persistent ctx is otherwise stale leftover data
+    // from whatever unrelated node it last helped with. Populate it
+    // here, once, before the move loop (ctx->uPly doesn't change across
+    // iterations -- each one returns here via MakeMove/UnmakeMove before
+    // the next _GetNextParallelMove call), matching Search()'s own
+    // convention so the kernel's ASSERT means the same thing for both
+    // callers.
+    memcpy(&ctx->sPlyInfo[ctx->uPly].sPosition, &ctx->sPosition, sizeof(POSITION));
 #endif
 
     iOrigExtend = g_SplitInfo[u].iPositionExtend;
@@ -1136,15 +1169,32 @@ Return value:
     uOrigDepth = g_SplitInfo[u].uDepth;
     ctx->sSearchFlags = g_SplitInfo[u].sSearchFlags;
     SCORE iEval = g_SplitInfo[u].iEval;
-    // iImprovement (g_SplitInfo[u].iImprovement) is no longer read here
-    // -- GetLMRReduction dropped that parameter (2026-10-04, see its
-    // own routine-description comment: declared, never read in the
-    // body). Still stored into g_SplitInfo by the originating thread
-    // (search.c/searchsup.c's split setup) for any other consumer.
+
+    // Node-level hoists -- node-constant for the life of this split
+    // (same convention search.c's own move loop uses: compute once,
+    // not once per move). g_SplitInfo[u].iImprovement is real input
+    // here (unlike the GetLMRReduction parameter removed 2026-10-04 --
+    // see this field's own chess.h comment), feeding the joint-table
+    // lookup SearchOneMove now does internally via CanWeReduceOrPruneMove/
+    // ShouldLatePrune, which this call site never had before.
+    uRawDepthB = DepthBucket(uOrigDepth / ONE_PLY);
+    uRawImproveB = ImproveBucket(g_SplitInfo[u].iImprovement);
 
     do
     {
-        iExtend = iOrigExtend;
+        ULONG uLegalMoves;
+        ULONG x;
+#ifdef PERF_COUNTERS
+        FLAG fMoveIsPruneReduceEligibleUnused;
+        ULONG uThisMoveHangBucketUnused;
+        ULONG uThisMoveGapBucketUnused;
+        ULONG uThisMoveNumBucketUnused;
+        ULONG uThisMoveDepthBucketUnused;
+#endif
+#ifdef TEST_NULL
+        UINT64 uNullCalibFirstMoveNodeCostUnused;
+#endif
+
         uDepth = uOrigDepth;
         ASSERT(ctx->uPly == g_SplitInfo[u].uSplitPly);
 
@@ -1162,170 +1212,121 @@ Return value:
         ASSERT(IS_VALID_SCORE(iAlpha));
         ASSERT(IS_VALID_SCORE(iBeta));
         ASSERT(iAlpha < iBeta);
-        ASSERT(iExtend >= -ONE_PLY);
-        ASSERT(iExtend <= +ONE_PLY);
-
-        // SEE on the PRE-move position -- see search.c's identical comment.
-        iMoveSee = 0;
-        if (IS_CHECKING_MOVE(mv) ||
-            (mv.pCaptured && !IS_PAWN(mv.pCaptured) && (uDepth <= THREE_PLY)))
-        {
-            iMoveSee = GetMoveSee(ctx,
-                                    mv,
-                                    ctx->sMoveStack.uBegin[ctx->uPly] +
-                                        uMoveNum);
-        }
-
-        if (TRUE == MakeMove(ctx, mv))
-        {
-            ASSERT((IS_CHECKING_MOVE(mv) && InCheck(&ctx->sPosition, ctx->sPosition.uToMove)) ||
-                   (!IS_CHECKING_MOVE(mv) && !InCheck(&ctx->sPosition, ctx->sPosition.uToMove)));
-
-            // Compute extension. fIsLeftoverMove is unconditionally TRUE
-            // here: search.c only ever starts a split when the move it
-            // was about to try was itself already classified as a
-            // leftover move (see its own fIsLeftoverMove-gated split
-            // condition) -- every move HelpSearch pulls from the shared
-            // pool for that split is drawn from that same leftover
-            // range, so there's no "early, well-ordered move" case to
-            // distinguish here.
-            ComputeMoveExtension(ctx,
-                                 iAlpha,
-                                 iBeta,
-                                 (ctx->sMoveStack.uBegin[ctx->uPly - 1] + uMoveNum),
-                                 iEval,
-                                 uDepth,
-                                 iMoveSee,
-                                 TRUE,
-                                 &iExtend);
 
-            //
-            // Note: MAX_EXTEND_PER_LINE (a flat, non-depth-relative cap on
-            // total extension spent per line) used to be applied here.
-            // Removed -- g_uExtensionReduction[] is the sole extension-
-            // runaway guard now, same as the non-split move loop in
-            // search.c.
-            //
+        // x such that (x - 1) is mv's own absolute index in
+        // ctx->sMoveStack -- same convention search.c's move loop
+        // uses (there, x is already one past the move being tried;
+        // here, uBegin[ctx->uPly] + uMoveNum IS that move's absolute
+        // index, computed pre-MakeMove while ctx->uPly is still the
+        // split ply, so + 1 reproduces the same "(x - 1) == absolute
+        // index" relationship SearchOneMove's other callers rely on).
+        x = ctx->sMoveStack.uBegin[ctx->uPly] + uMoveNum + 1;
 
-            //
-            // Decide how much (if any) to reduce this move's depth.
-            //
-            // Gate matches search.c's non-split move loop exactly (the
-            // preconditions used to live inside GetLMRReduction itself;
-            // a refactor moved them out to the caller in search.c but
-            // this call site was missed, so GetLMRReduction ran
-            // unconditionally here -- including on checking moves,
-            // captures, PV nodes, etc. -- and its own internal
-            // ASSERT(!InCheck(...)) could fire for any checking move
-            // that also passed the killer-move/fail-high filter still
-            // inside GetLMRReduction. Only reachable through a split
-            // (i.e. only under real multithreading), which is why this
-            // looked like a rare, hard-to-reproduce race rather than
-            // the deterministic missing-gate bug it actually was.
-            //
-            if ((uDepth > TWO_PLY) &&
-                (NODE_PV != g_SplitInfo[u].eNodeType) &&
-                ((g_SplitInfo[u].uAlreadyDone + uMoveNum + 1) > 5) &&
-                (0 == iExtend) &&
-                (!IS_ESCAPING_CHECK(mv)) &&
-                (!IS_CAPTURE_OR_PROMOTION(mv)) &&
-                (!IS_CHECKING_MOVE(mv)))
-            {
-                // 2026-10-04: GetLMRReduction no longer looks up the
-                // dynamic FH rate or the joint table's static
-                // prediction itself -- search.c's non-split move loop
-                // looks these up once per move and passes them in now
-                // (see its hoisted per-move lookup block). This call
-                // site has no such shared lookup to reuse, so it does
-                // its own, matching what GetLMRReduction used to do
-                // internally. EstimateHangingMaterialCp still gets
-                // all-zero attack-table snapshots -- this thread has
-                // no access to the originating node's Search()-local
-                // bbMinorAttacksSnapshot/bbRookAttacksSnapshot, same
-                // reason this call site never got the joint-table
-                // treatment at all before 2026-10-02. It degrades
-                // safely on zeros (every "attacked by X" snapshot
-                // check becomes unconditionally false), the same
-                // already-accepted degradation search.c's own move
-                // loop uses when ITS snapshot is stale (see search.c's
-                // fParentAttackTablesAreFresh). puExtraTier is NULL --
-                // this call site has no joint-extra-tier re-search/
-                // calibration instrumentation to feed.
-                BITBOARD bbNoMinorAttacksSnapshot[2] = { 0, 0 };
-                BITBOARD bbNoRookAttacksSnapshot[2] = { 0, 0 };
-                ULONG uSplitLegalMoves = (g_SplitInfo[u].uAlreadyDone +
-                                          uMoveNum + 1);
-                ULONG uSplitFHAttempts = 0;
-                ULONG uSplitFHPctThousandths =
-                    GetMoveFailHighPctThousandths(mv, &uSplitFHAttempts);
-                SCORE iSplitHangCp = EstimateHangingMaterialCp(
-                                         ctx, mv, uSplitLegalMoves,
-                                         bbNoMinorAttacksSnapshot,
-                                         bbNoRookAttacksSnapshot);
-                ULONG uSplitPredictedFH = _JointTablePredictedFHThousandths(
-                    DepthBucket(uDepth / ONE_PLY),
-                    HangCpBucket(iSplitHangCp),
-                    GapCpBucket((SCORE)(iAlpha - iEval)),
-                    MoveNumBucket(uSplitLegalMoves),
-                    ImproveBucket(g_SplitInfo[u].iImprovement));
-                INT iLMR = GetLMRReduction(ctx,
-                                           uDepth,
-                                           uSplitLegalMoves,
-                                           mv,
-                                           iExtend,
-                                           uSplitFHAttempts,
-                                           uSplitFHPctThousandths,
-                                           uSplitPredictedFH,
-                                           NULL);
-                if (iLMR < 0)
-                {
-                    ASSERT(iExtend == 0);
-                    iExtend = iLMR;
-                }
-            }
+        // Pre-increment count: SearchOneMove itself does
+        // (*puLegalMoves)++ right after MakeMove succeeds, so passing
+        // the already-"+1" uAlreadyDone + uMoveNum + 1 here would
+        // double-count. uAlreadyDone + uMoveNum is this move's own
+        // 0-based ordinal among every move tried at this split node so
+        // far (across every helper thread); the kernel's own
+        // increment turns it into the right 1-based count.
+        uLegalMoves = g_SplitInfo[u].uAlreadyDone + uMoveNum;
 
-            // Record the extension/reduction actually applied to THIS
-            // move, unconditionally -- not just when it's negative.
-            // ctx->uPly - 1 (not ctx->uPly) because MakeMove above has
-            // already advanced ctx->uPly; the mover's own ply slot is
-            // one below the position it just produced. Matches
-            // search.c's identical convention/comment on its own
-            // pi->iExtensionAmount write (pi is bound pre-MakeMove
-            // there, so no -1 is needed in that file). Unconditional so
-            // a later sibling move at this split ply never inherits a
-            // stale reduction value left by an earlier one.
-            ctx->sPlyInfo[ctx->uPly - 1].iExtensionAmount = iExtend;
+        // fFullWindow is always FALSE here, never ASSERT-worthy of
+        // being TRUE: a helper thread's first move off the shared pool
+        // is never this node's actual first move -- search.c's own
+        // move loop (uLegalMoves==1, full a..b window) already
+        // searched the real first move before any split could launch
+        // (uAlreadyDone >= 1 always). See SearchOneMove's own
+        // fFullWindow parameter comment (chess.h) for why this can't
+        // be inferred from a sentinel the way the old inline code did.
+        //
+        // fIsGeneratedMoveStage is always TRUE: HelpSearch only ever
+        // pulls from the generated-move pool handed off at split-launch
+        // time, never a hash move.
+        //
+        // fIsLeftoverMove is always TRUE: search.c only ever starts a
+        // split when the move it was about to try was itself already
+        // classified as a leftover move -- every move HelpSearch pulls
+        // from the shared pool for that split is drawn from that same
+        // leftover range.
+        //
+        // g_SplitInfo[u].fThreat/bbMinorAttacksSnapshot/
+        // bbRookAttacksSnapshot are the real values from the
+        // originating thread (chess.h/split.c split-launch plumbing),
+        // not the zeroed snapshots and missing fThreat this call site
+        // used to have -- this is the actual unification: CanWeReduceOrPruneMove's
+        // real 9-check+2-exemption gate and ShouldLatePrune's LMP now
+        // run here for the first time, instead of the hand-rolled
+        // 7-condition approximation (missing the SEE-negative-capture
+        // carve-out, the mate-magnitude-alpha check, fThreat, and both
+        // move-identity exemptions) this function used before.
+        //
+        // MOVE_ORDERING_EVIDENCE_COLLECTION/TEST_NULL instrumentation
+        // is deliberately never exercised from a helper thread -- both
+        // are single-threaded-only diagnostics by design (confirmed
+        // with the user), so FALSE/0/throwaway-output is correct here,
+        // not a gap to fill in later.
+        eMoveKernelResult = SearchOneMove(ctx,
+                                          mv,
+                                          x,
+                                          TRUE,                    // fIsGeneratedMoveStage
+                                          uDepth,
+                                          iAlpha,
+                                          iBeta,
+                                          iEval,
+                                          iOrigExtend,              // iPositionalExtend
+                                          TRUE,                    // fIsLeftoverMove
+                                          g_SplitInfo[u].fThreat,
+                                          FALSE,                   // fFullWindow
+                                          uRawDepthB,
+                                          uRawImproveB,
+                                          g_SplitInfo[u].bbMinorAttacksSnapshot,
+                                          g_SplitInfo[u].bbRookAttacksSnapshot,
+#ifdef MOVE_ORDERING_EVIDENCE_COLLECTION
+                                          FALSE,                   // fThisMoveIsCountermoveMatch
+                                          0,                       // uCMEvidenceBucket
+#endif
+#ifdef TEST_NULL
+                                          FALSE,                   // fNullCalibEligible
+                                          0,                       // uNullCalibBucket
+                                          &uNullCalibFirstMoveNodeCostUnused,
+#endif
+                                          &uLegalMoves,
+                                          &iScore
+#ifdef PERF_COUNTERS
+                                          , &fMoveIsPruneReduceEligibleUnused
+                                          , &uThisMoveHangBucketUnused
+                                          , &uThisMoveGapBucketUnused
+                                          , &uThisMoveNumBucketUnused
+                                          , &uThisMoveDepthBucketUnused
+#endif
+                                          );
+        ASSERT(PositionsAreEquivalent(&ctx->sPosition, &board));
 
-            //
-            // Compute next depth
-            //
-            uDepth = uDepth - ONE_PLY + iExtend;
-            if (uDepth >= MAX_DEPTH_PER_SEARCH) uDepth = 0;
-            ctx->sSearchFlags.iCumulativeExtend += iExtend;
+        if (MOVE_KERNEL_ILLEGAL == eMoveKernelResult)
+        {
+            continue;
+        }
 
-            iScore = -Search(ctx, -iAlpha - 1, -iAlpha, uDepth);
-            if ((iAlpha < iScore) && (iScore < iBeta))
-            {
-                iScore = -Search(ctx, -iBeta, -iAlpha, uDepth);
-            }
+        // PRUNED never made the move at all -- no real score to
+        // report. SEARCHED and ABORTED both genuinely made, searched,
+        // and unmade mv (ABORTED's score is a real partial result, per
+        // the kernel's own return-value comment), so both get a
+        // chance to update the split's best score.
+        if ((MOVE_KERNEL_PRUNED != eMoveKernelResult) &&
+            (iScore > iBestScore))
+        {
+            _UpdateSplitInfo(ctx, mv, iScore, u);
+        }
 
-            //
-            // Decide whether to research reduced branches to full depth.
-            //
-            if ((iExtend < 0) && (iScore >= iBeta))
-            {
-                uDepth -= iExtend; // undo the full reduction, whatever its magnitude
-                ctx->sPlyInfo[ctx->uPly - 1].iExtensionAmount = 0;
-                iScore = -Search(ctx, -iBeta, -iAlpha, uDepth);
-            }
-            UnmakeMove(ctx, mv);
-            ctx->sSearchFlags.iCumulativeExtend -= iExtend;
-            ASSERT(PositionsAreEquivalent(&ctx->sPosition, &board));
-            if (TRUE == g_SplitInfo[u].fTerminate) break;
-            if (iScore > iBestScore)
-            {
-                _UpdateSplitInfo(ctx, mv, iScore, u);
-            }
+        // ABORTED means WE_SHOULD_STOP_SEARCHING fired -- a global
+        // stop condition, stronger than (and never explicitly checked
+        // by) this function's own fTerminate flag before unification.
+        // Stop pulling more moves either way.
+        if ((MOVE_KERNEL_ABORTED == eMoveKernelResult) ||
+            (TRUE == g_SplitInfo[u].fTerminate))
+        {
+            break;
         }
     }
     while(1);