opencl: fix out‐of‐bound reads in the Adreno image kernels (#27632)

* 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
This commit is contained in:
Hongqiang Wang
2026-09-01 22:28:45 -07:00
committed by GitHub
parent 69320fef12
commit 43d87ff2dd
2 changed files with 95 additions and 27 deletions
+67 -16
View File
@@ -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;
}
}
@@ -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