From e26d13bb318776bd0b156e55bb392d096a7dc37a Mon Sep 17 00:00:00 2001 From: Marco Costalba Date: Tue, 20 Mar 2012 11:45:27 +0100 Subject: [PATCH 1/7] Use a local copy of tte->value() This should avoid some aliasing issues with TT table access. After 3913 games at 10"+0.05 Mod vs Orig 662 - 651 - 2600 ELO +0 (+- 6.4) Signed-off-by: Marco Costalba --- src/search.cpp | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/src/search.cpp b/src/search.cpp index b6fc0697..8d1b43b0 100644 --- a/src/search.cpp +++ b/src/search.cpp @@ -136,9 +136,9 @@ namespace { bool connected_moves(const Position& pos, Move m1, Move m2); Value value_to_tt(Value v, int ply); Value value_from_tt(Value v, int ply); - bool can_return_tt(const TTEntry* tte, Depth depth, Value beta, int ply); + bool can_return_tt(const TTEntry* tte, Depth depth, Value ttValue, Value beta); bool connected_threat(const Position& pos, Move m, Move threat); - Value refine_eval(const TTEntry* tte, Value defaultEval, int ply); + Value refine_eval(const TTEntry* tte, Value ttValue, Value defaultEval); Move do_skill_level(); string score_to_uci(Value v, Value alpha = -VALUE_INFINITE, Value beta = VALUE_INFINITE); void pv_info_to_log(Position& pos, int depth, Value score, int time, Move pv[]); @@ -542,7 +542,7 @@ namespace { Move ttMove, move, excludedMove, bestMove, threatMove; Depth ext, newDepth; Bound bt; - Value bestValue, value, oldAlpha; + Value bestValue, value, oldAlpha, ttValue; Value refinedValue, nullValue, futilityBase, futilityValue; bool isPvMove, inCheck, singularExtensionNode, givesCheck; bool captureOrPromotion, dangerous, doFullDepthSearch; @@ -613,19 +613,19 @@ namespace { posKey = excludedMove ? pos.exclusion_key() : pos.key(); tte = TT.probe(posKey); ttMove = RootNode ? RootMoves[PVIdx].pv[0] : tte ? tte->move() : MOVE_NONE; + ttValue = tte ? value_from_tt(tte->value(), ss->ply) : VALUE_ZERO; // At PV nodes we check for exact scores, while at non-PV nodes we check for // a fail high/low. Biggest advantage at probing at PV nodes is to have a // smooth experience in analysis mode. We don't probe at Root nodes otherwise // we should also update RootMoveList to avoid bogus output. if (!RootNode && tte && (PvNode ? tte->depth() >= depth && tte->type() == BOUND_EXACT - : can_return_tt(tte, depth, beta, ss->ply))) + : can_return_tt(tte, depth, ttValue, beta))) { TT.refresh(tte); ss->currentMove = ttMove; // Can be MOVE_NONE - value = value_from_tt(tte->value(), ss->ply); - if ( value >= beta + if ( ttValue >= beta && ttMove && !pos.is_capture_or_promotion(ttMove) && ttMove != ss->killers[0]) @@ -633,7 +633,7 @@ namespace { ss->killers[1] = ss->killers[0]; ss->killers[0] = ttMove; } - return value; + return ttValue; } // Step 5. Evaluate the position statically and update parent's gain statistics @@ -645,7 +645,7 @@ namespace { ss->eval = tte->static_value(); ss->evalMargin = tte->static_value_margin(); - refinedValue = refine_eval(tte, ss->eval, ss->ply); + refinedValue = refine_eval(tte, ttValue, ss->eval); } else { @@ -878,8 +878,6 @@ split_point_start: // At split points actual search starts from here && move == ttMove && pos.pl_move_is_legal(move, ci.pinned)) { - Value ttValue = value_from_tt(tte->value(), ss->ply); - if (abs(ttValue) < VALUE_KNOWN_WIN) { Value rBeta = ttValue - int(depth); @@ -1139,7 +1137,7 @@ split_point_start: // At split points actual search starts from here StateInfo st; Move ttMove, move, bestMove; - Value bestValue, value, evalMargin, futilityValue, futilityBase; + Value ttValue, bestValue, value, evalMargin, futilityValue, futilityBase; bool inCheck, enoughMaterial, givesCheck, evasionPrunable; const TTEntry* tte; Depth ttDepth; @@ -1163,11 +1161,12 @@ split_point_start: // At split points actual search starts from here // pruning, but only for move ordering. tte = TT.probe(pos.key()); ttMove = (tte ? tte->move() : MOVE_NONE); + ttValue = tte ? value_from_tt(tte->value(),ss->ply) : VALUE_ZERO; - if (!PvNode && tte && can_return_tt(tte, ttDepth, beta, ss->ply)) + if (!PvNode && tte && can_return_tt(tte, ttDepth, ttValue, beta)) { ss->currentMove = ttMove; // Can be MOVE_NONE - return value_from_tt(tte->value(), ss->ply); + return ttValue; } // Evaluate the position statically @@ -1485,9 +1484,7 @@ split_point_start: // At split points actual search starts from here // can_return_tt() returns true if a transposition table score can be used to // cut-off at a given point in search. - bool can_return_tt(const TTEntry* tte, Depth depth, Value beta, int ply) { - - Value v = value_from_tt(tte->value(), ply); + bool can_return_tt(const TTEntry* tte, Depth depth, Value v, Value beta) { return ( tte->depth() >= depth || v >= std::max(VALUE_MATE_IN_MAX_PLY, beta) @@ -1501,12 +1498,10 @@ split_point_start: // At split points actual search starts from here // refine_eval() returns the transposition table score if possible, otherwise // falls back on static position evaluation. - Value refine_eval(const TTEntry* tte, Value defaultEval, int ply) { + Value refine_eval(const TTEntry* tte, Value v, Value defaultEval) { assert(tte); - Value v = value_from_tt(tte->value(), ply); - if ( ((tte->type() & BOUND_LOWER) && v >= defaultEval) || ((tte->type() & BOUND_UPPER) && v < defaultEval)) return v; From c47a74ec62c8c4fdacf6d16eef068b62634122f1 Mon Sep 17 00:00:00 2001 From: Marco Costalba Date: Thu, 22 Mar 2012 22:39:10 +0100 Subject: [PATCH 2/7] Merge two loops in ThreadsManager::init() In analogy with ThreadsManager::exit() No functional change. Signed-off-by: Marco Costalba --- src/misc.h | 3 +-- src/thread.cpp | 15 ++++++--------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/misc.h b/src/misc.h index f37fd5ef..84ea9cd5 100644 --- a/src/misc.h +++ b/src/misc.h @@ -30,6 +30,7 @@ extern const std::string engine_info(bool to_uci = false); extern int cpu_count(); extern void timed_wait(WaitCondition&, Lock&, int); extern void prefetch(char* addr); +extern void start_logger(bool b); extern void dbg_hit_on(bool b); extern void dbg_hit_on_c(bool c, bool b); @@ -60,6 +61,4 @@ private: sys_time_t t; }; -extern void start_logger(bool b); - #endif // !defined(MISC_H_INCLUDED) diff --git a/src/thread.cpp b/src/thread.cpp index 358ced5d..31c5961d 100644 --- a/src/thread.cpp +++ b/src/thread.cpp @@ -221,15 +221,6 @@ void ThreadsManager::init() { cond_init(sleepCond); lock_init(splitLock); - for (int i = 0; i <= MAX_THREADS; i++) - { - lock_init(threads[i].sleepLock); - cond_init(threads[i].sleepCond); - - for (int j = 0; j < MAX_SPLITPOINTS_PER_THREAD; j++) - lock_init(threads[i].splitPoints[j].lock); - } - // Allocate main thread tables to call evaluate() also when not searching threads[0].pawnTable.init(); threads[0].materialTable.init(); @@ -241,6 +232,12 @@ void ThreadsManager::init() { threads[i].do_sleep = (i != 0); // Avoid a race with start_thinking() threads[i].threadID = i; + lock_init(threads[i].sleepLock); + cond_init(threads[i].sleepCond); + + for (int j = 0; j < MAX_SPLITPOINTS_PER_THREAD; j++) + lock_init(threads[i].splitPoints[j].lock); + if (!thread_create(threads[i].handle, start_routine, threads[i])) { std::cerr << "Failed to create thread number " << i << std::endl; From 06f33ff1ee0c0e8345cb6d5b84ce5db9f043203d Mon Sep 17 00:00:00 2001 From: Marco Costalba Date: Fri, 23 Mar 2012 13:12:26 +0100 Subject: [PATCH 3/7] Remove last platform specific code form thread.cpp A somewhat tricky function pointer cast allows us to move the platform specifics to lock.h, the cast is tricky because return type is not the same of the casted function in Linux (for Windows return type is a DWORD that is a long) but this should not be a problem as long as the size is the same; From: http://stackoverflow.com/questions/188839/function-pointer-cast-to-different-signature "OpenSSL was only casting functions pointers to other function types taking and returning the same number of values of the same exact sizes, and this (assuming you're not dealing with floating-point) happens to be safe across all the platforms and calling conventions I know of. However, anything else is potentially unsafe." No functional change. Signed-off-by: Marco Costalba --- src/lock.h | 5 +++-- src/thread.cpp | 10 ++-------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/lock.h b/src/lock.h index 1e71305e..85d32a5c 100644 --- a/src/lock.h +++ b/src/lock.h @@ -27,6 +27,7 @@ typedef pthread_mutex_t Lock; typedef pthread_cond_t WaitCondition; typedef pthread_t ThreadHandle; +typedef void*(*start_fn)(void*); # define lock_init(x) pthread_mutex_init(&(x), NULL) # define lock_grab(x) pthread_mutex_lock(&(x)) @@ -37,7 +38,7 @@ typedef pthread_t ThreadHandle; # define cond_signal(x) pthread_cond_signal(&(x)) # define cond_wait(x,y) pthread_cond_wait(&(x),&(y)) # define cond_timedwait(x,y,z) pthread_cond_timedwait(&(x),&(y),z) -# define thread_create(x,f,id) !pthread_create(&(x),NULL,f,&(id)) +# define thread_create(x,f,id) !pthread_create(&(x),NULL,(start_fn)f,&(id)) # define thread_join(x) pthread_join(x, NULL) #else @@ -67,7 +68,7 @@ typedef HANDLE ThreadHandle; # define cond_signal(x) SetEvent(x) # define cond_wait(x,y) { lock_release(y); WaitForSingleObject(x, INFINITE); lock_grab(y); } # define cond_timedwait(x,y,z) { lock_release(y); WaitForSingleObject(x,z); lock_grab(y); } -# define thread_create(x,f,id) (x = CreateThread(NULL,0,f,&(id),0,NULL), x != NULL) +# define thread_create(x,f,id) (x = CreateThread(NULL,0,(LPTHREAD_START_ROUTINE)f,&(id),0,NULL), x != NULL) # define thread_join(x) { WaitForSingleObject(x, INFINITE); CloseHandle(x); } #endif diff --git a/src/thread.cpp b/src/thread.cpp index 31c5961d..5c23aa50 100644 --- a/src/thread.cpp +++ b/src/thread.cpp @@ -36,13 +36,7 @@ namespace { extern "C" { // and last thread are special. First one is the main search thread while the // last one mimics a timer, they run in main_loop() and timer_loop(). -#if defined(_WIN32) || defined(_WIN64) - DWORD WINAPI start_routine(LPVOID thread) { -#else - void* start_routine(void* thread) { -#endif - - Thread* th = (Thread*)thread; + long start_routine(Thread* th) { if (th->threadID == 0) th->main_loop(); @@ -299,7 +293,7 @@ bool ThreadsManager::available_slave_exists(int master) const { template Value ThreadsManager::split(Position& pos, Stack* ss, Value alpha, Value beta, Value bestValue, Move* bestMove, Depth depth, - Move threatMove, int moveCount, MovePicker *mp, int nodeType) { + Move threatMove, int moveCount, MovePicker* mp, int nodeType) { assert(pos.pos_is_ok()); assert(bestValue > -VALUE_INFINITE); assert(bestValue <= alpha); From b356e0fae3b78e39af2ae8aca6ca6197e8669819 Mon Sep 17 00:00:00 2001 From: Marco Costalba Date: Sat, 24 Mar 2012 09:52:41 +0100 Subject: [PATCH 4/7] Rename lock.h to platform.h And move some more platform specific code there. No functional change. Signed-off-by: Marco Costalba --- src/misc.h | 1 - src/{lock.h => platform.h} | 45 +++++++++++++++++++++++++++++++++----- src/thread.h | 1 - src/types.h | 36 +----------------------------- 4 files changed, 40 insertions(+), 43 deletions(-) rename src/{lock.h => platform.h} (68%) diff --git a/src/misc.h b/src/misc.h index 84ea9cd5..8d21c1ad 100644 --- a/src/misc.h +++ b/src/misc.h @@ -23,7 +23,6 @@ #include #include -#include "lock.h" #include "types.h" extern const std::string engine_info(bool to_uci = false); diff --git a/src/lock.h b/src/platform.h similarity index 68% rename from src/lock.h rename to src/platform.h index 85d32a5c..26c3abfb 100644 --- a/src/lock.h +++ b/src/platform.h @@ -17,13 +17,40 @@ along with this program. If not, see . */ -#if !defined(LOCK_H_INCLUDED) -#define LOCK_H_INCLUDED +#if !defined(PLATFORM_H_INCLUDED) +#define PLATFORM_H_INCLUDED -#if !defined(_WIN32) && !defined(_WIN64) +#if defined(_MSC_VER) + +// Disable some silly and noisy warning from MSVC compiler +#pragma warning(disable: 4127) // Conditional expression is constant +#pragma warning(disable: 4146) // Unary minus operator applied to unsigned type +#pragma warning(disable: 4800) // Forcing value to bool 'true' or 'false' +#pragma warning(disable: 4996) // Function _ftime() may be unsafe + +// MSVC does not support +typedef signed __int8 int8_t; +typedef unsigned __int8 uint8_t; +typedef signed __int16 int16_t; +typedef unsigned __int16 uint16_t; +typedef signed __int32 int32_t; +typedef unsigned __int32 uint32_t; +typedef signed __int64 int64_t; +typedef unsigned __int64 uint64_t; + +#else +# include +#endif + +#if !defined(_WIN32) && !defined(_WIN64) // Linux - Unix + +# include +typedef timeval sys_time_t; + +inline void system_time(sys_time_t* t) { gettimeofday(t, NULL); } +inline uint64_t time_to_msec(const sys_time_t& t) { return t.tv_sec * 1000LL + t.tv_usec / 1000; } # include - typedef pthread_mutex_t Lock; typedef pthread_cond_t WaitCondition; typedef pthread_t ThreadHandle; @@ -41,7 +68,13 @@ typedef void*(*start_fn)(void*); # define thread_create(x,f,id) !pthread_create(&(x),NULL,(start_fn)f,&(id)) # define thread_join(x) pthread_join(x, NULL) -#else +#else // Windows and MinGW + +# include +typedef _timeb sys_time_t; + +inline void system_time(sys_time_t* t) { _ftime(t); } +inline uint64_t time_to_msec(const sys_time_t& t) { return t.time * 1000LL + t.millitm; } #if !defined(NOMINMAX) # define NOMINMAX // disable macros min() and max() @@ -73,4 +106,4 @@ typedef HANDLE ThreadHandle; #endif -#endif // !defined(LOCK_H_INCLUDED) +#endif // !defined(PLATFORM_H_INCLUDED) diff --git a/src/thread.h b/src/thread.h index 11c8778d..e71db835 100644 --- a/src/thread.h +++ b/src/thread.h @@ -22,7 +22,6 @@ #include -#include "lock.h" #include "material.h" #include "movepick.h" #include "pawns.h" diff --git a/src/types.h b/src/types.h index 9567d6b8..7de31d45 100644 --- a/src/types.h +++ b/src/types.h @@ -38,41 +38,7 @@ #include #include -#if defined(_MSC_VER) - -// Disable some silly and noisy warning from MSVC compiler -#pragma warning(disable: 4127) // Conditional expression is constant -#pragma warning(disable: 4146) // Unary minus operator applied to unsigned type -#pragma warning(disable: 4800) // Forcing value to bool 'true' or 'false' -#pragma warning(disable: 4996) // Function _ftime() may be unsafe - -// MSVC does not support -typedef signed __int8 int8_t; -typedef unsigned __int8 uint8_t; -typedef signed __int16 int16_t; -typedef unsigned __int16 uint16_t; -typedef signed __int32 int32_t; -typedef unsigned __int32 uint32_t; -typedef signed __int64 int64_t; -typedef unsigned __int64 uint64_t; - -#else -# include -#endif - -#if defined(_WIN32) || defined(_WIN64) -# include -typedef _timeb sys_time_t; - -inline void system_time(sys_time_t* t) { _ftime(t); } -inline uint64_t time_to_msec(const sys_time_t& t) { return t.time * 1000LL + t.millitm; } -#else -# include -typedef timeval sys_time_t; - -inline void system_time(sys_time_t* t) { gettimeofday(t, NULL); } -inline uint64_t time_to_msec(const sys_time_t& t) { return t.tv_sec * 1000LL + t.tv_usec / 1000; } -#endif +#include "platform.h" #if defined(_WIN64) # include // MSVC popcnt and bsfq instrinsics From f8224fc7d31324376c36c4788f1935a341d2187b Mon Sep 17 00:00:00 2001 From: Marco Costalba Date: Sat, 24 Mar 2012 10:14:21 +0100 Subject: [PATCH 5/7] Fix a MSVC warning Not correct warning about use of an uninitialized variable. The warning is not correct becuase we can never reach the warned code when in SpNode, anyhow the fix is simple. No functional change. Signed-off-by: Marco Costalba --- src/search.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/search.cpp b/src/search.cpp index 8d1b43b0..fb5d375e 100644 --- a/src/search.cpp +++ b/src/search.cpp @@ -564,6 +564,7 @@ namespace { { tte = NULL; ttMove = excludedMove = MOVE_NONE; + ttValue = VALUE_ZERO; sp = ss->sp; bestMove = sp->bestMove; threatMove = sp->threatMove; From 11b0c7b44a2ce6b0a39bcb40849dd88b40d49a09 Mon Sep 17 00:00:00 2001 From: Marco Costalba Date: Sun, 25 Mar 2012 09:54:20 +0100 Subject: [PATCH 6/7] Don't ceil cpu_count() It is already done at calling site where it is more appropiate. No functional change. Signed-off-by: Marco Costalba --- src/misc.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/misc.cpp b/src/misc.cpp index 8bc413f4..459c13cc 100644 --- a/src/misc.cpp +++ b/src/misc.cpp @@ -35,7 +35,6 @@ # include #endif -#include #include #include #include @@ -174,16 +173,16 @@ int cpu_count() { #if defined(_WIN32) || defined(_WIN64) SYSTEM_INFO s; GetSystemInfo(&s); - return std::min(int(s.dwNumberOfProcessors), MAX_THREADS); + return s.dwNumberOfProcessors; #else # if defined(_SC_NPROCESSORS_ONLN) - return std::min((int)sysconf(_SC_NPROCESSORS_ONLN), MAX_THREADS); + return sysconf(_SC_NPROCESSORS_ONLN); # elif defined(__hpux) struct pst_dynamic psd; if (pstat_getdynamic(&psd, sizeof(psd), (size_t)1, 0) == -1) return 1; - return std::min((int)psd.psd_proc_cnt, MAX_THREADS); + return psd.psd_proc_cnt; # else return 1; # endif From 8ec421fa1428584e47388f68db931b3e9776b7c6 Mon Sep 17 00:00:00 2001 From: Marco Costalba Date: Sun, 25 Mar 2012 10:23:16 +0100 Subject: [PATCH 7/7] Revert "Don't sync with C library I/O buffers" It seems is the cause of strange and rare hangs reported by some users where Stockfish stops responding to GUI. It is not clear why but for the moment revert the patch. No functional change. Signed-off-by: Marco Costalba --- src/main.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 31337fa6..1d5c6ea7 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -35,11 +35,6 @@ extern void kpk_bitbase_init(); int main(int argc, char* argv[]) { - // Don't sync with C library I/O buffers, faster but now using printf() - // or scanf() could yield to issues because buffers are independent. - cout.sync_with_stdio(false); - cin.sync_with_stdio(false); - cout << engine_info() << endl; bitboards_init();