From dedd235b6875dd6241844aade4382252ca4fb403 Mon Sep 17 00:00:00 2001 From: bri-prism <288398250+bri-prism@users.noreply.github.com> Date: Wed, 2 Sep 2026 21:29:07 -0700 Subject: [PATCH 1/2] ggml-cpu: restore the src1 guard on the F16 Hadamard matmul path Two things about the F16 src1 support in forward_mul_mat. The conversion block it lives in runs whenever src1->type != vec_dot_type, and the F16 branch writes plain floats into wdata. That is only valid when the wdata rows are floats. With a quantized src0 the vec_dot_type is Q8_0/Q8_1/Q8_K, whose rows are about one byte per element (34 bytes per 32 for Q8_0), so writing four bytes per element would run roughly 3.8x off the end of the buffer. Nothing reaches that today: supports_op only admits an F16 src1 when src0 and dst are F32, so the scheduler never routes the bad combination to the CPU backend. But the assertion here used to be src1->type == GGML_TYPE_F32, which would have caught it, and widening it to admit F16 unconditionally left the invariant undefended for any caller that reaches forward_mul_mat without consulting supports_op. Assert what the code actually handles instead. Second, the branch hand-rolled a scalar F16 to F32 loop. ggml_cpu_fp16_to_fp32 is declared in ggml-cpu.h and defined further down this same file with F16C and AVX512F paths, so call it. Behaviour-preserving; no speedup measured or claimed, since this path only runs for Hadamard-hinted F32 matmuls. test-backend-ops on CPU: MUL_MAT 1249/1249, MUL_MAT_HADAMARD 18/18 including the four f16 src1 cases and the f16 FWHT case. Note the suite would not have caught the widened assert: no case drives a quantized src0 with an F16 src1, because supports_op refuses it and the harness skips it as unsupported. --- ggml/src/ggml-cpu/ggml-cpu.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/ggml/src/ggml-cpu/ggml-cpu.c b/ggml/src/ggml-cpu/ggml-cpu.c index 7a1bcdf355b3..654c9366eff1 100644 --- a/ggml/src/ggml-cpu/ggml-cpu.c +++ b/ggml/src/ggml-cpu/ggml-cpu.c @@ -5,6 +5,7 @@ #include "ggml-backend.h" #include "traits.h" #include "ggml-cpu-impl.h" +#include "ggml-cpu.h" #include "ggml-impl.h" #include "quants.h" #include "ggml-threading.h" @@ -1340,7 +1341,11 @@ UseGgmlGemm1:; const size_t nbw3 = nbw2*ne12; assert(params->wsize >= ne13*nbw3); - GGML_ASSERT(src1->type == GGML_TYPE_F32 || src1->type == GGML_TYPE_F16); + // an F16 src1 is converted straight to float below, so it is only valid when the + // wdata rows are floats. A quantized vec_dot_type would size them at about one + // byte per element and the conversion would run off the end of the buffer. + GGML_ASSERT(src1->type == GGML_TYPE_F32 || + (src1->type == GGML_TYPE_F16 && vec_dot_type == GGML_TYPE_F32)); #if 0 for (int64_t i13 = 0; i13 < ne13; ++i13) { @@ -1366,11 +1371,7 @@ UseGgmlGemm1:; if (src1->type == GGML_TYPE_F32) { from_float((const float *) src1_block, dst_block, n_block); } else { - const ggml_fp16_t * src_f16 = (const ggml_fp16_t *) src1_block; - float * dst_f32 = (float *) dst_block; - for (int64_t i = 0; i < n_block; ++i) { - dst_f32[i] = GGML_CPU_FP16_TO_FP32(src_f16[i]); - } + ggml_cpu_fp16_to_fp32((const ggml_fp16_t *) src1_block, (float *) dst_block, n_block); } } } From 7be8db7a3ce4ffbbb924160f66c3b384eb232a06 Mon Sep 17 00:00:00 2001 From: bri-prism <288398250+bri-prism@users.noreply.github.com> Date: Thu, 3 Sep 2026 23:35:37 -0700 Subject: [PATCH 2/2] ggml-cpu: keep the F16 src1 invariant comment on one line AGENTS.md asks for concise comments that are not hard-wrapped mid-sentence. The guard's explanation was split across three lines at a fixed width; it is now one line, which is within the width already used elsewhere in this file. Comment only, no change in behaviour. --- ggml/src/ggml-cpu/ggml-cpu.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ggml/src/ggml-cpu/ggml-cpu.c b/ggml/src/ggml-cpu/ggml-cpu.c index 654c9366eff1..9cd2c2d39a5a 100644 --- a/ggml/src/ggml-cpu/ggml-cpu.c +++ b/ggml/src/ggml-cpu/ggml-cpu.c @@ -1341,9 +1341,7 @@ UseGgmlGemm1:; const size_t nbw3 = nbw2*ne12; assert(params->wsize >= ne13*nbw3); - // an F16 src1 is converted straight to float below, so it is only valid when the - // wdata rows are floats. A quantized vec_dot_type would size them at about one - // byte per element and the conversion would run off the end of the buffer. + // F16 src1 converts straight to float, so wdata rows must be floats; a quantized vec_dot_type would overrun the buffer GGML_ASSERT(src1->type == GGML_TYPE_F32 || (src1->type == GGML_TYPE_F16 && vec_dot_type == GGML_TYPE_F32));