[PyTorch] Build the attention padding mask without host syncs - #24
Closed
pggPL wants to merge 2 commits into
Closed
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. Same result -- verified against a reference implementation over mixed, empty and full-length sequences -- with no synchronization. The new test also covers cu_seqlens buffers longer than batch_size + 1, which inference allocates, and asserts the absence of synchronization via torch.cuda.set_sync_debug_mode. Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
The rewrite is already covered by the thd and padding cases in test_attention.py and by test_kv_cache.py. Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
Owner
Author
|
Superseded by NVIDIA#3268, opened upstream with the same change squashed into a single commit. |
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,backends.py, when"padding"is in the mask type and the user passed cumulative sequence lengths rather than a mask).It was written as a loop over the batch that used 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 a design decision that is being second-guessed here. The construction was written as reference-mask code on the test side in NVIDIA#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 NVIDIA#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 was checked exhaustively against the previous implementation 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 produces 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: those suites pass unchanged.Split out of a larger torch.compile change for
DotProductAttention, where the same loop is also untraceable by dynamo (the sequence lengths become data-dependent, unbacked SymInts). This part stands on its own as an eager-mode fix and is reviewable without any torch.compile context.Checklist:
🤖 Generated with Claude Code