-
Notifications
You must be signed in to change notification settings - Fork 2.7k
fix missing condition #6463
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix missing condition #6463
Conversation
|
Result of Result of Edit: Confirmed that there is no mismatch between dts generations of x86-64-avx512icl and x86-64-vnni512. Test codediff --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 |
|
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; |
|
https://tests.stockfishchess.org/tests/view/692fb2d8b23dfeae38d01c81 Test passed. I'd expect it's a small gain, up to 0.8 Elo on STC. I assume LTC is not needed but maintainers would have to decide that |
The modifications to the DirtyThreats bitboards should only happen if
PutPieceis 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 theupdate_piece_threatsPR as it'll cause intermittent wrong benches against master...