Skip to content

[FlyDSL] gfx942 fp8_mqa_logits: let _auto_variant choose rows_per_block - #4963

Open
jin-amd wants to merge 6 commits into
ROCm:mainfrom
jin-amd:gfx942-fp8-mqa-logits-rows-per-block
Open

jin-amd wants to merge 6 commits into
ROCm:mainfrom
jin-amd:gfx942-fp8-mqa-logits-rows-per-block

Conversation

@jin-amd

@jin-amd jin-amd commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Motivation

On gfx942 _auto_variant returns f"mfma_r2_w{wpb}", so rows_per_block is pinned at 2 and
seven of the nine registered variants — including the entire r4 family — can never be
selected. At the shape vLLM actually issues for long-context indexer prefill (seq_len 1024,
seq_len_kv 131072) the pinned pick is the median of the nine by speed, with the best 1.65x
faster. vLLM chunks prefill to fit VLLM_SPARSE_INDEXER_MAX_LOGITS_MB (512 MB), which caps
seq_len at 1024 at that context, so the selector's own seq_len >= 2048 branch can never
fire there. Since vllm-project/vllm#49544 routes gfx942 fp8_mqa_logits to this kernel by
default, every MI300X/MI325X user inherits it.

Technical Details

Sweeping all nine variants over 180 shapes surfaced two separable effects.

RPB tracks the logits element count, not seq_len. Over
seq_len ∈ [1, 8192] × seq_len_kv ∈ [1024, 262144] the boundaries land on the same value of
seq_len * seq_len_kv at all six contexts: r1 below 2^19 (27/27 shapes), r2 at 2^19 (6/6),
r4 from 2^21 up (38/38), with 2^20 a transition band split 3/3.

RPB must also divide seq_len. When it does not, the launcher pads with four torch.cat
calls. That is a flat ~44 us of host-side overhead, independent of seq_len_kv: at seq_len 1,
seq_len_kv 131072, r1 runs 23.1 us and r2 67.8 us, of which the four cats measure 44.1 us
(0.7 us unexplained), and pre-padding the input by hand recovers all of it. So the cost is not
proportional to the wasted rows — one padded row of two costs the same as three of four — and the
pinned r2 walks into it on every odd seq_len, 37 of the 180 shapes swept, at a median 2.97x
penalty.

The step-down to a divisor has to be gated to small shapes, where the kernel is cheap relative to
44 us. Above the threshold the reuse wins even when padding: at seq_len 1025, seq_len_kv
131072, the dividing r1 takes 3880 us against 2601 us for a padded r2.

elems = seq_len * seq_len_kv
if elems < 2**19:
    rpb = 1
elif elems < 2**21:
    rpb = 2 if seq_len % 2 == 0 else 1
else:
    rpb = 4
wpb = 2 if (seq_len >= 2048 and seq_len_kv >= 8192) else 4

waves_per_block is deliberately left unchanged. It is worth a few percent at most here, and
unlike RPB its optimum moves with the head count — at 64 heads the current rule costs 1.61x worst
case where a fixed w4 costs 1.07x — so it warrants its own sweep rather than a change fitted to
one head count.

Test Plan

MI325X (gfx942), all nine variants timed at 180 shapes (1620 timings): a power-of-two
seq_len × seq_len_kv grid, plus three held-out sets the rule was not fitted on —
non-power-of-two shapes, a fine seq_len sweep through the divisibility cliff, and head counts
16 and 64. Selector changes were verified end to end through flydsl_fp8_mqa_logits(variant=None)
with no FLYDSL_FP8_MQA_LOGITS_VARIANT set, so the selector itself is under test. Every variant's
output was compared against a reference at every shape.

Test Result

Old pick vs new, no env override:

seq_len seq_len_kv before us after us speedup
1024 131072 r2_w4 2598.7 r4_w4 1623.5 1.60x
1025 131072 r2_w4 2609.3 r4_w4 1594.0 1.64x
700 50000 r2_w4 661.6 r4_w4 440.3 1.50x
333 12000 r2_w4 118.4 r4_w4 97.1 1.22x
3 131072 r2_w4 66.4 r1_w4 28.1 2.36x
1 1024 r2_w4 66.2 r1_w4 22.2 2.98x

No shape measured regresses; the smallest gain is 1.04x. Scored against the fastest of the nine
at each shape, pooled over the held-out data, the geometric mean cost falls from 1.45x to 1.03x
and the worst case from 3.17x to 1.41x.

Logits are bitwise identical across all nine variants at all 180 shapes, so this is purely a
blocking/occupancy change with no numerical trade-off.

End to end on 8x MI325X, TP8, GLM-5.2-FP8, 131072 in / 1024 out, concurrency 8: median TPOT
improves 7.22% and output throughput 6.60%. This kernel is 16.8% of GPU time at that point, the
largest single item in the profile.

Submission Checklist

jin-amd and others added 2 commits August 24, 2026 07:40
_auto_variant returned f"mfma_r2_w{wpb}", so rows_per_block was pinned at 2 and
seven of the nine registered variants -- including every member of the r4 family
-- could never be selected.

r4 amortizes each KV tile load over twice as many query rows and is faster from
seq_len 8 upward. The gap is widest exactly where it costs most: vLLM chunks
indexer prefill to fit VLLM_SPARSE_INDEXER_MAX_LOGITS_MB (512 MB), which caps
seq_len at 1024 when seq_len_kv is 131072, so the existing seq_len >= 2048 branch
cannot fire at long context and every such call took mfma_r2_w4 -- the median of
the nine by speed, with the best 1.65x faster.

Measured on MI325X (gfx942), seq_len_kv 131072, best variant vs the r2 pick:

  seq_len    1   r2 25.2 us    r4 79.1 us     r4 3.1x worse
  seq_len    4   r2 24.3 us    r4 31.5 us     r4 1.3x worse
  seq_len    8   r2 34.0 us    r4 32.9 us     r4 1.03x better
  seq_len   16   r2 56.5 us    r4 47.5 us     r4 1.19x better
  seq_len 1024   r2 2564.3 us  r4 1520.3 us   r4 1.69x better

Below seq_len 8 the host padding of seq_len up to a multiple of RPB dominates --
at seq_len 1 an r4 kernel computes 4 rows to obtain 1 -- so r2 is kept there and
behaviour for those shapes is unchanged.

Logits are bitwise identical across all variants at every shape tested, so this
is purely a blocking/occupancy change.

End to end on 8x MI325X, TP8, GLM-5.2-FP8, 131072 in / 1024 out, concurrency 8:
median TPOT improves 7.22% and output throughput 6.60%. This kernel is 16.8% of
GPU time at that point.

Signed-off-by: Jin Tao <jin.tao@amd.com>
…t a divisor

Refines the previous commit's rule after a 2-D sweep. That rule keyed RPB off
seq_len alone with a crossover measured only at seq_len_kv=131072; sweeping the
other contexts shows the crossover is not a seq_len threshold at all, and that a
second effect was being read as one.

RPB tracks the logits element count. Over seq_len 1..8192 x seq_len_kv
1024..262144 on MI325X, the boundaries land on the same element count at every
context: RPB=1 wins below 2**19 elements (27/27 shapes), RPB=2 at 2**19 (6/6),
RPB=4 from 2**21 up (38/38), with 2**20 a transition band split 3/3. Keying off
seq_len instead put the previous rule on the wrong side at low context: at
seq_len 16, seq_len_kv 1024 it chose RPB=4 and ran 1.26x slower than RPB=1.

RPB must also divide seq_len. When it does not, the launcher pads with four
torch.cat calls; that is a flat ~44 us of host-side overhead, independent of
seq_len_kv, and it is the whole of the "small seq_len" penalty the previous
commit attributed to wasted rows. At seq_len 1, seq_len_kv 131072: RPB=1 23.1 us,
RPB=2 67.8 us, of which the four cats are 44.1 us and pre-padding by hand
recovers all of it (21.9 us). So the penalty is not proportional to the padding
-- 1 wasted row of 2 costs the same as 3 of 4 -- and it applies to every odd
seq_len, which the old rule sent to RPB=2 unconditionally.

Stepping down to a divisor is only right while the kernel is cheap relative to
that fixed cost, so it is gated to the same 2**21 elements: at seq_len 1025,
seq_len_kv 131072 the dividing RPB=1 takes 3880 us against 2601 us for a padded
RPB=2.

Measured on MI325X, no FLYDSL_FP8_MQA_LOGITS_VARIANT set, old pick vs new:

  seq_len  seq_len_kv        old         new   speedup
     1024      131072   r2_w4 2598.7   r4_w4 1623.5    1.60x
     1025      131072   r2_w4 2609.3   r4_w4 1594.0    1.64x
      512      131072   r2_w4 1190.1   r4_w4  795.3    1.50x
      700       50000   r2_w4  661.6   r4_w4  440.3    1.50x
      333       12000   r2_w4  118.4   r4_w4   97.1    1.22x
       16      131072   r2_w4   58.3   r4_w4   47.4    1.23x
        3      131072   r2_w4   66.4   r1_w4   28.1    2.36x
        1      131072   r2_w4   69.4   r1_w4   23.2    2.99x
        1        1024   r2_w4   66.2   r1_w4   22.2    2.98x

No shape measured regresses; the smallest gain is 1.04x. Against the best of the
nine variants at each shape, pooled over held-out data (non-power-of-two shapes,
a fine seq_len sweep, and head counts 16 and 64), the geometric mean cost falls
from 1.45x to 1.03x and the worst case from 3.17x to 1.41x.

Logits are bitwise identical across all nine variants at all 180 shapes swept
(1620 timings), so this remains purely a blocking/occupancy change.

WPB is deliberately left alone. It is worth a few percent at most here, and
unlike RPB its optimum moves with the head count -- at 64 heads the current
WPB rule costs 1.61x worst case where a fixed WPB=4 costs 1.07x -- so it needs
its own sweep rather than a change fitted to one head count.

Signed-off-by: Jin Tao <jin.tao@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@jin-amd
jin-amd requested review from a team and a lite review from Copilot August 24, 2026 08:49
@github-actions

