Skip to content

Commit

Permalink
Simplify signature of remove_piece()
Browse files Browse the repository at this point in the history
This is a non-functional simplification. Instead of passing the piece type
for remove_piece, we can rely on the board. The only exception is en-passant
which must be explicitly set because the destination square for the capture
is not the same as the piece to remove.

Verified also in the Chess960 castling case by running a couple of perft, see
the pull request discussion: #2460

STC
LLR: 2.94 (-2.94,2.94) [-3.00,1.00]
Total: 18624 W: 4147 L: 4070 D: 10407
Ptnml(0-2): 223, 1933, 4945, 1938, 260
http://tests.stockfishchess.org/tests/view/5dfeaa93e70446e17e451163

No functional change
  • Loading branch information
protonspring authored and snicolet committed Jan 23, 2020
1 parent bcf9282 commit 7a7bcd6
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 14 deletions.
21 changes: 11 additions & 10 deletions src/position.cpp
Expand Up @@ -743,8 +743,6 @@ void Position::do_move(Move m, StateInfo& newSt, bool givesCheck) {
assert(relative_rank(us, to) == RANK_6);
assert(piece_on(to) == NO_PIECE);
assert(piece_on(capsq) == make_piece(them, PAWN));

board[capsq] = NO_PIECE; // Not done by remove_piece()
}

st->pawnKey ^= Zobrist::psq[captured][capsq];
Expand All @@ -753,7 +751,10 @@ void Position::do_move(Move m, StateInfo& newSt, bool givesCheck) {
st->nonPawnMaterial[them] -= PieceValue[MG][captured];

// Update board and piece lists
remove_piece(captured, capsq);
remove_piece(capsq);

if (type_of(m) == ENPASSANT)
board[capsq] = NO_PIECE;

// Update material hash key and prefetch access to materialTable
k ^= Zobrist::psq[captured][capsq];
Expand Down Expand Up @@ -784,7 +785,7 @@ void Position::do_move(Move m, StateInfo& newSt, bool givesCheck) {

// Move the piece. The tricky Chess960 castling is handled earlier
if (type_of(m) != CASTLING)
move_piece(pc, from, to);
move_piece(from, to);

// If the moving piece is a pawn do some special extra work
if (type_of(pc) == PAWN)
Expand All @@ -804,7 +805,7 @@ void Position::do_move(Move m, StateInfo& newSt, bool givesCheck) {
assert(relative_rank(us, to) == RANK_8);
assert(type_of(promotion) >= KNIGHT && type_of(promotion) <= QUEEN);

remove_piece(pc, to);
remove_piece(to);
put_piece(promotion, to);

// Update hash keys
Expand Down Expand Up @@ -884,7 +885,7 @@ void Position::undo_move(Move m) {
assert(type_of(pc) == promotion_type(m));
assert(type_of(pc) >= KNIGHT && type_of(pc) <= QUEEN);

remove_piece(pc, to);
remove_piece(to);
pc = make_piece(us, PAWN);
put_piece(pc, to);
}
Expand All @@ -896,7 +897,7 @@ void Position::undo_move(Move m) {
}
else
{
move_piece(pc, to, from); // Put the piece back at the source square
move_piece(to, from); // Put the piece back at the source square

if (st->capturedPiece)
{
Expand Down Expand Up @@ -936,9 +937,9 @@ void Position::do_castling(Color us, Square from, Square& to, Square& rfrom, Squ
to = relative_square(us, kingSide ? SQ_G1 : SQ_C1);

// Remove both pieces first since squares could overlap in Chess960
remove_piece(make_piece(us, KING), Do ? from : to);
remove_piece(make_piece(us, ROOK), Do ? rfrom : rto);
board[Do ? from : to] = board[Do ? rfrom : rto] = NO_PIECE; // Since remove_piece doesn't do it for us
remove_piece(Do ? from : to);
remove_piece(Do ? rfrom : rto);
board[Do ? from : to] = board[Do ? rfrom : rto] = NO_PIECE; // Since remove_piece doesn't do this for us
put_piece(make_piece(us, KING), Do ? to : from);
put_piece(make_piece(us, ROOK), Do ? rto : rfrom);
}
Expand Down
10 changes: 6 additions & 4 deletions src/position.h
Expand Up @@ -174,8 +174,8 @@ class Position {

// Other helpers
void put_piece(Piece pc, Square s);
void remove_piece(Piece pc, Square s);
void move_piece(Piece pc, Square from, Square to);
void remove_piece(Square s);
void move_piece(Square from, Square to);
template<bool Do>
void do_castling(Color us, Square from, Square& to, Square& rfrom, Square& rto);

Expand Down Expand Up @@ -412,12 +412,13 @@ inline void Position::put_piece(Piece pc, Square s) {
psq += PSQT::psq[pc][s];
}

inline void Position::remove_piece(Piece pc, Square s) {
inline void Position::remove_piece(Square s) {

// WARNING: This is not a reversible operation. If we remove a piece in
// do_move() and then replace it in undo_move() we will put it at the end of
// the list and not in its original place, it means index[] and pieceList[]
// are not invariant to a do_move() + undo_move() sequence.
Piece pc = board[s];
byTypeBB[ALL_PIECES] ^= s;
byTypeBB[type_of(pc)] ^= s;
byColorBB[color_of(pc)] ^= s;
Expand All @@ -430,10 +431,11 @@ inline void Position::remove_piece(Piece pc, Square s) {
psq -= PSQT::psq[pc][s];
}

inline void Position::move_piece(Piece pc, Square from, Square to) {
inline void Position::move_piece(Square from, Square to) {

// index[from] is not updated and becomes stale. This works as long as index[]
// is accessed just by known occupied squares.
Piece pc = board[from];
Bitboard fromTo = from | to;
byTypeBB[ALL_PIECES] ^= fromTo;
byTypeBB[type_of(pc)] ^= fromTo;
Expand Down

0 comments on commit 7a7bcd6

Please sign in to comment.