ggml: fix MSVC linkage conflict on the TurboQuant vec_dot - #6
Open
Pascal-SAPUI5 wants to merge 1 commit into
Open
ggml: fix MSVC linkage conflict on the TurboQuant vec_dot#6Pascal-SAPUI5 wants to merge 1 commit into
Pascal-SAPUI5 wants to merge 1 commit into
Conversation
ggml_vec_dot_turbo3_0 and _turbo4_0 were declared twice: in ggml-quants.h with GGML_API, and in ggml-cpu/quants.h without it. ggml-cpu/quants.c includes both, so MSVC saw one declaration with __declspec and one without and rejected it: ggml-quants.h(112,15): error C2375: 'ggml_vec_dot_turbo3_0': redefinition; different linkage gcc and clang accept this silently, which is why it only ever showed up in the Windows job of an inherited upstream workflow. GGML_API is the correct attribute here, not the spurious one: unlike every other vec_dot, TurboQuant's is defined in ggml-quants.c, which builds into ggml-base, while its only caller is the type-traits table in ggml-cpu — a different shared library. So the declaration without the attribute is the one that had to go, and ggml-cpu.c now takes it from ggml-quants.h directly. Adds a Windows MSVC job to the fork's CI. This class of error is invisible to both existing jobs, and windows-latest is free on a public repo. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q5jMcvdevae8j4T36C9h2z
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 inherited
Serverworkflow has been failing itsserver-windowsjob. It is a real bug in the TurboQuant code, not fork infrastructure:Cause
ggml_vec_dot_turbo3_0/_turbo4_0were declared in two places:ggml/src/ggml-quants.h— withGGML_APIggml/src/ggml-cpu/quants.h— without itggml-cpu/quants.cincludes both. On MSVCGGML_APIexpands to__declspec(dllexport/dllimport), so the two declarations disagree on linkage and the compiler rejects it. gcc and clang accept it silently, so neither of our own CI jobs could see it.Which declaration is wrong
GGML_APIis the correct one. Every othervec_dotis defined inggml-cpu/quants.cand used within the same library, so none of them needs an export attribute. TurboQuant's is the exception: it is defined inggml-quants.c, which builds into ggml-base, and its only caller is the type-traits table inggml-cpu/ggml-cpu.c— a different shared library. Crossing that boundary requires the attribute.So the undecorated declaration is the one that goes, and
ggml-cpu.cnow takes the declaration fromggml-quants.h.(Moving the implementation into
ggml-cpu/quants.cwhere the othervec_dots live would also fix it and match upstream layering better, but it would drag the codebooks and unpack helpers across the same boundary — those are needed by the reference quantizer in ggml-base. Not worth it for this.)Also: a Windows job
Added to the fork's CI. This error class is invisible to both existing jobs, and
windows-latestcosts nothing on a public repo. It buildsggml-cpu— the target that broke — and runstest-turboquant.Verified
test-turboquant23/23🤖 Generated with Claude Code
https://claude.ai/code/session_01Q5jMcvdevae8j4T36C9h2z