From bfeb36eb3d488bc767dc273ec515ffb7e03189ef Mon Sep 17 00:00:00 2001 From: Disservin Date: Sun, 8 Feb 2026 14:56:18 +0100 Subject: [PATCH] Fix Strict Aliasing Violation Even though this failed with non regression bounds on fishtest, merging it as is to improve the current situation. Failed Non Regression: https://tests.stockfishchess.org/tests/view/6988814db0f3ca5200aafb50 LLR: -2.94 (-2.94,2.94) <-1.75,0.25> Total: 185792 W: 48109 L: 48565 D: 89118 Ptnml(0-2): 553, 21228, 49811, 20730, 574 Previously we had code similar to this ``` alignas(64) std::uint8_t foo[1024]; const auto* int_ptr = reinterpret_cast(foo); ``` Which is a strict aliasing violation once we dereference int_ptr, which invokes UB and causes newer gcc version to emit incorrect code. fixes https://github.com/official-stockfish/Stockfish/issues/6598 fixes https://github.com/official-stockfish/Stockfish/issues/4932 This violation was introduced in PR #3287 and merged in commit 23c385e. Due to aggressive compiler optimizations, users may need to include CXXFLAGS=-fno-strict-aliasing as a build argument to ensure a correctly functioning binary. Also thanks to @anematode for writing parts of this and realizing this strict aliasing violation. closes https://github.com/official-stockfish/Stockfish/pull/6599 Bench: 2668754 Co-Authored-By: Timothy Herchen --- src/memory.h | 13 ++++++- src/nnue/layers/affine_transform.h | 9 +++-- .../layers/affine_transform_sparse_input.h | 38 ++++++++++--------- src/nnue/layers/clipped_relu.h | 8 ++-- src/nnue/simd.h | 16 +++++--- 5 files changed, 53 insertions(+), 31 deletions(-) diff --git a/src/memory.h b/src/memory.h index c307a1317..056b07c6d 100644 --- a/src/memory.h +++ b/src/memory.h @@ -20,12 +20,12 @@ #define MEMORY_H_INCLUDED #include -#include #include #include #include #include #include +#include #include "types.h" @@ -317,6 +317,17 @@ auto windows_try_with_large_page_priviliges([[maybe_unused]] FuncYesT&& fyes, Fu #endif +template +T load_as(const ByteT* buffer) { + static_assert(std::is_trivially_copyable::value, "Type must be trivially copyable"); + static_assert(sizeof(ByteT) == 1); + + T value; + std::memcpy(&value, buffer, sizeof(T)); + + return value; +} + } // namespace Stockfish #endif // #ifndef MEMORY_H_INCLUDED diff --git a/src/nnue/layers/affine_transform.h b/src/nnue/layers/affine_transform.h index a3d072f67..6cd44e19c 100644 --- a/src/nnue/layers/affine_transform.h +++ b/src/nnue/layers/affine_transform.h @@ -24,6 +24,7 @@ #include #include +#include "../../memory.h" #include "../nnue_common.h" #include "../simd.h" @@ -95,7 +96,7 @@ static void affine_transform_non_ssse3(std::int32_t* output, #elif defined(USE_NEON) int32x4_t sum = {biases[i]}; - const auto row = reinterpret_cast(&weights[offset]); + const auto row = reinterpret_cast(&weights[offset]); for (IndexType j = 0; j < NumChunks; ++j) { int16x8_t product = vmull_s8(inputVector[j * 2], row[j * 2]); @@ -224,7 +225,6 @@ class AffineTransform { constexpr IndexType NumChunks = ceil_to_multiple(InputDimensions, 8) / 4; constexpr IndexType NumRegs = OutputDimensions / OutputSimdWidth; - const auto input32 = reinterpret_cast(input); const vec_t* biasvec = reinterpret_cast(biases); vec_t acc[NumRegs]; for (IndexType k = 0; k < NumRegs; ++k) @@ -232,8 +232,9 @@ class AffineTransform { for (IndexType i = 0; i < NumChunks; ++i) { - const vec_t in0 = vec_set_32(input32[i]); - const auto col0 = + const vec_t in0 = + vec_set_32(load_as(input + i * sizeof(std::int32_t))); + const auto col0 = reinterpret_cast(&weights[i * OutputDimensions * 4]); for (IndexType k = 0; k < NumRegs; ++k) diff --git a/src/nnue/layers/affine_transform_sparse_input.h b/src/nnue/layers/affine_transform_sparse_input.h index 5e0551f69..059a77397 100644 --- a/src/nnue/layers/affine_transform_sparse_input.h +++ b/src/nnue/layers/affine_transform_sparse_input.h @@ -27,6 +27,7 @@ #include #include "../../bitboard.h" +#include "../../memory.h" #include "../simd.h" #include "../nnue_common.h" @@ -77,15 +78,17 @@ alignas(CacheLineSize) static constexpr struct OffsetIndices { #define RESTRICT #endif -// Find indices of nonzero numbers in an int32_t array +// Find indices of nonzero 32-bit values in a packed byte buffer. +// The input pointer addresses a sequence of 32-bit blocks stored in a +// std::uint8_t array. template -void find_nnz(const std::int32_t* RESTRICT input, +void find_nnz(const std::uint8_t* RESTRICT input, std::uint16_t* RESTRICT out, IndexType& count_out) { #if defined(USE_AVX512ICL) - constexpr IndexType SimdWidthIn = 16; // 512 bits / 32 bits + constexpr IndexType SimdWidthIn = 64; // 512 bits constexpr IndexType SimdWidthOut = 32; // 512 bits / 16 bits constexpr IndexType NumChunks = InputDimensions / SimdWidthOut; const __m512i increment = _mm512_set1_epi16(SimdWidthOut); @@ -122,7 +125,7 @@ void find_nnz(const std::int32_t* RESTRICT input, IndexType count = 0; for (IndexType i = 0; i < NumChunks; ++i) { - const __m512i inputV = _mm512_load_si512(input + i * SimdWidth); + const __m512i inputV = _mm512_load_si512(input + i * SimdWidth * sizeof(std::uint32_t)); // Get a bitmask and gather non zero indices const __mmask16 nnzMask = _mm512_test_epi32_mask(inputV, inputV); @@ -293,10 +296,8 @@ class AffineTransformSparseInput { std::uint16_t nnz[NumChunks]; IndexType count; - const auto input32 = reinterpret_cast(input); - // Find indices of nonzero 32-bit blocks - find_nnz(input32, nnz, count); + find_nnz(input, nnz, count); const outvec_t* biasvec = reinterpret_cast(biases); outvec_t acc[NumRegs]; @@ -314,13 +315,16 @@ class AffineTransformSparseInput { while (start < end - 2) { - const std::ptrdiff_t i0 = *start++; - const std::ptrdiff_t i1 = *start++; - const std::ptrdiff_t i2 = *start++; - const invec_t in0 = vec_set_32(input32[i0]); - const invec_t in1 = vec_set_32(input32[i1]); - const invec_t in2 = vec_set_32(input32[i2]); - const auto col0 = + const std::ptrdiff_t i0 = *start++; + const std::ptrdiff_t i1 = *start++; + const std::ptrdiff_t i2 = *start++; + const invec_t in0 = + vec_set_32(load_as(input + i0 * sizeof(std::int32_t))); + const invec_t in1 = + vec_set_32(load_as(input + i1 * sizeof(std::int32_t))); + const invec_t in2 = + vec_set_32(load_as(input + i2 * sizeof(std::int32_t))); + const auto col0 = reinterpret_cast(&weights_cp[i0 * OutputDimensions * ChunkSize]); const auto col1 = reinterpret_cast(&weights_cp[i1 * OutputDimensions * ChunkSize]); @@ -338,9 +342,9 @@ class AffineTransformSparseInput { #endif while (start < end) { - const std::ptrdiff_t i = *start++; - const invec_t in = vec_set_32(input32[i]); - const auto col = + const std::ptrdiff_t i = *start++; + const invec_t in = vec_set_32(load_as(input + i * sizeof(std::int32_t))); + const auto col = reinterpret_cast(&weights_cp[i * OutputDimensions * ChunkSize]); for (IndexType k = 0; k < NumAccums; ++k) vec_add_dpbusd_32(acc[k], in, col[k]); diff --git a/src/nnue/layers/clipped_relu.h b/src/nnue/layers/clipped_relu.h index 7284c1033..9ce85d3fa 100644 --- a/src/nnue/layers/clipped_relu.h +++ b/src/nnue/layers/clipped_relu.h @@ -141,10 +141,10 @@ class ClippedReLU { constexpr IndexType Start = NumChunks * SimdWidth; #elif defined(USE_NEON) - constexpr IndexType NumChunks = InputDimensions / (SimdWidth / 2); - const int8x8_t Zero = {0}; - const auto in = reinterpret_cast(input); - const auto out = reinterpret_cast(output); + constexpr IndexType NumChunks = InputDimensions / (SimdWidth / 2); + const SIMD::vec_i8x8_t Zero = {0}; + const auto in = reinterpret_cast(input); + const auto out = reinterpret_cast(output); for (IndexType i = 0; i < NumChunks; ++i) { int16x8_t shifted; diff --git a/src/nnue/simd.h b/src/nnue/simd.h index 9c689b098..601792c1c 100644 --- a/src/nnue/simd.h +++ b/src/nnue/simd.h @@ -181,11 +181,17 @@ inline __m128i vec_convert_8_16(uint64_t x) { #define MaxChunkSize 16 #elif USE_NEON -using vec_t = int16x8_t; -using vec_i8_t = int8x16_t; -using psqt_vec_t = int32x4_t; -using vec128_t = uint16x8_t; -using vec_uint_t = uint32x4_t; +using vec_i8x8_t __attribute__((may_alias)) = int8x8_t; +using vec_i16x8_t __attribute__((may_alias)) = int16x8_t; +using vec_i8x16_t __attribute__((may_alias)) = int8x16_t; +using vec_u16x8_t __attribute__((may_alias)) = uint16x8_t; +using vec_i32x4_t __attribute__((may_alias)) = int32x4_t; + +using vec_t __attribute__((may_alias)) = int16x8_t; +using vec_i8_t __attribute__((may_alias)) = int8x16_t; +using psqt_vec_t __attribute__((may_alias)) = int32x4_t; +using vec128_t __attribute__((may_alias)) = uint16x8_t; +using vec_uint_t __attribute__((may_alias)) = uint32x4_t; #define vec_load(a) (*(a)) #define vec_store(a, b) *(a) = (b) #define vec_add_16(a, b) vaddq_s16(a, b)