Retire RootMoveNumber and use FirstRootMove instead

It is more clear why we use that global flag.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Save mateThreat flag in splitPoint and make use of it

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Document one test result

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Synchronize root_search() with other search routines

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Remove obsolete code snippet from root_search

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Sort again root moves after a fail low

Currently we use original sorting after a fail low to
research at wider window. This patch instead sorts the
moves according to the last available move's scores.

Strangely no functional change, but should be.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Additional search.cpp cleanup

Changed FutilityMarginsMatrix dimensions to be a power of two
so that compiler can produce a faster accessing code.

Introduced print_pv_info() to remove some redundant code in
root_search()

Remaining stuff is triviality and documentation tweaks.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Destroy all locks before to exit

And use platform-independent functions
where possible.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

We don't need lpThreadId parameter in CreateThread()

Under Windows we use CreateThread() to setup threads and
we pass a pointer to a variable that receives the thread
identifier, but this parameter is optional and we don't
use it, so remove it.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Function init_thread() should return an integer under Windows

It happens that NULL is 0, but the conventional meaning is of
a zero pointer, so repleace with an explicit 0 integer value.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Try bad captures before non-captures

Consider sligtly negative captures as good if at low
depth and far from beta.

After 999 games at 1+0
Mod vs Orig +169 =694 -136  +11 ELO

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Revert previous patch

It raises an assert under Windows, it is not clear why but it
happens that idle_loop() is called with incorrect threadID and
the assert triggered is:

assert(threadID >= 0 && threadID < MAX_THREADS);

So revert the patch for now, but we should understand why it
fails.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Do not wait for sleeping in init_threads()

We can't do it with full guarantee anyway because
there is always a possible race between the setting of
state to THREAD_SLEEPING and actual sleeping.

So just remove the not perfect code to avoid misunderstandings.
This reflects what we have done in wake_sleeping_threads() in
the previous patch.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Remove an incorrect assert in wake_sleeping_threads()

Currently there is no guarantee that threads are sleeping
when calling wake_sleeping_threads() because put_threads_to_sleep()
returns without waiting for threads to actually sleep.

Assert can be easily triggered calling put_threads_to_sleep() and
wake_sleeping_threads() in a tight loop.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Add some commentary

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Clean up common adjustments

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Remove "Threat Depth" ucioption

This option likely has very low meaning for playing strength and style,
so I see no need to keep this configurable

No functional change

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Cleanup steps 12, 14

No functional change

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Clean up step 11

No functional change

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Clean steps 8 and 9.

No functional change

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Clean razoring code (step 6)

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Synchronize variable listing of 4 different search routines

search() is used as a "leading star" and other routines
are modified according to it.

No functional change

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Remove current line printing in SMP mode

Was broken and fixing would be too messy.
Now this option is only activated in single thread mode

No functional change

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Synchronize sp_search() with search() part I

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Rename staticValue to refinedValue

Just to avoid misunderstandings.
True staticValue is available through search stack

No functional change

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Fix repetition detection bug

Bug spotted by Jouni Uski and fix suggested by Pablo Vazquez

Also add note that we are not always handling fifty move rule correctly

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Synchronize sp_search_pv() with search_pv()

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Synchronize sp_search() with search() part II

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Simplify locking in splitpoint search

One rule: Always lock before picking up a move.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Synchronize search_pv() with search take II

No functional change

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Synchronize search_pv() with search take I

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Search code documentation, take III

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Search code documentation take II

No functional change

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Separate razoring from null move

I cannot see connection between the two.

Also add one FIXME for illogical behaviour

No functional change

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Split search() in independent sections

I don't know if enumerating sections is a good idea,
but for me code is more readable this way

No functional change

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Avoid research in case thread has already been asked to stop

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Add one assert

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Do not wait for threads falling asleep

I cannot see any reason to do this. Even this is not enough to fix
theoretical race case on Windows which doesn't seem to cause any
problems in practice anyhow

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Remove unnecessary conditions from if-clauses and replace them with asserts

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Fix crash in debug mode

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Fix some races

Resurrect extra check for sleeping in POSIX code.
This necessary to prevent ugly races between
thread_broadcast and thread_cond_wait.

