Skip to content

Conversation

@anematode
Copy link
Contributor

@anematode anematode commented Dec 2, 2025

The modifications to the DirtyThreats bitboards should only happen if PutPiece is true. This somehow didn't affect bench at the default parameters until Mineta's recent test which failed on ICL: link. Sorry about this; I should have tested more extensively. Probably worth either merging this soon or reverting the update_piece_threats PR as it'll cause intermittent wrong benches against master...

Reference AVX2:
./stockfish.killdeer-fix.avx2 bench 64 1 23
Nodes searched  : 178140156
Nodes/second    : 1503152

before patch:
./stockfish.master.avx512icl  bench 64 1 23
Nodes searched  : 218349728
Nodes/second    : 1743450

after patch:
./stockfish.killdeer-fix.avx512icl bench 64 1 23
Nodes searched  : 178140156
Nodes/second    : 1727520

@anematode anematode marked this pull request as draft December 3, 2025 00:11
@anematode anematode marked this pull request as ready for review December 3, 2025 01:40
@MinetaS
Copy link
Contributor

MinetaS commented Dec 3, 2025

Result of bench 1024 1 30 default depth:

master, x86-64-vnni512: 5763584564
patch, x86-64-avx512icl: 5763584564

Result of speedtest 16 (AMD Ryzen 9 7950X):

master, x86-64-avx512icl: 28638442
patch, x86-64-avx512icl: 28743670

Edit: Confirmed that there is no mismatch between dts generations of x86-64-avx512icl and x86-64-vnni512.

Test code
diff --git a/src/position.cpp b/src/position.cpp
index b08cabf3..877f45af 100644
--- a/src/position.cpp
+++ b/src/position.cpp
@@ -29,6 +29,7 @@
 #include <iostream>
 #include <sstream>
 #include <string_view>
+#include <unordered_set>
 #include <utility>
 
 #include "bitboard.h"
@@ -1092,11 +1093,105 @@ void write_multiple_dirties(const Position& p,
 }
 #endif
 
