[MOD-17844] Stop ARM SIMD tiers executing instructions the running CPU lacks - #1018
Open
dor-forer wants to merge 4 commits into
Open
[MOD-17844] Stop ARM SIMD tiers executing instructions the running CPU lacks#1018dor-forer wants to merge 4 commits into
dor-forer wants to merge 4 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Please upload reports for the commit 60c9c60 to get more accurate results. Additional details and impacted files@@ Coverage Diff @@
## main #1018 +/- ##
=======================================
Coverage 97.18% 97.18%
=======================================
Files 141 141
Lines 8420 8420
=======================================
Hits 8183 8183
Misses 237 237 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
dor-forer
force-pushed
the
MOD-17844-arm-simd-isa-safety
branch
from
August 20, 2026 08:35
f527661 to
f2757e4
Compare
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.
dor-forer
force-pushed
the
MOD-17844-arm-simd-isa-safety
branch
from
August 20, 2026 11:11
f2757e4 to
0d42fb4
Compare
dor-forer
added a commit
that referenced
this pull request
Aug 20, 2026
`FP16_L2Sqr` and `FP16_InnerProduct` widen each stored half with FP16_to_FP32 and accumulate into a `float`. Four SIMD tiers did not: AVX512FP16 kept `__m512h sum` and reduced into a `_Float16`, NEON kept `float16x8_t acc` with `vfmaq_f16`, and SVE kept `svfloat16_t acc` with `svmla_f16_x`. On any CPU that selects one of those tiers the whole vector was summed in an 11-bit mantissa, so the same function returned a different answer depending on the machine. Two consequences, both silent. Precision: simulating both accumulation orders over 20,000 random 128-dimension fp16 vectors gives a maximum relative error of 5.6e-3 for the fp16 accumulator against 1.9e-7 for the fp32 one, so nearest neighbours and their ordering change. Overflow: 65504 is the largest finite fp16 value, so 32 elements of 200.0, all ordinary fp16 values, drive a half precision accumulator past it and the result becomes infinity. All four tiers now widen and accumulate in fp32. The x86 kernels mirror L2_AVX512F_FP16.h and IP_AVX512F_FP16.h step for step, since after widening there is nothing half-precision-specific left to do differently; they also gain the second accumulator that #984 added to the sibling fp16 tiers and not to these. NEON and SVE keep their existing four-way unrolling, with each accumulator becoming a pair covering the lower and upper halves of a register. The unit tests could not have caught this. Both fp16 baselines accumulated the reference in `_Float16` too, so the test compared a half precision kernel against a half precision reference and passed within its 1% tolerance whatever the kernel did. They now accumulate in `float` via FP16_to_FP32, mirroring the scalar functions exactly. That alone is still not a regression test: the randomized cases draw values from [-0.99, 0.99], where a half precision accumulator's worst error over dim 32..256 is about 0.54%, inside the 1% budget. FP16SpacesTest.LargeValuesDoNotOverflowTheAccumulator closes that gap with inputs whose expected totals are exact in fp32 and infinite in fp16, and it calls the public choosers so whichever tier the running CPU selects is tested. The ARM half of this change depends on the NEON_HP/NEON_FHM translation unit split from #1018. Widening to fp32 and then issuing vfmaq_f32 is exactly the pattern gcc contracts into FMLAL, so in a translation unit compiled with +fp16fml the fix would emit FMLAL into the plain half-precision path and fault on any core without FEAT_FHM. Measured on gcc 12: the NEON kernels compile to 4 FMLAL at -march=armv8.2-a+fp16fml and 0 at +fp16. With #1018 the HP tier is compiled at +fp16 only, and NEON_HP.cpp.o contains no FMLAL. Verified on an AWS Graviton2 (Neoverse-N1, asimdhp without asimdfhm, gcc 12), which executes the NEON path: the full spaces suite passes 1529/1529 with this change on top of #1019, where the same change on a main base fails 106 tests with SIGILL. On x86 (Ice Lake, gcc 13) the suite passes 1569/1569, and the AVX512FP16 kernels compile with -mavx512fp16 -Werror leaving no fmadd*ph, subph or mulph. The AVX512FP16 tier itself needs Sapphire Rapids or later to execute and the SVE tier needs an SVE core, so neither runs on the hardware available here; both are covered by CI.
2 tasks
dor-forer
added a commit
that referenced
this pull request
Aug 23, 2026
`FP16_L2Sqr` and `FP16_InnerProduct` widen each stored half with FP16_to_FP32 and accumulate into a `float`. Four SIMD tiers did not: AVX512FP16 kept `__m512h sum` and reduced into a `_Float16`, NEON kept `float16x8_t acc` with `vfmaq_f16`, and SVE kept `svfloat16_t acc` with `svmla_f16_x`. On any CPU that selects one of those tiers the whole vector was summed in an 11-bit mantissa, so the same function returned a different answer depending on the machine. Two consequences, both silent. Precision: simulating both accumulation orders over 20,000 random 128-dimension fp16 vectors gives a maximum relative error of 5.6e-3 for the fp16 accumulator against 1.9e-7 for the fp32 one, so nearest neighbours and their ordering change. Overflow: 65504 is the largest finite fp16 value, so 32 elements of 200.0, all ordinary fp16 values, drive a half precision accumulator past it and the result becomes infinity. All four tiers now widen and accumulate in fp32. The x86 kernels mirror L2_AVX512F_FP16.h and IP_AVX512F_FP16.h step for step, since after widening there is nothing half-precision-specific left to do differently; they also gain the second accumulator that #984 added to the sibling fp16 tiers and not to these. NEON and SVE keep their existing four-way unrolling, with each accumulator becoming a pair covering the lower and upper halves of a register. The unit tests could not have caught this. Both fp16 baselines accumulated the reference in `_Float16` too, so the test compared a half precision kernel against a half precision reference and passed within its 1% tolerance whatever the kernel did. They now accumulate in `float` via FP16_to_FP32, mirroring the scalar functions exactly. That alone is still not a regression test: the randomized cases draw values from [-0.99, 0.99], where a half precision accumulator's worst error over dim 32..256 is about 0.54%, inside the 1% budget. FP16SpacesTest.LargeValuesDoNotOverflowTheAccumulator closes that gap with inputs whose expected totals are exact in fp32 and infinite in fp16, and it calls the public choosers so whichever tier the running CPU selects is tested. The ARM half of this change depends on the NEON_HP/NEON_FHM translation unit split from #1018. Widening to fp32 and then issuing vfmaq_f32 is exactly the pattern gcc contracts into FMLAL, so in a translation unit compiled with +fp16fml the fix would emit FMLAL into the plain half-precision path and fault on any core without FEAT_FHM. Measured on gcc 12: the NEON kernels compile to 4 FMLAL at -march=armv8.2-a+fp16fml and 0 at +fp16. With #1018 the HP tier is compiled at +fp16 only, and NEON_HP.cpp.o contains no FMLAL. Verified on an AWS Graviton2 (Neoverse-N1, asimdhp without asimdfhm, gcc 12), which executes the NEON path: the full spaces suite passes 1529/1529 with this change on top of #1019, where the same change on a main base fails 106 tests with SIGILL. On x86 (Ice Lake, gcc 13) the suite passes 1569/1569, and the AVX512FP16 kernels compile with -mavx512fp16 -Werror leaving no fmadd*ph, subph or mulph. The AVX512FP16 tier itself needs Sapphire Rapids or later to execute and the SVE tier needs an SVE core, so neither runs on the hardware available here; both are covered by CI.
dor-forer
added a commit
that referenced
this pull request
Aug 23, 2026
`FP16_L2Sqr` and `FP16_InnerProduct` widen each stored half with FP16_to_FP32 and accumulate into a `float`. Four SIMD tiers did not: AVX512FP16 kept `__m512h sum` and reduced into a `_Float16`, NEON kept `float16x8_t acc` with `vfmaq_f16`, and SVE kept `svfloat16_t acc` with `svmla_f16_x`. On any CPU that selects one of those tiers the whole vector was summed in an 11-bit mantissa, so the same function returned a different answer depending on the machine. Two consequences, both silent. Precision: simulating both accumulation orders over 20,000 random 128-dimension fp16 vectors gives a maximum relative error of 5.6e-3 for the fp16 accumulator against 1.9e-7 for the fp32 one, so nearest neighbours and their ordering change. Overflow: 65504 is the largest finite fp16 value, so 32 elements of 200.0, all ordinary fp16 values, drive a half precision accumulator past it and the result becomes infinity. All four tiers now widen and accumulate in fp32. The x86 kernels mirror L2_AVX512F_FP16.h and IP_AVX512F_FP16.h step for step, since after widening there is nothing half-precision-specific left to do differently; they also gain the second accumulator that #984 added to the sibling fp16 tiers and not to these. NEON and SVE keep their existing four-way unrolling, with each accumulator becoming a pair covering the lower and upper halves of a register. The unit tests could not have caught this. Both fp16 baselines accumulated the reference in `_Float16` too, so the test compared a half precision kernel against a half precision reference and passed within its 1% tolerance whatever the kernel did. They now accumulate in `float` via FP16_to_FP32, mirroring the scalar functions exactly. That alone is still not a regression test: the randomized cases draw values from [-0.99, 0.99], where a half precision accumulator's worst error over dim 32..256 is about 0.54%, inside the 1% budget. FP16SpacesTest.LargeValuesDoNotOverflowTheAccumulator closes that gap with inputs whose expected totals are exact in fp32 and infinite in fp16, and it calls the public choosers so whichever tier the running CPU selects is tested. The ARM half of this change depends on the NEON_HP/NEON_FHM translation unit split from #1018. Widening to fp32 and then issuing vfmaq_f32 is exactly the pattern gcc contracts into FMLAL, so in a translation unit compiled with +fp16fml the fix would emit FMLAL into the plain half-precision path and fault on any core without FEAT_FHM. Measured on gcc 12: the NEON kernels compile to 4 FMLAL at -march=armv8.2-a+fp16fml and 0 at +fp16. With #1018 the HP tier is compiled at +fp16 only, and NEON_HP.cpp.o contains no FMLAL. Verified on an AWS Graviton2 (Neoverse-N1, asimdhp without asimdfhm, gcc 12), which executes the NEON path: the full spaces suite passes 1529/1529 with this change on top of #1019, where the same change on a main base fails 106 tests with SIGILL. On x86 (Ice Lake, gcc 13) the suite passes 1569/1569, and the AVX512FP16 kernels compile with -mavx512fp16 -Werror leaving no fmadd*ph, subph or mulph. The AVX512FP16 tier itself needs Sapphire Rapids or later to execute and the SVE tier needs an SVE core, so neither runs on the hardware available here; both are covered by CI.
GuyAv46
reviewed
Aug 23, 2026
Comment on lines
+11
to
+23
| // Hoisted above the anonymous namespace below so that the standard library and the shared | ||
| // type headers keep external linkage. Wrapping them would pull <cstring> 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 <arm_neon.h> | ||
|
|
||
| // 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. |
Collaborator
There was a problem hiding this comment.
Consider renaming the problematic functions instead of adding these includes everywhere
Collaborator
Author
There was a problem hiding this comment.
You were right, and more right than I realised. Done in 36042c9, which drops the linkage commit entirely and replaces it with renaming.
Measuring which tier pairs actually collide, on this PR's base commit:
| pair | shared external symbols |
|---|---|
SVE.o / SVE2.o |
152 |
| the other 27 ARM pairs | 0 |
| all 105 x86 pairs | 0 |
So the change to NEON.cpp you were reading, and the ones to NEON_BF16.cpp, NEON_DOTPROD.cpp, NEON_HP.cpp, NEON_FHM.cpp and SVE_BF16.cpp, fixed nothing. Six of the eight TUs I touched had no collision to fix. NEON.cpp was a reasonable place to ask.
I should also correct the description I had written: it claimed NEON.o and NEON_DOTPROD.o shared 93 symbols. They share 0, and not because main moved under the branch. #1014 and #1015 are both ancestors of this PR's base 7a5fe7f9, so the renames they did had already removed that collision before I opened this. The number was stale when I wrote it. Scenario 3 in the description depended on it and is gone too.
On renaming: the leaf kernels already carry their ISA (INT8_InnerProductSIMD16_NEON vs _NEON_DOTPROD), so there was nothing misnamed to fix. The one real collision is that SVE2.cpp includes fourteen of SVE.cpp's headers on purpose and recompiles them at armv9-a+sve2, so it is one source text producing two different bodies under one name. The rename has to happen at the include site, which is 19 #defines at the top of SVE2.cpp:
#define FP32_InnerProductSIMD_SVE FP32_InnerProductSIMD_SVE2
#define FP32_L2SqrSIMD_SVE FP32_L2SqrSIMD_SVE2
...Each one covers both the definition in the header and the use in the Choose_* body below, since those reference the same names. 1 file, +26/-15, instead of 8 files and +190.
Your suggestion also turned out to fix something the anonymous namespace did not make visible. SVE.cpp.o precedes SVE2.cpp.o in the archive and first-in-link-order wins, so on main today every Choose_*_SVE2 except the three SQ8_FP16 ones is dispatching to armv8-a+sve bodies. The SVE2 tier is selected on SVE2 hardware and runs base-SVE codegen. The linked FP16_L2Sqr_SVE<false,0> is SVE.cpp.o's 51 instructions, not SVE2.cpp.o's 49. Internal linkage would have kept the right body but left the name lying about which -march built it; renaming makes it say so.
And the best argument for your approach was already in this PR. NEON_HP.cpp and NEON_FHM.cpp include the same two SQ8_FP16 headers at +fp16 and +fp16fml, which is structurally the identical hazard, and they share 0 symbols with no linkage tricks at all, purely because the kernels inside are named _NEON_HP and _NEON_FHM. Defect 1's own fix already used naming. SVE2.cpp was the only file not following the convention.
The one thing the boilerplate did buy was covering headers nobody had thought about yet, so I replaced that with a check instead: tests/unit/check_tier_linkage.py, run from ctest as tier_linkage. It nms the archive and fails if any two tier objects define a symbol in common. Architecture-agnostic, passes on x86 (15 objects, 105 pairs, 0 shared), and if I strip the renames back out it fails with all 152 symbols named. So the invariant is enforced rather than remembered, and it protects the NEON tiers without putting anything in them.
Neoverse, gcc 12: full build clean, 1529/1529.
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.
SVE2.cpp recompiles fourteen of SVE.cpp's kernel headers, at -march=armv9-a+sve2 rather than -march=armv8-a+sve. The kernels are templates at namespace scope, so both objects define the same mangled symbols holding bodies built for different architectures, and the linker keeps whichever object it saw first. Nothing in the source decides which. SVE.cpp.o precedes SVE2.cpp.o in the archive, so today the SVE bodies win and every Choose_*_SVE2 entry point except the three SQ8_FP16 ones dispatches to armv8-a+sve code. The SVE2 tier is selected on SVE2 hardware and runs base-SVE codegen. Measured on Neoverse with gcc 12: 152 shared symbols, all 152 bodies differing, and the linked body for FP16_L2Sqr_SVE<false,0> is SVE.cpp.o's 51 instructions rather than SVE2.cpp.o's 49. This is latent rather than fatal only because none of the 152 bodies currently contains an SVE2-only opcode: diffing the mnemonics of every shared body between the two objects gives the empty set in both directions. It becomes a SIGILL on an SVE-without-SVE2 core the first time the compiler emits one, and because link order alone picks the winner it can differ between builds of the same commit. Renaming this tier's instantiations to _SVE2 makes the two sets distinct, which is how every other pair of tiers sharing a header already stays safe: NEON_HP.cpp and NEON_FHM.cpp include the same two SQ8_FP16 headers at +fp16 and +fp16fml and share zero symbols, purely because the kernels inside carry the tier in their names. The neighbouring comments claimed the SVE2 implementation was identical to SVE, which is what made the duplicate compilation look free. Corrected to say the source is recompiled at SVE2 flags. Also adds tests/unit/check_tier_linkage.py, run from ctest as tier_linkage, asserting that no two tier objects in libVectorSimilaritySpaces.a define a symbol in common. It covers x86 as well: 15 tier objects, 105 pairs, 0 shared. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
force-pushed
the
MOD-17844-arm-simd-isa-safety
branch
from
August 23, 2026 14:16
0d42fb4 to
36042c9
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 36042c9. Configure here.
SQ8_SQ8_InnerProductSIMD_SVE_IMP is a non-static template in IP_SVE_SQ8_SQ8.h, called by name from the wrapper alongside it, so both SVE.cpp and SVE2.cpp instantiate it and it needs the same rename as its SQ8_FP32 sibling. Found by Cursor Bugbot. It escaped the first pass because that pass derived the list of names to rename from the symbols nm reported as shared in a release build, and at -O3 this helper is fully inlined into its callers, so it emits no out-of-line symbol at all: 0 in both objects, against 8 each for SQ8_FP32_InnerProductSIMD_SVE_IMP. Reading the declarations rather than the binary finds 20 externally linked templates in the fourteen headers the two tiers share, and the first pass covered 19. A debug build confirms the collision is real rather than theoretical: at -O0 nothing is inlined away and the helper appears 8 times in each object, and check_tier_linkage.py fails on that archive. The bodies happen to be byte-identical there, so nothing miscompiles today, but it is the same latent class as the rest of this change. This also shows the linkage check has a blind spot on release builds, since a kernel the optimiser inlines away cannot be seen in the symbol table. Running it against a debug archive closes that. Doing so needs two further cleanups first, both verified byte-identical between the tiers and so benign today: eight step helpers in the SVE kernel headers are plain `inline` rather than the `static inline` the NEON headers use, and vecsim_types::float16::cvt is a member function that cannot take internal linkage the same way. Left out of this change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe the changes in the pull request
Two defects that let an ARM SIMD tier execute instructions the running CPU does not have.
functions/NEON_HP.cppwas one translation unit compiled with-march=armv8.2-a+fp16fml, holding both the HP-only and the FHM entry points. The whole TU therefore carried the fp16fml licence, and the compiler contracted the HP path'svcvt_f32_f16plusvfmaq_f32intofmlal, even though the HP source uses no FMLAL intrinsic. That path is dispatched onfeatures.asimdhpalone, so on a core withasimdhpbut withoutasimdfhmit is a SIGILL, reachable from any SQ8_FP16 IP, L2 or Cosine query. Measured on Neoverse with gcc 12: 32 AdvSIMD FMLAL instructions in the HP-only wrappers at+fp16fml, 0 at+fp16, 96 across the whole TU.Fixed by splitting into two tiers:
NEON_HPat+fp16predicated onasimdhp, and a newNEON_FHMat+fp16fmlpredicated onasimdhp && asimdfhm. Tightening the predicate to require both bits instead would have deleted the HP-only fallback for exactly the CPUs that need it. The FHM tier still emits 64 FMLAL instructions, so its fast path is unchanged.SVE2.cpprecompiles fourteen ofSVE.cpp's kernel headers, at-march=armv9-a+sve2rather than-march=armv8-a+sve. The kernels are templates at namespace scope with nostatic, so each instantiation is a weak COMDAT symbol by language rule, and both objects emit the same mangled name holding bodies compiled under different-march. The linker keeps whichever comes first in link order. Nothing in the source decides which.Fixed by renaming this tier's instantiations to
_SVE2with 19#defines at the top ofSVE2.cpp, each covering both the definition in the header and the use in theChoose_*body. That is how every other pair of tiers sharing a header already stays safe, including the pair added by defect 1 above:NEON_HP.cppandNEON_FHM.cppinclude the same two SQ8_FP16 headers at+fp16and+fp16fmland share zero symbols, purely because the kernels inside carry the tier in their names.Defect 1 is reproduced, not just inferred. On an AWS Graviton2 (Neoverse-N1,
asimdhppresent andasimdfhmabsent, gcc 12), a program that callsL2_SQ8_FP16_GetDistFunc/IP_.../Cosine_...witharch_opt = nullptr, so the CPU's own feature bits choose the tier, dies withIllegal instruction (core dumped), exit 132, on the first call. On this branch the same binary returns from all three. The tier objects on that same box:NEON_HP.cpp.ocarries 96fmlalon main and 0 here, with the 64 in the newNEON_FHM.cpp.oreachable only whenasimdfhmis set.Scope of defect 2, measured
SVE.cpp.oandSVE2.cpp.oshare 152 externally defined symbols on this PR's base commit. Every one of the other 27 ARM tier pairs shares 0, and so do all 105 x86 tier pairs, because those kernel names embed the ISA. So the collision is confined to one file,SVE2.cpp, which is the only TU that includes another tier's kernel headers.What defect 2 costs today, and what it risks
Today it is a silent downgrade rather than a crash.
SVE.cpp.oprecedesSVE2.cpp.oin the archive and first-in-link-order wins, so the SVE bodies survive and everyChoose_*_SVE2entry point except the three SQ8_FP16 ones dispatches toarmv8-a+svecode. The SVE2 tier is selected on SVE2 hardware and runs base-SVE codegen. Confirmed by linking a probe: the survivingFP16_L2Sqr_SVE<false,0>body isSVE.cpp.o's 51 instructions, notSVE2.cpp.o's 49.It stays non-fatal only because none of the 152 bodies currently contains an SVE2-only opcode. All 152 differ between the two objects, but diffing the instruction mnemonics of every shared body gives the empty set in both directions, so the difference is scheduling and register allocation only. It becomes a SIGILL on an SVE-without-SVE2 core the first time gcc emits one, whether from a compiler upgrade or an edit to any shared SVE kernel. And because link order alone picks the winner, it can differ between builds of the same commit.
The neighbouring comments in
SVE2.cppclaimed the SVE2 implementation was identical to SVE, which is what made the duplicate compilation look free. Corrected to say the source is recompiled at SVE2 flags.Regression test
tests/unit/check_tier_linkage.py, run from ctest astier_linkage,nmslibVectorSimilaritySpaces.aand asserts that no two tier objects define a symbol in common. It is architecture-agnostic and passes on x86 as well (15 tier objects, 105 pairs, 0 shared). Removing the renames fromSVE2.cppmakes it fail with all 152 symbols named, so a new tier or a newly shared kernel header cannot reintroduce the collision unnoticed.Validation
Neoverse (gcc 12), this branch: full build clean, spaces suite plus the new check 1529/1529, all 28 ARM tier pairs sharing 0 symbols,
NEON_HP.cpp.oat 0fmlalandNEON_FHM.cpp.oat 64, andSVE2.cpp.onow emitting its own_SVE2bodies. Thetier_linkagecheck also passes against an x86_64 release archive. CI covers the rest of the x86 suite.Which issues this PR fixes
Main objects this PR modified
src/VecSim/spaces/functions/NEON_HP.{cpp,h}and newNEON_FHM.{cpp,h}src/VecSim/spaces/{IP,L2}_space.cppdispatch sites for SQ8_FP16cmake/aarch64InstructionFlags.cmakeandsrc/VecSim/spaces/CMakeLists.txtfor the new tiersrc/VecSim/spaces/functions/SVE2.cpptests/unit/check_tier_linkage.py,tests/unit/CMakeLists.txt,tests/unit/test_spaces.cppandtests/benchmark/spaces_benchmarks/Mark if applicable
Note
High Risk
Touches ARM distance-function dispatch on the query path. Wrong feature gating previously caused SIGILL on Graviton2 and silent SVE2 downgrades via link-order symbol collisions.
Overview
Fixes two ARM SIMD-tier bugs that could run instructions the host CPU does not have.
NEON HP vs FHM:
NEON_HP.cppwas compiled with+fp16fml, so gcc fused the HP-only SQ8↔FP16 path intofmlal. That path is selected onasimdhpalone, which SIGILLs on cores withoutasimdfhm. HP now builds at+fp16; FHM kernels move to a newNEON_FHMTU at+fp16fml, dispatched only when bothasimdhpandasimdfhmare set.SVE vs SVE2:
SVE2.cpprecompiled the same namespace-scope kernel templates asSVE.cppunder a different-march, so both objects defined the same symbols and link order picked the body. Instantiations are renamed to_SVE2so each tier keeps its own codegen.Adds
check_tier_linkage.py(ctesttier_linkage) so no two per-ISA objects share an external symbol, plus matching unit/benchmark dispatch updates.Reviewed by Cursor Bugbot for commit 60c9c60. Bugbot is set up for automated code reviews on this repo. Configure here.