Don't copy around DirtyThreats

The contents of DirtyThreats gets memcpyed twice for each call to do_move.
(Return value optimization doesn't apply to the do_move function itself because
it constructs a std::pair, so it gets copied; and the calls to reset also
require a copy.) This patch inserts the dirty info in place.

Sometimes the caller of do_move ignores the DirtyThreats info, so we pass in
scratch objects. I found that stack-allocating these scratch objects was bad on
Fishtest, so I begrudgingly put them in the Position struct. Both templating
the do_move function on whether dirty threats are needed and putting a
null-check branch for each use of dirty threats were slowdowns locally. Of
course, nothing prevents a future attempt at cleaning this up.

passed STC
LLR: 2.96 (-2.94,2.94) <0.00,2.00>
Total: 68448 W: 17770 L: 17418 D: 33260
Ptnml(0-2): 198, 7425, 18630, 7769, 202
https://tests.stockfishchess.org/tests/view/6914c01a7ca87818523318ba

closes https://github.com/official-stockfish/Stockfish/pull/6414

No functional change

src/nnue/nnue_accumulator.cpp

@@ -21,6 +21,7 @@
 #include <algorithm>
 #include <cassert>
 #include <cstdint>
+#include <new>
 #include <type_traits>
 
 #include "../bitboard.h"
@@ -122,11 +123,13 @@ void AccumulatorStack::reset() noexcept {
     size = 1;
 }
 
-void AccumulatorStack::push(const DirtyBoardData& dirtyBoardData) noexcept {
+std::pair<DirtyPiece&, DirtyThreats&> AccumulatorStack::push() noexcept {
     assert(size < MaxSize);
-    psq_accumulators[size].reset(dirtyBoardData.dp);
+    auto& dp  = psq_accumulators[size].reset();
-    threat_accumulators[size].reset(dirtyBoardData.dts);
+    auto& dts = threat_accumulators[size].reset();
+    new (&dts) DirtyThreats;
     size++;
+    return {dp, dts};
 }
 
 void AccumulatorStack::pop() noexcept {

src/nnue/nnue_accumulator.h

@@ -25,6 +25,7 @@
 #include <cstddef>
 #include <cstdint>
 #include <cstring>
+#include <utility>
 
 #include "../types.h"
 #include "nnue_architecture.h"
@@ -141,6 +142,12 @@ struct AccumulatorState {
         accumulatorBig.computed.fill(false);
         accumulatorSmall.computed.fill(false);
     }
+
+    typename FeatureSet::DiffType& reset() noexcept {
+        accumulatorBig.computed.fill(false);
+        accumulatorSmall.computed.fill(false);
+        return diff;
+    }
 };
 
 class AccumulatorStack {
@@ -152,6 +159,7 @@ class AccumulatorStack {
 
     void                                  reset() noexcept;
     void                                  push(const DirtyBoardData& dirtyBoardData) noexcept;
+    std::pair<DirtyPiece&, DirtyThreats&> push() noexcept;
     void                                  pop() noexcept;
 
     template<IndexType Dimensions>

src/position.cpp

@@ -201,7 +201,7 @@ Position& Position::set(const string& fenStr, bool isChess960, StateInfo* si) {
     Square             sq = SQ_A8;
     std::istringstream ss(fenStr);
 
-    std::memset(this, 0, sizeof(Position));
+    std::memset(reinterpret_cast<char*>(this), 0, sizeof(Position));
     std::memset(si, 0, sizeof(StateInfo));
     st = si;
 
@@ -688,9 +688,11 @@ bool Position::gives_check(Move m) const {
 // moves should be filtered out before this function is called.
 // If a pointer to the TT table is passed, the entry for the new position
 // will be prefetched
-DirtyBoardData Position::do_move(Move                      m,
+void Position::do_move(Move                      m,
                        StateInfo&                newSt,
                        bool                      givesCheck,
+                       DirtyPiece&               dp,
+                       DirtyThreats&             dts,
                        const TranspositionTable* tt = nullptr) {
 
     assert(m.is_ok());
@@ -720,12 +722,10 @@ DirtyBoardData Position::do_move(Move                      m,
 
     bool checkEP = false;
 
-    DirtyPiece dp;
     dp.pc             = pc;
     dp.from           = from;
     dp.to             = to;
     dp.add_sq         = SQ_NONE;
-    DirtyThreats dts;
     dts.us            = us;
     dts.prevKsq       = square<KING>(us);
     dts.threatenedSqs = dts.threateningSqs = 0;
@@ -970,8 +970,6 @@ DirtyBoardData Position::do_move(Move                      m,
     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, dts};
 }
 
 

src/position.h

@@ -24,6 +24,7 @@
 #include <deque>
 #include <iosfwd>
 #include <memory>
+#include <new>
 #include <string>
 
 #include "bitboard.h"
@@ -134,7 +135,12 @@ class Position {
 
     // Doing and undoing moves
     void do_move(Move m, StateInfo& newSt, const TranspositionTable* tt);
-    DirtyBoardData do_move(Move m, StateInfo& newSt, bool givesCheck, const TranspositionTable* tt);
+    void do_move(Move                      m,
+                 StateInfo&                newSt,
+                 bool                      givesCheck,
+                 DirtyPiece&               dp,
+                 DirtyThreats&             dts,
+                 const TranspositionTable* tt);
     void undo_move(Move m);
     void do_null_move(StateInfo& newSt, const TranspositionTable& tt);
     void undo_null_move();
@@ -204,6 +210,8 @@ class Position {
     int          gamePly;
     Color        sideToMove;
     bool         chess960;
+    DirtyPiece   scratch_dp;
+    DirtyThreats scratch_dts;
 };
 
 std::ostream& operator<<(std::ostream& os, const Position& pos);
@@ -389,7 +397,8 @@ inline void Position::swap_piece(Square s, Piece pc, DirtyThreats* const dts) {
 }
 
 inline void Position::do_move(Move m, StateInfo& newSt, const TranspositionTable* tt = nullptr) {
-    do_move(m, newSt, gives_check(m), tt);
+    new (&scratch_dts) DirtyThreats;
+    do_move(m, newSt, gives_check(m), scratch_dp, scratch_dts, tt);
 }
 
 inline StateInfo* Position::state() const { return st; }

src/search.cpp

@@ -531,16 +531,16 @@ void Search::Worker::do_move(
     bool capture = pos.capture_stage(move);
     nodes.fetch_add(1, std::memory_order_relaxed);
 
-    DirtyBoardData dirtyBoardData = pos.do_move(move, st, givesCheck, &tt);
+    auto [dirtyPiece, dirtyThreats] = accumulatorStack.push();
-    accumulatorStack.push(dirtyBoardData);
+    pos.do_move(move, st, givesCheck, dirtyPiece, dirtyThreats, &tt);
 
     if (ss != nullptr)
     {
         ss->currentMove = move;
         ss->continuationHistory =
-          &continuationHistory[ss->inCheck][capture][dirtyBoardData.dp.pc][move.to_sq()];
+          &continuationHistory[ss->inCheck][capture][dirtyPiece.pc][move.to_sq()];
         ss->continuationCorrectionHistory =
-          &continuationCorrectionHistory[dirtyBoardData.dp.pc][move.to_sq()];
+          &continuationCorrectionHistory[dirtyPiece.pc][move.to_sq()];
     }
 }