MakeMove: hoist GET_COLOR(mv.pMoved)/FLIP(pos->uToMove) into uColor/xColor

Same CSE Scott started by hand -- traced it through the rest of the
function. pos->uToMove flips from uColor to xColor partway through
(the side-to-move toggle after the move is played), so every
GET_COLOR(mv.pMoved) collapses to uColor and every post-flip
FLIP(pos->uToMove) also collapses to uColor.

One line intentionally NOT hoisted: the unconditional
ASSERT(!InCheck(pos, GET_COLOR(mv.pMoved))) after the mv.uMove!=0
block. It's reachable via the null-move (mv.uMove==0) fast path, which
skips the uColor/xColor assignment entirely (goto clear_ep before
pPiece = mv.pMoved runs) -- a first version used uColor here
unconditionally and read uninitialized stack memory on that path,
caught via gdb after TryNullmovePruning's null-move search crashed
inside InCheck/KingPosition with a garbage color index.
GET_COLOR(mv.pMoved) stays correct either way (0 for a null move,
since MOVE is a union and mv.uMove==0 zeroes mv.pMoved too).

Did not extend the same hoist into UnmakeMove -- uColor would only be
used in the narrow en-passant-capture branch there, not worth paying
the assignment cost on every unmake for two call sites in a rarely-hit
branch.

Measured end-to-end (sd10 + sn5M, all three curated suites, before vs.
after): node counts and solve counts bit-for-bit identical (pure CSE,
zero behavior change once the null-move bug was fixed). Wall-clock
~1.0% faster overall, all 6 measurements faster, none slower --
MakeMove is the hottest function in the engine.

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

1 files changed, 33 insertions, 21 deletions

diff --git a/src/move.c b/src/move.c
index 28e376c..23d7fdd 100644
--- a/src/move.c
+++ b/src/move.c
@@ -671,6 +671,7 @@ Return value:
     COOR cKing;
     PIECE xPawn;
     FLAG fMoveIsLegal = FALSE;
+    ULONG uColor, xColor;
     static int iSign[2] = { -1, +1 };
 
 #ifdef DEBUG
@@ -691,11 +692,13 @@ Return value:
     }
 
     pPiece = mv.pMoved;
+    uColor = GET_COLOR(pPiece);
+    xColor = FLIP(uColor);
     pCaptured = mv.pCaptured;
     ASSERT(!IS_EMPTY(pPiece));
     ASSERT(pPiece == pos->rgSquare[mv.cFrom].pPiece);
-    ASSERT(GET_COLOR(pPiece) == pos->uToMove);
-    
+    ASSERT(uColor == pos->uToMove);
+
     //
     // Is this a special move?  Special moves are: promotions, enpassant,
     // castling and double pawn moves.
@@ -708,7 +711,7 @@ Return value:
             if (pPromoted)
             {
                 ASSERT(!IS_PAWN(pPromoted));
-                ASSERT(GET_COLOR(pPromoted) == GET_COLOR(pPiece));
+                ASSERT(GET_COLOR(pPromoted) == uColor);
                 pPiece = LiftPiece(pos, mv.cFrom);
                 pCaptured = LiftPiece(pos, mv.cTo);
                 ASSERT(!pCaptured || (OPPOSITE_COLORS(pCaptured, pPiece)));
@@ -716,7 +719,7 @@ Return value:
                 pos->uFifty = 0;
                 goto update_castle_and_clear_ep;
             }
-            
+
             //
             // Is this an enpassant capture?
             // 
@@ -724,7 +727,7 @@ Return value:
             {
                 ASSERT(mv.cTo == pos->cEpSquare);
                 pCaptured = LiftPiece(pos, mv.cTo +
-                                      iSign[GET_COLOR(pPiece)] * 16);
+                                      iSign[uColor] * 16);
                 ASSERT(IS_PAWN(pCaptured));
                 ASSERT(OPPOSITE_COLORS(pPiece, pCaptured));
                 ASSERT(IS_EMPTY(pos->rgSquare[mv.cTo].pPiece));
