cmake: conditionally enable ASM via check_language to fix MSVC/CMake configuration failures#29
Closed
turbolego wants to merge 0 commit into
Conversation
0d1e269 to
d86eb0b
Compare
fukuro-kun
pushed a commit
to fukuro-kun/fukuro-llama-cpp-turboquant
that referenced
this pull request
Jul 5, 2026
…-ai#29 Codex post-commit review found: 1. TURBO_D was QK_TURBO3 (now 32) — broke turbo4 C array sizes 2. SET_ROWS kernel turbo3-specific but instantiated for turbo4 3. Tail block drop for non-128 head dims Fixed AtomicBot-ai#3 (TURBO_D). AtomicBot-ai#1 and AtomicBot-ai#2 don't affect turbo3+dk128 path. Co-Authored-By: tturney@psyguard.ai Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fukuro-kun
pushed a commit
to fukuro-kun/fukuro-llama-cpp-turboquant
that referenced
this pull request
Jul 5, 2026
…ling (Issue AtomicBot-ai#29) Three bugs from the block-size-32 refactor: 1. kernel_set_rows_turbo hardcoded turbo3 packing for turbo4 — split into separate kernel_set_rows_turbo3 and kernel_set_rows_turbo4 kernels. turbo4 now correctly does 3-bit PolarQuant + QJL residual correction. 2. Integer division in n_groups = nk0 / blocks_per_group silently dropped tail blocks for non-128-aligned head dims (e.g. dk=192). Added ceiling division with tail-group bounds checking in turbo3, and GGML_ASSERT in WHT dispatch to catch non-128-aligned tensors. 3. TURBO_D constant was semantically coupled to QK_TURBO4 — replaced with TURBO_ROT_DIM (= QK_TURBO3_GROUP) and added static_assert that QK_TURBO4 == QK_TURBO3_GROUP to guard against future drift. Closes AtomicBot-ai#29 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
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.
Overview
This PR fixes a Windows/MSVC configuration failure in
ggmlcaused by unconditional enabling of theASMlanguage in CMake.On modern CMake versions (CMake 4.x / CMP0194 policy behavior), MSVC is not reliably treated as a valid ASM compiler. This leads to early configuration failures such as:
even when a fully functional Visual Studio toolchain is present.
To address this, ASM support is now enabled conditionally using
check_language(ASM), and only activated when the toolchain actually supports it. On MSVC, ASM is skipped while remaining enabled for toolchains where it is supported (GNU, Clang, NASM, etc.).This preserves ASM optimizations where available while fixing Windows/MSVC build reliability issues without globally disabling ASM.
Additional information
This change avoids forcing ASM via:
Instead, ASM is now enabled only when CMake confirms support:
Why this is needed
Recent CMake versions changed ASM handling under policy CMP0194:
enable_language(ASM)may fail early in configurationSince ggml does not require ASM for CUDA / CPU builds on Windows, disabling ASM conditionally avoids breaking configuration while keeping ASM support intact on supported toolchains.
Impact
Requirements