+template<bool PutPiece, bool ComputeRay>
+static void update_piece_threats_ok(const Position& pos, Piece pc, Square s, DirtyThreats* const dts, Bitboard noRaysContaining) {
+    const Bitboard occupied     = pos.pieces();
+    const Bitboard rookQueens   = pos.pieces(ROOK, QUEEN);
+    const Bitboard bishopQueens = pos.pieces(BISHOP, QUEEN);
+    const Bitboard knights      = pos.pieces(KNIGHT);
+    const Bitboard kings        = pos.pieces(KING);
+    const Bitboard whitePawns   = pos.pieces(WHITE, PAWN);
+    const Bitboard blackPawns   = pos.pieces(BLACK, PAWN);
+
+    const Bitboard rAttacks = attacks_bb<ROOK>(s, occupied);
+    const Bitboard bAttacks = attacks_bb<BISHOP>(s, occupied);
+
+    Bitboard qAttacks = Bitboard(0);
+    if constexpr (ComputeRay)
+        qAttacks = rAttacks | bAttacks;
+    else if (type_of(pc) == QUEEN)
+        qAttacks = rAttacks | bAttacks;
+
+    Bitboard threatened;
+
+    switch (type_of(pc))
+    {
+    case PAWN :
+        threatened = PseudoAttacks[color_of(pc)][s];
+        break;
+    case BISHOP :
+        threatened = bAttacks;
+        break;
+    case ROOK :
+        threatened = rAttacks;
+        break;
+    case QUEEN :
+        threatened = qAttacks;
+        break;
+
+    default :
+        threatened = PseudoAttacks[type_of(pc)][s];
+    }
+
+    threatened &= occupied;
+    Bitboard sliders = (rookQueens & rAttacks) | (bishopQueens & bAttacks);
+    Bitboard incoming_threats =
+      (PseudoAttacks[KNIGHT][s] & knights) | (attacks_bb<PAWN>(s, WHITE) & blackPawns)
+      | (attacks_bb<PAWN>(s, BLACK) & whitePawns) | (PseudoAttacks[KING][s] & kings);
+
+    while (threatened)
+    {
+        Square threatenedSq = pop_lsb(threatened);
+        Piece  threatenedPc = pos.piece_on(threatenedSq);
+
+        assert(threatenedSq != s);
+        assert(threatenedPc);
+
+        add_dirty_threat<PutPiece>(dts, pc, threatenedPc, s, threatenedSq);
+    }
+
+    if constexpr (ComputeRay)
+    {
+        while (sliders)
+        {
+            Square sliderSq = pop_lsb(sliders);
+            Piece  slider   = pos.piece_on(sliderSq);
+
+            const Bitboard ray        = RayPassBB[sliderSq][s] & ~BetweenBB[sliderSq][s];
+            const Bitboard discovered = ray & qAttacks & occupied;
+
+            if (discovered && (RayPassBB[sliderSq][s] & noRaysContaining) != noRaysContaining)
+            {
+                const Square threatenedSq = lsb(discovered);
+                const Piece  threatenedPc = pos.piece_on(threatenedSq);
+                add_dirty_threat<!PutPiece>(dts, slider, threatenedPc, sliderSq, threatenedSq);
+            }
+
+            add_dirty_threat<PutPiece>(dts, slider, pc, sliderSq, s);
+        }
+    }
+    else
+    {
+        incoming_threats |= sliders;
+    }
+
+    while (incoming_threats)
+    {
+        Square srcSq = pop_lsb(incoming_threats);
+        Piece  srcPc = pos.piece_on(srcSq);
+        add_dirty_threat<PutPiece>(dts, srcPc, pc, srcSq, s);
+    }
+}
+
 template<bool PutPiece, bool ComputeRay>
 void Position::update_piece_threats(Piece               pc,
                                     Square              s,
                                     DirtyThreats* const dts,
                                     Bitboard            noRaysContaining) {
+    DirtyThreats dts_copy;
+    std::memcpy(&dts_copy, dts, sizeof(DirtyThreats));
+    update_piece_threats_ok<PutPiece, ComputeRay>(*this, pc, s, &dts_copy, noRaysContaining);
+
     const Bitboard occupied     = pieces();
     const Bitboard rookQueens   = pieces(ROOK, QUEEN);
     const Bitboard bishopQueens = pieces(BISHOP, QUEEN);
@@ -1221,6 +1316,34 @@ void Position::update_piece_threats(Piece               pc,
         add_dirty_threat<PutPiece>(dts, srcPc, pc, srcSq, s);
     }
 #endif
+
+    const std::array<bool, 3> wrong = {
+        dts->threatenedSqs != dts_copy.threatenedSqs,
+        dts->threateningSqs != dts_copy.threateningSqs,
+        [&]() {
+            if (dts->list.size() != dts_copy.list.size())
+                return true;
+
+            std::unordered_set<std::uint32_t> dts_exp, dts_get;
+
+            for (const DirtyThreat& dt : dts->list)
+                dts_get.insert(dt.raw());
+            for (const DirtyThreat& dt : dts_copy.list)
+                dts_exp.insert(dt.raw());
+
+            return dts_get != dts_exp;
+        }()
+    };
+
+    if (std::any_of(wrong.begin(), wrong.end(), [](bool b) { return b; })) {
+        std::cout << "Mismatch " << fen() << " -";
+        if (wrong[0])
+            std::cout << " threatenedSqs";
+        if (wrong[1])
+            std::cout << " threateningSqs";
+        if (wrong[2])
+            std::cout << " list";
+    }
 }
 
 // Helper used to do/undo a castling move. This is a bit

@MinetaS
Copy link
Contributor

MinetaS commented Dec 3, 2025

A nitpick but as I suggested in Discord, imo it's better to qualify this function as const:

    template<bool PutPiece, bool ComputeRay = true>
    void update_piece_threats(Piece               pc,
                              Square              s,
                              DirtyThreats* const dts,
                              Bitboard            noRaysContaining = -1ULL) const;

@anematode
Copy link
Contributor Author

@MinetaS alright if I make it part of #6460 ?

@MinetaS
Copy link
Contributor

MinetaS commented Dec 3, 2025

@MinetaS alright if I make it part of #6460 ?

Sure, that's the best.

@MinetaS
Copy link
Contributor

MinetaS commented Dec 3, 2025

https://tests.stockfishchess.org/tests/view/692fb2d8b23dfeae38d01c81

Test passed. I'd expect it's a small gain, up to 0.8 Elo on STC.
You can add this to the commit message.

I assume LTC is not needed but maintainers would have to decide that

@vondele vondele added bug to be merged Will be merged shortly labels Dec 3, 2025
@vondele vondele closed this in c109a88 Dec 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug to be merged Will be merged shortly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants