Fix passed-pawn bitboard bit-clear bug, LMR gate coupling, inline hot bitboard helpers

- bitboard.c: CoorFromBitBoardRank1ToRank8 cleared the lowest set bit
  unconditionally instead of the reported (highest) one, silently
  mis-walking doubled-pawn files in eval.c's passed-pawn detection.
- search.c/searchsup.c: move GetLMRReduction's precondition checks from
  inside the function to the caller in search.c (pre-existing work),
  finishing the split with a matching gate in split.c's HelpSearch --
  the parallel-search call site had no gate at all, letting it call
  GetLMRReduction unconditionally (including for checking moves),
  reachable only under real multithreading (--cpus > 1) and the
  intermittent root cause of assertion crashes seen under --cpus 4.
- eval.c: redirect CountBits/CoorFromBitBoardRank8ToRank1/
  CoorFromBitBoardRank1ToRank8 to inline compiler-builtin versions
  (gated !CROUTINES) instead of the real out-of-line asm calls, on
  eval.c's ~20 existing production call sites. CountBits' asm body
  isn't O(1) popcnt, it's a Kernighan bit-clearing loop plus call
  overhead, paid on every Eval() call.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Jntky4yGUTyQVaGCXms4F2

5 files changed, 105 insertions, 13 deletions

diff --git a/src/bitboard.c b/src/bitboard.c
index 0478fc1..9e9df48 100755
--- a/src/bitboard.c
+++ b/src/bitboard.c
@@ -389,10 +389,21 @@ Return value:
     {
         ASSERT(*pbb);
         uLastBit--;
-        *pbb &= (*pbb - 1);
         c = BIT_NUMBER_TO_COOR(uLastBit);
         ASSERT(c == SLOW_BIT_NUMBER_TO_COOR(uLastBit));
         ASSERT(IS_ON_BOARD(c));
+        // BUG (fixed): this used to be `*pbb &= (*pbb - 1)`, which
+        // clears the LOWEST set bit -- correct for
+        // CoorFromBitBoardRank8ToRank1's first-bit semantics, wrong
+        // here, where uLastBit is the HIGHEST set bit. With more than
+        // one bit set (e.g. doubled pawns on a file), that cleared the
+        // wrong bit: the reported (highest) bit was never actually
+        // removed, so a caller looping on this function would see it
+        // again next call (a duplicate) while the true lowest bit was
+        // silently skipped forever. Single-bit inputs never exposed
+        // this (lowest-bit-clear and highest-bit-clear coincide when
+        // there's only one bit).
+        *pbb &= ~COOR_TO_BB(c);
     }
     return(c);
 }
diff --git a/src/eval.c b/src/eval.c
index 3e856f1..23f830c 100755
--- a/src/eval.c
+++ b/src/eval.c
@@ -21,6 +21,68 @@ Revision History:
 
 #include "chess.h"
 
+//
+// CountBits and the CoorFromBitBoard{Rank8ToRank1,Rank1ToRank8}
+// wrappers (bitboard.c) are real out-of-line asm calls on every one of
+// eval.c's ~20 production call sites (passed-pawn detection, bishop-
+// pair logic, etc.), hit on every real Eval() call. CountBits' asm
+// body (x64.asm) isn't even O(1) popcnt, it's a Kernighan bit-clearing
+// loop -- O(popcount) iterations plus call overhead. Redirected via
+// #define, the same mechanism chess.h already uses for the OTHER
+// direction (#ifdef CROUTINES routes CountBits/FirstBit/LastBit to the
+// slow C fallbacks) -- this routes them to fast inline compiler
+// builtins instead, for every existing call site below with no further
+// edits. Gated on !CROUTINES so the CROUTINES debug/comparison build
+// still gets the real (slow, cross-checked) implementations.
+//
+// _FastCoorFromBitBoardRank1ToRank8 also bakes in the wrong-bit-clear
+// fix applied to bitboard.c's real CoorFromBitBoardRank1ToRank8 (that
+// function used to clear the LOWEST set bit via `*pbb &= (*pbb - 1)`
+// regardless of uLastBit, correct only when a single bit was set) --
+// this version clears the bit it actually just reported, via BBSQUARE.
+//
+#ifndef CROUTINES
+static ULONG INLINE
+_FastCountBits(IN BITBOARD bb)
+{
+    return (ULONG)__builtin_popcountll(bb);
+}
+
+static COOR INLINE
+_FastCoorFromBitBoardRank8ToRank1(IN OUT BITBOARD *pbb)
+{
+    COOR c = ILLEGAL_COOR;
+    ULONG uBitIndex;
+
+    if (*pbb)
+    {
+        uBitIndex = (ULONG)__builtin_ctzll(*pbb);
+        c = BIT_NUMBER_TO_COOR(uBitIndex);
+        *pbb &= (*pbb - 1);
+    }
+    return c;
+}
+
+static COOR INLINE
+_FastCoorFromBitBoardRank1ToRank8(IN OUT BITBOARD *pbb)
+{
+    COOR c = ILLEGAL_COOR;
+    ULONG uBitIndex;
+
+    if (*pbb)
+    {
+        uBitIndex = (ULONG)(63 - __builtin_clzll(*pbb));
+        c = BIT_NUMBER_TO_COOR(uBitIndex);
+        *pbb &= ~BBSQUARE[uBitIndex];
+    }
+    return c;
+}
+
+#define CountBits _FastCountBits
+#define CoorFromBitBoardRank8ToRank1 _FastCoorFromBitBoardRank8ToRank1
+#define CoorFromBitBoardRank1ToRank8 _FastCoorFromBitBoardRank1ToRank8
+#endif // !CROUTINES
+
 
 //
 // Bishop-mobility ray-walk outcome categories -- see BMobCaseTable in
