From 63c6f22627136dc1d900b5fef1c2cb415ab855e1 Mon Sep 17 00:00:00 2001 From: Carlos Esparza Date: Fri, 11 Apr 2025 11:58:20 -0700 Subject: [PATCH] Simplify DirtyPiece Simplifies the DirtyPiece struct. passed STC: https://tests.stockfishchess.org/tests/view/68054c9798cd372e3aea05d7 LLR: 2.94 (-2.94,2.94) <-1.75,0.25> Total: 36608 W: 9641 L: 9437 D: 17530 Ptnml(0-2): 89, 3630, 10668, 3822, 95 passed LTC: https://tests.stockfishchess.org/tests/view/6805514598cd372e3aea0783 LLR: 2.95 (-2.94,2.94) <-1.75,0.25> Total: 127620 W: 31993 L: 31894 D: 63733 Ptnml(0-2): 42, 10983, 41665, 11074, 46 closes https://github.com/official-stockfish/Stockfish/pull/6016 No functional change --- src/nnue/features/half_ka_v2_hm.cpp | 18 ++++++------ src/nnue/nnue_accumulator.cpp | 12 ++++---- src/position.cpp | 44 ++++++++++++++--------------- src/types.h | 18 +++++------- 4 files changed, 44 insertions(+), 48 deletions(-) diff --git a/src/nnue/features/half_ka_v2_hm.cpp b/src/nnue/features/half_ka_v2_hm.cpp index eb3c7e6a..c91da2cc 100644 --- a/src/nnue/features/half_ka_v2_hm.cpp +++ b/src/nnue/features/half_ka_v2_hm.cpp @@ -58,13 +58,15 @@ void HalfKAv2_hm::append_changed_indices(Square ksq, const DirtyPiece& dp, IndexList& removed, IndexList& added) { - for (int i = 0; i < dp.dirty_num; ++i) - { - if (dp.from[i] != SQ_NONE) - removed.push_back(make_index(dp.from[i], dp.piece[i], ksq)); - if (dp.to[i] != SQ_NONE) - added.push_back(make_index(dp.to[i], dp.piece[i], ksq)); - } + removed.push_back(make_index(dp.from, dp.pc, ksq)); + if (dp.to != SQ_NONE) + added.push_back(make_index(dp.to, dp.pc, ksq)); + + if (dp.remove_sq != SQ_NONE) + removed.push_back(make_index(dp.remove_sq, dp.remove_pc, ksq)); + + if (dp.add_sq != SQ_NONE) + added.push_back(make_index(dp.add_sq, dp.add_pc, ksq)); } // Explicit template instantiations @@ -78,7 +80,7 @@ template void HalfKAv2_hm::append_changed_indices(Square ksq, IndexList& added); bool HalfKAv2_hm::requires_refresh(const DirtyPiece& dirtyPiece, Color perspective) { - return dirtyPiece.piece[0] == make_piece(perspective, KING); + return dirtyPiece.pc == make_piece(perspective, KING); } } // namespace Stockfish::Eval::NNUE::Features diff --git a/src/nnue/nnue_accumulator.cpp b/src/nnue/nnue_accumulator.cpp index 5c112853..83128436 100644 --- a/src/nnue/nnue_accumulator.cpp +++ b/src/nnue/nnue_accumulator.cpp @@ -152,16 +152,16 @@ void AccumulatorStack::forward_update_incremental( { if (next + 1 < size) { - auto& dp1 = accumulators[next].dirtyPiece; - auto& dp2 = accumulators[next + 1].dirtyPiece; + DirtyPiece& dp1 = accumulators[next].dirtyPiece; + DirtyPiece& dp2 = accumulators[next + 1].dirtyPiece; - if (dp2.dirty_num >= 2 && dp1.piece[0] == dp2.piece[1] && dp1.to[0] == dp2.from[1]) + if (dp1.to != SQ_NONE && dp1.to == dp2.remove_sq) { - const Square captureSq = dp1.to[0]; - dp1.to[0] = dp2.from[1] = SQ_NONE; + const Square captureSq = dp1.to; + dp1.to = dp2.remove_sq = SQ_NONE; double_inc_update(featureTransformer, ksq, accumulators[next], accumulators[next + 1], accumulators[next - 1]); - dp1.to[0] = dp2.from[1] = captureSq; + dp1.to = dp2.remove_sq = captureSq; next++; continue; diff --git a/src/position.cpp b/src/position.cpp index 85ade69a..9f023b5f 100644 --- a/src/position.cpp +++ b/src/position.cpp @@ -706,9 +706,6 @@ DirtyPiece Position::do_move(Move m, ++st->rule50; ++st->pliesFromNull; - DirtyPiece dp; - dp.dirty_num = 1; - Color us = sideToMove; Color them = ~us; Square from = m.from_sq(); @@ -716,6 +713,12 @@ DirtyPiece Position::do_move(Move m, Piece pc = piece_on(from); Piece captured = m.type_of() == EN_PASSANT ? make_piece(them, PAWN) : piece_on(to); + DirtyPiece dp; + dp.pc = pc; + dp.from = from; + dp.to = to; + dp.add_sq = SQ_NONE; + assert(color_of(pc) == us); assert(captured == NO_PIECE || color_of(captured) == (m.type_of() != CASTLING ? them : us)); assert(type_of(captured) != KING); @@ -762,10 +765,8 @@ DirtyPiece Position::do_move(Move m, st->minorPieceKey ^= Zobrist::psq[captured][capsq]; } - dp.dirty_num = 2; // 1 piece moved, 1 piece captured - dp.piece[1] = captured; - dp.from[1] = capsq; - dp.to[1] = SQ_NONE; + dp.remove_pc = captured; + dp.remove_sq = capsq; // Update board and piece lists remove_piece(capsq); @@ -776,6 +777,8 @@ DirtyPiece Position::do_move(Move m, // Reset rule 50 counter st->rule50 = 0; } + else + dp.remove_sq = SQ_NONE; // Update hash key k ^= Zobrist::psq[pc][from] ^ Zobrist::psq[pc][to]; @@ -798,9 +801,6 @@ DirtyPiece Position::do_move(Move m, // Move the piece. The tricky Chess960 castling is handled earlier if (m.type_of() != CASTLING) { - dp.piece[0] = pc; - dp.from[0] = from; - dp.to[0] = to; move_piece(from, to); } @@ -827,12 +827,9 @@ DirtyPiece Position::do_move(Move m, remove_piece(to); put_piece(promotion, to); - // Promoting pawn to SQ_NONE, promoted piece from SQ_NONE - dp.to[0] = SQ_NONE; - dp.piece[dp.dirty_num] = promotion; - dp.from[dp.dirty_num] = SQ_NONE; - dp.to[dp.dirty_num] = to; - dp.dirty_num++; + dp.add_pc = promotion; + dp.add_sq = to; + dp.to = SQ_NONE; // Update hash keys // Zobrist::psq[pc][to] is zero, so we don't need to clear it @@ -899,6 +896,10 @@ DirtyPiece Position::do_move(Move m, assert(pos_is_ok()); + assert(dp.pc != NO_PIECE); + assert(!(bool(captured) || m.type_of() == CASTLING) ^ (dp.remove_sq != SQ_NONE)); + assert(dp.from != SQ_NONE); + assert(!(dp.add_sq != SQ_NONE) ^ (m.type_of() == PROMOTION || m.type_of() == CASTLING)); return dp; } @@ -981,13 +982,10 @@ void Position::do_castling( if (Do) { - dp->piece[0] = make_piece(us, KING); - dp->from[0] = from; - dp->to[0] = to; - dp->piece[1] = make_piece(us, ROOK); - dp->from[1] = rfrom; - dp->to[1] = rto; - dp->dirty_num = 2; + dp->to = to; + dp->remove_pc = dp->add_pc = make_piece(us, ROOK); + dp->remove_sq = rfrom; + dp->add_sq = rto; } // Remove both pieces first since squares could overlap in Chess960 diff --git a/src/types.h b/src/types.h index a76d0033..6cd6ee74 100644 --- a/src/types.h +++ b/src/types.h @@ -276,18 +276,14 @@ enum Rank : int { // Keep track of what a move changes on the board (used by NNUE) struct DirtyPiece { + Piece pc; // this is never allowed to be NO_PIECE + Square from, to; // to should be SQ_NONE for promotions - // Number of changed pieces - int dirty_num; - - // Max 3 pieces can change in one move. A promotion with capture moves - // both the pawn and the captured piece to SQ_NONE and the piece promoted - // to from SQ_NONE to the capture square. - Piece piece[3]; - - // From and to squares, which may be SQ_NONE - Square from[3]; - Square to[3]; + // if {add,remove}_sq is SQ_NONE, {add,remove}_pc is allowed to be + // uninitialized + // castling uses add_sq and remove_sq to remove and add the rook + Square remove_sq, add_sq; + Piece remove_pc, add_pc; }; #define ENABLE_INCR_OPERATORS_ON(T) \