Delay castling legality check

mcostalba · snicolet · commit 3c576efa77f4 · 2019-01-04T14:23:14.000+01:00
Delay legality check of castling moves at search time, just before making the move, as is the standard with all the other move types. This should avoid an useless and not trivial legality check when the castling is then not tried later. For instance due to a previous cut-off. The patch is also a big simplification and allows to entirely remove generate_castling() Bench changes due to a different move sequence out of MovePicker. STC: LLR: 2.95 (-2.94,2.94) [-3.00,1.00] Total: 45073 W: 9918 L: 9843 D: 25312 http://tests.stockfishchess.org/tests/view/5c2f176f0ebc596a450bdfb3 LTC: LLR: 3.15 (-2.94,2.94) [-3.00,1.00] Total: 10156 W: 1707 L: 1560 D: 6889 http://tests.stockfishchess.org/tests/view/5c2e7dfd0ebc596a450bcdf4 Verified with perft both in standard and Chess960 cases. Closes #1929 Bench: 3559104
diff --git a/src/movegen.cpp b/src/movegen.cpp
@@ -25,48 +25,6 @@
 
 namespace {
 
-  template<Color Us, CastlingSide Cs, bool Checks, bool Chess960>
-  ExtMove* generate_castling(const Position& pos, ExtMove* moveList) {
-
-    constexpr Color Them = (Us == WHITE ? BLACK : WHITE);
-    constexpr CastlingRight Cr = Us | Cs;
-    constexpr bool KingSide = (Cs == KING_SIDE);
-
-    if (pos.castling_impeded(Cr) || !pos.can_castle(Cr))
-        return moveList;
-
-    // After castling, the rook and king final positions are the same in Chess960
-    // as they would be in standard chess.
-    Square kfrom = pos.square<KING>(Us);
-    Square rfrom = pos.castling_rook_square(Cr);
-    Square kto = relative_square(Us, KingSide ? SQ_G1 : SQ_C1);
-    Bitboard enemies = pos.pieces(Them);
-
-    assert(!pos.checkers());
-
-    const Direction step = Chess960 ? kto > kfrom ? WEST : EAST
-                                    : KingSide    ? WEST : EAST;
-
-    for (Square s = kto; s != kfrom; s += step)
-        if (pos.attackers_to(s) & enemies)
-            return moveList;
-
-    // Because we generate only legal castling moves we need to verify that
-    // when moving the castling rook we do not discover some hidden checker.
-    // For instance an enemy queen in SQ_A1 when castling rook is in SQ_B1.
-    if (Chess960 && (attacks_bb<ROOK>(kto, pos.pieces() ^ rfrom) & pos.pieces(Them, ROOK, QUEEN)))
-        return moveList;
-
-    Move m = make<CASTLING>(kfrom, rfrom);
-
-    if (Checks && !pos.gives_check(m))
-        return moveList;
-
-    *moveList++ = m;
-    return moveList;
-  }
-
-
   template<GenType Type, Direction D>
   ExtMove* make_promotions(ExtMove* moveList, Square to, Square ksq) {
 
@@ -261,7 +219,9 @@ namespace {
   template<Color Us, GenType Type>
   ExtMove* generate_all(const Position& pos, ExtMove* moveList, Bitboard target) {
 
-    constexpr bool Checks = Type == QUIET_CHECKS;
+    constexpr CastlingRight OO  = Us | KING_SIDE;
+    constexpr CastlingRight OOO = Us | QUEEN_SIDE;
+    constexpr bool Checks = Type == QUIET_CHECKS; // Reduce template instantations
 
     moveList = generate_pawn_moves<Us, Type>(pos, moveList, target);
     moveList = generate_moves<KNIGHT, Checks>(pos, moveList, Us, target);
@@ -275,19 +235,14 @@ namespace {
         Bitboard b = pos.attacks_from<KING>(ksq) & target;
         while (b)
             *moveList++ = make_move(ksq, pop_lsb(&b));
-    }
 
-    if (Type != CAPTURES && Type != EVASIONS && pos.castling_rights(Us))
-    {
-        if (pos.is_chess960())
-        {
-            moveList = generate_castling<Us, KING_SIDE, Checks, true>(pos, moveList);
-            moveList = generate_castling<Us, QUEEN_SIDE, Checks, true>(pos, moveList);
-        }
-        else
+        if (Type != CAPTURES && pos.can_castle(CastlingRight(OO | OOO)))
         {
-            moveList = generate_castling<Us, KING_SIDE, Checks, false>(pos, moveList);
-            moveList = generate_castling<Us, QUEEN_SIDE, Checks, false>(pos, moveList);
+            if (!pos.castling_impeded(OO) && pos.can_castle(OO))
+                *moveList++ = make<CASTLING>(ksq, pos.castling_rook_square(OO));
+
+            if (!pos.castling_impeded(OOO) && pos.can_castle(OOO))
+                *moveList++ = make<CASTLING>(ksq, pos.castling_rook_square(OOO));
         }
     }
 
diff --git a/src/position.cpp b/src/position.cpp
@@ -540,6 +540,7 @@ bool Position::legal(Move m) const {
 
   Color us = sideToMove;
   Square from = from_sq(m);
+  Square to = to_sq(m);
 
   assert(color_of(moved_piece(m)) == us);
   assert(piece_on(square<KING>(us)) == make_piece(us, KING));
@@ -550,7 +551,6 @@ bool Position::legal(Move m) const {
   if (type_of(m) == ENPASSANT)
   {
       Square ksq = square<KING>(us);
-      Square to = to_sq(m);
       Square capsq = to - pawn_push(us);
       Bitboard occupied = (pieces() ^ from ^ capsq) | to;
 
@@ -563,16 +563,35 @@ bool Position::legal(Move m) const {
             && !(attacks_bb<BISHOP>(ksq, occupied) & pieces(~us, QUEEN, BISHOP));
   }
 
-  // If the moving piece is a king, check whether the destination
-  // square is attacked by the opponent. Castling moves are checked
-  // for legality during move generation.
+  // Castling moves generation does not check if the castling path is clear of
+  // enemy attacks, it is delayed at a later time: now!
+  if (type_of(m) == CASTLING)
+  {
+      // After castling, the rook and king final positions are the same in
+      // Chess960 as they would be in standard chess.
+      to = relative_square(us, to > from ? SQ_G1 : SQ_C1);
+      Direction step = to > from ? WEST : EAST;
+
+      for (Square s = to; s != from; s += step)
+          if (attackers_to(s) & pieces(~us))
+              return false;
+
+      // In case of Chess960, verify that when moving the castling rook we do
+      // not discover some hidden checker.
+      // For instance an enemy queen in SQ_A1 when castling rook is in SQ_B1.
+      return   !chess960
+            || !(attacks_bb<ROOK>(to, pieces() ^ to_sq(m)) & pieces(~us, ROOK, QUEEN));
+  }
+
+  // If the moving piece is a king, check whether the destination square is
+  // attacked by the opponent.
   if (type_of(piece_on(from)) == KING)
-      return type_of(m) == CASTLING || !(attackers_to(to_sq(m)) & pieces(~us));
+      return !(attackers_to(to) & pieces(~us));
 
   // A non-king move is legal if and only if it is not pinned or it
   // is moving along the ray towards or away from the king.
   return   !(blockers_for_king(us) & from)
-        ||  aligned(from, to_sq(m), square<KING>(us));
+        ||  aligned(from, to, square<KING>(us));
 }
 
 
diff --git a/src/position.h b/src/position.h
@@ -265,7 +265,7 @@ inline bool Position::can_castle(CastlingRight cr) const {
 }
 
 inline int Position::castling_rights(Color c) const {
-  return st->castlingRights & ((WHITE_OO | WHITE_OOO) << (2 * c));
+  return st->castlingRights & (c == WHITE ? WHITE_CASTLING : BLACK_CASTLING);
 }
 
 inline bool Position::castling_impeded(CastlingRight cr) const {
diff --git a/src/types.h b/src/types.h
@@ -141,7 +141,9 @@ enum CastlingRight {
   WHITE_OOO = WHITE_OO << 1,
   BLACK_OO  = WHITE_OO << 2,
   BLACK_OOO = WHITE_OO << 3,
-  ANY_CASTLING = WHITE_OO | WHITE_OOO | BLACK_OO | BLACK_OOO,
+  WHITE_CASTLING = WHITE_OO | WHITE_OOO,
+  BLACK_CASTLING = BLACK_OO | BLACK_OOO,
+  ANY_CASTLING = WHITE_CASTLING | BLACK_CASTLING,
   CASTLING_RIGHT_NB = 16
 };
 

Original file line number	Diff line number	Diff line change
`@@ -265,7 +265,7 @@ inline bool Position::can_castle(CastlingRight cr) const {`
`265`	`265`	`}`
`266`	`266`
`267`	`267`	`inline int Position::castling_rights(Color c) const {`
`268`		`- return st->castlingRights & ((WHITE_OO \| WHITE_OOO) << (2 * c));`
	`268`	`+ return st->castlingRights & (c == WHITE ? WHITE_CASTLING : BLACK_CASTLING);`
`269`	`269`	`}`
`270`	`270`
`271`	`271`	`inline bool Position::castling_impeded(CastlingRight cr) const {`