Passed non-regression STC at
tests.stockfishchess.org/tests/view/69c9b8c3a54395a761335044 (set to
gainer bounds, *mea culpa*)
There's a major performance penalty when compiling the fish with GCC 16.
@Torom 's measurements (similar to mine, but more precise):
```
GCC 15 (master) vs. 16 (master)
sf_base = 2549174 +/- 2984 (95%)
sf_test = 2435456 +/- 2239 (95%)
diff = -113718 +/- 3599 (95%)
speedup = -4.46100% +/- 0.141% (95%)
GCC 15 (master) vs. 16 (patch)
sf_base = 2544107 +/- 3091 (95%)
sf_test = 2568837 +/- 3164 (95%)
diff = 24729 +/- 4359 (95%)
speedup = 0.97204% +/- 0.171% (95%)
```
Based on looking at the disassembly + profiling data of the builds, the
root cause appears to be overly aggressive code duplication from
constant propagation of various functions especially in `search.cpp`.
The worst offender is `Worker::search`, which I think is being cloned
and optimized based on the value of `cutNode`. `perf stat` indicates
that the instruction cache miss rate goes up by ~50% on the GCC 16
build.
closes https://github.com/official-stockfish/Stockfish/pull/6691
No functional change
Passed simp STC
LLR: 2.94 (-2.94,2.94) <-1.75,0.25>
Total: 29632 W: 7772 L: 7552 D: 14308
Ptnml(0-2): 76, 3155, 8147, 3349,
Id expect there to be a small speedup (bc the compiler can skip some
stores) but i haven’t tried to measure it
closes https://github.com/official-stockfish/Stockfish/pull/6688
No functional change
In master for various reasons the given ponder move may get out of sync
with (the second move of) the last PV sent to the GUI:
1. If a ponder move is extracted from the TT despite bestmove leading to
a game ending draw.
2. If a ponder move is extracted from the TT from a fail high/low, since
after the ponder move extraction no new PV is sent to the GUI.
3. If `syzygy_extend_pv()` from within the call to `pv()` changes the PV
after the ponder move has been selected.
4. If the PV roll-back to protect mated-in scores differs in the second
move from the final PV sent to the GUI.
This PR keeps ponder move and the last printed PV in sync.
Fixes#6676.
closes https://github.com/official-stockfish/Stockfish/pull/6679
No functional change
STC
```
LLR: 5.45 (-2.94,2.94) <0.00,2.00>
Total: 612192 W: 158602 L: 157260 D: 296330
Ptnml(0-2): 1634, 67783, 166027, 68911, 1741
```
@Torom measured:
```
sf_base = 2380902 +/- 1016 (95%)
sf_test = 2391426 +/- 1065 (95%)
diff = 10524 +/- 1336 (95%)
speedup = 0.44203% +/- 0.056% (95%)
```
I've verified that `_mm512_slli_epi16(v, 0)` gets optimized out in GCC
9+ and clang 4+. Added constants to types.h so that people messing
around with the layout of `Move` don't have to know about this part of
the code.
closes https://github.com/official-stockfish/Stockfish/pull/6670
No functional change
Ever since #5094 master only lets the main thread terminate `go mate`
searches, reverting the earlier improvement #1215.
This PR restores the old logic. So any thread that found a "mate in x"
can now stop the search.
To make this work robustly, we need to guard against inexact mate scores
in the best thread selection. In addition, in contrast to time limits,
the main thread may now not complete a d1 search for a mated-in
position.
In master an aborted d1 search may have a PV beginning with
`Move::none()`, in which case no thread selection is performed. See
#623. We fix this bug here by checking if `lastBestPV` is empty or not.
For interrupted d1 searches we now label mated-in scores as inexact.
While at it, we also simplify the logic for detecting if we can
terminate a go mate x search, using the fact that threads.stop can only
be false if we have a completed iteration with a valid score.
The PR has no effect on game play, but should slightly improve general
mate finding and speed up multi-threaded `go mate` searches.
We also add a corresponding matecheck run to the CI. This only involves
61 mates up to mate-in-2. Test runs with the first 50 or 100 mates from
`mates2000.epd` did at times not finish within 30 minutes on my fork or
in local tests, possibly due to search explosions for some mate-in-3
positions.
closes https://github.com/official-stockfish/Stockfish/pull/6668
No functional change
fixes#6663, #6664, and a million others issues raised over the years.
This is similar to
https://github.com/official-stockfish/Stockfish/pull/4563/changes but
more conservative and errors are split between "Unsupported position"
and "Invalid FEN".
The FEN parser needs to be strict as a foundation for safety. It does
not specify much of the semantics, so this step is fairly simple. Parts
after the ep square are optional, however, since it's common, for
example in EPD notation.
Errors arising from positional semantics that were previously bucketed
under invalid FENs are now reported as "Unsupported position". Only
positions that are potentially problematic are designated as
unsupported. It is NOT guided by illegality of the position, but instead
by the ability of the engine to handle them correctly. This means that
some checks from the previous PR were removed.
std::exit is used instead of std::terminate so atexit handles will be
called.
Probably wise to run copilot on this or smth because I scribbled it
without much thought.
With these small changes and reduced, less controversial, scope I hope
this PR will finally make it and we will be done with these weekly
issues. I'll wait with putting it on fishtest until there's approval.
closes https://github.com/official-stockfish/Stockfish/pull/6665
No functional change
Thanks to MinetaS for discovering this and finding the culprit commit,
specific to this architecture. 9f42980 was subtly incorrect, leading to
spurious wrong benches on avx512icl. Because this piece of code has twice been
the source of wrong benches, I also added a brief comment explaining why it
needs to be this way, and added a sample position to bench.
closes https://github.com/official-stockfish/Stockfish/pull/6654
Bench: 2288704
Currently, the engine updates each level of the continuation history stack (ply -1, -2, -3, etc.) with fixed weights that are independent of one another. The idea behind CMHC is to scale the magnitude of these updates based on the consistency of the move's performance across the stack.
By tracking how often a move has historically performed well (positive history) within the current continuation context, we can adjust the update magnitude dynamically. Moves that demonstrate "continuity" (appearing as strong candidates across multiple historical contexts) receive more significant updates, while unproven or inconsistent moves are dampened.
Passed STC:
LLR: 2.94 (-2.94,2.94) <0.00,2.00>
Total: 103264 W: 26954 L: 26541 D: 49769
Ptnml(0-2): 338, 11910, 26728, 12313, 343
https://tests.stockfishchess.org/tests/view/69a81702fac54339cf435a87
Passed LTC:
LLR: 2.94 (-2.94,2.94) <0.50,2.50>
Total: 47406 W: 12259 L: 11920 D: 23227
Ptnml(0-2): 22, 5052, 13237, 5349, 43
https://tests.stockfishchess.org/tests/view/69aab480bd35724e4b290498
closes https://github.com/official-stockfish/Stockfish/pull/6653
Bench: 2356418
Co-Authored-By: Daniel Samek <samekdaniel@email.cz>
This PR removes the variable threads.abortedSearch introduced in #4990 and
relies on threads.stop and completedDepth instead.
The logic in master is only guaranteed to work in single threaded search. For
multiple threads, as soon as threads.stop is set, some of the threads may abort
their search, and so their mated-in scores are not reliable.
Compared to master, this patch prevents unproven mated-in scores in these
multi-threaded scenarios:
in game play (when time management is used)
when the uci commands quit or stop are received
when maximum depth is reached
for go mate commands
The patch will change the bestmove in those very rare situations where the bug
occurs in master. This is because bestmove is taken from rootMoves[0].pv[0],
which will now no longer contain unproven mated-in PVs. In that sense the patch
is "functional". But of course, it does not change bench.
While at it, we also fix a bug in master that suppressed PV output for inexact
mated-in scores on receiving threads.stop. In cases where uciScore < score with
score not being a proven loss, and where the new bestmove was different from
the last completed iteration, this led to a bestmove output that did not match
the final PV (which was for an exact score).
Fixes#6293.
Fixes#6626.
closes https://github.com/official-stockfish/Stockfish/pull/6636
No functional change
Passed STC (first run):
https://tests.stockfishchess.org/tests/view/698030ee6362aee5c8a552c7
LLR: 2.96 (-2.94,2.94) <0.00,2.00>
Total: 120800 W: 31336 L: 30912 D: 58552
Ptnml(0-2): 330, 13084, 33172, 13460, 354
Passed LTC:
https://tests.stockfishchess.org/tests/view/6983946d473df9d1d24a916d
LLR: 2.94 (-2.94,2.94) <0.50,2.50>
Total: 86850 W: 22352 L: 21951 D: 42547
Ptnml(0-2): 45, 8506, 25934, 8883, 57
Passed STC SMP (after changes made after opening the PR):
https://tests.stockfishchess.org/tests/view/6987b4c1b0f3ca5200aaf9da
LLR: 2.95 (-2.94,2.94) <0.00,2.00>
Total: 84480 W: 21994 L: 21627 D: 40859
Ptnml(0-2): 98, 9378, 22932, 9723, 109
Passed STC (second run, as a sanity check made after changes made after opening the PR -- these changed, however, didn't change assembly code produced by the compiler):
https://tests.stockfishchess.org/tests/view/6987ad21b0f3ca5200aaf9c9
LLR: 2.93 (-2.94,2.94) <0.00,2.00>
Total: 132192 W: 34526 L: 34083 D: 63583
Ptnml(0-2): 421, 14511, 35759, 15014, 391
GCC maps __builtin_prefetch locality=1 to PREFETCHT2 instruction, targeting L2
cache. To make things clear, added template-based approach as suggested by
Disservin.
PREFETCHT2 should be the appropriate choice for the 78 MB threat weight table:
avoids L1d pollution, parks data in L2 where it is promoted to L1d on actual
load.
With 200-500 cycle lead time from make_index, data has time to arrive in L2.
suggestions by Disservin and anematode included
closes https://github.com/official-stockfish/Stockfish/pull/6602
No functional change
Replace FNV-1a hash algorithm by public domain MurmurHash64A, by Austin
Appleby, which is faster and has better hash properties. This is also
the default hash algorithm of GNU C++ Standard Library.
Startup time on a given system (with x86-64-avx2 binaries) reduces from
378ms to 332ms (46ms faster). Hashing of the largest data input (46.1MB)
goes from 44.2ms to 6.2ms, so about 7x faster.
No functional change
closes https://github.com/official-stockfish/Stockfish/pull/6579
Bench: 2570943
Even though this failed with non regression bounds on fishtest, merging
it as is to improve the current situation.
Failed Non Regression:
https://tests.stockfishchess.org/tests/view/6988814db0f3ca5200aafb50
LLR: -2.94 (-2.94,2.94) <-1.75,0.25>
Total: 185792 W: 48109 L: 48565 D: 89118
Ptnml(0-2): 553, 21228, 49811, 20730, 574
Previously we had code similar to this
```
alignas(64) std::uint8_t foo[1024];
const auto* int_ptr = reinterpret_cast<const std::int32_t*>(foo);
```
Which is a strict aliasing violation once we dereference int_ptr, which
invokes UB and causes newer gcc version to emit incorrect code.
fixes https://github.com/official-stockfish/Stockfish/issues/6598
fixes https://github.com/official-stockfish/Stockfish/issues/4932
This violation was introduced in PR #3287 and merged in commit 23c385e.
Due to aggressive compiler optimizations, users may need to include
CXXFLAGS=-fno-strict-aliasing as a build argument to ensure a correctly
functioning binary.
Also thanks to @anematode for writing parts of this and realizing this
strict aliasing violation.
closes https://github.com/official-stockfish/Stockfish/pull/6599
Bench: 2668754
Co-Authored-By: Timothy Herchen <timothy.herchen@gmail.com>
