cherry-pick: #4199 fix(xqa): PDL load ordering and SM90 fp8 draft-mask dispatch - #4232
Conversation
…shinfer-ai#4199) ## 📌 Description Follow-up to flashinfer-ai#4137, addressing [the review feedback](flashinfer-ai#4137 (comment)) that arrived after auto-merge. Thanks @qsang-nv for the detailed analysis. Issues: - Under programmatic dependent launch (PDL), the generic XQA kernel (`csrc/xqa/mha.cu`) read `q_cu_seq_lens` and the scale tensors before the acquire that makes a producer kernel's writes visible. `q_cu_seq_lens` drives the output row offset, so a stale read could write into the wrong request's rows. - On SM90 with fp8 KV cache, the small-batch layout of the Hopper XQA kernel (`csrc/xqa/mha_sm90.cu`, used when `q_seq_len * head_group_ratio <= 32`) hardcodes a causal draft mask, so a full draft mask silently returned causal results. Existing refchecks use random data and cannot tell the two modes apart within fp8 tolerance. - The XQA trace template declared a `q_cu_seq_lens` input that the trace reference silently dropped. Fixes: - Move the `q_cu_seq_lens` and scale-tensor loads below the PDL acquire in `mha.cu`. - Extend the SM90 fp8 fallback so small-batch speculative decode also runs on the generic kernel, matching the sliding-window and ragged-Q cases. Restoring the Hopper fast path is tracked in flashinfer-ai#4198. - Add a deterministic mask test: zero Q and K make each output row an exact mean of the visible V values, so any deviation from the requested mask fails loudly on every architecture. It runs two shapes, one that falls back to the generic kernel and one that stays on the Hopper kernel on SM90 fp8. - Remove `q_cu_seq_lens` from the trace template until the trace reference supports ragged Q. - Normalize the ragged-Q module key inside the module getter, and build a separate ragged variant only when an SM90 target is compiled; on other targets it is identical to the uniform module. - Document the SM90 fp8 fallback in the `xqa()`, `xqa_batch_decode_with_kv_cache`, and `trtllm_batch_decode_with_kv_cache` docstrings. ## 🔍 Related Issues flashinfer-ai#4198 (restore the Hopper fp8 fast path for speculative decode). Review thread: flashinfer-ai#4137. ## 🚀 Pull Request Checklist Thank you for contributing to FlashInfer! Before we review your pull request, please make sure the following items are complete. ### ✅ Pre-commit Checks - [x] I have installed `pre-commit` by running `pip install pre-commit` (or used your preferred method). - [x] I have installed the hooks with `pre-commit install`. - [x] I have run the hooks manually with `pre-commit run --all-files` and fixed any reported issues. ## 🧪 Tests - Added `test_xqa_batch_decode_mask_mode_deterministic` (causal/full mask, bf16/fp8 KV, two head-group shapes) with exact expected outputs. - On SM120 (RTX 5080): the new test, the ragged-Q and sliding-window suites (256 cases), and the trace suite (970 cases) pass. - SM90/SM100 are covered by CI. On SM90 fp8 the new test's `head_grp_size=16` shape exercises the Hopper kernel and the `head_grp_size=4` shape exercises the widened fallback. ## Reviewer Notes - Behavior change on SM90 with fp8 KV cache: small-batch speculative decode now runs on the generic kernel, including causal masks. The mask lives on the device, so dispatch cannot check its content without a sync. The Hopper kernel previously returned causal results regardless of the requested mask; flashinfer-ai#4198 restores that fast path. - The `SPEC_Q_SEQ_LEN` build specialization is no longer reachable at run time but is kept: the fast-path restoration in flashinfer-ai#4198 re-enables it. - Ragged workloads cannot be represented in trace dumps until the trace reference supports ragged Q; removing the inert input beats shipping a wrong reference. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved speculative decoding handling for variable-length queries, including safer execution ordering for mask/sequence offsets. * Refined SM90 FP8 KV-cache kernel selection, extending the conditions that fall back to the generic kernel (ragged queries, attention sinks, sliding-window, and small head-group bound). * Improved compilation/caching behavior to better match supported speculative-decoding configurations. * **Documentation** * Updated the XQA documentation to clarify when the generic kernel is used for SM90 FP8 speculative decoding. * **Tests** * Added a deterministic test covering causal/full speculative-decoding masks for both BF16 and FP8 KV caches. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
release-v0.6.16Source
6812ddb4Test plan
tests/attention/test_xqa_batch_decode.pyon SM90 / relevant arches