From 27b593a94477a821f80a041320683f805114d4a3 Mon Sep 17 00:00:00 2001 From: Joost VandeVondele Date: Sun, 9 Aug 2020 18:11:38 +0200 Subject: [PATCH 1/1] Fix a data race for NNUE 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 | 4 ++-- src/thread.cpp | 13 +++++-------- src/thread.h | 1 + 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/Makefile b/src/Makefile index b7585a17..571172b2 100644 --- a/src/Makefile +++ b/src/Makefile @@ -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 diff --git a/src/thread.cpp b/src/thread.cpp index 44aea14e..1aa66a81 100644 --- a/src/thread.cpp +++ b/src/thread.cpp @@ -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(); } diff --git a/src/thread.h b/src/thread.h index 46da1e34..042bc2e9 100644 --- a/src/thread.h +++ b/src/thread.h @@ -65,6 +65,7 @@ public: std::atomic nodes, tbHits, bestMoveChanges; Position rootPos; + StateInfo rootState; Search::RootMoves rootMoves; Depth rootDepth, completedDepth; CounterMoveHistory counterMoves; -- 2.39.2