-
Notifications
You must be signed in to change notification settings - Fork 7.3k
Fix flash/sage varlen prep under torch.compile with dynamic shapes #14568
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e74d38e
f3bfd56
2628666
669b8cc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -599,13 +599,12 @@ def _prepare_for_flash_attn_or_sage_varlen_without_mask( | |
| ): | ||
| seqlens_q = torch.full((batch_size,), seq_len_q, dtype=torch.int32, device=device) | ||
| seqlens_k = torch.full((batch_size,), seq_len_kv, dtype=torch.int32, device=device) | ||
| cu_seqlens_q = torch.zeros(batch_size + 1, dtype=torch.int32, device=device) | ||
| cu_seqlens_k = torch.zeros(batch_size + 1, dtype=torch.int32, device=device) | ||
| cu_seqlens_q[1:] = torch.cumsum(seqlens_q, dim=0) | ||
| cu_seqlens_k[1:] = torch.cumsum(seqlens_k, dim=0) | ||
| max_seqlen_q = seqlens_q.max().item() | ||
| max_seqlen_k = seqlens_k.max().item() | ||
| return (seqlens_q, seqlens_k), (cu_seqlens_q, cu_seqlens_k), (max_seqlen_q, max_seqlen_k) | ||
| # Built with arange instead of cumsum(full(...)): inductor rewrites that pattern into | ||
| # `arange * fill_value`, which raises under dynamic shapes because the fill value is a | ||
| # symbolic sequence length. The lengths are uniform here, so arange is also cheaper. | ||
| cu_seqlens_q = torch.arange(0, (batch_size + 1) * seq_len_q, seq_len_q, dtype=torch.int32, device=device) | ||
| cu_seqlens_k = torch.arange(0, (batch_size + 1) * seq_len_kv, seq_len_kv, dtype=torch.int32, device=device) | ||
| return (seqlens_q, seqlens_k), (cu_seqlens_q, cu_seqlens_k), (seq_len_q, seq_len_kv) | ||
|
|
||
|
|
||
| def _prepare_for_flash_attn_or_sage_varlen_with_mask( | ||
|
|
@@ -616,13 +615,13 @@ def _prepare_for_flash_attn_or_sage_varlen_with_mask( | |
| ): | ||
| seqlens_q = torch.full((batch_size,), seq_len_q, dtype=torch.int32, device=device) | ||
| seqlens_k = attn_mask.sum(dim=1, dtype=torch.int32) | ||
| cu_seqlens_q = torch.zeros(batch_size + 1, dtype=torch.int32, device=device) | ||
| # Queries are uniform, so arange (see the no-mask helper: cumsum(full(...)) breaks inductor | ||
| # under dynamic shapes). Keys are data-dependent and keep the cumsum. | ||
| cu_seqlens_q = torch.arange(0, (batch_size + 1) * seq_len_q, seq_len_q, dtype=torch.int32, device=device) | ||
| cu_seqlens_k = torch.zeros(batch_size + 1, dtype=torch.int32, device=device) | ||
| cu_seqlens_q[1:] = torch.cumsum(seqlens_q, dim=0) | ||
| cu_seqlens_k[1:] = torch.cumsum(seqlens_k, dim=0) | ||
| max_seqlen_q = seqlens_q.max().item() | ||
| max_seqlen_k = seqlens_k.max().item() | ||
| return (seqlens_q, seqlens_k), (cu_seqlens_q, cu_seqlens_k), (max_seqlen_q, max_seqlen_k) | ||
| return (seqlens_q, seqlens_k), (cu_seqlens_q, cu_seqlens_k), (seq_len_q, max_seqlen_k) | ||
|
Comment on lines
-621
to
+624
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not return Cc: @zhtmike do you have any comments here?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think using But according to the PR description, the mask path is still broken under torch compile? Plus, I think we need a test to guard this
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. queries are always full length, so The masked path compiles too and is not broken. The remaining .item() is on the key side only, and it doesn't block fullgraph=True. On torch 2.13.0+cu129 with this PR: The description only meant that the key-side .item() stays, since padded lengths are data-dependent. |
||
|
|
||
|
|
||
| def _prepare_for_flash_attn_or_sage_varlen( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Behavioral edge case worth noting:
torch.arangewithstep=0raisesRuntimeError: step must be nonzero, so a zero-length query/key sequence now errors where the previouscumsum(zeros)path returned an all-zerocu_seqlens. I don't think any dispatcher path can reach here withseq_len == 0, so this is informational rather than blocking — just flagging it since the PR claims strict numerical/behavioral equivalence.