Skip to content

Commit 6b0f169

Browse files
committed
ggml-opencl: harden flash-attn finite-math strip and document is_causal 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.
1 parent d266267 commit 6b0f169

1 file changed

Lines changed: 18 additions & 2 deletions

File tree

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2149,11 +2149,23 @@ static void load_cl_kernels(ggml_backend_opencl_context *backend_ctx, ggml_cl_ve
21492149
for (const char* unsafe_flag : { " -cl-fast-relaxed-math",
21502150
" -cl-finite-math-only",
21512151
" -cl-unsafe-math-optimizations" }) {
2152-
const size_t pos = fa_compile_opts.find(unsafe_flag);
2153-
if (pos != std::string::npos) {
2152+
// Erase every occurrence: the strip must not depend on the flag
2153+
// appearing exactly once or in a particular position. A surviving
2154+
// copy would silently rebuild FA with finite-math and reintroduce
2155+
// the -INFINITY miscompile this strip exists to prevent.
2156+
for (size_t pos = fa_compile_opts.find(unsafe_flag);
2157+
pos != std::string::npos;
2158+
pos = fa_compile_opts.find(unsafe_flag)) {
21542159
fa_compile_opts.erase(pos, std::string(unsafe_flag).size());
21552160
}
21562161
}
2162+
// Fail loudly if any Inf-assuming flag survived (e.g. a future change
2163+
// to compile_opts spelling/spacing that the strip above misses),
2164+
// rather than shipping a silently miscompiled flash-attention kernel.
2165+
GGML_ASSERT(fa_compile_opts.find("finite-math") == std::string::npos &&
2166+
fa_compile_opts.find("fast-relaxed") == std::string::npos &&
2167+
fa_compile_opts.find("unsafe-math") == std::string::npos &&
2168+
"flash-attn kernels must not be built with finite-math/fast-math flags");
21572169

21582170
for (size_t i = 0; i < sizeof(fa_dims)/sizeof(fa_dims[0]); ++i) {
21592171
const int dk = fa_dims[i].dk;
@@ -9860,6 +9872,10 @@ static void ggml_cl_flash_attn(ggml_backend_t backend, const ggml_tensor * q, co
98609872
// causal. The previous `mask == NULL && n_q == n_kv` heuristic wrongly made
98619873
// the bidirectional Qwen3-VL vision tower attend causally, corrupting the
98629874
// image embedding (each patch only saw earlier patches).
9875+
//
9876+
// INVARIANT: this backend treats a null mask as bidirectional. Any caller
9877+
// that needs causal masking MUST supply an explicit causal mask; relying on
9878+
// shape inference here will silently produce bidirectional (wrong) output.
98639879
const int is_causal = 0;
98649880

98659881
const int n_head_log2_val = n_head > 0 ? 1u << (int)floorf(log2f((float)n_head)) : 0;

0 commit comments

Comments
 (0)