GGUF kernels: raise on unsupported quant type instead of returning uninitialized memory - #138
Open
vcruz305 wants to merge 1 commit into
Open
GGUF kernels: raise on unsupported quant type instead of returning uninitialized memory#138vcruz305 wants to merge 1 commit into
vcruz305 wants to merge 1 commit into
Conversation
None of the five switch (type) blocks in gguf_kernel.cu had a default: case, and the output tensor is allocated with torch::empty, so an unsupported quant type returned uninitialized memory rather than raising. ggml_moe_get_block_size fell through to return 0, which then divides or sizes a launch by zero downstream. The gap is reachable: ggml_mul_mat_vec_a8 and ggml_moe_a8_vec handle 19 types while ggml_mul_mat_a8 and ggml_moe_a8 handle 10, because the I-quants have no MMQ kernel. Any caller routing an I-quant to the MMQ path gets silent garbage, and the only symptom is a model that loads, runs at full speed, and produces fluent nonsense. Each default: now raises via TORCH_CHECK naming the function and the type families that entry point supports. ggml_dequantize also null-checks the ggml_get_to_cuda function pointer, which returns nullptr for an unknown type and was being called unconditionally. No kernel math changed. A comment above the first guard notes these kernels are vendored from sgl-kernel so a future re-vendor does not drop them.
|
Failure exists on any backend where an unsupported quant path falls through, +1 |
vcruz305
added a commit
to vcruz305/FreeToken
that referenced
this pull request
Aug 26, 2026
Three defects with one root: "gguf" is a container tag, not a weight layout. The checkpoint picks a ggml type per tensor and the concrete CPU format has to be recovered from the bank types, but two call sites tested the tag directly. _cpu_moe_executor_viable compared expert_quant against _WFMT_IDS, which answers False for EVERY GGUF checkpoint. That silently disabled the automatic residency split on hosts where CUDA pinning is quota-capped -- WSL caps it near 40% of RAM (measured: 81.78 GiB of 204). The symptom was not a clear refusal but cudaHostRegister failing partway through the banks, which reads as a memory shortage rather than a dispatch gap. gemm1_dot handled bf16 and q4_0 and then FELL THROUGH to the NVFP4 path, which dereferences scale/global pointers that are null for GGUF banks. An unhandled format therefore segfaulted inside a worker thread with no Python traceback. It now raises through TORCH_CHECK naming the format, same reasoning as the kernel default: guards in FlashML-org#138: an unhandled case that reads null or uninitialised memory is far worse than one that errors. GGUFEmbedding dequantized unconditionally, but the unquantized types are raw value bytes with no dequant kernel at all (ggml_dequantize rejects type 1 outright). DeepSeek-V4 ships token_embd as F16 and died on the first lookup. The gathered rows are now reinterpreted for those types, matching the fix already applied to fused_mul_mat_gguf.
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.
Splitting this out of #131 because it stands on its own and is quick to review. It is a correctness fix with no feature attached.
None of the five
switch (type)blocks ingguf_kernel.cuhas adefault:case, and the output tensor is allocated withtorch::empty. So passing a quant type the kernel does not implement returns uninitialized memory instead of raising:ggml_moe_get_block_sizehas the same shape and falls through toreturn 0, which then divides or sizes a launch by zero downstream.The gap is real rather than theoretical.
ggml_mul_mat_vec_a8andggml_moe_a8_vechandle 19 types, whileggml_mul_mat_a8andggml_moe_a8handle 10: the I-quants have no MMQ kernel. So any caller that routes an I-quant to the MMQ path today gets silent garbage, and the only symptom is a model that loads fine, runs at full speed and emits fluent nonsense. I lost a fair amount of time to exactly that before adding these guards.Each
default:now raises throughTORCH_CHECKand names the function plus which type families that entry point actually supports. I also null-check theggml_get_to_cudafunction pointer insideggml_dequantize, since it returnsnullptrfor an unknown type (dequantize.cuh) and the result was being called unconditionally.No kernel math is touched. The only behaviour change is that an unsupported type now fails loudly at the call instead of producing wrong numbers. There is a comment above the first guard noting these kernels are vendored from sgl-kernel and that the guards are a local addition, so a future re-vendor does not quietly drop them.
Found while working on #131 (GGUF quant types and qwen35moe), but it is independent of that branch.