Repository navigation
feat(hip-kernel-provider): add the gfx942 dense flash-attention prefill kernel - #10337
Merged
AnaghaRaoAMD merged 32 commits intoAug 14, 2026
Merged
Conversation
`ArchTarget.from_gfx(arch).lds_capacity_bytes` plus a 65536 fallback was open-coded in three places in the attention family, and had already drifted: one site caught `KeyError`, another caught bare `Exception`. `multi_arch_data_layout.md` puts LDS-budget rules for the family under `common/`, not duplicated per-gfx. Adds `attention_lds_capacity_bytes()` to `kernels/common/attention_arch.py` and migrates both `gfx942/attention_tiled_2d.py` call sites. Both had identical 65536 fallbacks, so this is behaviour-preserving. `common/attention_unified.py` keeps its own copy for now: different signature (no `arch` parameter, no fallback), so folding it in would change behaviour.
Ports the gfx950 dense prefill kernel (#9480 / AICK-1663) to gfx942: 32x32x8 atom with K-loop doubling (CDNA3 has no 32x32x16 fp16/bf16 atom), transposed-QK so P feeds the PV MFMA lane-locally, non-pipelined single-buffer LDS, element-wise V read. This is the correctness baseline; the perf levers layer on top in the commits that follow. Separate module rather than an arch branch in the gfx950 file: the algorithm genuinely diverges (atom, memory path, occupancy regime), which is what `dsl_docs/architecture/multi_arch_data_layout.md` says warrants a per-gfx module, and it keeps the gfx950 golden IR byte-identical by construction. `kernels/__init__.py` aliases the module rather than star-exporting it: the gfx950 `build_attention_dense` / `supports_attention_dense` are already re-exported unqualified and the symbol names are identical, so a second import would silently shadow the first. Reach it via `kernels.attention_dense_gfx942` or `from kernels.gfx942 import attention_dense`. Two CDNA3 hazards the port had to handle explicitly: * The tile-end rendezvous must be `sync_lds_only()`, not a bare `s_barrier`. gfx90a+ set FeatureBackOffBarrier, so SIInsertWaitcnts skips the conservative pre-barrier drain and places the lgkm wait at the ds_read's consumer, which the scheduler may sink past the barrier. Disassembly before the fix showed `ds_read_u16 x4 -> s_barrier -> s_waitcnt lgkmcnt(0)`; with NBUF=1 the next iteration's DMA can then overwrite V_lds under an in-flight read. The gfx950 sibling is not a precedent here: its NBUF=2 separates K but not V. `elide_trailing_barrier=False` pins the barrier against the elide pass, which targets exactly that slot. * `supports_attention_dense` is the single gate: it rejects everything the builder cannot emit, so `supports(spec)[0] is True` implies `build` succeeds. Without that equivalence dispatch selects specs it cannot build. It covers the deferred modes (persistent/varlen/ragged/sliding-window), `block_n` dividing the 256-row query tile (a floor in the causal clamp silently drops keys, and block_n > 256 gives a zero-trip loop -> rcp(0) -> NaN), the wave/tile divisibility the async K/V DMA needs, the 64 KB LDS budget (over-budget aborts in codegen with no usable message), non-positive extents (every dataclass validator is a divisibility test and Python's `%` is sign-following, so `num_query_heads=0` would emit `sdiv i32 %hq, 0`), and extents past 32-bit addressing. The DMA row-split check is a `ValueError`, not an `assert`: `python -O` strips asserts, and it guards a SILENT wrong answer -- skipped rows leave uninitialized LDS that do_qk/read_v consume as garbage, with no fault -- so it must survive optimized builds. Verified on gfx942: the in-scope cohort builds with the expected name and workgroup size; 0 register spill on the probed shapes within the VGPR/LDS budget; numeric validation vs an fp32 SDPA reference runs through the live benchmark.
The only runtime-behaviour change in this series. Opt-in only: matches solely on `algorithm="attention_dense"` or `spec_id="gfx942_attention_dense"`, so `auto` routing and the gfx950 arm are untouched. The arm registers at priority 3, which outranks every other attention candidate, so the opt-in check is the only thing keeping a correctness-first P0 kernel off the default gfx942 path. `dispatch/AGENTS.md`'s priority table did not document tier 3 at all and listed neither dense candidate; it now does, and says why a tier-3 candidate that forgets its opt-in gate claims all traffic for its arch. `_p0_req` resolves `dense_persistent="auto"` to "off" (persistent is P4) but leaves an EXPLICIT "on" intact, so `supports_attention_dense` rejects it rather than this candidate silently launching a default-grid kernel for a caller who asked for the persistent one. Scope is delegated entirely to `supports_attention_dense`. That closes a hole where `_dense_spec` sets `ragged=True` for any non-256-multiple self-attention length -- most real serving shapes -- and the arm reported support for a spec the builder then refused to emit. Also drops two achieved-TFLOPS figures from this file's docstrings, per the AGENTS.md rule that measured software performance never enters the repo.
Mirrors the gfx950 host lane. The live benchmark is the numeric gate for this kernel -- HIP-event timing, SDPA parity, PASS/FAIL, JSON/CSV, per-mode geomean -- the same role it plays on gfx950, where numeric correctness is a bench gate rather than a CI pytest. `_p0_occupancy.py` is the static spill/occupancy probe behind the AICK-1664 "0 spill, VGPR within cap" resource guard; it is the gfx942 analogue of the gfx950 sibling's `_profile_one.py`, and is re-run after every lever. Its sys.path surgery, kernel compiles and exit code all live inside main(), so importing it is side-effect free even though builders/ is a directory pytest can be pointed at. Both host paths call the kernel module's exported `attention_dense_signature`, `attention_dense_grid` and `attention_dense_block` instead of re-deriving them. There were three hand-rolled copies of the ABI and a fourth hardcoded copy of BLOCK_M; a reordering in any one of them is a silent argument-marshalling bug at launch, and the duplicate BLOCK_M defeated the import-time binding in the kernel module. The benchmark reports `p0_kernel_name(spec)`, not `spec.kernel_name()`: the latter omits batch and waves_per_eu, so a B=4 row would report the B=1 symbol -- exactly the confusion behind the cache collision the name exists to prevent. README covers scope, the CDNA3 deltas, the enforced limits (block_n, LDS budget, 32-bit extents) with the failure each one prevents, and the lever port map. Per AGENTS.md, lever effects are qualitative here; measured magnitudes live outside the repo. Targets are named by device (gfx942/gfx950), not product name.
…ention_dense 104 CPU-only tests, no GPU and no comgr. `test_attention_dense_gfx942.py` (90) covers the kernel surface: the in-scope cohort builds with the expected name and workgroup size, every scope rejection fires, kernel-name identity is unique in both baked-but-unnamed parameters (batch and waves_per_eu), and -- the load-bearing one -- a parametrized grid asserts the `supports(spec) is True implies build succeeds` contract in both directions, with a guard against the grid collapsing to one branch and making the contract test vacuous. Includes an IR-level regression guard on the tile-end barrier: that race shipped once and was caught only by hand disassembly, so it now fails a test instead. `tests/dispatch/attention/test_gfx942_dense_wiring.py` (14) is required by `dispatch/AGENTS.md` step 4 and follows the sibling `_Gfx942Arch` unittest idiom. It pins the opt-in behaviour that priority 3 makes load-bearing, and that an explicit `dense_persistent="on"` is rejected rather than downgraded. Neither file is collected by an automated gate: `platform/tests/run_all.py` pytests only `platform/tests/`, and no CMake `add_test` reaches `library/`. The kernel-surface file states that explicitly, matching the precedent in `test_attention_dense_golden.py`. A gfx942 golden-IR lane does not exist yet and is tracked in the AICK-1664 plan.
With the cfvst sign-flip disproven cross-part, lift the proven conflict-free-V store vehicle into the dense kernel: store V TRANSPOSED into V_lds[dim, token] via a perm_b32 2x2 store-path transpose (CK transpose_vectors masks 0x01000504 / 0x03020706), so the PV A-operand read is a single contiguous ds_read_b64 instead of the baseline's 4 element-wise ds_read_u16. ISA: PV LDS reads drop from 128 ds_read_u16 to 32 ds_read2_b64 per tile; s_waitcnt 95 -> 49. The lift is numerically identity-preserving -- read_v delivers the exact same (dim, key) MFMA operands, only the LDS layout and read width change -- so the cohort max_abs is unchanged from the naive path. Verified vs fp32 SDPA on BOTH gfx942 parts (304-CU and 228-CU), bit-identical to the naive baseline; 0 spill all shapes; CPU tests green. Gated to D128 fp16 (_p0_use_cfvst): D64 is VGPR-bound so the register round-trip regresses it (kept on naive V), and D128 bf16's .1k MFMA schedule spills over the waves-per-eu=2 cap (deferred until the fused rescale frees VGPR headroom). Both stay on the naive async-DMA V path. The 2x2 tile-exactness invariant is CFVST-PATH-ONLY and gated accordingly: computing it unconditionally would reject specs the naive path builds fine (at D64/block_n=32 with a 1024-thread CTA, V_BLOCKS=512 < THREADS=1024, yet read_v never touches V_BLOCKS there). It is mirrored into supports_attention_dense so support() and build() still accept exactly one set of specs, and it raises rather than asserts because `python -O` strips asserts and an unguarded tail item would read past the tile. Barrier caveat: V publication is now an in-loop ds_write, so the tile-START rendezvous is upgraded from a bare s_barrier to sync_lds_only (lgkmcnt drain before the barrier) -- gfx90a+ FeatureBackOffBarrier would otherwise sink the lgkm wait past the barrier and race the perm_b32 store. The barrier regression test now covers both the cfvst (D128 fp16) and naive (D64) tile-start structures. gfx950 untouched.
Swap the softmax exp2 (llvm.exp2.f32, a ~5-VALU guarded range reduction) for exp2_fast (llvm.amdgcn.exp2.f32, one v_exp_f32). Both softmax arguments are provably <= 0 -- alpha's (m_i - m_new) with m_new = max(m_i, tile_max), and p's (s - m_new) with m_new >= every s -- which is exactly exp2_fast's documented precondition, so the overflow/underflow guard is dead work. ISA: the guarded range reduction is gone (v_cmp_gt_f32 and v_ldexp_f32 drop to 0; v_cndmask_b32 98 -> 32; s_nop 306 -> 269). Numerically bit-identical on the cohort (llvm.exp2 and amdgcn.exp2 agree for these small args): cohort PASS vs fp32 SDPA, 0 spill all shapes, CPU tests green. This is the dominant lever on the post-conflict-free-V VALU-bound path. Gated to fp16 (_p0_use_exp2_fast): exp2_fast's result is live sooner, raising register pressure, and bf16 D128 -- the tightest config -- then spills 22 VGPR over the waves-per-eu=2 cap. bf16 keeps plain exp2 (0 spill) until the fused exp2->accumulate->cast->pack rescale frees VGPR headroom -- the same prerequisite as bf16 conflict-free V. gfx950 untouched.
Fused/lazy rescale: the softmax loop computed a full f32 p_vals matrix (N_SUB*16, 32 at D128), reduced it into l_local, then cast+packed it in a separate relayout_p pass -- holding all those f32 registers live across both. Restructure so each exp2 is computed inline, accumulated into l_local, and cast->packed into the PV B-operand in one pass. Same (nsub,i) walk order (KK_STEPS == N_SUB*4), so l_local and the packed casts are numerically bit-identical -- pure live-range relief. Peak live f32 p registers drop from N_SUB*16 to ~one pack; the now-dead relayout_p helper is removed. This also makes the kernel_name 'lazyrs' tag accurate (it was a documented cosmetic mismatch). The exp2_fast gate is now (head_size, dtype)-aware: enabled everywhere except bf16 D128, which still spills on the .1k MFMA schedule even after the fused rescale (175 -> 256 VGPR / 22 spill; kept on plain exp2). The freed headroom newly enables bf16 D64 exp2_fast at 0 spill. bf16 D128 cfvst likewise still spills (forced-on probe 256/2) and stays an occupancy item. Verified: CPU tests green (+5 covering the gate matrix, the emitted intrinsic and the fused cast count); GPU cohort PASS vs fp32 SDPA, max_abs bit-identical to the prior commit; 0 spill across all four shipped configs.
Per-config amdgpu-waves-per-eu for the gfx942 dense prefill kernel. New _p0_waves_per_eu(head_size, dtype): default 2, override bf16 D64 -> 4. Applied in the gfx942 arm of the shared _dense_spec (gated on req.arch == 'gfx942' so the gfx950 candidate keeps the spec default), so the kernel_name wpe tag and the emitted amdgpu-waves-per-eu attribute both carry the tuned value on both the name (dispatch select) and build paths. bf16 D64 at wpe=4 forces the allocator 215 -> ~117 VGPR (0 spill); its LDS is only 16 KB (allows 4 WGs), so a second workgroup co-resides (1 -> 2 WG/CU). The bf16 .1k schedule is serialized enough at wpe=2 that its HBM latency is exposed; the extra resident WG hides it -- net-positive across the whole seqlen sweep, so no seqlen gate. Kept at 2, measured: fp16 D64 (wpe=3 reaches 2 WG/CU but loses more ILP than the 2nd WG buys -- its wpe=2 already hides latency via ILP); D128 both dtypes (LDS-bound at ~35 KB, no wpe reaches a 2nd WG/CU -- that needs the K/V-pad or single-buffer LDS work). Correctness is waves-per-eu-independent (the knob only changes register allocation). Verified: CPU tests green (+6 covering the selector matrix, the baked attribute, and dispatch wiring including the do-not-touch-gfx950 guard and the gfx950-default pin); shipped-config occupancy 0 spill all four, bf16 D64 now 2 WG/CU.
Port the persistent (grid-stride) flash-attention variant from the gfx950
sibling to gfx942, the last required-by-scope AICK-1664 kernel variant.
Kernel: factor the per-work-item compute out of _build_attention_dense_p0 into a
nested _run_work_item(qb, hq, bt) shared by two grids. The default grid calls it
once with the CTA block ids; the persistent path launches a 1-D grid of
num_persistent CTAs, grid-strides an scf_for over W=(Sq//256)*Hq*B work items,
decodes each wi -> (qb, hq, bt), and calls the same body. Both the qb-major
(causal-balanced) and hkv-major (GQA L2-locality) work decodes are ported.
CTA-invariant setup (LDS allocs, buffer resources, the coordinate-free
cfvst/read_v/do_pv helpers) is emitted once and shared; a per-work-item
s_waitcnt(vmcnt=0)+barrier guards the reused LDS across grid strides.
Dispatch: supports_attention_dense accepts persistent; the gfx942 arm of the
shared _dense_spec defaults num_persistent to 304 (the 304-CU part's CU count)
and lets dense_persistent='auto' turn persistent on once the work fills the grid
(large Sq). gfx950 is untouched (arch-gated).
Device-querying the CU count instead of defaulting to 304 is deliberately out of
scope here and tracked separately; the static default mis-sizes the grid on the
228-CU part, which is a known and accepted limitation of this change.
The gfx942 dispatch wiring tests are refreshed for the new behaviour: an
explicit dense_persistent='on' is now accepted and yields a genuinely persistent
spec rather than being rejected/downgraded, and the support-implies-buildable
test builds the exact spec the dispatcher selects (auto->persistent at 2048)
instead of forcing 'off' and comparing against the auto name.
Verified: persistent PASS vs fp32 SDPA on the 304-CU part across 5 shapes x
{NP=228,8} x {qb,hkv}-major, max_abs bit-identical to the default grid; the
non-persistent refactor is cohort-PASS with unchanged spill and identical
max_abs (IR text reordered, no golden). 0 spill all persistent combos. CPU tests
green (+10 covering build x both decodes, the name tag, the shared-body op-count
invariant, and dispatch auto).
D64 has no K-LDS pad by construction (_p0_rows_per_instr(64) == 2 forces LDROW = D, pitch 128 B = exactly 32 banks -> a full 32-way conflict on the do_qk reads). Add a 2-row-group boundary pad (K_lds[1, block_n//2, 2*D + _P0_LDS_PAD], group stride 272 B -> 4-way, matching D128) and wire it per-config into the gfx942 dispatch. The lever is cross-part confirmed as a large improvement on the D64 path, with near-identical ON/OFF behaviour on both the 228-CU and 304-CU parts -- the CU-count-independence that is the determinism tell for an LDS-conflict fix. Correctness PASS in both flag states with max_abs bit-identical OFF<->ON (the MFMA operands are unchanged; only the LDS addressing moves). Static codegen A/B shows VGPR and the ds_read stream UNCHANGED, so the win is purely runtime bank-conflict-replay elimination, not register relief. That refutes the earlier "fixing the conflict frees ILP-headroom registers" hypothesis while confirming the lever itself. Wiring: add a shared d64_kpad spec field (default False -> gfx950 and all non-dispatch builds stay byte-identical); a _p0_d64_kpad(head_size) policy helper mirroring _p0_waves_per_eu; _dense_spec (gfx942 arm) folds d64_kpad=True for D64 both dtypes; _p0_d64_kpad_active reads spec.d64_kpad OR _P0_D64_KPAD; p0_kernel_name appends a _kpad tag so the name cache cannot collide. The module constant _P0_D64_KPAD stays False, so default-emitted IR is byte-identical.
Replace the NotImplementedError stub with the end-to-end torch entry point, mirroring the gfx950 sibling. Compiles (launcher-cached) and launches the dense prefill kernel on torch tensors for both the default and persistent grids (attention_dense_grid picks the launch shape). Returns out. The launcher cache is keyed on p0_kernel_name(spec), NOT the shared kernel_name(): this kernel bakes batch into the buffer-resource extents and waves_per_eu into the register-allocation attribute, both omitted by kernel_name(), so two specs differing only in batch/wpe must not share a cached binary -- a B>1 launch served the B=1 kernel reads out of bounds. cu_seqlens_* are rejected: gfx942 attention_dense is dense-only (varlen is rejected by supports), so the ABI has no cu_seqlens arguments. Verified on the 304-CU part: cohort PASS vs fp32 SDPA through run_attention_dense_torch (default + persistent, fp16/bf16, D64/D128, including batch > 1). +2 CPU guard tests (unsupported-spec NotImplementedError, cu_seqlens ValueError).
Add _P0_IGLP, a module-level probe knob that emits a single llvm.amdgcn.iglp.opt directive at the top of the KV loop. It is the canned backend MFMA/ds_read/ds_write interleave that CK's own C++ pipelines use, and its documented precondition -- in-loop ds_write traffic to interleave the MFMAs against -- only started holding for this kernel once conflict-free V introduced a real ds_write to V_lds. Left default-OFF: measured cross-part on both gfx942 parts, it is resource-neutral and performance-neutral. The directive only shifts the architected/AGPR register split, which is consistent with this loop being barrier-rendezvous-bound rather than in-loop-instruction-order-bound. It is kept as a knob rather than deleted because it is one line and is the natural starting point for the manual sched_group_barrier work that supersedes it. Byte-identical when off, mirroring _P0_D64_KPAD: default-emitted IR and the gfx942 golden lane are unchanged.
Byte-stability golden for the gfx942 dense kernel's Python-lowered LLVM IR, mirroring the gfx950 golden but in a SEPARATE file + fixture so the gfx950 golden is never touched. Cases cover the AICK-1664 acceptance matrix at both llvm20/llvm22 flavors: D64/D128 x bf16/fp16 x default/persistent x GQA/MHA, causal and full, both persistent decodes (qb_major/hkv_major) + interleave, and the block_n=128 tiling. Pure text lowering -- no GPU, no comgr. Two D64 cases are built through the gfx942 dispatch factory _dense_spec via a mk_dispatch() helper, so the fixture guards the SHIPPED D64 path (d64_kpad=True plus the bf16 waves_per_eu bump) rather than hard-coding levers, and will track future tuning changes on re-bless. The kpad-OFF cases are retained alongside them, pinning the probe-off codegen. No cpp/python byte-identity companion: library/kernels/ has no C++ engine mirror, so the dual-engine parity gate does not apply here -- the Python lowering is the ground truth. bn128 is at D64 only (bn128 at D128 is 68 KB > 64 KB and correctly rejected, so it is not a golden case). The library root is pinned at the front of sys.path so tests/ cannot shadow the real dispatch package when the script is run directly for re-bless. Re-bless: from rocke/library, PYTHONPATH=../platform/python:. python tests/test_attention_dense_gfx942_golden.py --write It never writes the gfx950 fixture.
library/tests/ had no automated gate: platform/tests/run_all.py cannot reach it (verbatim-copy guard + the one-way library->platform dependency rule), so collection has to be owned on the library side. Add the library counterpart to the platform runner: - pytest.ini: importlib mode + a registered `gpu` marker (unregistered marks warn under -W error). - run_all.py: CPU lane by default (`-m "not gpu"`, safe on any CI box), with --gpu / --gpu-only to add the on-GPU numeric lane and --only for a -k filter. - test_attention_dense_gfx942_numeric.py: the committed, CI-collectable form of the "GPU-numeric bench, both grid variants, vs fp32 SDPA" acceptance criterion, previously an out-of-tree verifier. Gated on gcnArchName "gfx942" rather than the marketing name (which misses MI308X), so it self-skips on CPU instead of failing. Also repoint the golden test's docstring at the new runner. Registering this runner in the actual pipeline job is a CI-team step and is deliberately left out of this change: the CPU lane belongs on every runner, the --gpu lane on a gfx942 runner.
…asured bound The kernel docstring and the builder README both still opened with the inherited "LDS-bank-conflict-bound -> MFMA-starved" framing, while the rocprof sweep had already measured this kernel as OCCUPANCY-bound -- and the docstring's own P5 bullet said "occupancy/latency-bound at 1 WG/CU". The files contradicted themselves and mis-ranked the remaining lever families for anyone picking up the port. Two mechanisms are recorded so the old framing cannot quietly come back: * The runbook's bottleneck decision tree short-circuits on low occupancy first, so the LDS_BOUND branch is structurally unreachable here regardless of conflict rate. * LDSBankConflict counts conflicts per LDS-ACTIVE CYCLE, so conflict-free V raises the rate while lowering the cost. Reading that rate as a bound is the trap that kept the LDS-bound story alive. README is re-baselined off the correctness-baseline-era text it had been stuck on: status is now "shipped" rather than "expect it to be slower", the persistent grid is documented as shipped rather than deferred, and the lever port map is replaced by a full lever RECORD -- every lever evaluated for this port, adopted or rejected, with its mechanism and the reason it won or lost. That record is qualitative per AGENTS.md; magnitudes stay in the plan and the protected page. The kernel docstring additionally fixes six stale claims (D64 K-pad TODO -> shipped, the persistent ablation, the NBUF=2 capacity claim, _P0_LDS_PAD, the _P0_D64_KPAD mechanism, _P0_IGLP proven-neutral) and scrubs two AGENTS.md "no software-achieved performance" violations. Comment/docstring/markdown only: no emitted-code change.
test_bf16_ring_enabled_for_d128 asserts that bf16 D128 takes the gfx942
sliced-K ring. The code it tests says the opposite, and has for some time:
# bf16 D128 stays off the ring (non-ring T=64 is its bs=64 optimum).
if _enable_gfx942_bf16_flash(problem):
return False
#9647 excluded all gfx942 D128 from the ring for correctness; #9662 recovered
ONLY fp16 D128, via a depth-2 ring. bf16 D128 deliberately keeps the non-ring
T=64 path. The test's comment ("the earlier generator-bug exclusion was fixed")
describes an intent the implementation never adopted for bf16.
This test is RED on develop today, independently of this branch:
_enable_gfx942_flash_k_sliced_ring and _enable_gfx942_bf16_flash are
byte-identical between develop's tip and this branch's base, and the test fails
in a clean develop checkout. Verified live: bf16 D128 ring=False,
fp16 D128 ring=True depth=2.
Rename to test_bf16_ring_disabled_for_d128 and assert the documented behaviour.
Out of scope for the gfx942 dense prefill port and separable on purpose: it is
the last commit and touches one test file, so it can be dropped or split into
its own PR without disturbing the rest of the series. It is included because the
series otherwise leaves a red test in a file it already touches.
github-actions
Bot
added
documentation
project: hip-kernel-provider
rocke-lib: attention
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
yraparti
reviewed
Aug 4, 2026
yraparti
approved these changes
Aug 5, 2026
yraparti
left a comment
yraparti
left a comment
Contributor
There was a problem hiding this comment.
Please see if you can use smem_pool to fit double buffer for K in LDS for D128 in your next PR.
develop's #10235 split the monolithic dispatch/attention.py into a per-arch package, which collided with this branch's gfx942 dense arm. Three conflicts: 1. dispatch/attention.py (modify/delete) -- accepted the upstream deletion and moved the gfx942 dense arm into dispatch/attention/gfx942.py, adapted to the new contracts: a declared Capability(arches, dtypes, supports_features) that now carries the arch/dtype/sliding-window/sinks gates, _supports= for the residual predicate, candidate.admits() at the call sites, and registration via the module's register(registry). gfx942 also gets its OWN _dense_spec instead of the two `if req.arch == "gfx942"` branches this branch had added to the shared one. Net effect: the port no longer touches dispatch/attention/gfx950.py at all, so gfx950 dispatch is byte-identical to develop by construction rather than by assertion. The two dispatch_d64_* golden-IR cases hash unchanged, which is the evidence the move is behaviour-preserving. Callers follow the spec factory to its new home (dispatch.attention.gfx942); the package-level dense_spec_for_request stays gfx950's, and gfx942's docstring says so, because handing a gfx942 request to it would silently yield an untuned spec. 2. dispatch/AGENTS.md -- took develop's richer priority table (declared arches + owning module) and added the gfx942 dense row; the opt-in list goes from two candidates to three. Kept this branch's note on why tier 3 must carry the opt-in check, rewritten for the Capability split: capability constrains the request's shape, not its selector, so the check cannot migrate into it. 3. tests/dispatch/attention/test_dense_pipe_wiring.py -- took develop's version wholesale. Upstream landed the same bf16-D128 ring fix as commit 4cc4eee (same rename, same assertion) plus a companion fp16 depth-2 test and a more accurate rationale, so that commit is fully superseded and drops out of the diff. Verified: 138 dispatch tests, 125 gfx942 dense tests (incl. the golden-IR lane) and 509 library tests pass. The 2 remaining failures in test_attention_builds.py::TestAttentionHarnessTimers are pre-existing on develop -- develop's test looks up builders.gfx950.attention.parity_unified_attention while develop's tree puts that file under .../attention/prefill/. Neither file is touched by this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…baseline Review feedback on #10337: record the performance gain versus the baseline without sharing absolute throughput. Adds a "Gain over the unified-attention baseline" table to the builder README, stated as relative percentages against the in-tree unified attention path (attention_tiled_2d) at its best per-config vehicle. Ratios only -- every TFLOP/s, counter and per-lever magnitude behind them stays on the protected results page, per AGENTS.md. D128 is reported; D64 is deliberately left open. The last D64 head-to-head was measured before the D64 K-bank-conflict pad was adopted, so it describes a configuration that no longer ships: it read -17% (fp16, short sequence) to +29% (fp16, long) and +23% (bf16, long), while the pad alone is worth roughly 1.6-2.1x on the D64 path. Composing the two separately-measured ratios would produce a plausible number that was never observed, so the README states the pre-pad figures, states why they understate, and names the re-measurement as the way to close the row. One further caveat is carried explicitly because it changes how the fp16-D128 row should be read: its baseline is unified attention's `fast` wide4 + conflict-free-V + sliced-K-ring vehicle, not its shipped config -- the shipped one overflows 64 KB LDS and fails codegen at llvm22. That row is measured against the strongest in-tree alternative, not the easiest one. The closing AGENTS.md note is tightened to say "absolute measured throughput", so it no longer reads as contradicting the ratio table above it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nse spec develop's #10102 ("restore LDS K padding at D=64 in gfx950 attention_dense") added `lds_k_group_pad: int = 8` to AttentionDenseSpec, immediately next to this branch's `d64_kpad: bool = False`. The two are the same lever -- pad between DMA row-groups to break the D64 QK bank pattern -- derived independently per arch, and they landed on the SAME 8-element pad from opposite directions: gfx950 from a bank/phase model, gfx942 from a measured ~2x ON/OFF ablation. Kept both fields. They stay separate because the consumers differ (a bool switch on a fixed gfx942 layout vs a parameterized gfx950 pitch) and collapsing them would re-bless both arches' goldens. Unifying them once the gfx942 layout is parameterized too is noted in the field comment as the follow-up. Two consequences had to be handled, both caught by the gfx942 golden lane: 1. The shared kernel_name() now emits a `kpad{N}` token whenever `128 // head_size > 1`, so every gfx942 D64 symbol picked up `kpad8` -- from a field this builder reads exactly zero times. It is not removable without touching gfx950's name (and its golden), so gfx942's own suffix is renamed `_kpad` -> `_d64kpad`. The tags are now spelled differently on purpose: a substring test for one can no longer match the other, and a gfx942 symbol cannot be misread as describing the gfx950 pitch. p0_kernel_name carries a warning saying so. 2. gfx942 golden re-blessed: 7 D64 cases x 2 flavors. Verified name-only, not codegen -- all 8 D128 cases are unchanged (matching upstream's `128 // head_size > 1` guard), the byte deltas are exactly the symbol-length change, and building the same spec with lds_k_group_pad 8 vs 24 gives a byte-identical gfx942 body modulo the symbol. Test assertions moved from `"_kpad" in name` to `name.endswith("_d64kpad")`, matching the anchoring convention this file already uses for `_wpe`/`_b`. 572 library tests + 138 dispatch tests pass; the 2 remaining failures in test_attention_builds.py are pre-existing on develop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… on gfx942 develop's #10102 added `lds_k_group_pad: int = 8` to AttentionDenseSpec for the gfx950 D64 K-LDS pad. That is the same lever this branch added as `d64_kpad: bool = False`, not merely a similar one: gfx950 LDROW = K_GROUP * D + spec.lds_k_group_pad gfx942 K_GROUP_STRIDE = ROWS_PER_INSTR * D + _P0_LDS_PAD (_P0_LDS_PAD == 8) Same formula, same group definition, same LDS shape, same constant, and equivalent gates (`128 // head_size > 1` vs `head_size == 64` over the supported {64,128}). `d64_kpad` was a strictly less expressive duplicate: pad=0 is d64_kpad=False, pad=8 is True. Keeping both was unjustified, so gfx942 now reads the shared field and `d64_kpad` is gone. What this buys beyond deleting a field: * The PR no longer touches kernels/gfx950/attention_dense.py AT ALL -- that 12-line field was the only reason it did. The do-not-touch-gfx950 rule now holds by construction rather than by review. * The `_kpad` vs `kpad{N}` tag collision disappears with the field. The shared kernel_name() already emits kpad{N} on the packed path, so p0_kernel_name adds no tag of its own -- one fact, one spelling. (Before this, a gfx942 D64 symbol said `kpad8` while emitting an UNPADDED layout whenever d64_kpad was False: the name was actively wrong.) * gfx942 inherits the parameterization for free. The A/B is now `lds_k_group_pad=0` rather than a module-level probe constant, so `_P0_D64_KPAD` and the `_p0_d64_kpad()` policy function are both deleted. * gfx950's `% 8 != 0` validator now covers gfx942 too. Shipped codegen is UNCHANGED, verified rather than argued: dispatch used to set d64_kpad=True (stride 2*64+8=136) and now inherits pad=8 (stride 136). Lowering the four dispatch specs before and after and hashing the IR with the symbol name normalised out gives identical bodies on all four -- fp16/bf16 x D64/D128: b7fa1ed9 / 7258cee1 / 1f37baee / 0992a24d. Only the symbol changed, losing the now-redundant suffix. Golden re-blessed. The `default_d64_*` cases now pass `lds_k_group_pad=0` explicitly: they were the unpadded-layout coverage, and with the shared field defaulting the pad ON nothing else would pin it. Their bodies are unchanged -- only the name moves kpad8 -> kpad0, which also makes it honest. 573 library + 138 dispatch tests pass; the 2 test_attention_builds.py failures are pre-existing on develop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
yraparti
reviewed
Aug 6, 2026
yraparti
reviewed
Aug 6, 2026
yraparti
reviewed
Aug 6, 2026
yraparti
reviewed
Aug 6, 2026
AnaghaRaoAMD
reviewed
Aug 7, 2026
…ifiers "P0" named a phase of the port, not a property of the code. The port is finished: the body it labels carries the full shipped lever set (conflict-free V, exp2_fast, fused rescale, per-config waves-per-eu, the persistent grid), so every `p0_`/`_p0_`/`_P0_` identifier advertises a stage that no longer exists and reads as "provisional" to anyone picking the file up. Renames, mechanical: p0_kernel_name -> gfx942_kernel_name _p0_rows_per_instr -> _rows_per_instr _p0_use_exp2_fast -> _use_exp2_fast _p0_use_cfvst -> _use_cfvst _p0_waves_per_eu -> _tuned_waves_per_eu _p0_lds_row_stride -> _lds_row_stride _p0_d64_kpad_active -> _k_group_pad_active _p0_k_group_stride -> _k_group_stride _p0_lds_bytes -> _lds_bytes _build_attention_dense_p0 -> _build_attention_dense_single_buffer _P0_LDS_PAD -> _LDS_ROW_PAD _P0_V_PAD -> _V_ROW_PAD _P0_IGLP -> _IGLP builders/.../_p0_occupancy.py -> _occupancy_probe.py Two are renames with content, not prefix strips. `_k_group_pad_active` says what the predicate tests (a row-GROUP pad) rather than which head size happens to need it -- the gate is `rows_per_instr > 1`, and D64 is merely today's only such config. `_build_attention_dense_single_buffer` names the one axis that distinguishes this body from a future one: NBUF=1. _P0_BLOCK_M is DELETED rather than renamed. It duplicated the _BLOCK_M imported from the gfx950 module, and the guard beneath it compared the two for equality. Renaming it to the bare name would have rebound the import and reduced that guard to `_BLOCK_M != _BLOCK_M` -- a check that can never fire, silently removed. The invariant it was actually protecting is re-expressed as the real one: `_BLOCK_M % 32 != 0` raises, because the body derives WAVES = _BLOCK_M // 32, which FLOORS, so a non-multiple leaves the top _BLOCK_M % 32 query rows of every block uncovered by any wave -- never computed, never written, no fault. Still a raise and not an assert: `python -O` strips asserts and this guards a silent wrong answer. The unit test that kept a third copy of 256 now imports the constant instead. Also drops the `align_up` re-export from the gfx950 import block. It was imported under `# noqa: F401` and is used by nothing, here or anywhere else in the tree. Proven codegen-neutral rather than argued. All four dispatch-shipped kernel names are byte-identical before and after, and so are all eight IR body hashes (four dispatch specs x llvm20/llvm22), hashed with the symbol normalised out. The gfx942 golden lane is green at both flavors with no re-bless, and both golden JSONs are byte-identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The static resource gate built its specs with `AttentionDenseSpec(batch=1, block_n=64, **shape)`, leaving waves_per_eu at the shared dataclass default of 2. But waves_per_eu is a per-config TUNED value that the dispatch spec factory fills from the kernel's own policy, and it is emitted as amdgpu-waves-per-eu -- it directly changes register allocation. So the probe was reporting the spill and occupancy of a binary nobody runs. That is not hypothetical: bf16 D64 is the one shipped config with an override (4, not 2), the override is precisely what forces the allocator down far enough for a second workgroup, and it is therefore the row most likely to spill when a lever raises register pressure. It was invisible to this gate twice over -- probed at the wrong wpe, and absent from SHAPES entirely. Resolve waves_per_eu through `_tuned_waves_per_eu` and add the bf16-D64 row. The probe now covers all four shipped configs at the configuration each of them actually ships. The tracker ID in the module docstring goes with it, per the DSL style guide. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`supports_attention_dense` promises a structured `(bool, str)` for anything it turns down -- that contract is what lets the dispatch arm decline a request instead of selecting it and dying at build time. The isinstance check that enforces "this is an AttentionDenseSpec" sat BELOW the dtype and head_size checks, both of which dereference `spec.dtype` / `spec.head_size` first. Pass anything else and the function raises AttributeError out of an API whose whole point is that it does not raise. Move the type check above the first attribute access. Behaviour is unchanged for every real spec; only the wrong-type path changes, from a traceback to the rejection reason the check was already written to return. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both harnesses -- the builder smoke path and the live benchmark -- hand-built an
AttentionDenseSpec from frozen CLI defaults. The tuning that actually ships
lives in `dispatch.attention.gfx942._dense_spec`, so a hardcoded default here
freezes whatever the policy happened to be the day the flag was written, and the
harness then times a kernel that does not exist in production.
Two concrete drifts were live in this tree:
* `--wpe` defaulted to 2. bf16/D64 ships 4. Different register allocation,
different binary, different symbol (`wpe2` vs `wpe4`).
* `--np` defaulted to 228. Dispatch ships 304 CTAs for the persistent grid,
and turns the grid on automatically for the whole large-Sq causal sweep --
which this bench could not reach at all, because `persistent` was listed as
an unimplemented mode and exited early.
This is the failure mode that once reported a real gain as a regression: a
reproducer that passed `waves_per_eu=` but not the lever adopted after it was
written. A harness measuring an unshipped config is worse than no harness,
because its output looks like evidence.
So the flow is now: CLI shape -> AttentionRequest -> `dense_spec_for_request`
(imported from the ARCH module; the `dispatch.attention` package re-export of
that name is gfx950's and hands back the untuned spec) -> `dataclasses.replace`
for only the flags the user EXPLICITLY passed. Every tuning flag defaults to
None = "whatever dispatch ships", so an omitted flag TRACKS future policy
instead of pinning today's value; omission becomes the safe default rather than
the trap.
`assert_tracks_dispatch` is the mechanism, not decoration: it re-resolves the
shipped spec itself and raises on any non-overridden field that differs, so a
future edit that hand-sets a field after resolution fails loudly instead of
quietly measuring the wrong kernel. A raise, not an assert -- `python -O` strips
asserts and this one guards a wrong number.
The CLI plumbing lives in the builder and the benchmark imports it. One
resolver, because two resolvers is how these drifted apart in the first place;
that coupling is also why this is one commit and not two.
Deliberate CLI breaks, both in developer tooling with no in-tree callers:
* omitting --bn / --wpe / --np no longer means 64 / 2 / 228, it means "ask
dispatch". Pass the flag to pin a value.
* `--mode swa` and `--mode varlen` exit 3, not 0. A mode that measured nothing
must never report a green gate. 3 is distinct from 1 (no GPU) and 2 (a shape
failed). `--dry-run` exits 3 for the same reason.
* `--mode persistent` no longer skips; it forces the persistent grid on over
the causal cohort, including the small-work shapes the auto rule leaves on
the default grid.
`--dry-run` resolves and prints the spec for every shape without a GPU, which is
how you check what the gate is about to measure before spending a node on it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The GPU numeric lane hand-rolled its AttentionDenseSpec, which pinned every tuned lever to the shared gfx950 dataclass default. The bf16/D64 row -- the one the cohort labels "the wpe=4 config" -- was therefore validated at waves_per_eu=2. That is not cosmetic: waves_per_eu is emitted as amdgpu-waves-per-eu, changes register allocation, and is tagged into the kernel name, so wpe2 and wpe4 are different binaries. The lane was asserting the correctness of a kernel that does not ship, and would have stayed green through a defect in the one that does. `num_persistent` was hardcoded to 304 beside a dispatch constant that already resolves to 304 -- two copies of one number, free to drift. Route through `dispatch.attention.gfx942._dense_spec`, the same factory the golden lane's dispatch cases already use. Only `dense_persistent` is pinned, because the cohort deliberately asserts BOTH grid variants at one fixed Sq where "auto" would pick one. Every other lever -- block_n, the K row-group pad, persist_decode, ragged, waves_per_eu -- is whatever the shipped path folds in, so a future tuning change is covered here with no edit. Re-verified on hardware after the change: 5/5 pass on gfx942 (ROCm 7.2, torch 2.12.1+rocm7.2), which is the first time the bf16/D64 row has been checked at the binary it actually ships. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`scaled_dot_product_attention(..., enable_gqa=)` is a recent addition. On an older ROCm torch the kwarg raises TypeError, which ERRORS the entire gpu-marked cohort rather than letting the device gate skip it -- a portability failure that presents as a wall of red on a machine that simply has no business running the lane. Repeat each kv head to its query heads with `repeat_interleave` along the head axis before the call. That is exactly what `enable_gqa` does internally, and it is the same mapping the kernel itself uses (hkv = hq // gqa), so the asserted reference is numerically unchanged. On the MHA row rep == 1 and it degenerates to a copy, matching the old `enable_gqa=(hkv != hq)` no-op. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three codegen knobs were module-level constants (_LDS_ROW_PAD, _V_ROW_PAD,
_IGLP) plus a hardcoded query tile. Turning any of them requires editing the
source, which means a sweep is a diff, an A/B is a stash, and the two arms of a
comparison are never simultaneously expressible. Promote them to a frozen
gfx942-owned dataclass threaded through every entry point with a shipped
default.
NOT added to AttentionDenseSpec, deliberately. That dataclass is gfx950-owned
and its `kernel_name()` IS the emitted IR symbol, backing an 18-case gfx950
golden and the C++/Python parity identity. Extending it to carry gfx942 sweep
knobs would move a fixture this port has no business touching. block_m in
particular must not be shared: the gfx950 body reads its module constant
directly with no spec field behind it, so a shared field would be silently
ignored there and a spec asking for 128 would build a 256-row gfx950 kernel. A
separate struct also states the ownership honestly -- these are knobs gfx950
either cannot emit or emits from a different derivation.
Tri-state is the load-bearing design choice. use_cfvst / use_exp2_fast /
waves_per_eu default to None = "use the measured policy", resolving through
_use_cfvst / _use_exp2_fast / spec.waves_per_eu (which dispatch fills from
_tuned_waves_per_eu). A harness that omits a VALUED field freezes the config at
whatever the default was the day it was written and then reports a stale
verdict; a harness that omits a None field auto-tracks whatever ships. The
policy functions stay the single resolution point and their verdicts are
deliberately not copied into the defaults, so there is nothing to drift.
Name tagging is conditional on "differs from the default", so at the shipped
configuration `_tuning_name_tags` is the empty string and every kernel name --
and therefore every golden -- is byte-identical. That conditionality is not
cosmetic: `_DENSE_LAUNCHER_CACHE` is keyed on the kernel name and its
`assert art.kernel_name == key` PASSES on a collision, so an untagged
IR-affecting knob silently serves a stale binary. This kernel has shipped that
exact bug twice, on batch and then on waves_per_eu. Tagging errs toward
over-naming: at D64 a non-default lds_row_pad emits identical IR and gets a tag
anyway, costing one duplicate compile. That direction is free; the other one is
the bug.
`supports_attention_dense` validates the struct, not just the spec, so the
module contract `supports(spec, tuning=t) => build(spec, tuning=t) succeeds`
holds over the sweep surface. A knob only the builder rejects is the same
dispatch fall-through hole every other check in that function exists to close.
Every new branch is inert at the default. Two fields have no rejected region and
say so rather than faking one: use_exp2_fast is a pure perf A/B (both softmax
arguments are always <= 0, so it is numerically safe in both directions, and
gating the known-spilling bf16-D128 arm would make the config unsweepable), and
iglp is a compile-time scheduler directive that leaves no runtime instruction.
Two secondary corrections come with it, both no-ops at the default:
* v_row_pad is now read at exactly two sites -- the _lds_bytes budget and the
V_lds allocation -- from one resolved struct, so the budget can no longer
under-count the allocation it is supposed to model.
* attention_dense_block derives the thread count from block_m rather than
spec.num_waves, which the gfx950 spec hardcodes to _BLOCK_M // 32. Identical
today; without it a block_m sweep point would launch a thread count the body
does not emit.
The lds_row_pad and v_row_pad docstrings also SCOPE the standing "SETTLED" pad
verdict, which had been over-read. What is settled, and catastrophically so, is
pad REMOVAL: D128 stays at 1 WG/CU even unpadded because the register floor
co-limits, so the pads come out for nothing and the conflicts come straight
back. The pad VALUE is a different question and it is open. 8 was inherited from
gfx950 and never re-derived: gfx950 has 64 LDS banks and gfx942 has 32, the
shared derivation is written against the 64-bank model verbatim, the arch docs
list the preferred swizzle as padding on gfx950 and XOR on gfx942 -- so even the
FORM may not transfer -- and gfx950's own sweep of the analogous V pad found +8
essentially indistinguishable from no pad at all. These fields are the knobs
that sweep will turn.
Dispatch touches none of this: every gfx942-private knob stays at its shipped
default, so they are sweep-visible and dispatch-invisible. Wiring one into the
factory would make it a production path and needs its own measured verdict
first.
Default-emitted IR and kernel names are unchanged -- all four dispatch kernel
names and all eight IR body hashes (llvm20/llvm22) are byte-identical, and the
golden lane is green at both flavors with no re-bless.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two guards, both against the same class of silent failure: something moves the emitted IR without moving the thing that is supposed to track it. Kernel-name injectivity. `_DENSE_LAUNCHER_CACHE` is keyed on the kernel name and its `assert art.kernel_name == key` PASSES on a collision, so two IR-distinct configs sharing a name serve the first-compiled binary -- a wrong answer, or an out-of-bounds read, which is what the batch collision was. The existing guard hand-enumerated two of nineteen spec fields, so every field added after it was written was uncovered by construction, which is exactly how batch and then waves_per_eu each shipped the bug. Replace it with a property driven by reflection over `__dataclass_fields__` of BOTH dataclasses: for every field, at every structurally distinct builder arm, a perturbation that moves the IR must move the name. A completeness test makes ADDING a field fail here until it is given a perturbation, which is the whole point. Two anti-vacuity assertions back it: each field must be legally perturbed somewhere, and must reach codegen somewhere, unless it is declared unbuildable or documented as name-only. The property is one-directional on purpose. IR-differs / name-identical is the dangerous direction. Name-differs / IR-identical is a duplicate compile and is allowed -- it is the normal state for `lazy_rescale` (a gfx950 lever this builder ignores but the shared name tags) and for the two row pads at D64. IR is compared with the symbol normalised out; without that, a name change alone would make the IR differ and the implication would hold vacuously. LDS budget vs emitted allocation. `_lds_bytes` is a hand-written model of what the builder allocates, consulted by `supports_attention_dense` to reject over-budget tiles before comgr aborts opaquely. Two models of one fact, free to drift -- and the pads becoming sweepable is exactly what would expose the drift. Assert the strong form across the cohort: `_lds_bytes(spec, tuning)` must EQUAL the size of the emitted `addrspace(3)` pool, not merely bound it. The regex is duplicated from the gfx950 golden's helper rather than imported: that file is a gfx950 test with an 18-case golden behind it and is not ours to touch. Also pins the capacity boundary. The gate is `> capacity`, so a footprint of exactly 65536 B is legal and must be accepted -- correct, since LDS is allocated whole and a workgroup using all of it simply pins 1 WG/CU. That boundary is what keeps the over-budget test honest: at lds_row_pad=0 the same bn128/D128/bf16 spec lands on exactly capacity, the gate flips to ACCEPT, and the over-budget test's premise would silently invert into a tautology. The contract grid's vacuity floors move from a frozen `>= 6` to a fraction of the grid, because the grid grows with every field added and a fixed floor stops being a real check the moment it is a small fraction of the rows. Absolute floors stay underneath so a shrinking grid cannot weaken it either. Adds an executable test for `iglp=True`. Until now the True arm of that branch had never been executed by anything -- it was a constant nobody flipped. Shipping a never-executed codegen branch as a user-settable knob is the highest-risk item in the struct: a knob that quietly emits nothing turns a sweep point into a duplicate of the baseline, reported as a measurement of the lever. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four corrections, all prose: `dispatch/AGENTS.md` claimed the dispatcher "does NOT decide ... waves_per_eu". That is true on the unified path and false for this arm: `gfx942._dense_spec` resolves waves_per_eu, and must. Scope the claim to the unified path and record the standalone-candidate exception with the one rule that governs it -- any value the kernel bakes into its kernel_name must be resolved from the kernel's own policy function, never pinned in the factory, or the name tag and the compiled binary disagree and the name-keyed launcher cache serves the wrong HSACO. The registry row is refreshed to say what the candidate actually covers. The builder README described levers that "default OFF at module scope" by naming two constants that no longer exist, and claimed a non-dispatch build is byte-identical as a consequence. The first half is stale and the second is the wrong reason: the D64 K pad is ON by default (it is the shared `lds_k_group_pad`, whose default is 8, and the `kpad8` name token says so), and the gfx950 golden is untouched because this is a separate kernel module emitting its own symbol against its own fixture -- not because any lever defaults off. Replaced with the split a reader building a spec by hand actually needs, now that the gfx942-private knobs live in `Gfx942DenseTuning`. The "partial-vmcnt software prefetch" lever row gave a reason that does not hold: "NBUF=2 does not fit 64 KB LDS at D128" is only true at the shipped block_n=64, and a K-only double buffer (or block_n=32) does fit. What actually closes the door is the measured block_n=32 verdict in the row above it, so the row says that instead of arithmetic a reader can falsify. Removes the remaining AICK-#### tracker IDs from the files this port owns, per the DSL style guide: an internal tracker reference is meaningless to anyone reading this outside the issue system, and the pointers it stood in for are either in the docstrings already or live in the optimization plan and the protected results page, which the text now names directly. IDs elsewhere in the tree belong to other work and are left alone. The PR description keeps its `ISSUE ID :` line, which is bot policy and separate from repo content. Comment, docstring and markdown only: no emitted-code change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AnaghaRaoAMD
approved these changes
Aug 11, 2026
yraparti
approved these changes
Aug 11, 2026
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.
ISSUE ID : AICK-1664
Ports the gfx950 dense flash-attention prefill kernel (
attention_dense, #9480 / AICK-1663) togfx942 (MI300, CDNA3) and productionizes it there: kernel, builder, live benchmark, dispatch
arm, torch entry point, and the golden-IR + on-GPU numeric CI lanes.
Opt-in only —
autorouting and the gfx950 arm are untouched.Why a separate kernel, not an arch branch in the gfx950 file
The gfx950 body bakes in CDNA4-only primitives, so the algorithm genuinely diverges on CDNA3.
dsl_docs/architecture/multi_arch_data_layout.mdputs that case in a per-gfx module, and it keepsthe gfx950 golden IR byte-identical by construction.
mfma_f32_32x32x16(K=16)mfma_f32_32x32x8(K=8) → 2× per K=16 tile, A/B repack; C-layout identicalds_read_b64_tr_b16(transpose read)ds_read_tr16→perm_b32store-path transposepermlane32_swapperm_b32/ds_bpermutenum_persistents_barrieris safe (NBUF=2)lgkmcnt(sync_lds_only)Two CDNA3 hazards worth reviewer attention
sync_lds_only(), not a bares_barrier. gfx90a+ setFeatureBackOffBarrier, soSIInsertWaitcnts skips the conservative pre-barrier drain and places the lgkm wait at the
ds_read's consumer, which the scheduler may sink past the barrier. Disassembly before the fixshowed
ds_read_u16 x4 -> s_barrier -> s_waitcnt lgkmcnt(0); with NBUF=1 the next iteration'sDMA can then overwrite
V_ldsunder an in-flight read. There is an IR-level regression test forthis, covering both the conflict-free-V and naive tile-start structures.
supports_attention_denseis the single gate.supports(spec)[0] is Trueimpliesbuild_attention_dense(spec)succeeds; a parametrized test asserts that contract in bothdirections, with a guard against the grid collapsing to one branch and making it vacuous.
Divisibility invariants that guard a silent wrong answer raise
ValueErrorrather thanassert, becausepython -Ostrips asserts.Commit series
One revertable lever per commit, each with its rationale in the body:
refactorfeatfeatfeattestperfperfexp2_fastin the softmaxperfexp2_fastto bf16 D64perfwaves-per-eu→ 2 WG/CU at bf16 D64perfperffeatrun_attention_dense_torchentry pointperf_P0_IGLPscheduler-hint knobtesttestdocstestNotes for the reviewer
kernels/gfx950/attention_dense.pyis touched (+12) only for the sharedd64_kpadspecfield (default
False). gfx950 codegen is unchanged and its golden is never re-blessed. Thisis not cross-arch creep.
(head_size, dtype), not globally; the builderREADME.mdcarries the full record of every lever evaluated — adopted and rejected — with themechanism and the reason each won or lost. Module-level knobs (
_P0_D64_KPAD,_P0_IGLP)default off, so non-dispatch builds are byte-identical.
test_bf16_ring_enabled_for_d128asserts thatbf16 D128 takes the sliced-K ring; the code it tests says the opposite
(
# bf16 D128 stays off the ring→return False) and has since fix(rocke/attention): exclude gfx942 D128 from sliced-K ring (correctness) #9647/perf(rocke/attention): recover fp16 gfx942 D128 prefill via depth-2 sliced-K ring #9662. The test is red ondeveloptoday, independently of this branch. It is corrected here as the final commit touchingone test file, so it can be dropped or split into its own PR without disturbing the series.
AGENTS.md, no measured performance data appears in the code, the commit messages, or thisdescription. Lever effects are described qualitatively; magnitudes live in the protected results
page.
Test plan
python rocke/library/tests/run_all.py. All gfx942 dense surface, dispatchwiring, and golden-IR tests pass.
Covers D64/D128 × bf16/fp16 × default/persistent × GQA/MHA × both persistent decodes, at both
llvm20 and llvm22. Two cases are built through the dispatch factory so they guard the
shipped tuning rather than hard-coded levers.
python rocke/library/tests/run_all.py --gpu-only→ 5/5 pass ongfx942 (ROCm 7.2, torch 2.12.1+rocm7.2), driving the public
run_attention_dense_torchagainst an fp32 SDPA oracle for both grid variants. Verified on the rebased tree.
configs (
builders/gfx942/attention/prefill/_p0_occupancy.py).(bf16+fp16 × D64+D128, MHA + GQA including non-power-of-2, causal and full, batch > 1) on
both gfx942 CU counts.
pre-commit run --from-ref origin/develop --to-ref HEADpasses.Deferred / not covered by this PR
gpu-marked and self-skips on CPU CI. It gates ongcnArchNamecontaining
gfx942rather than the marketing name, so it correctly covers MI308X. A CPU-onlyrunner will report it skipped, not passed — please read it that way.
rocke/library/tests/run_all.pyis left to the CI owners (CPU lane onevery runner,
--gpuon a gfx942 runner), consistent with the gfx950 precedent.library/kernels/has none, so the dual-engine byte-identity gate doesnot apply here — the Python lowering is the ground truth.
a structured reason by
supports_attention_dense.Known-red on
develop, not caused by this PRRunning the full library suite on a GPU runner shows 8 failures. All 8 reproduce on a clean
developcheckout, which shows 9 — this branch has one fewer, because the final commit fixes thering-gate test.
tests/test_attention_builds.py(3) — fail identically on cleandevelop; untouched here.tests/test_lds_budget_resolver.py(5) — a pre-existing test-order bug, not a code bug. Thefile passes 9/9 in isolation.
tests/test_attn_bf16_d128_ring.pywrites the module-level cache_RESOLVED_ATTENTION_ARCH = "gfx942"without restoring it on every path, while the resolvertests patch only the
_resolve_attention_archfunction — so they read the leaked value andassert a gfx950 LDS cap against a gfx942 one. It is invisible on CPU because the polluting tests
skip without a device. Verified by pairing: this PR's tests followed by the resolver pass
(14 and 133 passed); the pre-existing ring test followed by the resolver fails (5).
Worth a separate
develop-side fix; deliberately not bundled here, and deliberately notexcluded from the new runner, since silencing them would hide
developbreakage behind this gate.🤖 Generated with Claude Code