webgpu: Fix TurboQuant quantized KV cache for batch>1 with per-batch seqlens - #29752
Draft
qjia7 wants to merge 1 commit into
Draft
webgpu: Fix TurboQuant quantized KV cache for batch>1 with per-batch seqlens#29752qjia7 wants to merge 1 commit into
qjia7 wants to merge 1 commit into
Conversation
…seqlens The TurboQuant copy-to-quantized-KV-cache kernels previously read seqlen_k[0] for every batch, so batches 1..N-1 used the wrong past sequence length and produced corrupted output. genai decode runs batch_size==1 so this was not caught, but right-padded batched GQA (batch>1) needs per-batch seqlens. Fixes: - turbo_quant_hadamard.cc / turbo_quant_hadamard.wgsl.template: remove the batch_size==1 restriction and read seqlen_k[batch]. batch/head/seq are unflattened from the uniform copy sequence length (matching the host dispatch layout), then total_seq_length is derived per batch from seqlen_k[batch]. - turbo_quant_fused_rotary_hadamard.wgsl.template: compute the batch id (per Q/K/V workgroup type) before accessing seqlen_k, then read seqlen_k[batch]. Tests (WebGPU, TurboQuant-4bit EP): - WebGPU_TurboQuant_Decode_MultiBatch_UsesPerBatchSeqlensK (rotary path, fused rotary+Hadamard kernel) - WebGPU_TurboQuant_Decode_MultiBatch_NoRotary_UsesPerBatchSeqlensK (plain Hadamard kernel) Both use a swap-invariance check: running batches [A,B] with seqlens [sA,sB] and the physically-swapped [B,A] with [sB,sA] must yield swapped outputs; this fails if the kernel reads seqlen_k[0] for all batches.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes the WebGPU TurboQuant (quantized KV cache) path to correctly honor per-batch seqlens_k[b] when batch_size > 1, removing the previous hard restriction to batch_size == 1 and adding regression coverage to catch seqlen indexing mistakes.
Changes:
- Update TurboQuant WGSL kernels to index
seqlen_k[batch](notseqlen_k[0]) and to computebatch/head/seqconsistently with the host dispatch layout. - Remove the
batch_size == 1validation guard in TurboQuant WebGPU host code. - Add multi-batch “swap-invariance” decode tests to validate per-batch
seqlens_kbehavior for both rotary and non-rotary TurboQuant copy paths.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| onnxruntime/test/contrib_ops/group_query_attention_op_test.cc | Adds swap-invariance multi-batch decode tests for TurboQuant rotary and non-rotary paths. |
| onnxruntime/contrib_ops/webgpu/bert/turbo_quant_hadamard.wgsl.template | Unflattens (batch, head, seq) using uniform dispatch layout and switches to seqlen_k[batch] for per-batch totals. |
| onnxruntime/contrib_ops/webgpu/bert/turbo_quant_hadamard.cc | Removes the previous batch_size == 1 rejection when seqlen_k is provided. |
| onnxruntime/contrib_ops/webgpu/bert/turbo_quant_fused_rotary_hadamard.wgsl.template | Computes batch id before reading seqlen_k, switching total length to seqlen_k[batch]. |
Comment on lines
+67
to
+68
| // uniforms.kv_sequence_length is the sequence length of the new key/values. | ||
| let past_seq_length = total_seq_length - uniforms.kv_sequence_length; |
Comment on lines
63
to
67
| #if use_seqlen_k | ||
| let total_seq_length = u32(seqlen_k[0u]) + 1u; | ||
| let total_seq_length = u32(seqlen_k[batch]) + 1u; | ||
| #else | ||
| let total_seq_length = uniforms.total_sequence_length; | ||
| #endif |
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.
Summary
seqlen_k[batch]instead ofseqlen_k[0], so each batch uses its own past sequence length.batch_size == 1restriction inTurboQuantCopyToQuantizedKVCacheandTurboQuantApplyRotaryAndCopyToQuantizedKVCache.turbo_quant_hadamard.wgsl.template, unflattenbatch/head/seqfrom the uniform copy sequence length (matching the host dispatch layout), then derive per-batchtotal_seq_lengthfromseqlen_k[batch].turbo_quant_fused_rotary_hadamard.wgsl.template, compute the batch id (per Q/K/V workgroup type) before accessingseqlen_k.Motivation
PR #29247 enabled FlashAttention for batched GQA with right-padded prompts, and the later TurboQuant work added a quantized KV cache path. However, the TurboQuant KV-cache copy kernels read
seqlen_k[0]for every batch, so batches1..N-1used the wrong past length and produced corrupted output. This was not caught because genai decode runsbatch_size == 1; right-padded batched GQA (batch > 1) with quantized KV cache exercises the broken path.Test plan
WebGPU_TurboQuant_Decode_MultiBatch_UsesPerBatchSeqlensK(rotary → fused rotary+Hadamard kernel)WebGPU_TurboQuant_Decode_MultiBatch_NoRotary_UsesPerBatchSeqlensK(non-rotary → plain Hadamard kernel)[A,B]with seqlens[sA,sB]and the physically-swapped[B,A]with[sB,sA]must yield swapped outputs; this fails if the kernel readsseqlen_k[0]for all batches.GroupQueryAttentionTestsuite passes: 72 passed, 14 skipped (CUDA-only, no CUDA device); 0 failures.lintrunner -a: no issues.