Skip to content

[fa] Make the FlashAttention backend extensible by subclasses - #33426

Merged
alexnails merged 3 commits into
sgl-project:mainfrom
klshuster:kurt/fa4-backend-hooks
Sep 14, 2026
Merged

alexnails merged 3 commits into
sgl-project:mainfrom
klshuster:kurt/fa4-backend-hooks

Conversation

@klshuster

@klshuster klshuster commented Aug 3, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

FlashAttentionBackend is hard to specialize without copying it. Three things a subclass may need are not exposed today, leaving only the options of duplicating the backend or patching module globals: the logsumexp the kernel already computes, per-layer control of split-KV, and the choice of FA4 build. This adds a seam for each. All three are no-ops for current behavior.

Modifications

return_lse on forward_extend / forward_decode. The kernel already computes the logsumexp for the cascade merge and the backend discards it, and the forwards accept neither return_lse nor **kwargs, so a caller that needs the softmax normalizer has no way to get it. With return_lse=True they return (output, lse), lse shaped [total_q, num_heads] (the varlen kernel emits it head-major). _check_return_lse raises on the combinations the paged path cannot serve — non-FA4, MLA, fa_skip_kv_cache, cross-attention, context parallelism, local and cascade attention — instead of returning a quietly wrong tensor. Defaults to False.

_resolve_num_splits(layer, forward_batch, is_swa_layer). Split-KV was read from self.num_splits at 13 kernel arguments, making it one value for the whole run. It is now resolved once per forward and threaded through, so a subclass can vary it per layer or per batch — for example forcing 1 on sliding-window layers, which degenerate when split. The base implementation returns self.num_splits. Since FA3's scheduler_metadata is precomputed from self.num_splits, the existing sched_meta gate also requires num_splits == self.num_splits so a subclass cannot reuse a schedule built for a different count; always true for the base class.

Kernel entry points as class attributes. flash_attn_with_kvcache and flash_attn_varlen_func are bound on the class and the 15 invocations go through self._flash_attn_*, so a subclass can substitute a different FA4 build.

Accuracy Tests

Two cases added to test/registered/attention/unittests/dense/test_fa4.py, self-contained so the shared dense kit is untouched:

  • test_return_lse — calls the forwards directly, since return_lse is a backend-level contract and the RadixAttention dispatcher's custom-op schema cannot carry it, then checks the output and the LSE against a causal fp32 reference for MHA extend and GQA decode, plus the [total_q, num_heads] shape.
  • test_return_lse_rejects_unsupported_paths — the guarded paths raise, and the check is a no-op when the LSE is not requested.

Both pass on a GB300 with no skips. Perturbing the returned LSE by a constant fails test_return_lse on 100% of elements, confirming the comparison is load-bearing.

Not covered: windowed layers, because the dense harness leaves attention_chunk_size unset, so a sliding_window_size case is skipped for fa4 before it asserts anything. The FA3 sched_meta gate was also not exercised, as the FA3 dense suite skips on that hardware; it is inert for the base class, whose hook returns self.num_splits.

Speed Tests and Profiling

Not applicable. No behavior changes for existing callers: return_lse defaults to False, _resolve_num_splits returns self.num_splits, the _flash_attn_* attributes are bound to the same functions the module already imported, and the new sched_meta condition is always true unless a subclass varies the split count.

Checklist


CI States

Latest PR Test (Base): ⏳ Run #34800666242
Latest PR Test (Extra): ❌ Run #34800666040
Latest PR Test (AMD ROCm 10): ⏳ Run #34800666284

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

Comment thread python/sglang/srt/layers/attention/flashattention_backend.py Outdated
Comment thread test/registered/attention/unittests/dense/test_fa4.py Outdated
@klshuster
klshuster force-pushed the kurt/fa4-backend-hooks branch from 3cab6d0 to fa0984d Compare August 24, 2026 19:18
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Aug 24, 2026
@klshuster
klshuster force-pushed the kurt/fa4-backend-hooks branch from fa0984d to 6077718 Compare August 24, 2026 19:19
@klshuster

Copy link
Copy Markdown
Contributor Author

@Fridge003, I rebased onto current main and cut this down a lot. Dropped _resolve_num_splits entirely since decode_num_splits / _get_fa_runtime_policy covers it, and removed the return_lse check and its test as you asked. The kernel indirection now just aliases the self.flash_attn_* attributes __init__ already assigns, so all 15 call sites are unchanged; what's left is return_lse plus one test.

FA4 already computes the logsumexp for the cascade merge and the backend
discards it, and the forwards accept neither return_lse nor **kwargs, so a
caller that needs the softmax normalizer has no way to get it. With
return_lse=True they return (output, lse), lse in [total_q, num_heads].
Asking for it on a path this does not wire fails on the existing
assert lse_out is not None.

The 15 kernel invocations now go through the self.flash_attn_* attributes
__init__ already assigns, so a subclass can substitute a different FA4 build;
that also retires the module-level import those call sites used.

Tests: test_fa4.py gains test_return_lse, which calls the forwards directly --
return_lse is a backend contract and the RadixAttention dispatcher's custom-op
schema cannot carry it -- and checks the output and the LSE against a causal
fp32 reference for MHA extend and GQA decode.

Signed-off-by: Kurt Shuster <kurt@thinkingmachines.ai>
@klshuster
klshuster force-pushed the kurt/fa4-backend-hooks branch from 6077718 to 3715c29 Compare September 1, 2026 23:04
@klshuster

Copy link
Copy Markdown
Contributor Author

@Fridge003 do you need anything else from me to push this through? just rebased onto main again

@klshuster
klshuster requested a review from Fridge003 September 3, 2026 18:10
@Fridge003 Fridge003 added the run-ci CI: run the baseline test suite on this PR label Sep 13, 2026
@alexnails
alexnails merged commit f211171 into sgl-project:main Sep 14, 2026
109 of 154 checks passed
hnyls2002 added a commit that referenced this pull request Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation run-ci CI: run the baseline test suite on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants