Gate short-query ragged prefill on gfx942, and settle the decode routing question by measuring it by demandal25 · Pull Request #373 · AMD-Ecosystem/flashinfer

demandal25 · 2026-09-14T06:36:42Z

Summary

Takes the gfx942 ragged short-query win that #372 measured and deliberately left unclaimed, and settles the decode routing question #362 left open — by measuring it, and concluding that no decode gate should be built.

What changed

Routing

flashinfer/rocm/arch_caps.py — new _AITER_RAGGED_GATED_Q_LEN / aiter_ragged_gated_q_len, {"gfx942": 16, "gfx950": None}. Separate from the flat-gather table because the mechanism is different and gfx950's answer is "never".
flashinfer/rocm/prefill.py — _aiter_ragged_short_query / _ragged_short_query_reason, a ragged_q_len= kwarg on _auto_select_prefill_backend evaluated last in the elif chain, and the ragged wrapper's own _backend_short_query_demoted lifecycle.

Benchmark

benchmarks/rocm/bench_batch_decode.py — _KV_LENS extended down to 128, a query-head axis, and --batches / --kv-lens / --qo-heads / --output-dir.

Docs

docs/rocm/backends.md — the ragged gate, and the decode exemption restated as a measurement rather than an assertion.

Architecture / design notes

Arming the ragged path was not a matter of passing max_q_len=. Three things had to be their own:

	why
table	gfx950's row is `None` — a measured verdict, not a gap. Both accessors are bare `dict.get`, so an explicit `None` is indistinguishable from a deleted row; the test asserts membership.
reason string	the flat-gather reason blames a copy of the whole KV cache, which ragged never makes. The paged demotion site matches its own string by equality, so sharing one would misreport and cross-trigger.
demotion state	every `_backend_short_query_demoted` site was in the paged wrapper. Without a reset, one short extend strands a wrapper on `fa2` for every later long prefill.

The gate sits last in the selector's elif chain, where #362 put its sibling: ahead of it, a short batch would never reach the capability row or the dtype/layout checks, and backend_fallback_reason would report "too short" on a machine with no AITER installed.

A wrapper-scoped gate fires at most once, and that is the whole bug worth reading this PR for. The first implementation put the gate inside if self._backend == "auto", so a wrapper that resolved to AITER on a long prefill never re-entered the block — every later short plan went to AITER ungated. That is the long-then-short order, which is precisely what this gate exists for: a chat turn on a cached prefix is a short query after a long one, and it is the 4.74× cell. Short-then-long worked, and all four original tests happened to plan short first, so it shipped green through a full suite. The paged wrapper avoids this only because its native-paging probe gives it a second re-check; ragged has no probe, so the re-check is explicit.

Cudagraph is asymmetric and deliberately so: demotion fires freely on a first plan, re-promotion never does, and the late re-check stays off under capture. Re-promoting swaps _cached_module/_plan_info that a captured graph still points at. Blocking demotion too — the obvious reading — would make the gate inert wherever graphs are used. Under capture the first plan decides, now documented alongside the paged wrapper's mirror-image exception.

Benchmark results

