From 9f968446cbbe7630b3044526359baef4f67fa62d Mon Sep 17 00:00:00 2001 From: disservin Date: Wed, 28 Jan 2026 09:41:21 +0100 Subject: [PATCH] Improve usage of sf_assume Make the sf_assume also have an effect with clang and help the compiler proof it's side effect free by comparing against a variable. https://tests.stockfishchess.org/tests/view/6979d14fa2c75f923be1e681 LLR: 2.93 (-2.94,2.94) <0.00,2.00> Total: 63488 W: 16530 L: 16188 D: 30770 Ptnml(0-2): 167, 6904, 17277, 7212, 184 closes https://github.com/official-stockfish/Stockfish/pull/6573 No functional change --- src/misc.h | 10 ++++++- src/nnue/nnue_accumulator.cpp | 50 +++++++++++++++++++++-------------- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/src/misc.h b/src/misc.h index 8adc72409..1471b1b6a 100644 --- a/src/misc.h +++ b/src/misc.h @@ -444,7 +444,9 @@ void move_to_front(std::vector& vec, Predicate pred) { #define sf_always_inline #endif -#if defined(__GNUC__) && !defined(__clang__) +#if defined(__clang__) + #define sf_assume(cond) __builtin_assume(cond) +#elif defined(__GNUC__) #if __GNUC__ >= 13 #define sf_assume(cond) __attribute__((assume(cond))) #else @@ -460,6 +462,12 @@ void move_to_front(std::vector& vec, Predicate pred) { #define sf_assume(cond) #endif +#ifdef __GNUC__ + #define sf_unreachable() __builtin_unreachable() +#else + #define sf_unreachable() +#endif + } // namespace Stockfish template diff --git a/src/nnue/nnue_accumulator.cpp b/src/nnue/nnue_accumulator.cpp index 16af8d5f6..926e82d93 100644 --- a/src/nnue/nnue_accumulator.cpp +++ b/src/nnue/nnue_accumulator.cpp @@ -507,19 +507,23 @@ void double_inc_update(Color p assert(added.size() < 2); PSQFeatureSet::append_changed_indices(perspective, ksq, target_state.diff, removed, added); - assert(added.size() == 1); - assert(removed.size() == 2 || removed.size() == 3); + [[maybe_unused]] const int addedSize = added.ssize(); + [[maybe_unused]] const int removedSize = removed.ssize(); + + assert(addedSize == 1); + assert(removedSize == 2 || removedSize == 3); // Workaround compiler warning for uninitialized variables, replicated on // profile builds on windows with gcc 14.2.0. - // TODO remove once unneeded - sf_assume(added.size() == 1); - sf_assume(removed.size() == 2 || removed.size() == 3); + // Also helps with optimizations on some compilers. + + sf_assume(addedSize == 1); + sf_assume(removedSize == 2 || removedSize == 3); auto updateContext = make_accumulator_update_context(perspective, featureTransformer, computed, target_state); - if (removed.size() == 2) + if (removedSize == 2) { updateContext.template apply(added[0], removed[0], removed[1]); } @@ -593,35 +597,41 @@ void update_accumulator_incremental( updateContext.apply(added, removed); else { - assert(added.size() == 1 || added.size() == 2); - assert(removed.size() == 1 || removed.size() == 2); - assert((Forward && added.size() <= removed.size()) - || (!Forward && added.size() >= removed.size())); + [[maybe_unused]] const int addedSize = added.ssize(); + [[maybe_unused]] const int removedSize = removed.ssize(); + + assert(addedSize == 1 || addedSize == 2); + assert(removedSize == 1 || removedSize == 2); + assert((Forward && addedSize <= removedSize) || (!Forward && addedSize >= removedSize)); // Workaround compiler warning for uninitialized variables, replicated // on profile builds on windows with gcc 14.2.0. - // TODO remove once unneeded - sf_assume(added.size() == 1 || added.size() == 2); - sf_assume(removed.size() == 1 || removed.size() == 2); + // Also helps with optimizations on some compilers. - if ((Forward && removed.size() == 1) || (!Forward && added.size() == 1)) + sf_assume(addedSize == 1 || addedSize == 2); + sf_assume(removedSize == 1 || removedSize == 2); + + if (!(removedSize == 1 || removedSize == 2) || !(addedSize == 1 || addedSize == 2)) + sf_unreachable(); + + if ((Forward && removedSize == 1) || (!Forward && addedSize == 1)) { - assert(added.size() == 1 && removed.size() == 1); + assert(addedSize == 1 && removedSize == 1); updateContext.template apply(added[0], removed[0]); } - else if (Forward && added.size() == 1) + else if (Forward && addedSize == 1) { - assert(removed.size() == 2); + assert(removedSize == 2); updateContext.template apply(added[0], removed[0], removed[1]); } - else if (!Forward && removed.size() == 1) + else if (!Forward && removedSize == 1) { - assert(added.size() == 2); + assert(addedSize == 2); updateContext.template apply(added[0], added[1], removed[0]); } else { - assert(added.size() == 2 && removed.size() == 2); + assert(addedSize == 2 && removedSize == 2); updateContext.template apply(added[0], added[1], removed[0], removed[1]); }