Fix a data race for NNUE
authorJoost VandeVondele <Joost.VandeVondele@gmail.com>
Sun, 9 Aug 2020 16:11:38 +0000 (18:11 +0200)
committerJoost VandeVondele <Joost.VandeVondele@gmail.com>
Sun, 9 Aug 2020 21:51:07 +0000 (23:51 +0200)
the stateInfo at the rootPos is no longer read-only, as the NNUE accumulator is part of it.
Threads can thus not share this object and need their own copy.

tested for no regression
https://tests.stockfishchess.org/tests/view/5f3022239081672066536bce
LLR: 2.96 (-2.94,2.94) {-1.50,0.50}
Total: 52800 W: 6843 L: 6802 D: 39155
Ptnml(0-2): 336, 4646, 16399, 4679, 340

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

fixes https://github.com/official-stockfish/Stockfish/issues/2933

No functional change

src/Makefile
src/thread.cpp
src/thread.h

index b7585a174c6bc87163e584719eeeb339345cd5f4..571172b29e0b13209f50d66733952d6a6c0a34ef 100644 (file)
@@ -354,8 +354,8 @@ endif
 endif
 
 ifeq ($(KERNEL),Darwin)
-       CXXFLAGS += -arch $(arch) -mmacosx-version-min=10.13
-       LDFLAGS += -arch $(arch) -mmacosx-version-min=10.13
+       CXXFLAGS += -arch $(arch) -mmacosx-version-min=10.14
+       LDFLAGS += -arch $(arch) -mmacosx-version-min=10.14
 endif
 
 ### Travis CI script uses COMPILER to overwrite CXX
index 44aea14e6e0aab0a2d11d03ce9f4817a3fe12fff..1aa66a816619549c60f76f38cf8b030e0828a48d 100644 (file)
@@ -204,21 +204,18 @@ void ThreadPool::start_thinking(Position& pos, StateListPtr& states,
 
   // We use Position::set() to set root position across threads. But there are
   // some StateInfo fields (previous, pliesFromNull, capturedPiece) that cannot
-  // be deduced from a fen string, so set() clears them and to not lose the info
-  // we need to backup and later restore setupStates->back(). Note that setupStates
-  // is shared by threads but is accessed in read-only mode.
-  StateInfo tmp = setupStates->back();
-
+  // be deduced from a fen string, so set() clears them and they are set from
+  // setupStates->back() later. The rootState is per thread, earlier states are shared
+  // since they are read-only.
   for (Thread* th : *this)
   {
       th->nodes = th->tbHits = th->nmpMinPly = th->bestMoveChanges = 0;
       th->rootDepth = th->completedDepth = 0;
       th->rootMoves = rootMoves;
-      th->rootPos.set(pos.fen(), pos.is_chess960(), &setupStates->back(), th);
+      th->rootPos.set(pos.fen(), pos.is_chess960(), &th->rootState, th);
+      th->rootState = setupStates->back();
   }
 
-  setupStates->back() = tmp;
-
   main()->start_searching();
 }
 
index 46da1e348d128fbf01a8f39c458c8a57b3b94450..042bc2e9231588d5db2bd1fc4b5a097658df6399 100644 (file)
@@ -65,6 +65,7 @@ public:
   std::atomic<uint64_t> nodes, tbHits, bestMoveChanges;
 
   Position rootPos;
+  StateInfo rootState;
   Search::RootMoves rootMoves;
   Depth rootDepth, completedDepth;
   CounterMoveHistory counterMoves;