Fix i2_s ARM CPU kernel: correct 128/32 weight-group layout (fixes #585) - #586
Fix i2_s ARM CPU kernel: correct 128/32 weight-group layout (fixes #585)#586Alexander Hoffman (EUPHEMEME) wants to merge 1 commit into
Conversation
The ARM CPU i2_s dot-product path decoded weights with a 64-weight / 16-byte group layout (QK_I2_S=64), but the gguf packs i2_s weights in the 128-weight / 32-byte layout used by dequantize_row_i2_s. It therefore read every weight matrix in scrambled order, producing wrong-but-finite output and garbage generation on all ARM CPU inference (Jetson / ARM-Linux, and Apple Silicon with -ngl 0; the default Metal path masks it). Replace the ARM branch of the ggml_vec_dot_i2_i8_s dispatcher with a correct 128/32-layout NEON decode (dotprod + scalar fallback); the x86/AVX path is unchanged. After the fix the CPU forward pass is bit-identical to the Metal/dequant path (modulo the expected int8-vs-f16 activation difference) and generation is coherent. Verified on a Jetson Orin Nano (aarch64). Fixes microsoft#585 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@microsoft-github-policy-service agree |
|
Flagging that this is now an incomplete fix for When this PR was opened, the ARM #elif defined(__ARM_NEON)
#include <arm_neon.h>
#define QK_I2_S 64block now also appears in:
Both carry the identical Happy to extend this PR to cover both submodule sites, or to split them into a separate PR against the submodule repo — whichever you'd prefer. Just let me know. Separately, while testing this I found a live SIGSEGV on any i2_s model at |
Fixes #585.
What
The i2_s ternary CPU dot-product kernel unpacked weights with a 64-weight / 16-byte group layout, but the gguf packs them in the 128-weight / 32-byte layout (the one
dequantize_row_i2_suses). It read every weight matrix in scrambled order → wrong-but-finite output → garbage generation on all ARM CPU inference. On Apple Silicon the default Metal backend hides it (it dequantizes weights, so uses the correct layout); force CPU with-ngl 0and it reproduces, exactly like Jetson / ARM-Linux.Fix
Replace the ARM branch of the
ggml_vec_dot_i2_i8_sdispatcher with a correct 128/32-layout NEON decode (dotprod path + scalar fallback). The x86/AVX path is unchanged.Validation
[3] MUL_MAT "Qcur-0"(first ternary matmul, layer 0). With the fix, the full CPU forward pass is bit-identical to the Metal/dequant path, modulo the expected int8-vs-f16 activation-quantization difference (~0.3–1% per element).BitNet-b1.58-2B-4Ton a Jetson Orin Nano (aarch64, clang 14 / gcc 11) goes from!!!!to coherent English.See #585 for the full op-by-op trace and the in-kernel proof (the NEON arithmetic is correct; only the unpack layout was wrong).
Follow-up (not in this PR): the unused
_1xN/_Nx1/_1x4_32WARM sub-kernels and the ARM branch ofquantize_i2_sstill assume the 64/16 layout — dead on the inference read path for this model, but worth aligning.