diff --git a/src/search.c b/src/search.c
index 807f69d..1b83891 100755
--- a/src/search.c
+++ b/src/search.c
@@ -664,7 +664,7 @@ Search(IN SEARCHER_THREAD_CONTEXT *ctx,
         // underflow (ULONG) in that case, which is exactly why the
         // uDepth >= ONE_PLY check below short-circuits before the
         // g_fCanSplit[] indexing ever evaluates it.
-        if (((uLegalMoves >= 3)) &&
+        if (((uLegalMoves >= 2) && fIsLeftoverMove) &&
             (0 != g_uNumHelpersAvailable) &&
             (FALSE == pi->fMovesRescoredByIID) &&
             (0 == uFutilityMargin) &&
@@ -750,6 +750,14 @@ Search(IN SEARCHER_THREAD_CONTEXT *ctx,
 
             // Decide how much (if any) to reduce this move's depth --
             // graded LMR.
+            if ((uDepth > TWO_PLY) &&
+                !pi->fPvNode &&
+                !(ctx->sPlyInfo[ctx->uPly - 1].fPvNode) &&
+                (uLegalMoves > 5) &&
+                (0 == iExtend) &&
+                (!IS_ESCAPING_CHECK(mv)) &&
+                (!IS_CAPTURE_OR_PROMOTION(mv)) &&
+                (!IS_CHECKING_MOVE(mv)))
             {
                 INT iLMR = GetLMRReduction(iEval,
                                            iAlpha,
diff --git a/src/searchsup.c b/src/searchsup.c
index b79db3a..4e6a0ed 100644
--- a/src/searchsup.c
+++ b/src/searchsup.c
@@ -219,14 +219,7 @@ Return value:
     ASSERT(mv.uMove);
     ASSERT((uMoveNum > 0) || (uLegalMoves == 0));
 
-    if ((uRemainingDepth >= TWO_PLY) &&
-        (FALSE == ctx->sPlyInfo[ctx->uPly - 1].fPvNode) &&
-        (uLegalMoves > 5) &&
-        (0 == iExtend) &&
-        (!IS_ESCAPING_CHECK(mv)) &&
-        (!IS_CAPTURE_OR_PROMOTION(mv)) &&
-        (!IS_CHECKING_MOVE(mv)) &&
-        (!IS_SAME_MOVE(mv, ctx->mvKiller[ctx->uPly-1][0])) &&
+    if ((!IS_SAME_MOVE(mv, ctx->mvKiller[ctx->uPly-1][0])) &&
         (!IS_SAME_MOVE(mv, ctx->mvKiller[ctx->uPly-1][1])) &&
         ((ctx->uPly < 3) ||
          (!IS_SAME_MOVE(mv, ctx->mvKiller[ctx->uPly-3][0]) &&
@@ -234,9 +227,6 @@ Return value:
         (GetMoveFailHighPercentage(mv, NULL) <= 10))
     {
         ASSERT(!InCheck(&ctx->sPosition, ctx->sPosition.uToMove));
-        if (iImprovement <= 40) {
-            return(-(ONE_PLY + QUARTER_PLY));
-        }
         return(-ONE_PLY);
     }
     return(0);
diff --git a/src/split.c b/src/split.c
index a1f9d32..f8da7ed 100755
--- a/src/split.c
+++ b/src/split.c
@@ -1138,6 +1138,27 @@ Return value:
             //
             // 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) &&
+                !(ctx->sPlyInfo[ctx->uPly].fPvNode) &&
+                !(ctx->sPlyInfo[ctx->uPly - 1].fPvNode) &&
+                ((g_SplitInfo[u].uAlreadyDone + uMoveNum + 1) > 5) &&
+                (0 == iExtend) &&
+                (!IS_ESCAPING_CHECK(mv)) &&
+                (!IS_CAPTURE_OR_PROMOTION(mv)) &&
+                (!IS_CHECKING_MOVE(mv)))
             {
                 INT iLMR = GetLMRReduction(iEval,
                                            iAlpha,