Skip to content

Commit b7ad6d4

Browse files
iancrisclaude
andcommitted
ggml-opencl: port FA barrier fix to f16/f32_f16 siblings + guard upscale zero dims
PR-review follow-ups: - Port the flash-attn tile-loop barrier/divergence fix (previously only in flash_attn_f32.cl) to the f16 and f32_f16 sibling kernels: guard the score loop with `if (my_query_row < n_q)` instead of an early `continue`, and add a trailing barrier at the end of the K/V tile loop so out-of-range lanes cannot race ahead into the next tile's load while active lanes still read l_k/l_v. flash_attn_f32_f16 is the kernel the Qwen3-VL vision tower actually dispatches (K/V cast to F16), so the fix was missing from the validated workload. Score-loop bodies are unchanged. - Guard zero source dimensions in ggml_cl_upscale: the sf* scale factors divide by the source dims, so a zero source dim yields +inf; the existing early-exit only covered zero destination dims. Validated on-device (S25 Ultra / Adreno 830 OpenCL, QVAC-21297): the GPU vision projector matches CPU exactly (Delta 0.0 pp, task-for-task) and is ~26% faster than the shipping CPU projector on encode; 0 device-loss. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent dc64397 commit b7ad6d4

3 files changed

Lines changed: 34 additions & 8 deletions

File tree

ggml/src/ggml-opencl/ggml-opencl.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9573,6 +9573,12 @@ static void ggml_cl_upscale(ggml_backend_t backend, const ggml_tensor * src0, gg
95739573
const int ne2 = dst->ne[2];
95749574
const int ne3 = dst->ne[3];
95759575

9576+
// Zero source dims would make the sf* scale factors below divide by zero (+inf);
9577+
// the later dst-zero early-exit does not cover this.
9578+
if (ne00 == 0 || ne01 == 0 || ne02 == 0 || ne03 == 0) {
9579+
return;
9580+
}
9581+
95769582
float sf0 = (float)ne0 / ne00;
95779583
float sf1 = (float)ne1 / ne01;
95789584
float sf2 = (float)ne2 / ne02;

ggml/src/ggml-opencl/kernels/flash_attn_f16.cl

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,15 @@ __kernel void flash_attn_f16(
121121
}
122122
barrier(CLK_LOCAL_MEM_FENCE);
123123

124-
if (my_query_row >= n_q) {
125-
continue;
126-
}
127-
124+
// NOTE: do NOT `continue` for out-of-range query rows here. Every
125+
// work-item must reach the trailing barrier at the end of this loop,
126+
// otherwise the extra lanes (my_query_row >= n_q in the last partial
127+
// BLOCK_M block) race ahead to the next tile's load and overwrite
128+
// l_k/l_v while active lanes are still reading them. That shared-memory
129+
// race silently corrupts the K/V tiles for any sequence spanning more
130+
// than one BLOCK_N tile (e.g. the bidirectional Qwen3-VL vision tower,
131+
// n_kv=247), degrading the encode. Guard the score loop instead.
132+
if (my_query_row < n_q) {
128133
for (int j = 0; j < BLOCK_N; j += 2) {
129134
const int k_row0 = k_start + j;
130135
const int k_row1 = k_start + j + 1;
@@ -170,6 +175,11 @@ __kernel void flash_attn_f16(
170175
l_i = l_i * scale_prev + p0 + p1;
171176
m_i = m_new;
172177
}
178+
} // end if (my_query_row < n_q)
179+
180+
// Ensure every work-item has finished reading l_k/l_v before the next
181+
// iteration overwrites the shared K/V tiles.
182+
barrier(CLK_LOCAL_MEM_FENCE);
173183
}
174184

175185
if (my_query_row < n_q) {

ggml/src/ggml-opencl/kernels/flash_attn_f32_f16.cl

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -196,10 +196,15 @@ __kernel void flash_attn_f32_f16(
196196
}
197197
barrier(CLK_LOCAL_MEM_FENCE);
198198

199-
if (my_query_row >= n_q) {
200-
continue;
201-
}
202-
199+
// NOTE: do NOT `continue` for out-of-range query rows here. Every
200+
// work-item must reach the trailing barrier at the end of this loop,
201+
// otherwise the extra lanes (my_query_row >= n_q in the last partial
202+
// BLOCK_M block) race ahead to the next tile's load and overwrite
203+
// l_k/l_v while active lanes are still reading them. That shared-memory
204+
// race silently corrupts the K/V tiles for any sequence spanning more
205+
// than one BLOCK_N tile (e.g. the bidirectional Qwen3-VL vision tower,
206+
// n_kv=247), degrading the encode. Guard the score loop instead.
207+
if (my_query_row < n_q) {
203208
for (int j = 0; j < BLOCK_N; j += 2) {
204209
const int k_row0 = k_start + j;
205210
const int k_row1 = k_start + j + 1;
@@ -245,6 +250,11 @@ __kernel void flash_attn_f32_f16(
245250
l_i = l_i * scale_prev + p0 + p1;
246251
m_i = m_new;
247252
}
253+
} // end if (my_query_row < n_q)
254+
255+
// Ensure every work-item has finished reading l_k/l_v before the next
256+
// iteration overwrites the shared K/V tiles.
257+
barrier(CLK_LOCAL_MEM_FENCE);
248258
}
249259

250260
if (my_query_row < n_q) {

0 commit comments

Comments
 (0)