From 2e7aa6c27ca98f998cc4f87513a28e64322db80d Mon Sep 17 00:00:00 2001 From: Dor Forer Date: Wed, 19 Aug 2026 14:02:38 +0300 Subject: [PATCH 1/3] Split the NEON half-precision tier into NEON_HP and NEON_FHM NEON_HP.cpp was one translation unit compiled with -march=armv8.2-a+fp16fml, but its HP-only entry points were dispatched on features.asimdhp alone. Because the whole TU carried +fp16fml, the compiler was licensed to emit FMLAL/FMLSL instructions anywhere in it, including into the HP-only functions whose source has no FMLAL intrinsic. Measured on arm-r8g.xlarge with gcc 12: the HP-only wrappers compiled from identical source went from 32 AdvSIMD FMLAL/FMLSL instructions at +fp16fml down to 0 once compiled at +fp16 alone. On a core with asimdhp but without asimdfhm, the old HP path was therefore a SIGILL. Tightening the predicate to require both asimdhp and asimdfhm would not have fixed this: it would have deleted the HP-only fallback for exactly the CPUs that need it. The fix is two tiers with two translation units, each compiled only with the license its own kernels need: NEON_HP.cpp now builds at +fp16, and the new NEON_FHM.cpp carries the FHM-only entry points at +fp16fml, where they still measure 64 AdvSIMD FMLAL/FMLSL instructions, so the fast path is intact. Dispatch sites gain a second, independently guarded branch (asimdhp && asimdfhm) that tries NEON_FHM first and falls back to the existing asimdhp-only NEON_HP branch. --- cmake/aarch64InstructionFlags.cmake | 7 +++- src/VecSim/spaces/CMakeLists.txt | 9 ++++- src/VecSim/spaces/IP_space.cpp | 13 +++++-- src/VecSim/spaces/L2_space.cpp | 7 +++- src/VecSim/spaces/functions/NEON_FHM.cpp | 38 +++++++++++++++++++ src/VecSim/spaces/functions/NEON_FHM.h | 19 ++++++++++ src/VecSim/spaces/functions/NEON_HP.cpp | 19 ---------- src/VecSim/spaces/functions/NEON_HP.h | 4 -- tests/benchmark/spaces_benchmarks/bm_spaces.h | 1 + .../spaces_benchmarks/bm_spaces_sq8_fp16.cpp | 4 +- tests/unit/test_spaces.cpp | 13 +++++-- 11 files changed, 99 insertions(+), 35 deletions(-) create mode 100644 src/VecSim/spaces/functions/NEON_FHM.cpp create mode 100644 src/VecSim/spaces/functions/NEON_FHM.h diff --git a/cmake/aarch64InstructionFlags.cmake b/cmake/aarch64InstructionFlags.cmake index 1a2842265..5cd84331a 100644 --- a/cmake/aarch64InstructionFlags.cmake +++ b/cmake/aarch64InstructionFlags.cmake @@ -9,7 +9,8 @@ CHECK_CXX_COMPILER_FLAG("-march=armv8-a" CXX_ARMV8A) CHECK_CXX_COMPILER_FLAG("-march=armv8.2-a+dotprod" CXX_NEON_DOTPROD) CHECK_CXX_COMPILER_FLAG("-march=armv8-a+sve" CXX_SVE) CHECK_CXX_COMPILER_FLAG("-march=armv9-a+sve2" CXX_SVE2) -CHECK_CXX_COMPILER_FLAG("-march=armv8.2-a+fp16fml" CXX_NEON_HP) +CHECK_CXX_COMPILER_FLAG("-march=armv8.2-a+fp16" CXX_NEON_HP) +CHECK_CXX_COMPILER_FLAG("-march=armv8.2-a+fp16fml" CXX_NEON_FHM) CHECK_CXX_COMPILER_FLAG("-march=armv8.2-a+bf16" CXX_NEON_BF16) CHECK_CXX_COMPILER_FLAG("-march=armv8.2-a+sve+bf16" CXX_SVE_BF16) @@ -29,6 +30,10 @@ if (CXX_NEON_HP) message(STATUS "Using ARMv8.2-a with NEON half-percision extension") add_compile_definitions(OPT_NEON_HP) endif() +if (CXX_NEON_FHM) + message(STATUS "Using ARMv8.2-a with NEON FHM extension") + add_compile_definitions(OPT_NEON_FHM) +endif() if (CXX_NEON_BF16) add_compile_definitions(OPT_NEON_BF16) endif() diff --git a/src/VecSim/spaces/CMakeLists.txt b/src/VecSim/spaces/CMakeLists.txt index 309d3f3a4..eb8a15fdd 100644 --- a/src/VecSim/spaces/CMakeLists.txt +++ b/src/VecSim/spaces/CMakeLists.txt @@ -141,10 +141,17 @@ if (CMAKE_SYSTEM_PROCESSOR MATCHES "(aarch64)|(arm64)|(ARM64)|(armv.*)") # NEON half-precision support if (CXX_NEON_HP AND CXX_ARMV8A) message("Building with NEON+HP") - set_source_files_properties(functions/NEON_HP.cpp PROPERTIES COMPILE_FLAGS "-march=armv8.2-a+fp16fml") + set_source_files_properties(functions/NEON_HP.cpp PROPERTIES COMPILE_FLAGS "-march=armv8.2-a+fp16") list(APPEND OPTIMIZATIONS functions/NEON_HP.cpp) endif() + # NEON FHM (FEAT_FHM / asimdfhm) support + if (CXX_NEON_FHM AND CXX_ARMV8A) + message("Building with NEON+FHM") + set_source_files_properties(functions/NEON_FHM.cpp PROPERTIES COMPILE_FLAGS "-march=armv8.2-a+fp16fml") + list(APPEND OPTIMIZATIONS functions/NEON_FHM.cpp) + endif() + # NEON bfloat16 support if (CXX_NEON_BF16) message("Building with NEON + BF16") diff --git a/src/VecSim/spaces/IP_space.cpp b/src/VecSim/spaces/IP_space.cpp index 1f5ee55c2..b7f963d2c 100644 --- a/src/VecSim/spaces/IP_space.cpp +++ b/src/VecSim/spaces/IP_space.cpp @@ -29,6 +29,7 @@ #include "VecSim/spaces/functions/NEON.h" #include "VecSim/spaces/functions/NEON_DOTPROD.h" #include "VecSim/spaces/functions/NEON_HP.h" +#include "VecSim/spaces/functions/NEON_FHM.h" #include "VecSim/spaces/functions/NEON_BF16.h" #include "VecSim/spaces/functions/SVE.h" #include "VecSim/spaces/functions/SVE_BF16.h" @@ -241,10 +242,12 @@ dist_func_t IP_SQ8_FP16_GetDistFunc(size_t dim, unsigned char *alignment, return Choose_SQ8_FP16_IP_implementation_SVE(dim); } #endif -#ifdef OPT_NEON_HP - if (features.asimdfhm) { +#ifdef OPT_NEON_FHM + if (features.asimdhp && features.asimdfhm) { return Choose_SQ8_FP16_IP_implementation_NEON_FHM(dim); } +#endif +#ifdef OPT_NEON_HP if (features.asimdhp) { return Choose_SQ8_FP16_IP_implementation_NEON_HP(dim); } @@ -313,10 +316,12 @@ dist_func_t Cosine_SQ8_FP16_GetDistFunc(size_t dim, unsigned char *alignm return Choose_SQ8_FP16_Cosine_implementation_SVE(dim); } #endif -#ifdef OPT_NEON_HP - if (features.asimdfhm) { +#ifdef OPT_NEON_FHM + if (features.asimdhp && features.asimdfhm) { return Choose_SQ8_FP16_Cosine_implementation_NEON_FHM(dim); } +#endif +#ifdef OPT_NEON_HP if (features.asimdhp) { return Choose_SQ8_FP16_Cosine_implementation_NEON_HP(dim); } diff --git a/src/VecSim/spaces/L2_space.cpp b/src/VecSim/spaces/L2_space.cpp index cf0b52f7c..2c9223b67 100644 --- a/src/VecSim/spaces/L2_space.cpp +++ b/src/VecSim/spaces/L2_space.cpp @@ -28,6 +28,7 @@ #include "VecSim/spaces/functions/NEON.h" #include "VecSim/spaces/functions/NEON_DOTPROD.h" #include "VecSim/spaces/functions/NEON_HP.h" +#include "VecSim/spaces/functions/NEON_FHM.h" #include "VecSim/spaces/functions/NEON_BF16.h" #include "VecSim/spaces/functions/SVE.h" #include "VecSim/spaces/functions/SVE_BF16.h" @@ -170,10 +171,12 @@ dist_func_t L2_SQ8_FP16_GetDistFunc(size_t dim, unsigned char *alignment, return Choose_SQ8_FP16_L2_implementation_SVE(dim); } #endif -#ifdef OPT_NEON_HP - if (features.asimdfhm) { +#ifdef OPT_NEON_FHM + if (features.asimdhp && features.asimdfhm) { return Choose_SQ8_FP16_L2_implementation_NEON_FHM(dim); } +#endif +#ifdef OPT_NEON_HP if (features.asimdhp) { return Choose_SQ8_FP16_L2_implementation_NEON_HP(dim); } diff --git a/src/VecSim/spaces/functions/NEON_FHM.cpp b/src/VecSim/spaces/functions/NEON_FHM.cpp new file mode 100644 index 000000000..2beceed0c --- /dev/null +++ b/src/VecSim/spaces/functions/NEON_FHM.cpp @@ -0,0 +1,38 @@ +/* + * Copyright (c) 2006-Present, Redis Ltd. + * All rights reserved. + * + * Licensed under your choice of the Redis Source Available License 2.0 + * (RSALv2); or (b) the Server Side Public License v1 (SSPLv1); or (c) the + * GNU Affero General Public License v3 (AGPLv3). + */ +#include "NEON_FHM.h" + +#include "VecSim/spaces/IP/IP_NEON_SQ8_FP16.h" +#include "VecSim/spaces/L2/L2_NEON_SQ8_FP16.h" + +namespace spaces { + +#include "implementation_chooser.h" + +dist_func_t Choose_SQ8_FP16_IP_implementation_NEON_FHM(size_t dim) { + dist_func_t ret_dist_func; + CHOOSE_IMPLEMENTATION(ret_dist_func, dim, 16, SQ8_FP16_InnerProductSIMD16_NEON_FHM); + return ret_dist_func; +} + +dist_func_t Choose_SQ8_FP16_L2_implementation_NEON_FHM(size_t dim) { + dist_func_t ret_dist_func; + CHOOSE_IMPLEMENTATION(ret_dist_func, dim, 16, SQ8_FP16_L2SqrSIMD16_NEON_FHM); + return ret_dist_func; +} + +dist_func_t Choose_SQ8_FP16_Cosine_implementation_NEON_FHM(size_t dim) { + dist_func_t ret_dist_func; + CHOOSE_IMPLEMENTATION(ret_dist_func, dim, 16, SQ8_FP16_CosineSIMD16_NEON_FHM); + return ret_dist_func; +} + +#include "implementation_chooser_cleanup.h" + +} // namespace spaces diff --git a/src/VecSim/spaces/functions/NEON_FHM.h b/src/VecSim/spaces/functions/NEON_FHM.h new file mode 100644 index 000000000..0993aa69f --- /dev/null +++ b/src/VecSim/spaces/functions/NEON_FHM.h @@ -0,0 +1,19 @@ +/* + * Copyright (c) 2006-Present, Redis Ltd. + * All rights reserved. + * + * Licensed under your choice of the Redis Source Available License 2.0 + * (RSALv2); or (b) the Server Side Public License v1 (SSPLv1); or (c) the + * GNU Affero General Public License v3 (AGPLv3). + */ +#pragma once + +#include "VecSim/spaces/spaces.h" + +namespace spaces { + +dist_func_t Choose_SQ8_FP16_IP_implementation_NEON_FHM(size_t dim); +dist_func_t Choose_SQ8_FP16_L2_implementation_NEON_FHM(size_t dim); +dist_func_t Choose_SQ8_FP16_Cosine_implementation_NEON_FHM(size_t dim); + +} // namespace spaces diff --git a/src/VecSim/spaces/functions/NEON_HP.cpp b/src/VecSim/spaces/functions/NEON_HP.cpp index 15e40ba82..20d93a517 100644 --- a/src/VecSim/spaces/functions/NEON_HP.cpp +++ b/src/VecSim/spaces/functions/NEON_HP.cpp @@ -47,25 +47,6 @@ dist_func_t Choose_SQ8_FP16_Cosine_implementation_NEON_HP(size_t dim) { return ret_dist_func; } -// FMLAL (FEAT_FHM / asimdfhm) variants. -dist_func_t Choose_SQ8_FP16_IP_implementation_NEON_FHM(size_t dim) { - dist_func_t ret_dist_func; - CHOOSE_IMPLEMENTATION(ret_dist_func, dim, 16, SQ8_FP16_InnerProductSIMD16_NEON_FHM); - return ret_dist_func; -} - -dist_func_t Choose_SQ8_FP16_L2_implementation_NEON_FHM(size_t dim) { - dist_func_t ret_dist_func; - CHOOSE_IMPLEMENTATION(ret_dist_func, dim, 16, SQ8_FP16_L2SqrSIMD16_NEON_FHM); - return ret_dist_func; -} - -dist_func_t Choose_SQ8_FP16_Cosine_implementation_NEON_FHM(size_t dim) { - dist_func_t ret_dist_func; - CHOOSE_IMPLEMENTATION(ret_dist_func, dim, 16, SQ8_FP16_CosineSIMD16_NEON_FHM); - return ret_dist_func; -} - #include "implementation_chooser_cleanup.h" } // namespace spaces diff --git a/src/VecSim/spaces/functions/NEON_HP.h b/src/VecSim/spaces/functions/NEON_HP.h index 83579d2b7..889eb0919 100644 --- a/src/VecSim/spaces/functions/NEON_HP.h +++ b/src/VecSim/spaces/functions/NEON_HP.h @@ -20,8 +20,4 @@ dist_func_t Choose_SQ8_FP16_IP_implementation_NEON_HP(size_t dim); dist_func_t Choose_SQ8_FP16_L2_implementation_NEON_HP(size_t dim); dist_func_t Choose_SQ8_FP16_Cosine_implementation_NEON_HP(size_t dim); -dist_func_t Choose_SQ8_FP16_IP_implementation_NEON_FHM(size_t dim); -dist_func_t Choose_SQ8_FP16_L2_implementation_NEON_FHM(size_t dim); -dist_func_t Choose_SQ8_FP16_Cosine_implementation_NEON_FHM(size_t dim); - } // namespace spaces diff --git a/tests/benchmark/spaces_benchmarks/bm_spaces.h b/tests/benchmark/spaces_benchmarks/bm_spaces.h index 2303eac0a..38995e35f 100644 --- a/tests/benchmark/spaces_benchmarks/bm_spaces.h +++ b/tests/benchmark/spaces_benchmarks/bm_spaces.h @@ -35,6 +35,7 @@ #include "VecSim/spaces/functions/NEON.h" #include "VecSim/spaces/functions/NEON_DOTPROD.h" #include "VecSim/spaces/functions/NEON_HP.h" +#include "VecSim/spaces/functions/NEON_FHM.h" #include "VecSim/spaces/functions/NEON_BF16.h" #include "VecSim/spaces/functions/SVE.h" #include "VecSim/spaces/functions/SVE_BF16.h" diff --git a/tests/benchmark/spaces_benchmarks/bm_spaces_sq8_fp16.cpp b/tests/benchmark/spaces_benchmarks/bm_spaces_sq8_fp16.cpp index 5ab529372..c7450264b 100644 --- a/tests/benchmark/spaces_benchmarks/bm_spaces_sq8_fp16.cpp +++ b/tests/benchmark/spaces_benchmarks/bm_spaces_sq8_fp16.cpp @@ -105,8 +105,10 @@ bool neon_hp_supported = arm_opt.asimdhp; INITIALIZE_BENCHMARKS_SET_L2_IP(BM_VecSimSpaces_SQ8_FP16, SQ8_FP16, NEON_HP, 16, neon_hp_supported); INITIALIZE_BENCHMARKS_SET_Cosine(BM_VecSimSpaces_SQ8_FP16, SQ8_FP16, NEON_HP, 16, neon_hp_supported); +#endif -bool neon_fhm_supported = arm_opt.asimdfhm; +#ifdef OPT_NEON_FHM +bool neon_fhm_supported = arm_opt.asimdhp && arm_opt.asimdfhm; INITIALIZE_BENCHMARKS_SET_L2_IP(BM_VecSimSpaces_SQ8_FP16, SQ8_FP16, NEON_FHM, 16, neon_fhm_supported); INITIALIZE_BENCHMARKS_SET_Cosine(BM_VecSimSpaces_SQ8_FP16, SQ8_FP16, NEON_FHM, 16, diff --git a/tests/unit/test_spaces.cpp b/tests/unit/test_spaces.cpp index c2aab0fd5..18ef7a625 100644 --- a/tests/unit/test_spaces.cpp +++ b/tests/unit/test_spaces.cpp @@ -43,6 +43,7 @@ #include "VecSim/spaces/functions/NEON.h" #include "VecSim/spaces/functions/NEON_DOTPROD.h" #include "VecSim/spaces/functions/NEON_HP.h" +#include "VecSim/spaces/functions/NEON_FHM.h" #include "VecSim/spaces/functions/NEON_BF16.h" #include "VecSim/spaces/functions/SVE.h" #include "VecSim/spaces/functions/SVE_BF16.h" @@ -3370,7 +3371,7 @@ TEST_P(SQ8_FP16_SpacesOptimizationTest, SQ8_FP16_L2SqrTest) { optimization.sve = 0; } #endif -#ifdef OPT_NEON_HP +#ifdef OPT_NEON_FHM if (optimization.asimdfhm) { unsigned char alignment = 0; arch_opt_func = L2_SQ8_FP16_GetDistFunc(dim, &alignment, &optimization); @@ -3381,6 +3382,8 @@ TEST_P(SQ8_FP16_SpacesOptimizationTest, SQ8_FP16_L2SqrTest) { ASSERT_EQ(alignment, 0) << "No alignment NEON_FHM with dim " << dim; optimization.asimdfhm = 0; } +#endif +#ifdef OPT_NEON_HP if (optimization.asimdhp) { unsigned char alignment = 0; arch_opt_func = L2_SQ8_FP16_GetDistFunc(dim, &alignment, &optimization); @@ -3494,7 +3497,7 @@ TEST_P(SQ8_FP16_SpacesOptimizationTest, SQ8_FP16_InnerProductTest) { optimization.sve = 0; } #endif -#ifdef OPT_NEON_HP +#ifdef OPT_NEON_FHM if (optimization.asimdfhm) { unsigned char alignment = 0; arch_opt_func = IP_SQ8_FP16_GetDistFunc(dim, &alignment, &optimization); @@ -3505,6 +3508,8 @@ TEST_P(SQ8_FP16_SpacesOptimizationTest, SQ8_FP16_InnerProductTest) { ASSERT_EQ(alignment, 0) << "No alignment NEON_FHM with dim " << dim; optimization.asimdfhm = 0; } +#endif +#ifdef OPT_NEON_HP if (optimization.asimdhp) { unsigned char alignment = 0; arch_opt_func = IP_SQ8_FP16_GetDistFunc(dim, &alignment, &optimization); @@ -3618,7 +3623,7 @@ TEST_P(SQ8_FP16_SpacesOptimizationTest, SQ8_FP16_CosineTest) { optimization.sve = 0; } #endif -#ifdef OPT_NEON_HP +#ifdef OPT_NEON_FHM if (optimization.asimdfhm) { unsigned char alignment = 0; arch_opt_func = Cosine_SQ8_FP16_GetDistFunc(dim, &alignment, &optimization); @@ -3629,6 +3634,8 @@ TEST_P(SQ8_FP16_SpacesOptimizationTest, SQ8_FP16_CosineTest) { ASSERT_EQ(alignment, 0) << "No alignment NEON_FHM with dim " << dim; optimization.asimdfhm = 0; } +#endif +#ifdef OPT_NEON_HP if (optimization.asimdhp) { unsigned char alignment = 0; arch_opt_func = Cosine_SQ8_FP16_GetDistFunc(dim, &alignment, &optimization); From 246dd131171fa6eee72e63b0949cc3e8b4f7b0d1 Mon Sep 17 00:00:00 2001 From: Dor Forer Date: Wed, 19 Aug 2026 14:14:27 +0300 Subject: [PATCH 2/3] Give ARM kernel instantiations tier-local linkage Two tier translation units that include the same kernel header emitted the same weak (COMDAT) symbols, compiled under different -march flags. The linker then kept one body and discarded the other, chosen by link order, with nothing in the source deciding which. For each of the eight ARM tier TUs (NEON, NEON_DOTPROD, NEON_HP, NEON_FHM, NEON_BF16, SVE, SVE2, SVE_BF16), wrap the kernel-header includes in an anonymous namespace so the kernel instantiations get internal linkage, unique to each translation unit. Only the Choose_* entry points, still declared in the tier header, keep external linkage. The shared dependencies (space_includes.h, spaces.h, the type headers, and the ARM intrinsics headers) are hoisted above the anonymous namespace: wrapping them along with the kernel includes pulls the standard library into the anonymous namespace and fails to compile. Measured on arm-r8g.xlarge (gcc 12) with nm -g --defined-only before this change: NEON and NEON_DOTPROD shared 93 externally-defined symbols, SVE and SVE2 shared 166. This change makes the whole class of collision structurally impossible rather than fixing only those two known pairs. x86 was measured at 0 shared symbols across all 15 tier objects, every pair, because x86 kernel names embed the ISA and no x86 tier TU reuses another tier's kernel headers. x86 tier TUs are therefore untouched. Verification of the after state (all 28 ARM pairs sharing zero symbols, and every object still exporting its own Choose_* entry points) requires building on real ARM hardware and was not run as part of this change. --- src/VecSim/spaces/functions/NEON.cpp | 16 ++++++++++++++++ src/VecSim/spaces/functions/NEON_BF16.cpp | 15 +++++++++++++++ src/VecSim/spaces/functions/NEON_DOTPROD.cpp | 16 ++++++++++++++++ src/VecSim/spaces/functions/NEON_FHM.cpp | 15 +++++++++++++++ src/VecSim/spaces/functions/NEON_HP.cpp | 15 +++++++++++++++ src/VecSim/spaces/functions/SVE.cpp | 16 ++++++++++++++++ src/VecSim/spaces/functions/SVE2.cpp | 15 +++++++++++++++ src/VecSim/spaces/functions/SVE_BF16.cpp | 15 +++++++++++++++ 8 files changed, 123 insertions(+) diff --git a/src/VecSim/spaces/functions/NEON.cpp b/src/VecSim/spaces/functions/NEON.cpp index 0c9a286e3..8d5c286be 100644 --- a/src/VecSim/spaces/functions/NEON.cpp +++ b/src/VecSim/spaces/functions/NEON.cpp @@ -7,6 +7,21 @@ * GNU Affero General Public License v3 (AGPLv3). */ #include "NEON.h" + +// Hoisted above the anonymous namespace below so that the standard library and the shared +// type headers keep external linkage. Wrapping them would pull and friends into +// the anonymous namespace and fail to compile. +#include "VecSim/spaces/space_includes.h" +#include "VecSim/spaces/spaces.h" +#include "VecSim/types/bfloat16.h" +#include "VecSim/types/float16.h" +#include "VecSim/types/sq8.h" +#include + +// Kernel instantiations get internal linkage, unique to this translation unit, so two tiers +// that share a kernel header cannot emit the same weak symbol and let link order pick the +// body. Only this tier's Choose_* entry points stay external. +namespace { #include "VecSim/spaces/L2/L2_NEON_FP32.h" #include "VecSim/spaces/IP/IP_NEON_FP32.h" #include "VecSim/spaces/L2/L2_NEON_INT8.h" @@ -19,6 +34,7 @@ #include "VecSim/spaces/IP/IP_NEON_SQ8_FP32.h" #include "VecSim/spaces/IP/IP_NEON_SQ8_SQ8.h" #include "VecSim/spaces/L2/L2_NEON_SQ8_SQ8.h" +} // namespace namespace spaces { diff --git a/src/VecSim/spaces/functions/NEON_BF16.cpp b/src/VecSim/spaces/functions/NEON_BF16.cpp index 4de205bb8..1a515943e 100644 --- a/src/VecSim/spaces/functions/NEON_BF16.cpp +++ b/src/VecSim/spaces/functions/NEON_BF16.cpp @@ -8,8 +8,23 @@ */ #include "NEON_BF16.h" +// Hoisted above the anonymous namespace below so that the standard library and the shared +// type headers keep external linkage. Wrapping them would pull and friends into +// the anonymous namespace and fail to compile. +#include "VecSim/spaces/space_includes.h" +#include "VecSim/spaces/spaces.h" +#include "VecSim/types/bfloat16.h" +#include "VecSim/types/float16.h" +#include "VecSim/types/sq8.h" +#include + +// Kernel instantiations get internal linkage, unique to this translation unit, so two tiers +// that share a kernel header cannot emit the same weak symbol and let link order pick the +// body. Only this tier's Choose_* entry points stay external. +namespace { #include "VecSim/spaces/L2/L2_NEON_BF16.h" #include "VecSim/spaces/IP/IP_NEON_BF16.h" +} // namespace namespace spaces { diff --git a/src/VecSim/spaces/functions/NEON_DOTPROD.cpp b/src/VecSim/spaces/functions/NEON_DOTPROD.cpp index 12f762093..ade17bb31 100644 --- a/src/VecSim/spaces/functions/NEON_DOTPROD.cpp +++ b/src/VecSim/spaces/functions/NEON_DOTPROD.cpp @@ -7,12 +7,28 @@ * GNU Affero General Public License v3 (AGPLv3). */ #include "NEON.h" + +// Hoisted above the anonymous namespace below so that the standard library and the shared +// type headers keep external linkage. Wrapping them would pull and friends into +// the anonymous namespace and fail to compile. +#include "VecSim/spaces/space_includes.h" +#include "VecSim/spaces/spaces.h" +#include "VecSim/types/bfloat16.h" +#include "VecSim/types/float16.h" +#include "VecSim/types/sq8.h" +#include + +// Kernel instantiations get internal linkage, unique to this translation unit, so two tiers +// that share a kernel header cannot emit the same weak symbol and let link order pick the +// body. Only this tier's Choose_* entry points stay external. +namespace { #include "VecSim/spaces/IP/IP_NEON_DOTPROD_INT8.h" #include "VecSim/spaces/IP/IP_NEON_DOTPROD_UINT8.h" #include "VecSim/spaces/IP/IP_NEON_DOTPROD_SQ8_SQ8.h" #include "VecSim/spaces/L2/L2_NEON_DOTPROD_INT8.h" #include "VecSim/spaces/L2/L2_NEON_DOTPROD_UINT8.h" #include "VecSim/spaces/L2/L2_NEON_DOTPROD_SQ8_SQ8.h" +} // namespace namespace spaces { diff --git a/src/VecSim/spaces/functions/NEON_FHM.cpp b/src/VecSim/spaces/functions/NEON_FHM.cpp index 2beceed0c..ed92bec47 100644 --- a/src/VecSim/spaces/functions/NEON_FHM.cpp +++ b/src/VecSim/spaces/functions/NEON_FHM.cpp @@ -8,8 +8,23 @@ */ #include "NEON_FHM.h" +// Hoisted above the anonymous namespace below so that the standard library and the shared +// type headers keep external linkage. Wrapping them would pull and friends into +// the anonymous namespace and fail to compile. +#include "VecSim/spaces/space_includes.h" +#include "VecSim/spaces/spaces.h" +#include "VecSim/types/bfloat16.h" +#include "VecSim/types/float16.h" +#include "VecSim/types/sq8.h" +#include + +// Kernel instantiations get internal linkage, unique to this translation unit, so two tiers +// that share a kernel header cannot emit the same weak symbol and let link order pick the +// body. Only this tier's Choose_* entry points stay external. +namespace { #include "VecSim/spaces/IP/IP_NEON_SQ8_FP16.h" #include "VecSim/spaces/L2/L2_NEON_SQ8_FP16.h" +} // namespace namespace spaces { diff --git a/src/VecSim/spaces/functions/NEON_HP.cpp b/src/VecSim/spaces/functions/NEON_HP.cpp index 20d93a517..412963066 100644 --- a/src/VecSim/spaces/functions/NEON_HP.cpp +++ b/src/VecSim/spaces/functions/NEON_HP.cpp @@ -8,10 +8,25 @@ */ #include "NEON_HP.h" +// Hoisted above the anonymous namespace below so that the standard library and the shared +// type headers keep external linkage. Wrapping them would pull and friends into +// the anonymous namespace and fail to compile. +#include "VecSim/spaces/space_includes.h" +#include "VecSim/spaces/spaces.h" +#include "VecSim/types/bfloat16.h" +#include "VecSim/types/float16.h" +#include "VecSim/types/sq8.h" +#include + +// Kernel instantiations get internal linkage, unique to this translation unit, so two tiers +// that share a kernel header cannot emit the same weak symbol and let link order pick the +// body. Only this tier's Choose_* entry points stay external. +namespace { #include "VecSim/spaces/L2/L2_NEON_FP16.h" #include "VecSim/spaces/IP/IP_NEON_FP16.h" #include "VecSim/spaces/IP/IP_NEON_SQ8_FP16.h" #include "VecSim/spaces/L2/L2_NEON_SQ8_FP16.h" +} // namespace namespace spaces { diff --git a/src/VecSim/spaces/functions/SVE.cpp b/src/VecSim/spaces/functions/SVE.cpp index bd197c84c..e306accbb 100644 --- a/src/VecSim/spaces/functions/SVE.cpp +++ b/src/VecSim/spaces/functions/SVE.cpp @@ -8,6 +8,21 @@ */ #include "SVE.h" +// Hoisted above the anonymous namespace below so that the standard library and the shared +// type headers keep external linkage. Wrapping them would pull and friends into +// the anonymous namespace and fail to compile. +#include "VecSim/spaces/space_includes.h" +#include "VecSim/spaces/spaces.h" +#include "VecSim/types/bfloat16.h" +#include "VecSim/types/float16.h" +#include "VecSim/types/sq8.h" +#include +#include + +// Kernel instantiations get internal linkage, unique to this translation unit, so two tiers +// that share a kernel header cannot emit the same weak symbol and let link order pick the +// body. Only this tier's Choose_* entry points stay external. +namespace { #include "VecSim/spaces/L2/L2_SVE_FP32.h" #include "VecSim/spaces/IP/IP_SVE_FP32.h" @@ -30,6 +45,7 @@ #include "VecSim/spaces/IP/IP_SVE_SQ8_SQ8.h" #include "VecSim/spaces/L2/L2_SVE_SQ8_SQ8.h" +} // namespace namespace spaces { diff --git a/src/VecSim/spaces/functions/SVE2.cpp b/src/VecSim/spaces/functions/SVE2.cpp index 9eea81523..d15780d73 100644 --- a/src/VecSim/spaces/functions/SVE2.cpp +++ b/src/VecSim/spaces/functions/SVE2.cpp @@ -8,6 +8,20 @@ */ #include "SVE2.h" +// Hoisted above the anonymous namespace below so that the standard library and the shared +// type headers keep external linkage. Wrapping them would pull and friends into +// the anonymous namespace and fail to compile. +#include "VecSim/spaces/space_includes.h" +#include "VecSim/spaces/spaces.h" +#include "VecSim/types/bfloat16.h" +#include "VecSim/types/float16.h" +#include "VecSim/types/sq8.h" +#include + +// Kernel instantiations get internal linkage, unique to this translation unit, so two tiers +// that share a kernel header cannot emit the same weak symbol and let link order pick the +// body. Only this tier's Choose_* entry points stay external. +namespace { #include "VecSim/spaces/L2/L2_SVE_FP32.h" #include "VecSim/spaces/IP/IP_SVE_FP32.h" @@ -26,6 +40,7 @@ #include "VecSim/spaces/L2/L2_SVE2_SQ8_FP16.h" // SVE2 fast path: FMLALB/FMLALT widening #include "VecSim/spaces/IP/IP_SVE_SQ8_SQ8.h" // SVE2 implementation is identical to SVE #include "VecSim/spaces/L2/L2_SVE_SQ8_SQ8.h" // SVE2 implementation is identical to SVE +} // namespace namespace spaces { diff --git a/src/VecSim/spaces/functions/SVE_BF16.cpp b/src/VecSim/spaces/functions/SVE_BF16.cpp index b457cdb7f..d7b9a6994 100644 --- a/src/VecSim/spaces/functions/SVE_BF16.cpp +++ b/src/VecSim/spaces/functions/SVE_BF16.cpp @@ -8,8 +8,23 @@ */ #include "SVE_BF16.h" +// Hoisted above the anonymous namespace below so that the standard library and the shared +// type headers keep external linkage. Wrapping them would pull and friends into +// the anonymous namespace and fail to compile. +#include "VecSim/spaces/space_includes.h" +#include "VecSim/spaces/spaces.h" +#include "VecSim/types/bfloat16.h" +#include "VecSim/types/float16.h" +#include "VecSim/types/sq8.h" +#include + +// Kernel instantiations get internal linkage, unique to this translation unit, so two tiers +// that share a kernel header cannot emit the same weak symbol and let link order pick the +// body. Only this tier's Choose_* entry points stay external. +namespace { #include "VecSim/spaces/IP/IP_SVE_BF16.h" #include "VecSim/spaces/L2/L2_SVE_BF16.h" +} // namespace namespace spaces { From 0d42fb4f9c6140956eddba55a3598db88e2b54b7 Mon Sep 17 00:00:00 2001 From: Dor Forer Date: Wed, 19 Aug 2026 18:00:34 +0300 Subject: [PATCH 3/3] Mirror the NEON_FHM dispatch predicate in its unit-test guards The three SQ8_FP16 optimization tests gated their FHM branch on optimization.asimdfhm alone, while the dispatcher now requires features.asimdhp && features.asimdfhm. The benchmark registrations already match the dispatcher; these three did not. Harmless in practice, since no core reports asimdfhm without asimdhp, but a test whose guard is looser than the code it tests will not catch the case it looks like it covers. --- tests/unit/test_spaces.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_spaces.cpp b/tests/unit/test_spaces.cpp index 18ef7a625..96456a9c5 100644 --- a/tests/unit/test_spaces.cpp +++ b/tests/unit/test_spaces.cpp @@ -3372,7 +3372,7 @@ TEST_P(SQ8_FP16_SpacesOptimizationTest, SQ8_FP16_L2SqrTest) { } #endif #ifdef OPT_NEON_FHM - if (optimization.asimdfhm) { + if (optimization.asimdhp && optimization.asimdfhm) { unsigned char alignment = 0; arch_opt_func = L2_SQ8_FP16_GetDistFunc(dim, &alignment, &optimization); ASSERT_EQ(arch_opt_func, Choose_SQ8_FP16_L2_implementation_NEON_FHM(dim)) @@ -3498,7 +3498,7 @@ TEST_P(SQ8_FP16_SpacesOptimizationTest, SQ8_FP16_InnerProductTest) { } #endif #ifdef OPT_NEON_FHM - if (optimization.asimdfhm) { + if (optimization.asimdhp && optimization.asimdfhm) { unsigned char alignment = 0; arch_opt_func = IP_SQ8_FP16_GetDistFunc(dim, &alignment, &optimization); ASSERT_EQ(arch_opt_func, Choose_SQ8_FP16_IP_implementation_NEON_FHM(dim)) @@ -3624,7 +3624,7 @@ TEST_P(SQ8_FP16_SpacesOptimizationTest, SQ8_FP16_CosineTest) { } #endif #ifdef OPT_NEON_FHM - if (optimization.asimdfhm) { + if (optimization.asimdhp && optimization.asimdfhm) { unsigned char alignment = 0; arch_opt_func = Cosine_SQ8_FP16_GetDistFunc(dim, &alignment, &optimization); ASSERT_EQ(arch_opt_func, Choose_SQ8_FP16_Cosine_implementation_NEON_FHM(dim))