[PyTorch] Build the attention padding mask without host syncs - #3268
Merged
Conversation
get_padding_mask converts cu_seqlens into the boolean padding mask that UnfusedDotProductAttention consumes. It looped over the batch and used each sequence length as a Python list multiplier, which reads a GPU tensor on the host: one device synchronization per sequence, plus a torch.cat per sequence, on every forward pass that takes this path. Build the mask on the device instead: positions >= seqlens, broadcast over the batch. Equivalence with the previous implementation was checked exhaustively over every combination of sequence lengths for small batch sizes, for both self and cross attention. Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
Contributor
Greptile SummaryThe PR vectorizes padding-mask construction to avoid per-sequence host synchronization while preserving existing batch slicing and CUDA mask placement.
Confidence Score: 5/5The PR appears safe to merge. The previously reported device mismatch is resolved by transferring the completed padding mask to CUDA before it is consumed, and no blocking failure remains. Important Files Changed
Reviews (2): Last reviewed commit: "Keep the padding mask on the attention d..." | Re-trigger Greptile |
8 tasks
cu_seqlens is not required to be a CUDA tensor, and the previous implementation always moved the finished mask to CUDA. Building it on cu_seqlens' device regressed bshd/sbhd callers that pass CPU cu_seqlens: the mask reached the attention scores on the wrong device. Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
cyanguwa
approved these changes
Jul 28, 2026
Collaborator
Author
|
/te-ci pytorch |
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.
Description
get_padding_maskturnscu_seqlensinto the boolean padding mask thatUnfusedDotProductAttentionconsumes (its only caller, inbackends.py, when"padding"is in the mask type and the caller supplied cumulative sequence lengths rather than a mask).It is written as a loop over the batch that uses each sequence length as a Python list multiplier:
seqlens_q[i]is an element of a CUDA tensor, so[False] * seqlens_q[i]forces an implicit.item(): one device synchronization per sequence, plus onetorch.catper sequence, on every forward pass that takes this path.This is not second-guessing a design decision. The construction was written as reference-mask code on the test side in #818, where a host round-trip per sequence costs nothing, and was lifted into the library when
get_padding_maskwas introduced for KV caching in #1355. Its twin is still intests/pytorch/attention/test_attention.pyand stays there — on the test side it is perfectly fine. This PR only removes the consequence of that move onto a per-forward path.The mask is
positions >= seqlens, which needs no host round-trip:Type of change
Changes
get_padding_mask: vectorized, device-side construction; no synchronization, one kernel instead ofbatch_sizeiterations.[1 : batch_size + 1]is deliberate and preserves existing behaviour:cu_seqlensmay hold more thanbatch_size + 1entries, because inference allocates it for the maximum batch size. The old loop ignored the extra entries by iteratingrange(batch_size).cu_seqlens.deviceinstead of a hard-coded"cuda".DotProductAttentionasserts CUDA inputs, so this is not reachable from there.Equivalence
The input space is discrete and small, so equivalence with the previous implementation was checked exhaustively rather than sampled — values, shape, dtype and device:
[0, max_seqlen]formax_seqlen1..8 andbatch_size1..4, with and without acu_seqlensbuffer longer thanbatch_size + 1— 35368 cases, 0 mismatches;max_seqlen_q,max_seqlen_kv1..5 andbatch_size1..3 — 202100 cases, 0 mismatches.The only inputs where the two differ are invalid ones: a sequence longer than
max_seqlenmade the old code raise fromtorch.cat(rows of unequal length), and now yields an all-Falserow.Testing
No new tests. The function is already exercised by the
thdandpaddingcases intests/pytorch/attention/test_attention.py(the-k thdsubset alone calls it 10 times, with numerics compared against a reference) and bytests/pytorch/attention/test_kv_cache.py, which is what covers the longer-than-batchcu_seqlensbuffers. Verified on RTX Ada (sm89):test_attention.py,test_kv_cache.pyandtest_gqa.pypass with no new failures relative tomain.This is split out of a larger
torch.compilechange forDotProductAttention, which will be submitted separately — there the same loop is additionally untraceable by dynamo, because the sequence lengths become data-dependent (unbacked) SymInts. This part stands on its own as an eager-mode fix and is reviewable without anytorch.compilecontext.Checklist: