feat(kernel): Add gluon kernel for DSA#627
Conversation
Signed-off-by: Max Dawkins <max.dawkins@gmail.com>
Signed-off-by: Max Dawkins <max.dawkins@gmail.com>
Signed-off-by: Max Dawkins <max.dawkins@gmail.com>
Signed-off-by: Max Dawkins <max.dawkins@gmail.com>
Signed-off-by: Max Dawkins <max.dawkins@gmail.com>
Signed-off-by: Max Dawkins <max.dawkins@gmail.com>
Signed-off-by: Max Dawkins <max.dawkins@gmail.com>
Signed-off-by: Max Dawkins <max.dawkins@gmail.com>
Signed-off-by: Max Dawkins <max.dawkins@gmail.com>
Signed-off-by: Max Dawkins <max.dawkins@gmail.com>
Signed-off-by: Max Dawkins <max.dawkins@gmail.com>
Signed-off-by: Max Dawkins <max.dawkins@gmail.com>
abecc91 to
eb17ddb
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb17ddb706
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| softmax_scale: float, | ||
| q_len_per_req: int = 1, | ||
| index_k_cache: torch.Tensor | None = None, | ||
| plan: object | None = None, |
There was a problem hiding this comment.
Accept forwarded seq_lens_2d in Gluon decode top-k
When this kernel is selected through tokenspeed_kernel.ops.attention.dsa_decode_topk, the public wrapper always forwards seq_lens_2d=... (even when it is None). This implementation does not accept that keyword, so the new GFX950 default selection raises TypeError: unexpected keyword argument 'seq_lens_2d' before launching for every decode top-k call; mirror the Triton/DeepGEMM wrapper signature or absorb the unused kwarg.
Useful? React with 👍 / 👎.
| out: torch.Tensor | None = None, | ||
| lens_out: torch.Tensor | None = None, | ||
| ) -> tuple[torch.Tensor, torch.Tensor]: | ||
| del plan, q_len_per_req |
There was a problem hiding this comment.
Honor q_len_per_req for request-scoped decode inputs
For spec-verify decode with q_len_per_req > 1, the public contract passes seq_lens and block_table per request, and each token's visible length is derived from seq_lens[req] - (q_len_per_req - 1) + j. Dropping q_len_per_req makes the wrapper require one seq_lens/block_table row per token and the kernel indexes them by token, so valid multi-token requests that this kernel advertises in its traits are either rejected or read the wrong request row.
Useful? React with 👍 / 👎.
| [BLOCK_H, D_V], | ||
| [1, 0], | ||
| ) | ||
| sh_qrope: gl.constexpr = gl.SwizzledSharedLayout( |
There was a problem hiding this comment.
why are we using swizzled shared not padded shared here?
There was a problem hiding this comment.
I didn't optimize too much yet. I'll try padded shared when I move on to optimizing the kernel further, but I just wanted to land a basic implementation first.
There was a problem hiding this comment.
yeah sounds good. we can double check layout part later
| device=q.device, dtype=torch.int64 | ||
| ).contiguous() | ||
| row_starts = row_starts.to(device=q.device, dtype=torch.int32).contiguous() | ||
| row_ends = row_ends.to(device=q.device, dtype=torch.int32).contiguous() |
There was a problem hiding this comment.
is that true we need to make all of the contiguous? this may have overhead. we can not support stride tensor?
There was a problem hiding this comment.
Right now the kernel assumes flat layout, so for correctness, we should technically be making them contiguous here, but I'll add this to my list to look into for optimization work.
| else: | ||
| max_query_rows = max(1, int(max_logits_bytes) // (max(seq_len_sum, 1) * 4)) | ||
| block_n = 32 | ||
| select_warps = 8 |
There was a problem hiding this comment.
any reason we use 8 warps instead of 4 here?
There was a problem hiding this comment.
Codex claims this is a tuning decision, although I don't really believe Codex did proper benchmarking/tuning to determine this :P
I'll look into this as well when I optimize.
| equal_i32 = equal.to(gl.int32) | ||
| tile_greater = gl.sum(greater_i32, axis=0).to(gl.int32) | ||
| tile_equal = gl.sum(equal_i32, axis=0).to(gl.int32) | ||
| greater_start = gl.atomic_add( |
There was a problem hiding this comment.
we have buffer_atomic_add will that be better?
There was a problem hiding this comment.
I'll experiment with this as well during the optimization phase. Apparently, buffer_atomic_add does not support signed integers (I'd have to use unsigned). I think unsigned should work here too, but I'll wait until I start collecting profiles before I make the change.
Summary
Adds a gluon kernel implementation for DSA, including the scoring, topk, and sparse attention kernels.
Benchmark results
Benchmarks are compared against the triton implementation using GLM shapes at different sequence lengths:
Decode
Prefill
Test Plan
Adds new unit tests. All benchmarks above also match numerics between gluon and triton to within
10e-8.