@@ -741,9 +744,9 @@ Return value:
             else
             {
                 c = mv.cFrom;
-                c += (iSign[GET_COLOR(pPiece)] * -16);
+                c += (iSign[uColor] * -16);
                 ASSERT(RANK3(c) || RANK6(c));
-                ASSERT(c + (iSign[GET_COLOR(pPiece)] * -16) == mv.cTo);
+                ASSERT(c + (iSign[uColor] * -16) == mv.cTo);
                 ASSERT(IS_EMPTY(pos->rgSquare[c].pPiece));
                 ASSERT(IS_EMPTY(pos->rgSquare[mv.cTo].pPiece));
                 ASSERT(IS_PAWN(pPiece));
@@ -755,7 +758,7 @@ Return value:
                 // possible en-passant if there's an enemy who can
                 // take advantage of it.
                 //
-                xPawn = BLACK_PAWN | FLIP(GET_COLOR(pPiece));
+                xPawn = BLACK_PAWN | xColor;
                 if (((!IS_ON_BOARD(mv.cTo - 1)) ||
                      (pos->rgSquare[mv.cTo - 1].pPiece != xPawn)) &&
                     ((!IS_ON_BOARD(mv.cTo + 1)) ||
@@ -930,7 +933,7 @@ Return value:
     //
     if (mv.uMove != 0)
     {
-        cKing = KingPosition(pos, FLIP(pos->uToMove));
+        cKing = KingPosition(pos, uColor);
         ASSERT(IS_KING(pos->rgSquare[cKing].pPiece));
 
         if (IS_KING(mv.pMoved) || IS_ESCAPING_CHECK(mv))
@@ -942,8 +945,7 @@ Return value:
             // table to test ALL pieces in the enemy army and see if
             // they are attacking the just-moved king.
             //
-            ASSERT(GET_COLOR(mv.pMoved) == FLIP(pos->uToMove));
-            if (InCheck(pos, GET_COLOR(mv.pMoved)))
+            if (InCheck(pos, uColor))
             {
                 goto takeback;
             }
@@ -969,7 +971,7 @@ Return value:
             //
             if (IS_ON_BOARD(ExposesCheck(pos, mv.cFrom, cKing)))
             {
-                ASSERT(InCheck(pos, GET_COLOR(mv.pMoved)));
+                ASSERT(InCheck(pos, uColor));
                 goto takeback;
             }
             
@@ -987,12 +989,12 @@ Return value:
                 ASSERT(IS_PAWN(mv.pMoved));
                 ASSERT(IS_PAWN(mv.pCaptured));
                 if (IS_ON_BOARD(ExposesCheck(pos,
-                                             ((pos->uToMove == WHITE) ?
-                                              (mv.cTo + 16) : 
-                                              (mv.cTo - 16)), 
+                                             ((xColor == WHITE) ?
+                                              (mv.cTo + 16) :
+                                              (mv.cTo - 16)),
                                              cKing)))
                 {
-                    ASSERT(InCheck(pos, GET_COLOR(mv.pMoved)));
+                    ASSERT(InCheck(pos, uColor));
                     goto takeback;
                 }
             }
@@ -1002,6 +1004,16 @@ Return value:
         ASSERT((pos->rgSquare[mv.cTo].pPiece == mv.pMoved) ||
                (pos->rgSquare[mv.cTo].pPiece == mv.pPromoted));
     }
+    // NOT uColor here -- this line is reachable via the mv.uMove==0
+    // (null move) early path, which skips the uColor/xColor
+    // assignment entirely (goto clear_ep at the top, before pPiece =
+    // mv.pMoved runs). GET_COLOR(mv.pMoved) stays well-defined either
+    // way: 0 for a null move (mv.pMoved == 0 too, since MOVE is a
+    // union and mv.uMove == 0 zeroes the whole thing), uColor's value
+    // for a real move. A first version of this substitution used
+    // uColor unconditionally here and read uninitialized stack memory
+    // on the null-move path -- caught via gdb after TryNullmovePruning
+    // crashed inside InCheck/KingPosition with a garbage color index.
     ASSERT(!InCheck(pos, GET_COLOR(mv.pMoved)));
     fMoveIsLegal = TRUE;
 #ifdef DEBUG
@@ -1021,7 +1033,7 @@ Return value:
     // the whole move so we can't just return false (illegal move)
     // from there.  First we have to unmake the move we just made.
     //
-    ASSERT(InCheck(pos, FLIP(pos->uToMove)));
+    ASSERT(InCheck(pos, uColor));
     UnmakeMove(ctx, mv);
 #ifdef DEBUG
     VerifyPositionConsistency(pos, FALSE);
@@ -1125,11 +1137,11 @@ Return value:
     ASSERT(IS_ON_BOARD(mv.cTo));
     ASSERT(IS_ON_BOARD(mv.cFrom));
 #endif
-    if (mv.uMove == 0) 
+    if (mv.uMove == 0)
     {
         goto restore_saved_position_data;
     }
-    
+
     pPiece = mv.pMoved;
     pCaptured = mv.pCaptured;
     if (IS_CASTLE(mv))
@@ -1221,9 +1233,9 @@ Return value:
                 // Resurrect the dead piece in the enpassant location 
                 // where it died (not the same as where the killer moved).
                 // 
-                ASSERT(IS_EMPTY(pos->rgSquare[mv.cTo + 16 * 
+                ASSERT(IS_EMPTY(pos->rgSquare[mv.cTo + 16 *
                                        iSign[GET_COLOR(mv.pMoved)]].pPiece));
-                PlacePieceWithoutSigs(pos, 
+                PlacePieceWithoutSigs(pos,
                                      mv.cTo + 16 * iSign[GET_COLOR(mv.pMoved)],
                                       pCaptured);
             }