Repository navigation
feat(rocke): gfx950 dense flash-attn prefill (~940-970 TFLOPS) - #9480
Merged
yraparti merged 13 commits intoJul 23, 2026
Merged
Conversation
✅ 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. |
…lden test - Kernel: add always-on partial-vmcnt software prefetch + PV-only s_setprio to attention_dense (both default and persistent paths). Bit-identical (err 1.46e-3), 0 spill. Sq=8192 causal: default 521->~543, persistent ~852->~877 TFLOPS. - Dispatch: register an opt-in gfx950 attention_dense KernelCandidate (algorithm="attention_dense", priority 3) that never auto-overrides unified_2d; promote attention_dense_grid/_block/_signature + a run_attention_dense_torch framework entry, exported from kernels/__init__. - Test: add a golden LLVM-IR SHA256 byte-stability test (both llvm flavors) under library/tests/, deliberately NOT wired into CI (no parity emit pair, no harness case, no CMake add_test; library/tests is outside the run_all.py gate). Co-authored-by: Cursor <cursoragent@cursor.com>
…ion) Co-authored-by: Cursor <cursoragent@cursor.com>
Date: Thu Jul 16 23:03:39 2026 +0000
feat(rocke): dense attn gfx950 — sliding-window, varlen, GQA L2 decode, V-pad
Extend the productized dense flash-attention prefill kernel
(kernels/gfx950/attention_dense.py) and add profiling/benchmark harnesses:
- Sliding-window attention (sliding_window spec field, 0 = byte-identical to
the always-on causal path): three-phase band KV loop (window-edge / mask-free
interior / causal-edge) with per-lane bounds reusing the QK-output layout (no
relayout); prunes the KV loop to ~W/block_n tiles. Both default and persistent
builders. Bit-exact vs banded-mask SDPA.
- Varlen batching (varlen + cu_seqlens_q/kv): packed [total_tok, H, D] with
per-sequence runtime offsets/bounds and scf.if early-exit; composes with SWA.
- GQA L2-locality persistent decode (persist_decode=auto, default): hkv-major
causal-balanced work decode keeps each grid-stride phase on ~1 KV head so the
shared GQA K/V stays L2-resident. Measured L2 hit 57%->93%, HBM misses 5.9x
lower, ~+7% TFLOPS @ S=8192 GQA-8 (paired same-session A/B). Balance-safe
gating (gqa*NQB*B >= 2*NP) falls back to qb_major elsewhere (no regression).
- V-row LDS pad (_LDS_PAD_V=32) for the transposed PV read (ds_read_b64_tr_b16):
swept {0:30, 8:29, 16:11, 32:0} conflicts -> +32 fully clears them (K's +8 is
insufficient for the transpose read). Row-strided V DMA to carry the pad.
~+5% TFLOPS, 0 conflicts, no occupancy loss. Headline S=8192 GQA-8: ~953 TFLOPS.
- Host (attention_dense_prefill.py): --sw flag, banded-mask parity, honest
banded/windowed FLOP accounting.
- Harnesses: bench_swa_varlen.py (dense vs flyDSL 0.2.4 vs unified_attention
across sliding-window/varlen/MHA/GQA), _flydsl_bench.py, _prof_dense.py,
_prof_compare.sh (rocprofv3 single-pass counter comparison), plan.md.
Flip AttentionDenseSpec.lazy_rescale default False->True so the parity-identical (1.46e-3) lazy O/l rescale is always-on: persistent Sq=8192 GQA reaches ~980 TFLOPS out-of-the-box (was ~877). Re-bless the golden IR fixture and keep lazy-off branch coverage; C++/Python backend=both remains 13/13 byte-identical.
AnaghaRaoAMD
approved these changes
Jul 21, 2026
…gfx950 Resolve conflict in dnn-providers/hip-kernel-provider/rocke/library/dispatch/ attention.py: develop added the `_make_d256_decode_candidate` (D256 bf16 3D decode) at the same registry insertion point where this branch added the `_make_gfx950_attention_dense_candidate` (+ `_dense_spec` / `dense_spec_for_request`). Both candidates share identical support/select/KernelCandidate scaffolding, so the auto-merge interleaved them; resolved by keeping BOTH as separate factory functions and registering both (dense priority 3, d256 priority 5). Also aligned a stale unit test surfaced by the merge: develop intentionally enabled the bf16 D128 sliced-K ring (`_enable_gfx942_flash_k_sliced_ring`), so `test_bf16_ring_disabled_for_d128` -> `test_bf16_ring_enabled_for_d128` (assertTrue). Post-merge validation: dispatch 41/41, attention_dense golden 18/18 (no IR drift), dense persistent GPU parity 1.41e-3. Co-authored-by: Cursor <cursoragent@cursor.com>
…gfx950 develop advanced (incl. #9233 "D256 gfx950 bf16 prefill 32x32 + softmax-MFMA interleave"), re-conflicting dispatch/attention.py. Same keep-both resolution as the prior merge: - AttentionSpec: keep BOTH the dense `kernel_name_override` and develop's `tiled_overrides` fields. - Keep BOTH candidate factories as separate functions: this branch's `_make_gfx950_attention_dense_candidate` (+ `_dense_spec`/`dense_spec_for_request`, priority 3, opt-in) and develop's new `_make_gfx950_d256_candidate` (priority 5); register both (alongside the existing d256_decode candidate). Post-merge: all 72 dispatch/attention tests pass, attention_dense golden 18/18 (no IR drift). Co-authored-by: Cursor <cursoragent@cursor.com>
yraparti
enabled auto-merge (squash)
July 22, 2026 15:55
2 of 3 tasks
BrianHarrisonAMD
added a commit
that referenced
this pull request
Jul 30, 2026
Section 13 carried eleven `file.py:NNN` citations plus repeated long paths and the fully-qualified `_make_gfx950_attention_dense_candidate` symbol. None were links, and line numbers against a moving branch are stale on arrival. The section now names the kernel once, links PR #9480 (the document's existing convention for source references), and afterwards refers to greppable symbols: AttentionDenseSpec, build_attention_dense, supports_attention_dense, and "the dispatch candidate's support function". The descriptor comments were essays that argued design philosophy and pointed at other sections mid-thought. They now say what the adjacent lines do. The Section 5 callback in the optional-tensor comment is gone, along with similar callbacks in the pin, scale, and mask-classifier comments. Also dropped the retired C++ client's translate()/select() paragraph from 13.1. Section 13 prose is 11.3% shorter. All descriptors, counts, arithmetic, and verified facts are unchanged.
BrianHarrisonAMD
added a commit
that referenced
this pull request
Jul 30, 2026
## Summary Draft PR **against the RFC branch** (`users-bharriso-rfc-ukd-kernel-ingestor`), not `develop`, so the delta from review feedback is reviewable in isolation before it folds into [#9533](#9533). Addresses the **20 unresolved review threads** on #9533 plus the author directives that came out of them. Resolved threads were explicitly excluded. ### Structural additions - **New Section 8, End-to-End Flow.** The largest gap. Covers the nine-step path from applicability request to kernel launch, named against hipDNN's real interfaces (`IEngine::isApplicable`, `initializeExecutionContext`, `getMaxWorkspaceSize`, `IPlan::execute`), with a step/descriptor/interface table, a sequence diagram, and the descriptor/data dependency table reviewers asked for. States plainly that the catalog and bound token state are **provider-owned**. (Second round: the key is now `(engine id, graph id, device id)` against a graph identity this RFC adds, rather than a provider-computed hash. See below.) - **Rewrote the SDPA worked example (§13).** *Superseded in the second round: that rewrite was grounded in the wrong kernel family and has been replaced. See below.* The first-round version was a two-stage accept/reject grounded in the 21 decline conditions `SdpaGraphAdapter` enforced. Shows all three decline shapes (graph-level flag, absence-across-23-optional-tensors, all three tri-state flavors), encodes `classifyMaskMode`, drives geometry from `$kernel.*` instead of a hard-coded `block.x: 256`, and includes a catalog-stage decline where the matcher accepts but no kernel declares that mask mode. ### Policy and lifecycle - KDP schema/SDK version tag, framed as mirroring hipDNN's existing per-graph minimum-required-API-version mechanism rather than as a new invention. - Per-file-type major.minor versioning: reject newer than understood, accept older within the major. - Additive vs. breaking KMD change classification, replacing the previously unconditional retrain statement. - Knobs restricted to the values actually present in the loaded catalog. - Provider boundary: a new runtime dependency means a new provider, plus the enumerated minimum provider surface and the no-Find-mode statement. - `HIPDNN_DISABLE_UKDS`, completing the engine/pack/kernel disable ladder. ### Corrections where the RFC contradicted commitments made in review - Fail-closed is a **base-path** invariant, not composite-only. - Fusion is day-one for *matching*; the fuse decision is the host's and there is no fusion cost model. - MIOpen and hipBLASLt explicitly carved out of provider normalization. - Engine selection is an ordered-policy loop, **not** first-claim-wins; corrected the ONNX Runtime prior-art row accordingly. - Duplicate-id caveat for in-place retunes, deferred to RFC 0009. ## Second round: review-driven corrections A deep review of the first round produced eight blocking findings, and verification passes over the corrections themselves found more. All are addressed here. - **§8's applicability cache is rebuilt on a graph identity.** The first-round design hashed the serialized graph bytes and confirmed a hit with `memcmp`. That cannot be implemented at the `IEngine` layer, which receives a deserialized `IGraph` with no buffer accessor, and it obliged the provider to retain a copy of every cached graph. The key is now `(engine id, graph id, device id)`. Adding the engine id fixes a real aliasing defect: the catalog is per-engine, so the old key let one engine's catalog answer for another in the same provider. - **A drop-in pack appearing now invalidates the cache.** The first round covered removal only, so a dropped-in kernel would never have been picked up for a graph shape the process had already seen, which is the common case in a long-running server. An inventory generation counter is folded into the key. - **A UED's identity is reconciled with hipDNN's engine-id space.** A UED carries a name; the engine id is that name hashed, as for any hand-written engine. Names must be globally unique and should be namespace-scoped (`rocke:SDPA`), and because descriptor engines register at load rather than at build, collision detection moves to a load-time check. - **Autotuning is a single self-measure lever, not per-candidate enumeration.** Modelled on the MIOpen provider's existing `global.benchmarking` knob: the engine measures its own catalog and caches the winner. This is what makes "search within one engine's full catalog" true, since addressing kernels from outside through knob settings would only ever reach the fields an engine chose to expose. - **Adding a kernel is stated as the two-step it is.** A new UKD under an unchanged KMD is loaded, catalogued, and immediately measurable, but it is not ranked against its peers until the engine's UHD is retrained, unless it is the only kernel matching the graph. The first round implied a new kernel was free; §13.8 said "no heuristic". Retraining is self-serve and is now named in §11. - **The UHD scorer interface is pointwise by construction.** A scorer sees one kernel at a time and is never handed the catalog, so knob filtering and ranking commute structurally rather than by author discipline. Set-aware selection is named as future work in the UHD follow-up. - **§5's operator table is normative and closed**, and every operator used in the document is in it. This round added `value_or_default`, `ceil_div`, `rsqrt`, `min`, and `max`, each justified by real dispatch code. - **§14 states the real dependency order.** "No strict build order" was not credible: six of the ten follow-ups are a chain, KMD first, and nothing runs until all six land. ### §13 regrounded on gfx950 dense attention The first-round worked example was built against the wrong kernel family. It cited `attention_unified`'s geometry while the adapter it claimed to encode targets the FMHA-fwd-MFMA family, which is f16-only and has no `num_warps`/`tile_size` axis, so the example's headline **accept** case (bf16) would in reality have **declined**. §13 is now grounded on `rocke/library/kernels/gfx950/attention_dense.py` ([#9480](#9480)), the current dense flash-attention prefill kernel: - Its own spec is already split into functional compile-time fields and validated performance knobs, which maps almost one-to-one onto matcher criteria and KMD fields. - Its real ABI is **five arguments** (four pointers plus `scale`; seven with varlen), not the eight of the first round, because every shape quantity is baked at compile time. Nothing in it needs a source kind this design does not have. - Its two real, measured cohorts (`persistent` vs default) differ in **grid formula shape** rather than argument slots, which surfaced a genuine gap in the one-UDD-per-KDP rule. §6 now names formula shape as a second trigger for a separate pack, distinct from one kernel needing several dispatches, which remains a multi-launch pack. - Where the example depends on something that does not exist yet it says so: there is no AOT artifact for this family today, and the build-only rocKE adapter of §9.1 is the named bridge. - §13.8 now also counts the **new-family** case honestly, which is what this example actually is: ten descriptors, of which eight are authored once and shared by every kernel that later joins. ## Changes this RFC asks of hipDNN core The first round claimed no changes outside the provider. That was too strong. Four are now named in §3 rather than left to be discovered. Each is additive, and none changes an interface an engine implements: | Addition | Why | Section | |---|---|---| | A graph identity, minted at graph-descriptor finalize | so a provider can cache per-graph work; mirrors the existing serialized-buffer cache and its invalidation | §8 | | An extended `$device.*` property set | the expression vocabulary reads device facts hipDNN does not carry today | §5 | | UED names in the engine-id space | a descriptor-backed engine needs an id the host already understands | §4 | | A workspace entry point for custom plans | the workspace query arrives before a plan exists, so a custom-plan handler must answer it | §6 | ## Open decisions for reviewers The seven questions the first round raised (Q1-Q7) are answered in-text by this round. Two decisions remain genuinely open and are the ones worth a reviewer's attention: | # | Question | Where | |---|---|---| | 1 | Whether a future UHD may reason over the candidate set as a whole, giving up the commutativity the pointwise scorer buys | UHD follow-up; scorer interface in §9.2 | | 2 | Whether an operation's conventional attribute defaults, such as SDPA's `1/sqrt(head_size)` scale, are settled once in the schema or left to each pack | §17, Open Question 6 | Two earlier answers are worth re-reading because this round changed them: Q1 (one UDD per KDP) now has a **second** trigger, grid/block/workspace formula shape, not just argument-slot presence; and Q7 (pinning a specific UKD) is now clearly optional rather than load-bearing, because the self-measure lever reaches the whole catalog without per-kernel addressing. ## Third round: rocKE integration and four review findings The second round's §13 described what did not exist yet: a `kernel_source` pointing at an `hsaco` file nobody builds, a `custom_library` UHD wrapping a hardcoded host threshold, and a §13.8 leading with the missing build step. This round describes the integration instead, and fixes four findings a scoped review pass raised against §5, §6, and §13. ### rocKE becomes a real kernel source - **New `rocke` kernel-source kind** (§7, §9.1). A rocKE kernel is a Python builder that emits IR from a frozen spec, so it is neither directly loadable nor one artifact: one builder yields a different code object per spec. The source therefore carries both halves, `source` + `entry` plus the explicit `build` values for one instance, and the build-only adapter calls the builder, runs the rocKE AOT compile, and packs the result. - **A pack's kernel vector is its AOT build list.** Every UKD names the same builder and differs only in its build values, so what gets compiled is derived from what is catalogued rather than maintained beside it. No descriptor names an artifact file; the adapter owns the name. - **§13.6's UHD is a real UHD**: a model ranking the engine's whole catalog, with `persistent` and `num_persistent` as ordinary features and the work term the retired host threshold branched on as a derived feature. The hardcoded `work >= num_persistent` rule is one decision boundary in the training data, not a rule the descriptors carry. - **§13.8 no longer narrates absence.** Step 1 is picking build values, not waiting for tooling. Ragged, varlen, and sliding-window stay as extension points. - **rocKE is never called a DSL.** Generic DSL usage elsewhere is unchanged. ### Review findings - **`batch` was baked but never pinned** (blocking). `AttentionDenseSpec` bakes batch (`attention_dense.py:107`); it sizes the K/V buffer-resource bounds and, in the persistent cohort, the grid-stride trip count. §13's KMD had no `batch` field and the matcher no `$kernel.batch` pin, so it accepted any batch and could hand a graph to a kernel built for another, the exact silent-wrong-kernel class §5's pinning rule exists to prevent. Added the field and the pin. - **`$scale.value` was not expressible** (blocking). It named an eight-arm flatbuffers union with no generic numeric accessor, and it was passed as the second argument to `value_or_default`, whose §5 definition took a literal. Resolved by adding `$q.value_f32` as a **precomputed field** (§5's existing mechanism, the same one `$q.packed` and `$q.stride_order` use), generalizing `value_or_default`'s fallback to any type-compatible expression including a field reference, and gating the scale tensor's dtype so a non-float value cannot narrow silently into an `f32` kernarg. - **§13.7 contradicted §13.5** (blocking). The table claimed `args_signature` needs "no computed sources" while §13.5's `scale` is an explicit `expr` source. The row now says what is there. - **Two ungrounded `$graph.*` tokens.** `$graph.node_kind` is not a field in `graph.fbs` and is redundant with the structural pattern, so it is deleted. `$graph.override_shape` mis-named the real `is_override_shape_enabled`, now renamed and declared in §5's Graph namespace. - **Three baked tuning axes were missing from the KMD**, found while verifying the round above. `waves_per_eu`, `lazy_rescale`, and `interleave` are baked into the IR by both builders but were not KMD fields, so two kernels differing only in one of them would collide on the catalog key. Making the `build` values explicit is what exposed this; the artifact filenames had hidden it. All three added with the spec's own defaults. - **`persist_decode` carries the resolved decode, never `"auto"`.** `"auto"` is not a third configuration but a request to compute one (`resolved_persist_decode`), and the KMD is both the catalog key and the heuristic's feature space: two UKDs carrying `"auto"` but resolving differently would collide on the key and rank on a string rather than the decode they were built with. This answers the reviewer's open question on the point. - **Residual drafting artifacts** removed: three places narrated the edit history ("is the fix") rather than stating the design. Verified correct by two reviewers against source and left unchanged: the 27-name operand map vs `sdpa_attributes.fbs`; the 23-of-24 refusal list vs `AttentionDenseSpec`; §13.3's mask classifier vs `getMaskType`; all three §13.4 cases re-derived numerically; grid/block/args vs `attention_dense_grid`/`_block`/`_signature`; `shared_mem_bytes: 0`; the dtype pin spelling; and every §13 prose count. ## Risk Assessment Risk 1 (docs only). No product code, schemas, plugin ABI, or build wiring changes in this PR; blast radius is one file under `projects/hipdnn/docs/rfcs/`. Note that the design it describes now asks for four additive changes to hipDNN core (see above) which are not implemented here. ## Testing Summary - Anchors and TOC: every in-document reference resolves; TOC matches all 19 headings. - All complete JSONC descriptor blocks parse; multi-object blocks verified object by object. - Descriptor cross-references resolve: every `matchers`, `engine`, `dispatch`, `heuristic`, and `metadata` id resolves to a descriptor defined in the document, except the two in §15.2's explicitly-labelled UCD sketch, which are pre-existing. - Every `$kernel.*` field referenced by §13's matcher and both UDDs is declared in its KMD; the two UKDs' metadata tuples are distinct; both `args_signature` lists match the real five-argument ABI. - Operator closure: every operator used anywhere in the document appears in §5's normative table. - §13's factual claims verified against `attention_dense.py` and `dispatch/attention.py` on `origin/develop`, including the accept/decline arithmetic. - Code fences balanced; no em-dashes, tabs, CRLF, or trailing whitespace; final newline present. - `pre-commit run --files` on the changed file: passed. ## Testing Checklist - [x] Doc validation (links/anchors, TOC, fences, whitespace/EOL) - Status: Passed - [x] `pre-commit` - Status: Passed - [ ] PR CI - GitHub PR checks (`ci:docs-only`) - Status: Pending ## Technical Changes `projects/hipdnn/docs/rfcs/0017_UniversalKernelDescriptor.md`: +164 / -89 this round (+1570 / -341 against the base branch). §13 rewritten and regrounded on `attention_dense`; §8 cache rebuilt on a graph identity with an inventory generation; §5 operator table made normative and closed; §4 UED identity; §2 self-measure lever; §16 dormancy and lifecycle; §6 formula-shape pack trigger and the custom-plan workspace obligation; §3 core-additions table; §11 retraining tooling; §14 dependency order; §17 two new open questions; §19 glossary entries for the new terms. Third round: §7 and §9.1 gain the `rocke` source kind; §13.6's UKDs and UHD are rebuilt on it; §5 declares `value_f32`, `is_runtime_pass_by_value`, and `is_override_shape_enabled`, and generalizes `value_or_default`; §13.2 pins `batch` and gates the scale tensor's dtype.
6 tasks done
AnaghaRaoAMD
pushed a commit
that referenced
this pull request
Aug 14, 2026
…ll kernel (#10337) ISSUE ID : AICK-1664 Ports the gfx950 dense flash-attention prefill kernel (`attention_dense`, #9480 / AICK-1663) to **gfx942 (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 — `auto` routing 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.md` puts that case in a per-gfx module, and it keeps the gfx950 golden IR byte-identical by construction. | Concern | gfx950 (CDNA4) | gfx942 (CDNA3) | |---|---|---| | MFMA atom | `mfma_f32_32x32x16` (K=16) | `mfma_f32_32x32x8` (K=8) → 2× per K=16 tile, A/B repack; **C-layout identical** | | Conflict-free V | `ds_read_b64_tr_b16` (transpose read) | **no `ds_read_tr16`** → `perm_b32` store-path transpose | | Cross-half exchange | `permlane32_swap` | **absent** → `perm_b32` / `ds_bpermute` | | LDS / CUs | 160 KB, one CU count | 64 KB, two CU counts → retune occupancy, `num_persistent` | | Tile barrier | bare `s_barrier` is safe (NBUF=2) | single buffer → the tile barrier **must** drain `lgkmcnt` (`sync_lds_only`) | ## Two CDNA3 hazards worth reviewer attention - **`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. There is an IR-level regression test for this, covering both the conflict-free-V and naive tile-start structures. - **`supports_attention_dense` is the single gate.** `supports(spec)[0] is True` implies `build_attention_dense(spec)` succeeds; a parametrized test asserts that contract in both directions, with a guard against the grid collapsing to one branch and making it vacuous. Divisibility invariants that guard a *silent wrong answer* raise `ValueError` rather than `assert`, because `python -O` strips asserts. ## Commit series One revertable lever per commit, each with its rationale in the body: | | | |---|---| | `refactor` | share the attention LDS-capacity accessor | | `feat` | the gfx942 dense prefill kernel + arch alias export | | `feat` | opt-in dispatch candidate | | `feat` | builder + live benchmark | | `test` | CPU surface + dispatch-wiring tests | | `perf` | lift conflict-free-V (D128 fp16) | | `perf` | `exp2_fast` in the softmax | | `perf` | fuse the softmax rescale; widen `exp2_fast` to bf16 D64 | | `perf` | tune `waves-per-eu` → 2 WG/CU at bf16 D64 | | `perf` | persistent grid-stride variant | | `perf` | adopt the D64 K-bank-conflict pad | | `feat` | `run_attention_dense_torch` entry point | | `perf` | default-off `_P0_IGLP` scheduler-hint knob | | `test` | golden-IR lane | | `test` | wire the library tests into a collectable CI lane | | `docs` | re-baseline the kernel + README to the measured bound | | `test` | correct the bf16-D128 sliced-K ring gate test *(separable — see below)* | ## Notes for the reviewer - **`kernels/gfx950/attention_dense.py` is touched (+12) only for the shared `d64_kpad` spec field** (default `False`). gfx950 codegen is unchanged and its golden is never re-blessed. This is not cross-arch creep. - **Per-config tuning.** Levers are gated per `(head_size, dtype)`, not globally; the builder `README.md` carries the full record of every lever evaluated — adopted *and* rejected — with the mechanism and the reason each won or lost. Module-level knobs (`_P0_D64_KPAD`, `_P0_IGLP`) default off, so non-dispatch builds are byte-identical. - **The last commit is deliberately separable.** `test_bf16_ring_enabled_for_d128` asserts that bf16 D128 takes the sliced-K ring; the code it tests says the opposite (`# bf16 D128 stays off the ring` → `return False`) and has since #9647/#9662. The test is red on `develop` today, independently of this branch. It is corrected here as the final commit touching one test file, so it can be dropped or split into its own PR without disturbing the series. - Per `AGENTS.md`, no measured performance data appears in the code, the commit messages, or this description. Lever effects are described qualitatively; magnitudes live in the protected results page. ## Test plan - [x] **CPU lane** — `python rocke/library/tests/run_all.py`. All gfx942 dense surface, dispatch wiring, and golden-IR tests pass. - [x] **Golden-IR (gfx942)** — new separate file + fixture; the gfx950 fixture is never written. 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. - [x] **On-GPU numeric** — `python rocke/library/tests/run_all.py --gpu-only` → **5/5 pass** on gfx942 (ROCm 7.2, torch 2.12.1+rocm7.2), driving the public `run_attention_dense_torch` against an fp32 SDPA oracle for both grid variants. Verified on the rebased tree. - [x] **Resource guard** — 0 VGPR/SGPR spill and within the VGPR/LDS budget across all shipped configs (`builders/gfx942/attention/prefill/_p0_occupancy.py`). - [x] **Cohort parity** — validated against an fp32 SDPA reference across the in-scope cohort (bf16+fp16 × D64+D128, MHA + GQA including non-power-of-2, causal and full, batch > 1) on **both** gfx942 CU counts. - [x] `pre-commit run --from-ref origin/develop --to-ref HEAD` passes. ### Deferred / not covered by this PR - **hipDNN reachability** (epic DoD) is still open and needs coordination with the hipDNN team. - **The on-GPU numeric lane is `gpu`-marked and self-skips on CPU CI.** It gates on `gcnArchName` containing `gfx942` rather than the marketing name, so it correctly covers MI308X. A CPU-only runner will report it skipped, not passed — please read it that way. - **CI registration of `rocke/library/tests/run_all.py` is left to the CI owners** (CPU lane on every runner, `--gpu` on a gfx942 runner), consistent with the gfx950 precedent. - **No C++ engine mirror**: `library/kernels/` has none, so the dual-engine byte-identity gate does not apply here — the Python lowering is the ground truth. - D256 is out of scope (AICK-1495 / AICK-1496). Varlen, ragged and sliding-window are rejected with a structured reason by `supports_attention_dense`. ### Known-red on `develop`, not caused by this PR Running the full library suite **on a GPU runner** shows 8 failures. All 8 reproduce on a clean `develop` checkout, which shows 9 — this branch has one *fewer*, because the final commit fixes the ring-gate test. - `tests/test_attention_builds.py` (3) — fail identically on clean `develop`; untouched here. - `tests/test_lds_budget_resolver.py` (5) — a pre-existing **test-order** bug, not a code bug. The file passes 9/9 in isolation. `tests/test_attn_bf16_d128_ring.py` writes the module-level cache `_RESOLVED_ATTENTION_ARCH = "gfx942"` without restoring it on every path, while the resolver tests patch only the `_resolve_attention_arch` *function* — so they read the leaked value and assert 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 **not** excluded from the new runner, since silencing them would hide `develop` breakage behind this gate. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Yaswanth Raparti <113389104+yraparti@users.noreply.github.com>
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.
JIRA ID : AICK-1663
Summary
kernels/gfx950/attention_dense.py(AttentionDenseSpec/build_attention_dense/supports_attention_dense) + host builder + README.exp2_fast, full sched_group_barrier template, diagonal-only causal masking, depth-1 cluster, partial-vmcnt software prefetch, PV-onlys_setprio, vectorized O store.persistent=True,num_persistent): amortizes per-CTA launch/setup + K/V cold-start over many query blocks (qb-major decode).IRBuilder.exp2_fast(llvm.amdgcn.exp2.f32→ onev_exp_f32) + LLVM-direct lowering.attention_denseKernelCandidate(never auto-overridesunified_2d), publicattention_dense_grid/_block/_signature, and arun_attention_dense_torchentry point exported fromkernels/__init__.Measured (MI355X, bf16, D=128, Hq=128, Hkv=8, causal, 0 spill, err ~1.46e-3 vs SDPA)
Test plan
max_abs ~1.46e-3@ Sq=8192, both default and persistent.algorithm="attention_dense"selects the candidate;autostill routes tounified_2d(no behavior change).library/tests/test_attention_dense_golden.py, both llvm flavors) — passes; intentionally NOT wired into CI yet.exp2_fastin the C++ engine + byte-identity coverage (follow-up; LLVM-direct only for now).