Skip to content

feat(ggml-opencl): correct bidirectional encoder attention + Adreno vision-encoder fixes#170

Merged
gianni-cor merged 6 commits into
temp-9341from
feat/opencl-vision-encoder-qwen3vl-s25
Jul 6, 2026
Merged

feat(ggml-opencl): correct bidirectional encoder attention + Adreno vision-encoder fixes#170
gianni-cor merged 6 commits into
temp-9341from
feat/opencl-vision-encoder-qwen3vl-s25

Conversation

@olyasir

@olyasir olyasir commented Jun 25, 2026

Copy link
Copy Markdown

What

Three OpenCL backend fixes that let the Qwen3-VL / SigLIP vision encoder
run correctly and fast on Adreno (Snapdragon OpenCL), so the vision
(mmproj/clip) tower can move off the CPU. On a Samsung S25 (Adreno 830)
this gives a ~2.3x faster vision encode with no accuracy loss vs the
CPU baseline.

Targets temp-9341 (the 9341 integration branch). All three commits
touch only ggml-opencl.cpp. Intended to publish as qvac-fabric 9341.1.0.

Commits

  1. support antialiased BILINEAR upscale when upsamplingsupports_op
    rejected BILINEAR|ANTIALIAS, forcing the Qwen3-VL position-embedding
    interpolation onto the CPU and splitting the compute graph (3 splits).
    Antialiasing only affects downsampling; when upsampling (the vision
    tower's case) the result is identical to plain bilinear, which the
    existing kernel already computes. Accepting it keeps the whole CLIP
    graph on OpenCL (graph splits 3 → 1).
  2. build flash-attn kernels without finite-math — the kernels were
    built with -cl-finite-math-only / -cl-fast-relaxed-math /
    -cl-unsafe-math-optimizations, which let the compiler assume no
    Inf/NaN. Flash-attention initialises its online-softmax running max to
    -INFINITY and masks padded score rows (k_row >= n_kv) with
    -INFINITY; with n_kv not a multiple of the tile width those paths are
    exercised, so finite-math miscompiles them. Strip those three flags for
    the FA programs (keep -cl-mad-enable).
  3. treat null attention mask as bidirectional, not causal (the
    decisive accuracy fix)
    — the FA dispatch inferred causal masking from
    shape: is_causal = (mask == NULL && n_q > 1 && n_q == n_kv). A null
    mask means no masking (bidirectional — the SigLIP vision/embedding
    encoders); causal attention always supplies an explicit -INFINITY
    causal mask in this codebase. The heuristic made the bidirectional
    Qwen3-VL vision tower attend causally, so each patch only saw
    earlier patches and the image embedding was corrupted. Set
    is_causal = 0 unconditionally — causality is always expressed via the
    explicit mask.

Why it's safe for the LLM

The LLM already passes a real causal kq_mask filled with -INFINITY
(llama-graph.cpp build_attn), so it was already is_causal = 0 and
relies on that mask; incremental decode (n_q=1) uses the single-query
kernel which ignores is_causal. The shape-heuristic's causal branch was
only ever hit by bidirectional encoders passing a null mask, so removing
it cannot change LLM behaviour. The fix benefits all bidirectional
encoders (vision + embeddings) on Adreno OpenCL, not just this model.

Scope

Deliberately minimal — only the changes the validated path needs. The
vision tower casts K/V to F16 (clip.cpp), so its attention dispatches the
mixed flash_attn_f32_f16 kernel; commits 2 and 3 cover that kernel
(no-finite-math is applied to all FA programs; is_causal is host-side).

Validation

  • On-device (S25 / Adreno 830, isolated vision-encode, 3-rep medians):
    CPU 2180ms → OpenCL 945ms = 2.31x, output correct in both.
  • Built + tested across all six qvac-fabric consumers via the monorepo
    overlay (classification-ggml, embed-llamacpp, llm-llamacpp, ocr-ggml,
    translation-nmtcpp, vla-ggml) — android-arm64 (the OpenCL build)
    green; no consumer build/test regressions.

Consumer validation CI (9341.1.1 rollout Phase A)

Validated from monorepo branch test/QVAC-21297-fabric-9341.1.1-overlay — a shared vcpkg overlay port pinning this PR's head (fe61104b, built from source in every consumer's CI; no registry/baseline changes), dispatched per consumer via workflow_dispatch:

Consumer Run Result
classification-ggml 28590246348 ✅ green
embed-llamacpp 28590246855 ✅ green
llm-llamacpp 28590248055 ✅ green except one test-darwin-x64 known issue
ocr-ggml 28590250522 ✅ green
translation-nmtcpp 28590252222 ✅ green
vla 28590253546 ✅ green

On-device benchmark (S25 Ultra / Adreno 830, OpenCL — AWS Device Farm)