Ragged prefill, gfx942 (from #372, the basis for the threshold)

All five measured shapes lose at 16 query tokens; first win at q24.

shape	AITER/fa2 at q16
bs1 / kv1024	1.18×
bs1 / kv8192	4.74×
(five shapes, range)	1.18–4.74×

gfx950 has a shape (bs32/kv2048) favouring AITER at every query length measured, so no gfx950 threshold serves and none is set.

Batch decode, fa2 vs AITER — the measurement that says not to gate

Swept at 0c43abef on MI300X and MI350X, AITER 0.1.20+rocm10.1.0a20260819.3135022 on both, bf16 / head_dim 128 / 8 KV heads / page_size 16, medians over a 1 s repeat budget.

arch	qo heads	cells	AITER loses in	worst for AITER	best for AITER
gfx942	32	30	16	1.70× (b128/kv128)	1.6× faster
gfx950	32	30	15	1.16× (b256/kv128)	1.7× faster
gfx942	64	30	0	0.65×	6.5× faster
gfx950	64	30	0	0.86×	7.7× faster

Three results, none of which supports a gate:

There is no AITER decode "floor" at 0.16 ms. AITER's fixed per-call cost is ~49–51 µs against fa2's ~38–44 µs. The entire losing region is that ~10 µs gap showing up while both kernels are launch-bound.
The losing region is bounded by head count, not length. At 64 query heads AITER wins every cell on both architectures. A gate keyed on batch × kv_len — which the bandwidth model predicts and which an earlier revision of this work proposed — would mis-route 70B and 405B decode by up to 7.7×.
What is left to win is ~5–26 µs per call, on h=32 only. Every launch swept was eager, and eager is also where auto mostly reaches AITER: under graph capture it resolves to fa2 unless the caller passes max_seq_len, which vLLM and SGLang do not. Graph-mode and explicit-aiter launches were not timed. Against a 7× downside, that is not a trade worth making.

Recorded rather than acted on: fa2's decode kernel is capped by query head count and AITER's is not — fa2 holds ~5.3 TFLOPS at h=64 on both arches regardless of shape while AITER reaches 37–41 TFLOPS, e.g. 6.41 ms vs 0.92 ms at b256/kv4096 on gfx950. KV traffic is identical at h=32 and h=64, so this is not bandwidth. Root cause unprofiled; it is a separate item, not this PR's.

Those 64-head timings had no correctness coverage behind them when first posted: test_batch_decode_aiter_vs_fa2 parametrised GQA ratios 1, 4 and 4, so ratio 8 — where AITER's advantage is largest — was unverified, and a fast-but-wrong kernel would have read as a win. (64, 8) is now in the matrix and its 144 cases pass, so the figures above stand on a checked kernel.

Test plan

test_arch_caps.py::TestAiterRaggedQLenGate — per-arch values, unknown-arch disarm, and gfx950 membership so deleting the row cannot pass
test_prefill_decode_dispatch.py — selector arms on gfx942, falls through on gfx950, and the two gates' reason strings differ
test_batch_prefill_kernels.py — ragged declines, long-then-short still demotes, demotion is not sticky, no re-promotion under cudagraph, explicit backend="aiter" survives, and gfx950 keeps AITER
A/B three ways: disarming the table fails test_gfx942_gates_through_16; removing ragged_q_len=self._max_q_len fails 3 of 4 wrapper tests; removing the late re-check fails exactly test_ragged_long_then_short_still_demotes and nothing else
Blast-radius suite (prefill, decode, dispatch, auto-fallback, arch-caps) — caught test_ragged_prefill_auto_demotes_to_fa2 planning at exactly the new threshold and asserting on the wrong reason; fixed the same way Support speculative-decode verify on ROCm, and stop auto routing short-query prefill to AITER #362 fixed its paged sibling
test_batch_decode_aiter.py — GQA ratio 8 added to the parity matrix (144 cases), closing the gap under this PR's own 64-head measurements
gfx942 (MI300X) and gfx950 (MI350X)
pre-commit run -a, /code-review xhigh

Benchmark-script defaults are left as they were: every config is materialised before the first run, so a bare invocation is ~55 GiB resident and extending kv_lens to 16384 would take it to ~116 GiB. The short-KV/multi-head sweep runs behind the flags in two passes, ~23 and ~45 GiB.

Rollout

Both behaviour changes are silent to callers: a backend="auto" ragged wrapper at max_q_len <= 16 on gfx942 now gets fa2, with no API change and no opt-out short of backend="aiter". backend_fallback_reason becomes non-None where it was None for those calls.

Both decode sweeps started at kv_len 1024 -- the testlist at batch {1,16,64} x s_kv {1024,8192}, this script at batch {1,8,32,128,256} x kv_len {1024..8192}. Neither reaches the corner where a fixed per-call cost stops being amortised, which is where the fa2/aiter crossover has to be if there is one. #362 recorded decode as deliberately ungated on the strength of "AITER wins 7 of 8 measured rows"; one row already lost, and none of the eight sits below 1024. Extends kv_len down to 128 and adds a query-head axis. AITER's partition workspace and reduce cost both scale with num_heads, so a threshold fitted at 32 heads would steer the 64- and 128-head models on an extrapolation -- the failure aiter_flat_gather_gated_q_len's docstring warns about for its own per-arch scalar. Every config is planned before the first run, so the widened default grid is ~106 GiB of KV resident at once. That nominally fits an MI300X and reliably will not on a shared one, hence --batches/--kv-lens/ --qo-heads: run it in passes rather than allocating the cross product. --output-dir for the same reason a long run belongs on a pinned commit: output defaulted beside the script, inside the source tree, so writing it dirtied the very checkout the run was reading. Verified the short cells are measurable rather than silently degenerate: page_size 16 <= AITER's 32 ceiling, kv_len 128 is 8 pages, and every cell from 128 to 16384 yields npar_loops = 1, so the whole sweep compiles to one AITER variant and no cell changes kernel underneath the comparison. Co-Authored-By: Claude <noreply@anthropic.com>

#372 measured this and deliberately left it unclaimed: gfx942 loses all five ragged shapes at 16 query tokens (1.18-4.74x, first win at q24), while gfx950 has a shape -- bs32/kv2048 -- favouring AITER at every length measured, so no gfx950 threshold serves. This takes the gfx942 win and leaves gfx950 ungated. The shape it fires on is a short query against a long context: the 4.74x cell is bs1/kv8192/q16, which is a multi-turn chat turn landing on a cached prefix. It is not chunked prefill (chunks are 512-2048, far above the threshold) and not EAGLE verify (tree drafts carry custom_mask, declined at position 2 of the elif chain, long before any length gate). Three things this needed beyond arming the existing gate, none of them one-liners: Its own table. aiter_ragged_gated_q_len is separate from aiter_flat_gather_gated_q_len because the mechanism is different -- no gather, just fixed kernel cost -- and because gfx950's entry is None. That None is a measured verdict, not a gap, and since both accessors are bare dict.get it is indistinguishable from a deleted row; the test asserts membership so dropping gfx950 cannot pass silently. Its own reason string. The flat-gather reason blames a copy of the whole KV cache, which ragged never makes -- mha_varlen_fwd takes contiguous KV. Reusing it would misreport the cause and collide with the paged demotion site, which recognises its own string by equality. Its own demotion bookkeeping. Every _backend_short_query_demoted site was inside the paged wrapper; the ragged wrapper had none. Without the reset, one short extend would strand a wrapper on fa2 for every later long prefill. The reset carries the paged wrapper's cudagraph guard for the same reason: re-promoting swaps _cached_module and _plan_info, which a captured graph still points at. The gate is a routing preference, not a refusal -- an explicit backend="aiter" is still honoured, so the slow side stays measurable. Co-Authored-By: Claude <noreply@anthropic.com>

…ment #362 wrote decode down as "genuinely one query row, and AITER wins there" on the strength of 7 of 8 rows, none of which sat below kv_len 1024 and one of which already lost. Sweeping the corner it missed -- kv_len down to 128, and across query head counts -- says AITER still should not be gated, but for a different reason than the one recorded, and the difference matters to anyone reading this to choose a backend. Measured at 0c43abe on MI300X and MI350X, same aiter 0.1.20 build on both, bf16 / head_dim 128 / 8 KV heads / page_size 16: arch qo heads cells aiter loses worst best gfx942 32 30 16 1.70x 1.6x faster gfx950 32 30 15 1.16x 1.7x faster gfx942 64 30 0 0.65x 6.5x faster gfx950 64 30 0 0.86x 7.7x faster There is no 0.16 ms AITER floor -- that figure came from an older stack and does not reproduce. AITER's fixed per-call cost is ~49-51 us against fa2's ~38-44, and the whole losing region is that ~10 us gap showing up while both kernels are launch-bound. So the region is bounded by head count, not by length: a gate keyed on batch x kv_len, which the bandwidth model predicts and an earlier revision of this work proposed, would have mis-routed 70B and 405B decode by up to 7.7x. Also records what the sweep turned up on the way past, because it is larger than anything a gate here could win and a backend-choosing reader needs it: fa2 holds ~5.3 TFLOPS at 64 query heads on both arches regardless of shape while AITER reaches 37-41 -- 6.41 ms vs 0.92 ms at b256/kv4096 on gfx950. KV traffic is identical at 32 and 64 query heads, so it tracks query heads rather than bandwidth. Root cause unprofiled and deliberately not chased here. The gfx950-keeps-AITER test is asserted rather than skipped: every other ragged test skips on gfx950, so a stray threshold would otherwise land with nothing to catch it. Co-Authored-By: Claude <noreply@anthropic.com>

The gate ran only inside `if self._backend == "auto"`, so it fired at most once per wrapper -- on the first plan(). A wrapper that resolved to AITER on a long prefill never re-entered that block, so every later short plan was served by AITER ungated. That is the long-then-short order, and it is the one this gate exists for: a chat turn landing on a cached prefix is exactly a short query after a long one, and it is the 4.74x cell. The short-then-long order worked, which is the order all four original tests happened to use, so the gap shipped green. The paged wrapper does not have this hole because its probe site gives it a second re-check; ragged has no probe, so the re-check is explicit. It stays off under cudagraph for the same reason re-promotion does -- a captured graph still points at the module a switch would swap -- so under capture the first plan still decides, now documented alongside the paged wrapper's mirror-image exception. Two smaller holes found in the same pass: max_q_len <= 0 is an empty batch, not a short query. It read as "short" and demoted on a reason describing no query at all; eager re-promotes, but under cudagraph the wrapper was pinned to fa2 for life on the strength of an empty batch. Fixed in both predicates. Arming both max_q_len and ragged_q_len was asserted to be impossible in a comment and enforced by an `else`. Had a caller ever passed both, the flat-gather reason would win while the ragged demotion site compared against its own string, silently leaving _backend_short_query_demoted False -- a permanent demotion. Both thresholds are 16 on gfx942, so no value-level assertion could have seen it. Now raises. Co-Authored-By: Claude <noreply@anthropic.com>

The widened grid was left as the default, so a bare invocation built 2 backends x 5 batches x 8 kv_lens = 80 configs before the first run, ~106 GiB of KV. That fits an idle MI300X and not a shared one, and it is the first thing a new user runs. Defaults go back to the narrow grid; the short-KV/multi-head sweep is the documented two-pass invocation. --batches "" mapped to [] which is falsy, so a typo'd override silently ran the default grid instead of erroring, and --qo-heads 12 built a fractional GQA group that failed inside plan() after the JIT build rather than at argparse. Both now reject at parse time. Test hardening, all three the same defect -- asserting a backend before checking why it was chosen, so an unrelated AITER decline satisfies the assertion and the subject of the test is never exercised: test_ragged_short_query_declines_aiter hard-failed rather than skipping when AITER was declined earlier in the elif chain, reporting a routing regression that did not exist. test_ragged_short_query_does_not_re_promote_under_cudagraph asserted fa2 on both plans without checking the first was the gate's doing. docs claimed a 7.7x mis-route for 128-head decode; the sweep covered 32 and 64 only. Now stated as the trend it is. Co-Authored-By: Claude <noreply@anthropic.com>

test_ragged_prefill_auto_demotes_to_fa2 planned at qo_len 16 -- exactly aiter_ragged_gated_q_len on gfx942 -- so the new gate answered before the bootstrap failure the test injects, and it asserted on the wrong reason. The same collision hit the paged sibling when #362 landed and the fix is the same: plan above the threshold and say why, so the next gate does not silently retire this test instead of failing it. Co-Authored-By: Claude <noreply@anthropic.com>

Copilot

🟡 Changes recommended

Fallback diagnostics, benchmark output preservation, argument validation, and 64-head correctness coverage need correction.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Routes short gfx942 ragged prefill workloads to FA2, expands decode benchmarking, and documents the measured backend decisions.

Changes:

Adds architecture-specific ragged-prefill routing.
Adds lifecycle and dispatch coverage for backend demotion.
Expands decode benchmark controls and backend documentation.

File summaries

File	Description
`flashinfer/rocm/arch_caps.py`	Defines ragged query thresholds.
`flashinfer/rocm/prefill.py`	Implements ragged routing and demotion lifecycle.
`benchmarks/rocm/bench_batch_decode.py`	Adds configurable decode sweep axes and output paths.
`docs/rocm/backends.md`	Documents routing and benchmark conclusions.
`tests/rocm/test_arch_caps.py`	Tests architecture thresholds.
`tests/rocm/test_prefill_decode_dispatch.py`	Tests selector behavior and reasons.
`tests/rocm/test_batch_prefill_kernels.py`	Tests wrapper routing transitions.
`tests/rocm/test_aiter_auto_fallback.py`	Adjusts fallback coverage above the threshold.

Review details

Files reviewed: 8/8 changed files
Comments generated: 5
Review effort level: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Four corrections from review, one of which was load-bearing for this PR's own conclusion. The perf gates decided before the amd-aiter importability check, and the selector returns as soon as a reason is set -- so on a box with no amd-aiter installed, a short query was told "query length <= 16" rather than that the package is missing. The routing was right and the diagnostic was actively misleading. Both gates now arm only when AITER could have run. Pre-existing for the flat-gather gate since #362; fixed for both rather than replicated. test_batch_decode_aiter_vs_fa2 parametrised (8,8), (16,4), (32,8) -- GQA ratios 1, 4, 4. Ratio 8 had no numerical coverage anywhere, and it is exactly where the decode sweep measured AITER's largest advantage, so "7.7x faster" rested on a kernel nothing had checked for correctness. Added (64,8): 144 cases pass, so the figure stands on a verified kernel. This is the one finding that could have invalidated a claim in the PR body rather than just tidying one. The two documented benchmark passes shared the default "decode" label, and the timing CSV is opened "w" -- the second pass overwrote the first. Distinct labels, and the reason recorded next to them. --kv-lens help advertised the pre-narrowing default; "1,,8" parsed as [1, 8] rather than erroring, silently sweeping a grid nobody asked for. Moved the both-gates-armed check to the top of the selector: it is argument validation and should not depend on which constraint the elif chain happens to answer first. Co-Authored-By: Claude <noreply@anthropic.com>

Copilot

🔵 Needs a closer look

The benchmark documentation substantially overstates the memory required by its default and short-query grids.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

benchmarks/rocm/bench_batch_decode.py:37

The documented short-grid footprint is overstated: this command allocates about 12.45 GiB of bf16 KV plus 10 GiB of 128 MiB workspaces, or roughly 22.5 GiB resident—not 106 GiB. Correcting this matters because the estimate is used to decide whether the sweep fits on a shared board.

This issue also appears in the following locations of the same file:

line 110
line 158

benchmarks/rocm/bench_batch_decode.py:112

The default grid is about 49.8 GiB of bf16 KV (about 54.8 GiB including its 40 workspaces), not ~100 GiB of KV. Please keep this estimate consistent with the actual tensor dtype and dimensions.

# Every config is built before the first run, so a wide grid is resident all at
# once -- the full default sweep is ~100 GiB of KV. Override these to run it in
# passes on a shared board.

benchmarks/rocm/bench_batch_decode.py:160

This repeats the incorrect 106 GiB estimate. For the documented short-KV/multi-head command, the tensors retained by the configurations total about 22.5 GiB (12.45 GiB KV plus 10 GiB workspace), so the operational guidance currently overstates the required capacity by nearly 5×.

# Defaults stay narrow because every config is built before the first run: the
# short-KV/multi-head grid this script was extended for is ~106 GiB resident,
# which fits an idle MI300X and not a shared one. Widen it with the flags, in

Files reviewed: 9/9 changed files
Comments generated: 0 new
Review effort level: Balanced

The ~106 GiB figure was measured against the full cross product when that was the default. Narrowing the default in the previous commit left three comments quoting it, two of them now attached to grids it never described -- the documented short-KV pass is 12.5 GiB of KV, not 106. Recomputed, KV plus the 128 MiB per-config workspace: bare default (40 cfgs) 55 GiB default widened to kv 16384 (80) 116 GiB documented pass 1, short (80) 23 GiB documented pass 2, mid (40) 45 GiB both documented passes at once (120) 67 GiB A capacity figure is read to decide whether a sweep fits a shared board, so a 5x overstatement is not a harmless comment -- it is the kind of stale number that reads as rigour. Co-Authored-By: Claude <noreply@anthropic.com>

demandal25 · 2026-09-14T11:14:59Z

Suppressed comments (3), all correct — fixed in 22adf10. The ~106 GiB was measured against the full cross product when that was the default; narrowing it in 745bf3d left the figure attached to grids it never described. Recomputed: bare default 55 GiB, widened to kv 16384 116 GiB, documented passes 23 and 45 GiB.

Copilot

🔵 Needs a closer look

The late ragged re-check can replace a higher-priority per-plan fallback reason with the short-query performance reason.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

flashinfer/rocm/prefill.py:4094

This late re-check bypasses _auto_select_prefill_backend, so it does not preserve the documented “performance gate last” ordering. For example, after a long auto plan resolves to AITER, a subsequent short plan with custom_mask (or another changed per-plan constraint) is demoted here with the ragged-performance reason instead of reporting that AITER cannot serve the current call. Re-run the selector for this plan (then mark _backend_short_query_demoted only when its returned reason equals the ragged reason) so backend_fallback_reason remains accurate.

Files reviewed: 9/9 changed files
Comments generated: 0 new
Review effort level: Balanced

The late re-check added in a9baadb evaluated only the ragged threshold, so it reported the short-query reason for a call AITER had declined for some other per-plan reason. Chasing that exposed the larger version of the same bug, which predates this PR: the selector ran only while _backend was "auto", so a wrapper that resolved to AITER never re-checked any per-plan constraint again. Measured, before this commit -- a wrapper that planned long and mask-free, then planned again with a custom mask: plan1 long, no mask -> aiter plan2 long, mask -> aiter, backend_fallback_reason None The selector declines AITER for a custom mask ("custom mask (not supported by AITER)"), so that second plan ran a masked call on a backend that cannot honour the mask, silently. Nothing downstream guards it: the aiter branch raises for the soft-cap defect, pos_encoding_mode and kv_layout, but not for a mask. Re-running the whole selector rather than the one predicate fixes both, and is simpler than what it replaces -- the short-query demotion flag and its reset are gone from the ragged wrapper, because re-resolution subsumes them. The paged wrapper keeps its flag: its probe site does a partial re-check and still needs one. First attempt at this only widened the condition to cover _backend == "aiter", which closed the mask hole and broke re-promotion -- any non-gate demotion then stuck, because neither arm matched "fa2". Keying on resolved_from_auto instead covers every transition: 1 long, no mask -> aiter 2 short -> fa2, "query length <= 16 on ragged KV" 3 long again -> aiter 4 long, mask -> fa2, "custom mask (not supported by AITER)" 5 long, no mask -> aiter Cudagraph is unchanged: the first plan decides and nothing re-resolves, since switching would swap a module a captured graph still points at. The paged wrapper has the same hole on the same reasoning. It is pre-existing, out of scope here, and not fixed in this PR. Co-Authored-By: Claude <noreply@anthropic.com>

demandal25 · 2026-09-14T11:35:00Z

Suppressed comment (1) — correct, and it was hiding a bigger one. Fixed in f0234b8 by re-running the whole selector rather than the single predicate. The narrow version: before this, a wrapper resolved to AITER never re-checked any per-plan constraint, so plan(long, no mask) -> aiter followed by plan(long, custom_mask) -> aiter, reason None ran a masked call on a backend that declines masks. The paged wrapper has the same hole; pre-existing and not fixed here.

Copilot

🟡 Changes recommended

Eager re-routing can select AITER while a stale custom-mask buffer remains active.

Get a fresh assessment by requesting another Copilot review.

Review details

Files reviewed: 9/9 changed files
Comments generated: 1
Review effort level: Balanced

f0234b8 made a latent bug reachable. The eager path assigned _custom_mask_buf only when a mask was supplied, never clearing it, while run() picks MaskMode off that buffer -- so a maskless plan following a masked one attended under the previous plan's mask. Before f0234b8 the wrapper stayed on fa2 after the masked plan and never came back, which hid it; re-resolving every plan is what exposed it. Two halves, both mirroring the paged wrapper, which already does this: clear _custom_mask_buf / _mask_indptr_buf in the else branch, so the buffer describes this plan rather than the last one that had a mask; key the selector on the buffer rather than on the argument, so the selector and run() cannot disagree about whether a mask is live. That disagreement was the actual defect: the selector saw no mask and chose AITER while run() saw the stale buffer and asked for MaskMode.CUSTOM, which AITER cannot honour. The test asserts the output against an fa2 reference rather than the resolved backend, because the failure is silent wrong numbers rather than a wrong route. Removing the clear fails it. Co-Authored-By: Claude <noreply@anthropic.com>

Copilot

🔵 Needs a closer look

The documentation overstates the eager-only scope despite graph-mode AITER routes remaining possible.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

docs/rocm/backends.md:535

“Only in eager mode” is too broad: flashinfer/rocm/decode.py:1549-1581 selects AITER under graph capture when max_seq_len is supplied, and explicit AITER is also possible. This sweep measured eager launches, so qualify the statement rather than presenting unmeasured graph behavior as a conclusion.

Files reviewed: 9/9 changed files
Comments generated: 0 new
Review effort level: Balanced

"Only in eager mode" contradicted this same file three paragraphs later: auto reaches AITER under graph capture when the wrapper is given max_seq_len, and an explicit backend="aiter" reaches it either way. The sweep timed eager launches only, so the figures describe eager decode and say nothing about the other two paths -- which is a smaller claim than the one the sentence was making. Co-Authored-By: Claude <noreply@anthropic.com>

demandal25 · 2026-09-14T12:04:42Z

Suppressed comment (1) — correct, fixed in 067e733. "Only in eager mode" contradicted this same file three paragraphs later; the sweep timed eager launches only, so the figures now claim eager decode and nothing about graph-capture or explicit-aiter paths.

Copilot

🟢 Approval recommended

The routing behavior is consistently implemented, documented, and covered by focused regression and parity tests, with all prior findings addressed.

Review details

Files reviewed: 9/9 changed files
Comments generated: 0 new
Review effort level: Balanced

demandal25 and others added 6 commits September 13, 2026 23:56

Copilot AI balanced review requested due to automatic review settings September 14, 2026 06:36

Copilot started reviewing on behalf of demandal25 September 14, 2026 06:37 View session