feat(ggml-opencl): correct bidirectional encoder attention + Adreno vision-encoder fixes#170
Merged
Merged
Conversation
f963db0 to
d266267
Compare
19c1755 to
fe61104
Compare
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>
fe61104 to
b7ad6d4
Compare
This was referenced Jul 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 commitstouch only
ggml-opencl.cpp. Intended to publish asqvac-fabric 9341.1.0.Commits
supports_oprejected
BILINEAR|ANTIALIAS, forcing the Qwen3-VL position-embeddinginterpolation 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).
built with
-cl-finite-math-only/-cl-fast-relaxed-math/-cl-unsafe-math-optimizations, which let the compiler assume noInf/NaN. Flash-attention initialises its online-softmax running max to
-INFINITYand masks padded score rows (k_row >= n_kv) with-INFINITY; with n_kv not a multiple of the tile width those paths areexercised, so finite-math miscompiles them. Strip those three flags for
the FA programs (keep
-cl-mad-enable).decisive accuracy fix) — the FA dispatch inferred causal masking from
shape:
is_causal = (mask == NULL && n_q > 1 && n_q == n_kv). A nullmask means no masking (bidirectional — the SigLIP vision/embedding
encoders); causal attention always supplies an explicit
-INFINITYcausal 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 = 0unconditionally — causality is always expressed via theexplicit mask.
Why it's safe for the LLM
The LLM already passes a real causal
kq_maskfilled with-INFINITY(llama-graph.cpp
build_attn), so it was alreadyis_causal = 0andrelies on that mask; incremental decode (n_q=1) uses the single-query
kernel which ignores
is_causal. The shape-heuristic's causal branch wasonly 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_f16kernel; commits 2 and 3 cover that kernel(no-finite-math is applied to all FA programs;
is_causalis host-side).Validation
CPU 2180ms → OpenCL 945ms = 2.31x, output correct in both.
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 viaworkflow_dispatch:test-darwin-x64known issueOn-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-encodefingerprint), Qwen3.5-0.8B + mmproj-Q8_0, projector CPU-vs-GPU per run, lmms-eval quality gate: