With the latest master I started getting:
"A breakpoint instruction (__debugbreak() statement or a similar call) was executed in Stockfish.exe." when debugging in MSVC.
I don't really understand the issue but Fable fixed it and I verified it works.
Here is its summary:
Bug: Calling _get_wpgmptr() in CommandLine::get_binary_directory() triggered a debug CRT assertion failure (__debugbreak()) on Windows. The CRT only initializes the wide program name _wpgmptr when the application uses a wide entry point (wmain). Since Stockfish uses the narrow main(int, char**) entry point, _wpgmptr remains nullptr, and the debug CRT's parameter validation asserts when _get_wpgmptr is called.
Fix: Replaced _get_wpgmptr() with GetModuleFileNameW(), which queries the executable path directly from the OS and does not depend on the CRT entry point type. The implementation uses a grow-and-retry loop, so paths longer than MAX_PATH are handled correctly. If the API fails, the code falls back to the original argv0-based behavior. The #ifdef _MSC_VER guard was also removed, since the fix no longer relies on the MSVC CRT — non-MSVC Windows builds (e.g., MinGW) now benefit from the accurate path as well.
closes https://github.com/official-stockfish/Stockfish/pull/6972
No functional change
The following have zero references repo-wide:
Move::to_sq_unchecked() (types.h): added in 542c30c2 for a
branchless correction-history update; its only caller was removed
in 5ae13d2b ("Remove Redundant Branchless Execution"), leaving the
method unused.
PipeDeleter (misc.h): the custom deleter for the std::unique_ptr<FILE,
PipeDeleter> that wrapped popen("lscpu"). That pipe was removed in
c8375c2f ("On linux use sysfs instead of lscpu"), so the struct has
been unused since.
Dropping PipeDeleter also removes the last user of <cstdio> in misc.h, so that include is dropped as well.
closes https://github.com/official-stockfish/Stockfish/pull/6948
No functional change
This reverts commit 9d4090e826.
causes very poor mate finding performance, ultimately causing a CI hang on a mate in 2
Testcase:
position fen k1B5/2p5/NbN5/P7/3p4/P2p4/P1prp3/2RbK3 b - - 0 1 setoption name Hash value 16
setoption name Threads value 4
go mate 2
ucinewgame
closes https://github.com/official-stockfish/Stockfish/pull/6966
Bench: 2466447
`Skill::pick_best()` assumes the root moves are sorted by score in descending order:
```cpp
Value topScore = rootMoves[0].score;
int delta = std::min(topScore - rootMoves[multiPV - 1].score, int(PawnValue));
```
That does not have to be so with Syzygy tablebases at the root, where the moves are ordered by `tbRank` instead. `rootMoves[0]` is then not the highest score and `delta` can go negative. Since `delta * (rng.rand<unsigned>() % int(weakness))` is evaluated as unsigned, a negative `delta` wraps to a large value and the `int(...)` cast overflows, so the `score + push >= maxScore` check never passes and `best` stays `Move::none()`. The caller
```cpp
std::swap(rootMoves[0],
*std::find(rootMoves.begin(), rootMoves.end(),
skill.best ? skill.best : skill.pick_best(rootMoves, multiPV)));
```
then dereferences `end()`. The result is a crash, or sometimes `bestmove (none)` / an illegal move, in tablebase endgames when `UCI_LimitStrength` is set (or `Skill Level` is below 20).
The fix computes the score range over the candidate moves directly, so `delta` stays non-negative and a valid move is always returned.
**To reproduce**:
```
setoption name UCI_LimitStrength value true
setoption name UCI_Elo value 2900
setoption name SyzygyPath value <syzygy tablebases path>
position fen 8/8/8/4k3/8/8/3BKN2/8 b - - 0 1
go wtime 250 btime 250 winc 100 binc 100
```
You need to call the go command a couple of times before crash, after which is ends with
```
bestmove a1e5 ponder (none)
```
and on the next `go wtime 250 btime 250 winc 100 binc 100` it crashes printing:
```
info string Available processors: 0-31
info string Using 1 thread
info string NNUE evaluation using nn-af1339a6dea3.nnue (106MiB, (83248, 1024, 31, 32, 1))
info string Network replica 1: Shared memory.
```
Bench is unchanged, since this code only runs under `UCI_LimitStrength` / `Skill Level`.
closes https://github.com/official-stockfish/Stockfish/pull/6957
No functional change
Reduce the number of *easy* ways to hit UB or an uncaught exception. This builds on Sopel's work on FEN validation.
Obviously SF is not and will never be a security boundary, but at least we can reduce the number of reports about uncaught `std::stoi` out of range, etc.
I also added a gentle fallback to `NumaConfig` specifically because that's something that (as a dev) I frequently set by hand in the CLI, but happy to revert if that's too annoying of a change.
closes https://github.com/official-stockfish/Stockfish/pull/6844
No functional change
In the spirit of previous such PRs, this PR proposes a collection of nonfunctional changes.
I am happy to incorporate changes from other devs, and revert some of the proposed changes if the maintainers ask me to.
Apart from trivial changes, the proposed changes so far include:
* A requested edit to `AUTHORS` and a resorting of all entries, following DIN 5007 for the treatment of any special characters.
* Exclude the two recent integer type renaming commits from git blame.
* Tightening of some static asserts in `history.h` to avoid overflows. (Note that rounding errors for floating point types could lead to the assert in `operator<<` triggering at run-time.)
* Re-instate the 0.5s maximal thinking time in case of a single legal move and reword the comment to make it clear that it should not be tuned.
* ~~A small refactoring of the network loading code thanks to @dubslow.~~
* A refactoring of the "dtz is dtm" code, also thanks to @dubslow.
closes https://github.com/official-stockfish/Stockfish/pull/6928
No functional change
GCC 12.2 added `__rbitll` intrinsic to `arm_acle.h`, so we can add a fallback for older versions
Clang 10 (our new minimum supported version) is unaffected by all this
closes https://github.com/official-stockfish/Stockfish/pull/6949
No functional change
Fixes problems with non-ASCII characters in paths. Fixes#3424.
Changes
- Bump the minimum supported Clang version to 11 to support std::filesystem
- Use std::optional for empty values instead of string sentinels like "None"
- Remove the FixedString class
- Move the EvalFile network path from the network struct to the engine
- Simplify parts of the command-line handling
- Remove the old dual-net logic from export_net
- And ofc we now support non-ascii paths for windows
Behavior Change
- Because the EvalFile is no longer part of the network, it won't be included in the hashing, thus the network sharing is no longer dependent on the EvalFile path
- As a result, if the binary version and network are the same, changing only the EvalFile location will now reuse the same shared network
This also fixes a master bug where:
```
export_net None
setoption name EvalFile value None
export_net None
Failed to export a net
```
would fail on the second export. With this patch, the second export succeeds.
Quick Fishtest Check that nets load like normal.
https://tests.stockfishchess.org/tests/view/6a455593f97ff95f787954da
<img width="833" height="1137" alt="image" src="https://github.com/user-attachments/assets/fffee63b-c63a-4d5e-b13d-87d58bc28c88" />
closes https://github.com/official-stockfish/Stockfish/pull/6937
No functional change
Co-authored-by: anematode <timothy.herchen@gmail.com>
Performance on Spacemit K3, thanks @edolnx for testing
master:
Total time (ms) : 65200
Nodes searched : 3493826
Nodes/second : 53586
riscv-scalable-port:
Total time (ms) : 15834
Nodes searched : 3493826
Nodes/second : 220653
Also thanks to @camel-cdr for guidance on RVV programming, and https://cloud-v.co for supplying an RVV instance to test with
passed STC:
LLR: 2.81 (-2.94,2.94) <0.00,2.00>
Total: 1152 W: 527 L: 108 D: 517
Ptnml(0-2): 0, 17, 167, 348, 44
https://tests.stockfishchess.org/tests/view/6a39895b3036e45021aeb368
## Summary
We've had a `riscv64` target for a while, but haven't really optimized for it, in particular the vector extension (RVV).
RVV, like SVE, is based on a scalable vector system where the vector length ranges from 128 to 65536. In practice implementations are between 128 and 2048, and 256 bits is quite common (e.g. the Spacemit K3 system above). Unfortunately this doesn't fit well into the rest of our code which assumes a fixed vector length, so what I've done is bypass the `VECTOR` ifdef (which now basically means "FIXED_LENGTH_VECTOR") and just have RVV-specific paths.
The ability to explicitly control `vl` makes the code quite readable, in my opinion. We use LMUL>1 in most places to take advantage of multi-vector instructions. Generally the LMULs were chosen to best support a 256-bit vlen, which is very common, but by virtue of how the vlen control works, the code works with any vlen. In a couple places, i.e., `get_changed_pieces` and `AffineTransformSparseInput::propagate`, we have separate implementations depending on the vlen, because the optimal LMUL varies a lot between implementations.
One little wrinkle is that `load_as` is compiled to a sequence of byte loads, because although unaligned loads are legal in RVA23, the spec says that they *may* be extremely slow (even though they usually aren't, in actual hw), so compilers are conservative. Thus I aligned the relevant buffers and made the semantics of `load_as` that the operand is aligned, by adding a runtime assertion.
### Universal binary
Adding a universal binary is pretty easy and we can just cross-compile. There are two targets: baseline rv64gc and riscv64-rva23, which is actually a smaller subset of RVA23 that also works on some older processors that don't support the full thing. We use clang because GCC, until recently, has a nasty bug with LTO and RVV.
Like the universal ARM and x86 builds, we check all the builds in CI. In this case we run bench with multiple vlens, 128 through 1024.
In the meantime I deleted the existing broken and unused riscv64 tests.
### Follow-ups
- Optimizations
- zvdot4a8i path
closes https://github.com/official-stockfish/Stockfish/pull/6920
No functional change
Passed STC non-regression on avx512icl (https://tests.stockfishchess.org/tests/view/6a421b46f97ff95f78795061)
LLR: 2.96 (-2.94,2.94) <-1.75,0.25>
Total: 81632 W: 21240 L: 21075 D: 39317
Ptnml(0-2): 184, 8907, 22464, 9082, 179
Passed STC non-regression on bmi2 (https://tests.stockfishchess.org/tests/view/6a41f6f9f97ff95f7879503e)
LLR: 2.93 (-2.94,2.94) <-1.75,0.25>
Total: 78112 W: 20411 L: 20245 D: 37456
Ptnml(0-2): 188, 8449, 21632, 8583, 204
After #6845 , pext attacks are slower or equal to parallel-hyperbola attacks which works on all architectures that pext supports. SVE2 supports pext/pdep in vector registers, but it's much slower than the `rbit`-based HQ that we have on ARM, so I don't foresee the code being useful there.
Ofc I don't think we should remove the `bmi2` build or anything, because it's nice for people to be able to freely use pext when testing ideas, and I'm sure we'll some day find another use for it!
closes https://github.com/official-stockfish/Stockfish/pull/6939
No functional change
Co-authored-by: Dubslow <bunslow@gmail.com>
When compiling for Android (COMP=ndk) on macOS, the build fails because the
NDK's LLVM toolchain does not recognize the macOS-specific flags
-mdynamic-no-pic and -mmacosx-version-min.
This PR wraps these flags with ifneq ($(COMP),ndk) condition to skip them
during Android cross-compilation.
Please squash and merge this PR.
closes https://github.com/official-stockfish/Stockfish/pull/6927
No functional change
This network is trained by adding the following binpacks, relabeled by @vondele and totaling 74B positions, to the distillation fine-tuning stage:
```
- vondele/from_kaggle_1_relabel/leela96-filt-v2.min.split_0.relabel-BT4-tf13tune.binpack
- vondele/from_kaggle_1_relabel/leela96-filt-v2.min.split_1.relabel-BT4-tf13tune.binpack
- vondele/from_kaggle_1_relabel/leela96-filt-v2.min.split_2.relabel-BT4-tf13tune.binpack
- vondele/from_kaggle_1_relabel/leela96-filt-v2.min.split_3.relabel-BT4-tf13tune.binpack
- vondele/from_kaggle_1_relabel/leela96-filt-v2.min.split_4.relabel-BT4-tf13tune.binpack
- vondele/from_kaggle_2_relabel/T60T70wIsRightFarseerT60T74T75T76.split_0.relabel-BT4-tf13tune.binpack
- vondele/from_kaggle_2_relabel/T60T70wIsRightFarseerT60T74T75T76.split_1.relabel-BT4-tf13tune.binpack
- vondele/from_kaggle_2_relabel/T60T70wIsRightFarseerT60T74T75T76.split_2.relabel-BT4-tf13tune.binpack
- vondele/from_kaggle_2_relabel/T60T70wIsRightFarseerT60T74T75T76.split_3.relabel-BT4-tf13tune.binpack
- vondele/from_kaggle_2_relabel/T60T70wIsRightFarseerT60T74T75T76.split_4.relabel-BT4-tf13tune.binpack
```
The relabeling effort has been completed during the testing of this patch, and a full training run is on the way. Thanks to @vondele, @anematode, @Disservin, @yl25946, and all who've contributed to the process.
Passed STC:
LLR: 2.95 (-2.94,2.94) <0.00,2.00>
Total: 24512 W: 6500 L: 6201 D: 11811
Ptnml(0-2): 69, 2772, 6300, 3021, 94
https://tests.stockfishchess.org/tests/view/6a40b7083036e45021aebbd6
Passed LTC:
LLR: 2.94 (-2.94,2.94) <0.50,2.50>
Total: 15708 W: 4232 L: 3960 D: 7516
Ptnml(0-2): 8, 1572, 4420, 1848, 6
https://tests.stockfishchess.org/tests/view/6a413d293036e45021aebc84
nettest PR: https://github.com/vondele/nettest/pull/388
closes https://github.com/official-stockfish/Stockfish/pull/6932
Bench: 2102535
Co-authored-by: Joost VandeVondele <Joost.VandeVondele@gmail.com>
Deduplicating Color-Specific Piece Validation.
The validation checks for the number of pawns and additional promoted pieces are duplicated for WHITE and BLACK. We can combine this logic into a single range-based for loop over both colors.
closes https://github.com/official-stockfish/Stockfish/pull/6922
No functional change
do_null_move copies the whole StateInfo from the previous state, which leaves capturedPiece holding the piece captured by the last real move, so inside the null-move subtree captured_piece() reports a stale capture. The priorCapture consumers in search are all guarded by prevSq != SQ_NONE or (ss-1)->currentMove.is_ok(), which are false at the null-move child, but the stalemate verification gate in qsearch reads captured_piece() unguarded and can be spuriously triggered by the stale value.
Clear the field, since a null move captures nothing.
Passed non-regression STC:
LLR: 2.93 (-2.94,2.94) <-1.75,0.25>
Total: 82784 W: 21172 L: 21011 D: 40601
Ptnml(0-2): 194, 8976, 22923, 9073, 226
https://tests.stockfishchess.org/tests/view/6a2b5b356b4aa63ddbf31518
Passed non-regression LTC:
LLR: 2.94 (-2.94,2.94) <-1.75,0.25>
Total: 118134 W: 30007 L: 29891 D: 58236
Ptnml(0-2): 66, 11856, 35108, 11970, 67
https://tests.stockfishchess.org/tests/view/6a2c4c9c0d5d4b19d0805301
closes https://github.com/official-stockfish/Stockfish/pull/6910
No functional change
The following have zero call sites repo-wide:
SearchManager:🆔 never read or written (also never initialized, nor was it ever used).
Search::Worker::elapsed_time(): never called. PV output uses tm.elapsed_time() (TimeManager) directly. (removed callers on 25361e5)
MovePicker::begin()/end(): unused private accessors. (removed callers on 8c2d21f)
closes https://github.com/official-stockfish/Stockfish/pull/6909
No functional change
Worker::do_move computes the successor hash key via the new
Position::key_after(m) and prefetches the TT entry one full do_move
earlier than the existing prefetch in Position::do_move. key_after does
not model castling, en passant or promotion keys exactly; for rare
moves the prefetch lands on an unused line.
`key_after` has been around since 2014 (https://github.com/official-stockfish/Stockfish/commit/82d065b0) and was removed in (https://github.com/official-stockfish/Stockfish/pull/5770). Adding back `prefetch_key` helps in common, normal moves at the cost of extra compute.
Speedup (PGO vs PGO, interleaved paired bench, n=48 pairs, Apple M2
Pro / apple-silicon): +0.69% [0.47, 0.91]
Passed STC:
https://tests.stockfishchess.org/tests/view/6a291f8d7c758d82accea17f
LLR: 4.24 (-2.94,2.94) <0.00,2.00>
Total: 473504 W: 121250 L: 120228 D: 232026
Ptnml(0-2): 1112, 51137, 131251, 52121, 1131
No functional change
closes https://github.com/official-stockfish/Stockfish/pull/6911
No functional change
This PR fixes the remaining corner cases in the treatment of MultiPV
mated-in PVs, as well as an oversight in #6886. See the discussion in
In particular:
1. `previousScore` and `previousPV` can only be trusted, if that
rootmove was indeed fully searched in the previous iteration.
2. A move beyond `pvIdx` (that was hence not fully searched) may have an
exact loss score that cannot be trusted. So if a MultiPV search gets
aborted while searching `pvIdx`, we mark all the following loss
scores as bounds.
3. The forgotten mate logic also got broken in #6886, because the
`previousPV` of the forgotten mate's bestmove can only be trusted if
that move was fully searched in the previous iteration, something
that is not guaranteed. So we now store both `lastBestMoveScore` and
`lastBestMovePV`.
Here some scenarios for MultiPV = 8 that explain how master was broken:
1. Move A with an inexact mated-in-2 score from the previous iteration
(so outside the top8 moves) gets flushed into the top8 moves for the
current iteration, because the previous top8 move B is now scored as
a mated-in-1. Hence we cannot trust `previousScore` or `previousPV`
for move A, if the search gets aborted while it is being searched.
2. In the scenario above, move B has `Score != -VALUE_INFINITE` and a
mated-in-1 score, which cannot be trusted as it was not fully
searched.
3. Iteration N has bestmove A with mated-in-10, which gets recorded in
`lastBestMoveScore` (renamed from `lastIterationScore`). Iteration 11
forgets the mate and has bestmove B with a cp score, move A may have
an incomplete PV, and may even have a non-mate score. Iteration 12
gets aborted, and in trying to remember the forgotten mate, master
recovers the `previousScore` and `previousPV` of move A, which may be
neither mate nor complete.
Passed STC non-reg:
LLR: 2.94 (-2.94,2.94) <-1.75,0.25>
Total: 69728 W: 17748 L: 17573 D: 34407
Ptnml(0-2): 143, 7571, 19274, 7720, 156
https://tests.stockfishchess.org/tests/view/6a2c40c60d5d4b19d08052f2
closes https://github.com/official-stockfish/Stockfish/pull/6906
No functional change
After merging the HalfKA and Threats accumulators (7c7fe322) and the
subsequent removal of the double-incremental/fused update, a number of
NNUE helpers and fields became unreachable. Each was verified to have
zero callers/readers across the source tree:
- FusedUpdateData logic in FullThreats: the fused-update branch of
append_changed_indices and the FusedUpdateData parameter are unused;
the accumulator update no longer passes fused data.
- FullThreats::requires_refresh: never called. The live king-bucket
refresh check is HalfKAv2_hm::requires_refresh (PSQFeatureSet), used
in nnue_accumulator.
- HalfKAv2_hm::append_active_indices: never called. The live
active-index builder is FullThreats::append_active_indices
(ThreatFeatureSet).
- DirtyThreats::us, prevKsq and ksq: written in do_move but only read by
the now-removed FullThreats::requires_refresh. Removing them also
drops three stores from the do_move path.
- Unused feature Name constants and the unused FtOneVal / HiddenMaxVal
constants in nnue_common.h.
- Two stale feature-header banner comments.
closes https://github.com/official-stockfish/Stockfish/pull/6898
No functional change
This version handles aborting engine processes more gracefully. This
also test the engine prior to use, as the process is nevertheless not
fully robust.
closes https://github.com/official-stockfish/Stockfish/pull/6893
No functional change
The FEN validation check intended to reject pawns on the first or eighth rank uses the `Rank` enum values in a bitwise OR operation:
`if (pieces(PAWN) & (RANK_1 | RANK_8))`
`RANK_1 | RANK_8` evaluates to the integer `0 | 7 == 7` instead of a bitboard, so the expression only tests squares A1, B1 and C1. As a result, unsupported positions with pawns elsewhere on the first or eighth rank are silently accepted. For instance, `position fen 3P3k/8/8/8/8/8/8/3K4 w - - 0 1` is accepted even though the pawn on d8 makes the position unsupported.
Use the `Rank1BB | Rank8BB` bitboard constants so any pawn on the first or eighth rank are correctly rejected.
closes https://github.com/official-stockfish/Stockfish/pull/6887
No functional change
