Improve commentary about DEPTH constants

Turns out they're only related to ttdepth, so some reorganization of related comment chunks is done as well.
Rename the constants too.

(Also "fix" movepicker to allow depths between CHECKS and NORMAL, which makes them easier to tweak, not that they get tweaked hardly ever.
This was more beneficial when there was a third stage to DEPTH_QS, but it's still an improvement now)

no functional change

src/movepick.cpp

@@ -361,8 +361,8 @@ Move MovePicker::next_move(bool skipQuiets) {
         if (select<Next>([]() { return true; }))
             return *(cur - 1);
 
-        // If we did not find any move and we do not try checks, we have finished
-        if (depth != DEPTH_QS_CHECKS)
+        // If we found no move and the depth is too low to try checks, then we have finished
+        if (depth <= DEPTH_QS_NORMAL)
             return Move::none();
 
         ++stage;

src/search.cpp

@@ -732,7 +732,7 @@ Value Search::Worker::search(
         ss->staticEval = eval = to_corrected_static_eval(unadjustedStaticEval, *thisThread, pos);
 
         // Static evaluation is saved as it was before adjustment by correction history
-        tte->save(posKey, VALUE_NONE, ss->ttPv, BOUND_NONE, DEPTH_NONE, Move::none(),
+        tte->save(posKey, VALUE_NONE, ss->ttPv, BOUND_NONE, DEPTH_UNSEARCHED, Move::none(),
                   unadjustedStaticEval, tt.generation());
     }
 
@@ -1375,8 +1375,11 @@ Value Search::Worker::search(
 }
 
 
-// Quiescence search function, which is called by the main search
-// function with zero depth, or recursively with further decreasing depth per call.
+// Quiescence search function, which is called by the main search function with zero depth, or
+// recursively with further decreasing depth per call. With depth <= 0, we "should" be using
+// static eval only, but tactical moves may confuse the static eval. To fight this horizon effect,
+// we implement this qsearch of tactical moves only.
+// See https://www.chessprogramming.org/Horizon_Effect and https://www.chessprogramming.org/Quiescence_Search
 // (~155 Elo)
 template<NodeType nodeType>
 Value Search::Worker::qsearch(Position& pos, Stack* ss, Value alpha, Value beta, Depth depth) {
@@ -1434,8 +1437,11 @@ Value Search::Worker::qsearch(Position& pos, Stack* ss, Value alpha, Value beta,
 
     assert(0 <= ss->ply && ss->ply < MAX_PLY);
 
-    // Decide the replacement and cutoff priority of the qsearch TT entries
-    ttDepth = ss->inCheck || depth >= DEPTH_QS_CHECKS ? DEPTH_QS_CHECKS : DEPTH_QS_NO_CHECKS;
+    // Note that unlike regular search, which stores literal depth, in QS we only store the
+    // current movegen stage, see comments in types.h. If in check, we search all
+    // evasions and thus store DEPTH_QS_CHECKS. (Evasions may be quiet, and _CHECKS include quiets.)
+    ttDepth = ss->inCheck || depth >= DEPTH_QS_CHECKS ? DEPTH_QS_CHECKS
+                                                      : DEPTH_QS_NORMAL;
 
     // Step 3. Transposition table lookup
     posKey  = pos.key();
@@ -1487,7 +1493,7 @@ Value Search::Worker::qsearch(Position& pos, Stack* ss, Value alpha, Value beta,
             if (std::abs(bestValue) < VALUE_TB_WIN_IN_MAX_PLY && !PvNode)
                 bestValue = (3 * bestValue + beta) / 4;
             if (!ss->ttHit)
-                tte->save(posKey, value_to_tt(bestValue, ss->ply), false, BOUND_LOWER, DEPTH_NONE,
+                tte->save(posKey, value_to_tt(bestValue, ss->ply), false, BOUND_LOWER, DEPTH_UNSEARCHED,
                           Move::none(), unadjustedStaticEval, tt.generation());
 
             return bestValue;
@@ -1502,16 +1508,16 @@ Value Search::Worker::qsearch(Position& pos, Stack* ss, Value alpha, Value beta,
     const PieceToHistory* contHist[] = {(ss - 1)->continuationHistory,
                                         (ss - 2)->continuationHistory};
 
-    // Initialize a MovePicker object for the current position, and prepare
-    // to search the moves. Because the depth is <= 0 here, only captures,
-    // queen promotions, and other checks (only if depth >= DEPTH_QS_CHECKS)
-    // will be generated.
+    // Initialize a MovePicker object for the current position, and prepare to search the moves.
+    // We presently use two stages of qs movegen, first captures+checks, then captures only.
+    // (When in check, we simply search all evasions.) See also comments in types.h.
+    // (Presently, having the checks stage is worth only 1 Elo, and may be removable in the near future,
+    // which would result in only a single stage of QS movegen.)
     Square     prevSq = ((ss - 1)->currentMove).is_ok() ? ((ss - 1)->currentMove).to_sq() : SQ_NONE;
     MovePicker mp(pos, ttMove, depth, &thisThread->mainHistory, &thisThread->captureHistory,
                   contHist, &thisThread->pawnHistory);
 
-    // Step 5. Loop through all pseudo-legal moves until no moves remain
-    // or a beta cutoff occurs.
+    // Step 5. Loop through all pseudo-legal moves until no moves remain or a beta cutoff occurs.
     while ((move = mp.next_move()) != Move::none())
     {
         assert(move.is_ok());

src/tt.cpp

@@ -30,6 +30,9 @@
 
 namespace Stockfish {
 
+// Per commit f7b3f0e, we use `bool(depth8)` to test if an entry is occupied. However we also need to
+// store negative depths for QS, so we add/subtract DEPTH_ENTRY_OFFSET, which only exists for this purpose.
+
 // Populates the TTEntry with a new node's data, possibly
 // overwriting an old position. The update is not atomic and can be racy.
 void TTEntry::save(
@@ -40,14 +43,14 @@ void TTEntry::save(
         move16 = m;
 
     // Overwrite less valuable entries (cheapest checks first)
-    if (b == BOUND_EXACT || uint16_t(k) != key16 || d - DEPTH_OFFSET + 2 * pv > depth8 - 4
+    if (b == BOUND_EXACT || uint16_t(k) != key16 || d - DEPTH_ENTRY_OFFSET + 2 * pv > depth8 - 4
         || relative_age(generation8))
     {
-        assert(d > DEPTH_OFFSET);
-        assert(d < 256 + DEPTH_OFFSET);
+        assert(d > DEPTH_ENTRY_OFFSET);
+        assert(d < 256 + DEPTH_ENTRY_OFFSET);
 
         key16     = uint16_t(k);
-        depth8    = uint8_t(d - DEPTH_OFFSET);
+        depth8    = uint8_t(d - DEPTH_ENTRY_OFFSET);
         genBound8 = uint8_t(generation8 | uint8_t(pv) << 2 | b);
         value16   = int16_t(v);
         eval16    = int16_t(ev);

src/tt.h

@@ -42,7 +42,7 @@ struct TTEntry {
     Move  move() const { return Move(move16); }
     Value value() const { return Value(value16); }
     Value eval() const { return Value(eval16); }
-    Depth depth() const { return Depth(depth8 + DEPTH_OFFSET); }
+    Depth depth() const { return Depth(depth8 + DEPTH_ENTRY_OFFSET); }
     bool  is_pv() const { return bool(genBound8 & 0x4); }
     Bound bound() const { return Bound(genBound8 & 0x3); }
     void  save(Key k, Value v, bool pv, Bound b, Depth d, Move m, Value ev, uint8_t generation8);

src/types.h

@@ -187,12 +187,17 @@ constexpr Value PieceValue[PIECE_NB] = {
 using Depth = int;
 
 enum : int {
-    DEPTH_QS_CHECKS    = 0,
-    DEPTH_QS_NO_CHECKS = -1,
-
-    DEPTH_NONE = -6,
-
-    DEPTH_OFFSET = -7  // value used only for TT entry occupancy check
+  // The following DEPTH_ constants are used for TT entries from search. In regular search,
+  // TT depth is literal: the search depth (effort) used to make the corresponding TT value.
+  // In qsearch, however, we only store which movegen stage of QS we were in when saving the
+  // ttentry (which should thus compare lower than any regular search depth).
+  DEPTH_QS_CHECKS =  0,
+  DEPTH_QS_NORMAL = -1,
+  // For TT entries where no searching at all was done (whether regular or qsearch) we use
+  // _UNSEARCHED, which should thus compare lower than any QS or regular depth. _ENTRY_OFFSET is used
+  // only for the TT entry occupancy check (see tt.cpp), and should thus be lower than _UNSEARCHED.
+  DEPTH_UNSEARCHED   = -6,
+  DEPTH_ENTRY_OFFSET = -7
 };
 
 // clang-format off