Copy link
Copy Markdown
Contributor

🏷️ CI Guide

Runs automatically on every PR:

  • ✅ Pre-checks (submodule verification, code formatting)
  • ✅ Aiter op tests (gfx942 + gfx950)
  • ✅ Triton tests on MI35X (only when aiter/ops/triton/** or related paths are changed)

Extended tests (opt-in via labels):

Label Tests
ci:gfx1250-ffm-triton Run the five-shard gfx1250 FFM Triton test suite
ci:triton-300x Run an additional Triton test job on MI300X in PRs; main branch always runs both MI35X and MI300X
ci:sglang SGLang integration tests: DeepSeek-R1-MXFP4 accuracy, Qwen 3.5 accuracy
ci:atom ATOM benchmark: DeepSeek-R1-0528, GPT-OSS-120B
ci:atom_full ATOM accuracy suite for PR and main models from ATOM models_accuracy.json
ci:vllm vLLM benchmark: GPT-OSS-120B, DeepSeek-R1-0528, Kimi-K2.5
ci:all All standard extended tests (excludes ci:atom_full)

Only add ci:atom_full for FlyDSL or Triton upgrades.
Add labels via the sidebar or gh pr edit 4963 --add-label <label>

PR title tags:
Component tags ([Triton/Gluon], [HIP], [CK], [ASM], ...) are added to the PR title automatically from the changed files and re-synced on every push — change-type tags like [fix]/[Perf] and op tags like [MLA] are left untouched. Add the no-auto-title label to opt this PR out of title tagging.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the gfx942 FlyDSL fp8_mqa_logits kernel variant auto-selector so rows_per_block (RPB) is no longer effectively pinned to 2, allowing the faster r4 variants to be selected for large “long-context prefill” shapes (and selecting r1 for small shapes where padding overhead dominates).

Changes:

  • Add element-count-based thresholds (seq_len * seq_len_kv) to choose RPB ∈ {1, 2, 4}, including a divisibility-aware fallback to avoid padding overhead on small/medium shapes.
  • Keep the existing WPB heuristic unchanged, while returning the full mfma_r{rpb}_w{wpb} variant tag so r1/r4 families are reachable.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@jin-amd

jin-amd commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

@vpietila-amd @valarLip — would appreciate a look when you have a moment.

Short version: on gfx942 _auto_variant returns f"mfma_r2_w{wpb}", so rows_per_block is
pinned at 2 and seven of the nine registered variants are unreachable. Choosing it from the
logits element count instead is 1.60x at the long-context prefill shape vLLM actually issues,
and up to 2.99x at small or odd seq_len. Outputs stay bitwise identical, and
test_flydsl_fp8_mqa_logits.py shows no new failures (368-case default matrix plus a 160-case
matrix that reaches r4, run patched and unpatched).

@samremes samremes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please trim the comments down, they don't need to tell the whole story how it ended up with these. State the logic very briefly.

Comment on lines +401 to +402
_RPB2_MIN_ELEMS = 2**19
_RPB4_MIN_ELEMS = 2**21

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could move this inside the _auto_variant as they are very local to the heuristic.

@zufayu
zufayu requested a review from yadaish August 25, 2026 01:22
Move RPB element-count thresholds into _auto_variant and shorten the
docstring; logic unchanged.

Signed-off-by: Jin Tao <jin.tao@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Copilot AI review requested due to automatic review settings August 25, 2026 07:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment on lines +391 to +395
"""Pick (RPB, WPB) from the problem shape.

RPB from ``seq_len * seq_len_kv`` thresholds; step down to a divisor of
``seq_len`` when padding overhead would dominate. WPB unchanged.
"""
Comment on lines +396 to +406
rpb2_min_elems = 2**19
rpb4_min_elems = 2**21
elems = seq_len * seq_len_kv
if elems < rpb2_min_elems:
rpb = 1
elif elems < rpb4_min_elems:
rpb = 2 if seq_len % 2 == 0 else 1
else:
rpb = 4
wpb = 2 if (seq_len >= 2048 and seq_len_kv >= 8192) else 4
return f"mfma_r2_w{wpb}"
return f"mfma_r{rpb}_w{wpb}"
@jin-amd

jin-amd commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Please trim the comments down, they don't need to tell the whole story how it ended up with these. State the logic very briefly.

Thank you @samremes Both addressed in d863e4d. The comments are trimmed down to a two-line docstring that just states the rule (RPB from seq_len * seq_len_kv thresholds, stepped down to a divisor of seq_len when padding overhead would dominate; WPB unchanged), and the two element-count constants are now locals inside _auto_variant. The rationale and measurements stay in the commit message. No logic change.

@jin-amd
jin-amd requested a review from samremes August 31, 2026 08:19
Copilot AI review requested due to automatic review settings August 31, 2026 08:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings September 2, 2026 07:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is localized to variant selection logic, matches the available registered variant tags, and does not alter kernel math/output semantics.

Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 7, 2026 09:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is a small, self-contained update to variant selection logic that only returns already-registered variants and preserves existing WPB behavior.

Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants