Check move legality at the beginning of the move loop (#764)

Losing elo for now, but hopefully will recover most of it with tuning and future optimizations/speedups of the code.

Elo   | -3.03 +- 2.13 (95%)
Conf  | 10.0+0.10s Threads=1 Hash=32MB
Games | N: 40012 W: 10796 L: 11145 D: 18071
Penta | [740, 5074, 8729, 4721, 742]
http://chess.grantnet.us/test/38661/

Elo   | -7.65 +- 1.84 (95%)
Conf  | 60.0+0.60s Threads=1 Hash=128MB
Games | N: 40000 W: 9583 L: 10464 D: 19953
Penta | [310, 5177, 9813, 4484, 216]
http://chess.grantnet.us/test/38657/

Bench: 24472040

src/makemove.c

@@ -194,7 +194,7 @@ void TakeMove(Position *pos) {
 }
 
 // Make a move - take it back and return false if move was illegal
-bool MakeMove(Position *pos, const Move move) {
+void MakeMove(Position *pos, const Move move) {
 
     TTPrefetch(KeyAfter(pos, move));
 
@@ -279,16 +279,10 @@ bool MakeMove(Position *pos, const Move move) {
     sideToMove ^= 1;
     HASH_SIDE;
 
-    // If own king is attacked after the move, take it back immediately
-    if (KingAttacked(pos, sideToMove^1))
-        return TakeMove(pos), false;
-
     pos->checkers = Checkers(pos);
     pos->nodes++;
 
     assert(PositionOk(pos));
-
-    return true;
 }
 
 // Pass the turn without moving

src/makemove.h

@@ -22,7 +22,7 @@
 #include "types.h"
 
 
-bool MakeMove(Position *pos, Move move);
+void MakeMove(Position *pos, Move move);
 void TakeMove(Position *pos);
 void MakeNullMove(Position *pos);
 void TakeNullMove(Position *pos);

src/move.c

@@ -71,6 +71,37 @@ bool MoveIsPseudoLegal(const Position *pos, const Move move) {
     return BB(to) & AttackBB(pieceTypeOn(from), from, pieceBB(ALL));
 }
 
+// Checks whether a move is legal
+// (assumes the move is pseudo-legal in the current position)
+bool MoveIsLegal(const Position *pos, const Move move) {
+
+    Color color = sideToMove;
+    Square from = fromSq(move);
+    Square to = toSq(move);
+
+    Square kingSq = PieceTypeOf(pieceOn(from)) == KING ? to : (Square)kingSq(color);
+
+    Bitboard occ = (pieceBB(ALL) ^ BB(from)) | BB(to);
+    Bitboard exclude = BB(to);
+
+    if (moveIsEnPas(move))
+        occ ^= BB(to ^ 8),
+        exclude ^= BB(to ^ 8);
+
+    Bitboard bishops = colorBB(!color) & (pieceBB(BISHOP) | pieceBB(QUEEN));
+    Bitboard rooks   = colorBB(!color) & (pieceBB(ROOK)   | pieceBB(QUEEN));
+
+    Bitboard kingAttackers =
+        (  (PawnAttackBB(color, kingSq)   & colorPieceBB(!color, PAWN))
+         | (AttackBB(KING,   kingSq, occ) & colorPieceBB(!color, KING))
+         | (AttackBB(KNIGHT, kingSq, occ) & colorPieceBB(!color, KNIGHT))
+         | (AttackBB(BISHOP, kingSq, occ) & bishops)
+         | (AttackBB(ROOK,   kingSq, occ) & rooks));
+
+    // If any non-excluded piece is attacking the king, the move is illegal
+    return !(kingAttackers & ~exclude);
+}
+
 // Translates a move to a string
 char *MoveToStr(const Move move) {
 

src/move.h

@@ -92,6 +92,7 @@ INLINE bool CastleLegal(const Position *pos, Square to) {
 }
 
 bool MoveIsPseudoLegal(const Position *pos, Move move);
+bool MoveIsLegal(const Position *pos, const Move move);
 char *MoveToStr(Move move);
 Move ParseMove(const char *ptrChar, const Position *pos);
 bool NotInSearchMoves(Move searchmoves[], Move move);

src/movegen.c

@@ -174,21 +174,19 @@ void GenAllMoves(const Position *pos, MoveList *list) {
     GenQuietMoves(pos, list);
 }
 
-void GenLegalMoves(Position *pos, MoveList *list) {
+void GenLegalMoves(const Position *pos, MoveList *list) {
     MoveList allMoves;
     allMoves.count = allMoves.next = 0;
     GenAllMoves(pos, &allMoves);
 
     for (int i = 0; i < allMoves.count; ++i) {
         Move move = allMoves.moves[i].move;
-        if (!MakeMove(pos, move)) continue;
+        if (!MoveIsLegal(pos, move)) continue;
         list->moves[list->count++].move = move;
-        TakeMove(pos);
     }
-    pos->nodes = 0;
 }
 
-int LegalMoveCount(Position *pos) {
+int LegalMoveCount(const Position *pos) {
     MoveList list;
     list.count = list.next = 0;
     GenLegalMoves(pos, &list);

src/movegen.h

@@ -37,5 +37,5 @@ typedef struct {
 void GenNoisyMoves(const Position *pos, MoveList *list);
 void GenQuietMoves(const Position *pos, MoveList *list);
 void GenAllMoves(const Position *pos, MoveList *list);
-void GenLegalMoves(Position *pos, MoveList *list);
-int LegalMoveCount(Position *pos);
+void GenLegalMoves(const Position *pos, MoveList *list);
+int LegalMoveCount(const Position *pos);

src/search.c

@@ -175,6 +175,7 @@ static int Quiescence(Thread *thread, Stack *ss, int alpha, int beta) {
     Move bestMove = NOMOVE;
     Move move;
     while ((move = NextMove(&mp))) {
+        if (!MoveIsLegal(pos, move)) continue;
 
         // Avoid pruning until at least one move avoids a terminal loss score
         if (isLoss(bestScore)) goto search;
@@ -201,8 +202,7 @@ static int Quiescence(Thread *thread, Stack *ss, int alpha, int beta) {
         ss->continuation = &thread->continuation[inCheck][moveIsCapture(move)][piece(move)][toSq(move)];
         ss->contCorr = &thread->contCorrHistory[piece(move)][toSq(move)];
 
-        // Recursively search the positions after making the moves, skipping illegal ones
-        if (!MakeMove(pos, move)) continue;
+        MakeMove(pos, move);
         int score = -Quiescence(thread, ss+1, -beta, -alpha);
         TakeMove(pos);
 
@@ -407,7 +407,8 @@ static int AlphaBeta(Thread *thread, Stack *ss, int alpha, int beta, Depth depth
 
             if (mp.stage > NOISY_GOOD) break;
 
-            if (!MakeMove(pos, move)) continue;
+            if (!MoveIsLegal(pos, move)) continue;
+            MakeMove(pos, move);
 
             ss->move = move;
             ss->continuation = &thread->continuation[inCheck][moveIsCapture(move)][piece(move)][toSq(move)];
@@ -447,6 +448,7 @@ static int AlphaBeta(Thread *thread, Stack *ss, int alpha, int beta, Depth depth
         if (move == ss->excluded) continue;
         if (root && AlreadySearchedMultiPV(thread, move)) continue;
         if (root && NotInSearchMoves(Limits.searchmoves, move)) continue;
+        if (!MoveIsLegal(pos, move)) continue;
 
         bool quiet = moveIsQuiet(move);
 
@@ -475,9 +477,6 @@ static int AlphaBeta(Thread *thread, Stack *ss, int alpha, int beta, Depth depth
                 continue;
         }
 
-        // Make the move, skipping to the next if illegal
-        if (!MakeMove(pos, move)) continue;
-
         moveCount++;
 
         // Extension
@@ -495,9 +494,6 @@ static int AlphaBeta(Thread *thread, Stack *ss, int alpha, int beta, Depth depth
             && ttBound != BOUND_UPPER
             && !isTerminal(ttScore)) {
 
-            // ttMove has been made to check legality
-            TakeMove(pos);
-
             // Search to reduced depth with a zero window a bit lower than ttScore
             int singularBeta = ttScore - depth * (2 - pvNode);
             ss->excluded = move;
@@ -515,9 +511,6 @@ static int AlphaBeta(Thread *thread, Stack *ss, int alpha, int beta, Depth depth
             // Negative extension - not singular but likely still good enough to beat beta
             else if (ttScore >= beta)
                 extension = -1;
-
-            // Replay ttMove
-            MakeMove(pos, move);
         }
 
         // Extend when in check
@@ -526,6 +519,8 @@ static int AlphaBeta(Thread *thread, Stack *ss, int alpha, int beta, Depth depth
 
 skip_extensions:
 
+        MakeMove(pos, move);
+
         ss->move = move;
         ss->doubleExtensions = (ss-1)->doubleExtensions + (extension == 2);
         ss->continuation = &thread->continuation[inCheck][moveIsCapture(move)][piece(move)][toSq(move)];

src/tests.c

@@ -161,7 +161,8 @@ static uint64_t RecursivePerft(Position *pos, const Depth depth) {
     GenAllMoves(pos, &list);
 
     for (int i = 0; i < list.count; i++) {
-        if (!MakeMove(pos, list.moves[i].move)) continue;
+        if (!MoveIsLegal(pos, list.moves[i].move)) continue;
+        MakeMove(pos, list.moves[i].move);
         leafnodes += RecursivePerft(pos, depth - 1);
         TakeMove(pos);
     }