After thread has woken up, it marks itself as available.
Another thread must not do this, because of possible race.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Fix one assert

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Fix compile errors in debug mode

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Document struct SplitPoint fields constness

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Beta is never changed after an sp_search()

So we can use a const value instead of a pointer in
split().

Also pass NULL instead of a faked address of alpha in
case split is called from a non-PV node.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Supress make warning on missing .depend file

This is generated by make itself, so the warning
is useless.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Fix an icc warning

remark #1599: declaration hides variable "i" (declared at line 2651)

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Code style triviality in split()

No functional change

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Fix a warning with POPCNT and MSVC

Intrinsic __popcnt64() returns an unsigned __int64, cast
to an integer and silence the warning.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Revert "Recursive lock"

Joona says that sp_update_pv() does not pass the split point
boundaries, so there is no risk to corrupt data from another
split point. Also the race on thread_should_stop() is harmless
because of this.

So revert the patch and come back to single lock.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Remove a couple of useless thread_should_stop() calls

We test for it anyway few lines below and even under lock
protection.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Revert small state change optimization in idle_loop()

Joona says:

1. We should not be afraid of "AllThreadsShouldExit" flag.
Because when this is set to true we _must_not_ be searching (= All
splits must have been undone).
And if we are not searching it's impossible that some other thread
could give us work to do. So setting state to THREAD_AVAILABLE
doesn't do any harm. If you want to add check for this, you could do
it like this:

 if (threads[threadID].state == THREAD_WORKISWAITING)
 {
+    assert(!AllThreadsShouldExit)
     threads[threadID].state = THREAD_SEARCHING;

2a. If waitSp->cpus == 0, setting state to THREAD_AVAILABLE makes
no harm either, because helpful master concept dictates that _only_
our own slave can book us. If we don't have any slaves, noone has the
right to book us.

2b. If point (2a) is not correct then your extra check only adds extra race:
In smp code checking for waitSp->cpus > 0 is not enough. It's possible that
our slave immediately exits and another thread
books us as a slave when our state is still
THREAD_AVAILABLE. So instead of adding extra level of security we have
just introduced extra race.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Recursive lock all split point's chain

When we found a cut-off then lock all the split point chain,
not only current one to avoid races in case two threads running
on different split points where one is ancestor then the other,
find a beta cut-off at the same time, in this case we want only
one to call sp_update_pv().

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Retire per-thread stopRequest flag

This is a per split-point request, not per-thread. When we find
a beta cut-off in current thread's split point or in or in some
ancestor of the current split point then threads should stop
immediately the search and return to idle_loop().

The check is done by thread_should_stop() that now looks only
at split point's chain.

No functional change and a good semplification.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Use state instead of flags to track threads

This is easier to follow and also reduces the points
where state changes to mainly idle_loop() and split().

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Rename THREAD_MAX in MAX_THREADS

Also rename idle_thread_exists() in available_thread_exists()

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Search negative SEE moves in qsearch in PV

After 2704 games on slow single core
mod - orig: 1381 - 1323

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Use zero null move margin when depth < 4 * OnePly

This is because when we are below 4 * OnePly, the null move
will directly jump to qsearch and if we are below beta,
our opponent is above beta and will get immediate
stand pat cut off.

So basically this patch is just optimizing away useless
evaluation calls. dbg_hit_on() runs show that this heuristic
is correct >99% of cases. Transposition table probably causes
some inaccurary?

After 1148 games on QUAD
mod-orig: 583 - 565 +5 elo

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Fix another setting of a flag out of lock protection

In this case is dangerous because in split() we reset the flag to
false, but if it was set due to a cut-off higher in the tree we
completely miss that and go on with the full search.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Rename flag 'stop' in 'stopRequest'

Instead of other flags this is not a state flag, i.e. does
not defines a state for the thread, but a request because
after we raise 'stopRequest' flag the corresponding thread is
not stopped, but continues to run for a while until it returns
from sp_search() in idle_loop.

It is important the name reflects this.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Reset thread flags to a known state before to exit think()

Among them 'stop' and 'printCurrentLineRequest' could have
random value, so reset to a known state before to leave the
search.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Fix 'stop' flag changed out of lock protection

This is the first nice effect of previous patch !

Because thread_should_stop() should be declared 'const' we
need to remove the setting of 'stop' flag to true that
turns out to be a bug because thread_should_stop() is called
outside from lock protection while 'stop' flag is a volatile
shared variable so cannot be changed when not in lock.

Note that this bugs fires ONLY when we use more then 2 threads,
so commonly only in a QUAD or OCTAL machine.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Introduce ThreadsManager class

Main aim of this patch is to consolidate all the thread related stuff
behind a single class interface so to avoid messing with global flags
and having thread code scattered among non-thread related stuff.

Another advantage is that now access to thread's variables is
more controlled, in particular we can differentiate between
read and write accesses by the mean of different interfaces, it
is so simpler to understand how a function is related to threads.

Lastly this rewrite is the base for future code consolidations and
semplifications that are easier now that we have only one thread's
access point.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Fix compile error under gcc

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Ensure function boundaries for threads state changes

Ensure threads are sleeping when leaving init_threads() and
the newly introduced put_threads_to_sleep().

Also ensure threads are not sleeping when leaving
wake_sleeping_threads().

As a side effect we now leave think() with all the threads
(but the main one) guaranteed to sleep. So when we enter
again in think(), after the opponent next move, we know
threads must be sleeping.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Rename stop_threads() to exit_threads()

More stick to what actually happens.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Be sure threads are woken in wake_sleeping_threads()

Wait inside wake_sleeping_threads() for the threads to be
effectively and reliably woken up.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Use Thread c'tor to properly init the struct

This is what c'tors are for.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Add 'sleeping' flag to struct Thread

Will be used by future patches. Also:

- Renamed Idle in AllThreadsShouldSleep

- Explicitly inited AllThreadsShouldExit and AllThreadsShouldSleep
  in init_thread() instead of use an anonymous global initialization.

- Rewritten idle_loop() while condition to avoid a 'break' statement

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Allow build on HP-UX 11.X

Patch from Richard Lloyd (slightly edited from me), following the list
of changes as described by the author:

src/Makefile:
- Added PREFIX and BINDIR for the install: rule.
- Added a "make hpux" line to the help: rule.
- Added "make test"/"make check" rule that runs the $(PGOBENCH) command.
- "make clean" now additionally removes core and bench.txt.
- Added an hpux: rule.
- Added an install: rule to mkdir $(BINDIR), copy $(EXE) to $(BINDIR) and
 then strip it.
- "make strip" now ensures that $(EXE) is built first before trying to
 strip it.
- Hide errors and output from the g++ command used by the .depend: rule and
 then touch .depend in case g++ isn't available.
- Hide errors from the "include .depend" in case .depend doesn't exist
 (e.g. directly after a "make clean").

src/book.cpp and src/book.h:
- HP-UX's aCC really didn't like the const keywords used for the
 Book::file_name() definitions, so they were removed. I checked that this
 didn't affect a Linux build and it was still fine.

src/misc.cpp:
- HP-UX uses <sys/pstat.h> and pstat_getdynamic() to determine the number of
 CPU cores, so added conditional code for that (if pstat_getdynamic() fails,
 set the number of cores to 1).

src/tt.cpp:
- <xmmintrin.h> and _mm_prefetch() seem highly specific to the Intel x86(_64)
 and gcc platforms - neither exist in HP-UX, so conditionally avoid that
 code in HP-UX's case. Perhaps some sort of define is needed here
 such as -DHAS_MM_PREFETCH that could be #ifdef'ed for instead?

Even after these changes, it's more convenient for HP-UX users to edit the
default: rule in the Makefile to run "$(MAKE) hpux" before they build
stockfish, but that's not a big deal if they're warned about that first (the
same applies to all other builds other than the standard "$(MAKE) gcc" one).

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Fix a couple of new MSVC 2010 warnings

Compiler complains because in Book we have a d'tor but not
copy c'tor and assignement operator (warning C4511 and C4512),
note that after adding them (just declared) you now need also
default c'tor !

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Retire EvalInfo* in SearchStack

It is an hidden bug waiting to fire. The main problem is
that ss[ply] is overwritten by search() and qsearch() called
from IID and razoring, so that we cannot hold a pointer to a
local EvalInfo variable.

For instance if we go razoring then we overwrite the pointer
with the address of a variable local to qsearch(), when we return
from qsearch() variable goes out of scope and now ss[ply].evalInfo
holds a stale pointer !

Because we are not looking for troubles we go through the
safe route and we remove it entirely.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Small code style triviality

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Implement init_search()

No functional change

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Document lookup tables

No functional change

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Implement futility move count array

No functional change

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Implement futility margins matrix

No functinal change

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Use gain table to order non-captures

Gain value is multiplied by 16 to be of comparable magnitudo
of negative history, on average.

This patch shows very good results in tactical tests, but
started very bad in real games, so I have run two test matches.

After 896 games at 1+0
Mod vs Orig +187 =525 -184 +1 ELO

After 999 games at 1+0
Mod vs Orig +223 =590 -186 +13 ELO

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Use posKey instead of pos.get_key() after NonPVIID

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Use opening book when pondering

Otherwise we will not use move given by opening book
when we receive 'ponderhit'-command.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Delay sorting of negative scored non-captures

We can do this only when needed, if we get a cut-off
before we skip sorting entirely. This reduces sorting
time of about 20%.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Copy 4 SearchStack items in split()

In search routines we use information from previous ply
and init killers two plies ahead.

So for me it seems correct to copy 4 searchstack items
in split:

ply - 1, ply, ply + 1, ply + 2

Because
a) we do not split at root (ply == 0)
b) ply < PLY_MAX and SearchStack size is PLY_MAX_PLUS_2
there should be no risk of underflows or overflows

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Remove sorting optimization for many zeroes