4-run CPU-matched A/B (2 baseline + 2 fixed, paired by the build-invariant CPU mmproj-encode fingerprint), Qwen3.5-0.8B + mmproj-Q8_0, projector CPU-vs-GPU per run, lmms-eval quality gate:

  • Accuracy restored and neutral: GPU projector == CPU (Δ 0.0 pp) with this PR's fixes — versus the stock OpenCL GPU projector's corrupted encodes (−4.1 pp at base preset, −18.8 pp at full). The decisive fix is null-mask ⇒ bidirectional attention (the SigLIP tower was being force-cast to causal).
  • Speed: the fixed GPU projector is ~23 % faster than the shipping CPU projector on vision-encode (~19 % faster TTFT); within-run GPU-vs-CPU speedup ~1.43× on the lmms-eval fixture, up to ~2.31× on the isolated single-image best case (matches the number in Validation above). Correct attention costs ~31 % vs the buggy-but-fast stock GPU baseline — still well under CPU.

@olyasir olyasir requested a review from a team as a code owner June 25, 2026 07:56
@olyasir olyasir force-pushed the feat/opencl-vision-encoder-qwen3vl-s25 branch from f963db0 to d266267 Compare June 25, 2026 08:11
@iancris iancris force-pushed the feat/opencl-vision-encoder-qwen3vl-s25 branch from 19c1755 to fe61104 Compare July 1, 2026 17:15
olyasir and others added 6 commits July 6, 2026 11:50
The supports_op check for GGML_OP_UPSCALE rejected the
BILINEAR|ANTIALIAS mode, forcing the Qwen3-VL / SigLIP vision graph's
position-embedding interpolation onto the CPU and splitting the compute
graph. Antialiasing only affects downsampling; when the destination grid
is >= the source grid (the upsampling case the vision tower uses) the
antialiased result is identical to plain bilinear, which the existing
kernel_upscale_bilinear already computes exactly.

Relax supports_op to accept BILINEAR + ANTIALIAS when upsampling so the
whole vision graph stays on OpenCL (graph splits 3 -> 1).
The OpenCL kernels are compiled with -cl-finite-math-only and
-cl-fast-relaxed-math, which let the compiler assume no Inf/NaN. The
flash-attention online softmax initialises its running max to -INFINITY
and masks padded scores with -INFINITY, so finite-math miscompiles the
init/masking path.

Compile the flash-attention programs with a relaxed option set that
drops -cl-fast-relaxed-math, -cl-finite-math-only and
-cl-unsafe-math-optimizations (keeping -cl-mad-enable for speed) so the
-inf sentinels behave correctly.
The flash-attention dispatch inferred causal masking from shape with
`is_causal = (mask == NULL && n_q > 1 && n_q == n_kv)`. A null mask means
no masking, i.e. bidirectional attention (the SigLIP vision and embedding
encoders), while causal attention always supplies an explicit causal mask
in this codebase (llama-graph.cpp build_attn passes a kq_mask filled with
-INFINITY). The heuristic therefore wrongly made the bidirectional
Qwen3-VL vision tower attend causally, so each patch only saw earlier
patches and the image embedding was corrupted.

Set is_causal = 0 unconditionally; causality is always expressed via the
explicit mask. This cannot regress the LLM, which already passes a real
causal mask (is_causal was already 0 for it) and relies on that mask.
…al invariant

The flash-attn compile-flag strip removed only the first occurrence of each
unsafe flag and silently tolerated a miss. If compile_opts ever changes its
spelling/spacing or repeats a flag, a surviving -cl-finite-math-only /
-cl-fast-relaxed-math would rebuild the FA kernels with Inf-assuming math and
silently reintroduce the -INFINITY online-softmax/masking miscompile this
strip exists to prevent.

Erase every occurrence of each flag and add a GGML_ASSERT that no
finite-math/fast-math/unsafe-math flag survived, so a future regression fails
loudly at load time instead of producing silently wrong attention output.

Also document the is_causal invariant in ggml_cl_flash_attn: a null mask is
treated as bidirectional, so any caller needing causal masking must supply an
explicit causal mask rather than relying on shape inference.
The f32 flash-attention kernel loads K/V tiles into local memory,
barriers, reads them, then loops to overwrite the tiles for the next
K/V block without a trailing barrier. Out-of-range lanes (the last
partial BLOCK_M block) could also race ahead into the next tile load
while active lanes were still reading. With n_kv > BLOCK_N (e.g. the
bidirectional vision tower, n_kv=247) this corrupts the shared tiles.

Add a trailing barrier(CLK_LOCAL_MEM_FENCE) at the end of the K/V block
loop and guard the score computation with the query-row range check
instead of an early continue.
…ale 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>
@iancris iancris force-pushed the feat/opencl-vision-encoder-qwen3vl-s25 branch from fe61104 to b7ad6d4 Compare July 6, 2026 11:53
@iancris iancris requested review from a team as code owners July 6, 2026 11:53
@gianni-cor gianni-cor merged commit 6613cea into temp-9341 Jul 6, 2026
35 of 53 checks passed
@gianni-cor gianni-cor deleted the feat/opencl-vision-encoder-qwen3vl-s25 branch July 15, 2026 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants