From eeb836b8670c3e48819fbdc5310db6a234f0d115 Mon Sep 17 00:00:00 2001 From: Dor Forer Date: Wed, 8 Jul 2026 11:30:20 +0300 Subject: [PATCH 1/3] MOD-16730 Fix SIGSEGV in FP32/L2 SSE kernel: unaligned load in residual-3 path The residual % 4 == 3 path of FP32_L2SqrSIMD16_SSE used _mm_loadr_ps, which compiles to movaps and requires a 16-byte-aligned address. Vectors are not guaranteed that alignment: VecSimAllocator::allocate() returns malloc + 8 (allocation header), and the dispatcher sets no alignment hint when dim % 4 != 0 - which is always the case on this path. On machines whose dispatcher selects the SSE tier (no AVX), any FP32/L2 query with dim % 16 in {3, 7, 11, 15} crashed with SIGSEGV. Replace the reversed aligned load with an unaligned load plus an in-register reverse shuffle - the same register contents, so the _mm_move_ss masking trick and all results are unchanged. At machine level this swaps movaps for movups with the identical shufps the compiler already emitted; no added instructions. Add a regression test that feeds the kernel buffers at the allocator's exact placement (16-aligned base + 8); it segfaults on the previous kernel and passes now. --- src/VecSim/spaces/L2/L2_SSE_FP32.h | 13 +++++++++---- tests/unit/test_spaces.cpp | 22 ++++++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/VecSim/spaces/L2/L2_SSE_FP32.h b/src/VecSim/spaces/L2/L2_SSE_FP32.h index e04cc4fe5..c60a9dc81 100644 --- a/src/VecSim/spaces/L2/L2_SSE_FP32.h +++ b/src/VecSim/spaces/L2/L2_SSE_FP32.h @@ -31,10 +31,15 @@ float FP32_L2SqrSIMD16_SSE(const void *pVect1v, const void *pVect2v, size_t dime if constexpr (residual % 4) { __m128 v1, v2, diff; if constexpr (residual % 4 == 3) { - // Load 3 floats and set the last one to 0 - v1 = _mm_loadr_ps(pVect1); // load 4 floats - v2 = _mm_loadr_ps(pVect2); - // sets the last float of v1 to the last of v2, so the diff is 0. + // Load 4 floats (unaligned) and reverse them, so the out-of-residual 4th element + // lands in the low lane. Vectors are not guaranteed to be 16-byte aligned here + // (block stride is dim * 4 and no alignment hint is set when dim % 4 != 0), so an + // aligned load (_mm_loadr_ps / movaps) would fault. + v1 = _mm_loadu_ps(pVect1); + v2 = _mm_loadu_ps(pVect2); + v1 = _mm_shuffle_ps(v1, v1, _MM_SHUFFLE(0, 1, 2, 3)); + v2 = _mm_shuffle_ps(v2, v2, _MM_SHUFFLE(0, 1, 2, 3)); + // sets the low float of v1 to the low float of v2, so the diff is 0. v1 = _mm_move_ss(v1, v2); } else if constexpr (residual % 4 == 2) { // Load 2 floats and set the last two to 0 diff --git a/tests/unit/test_spaces.cpp b/tests/unit/test_spaces.cpp index 9a16bf30d..f42a6ad4b 100644 --- a/tests/unit/test_spaces.cpp +++ b/tests/unit/test_spaces.cpp @@ -581,6 +581,28 @@ TEST_F(SpacesTest, GetDistFuncSQ8FP16Asymmetric) { } #ifdef CPU_FEATURES_ARCH_X86_64 +#ifdef OPT_SSE +// Regression test for MOD-16730: the FP32 L2 SSE kernel's residual % 4 == 3 path used +// _mm_loadr_ps (movaps), which faults on non-16-byte-aligned addresses. Vectors are not +// guaranteed such alignment: VecSimAllocator::allocate() returns malloc + 8 (allocation +// header), and the dispatcher sets no alignment hint when dim % 4 != 0. This test feeds the +// kernel buffers at that exact placement (16-aligned base + 8). +TEST_F(SpacesTest, FP32_L2Sqr_SSE_misaligned_residual3) { + constexpr size_t dim = 19; // dim % 16 == 3 -> residual 3 path + alignas(16) static char raw1[16 + dim * sizeof(float)]; + alignas(16) static char raw2[16 + dim * sizeof(float)]; + float *v1 = reinterpret_cast(raw1 + 8); // address == 8 (mod 16) + float *v2 = reinterpret_cast(raw2 + 8); + for (size_t i = 0; i < dim; i++) { + v1[i] = float(i); + v2[i] = float(i) + 1.5f; + } + float baseline = FP32_L2Sqr(v1, v2, dim); + dist_func_t arch_opt_func = spaces::Choose_FP32_L2_implementation_SSE(dim); + ASSERT_EQ(baseline, arch_opt_func(v1, v2, dim)); +} +#endif // OPT_SSE + TEST_F(SpacesTest, smallDimChooser) { // Verify that small dimensions gets the no optimization function. for (size_t dim = 1; dim < 8; dim++) { From 26d3fd56a3f171d7b0bf7e6982a310bb8f92e95e Mon Sep 17 00:00:00 2001 From: Dor Forer Date: Wed, 8 Jul 2026 14:18:08 +0300 Subject: [PATCH 2/3] Compute diff first and mask with a single shuffle (review suggestion) Instead of reversing both vectors before the subtraction, subtract the raw unaligned loads, rotate the out-of-residual 4th element of the diff into the low lane with one shuffle, and zero it via the still-zero accumulator with _mm_move_ss. One shuffle instead of two; results unchanged (lane order is irrelevant to the squared sum). --- src/VecSim/spaces/L2/L2_SSE_FP32.h | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/VecSim/spaces/L2/L2_SSE_FP32.h b/src/VecSim/spaces/L2/L2_SSE_FP32.h index c60a9dc81..8f249dc68 100644 --- a/src/VecSim/spaces/L2/L2_SSE_FP32.h +++ b/src/VecSim/spaces/L2/L2_SSE_FP32.h @@ -31,28 +31,29 @@ float FP32_L2SqrSIMD16_SSE(const void *pVect1v, const void *pVect2v, size_t dime if constexpr (residual % 4) { __m128 v1, v2, diff; if constexpr (residual % 4 == 3) { - // Load 4 floats (unaligned) and reverse them, so the out-of-residual 4th element - // lands in the low lane. Vectors are not guaranteed to be 16-byte aligned here + // Load 4 floats (unaligned). Vectors are not guaranteed to be 16-byte aligned here // (block stride is dim * 4 and no alignment hint is set when dim % 4 != 0), so an // aligned load (_mm_loadr_ps / movaps) would fault. v1 = _mm_loadu_ps(pVect1); v2 = _mm_loadu_ps(pVect2); - v1 = _mm_shuffle_ps(v1, v1, _MM_SHUFFLE(0, 1, 2, 3)); - v2 = _mm_shuffle_ps(v2, v2, _MM_SHUFFLE(0, 1, 2, 3)); - // sets the low float of v1 to the low float of v2, so the diff is 0. - v1 = _mm_move_ss(v1, v2); + diff = _mm_sub_ps(v1, v2); + // Rotate the out-of-residual 4th element into the low lane and zero it via the + // (still zero) accumulator - that element is processed by the next step. + diff = _mm_shuffle_ps(diff, diff, _MM_SHUFFLE(2, 1, 0, 3)); + diff = _mm_move_ss(diff, sum); } else if constexpr (residual % 4 == 2) { // Load 2 floats and set the last two to 0 v1 = _mm_loadh_pi(_mm_setzero_ps(), (__m64 *)pVect1); v2 = _mm_loadh_pi(_mm_setzero_ps(), (__m64 *)pVect2); + diff = _mm_sub_ps(v1, v2); } else if constexpr (residual % 4 == 1) { // Load 1 float and set the last three to 0 v1 = _mm_load_ss(pVect1); v2 = _mm_load_ss(pVect2); + diff = _mm_sub_ps(v1, v2); } pVect1 += residual % 4; pVect2 += residual % 4; - diff = _mm_sub_ps(v1, v2); sum = _mm_mul_ps(diff, diff); } From 979548087cdaf553171d7109ba97f2413480ac10 Mon Sep 17 00:00:00 2001 From: Dor Forer Date: Wed, 8 Jul 2026 15:22:25 +0300 Subject: [PATCH 3/3] Use explicit zero for the move_ss mask source Avoids depending on sum still being zero at this point; the zero idiom is free and the compiler typically reuses an existing zeroed register. --- src/VecSim/spaces/L2/L2_SSE_FP32.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/VecSim/spaces/L2/L2_SSE_FP32.h b/src/VecSim/spaces/L2/L2_SSE_FP32.h index 8f249dc68..175fbb609 100644 --- a/src/VecSim/spaces/L2/L2_SSE_FP32.h +++ b/src/VecSim/spaces/L2/L2_SSE_FP32.h @@ -37,10 +37,10 @@ float FP32_L2SqrSIMD16_SSE(const void *pVect1v, const void *pVect2v, size_t dime v1 = _mm_loadu_ps(pVect1); v2 = _mm_loadu_ps(pVect2); diff = _mm_sub_ps(v1, v2); - // Rotate the out-of-residual 4th element into the low lane and zero it via the - // (still zero) accumulator - that element is processed by the next step. + // Rotate the out-of-residual 4th element into the low lane and zero it - that + // element is processed by the next step. diff = _mm_shuffle_ps(diff, diff, _MM_SHUFFLE(2, 1, 0, 3)); - diff = _mm_move_ss(diff, sum); + diff = _mm_move_ss(diff, _mm_setzero_ps()); } else if constexpr (residual % 4 == 2) { // Load 2 floats and set the last two to 0 v1 = _mm_loadh_pi(_mm_setzero_ps(), (__m64 *)pVect1);