With negative history we don't have anymore a
lot of zeroes to score, so just split moves in
positives and non-positives sets.

Speed up is almost zero, we cannot test speed directly
because node count changed due to reorder, but I have
verified sorting is correct. With a profiler I have
seen we gain a little in sort_moves() and lose a little
in insertion_sort(), so the net effect is almost zero,
but code is simpler.

No real change, just move reordering.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Give FailLow flag more descriptive name

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Remove Problem variable

It was only used to control StopOnPonderHit variable.
Now use FailLow variable instead.

Patch has a minor effect on time management when ponder is on.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Remove unused failHighPly1 flag

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Remove unused FailHigh flag

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Simplify time management

noProblemFound condition is never true.
This was verified by running 800 games 1+0 match in 1 CPU computer.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Be sure negative see evasions are at the bottom

Because H.move_ordering_score() can return negative values
some negative see moves could be searched before non-negative
see moves with negative history.

This patch restores proper ordering.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Score non-captures only by history

Now that history can go negative and is almost alwyas
non zero we have no more reasons to use also psqt term.

After 994 games at 1+0
Mod vs Orig +204 =597 -193 +4 ELO

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Reduction lookup table

No functional change

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Convert gains to use a piece-to mapping

Instead of piece-from-to, in this way it is similar
to what we already do for history.

Almost no change, but seems a bit simpler in this way.

After 995 games at 1+0
Mod vs Orig +207 =596 -192 +5 ELO

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Fix a compile error from previous patch

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Fix indentations

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Retire outdated aspiration search code

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Renamed stand pat as 'static null move pruning'

It seems more standard conformant. Also added a bit of
description directly from Tord.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Save futilityMargin for both colors

It will be needed by future patches.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Fix duplicated scaling function

We erroneusly added two times the same scaling function
to endgame's map.

Fix detected by valgrind becasue resulted in a memleak
of the first added scaling function.

Bug introduced by 30e8f0c9ad6a473 of 13/02/2009

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Increase TT size limit to 8 GB

We had an overflow due to use an integer for hash size,
now we use a size_t as we should, so we can increase to
an higher limit.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

Check bounds in set_option_value()

Normally it's up to the GUI to check for option's limits,
but we could receive the new value directly from the user
by teminal window. So let's check the bounds anyway.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>