fix(euclidean): Declare the missing AVX2 extern template for L2 at d=160 - #371
Open
ahuber21 wants to merge 1 commit into
Open
fix(euclidean): Declare the missing AVX2 extern template for L2 at d=160#371ahuber21 wants to merge 1 commit into
ahuber21 wants to merge 1 commit into
Conversation
The AVX2 `extern template` list in euclidean.h listed 200 twice and never listed 160, while avx2.cpp instantiates L2 at 160 like every other extent. With no declaration in the header, a consumer translation unit implicitly instantiates `L2Impl<160, ..., AVX2>` itself, at the consumer's own `-march`. Against the baseline `-march=x86-64` the library is built with, that local copy is a scalar loop (movss/subss/mulss/addss, 50 bytes) rather than the 508-byte AVX2 kernel that is sitting unused in libsvs_x86_objects.a -- so the AVX2 dispatch path silently runs unvectorized code at d=160, and differs between translation units, which is an ODR violation. inner_product.h and cosine.h already list all nine extents correctly; this brings euclidean.h in line with them and with avx2.cpp. Verified by compiling a consumer at -march=x86-64: `L2Impl<160, float, float, AVX2>` goes from a locally defined symbol to an undefined reference resolved from the archive, matching 128 and 200. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ahuber21
changed the title
ahuber21
marked this pull request as ready for review
August 24, 2026 11:42
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
1 participant
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.
The AVX2
extern templatelist ineuclidean.hlisted 200 twice and neverlisted 160, while
avx2.cppinstantiates L2 at 160 like every other extent. Amissing
extern templateis not an error: every consumer that computed an L2distance at d=160 instantiated the kernel itself instead, from the generic
primary template, and so got a scalar loop where the library had a vectorized
AVX2 kernel sitting unused in the archive.
One character of a duplicated line, and the reason the rest of this stack
exists: a hand-maintained list of 182 extents cannot be reviewed by reading it.
Part 1 of 5 of the ISA dispatching v2 milestone.