From 43d87ff2dd0e5706d3ce974fd67d7bbc69768109 Mon Sep 17 00:00:00 2001 From: Hongqiang Wang Date: Tue, 1 Sep 2026 22:28:45 -0700 Subject: [PATCH] =?UTF-8?q?opencl:=20fix=20out=E2=80=90of=E2=80=90bound=20?= =?UTF-8?q?reads=20in=20the=20Adreno=20image=20kernels=20(#27632)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * opencl: clamp the q4_K decode GEMV's fetch row on a padded x-grid * opencl: enforce the tiling contract of the image KQ/KQV GEMMs * opencl: decide the image KQ/KQV split at the dispatch, not from strides --- ggml/src/ggml-opencl/ggml-opencl.cpp | 83 +++++++++++++++---- .../kernels/gemv_noshuffle_q4_k_f32.cl | 39 ++++++--- 2 files changed, 95 insertions(+), 27 deletions(-) diff --git a/ggml/src/ggml-opencl/ggml-opencl.cpp b/ggml/src/ggml-opencl/ggml-opencl.cpp index 34d58f4ee8..d95123eb15 100644 --- a/ggml/src/ggml-opencl/ggml-opencl.cpp +++ b/ggml/src/ggml-opencl/ggml-opencl.cpp @@ -16254,7 +16254,13 @@ static void ggml_cl_conv_2d(ggml_backend_t backend, const ggml_tensor * src0, co backend_ctx->enqueue_ndrange_kernel(kernel, 2, global_work_size, local_work_size, dst); } -static void ggml_cl_mul_mat_kq_kqv_adreno(ggml_backend_t backend, const ggml_tensor * src0, const ggml_tensor * src1, ggml_tensor * dst) { +// is_kq selects which of the two products this call is, and it is decided by the +// CALLER -- the two admission arms in ggml_cl_mul_mat, each of which knows which +// one it matched. It used to be re-derived here from nb01 > nb02, i.e. "K is +// head-major, V^T is not". That discriminator COLLAPSES at n_head_kv == 1, where +// the two strides are equal because there is only one head to order, so nothing +// here could tell a KQ from a KQV. Pass it in rather than infer it. +static void ggml_cl_mul_mat_kq_kqv_adreno(ggml_backend_t backend, const ggml_tensor * src0, const ggml_tensor * src1, ggml_tensor * dst, bool is_kq) { ggml_backend_opencl_context *backend_ctx = (ggml_backend_opencl_context *)backend->context; ggml_tensor_extra_cl * extra0 = (ggml_tensor_extra_cl *)src0->extra; @@ -16296,19 +16302,14 @@ static void ggml_cl_mul_mat_kq_kqv_adreno(ggml_backend_t backend, const ggml_ten int N = ne1; int K = ne00; - if (nb01 > nb02) { - // KQ - kernel = backend_ctx->kernel_mul_mm_f16_f32_kq; - } else { - // KQV - kernel = backend_ctx->kernel_mul_mm_f16_f32_kqv; - } + kernel = is_kq ? backend_ctx->kernel_mul_mm_f16_f32_kq + : backend_ctx->kernel_mul_mm_f16_f32_kqv; // create sub-buffer for A // <--------------------------------------------> // extra0 = src0->view_src ? (ggml_tensor_extra_cl *)src0->view_src->extra : (ggml_tensor_extra_cl *)src0->extra; region.origin = (extra0->offset + src0->view_offs); - if (nb01 > nb02) { + if (is_kq) { // KQ region.size = nb01 * ne01; } else { @@ -16332,7 +16333,7 @@ static void ggml_cl_mul_mat_kq_kqv_adreno(ggml_backend_t backend, const ggml_ten img_fmt_1d = {CL_RGBA, CL_FLOAT}; memset(&img_desc_1d, 0, sizeof(img_desc_1d)); img_desc_1d.image_type = CL_MEM_OBJECT_IMAGE1D_BUFFER; - if (nb01 > nb02) { + if (is_kq) { img_desc_1d.image_width = (nb01 * ne01 / 4)/4; } else { @@ -19222,13 +19223,61 @@ static void ggml_cl_mul_mat(ggml_backend_t backend, const ggml_tensor * src0, co #ifdef GGML_OPENCL_USE_ADRENO_KERNELS if(src0t == GGML_TYPE_F16 && src1t == GGML_TYPE_F32){ - if (ne01 >= 64 && ne1 >= 32 && ne00 >= 16 && (ne12 % ne02) == 0 && + // Two tiling assumptions these kernels make but nothing enforced: + // + // ne00 % TILESIZE_K(16): the K loop has no tail, so a K that does not + // divide folds 1-15 rows of whatever follows the operands into every + // output. + // + // ne01 % TILESIZE_M(64): mm_store_c_N guards the n direction with its + // `mask` argument but nothing guards m -- the store walks all 64 rows + // of the tile at a stride of M. When M does not divide, the last tile + // does not run off the end of the buffer, it writes 64 - (M % 64) + // values ON TOP OF the next column, so the result is silently wrong. + // Reachable on the KQV side for any head size >= 64 that is not a + // multiple of it (80, 96, 112). + // + // Attention shapes in the graph satisfy both -- head sizes are multiples + // of 64 and n_kv is padded -- which is why this has stayed latent. + // Declining leaves the odd shapes on the generic GEMM, which handles them. + if (ne01 >= 64 && ne1 >= 32 && ne00 >= 16 && + (ne00 % 16) == 0 && (ne01 % 64) == 0 && (ne12 % ne02) == 0 && // the KQ/KQV image kernels do not handle dim 3 (multi-stream batches) ne03 == 1 && ne13 == 1 && // dst is wrapped with image1d_buffer, the size limit applies, also src0 (ne0 * ne1 * dst->ne[2] * dst->nb[0] / 4 <= backend_ctx->image_max_buffer_size)) { - // For KQ - if (ggml_is_permuted(src0) && ggml_is_permuted(src1) && + // For KQ. + // + // Layout admission, mirroring the KQV arm below. The KQ kernel takes + // no stride arguments for A or B: it derives them as K*D_A*2 and + // K*D_B*4, i.e. it assumes both operands pack exactly D heads of K + // elements per row. Every real KV-cache view and permuted-Q view + // does, but a view spanning part of a wider allocation does not, and + // the kernel then walks the wrong rows with nothing to range-check + // it. Gate on the packed layout itself rather than on the stride + // ORDERING, which a wider parent satisfies just as well. + const bool kq_packed_a = (nb01 == (cl_ulong)ne00 * ne02 * ggml_type_size(src0t)) && + (nb02 == (cl_ulong)ne00 * ggml_type_size(src0t)); + const bool kq_packed_b = (nb11 == (cl_ulong)ne10 * ne12 * ggml_type_size(src1t)) && + (nb12 == (cl_ulong)ne10 * ggml_type_size(src1t)); + // + // ggml_is_permuted(src0) stands in for "K is head-major", but it is + // only a proxy and it COLLAPSES at n_head_kv == 1: with a single + // head there is no head stride to be out of order, so nb01 == nb02 + // and the view reports itself unpermuted. Such a KQ was declined + // here and fell through to the generic GEMM (gemma-4 E2B, and any + // other multi-query model). The packed check above is the contract + // the kernel actually needs -- it pins both strides exactly -- so + // require permutedness only where there is more than one head for + // it to mean anything. + // + // Default on; GGML_OPENCL_KQ_NHEAD_KV1=0 restores the old proxy so + // the two routings can be compared in one binary. + static const char * kq_nhkv1_env = getenv("GGML_OPENCL_KQ_NHEAD_KV1"); + static const bool kq_nhkv1_on = + (kq_nhkv1_env == nullptr || kq_nhkv1_env[0] != '0'); + if ((ggml_is_permuted(src0) || (ne02 == 1 && kq_nhkv1_on)) && ggml_is_permuted(src1) && + kq_packed_a && kq_packed_b && ((nb01 * ne01 / 4)/4 <= backend_ctx->image_max_buffer_size) && nb00 <= nb02 && nb02 <= nb01 && @@ -19236,13 +19285,15 @@ static void ggml_cl_mul_mat(ggml_backend_t backend, const ggml_tensor * src0, co nb10 <= nb12 && nb12 <= nb11 && nb11 <= nb13) { - ggml_cl_mul_mat_kq_kqv_adreno(backend, src0, src1, dst); + ggml_cl_mul_mat_kq_kqv_adreno(backend, src0, src1, dst, /*is_kq =*/ true); return; } - // For KQV + // For KQV. Reaching this arm is what makes the op a KQV; the callee + // is told so explicitly rather than re-deriving it from the strides + // the arm above has already ruled on. if (!ggml_is_contiguous(src0) && ggml_is_contiguous(src1) && ((nb02 * ne02 / 4)/4 <= backend_ctx->image_max_buffer_size)) { - ggml_cl_mul_mat_kq_kqv_adreno(backend, src0, src1, dst); + ggml_cl_mul_mat_kq_kqv_adreno(backend, src0, src1, dst, /*is_kq =*/ false); return; } } diff --git a/ggml/src/ggml-opencl/kernels/gemv_noshuffle_q4_k_f32.cl b/ggml/src/ggml-opencl/kernels/gemv_noshuffle_q4_k_f32.cl index c1829fc382..9ab0dee693 100644 --- a/ggml/src/ggml-opencl/kernels/gemv_noshuffle_q4_k_f32.cl +++ b/ggml/src/ggml-opencl/kernels/gemv_noshuffle_q4_k_f32.cl @@ -235,6 +235,23 @@ kernel void kernel_gemv_noshuffle_q4_k_f32( uint LINE_STRIDE_A = M / 2; uint BLOCK_STRIDE_A = NSUBGROUPS * M; + // The x-grid is padded to CEIL_DIV(ne01/2,64)*64, so when ne01 % 128 != 0 the + // tail lanes hold gid >= ne01/2. The output stores below are guarded, but the + // input fetches are not: src0_d and src0_m are raw global half2 pointers, + // src0_s is a raw global uchar pointer, and read_imageui on an + // image1d_buffer_t is UNDEFINED out of range -- an image clamps only for + // SAMPLER reads, which these are not. Those lanes therefore read past the end + // of all three allocations. For a [2816, 2112] weight (2112 % 128 == 64) the + // top tail lane is gid = 1087 while only gid < 1056 is backed, and it runs + // 32 half2 past src0_d/src0_m, 31 uints past the quant image, and 63 bytes + // past src0_s. + // + // Clamp the row used for every fetch. The lanes stay ACTIVE, which the + // sub_group_broadcast in the dequant macros requires, and their results are + // still discarded by the existing output guard. No-op and byte-identical + // whenever ne01 % 128 == 0. + uint gid_s = min(gid, LINE_STRIDE_A - 1); + private uint4 regA; private half2 regS; private half2 regM; @@ -246,10 +263,10 @@ kernel void kernel_gemv_noshuffle_q4_k_f32( uint sb = k / 8; uint j = k % 8; - half2 d = src0_d[gid + sb * LINE_STRIDE_A]; - half2 dm = src0_m[gid + sb * LINE_STRIDE_A]; + half2 d = src0_d[gid_s + sb * LINE_STRIDE_A]; + half2 dm = src0_m[gid_s + sb * LINE_STRIDE_A]; - global const uchar * sc0 = src0_s + sb * 12 * M + 2 * gid; + global const uchar * sc0 = src0_s + sb * 12 * M + 2 * gid_s; global const uchar * sc1 = sc0 + 1; uchar sv0, mn0, sv1, mn1; @@ -265,20 +282,20 @@ kernel void kernel_gemv_noshuffle_q4_k_f32( } // load half weights for two blocks in consecutive rows - regA.s0 = read_imageui(src0_q, (gid + k * BLOCK_STRIDE_A + LINE_STRIDE_A * 0)).x; - regA.s1 = read_imageui(src0_q, (gid + k * BLOCK_STRIDE_A + LINE_STRIDE_A * 1)).x; - regA.s2 = read_imageui(src0_q, (gid + k * BLOCK_STRIDE_A + LINE_STRIDE_A * 2)).x; - regA.s3 = read_imageui(src0_q, (gid + k * BLOCK_STRIDE_A + LINE_STRIDE_A * 3)).x; + regA.s0 = read_imageui(src0_q, (gid_s + k * BLOCK_STRIDE_A + LINE_STRIDE_A * 0)).x; + regA.s1 = read_imageui(src0_q, (gid_s + k * BLOCK_STRIDE_A + LINE_STRIDE_A * 1)).x; + regA.s2 = read_imageui(src0_q, (gid_s + k * BLOCK_STRIDE_A + LINE_STRIDE_A * 2)).x; + regA.s3 = read_imageui(src0_q, (gid_s + k * BLOCK_STRIDE_A + LINE_STRIDE_A * 3)).x; #ifdef VECTOR_SUB_GROUP_BROADCAST dequantizeBlockAccum_ns_sgbroadcast_8_hi(totalSum, as_ushort8(regA), regS, regM, regB); #else dequantizeBlockAccum_ns_sgbroadcast_1_hi(totalSum, as_ushort8(regA), regS, regM, regB); #endif // VECTOR_SUB_GROUP_BROADCAST - regA.s0 = read_imageui(src0_q, (gid + k * BLOCK_STRIDE_A + LINE_STRIDE_A * 4)).x; - regA.s1 = read_imageui(src0_q, (gid + k * BLOCK_STRIDE_A + LINE_STRIDE_A * 5)).x; - regA.s2 = read_imageui(src0_q, (gid + k * BLOCK_STRIDE_A + LINE_STRIDE_A * 6)).x; - regA.s3 = read_imageui(src0_q, (gid + k * BLOCK_STRIDE_A + LINE_STRIDE_A * 7)).x; + regA.s0 = read_imageui(src0_q, (gid_s + k * BLOCK_STRIDE_A + LINE_STRIDE_A * 4)).x; + regA.s1 = read_imageui(src0_q, (gid_s + k * BLOCK_STRIDE_A + LINE_STRIDE_A * 5)).x; + regA.s2 = read_imageui(src0_q, (gid_s + k * BLOCK_STRIDE_A + LINE_STRIDE_A * 6)).x; + regA.s3 = read_imageui(src0_q, (gid_s + k * BLOCK_STRIDE_A + LINE_STRIDE_A * 7)).x; #ifdef VECTOR_SUB_GROUP_BROADCAST dequantizeBlockAccum_ns_sgbroadcast_8_lo(totalSum, as_ushort8(regA), regS, regM, regB); #else