opencoti F5 bug-226 — block-aware ggml-cpu concat for quantized types At 32k+q8_0 KV with M2 head split (--headinfer-gpu-heads-frac < 1.0) without NEO orchestration, the M2 head-axis concat path in get_k/get_v (llama-kv-cache.cpp:1881-1910) produced semantically corrupt K/V that the downstream flash-attn read as garbage, scoring 0.00% on RULER vt with completely wrong output (e.g. predicting 'SHRUU' instead of the five expected variables). 5-cell bisection at 32k+q8_0 isolated M2 alone (--headinfer-gpu-heads-frac 0.5) as the failing config; M0, M1, M3, and vanilla scored 100. Root cause: ggml_concat on block-quantized tensors falls through multiple broken paths. The M2 non-NEO get_k/get_v branch returns: ggml_concat(ctx, ggml_view_4d(k_s, head_dim, n_head_gpu, n_kv, 1), ggml_view_4d(kc_s, head_dim, n_head_cpu, n_kv, 1), 1) both views are q8_0. The CUDA backend rejects this (concat.cu:165-167 asserts F32-only — task #253's earlier supports_op tightening was correct), so the scheduler dispatches to CPU. CPU concat falls into ggml_compute_forward_concat_any (ops.cpp:1894-1935) for any non- F16/BF16/I16/I8/F32/I32 type. concat_any iterates i0 over LOGICAL elements (0..ne0) with stride nb00, calling memcpy(len = type_size). For q8_0, nb00 = type_size = 36 bytes (one block of 32 elements), but the code treats it as a per-element stride — reading blocks at offsets 0, 36, 72, ..., 127·36 = 4572 bytes from each row start, well past the actual row of (ne0/32)·36 = 144 bytes. Result: q8_0 blocks scrambled into uninterpretable data → FA reads garbage K → output is task-format-correct but value-wrong. At f16 KV (block size 1) this latent bug never triggered because concat dispatches to ggml_compute_forward_concat_f16 which correctly handles fp16 per-element. The full F5 stack at f16 + 32k scored 100 in the morning RULER comparison, masking the q8_0 case. Fix: a new ggml_compute_forward_concat_q that copies entire dim-0 rows at once for block-quantized types. Dispatched from ggml_compute_forward_concat's default branch when ggml_blck_size > 1. For dim != 0 (the M2 head-axis case): each (i1, i2, i3) maps to either src0 or src1 (mutually exclusive); one memcpy of (ne0/blck)·type_size bytes per row. For dim == 0: dst's dim-0 row = src0_row || src1_row, asserted block-aligned at the split; two memcpys per row. Non-block-quantized (F32/F16/BF16/I8/I16/I32) paths unchanged. The ggml-cuda CUDA path stays F32-only — that's correct since this fix lands at the CPU fallback the scheduler already routes to. Lives in ggml-cpu only — no CUDA DSO rebuild needed. Compatible with incremental cosmocc rebuild via `OPENCOTI_NO_CCACHE=1 bun run build:llamafile:make`. Verification: - T2 config smoke (M2-only, no NEO, q8_0, 32k, vt n=1) — bug repro pre-fix scored 0.00% with pred='SHRUU'; post-fix expected 100.00%. - Full F5 stack (M0+M1+M2+M3+NEO+Phase4) at q8_0 + 32k — unchanged from morning baseline (100%). - All M0/M1/M2/M3/M5 unified-mode regression-shield benches — unchanged (cosine ≥ 0.999); concat_q only fires on block-quantized types, and f16/bf16/f32 KV paths route to their own concat_fN as before. apply order: after 0034-m2-pinned-host (this is a ggml-cpu fix; the two are independent so order is bookkeeping only). Co-Authored-By: Claude Opus 4.7 (1M context) diff --git a/llama.cpp/ggml/src/ggml-cpu/ops.cpp b/llama.cpp/ggml/src/ggml-cpu/ops.cpp --- a/llama.cpp/ggml/src/ggml-cpu/ops.cpp +++ b/llama.cpp/ggml/src/ggml-cpu/ops.cpp @@ -2078,6 +2078,119 @@ static void ggml_compute_forward_concat_f32( } } +// opencoti F5 bug-226 fix (0035) — block-quantized concat. +// +// concat_any (above) does per-logical-element memcpy(len = type_size), +// stepping i0 from 0..ne0 with stride nb00. For block-quantized types +// (q8_0, q4_0, …) the parent tensor stores ne0 LOGICAL elements packed +// in ne0/blck blocks of type_size bytes each, with nb00 = type_size +// (one block, not one element). concat_any's loop therefore reads +// blocks at offsets 0, type_size, 2·type_size, … up to ne0·type_size — +// well past the actual row of (ne0/blck)·type_size bytes. The result +// is scrambled block data, which the downstream FA reads as garbage K +// (root cause of bug-226 at M2 head-axis q8_0 K/V concat). +// +// Fix: for block-quantized types, copy entire dim-0 rows at once. +// Each dim-0 row of src0/src1/dst is contiguous (nb00 = type_size, +// ne0/blck blocks per row, no gaps), so a single memcpy per +// (i1, i2, i3) selection on each source captures the right bytes. +// +// - dim != 0 (M2 head-axis case): per (i1, i2, i3), the whole dim-0 +// row comes from src0 or src1 (mutually exclusive). One memcpy. +// - dim == 0 (split on dim 0): dst's dim-0 row = src0's dim-0 row || +// src1's dim-0 row, concatenated head-to-tail. The split MUST land +// on a block boundary (ne00 % blck == 0); when it does, two memcpys. +// +// The M2 split in llama-kv-cache.cpp:get_k/get_v concats on dim 1 +// (head axis), so the dim != 0 branch is the hot path. dim 0 is +// covered for completeness — any future quantized-feature-axis split +// pattern lands correctly without re-discovering this bug. +static void ggml_compute_forward_concat_q( + const ggml_compute_params * params, + ggml_tensor * dst) { + + const ggml_tensor * src0 = dst->src[0]; + const ggml_tensor * src1 = dst->src[1]; + + const size_t type_size = ggml_type_size(src0->type); + const int64_t blck = ggml_blck_size(src0->type); + + GGML_ASSERT(blck > 1 && "concat_q is for block-quantized types"); + GGML_ASSERT(ggml_type_size(src1->type) == type_size); + GGML_ASSERT(ggml_blck_size(src1->type) == blck); + GGML_ASSERT(ggml_type_size(dst->type) == type_size); + GGML_ASSERT(ggml_blck_size(dst->type) == blck); + + const int ith = params->ith; + const int nth = params->nth; + + GGML_TENSOR_BINARY_OP_LOCALS + + const int32_t dim = ggml_get_op_params_i32(dst, 0); + + GGML_ASSERT(dim >= 0 && dim < 4); + + // Both srcs must have block-aligned dim-0 sizes — guaranteed by ggml + // for any well-formed quantized tensor. We assert anyway because if + // a caller violates this, the row math below silently corrupts data. + GGML_ASSERT((ne00 % blck) == 0); + GGML_ASSERT((ne10 % blck) == 0); + + // Bytes per dim-0 row of each source / dst. + const size_t row_bytes_src0 = (ne00 / blck) * type_size; + const size_t row_bytes_src1 = (ne10 / blck) * type_size; + const size_t row_bytes_dst = (ne0 / blck) * type_size; + + if (dim == 0) { + // dst row = src0 row (head) || src1 row (tail). Both fill at the + // block boundary. For non-concat dims i1/i2/i3 must match between + // src0 and src1 (ggml concat invariant), so no per-dim mux logic. + GGML_ASSERT(row_bytes_src0 + row_bytes_src1 == row_bytes_dst); + for (int i3 = 0; i3 < ne3; i3++) { + for (int i2 = ith; i2 < ne2; i2 += nth) { + for (int i1 = 0; i1 < ne1; i1++) { + const char * x0 = (const char *)src0->data + + i1*nb01 + i2*nb02 + i3*nb03; + const char * x1 = (const char *)src1->data + + i1*nb11 + i2*nb12 + i3*nb13; + char * y = (char *)dst->data + + i1*nb1 + i2*nb2 + i3*nb3; + memcpy(y, x0, row_bytes_src0); + memcpy(y + row_bytes_src0, x1, row_bytes_src1); + } + } + } + return; + } + + // dim != 0: each (i1, i2, i3) maps to either src0 or src1, never both. + // row_bytes is shared (src0/src1/dst all have the same dim-0 row). + GGML_ASSERT(row_bytes_src0 == row_bytes_src1); + GGML_ASSERT(row_bytes_src0 == row_bytes_dst); + + int64_t o[4] = {0, 0, 0, 0}; + o[dim] = src0->ne[dim]; + + // TODO: smarter multi-threading + for (int i3 = 0; i3 < ne3; i3++) { + for (int i2 = ith; i2 < ne2; i2 += nth) { + for (int i1 = 0; i1 < ne1; i1++) { + const char * x; + if (i1 < ne01 && i2 < ne02 && i3 < ne03) { + x = (const char *)src0->data + + (i1 )*nb01 + (i2 )*nb02 + (i3 )*nb03; + } else { + x = (const char *)src1->data + + (i1 - o[1])*nb11 + (i2 - o[2])*nb12 + (i3 - o[3])*nb13; + } + char * y = (char *)dst->data + + i1*nb1 + i2*nb2 + i3*nb3; + memcpy(y, x, row_bytes_dst); + } + } + } +} + void ggml_compute_forward_concat( const ggml_compute_params * params, ggml_tensor * dst) { @@ -2102,7 +2215,16 @@ void ggml_compute_forward_concat( } break; default: { - ggml_compute_forward_concat_any(params, dst); + // opencoti F5 bug-226 fix (0035): block-quantized types + // (q8_0, q4_0, q5_0, …) route to concat_q which iterates + // over BLOCKS, not logical elements. Without this they + // hit concat_any whose per-element memcpy with type_size + // = block-size scrambles q8_0 K/V at M2 head-axis concat. + if (ggml_blck_size(src0->type) > 1) { + ggml_compute_forward_concat_q(params, dst); + } else { + ggml_compute_forward_concat_any(params, dst); + } } } }