Skip to content

feat(rocke): attention band coverage on gfx942 - #13211

Draft
AnaghaRaoAMD wants to merge 33 commits into
developfrom
users/anarao/gfx942-attention-band
Draft

AnaghaRaoAMD wants to merge 33 commits into
developfrom
users/anarao/gfx942-attention-band

Conversation

@AnaghaRaoAMD

@AnaghaRaoAMD AnaghaRaoAMD commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

ISSUE ID : AICK-2520

Summary

Closes the gfx942 gap left by PR #13125: gfx942 forward attention now computes the same cuDNN/hipDNN attention band as gfx950 (top-left alignment, right bound / lookahead, non-causal and two-sided windows, NO_MASK = full attention, exactly-zero rows with no visible key) instead of serving every request with the bottom-right causal body. It also fixes three pre-existing gfx942 4-warp GQA bugs found along the way.

Risk Assessment

High risk (4). On gfx942 the default request mask (NO_MASK) now means full attention instead of being served by the causal body, square top-left requests now select the _tl kernels, and SLIDING_WINDOW with a window is a non-causal band; this mirrors what #13125 did for gfx950 and is covered by CPU routing tests and gfx942 GPU numeric tests. Ragged bottom-right chunked prefill now routes to the gfx942 dense kernel by default (covered at batch 2 on both grids). Strided decode now refuses top-left decode with right_bound = 0, which it previously ran bottom-right; this also changes gfx950. Default-mask IR is byte-identical except the gfx942 4-warp GQA bodies, whose bug fixes re-bless their golden entries. The binary ABI moves from 6 to 7 (spec struct layouts). The right_bound / window bodies are Python-only (C++ mirror tracked in AICK-2655).

ASIC Coverage

gfx942 run required (done locally, see below). Kernel changes are gfx942-only, apart from the band helpers shared with gfx950 (zero IR change). The dispatch change (normalize_band admits gfx942) alters gfx942 routing; on gfx950 the only routing change is strided decode refusing top-left decode with right_bound = 0 (previously run bottom-right), and its C++ tiled entry points reject causal_top_left, which only their Python bodies implement; gfx1250 keeps its legacy behaviour. Because default dispatch behaviour shifts on gfx942, PR CI should run the gfx942 lane. test_attention_band_gfx942_numpy_numeric.py runs the gfx942 band numerics without torch (numpy reference, rocke.runtime device buffers), so the gfx94X GPU CTest lane exercises the band in CI, and the C++ rejection CTest rocke_attention_tiled_band_rejects runs without pybind; the broader Torch-gated suites (test_attention_unified_mask_gfx942_numeric.py, test_attention_dense_gfx942_numeric.py) still skip in CI without Torch and were run locally.

Testing Summary

  • gfx942 hardware numeric: unified 2D / 3D / 4-warp kernels against an explicit reference for top-left, full attention, non-causal and two-sided windows, lookahead, decode, sinks, softcap, D64/D128/D256, GQA and exactly-zero empty rows; index-level truth tables (visible-key set equals d - k < left and k <= d + right) for unified 2D/3D and dense grid/persistent; request → dispatch → launch on a mixed-length cohort; dense band matrix at batch=2 including persistent, ragged cross-attention, sinks and LSE.
  • IR pinning: 20 gfx942 band cases added to the platform representative golden shards (each asserts its kernel-name tags); every pre-existing entry is byte-identical except the 4-warp D128 SWA fold, D256 and D192 entries, re-blessed for the 4-warp fixes. Top-left on the tiled 2D/3D bodies is also mirrored in the C++ engine with emit-pair configs.
  • Dispatch wiring: gfx942 band routing, kernel names and cache keys per band, 4-warp cohorts declining a right bound, gfx1250 unchanged.

Testing Checklist

Results are at b16fffa unless noted.

  • Definition-of-done gate - python tools/run_checks.py - ASICs: gfx942 host - Status: Passed for guard, byte-identity (llvm20 / llvm22 / llvm23 / llvm24), arch-domain, reproduction, instance parity and numeric. ir-validity reports 18 cases (gfx950 scalar attention, gfx1250 Winograd) that also fail on develop with the node's clang. (run at ba01620, before the latest develop merge and review fixes)
  • rocKE library CPU lane - pytest -m 'not gpu' library/tests (sharded) - Status: 67,642 passed; the 30 failures (strided-KV decode / LDS, conv ABI / dgrad hash, KDA hostpack, early-stop) also fail on develop (run at ba01620)
  • rocKE platform tests - python tests/test_rocke.py and python tests/run_all.py (from platform/) - Status: test_rocke.py OK (368 run); run_all.py gate GREEN, 5,513 passed, 24 hipcc / HIP-compile failures that reproduce on develop with the node's clang (run at 2476682)
  • Binding parity - python platform/cpp/bindings/prove_parity_binding.py - Status: all 5 tiled attention families VALIDATED (gfx942 2D 21/21, 3D 24/24; gfx950 2D 68/68, fastKV 7/7, 3D 20/20); 2 failures outside this PR (moe_fused_mega LDS budget; 3 gfx950 selector problems identical on develop) (run at ba01620); at the current head the C++ rejection CTest rocke_attention_tiled_band_rejects passes 1/1 and the pybind rejection tests 9/9
  • Attention CPU tests - pytest library/tests/dispatch/attention (incl. test_tuning_identity.py) plus the dense gfx942, dense golden, bottom-right, LSE, workspace and band-helper files - Status: Passed (1,461 passed, 0 failed; the 253 skips are the RDNA-only gfx1151 dispatch numerics)
  • gfx942 torch-free band numerics - pytest -m gpu library/tests/test_attention_band_gfx942_numpy_numeric.py (runs in CI without torch) - ASICs: gfx942 - Status: Passed (34 passed, with and without import torch blocked)
  • gfx942 attention GPU lane - pytest -m gpu on the torch-free band, unified mask, dense (incl. the BF16 CFVST / KDB / VPF pipeline) and head-fold files - ASICs: gfx942 - Status: Passed (unified mask, dense and head-fold: 728 passed, 0 failed; torch-free band: 34 passed)
  • Full rocKE library GPU lane - python library/tests/run_all.py --gpu-only - ASICs: gfx942 - Status: Not run (in a full-tree session every GPU test skips as "needs a gfx942 GPU"; the per-file run above is the attention coverage)
  • rocKE installed golden gate - python platform/tests/rocke_installed_golden_test.py - Status: Passed (llvm20, llvm22, llvm23, llvm24); test_rocke_ci_static.py 12 passed
  • Byte-identity gate - tools/check_byte_identity.py --only attention - Status: Passed (llvm20, llvm22, llvm23, llvm24; 11/11 attention families)
  • gfx942 parity harness - python -m builders.gfx942.attention.prefill.parity_unified_attention --set topleft|band|nomask - ASICs: gfx942 - Status: Passed (14/14, 28/28, 12/12)
  • Pre-commit hooks - pre-commit run --from-ref origin/develop --to-ref HEAD - Status: Passed
  • PR CI - GitHub PR checks - Status: Pending

Technical Changes

  • Routing changes vs develop (CPU differential of 18,144 requests, including layout='dense' decode: arch × mask × lengths × alignment × window × right bound × dtype × head size × batch): gfx1250 changes only in rejection wording. On gfx942, NO_MASK becomes full attention (_rbu, 4-warp cohorts decline it), right-bound / window bands previously rejected now route, top-left with Sq ≠ Sk keeps its alignment, and the dense factory refuses invalid band requests with a reason. Requests develop served that are now refused, all because develop computed the wrong mask: 72 gfx942 top-left NO_MASK + window decodes (develop ran the bottom-right window) and 192 gfx950 strided top-left decodes with right_bound = 0 (develop ran bottom-right). Bottom-right windowed decode at Sq = 1 keeps develop's kernel.

  • gfx942 tiled 2D / 3D: causal_top_left (context_len = 0, ALiBi / QQ-bias offsets follow, as on gfx950) and right_bound (0 causal, R lookahead, -1 unbounded) at every mask, KV-bound, window and phase-split site. Top-left is mirrored in the C++ twins; right_bound is Python-only and the pybind / C entry points reject it rather than drop it.

  • gfx942 dense: right bound, non-causal and two-sided windows, full and ragged cross-attention, and a zero-output guard for rows with no visible key, in the default and persistent grids, composed with develop's sinks, LSE and runtime-shape support.

  • Dispatch: gfx942 joins gfx950 in normalize_band; dense_pipe and the gfx942 dense candidate serve the band; the 4-warp GQA cohorts take top-left but decline a right bound (falling back to the generic tiled body); gfx942 live benchmarks pin BOTTOM_RIGHT_CAUSAL.

  • Pre-existing gfx942 4-warp GQA fixes (all reproduce on develop with the default mask): rows with no visible key returned NaN or averaged masked values (D128 SWA, D256 lean); the partial last q-block read Q past the end of the query tensor; the last KV tile's masked tail keys read block-table entries past the sequence's row (hipError 700).

  • Dense preflight (shared by gfx942 and gfx950): ragged specs also bound the Q/O byte extent (2 * qo_elems < 2**31), since the ragged bodies use an i32 byte voffset. The pybind 3D builder rejects right_bound like the 2D builders.

  • Torch-free gfx942 band test (test_attention_band_gfx942_numpy_numeric.py) for CI lanes without torch; run_unified_attention_torch launches raw device buffers (no device) eagerly, with HIP-allocated 3D scratch and no 2D graph replay, while torch tensors take the previous paths.

  • Coverage: the unified band GPU suites join the installed CTest lists, and docs/gpu-attention-test-coverage.md maps them and the gfx942 dense band additions.

  • Dense BF16 CFVST / KDB / VPF pipeline (develop's opt-in cohort): admitted only for the causal, non-ragged band (right_bound == 0, not ragged); ragged and lookahead requests take the default body, with the reason reported by supports_attention_dense.

  • Known gaps: gfx1250 tiled kernels remain causal-only; the right_bound / window bodies are Python-only (recorded in platform/dsl_docs/development/known_gaps.md; C++ mirror tracked in AICK-2655). The C++ binding rejects them, which a CPU test pins; that test skips in CI because no lane builds the pybind module.

@github-actions github-actions Bot added documentation project: hip-kernel-provider rocke-lib: attention Touches rocKE library attention/FMHA kernels rocke: platform Touches rocKE platform labels Oct 6, 2026
@AnaghaRaoAMD
AnaghaRaoAMD force-pushed the users/anarao/aick-2505-top-left-causal-coverage branch 5 times, most recently from 4597af4 to e08e170 Compare October 14, 2026 23:51
@AnaghaRaoAMD
AnaghaRaoAMD force-pushed the users/anarao/aick-2505-top-left-causal-coverage branch 4 times, most recently from 3d871e8 to 996c351 Compare October 16, 2026 22:18
Base automatically changed from users/anarao/aick-2505-top-left-causal-coverage to develop October 17, 2026 01:44
@therock-pr-bot

therock-pr-bot Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@AnaghaRaoAMD
AnaghaRaoAMD force-pushed the users/anarao/gfx942-attention-band branch from aade049 to 414c5ff Compare October 19, 2026 22:52
@therock-pr-bot

Copy link
Copy Markdown

Pre-commit check failed

⛔ pre-commit failed

Please run locally:

  • python -m pip install pre-commit
  • pre-commit install
  • pre-commit run --all-files --show-diff-on-failure

This repo uses .pre-commit-config.yaml.

@AnaghaRaoAMD
AnaghaRaoAMD marked this pull request as ready for review October 20, 2026 01:40
@AnaghaRaoAMD
AnaghaRaoAMD requested review from a team as code owners October 20, 2026 01:40
@AnaghaRaoAMD
AnaghaRaoAMD requested a review from dlangbe October 20, 2026 21:08

@dlangbe dlangbe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the new test_attention_unified_mask_gfx942_numeric.py isn't in the installed GPU CTest selection or the coverage map. platform/CMakeLists.txt _ROCKE_LIBRARY_GPU_TEST_RELPATHS lists test_attention_dense_gfx942_numeric.py and test_gfx942_gqa_head_fold_numeric.py, but not this file. docs/gpu-attention-test-coverage.md has no rows for it either, and its "Maintaining the map" section asks for both when a GPU test is added. The +473 lines in test_attention_dense_gfx942_numeric.py (band matrix at batch=2, ragged cross, empty rows) aren't in the map. The "Unequal query/KV lengths, bottom-right masks" gap row also still says the detailed cohort is gfx950-only.

This file is the main GPU coverage for the NO_MASK → full-attention flip on the unified path, and for the 4-warp block-table clamp (the hipError 700 fix). Nothing outside a local run pins either one. Also, the map's "gfx942 CI without Torch" column says Torch-based gfx942 suites skip in CI, so the green gfx94X hipkernelprovider shards on this PR aren't evidence for the band. Either add the file to the CTest list and the map, or note in the PR body that band GPU coverage is local-only. The gfx950 twin test_attention_unified_mask_gfx950_numeric.py from #13125 is missing from both too.

AnaghaRaoAMD added a commit that referenced this pull request Oct 20, 2026
… t3d_build

Review follow-ups on #13211:
- check_dense_spec_preflight (shared by gfx942 and gfx950): the ragged
  bodies address Q/O with an i32 byte voffset (addr * 2, mul nsw), so a
  ragged spec with qo_elems in [2**30, 2**31) passed the element bound and
  overflowed. Ragged specs now also require 2 * qo_elems < 2**31; the
  aligned path (element-indexed) is unchanged. Test on both arches with
  B8 S8200 Hq128 D128.
- pybind t3d_build rejects right_bound != 0 like t2d_build / fkv_build,
  instead of building the causal body under a name without rb<R>.
- gfx950 C++ tiled entry points: the causal_top_left reject now says the
  gfx950 C++ twin lacks it (gfx950 implements it in Python), and the
  shared 3D struct comment matches the gfx942 header.
- gfx942 dense: refresh the do_mask comment for non-causal windows.

Gate: gfx942_attention_tiled and gfx950_attention_tiled GREEN at
llvm20/llvm22/llvm23.

Co-Authored-By: Claude <noreply@anthropic.com>
AnaghaRaoAMD added a commit that referenced this pull request Oct 20, 2026
…overage

Review follow-up on #13211: test_attention_unified_mask_gfx942_numeric.py
joins the installed library GPU CTest list and its gfx950 twin from #13125
joins the attention GPU list next to gfx950 dense. The GPU attention
coverage map gains explicit rows for both unified suites and for the
gfx942 dense band additions (batch=2 band matrix, ragged cross-attention,
empty rows, sinks/LSE band, truth table), updated counts, and the
unequal-length gap row now lists the gfx942 cohorts. These suites are
Torch-gated and skip in CI without Torch.

Co-Authored-By: Claude <noreply@anthropic.com>
@AnaghaRaoAMD

Copy link
Copy Markdown
Contributor Author

@dlangbe thanks, the coverage gap is addressed in 412a42f:

  • test_attention_unified_mask_gfx942_numeric.py is now in _ROCKE_LIBRARY_GPU_TEST_RELPATHS. The gfx950 twin from feat(rocke): top-left causal and attention band coverage on gfx950 #13125 goes into _ROCKE_LIBRARY_ATTENTION_GPU_TEST_RELPATHS, next to gfx950 dense.
  • docs/gpu-attention-test-coverage.md has explicit rows for both unified suites and for the gfx942 dense additions (batch=2 band matrix, ragged cross-attention, empty rows, sinks/LSE band, truth table). It also has updated counts, and the unequal-length/bottom-right gap row now lists the gfx942 cohorts.
  • The PR body now states that these suites are Torch-gated and skip in CI without Torch, so the gfx94X CI shards are not evidence for the band, and that the gfx942 GPU results come from local runs: 627 passed across the dense, unified mask and head-fold files.

Two older map entries were already stale and are still out of scope here: about 17 develop-era gfx942 dense tests (sinks, LSE, non-default scale) have no shape rows. I corrected the selection label for gfx950 dense, which runs in rocke_library_attention_gpu_pytest.

@CongMa13 CongMa13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No new issues found. I agree with the following existing comments:

  • Ragged dense Q buffer byte extent overflow in 32-bit voffset (@dlangbe, comment ID: 4330704478, thread ID: PRRT_kwDOOej8qc6uigTb)
  • C++ bindings t3d_build missing reject_right_bound (@dlangbe, comment ID: 4330718942, thread ID: PRRT_kwDOOej8qc6uiibj)
  • Inaccurate error string and comment regarding causal_top_left on gfx950 (@dlangbe, comment ID: 4330727852, thread ID: PRRT_kwDOOej8qc6uijw1)
  • Stale comment for do_mask in attention_dense.py (@dlangbe, comment ID: 4330739879, thread ID: PRRT_kwDOOej8qc6uilkc)
  • Missing GPU test files in CTest selection and coverage documentation (@dlangbe, review ID: 5592204236)

@CongMa13 CongMa13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No new P0 or P1 issues were identified during this review. The changes are approved.

AnaghaRaoAMD and others added 3 commits November 2, 2026 15:51
The band added defaulted fields to the gfx942 tiled 2D / 3D kernel specs,
which moves every config_key of those variants. Pin a 2D and a 3D
tuning_id computed from develop and assert dispatch raises PinRefused
naming the new id, and that the new id replays.

Co-Authored-By: Claude <noreply@anthropic.com>
…e case

The band lets the gfx942 dense spec take ragged non-causal Sq != Sk, which
develop refused. Run 197x400 full attention, bf16, B2 against the NumPy
reference so CI without Torch covers the ragged q and k tails.

Co-Authored-By: Claude <noreply@anthropic.com>
_is_raw_device_buffer keyed on the missing device alone, so any q
without a device -- including the None placeholders of the host routing
tests -- skipped gfx942 2D graph replay, and
test_graph_replay_path_checks_the_cache no longer reached the graph
path's block-count guard. Require data_ptr() as well, which every raw
device buffer (DeviceMem wrappers) exposes.

Co-Authored-By: Claude <noreply@anthropic.com>
@AnaghaRaoAMD

Copy link
Copy Markdown
Contributor Author

@cgmillette thanks, both are addressed:

  • N1 (golden drift after fix(rocke): carry the raw running max in unified attention #14542): develop is merged into the branch (fd74969), and the attention shards were regenerated with the harness writer. Compared with develop, the shards only add the 20 gfx942 band cases and re-bless the three 4-warp entries (attention_d128_swa/gfx942/4warp_gqa_fold, attention_d256/gfx942/4warp_gqa, ..._d192). The 6 2d_fp16_d128_b64_* entries now match develop's hashes. On that merge:
    • byte identity (--only attention) is green at llvm20 / 22 / 23 / 24;
    • the installed golden gate passes at all four flavors;
    • gfx942 GPU: 683 passed, 0 skipped.
  • N2 (PR number in a comment): fixed in d68b613.

I also added one ragged cross-length dense case (197×400 full attention) to the torch-free band suite, as you suggested (2476682). The review fixes are at ba01620. The post-merge full-lane re-run is in progress, and the PR body will be updated when it finishes.

# Conflicts:
#	dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/README.md
#	dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py
#	dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py
@AnaghaRaoAMD
AnaghaRaoAMD enabled auto-merge (squash) November 3, 2026 01:46
@cgmillette

Copy link
Copy Markdown
Contributor

Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max

PR #13211 — attention band coverage on gfx942 (AnaghaRaoAMD, AICK-2520) — 🟠 Follow-up: duplicated band code, unrun tests

Prior item Status
dlangbe (10-20) ×5: ragged Q/O voffset overflow; t3d_build drops right_bound; gfx950 reject text; do_mask comment; band suites missing from CTest 🟢 FIXED 144200e8, e857e057
• all 5 intact after both develop merges (read)
• all 16 threads now resolved
CongMa13 (10-21) APPROVED at 412a42fc 🟢 APPROVED 412a42fc
• older head; still counts on develop; predates the 10-28 rework and both develop merges
• library/ still needs a rocke-lib owner's approval; dlangbe requested
Ours (10-21…10-29) ×4: conflicts, golden, band numerics in CI 🟢 FIXED
• hold at head; band suite 30 ✓ in both gfx94X CI shards (read)
Ours 🔴 N1 (10-31): golden drift after 14542 🟢 FIXED f64cb731
• re-recorded after 14542 (fd749696) and 13652 (f64cb731)
• golden ✓ ×4 on the merge; only gfx942 entries change (ran)
Ours 🟡 N2 (10-31): PR number in a comment 🟢 FIXED d68b6135
• none left; rule_scan 0 (ran)
yraparti (10-31) ×13: C++ band twin ×3; checks re-run; routing table; §H.2; §K.3; §Q.1 YAML; §E.2, §E.8, §Q.7, §S.7 🟡 DEFERRED (C++ twin)
• twin → AICK-2655, gap recorded; binding refuses, 9 ✓ (ran); fair, but see N2
• fixed: §E.8 d68b6135, §Q.7 83fd1909, §S.7 c5295f46; §K.3 refusal tests f376147d; §Q.1 YAML does exist; rest answered (read)
• §G.33 rerun: gfx950, gfx1250 unchanged; 6 gfx942 dense decode requests develop served now refused; body says none (ran)
Bots / CI 🟡 Running
• running: Windows gfx110X hipdnn-integration-tests re-run; first try: a slow runner hit hipDNN's own suite time limits, no rocke code (read: logs)
• gfx94X hipkernelprovider ×2 ✓; 42 ✓ · 1 running · 0 ✗
• merge: clean; auto-merge re-armed, stored title/message = live (read)
Our checks 🟢 Clean
• merge vs dc308362 clean; build, ctest 34 ✓ / 1 skip; byte identity ×4 (ran)
• golden ✓ ×4, gfx942 band + three 4-warp entries only; differential 0 drift (ran)
• tests 952 ✓ / 33 skip; compile clean; 13205's KDB/VPF refuses the band; diag split builds (ran)
Code quality 🟠 Should-fix
• gfx950 band code copied into gfx942 bodies; band fields in two homes (N3, N4)
• gap refusals, band CPU checks run in no lane (N2)
• new test re-implements shared helpers, band oracle (N5)
# Sev file:line Finding
N1 🟠 docs/gpu-attention-test-coverage.md:25, PR body (Testing Summary) Product name in place of the device id
• impact: breaks platform/AGENTS.md Compliance (device identifiers only); auto-merge's stored message carries the body's
• fix: drop both parentheticals; re-arm auto-merge after editing the body
N2 🟠 platform/tests/instances/test_attention_tiled_binding_band_rejects.py:58, library/tests/test_attention_unified_mask_gfx942_numeric.py:35 The gap's refusals and the band CPU tests run in no CI lane
• impact: CI skips all 9 binding cases (no pybind) and the mask module's CPU classes (no torch); test_attention_dense_gfx942.py is in no list (read: CI logs)
• fix: a ctest for the gfx950 entry points; move TestKernelIdentity's cases to test_unified_mask_wiring.py; list both dense files
N3 🟠 kernels/gfx942/attention_tiled_2d.py:1670, gfx942/attention_tiled_3d.py:524, gfx942/attention_dense.py:519 gfx950's band code copied into each gfx942 body and spec
• impact: band closures, right_bound checks and rbu/rb<R> name parts now have 2–4 copies; every fix must find them all (read)
• fix: one kernels/common/_attention_band.py, shared like _lse_store.py; dense band fields onto AttentionDenseSpec
N4 🟠 builders/common/attention_spec_builder.py:77, dispatch/attention/gfx942_dense.py:26 Band-to-spec fields in two homes; band arches in three
• impact: new _mask_spec_fields is _mask_fields minus its arch gate; gfx942 dense imports gfx950's private _dense_band/_is_ragged (read)
• fix: one band_spec_fields() and one arch constant beside _reject_mask_on_arch; dense helpers into dispatch/attention/common.py
N5 🟠 library/tests/test_attention_band_gfx942_numpy_numeric.py:86 New torch-free test re-implements shared helpers and the band oracle
• impact: copies DeviceArray, the bf16 codec and sdpa_reference's band math; the shared oracle matches on all 26 sequences (ran)
• fix: import them, as test_attention_tiled_3d_running_max.py does
N6 🟡 include/rocke/helper_helper_rocke.instances.gfx942.attention_tiled_2d.h:187, instance_gfx942_attention_tiled_3d.h:180, instance_gfx950_attention_tiled_3d.h:212 Spec struct layouts change without an ABI bump
• impact: causal_top_left moves emit_lse in the 2D spec and grows the 3D spec; abi.h says bump
• fix: bump ROCKE_ABI_VERSION and abi.py BINARY_ABI: develop is 6; takes develop + 1 when it lands

Utility: 🟢 Positive — gfx942 gains gfx950's attention band and three 4-warp fixes; band code duplicated.


Legend: 🔴 blocking · 🟠 should-fix · 🟡 minor nit · 🟢 info/good · ⚪ retracted

Follow-up review: first pass by a local qwen3.6 model, verified by the orchestrator (Claude Opus 5.5). Checks: merge with develop dc308362 (build, ctest, byte identity, parity, goldens, differential, tests, compile), golden key diff, routing differential vs develop, band × pipeline gates, binding rejections, CI logs at head, code quality. Flags whether a closer look is needed; not an approval or rejection.

@dlangbe dlangbe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: test_attention_dense_gfx942_golden.py:17 still says "varlen and ragged are rejected on gfx942, so unlike the gfx950 sibling they are absent here". After this PR only varlen is rejected; ragged (self and cross-length) is supported on both grids. Either update the sentence, or add a ragged case to this golden like the gfx950 sibling has, since the ragged body is new IR that only the representative golden pins today.

AnaghaRaoAMD and others added 12 commits November 3, 2026 13:11
# Conflicts:
#	dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py
#	dnn-providers/hip-kernel-provider/rocke/platform/dsl_docs/development/known_gaps.md
The docstring still said ragged is rejected on gfx942; only varlen is.
Add a self-length causal ragged case and a cross-length full-attention
ragged case (as the gfx950 representative golden does) and record their
hashes at every LLVM flavor. Existing case hashes are unchanged.

Co-Authored-By: Claude <noreply@anthropic.com>
rocke_attention_tiled_2d_spec_t gained causal_top_left ahead of emit_lse,
moving it, and rocke_unified_attention_3d_tiled_spec_t grew a trailing
causal_top_left. A C caller built against the old header would read both
structs at the wrong offsets, so bump ROCKE_ABI_VERSION and its Python
twin BINARY_ABI from 6 to 7.

Co-Authored-By: Claude <noreply@anthropic.com>
…rnel

A routing differential over layout='dense' requests found gfx942
decode requests that develop dispatched and this branch refused.

- NO_MASK + sliding_window with diagonal_alignment=BOTTOM_RIGHT and no
  right bound, at seqlen_q == 1: develop served the bottom-right causal
  window, which is exactly this band (one query row sits on the
  diagonal at its last key, so nothing lies to its right). The branch
  read it as a non-causal band, which strided decode cannot serve.
  normalize_band now folds an unbounded right side at bottom-right
  alignment and seqlen_q == 1 to right_bound 0, as it already does for
  full attention. gfx942 gets develop's kernel back; gfx950 picks the
  plain window kernel instead of the _rbu one for these requests.
- Strided decode bakes only the bottom-right causal band, but it
  refused only mask_type TOP_LEFT_CAUSAL. NO_MASK / SLIDING_WINDOW
  with right_bound=0 at top-left alignment (key 0 only, at
  seqlen_q == 1) reached it and ran bottom-right. It now refuses any
  problem with causal_top_left or a non-zero right bound. This stops
  gfx942 picking up the route and corrects gfx950, which served it on
  develop.

Top-left NO_MASK + window decode stays refused, as on gfx950: develop
gfx942 ran the legacy causal window (the last W keys), but the request
is a top-left non-causal band.

Co-Authored-By: Claude <noreply@anthropic.com>
- Move the gfx942 kernel-identity checks (spec names, cache keys,
  support gates) out of the torch-gated unified-mask numeric module into
  test_unified_mask_wiring.py, which the attention dispatch lane runs
  without torch.
- Add the rocke_attention_tiled_band_rejects CTest: the gfx950 tiled 2D,
  fastKV register-P and tiled 3D C entry points refuse causal_top_left
  and still lower the default spec. The pybind twin of this test skips
  in CI because no lane builds the extension. Categorize it in the
  quick and ffm-quick tiers.
- Enroll the gfx942 dense CPU suite and its IR golden in
  rocke_library_attention_cpu_pytest.

Co-Authored-By: Claude <noreply@anthropic.com>
Per the platform/AGENTS.md compliance rule, refer to hardware targets by
device identifier only. Drop the product names from the attention test
coverage inventory and the gfx942 dense numeric lane's docstrings and
skip reason.

Co-Authored-By: Claude <noreply@anthropic.com>
The gfx942 band port copied gfx950's band code into each gfx942 body
and spec. kernels/common/_attention_band.py now holds the one copy,
shared like _lse_store.py:

- check_right_bound / tiled_band_name_parts: the tiled 2D/3D spec
  checks and the tl / rbu / rb<R> name parts (four specs).
  right_bound_tag also names the scalar kernel, and check_right_bound
  validates the unified problem.
- diagonal_offset and BandMask (upper_lim, upper_ok, row_mask,
  max_prefix_len, last_key, valid_tail): the band closures and blocks of
  the tiled 2D/3D bodies, plus the 4-warp bodies' diagonal offset.
- check_dense_right_bound / dense_band_name_parts / upper_lim: the dense
  spec checks, the rb<R> name part and the mask's upper bound on both
  arches.

Each helper emits its ops at the call site, in the order the inlined
code did, so no IR changes. The attention golden shards regenerate with
zero diff, and the byte-identity gate (--only attention) is green at
llvm20/22/23/24. Kernel names and LLVM-IR sha256 match for 919 sampled
tiled 2D, 4-warp, 3D and dense kernels.

The dense right_bound field stays declared on each arch's spec.
attention_dense_cache_key, and with it KernelId.spec_hash, walks the
fields in declaration order. A base-class field reorders them, which
would change every gfx942 and gfx950 dense spec_hash.

Co-Authored-By: Claude <noreply@anthropic.com>
…dense band reading

- band_spec_fields(problem, arch) in kernels/common/attention_unified.py
  replaces dispatch's _mask_fields and the builders' _mask_spec_fields.
  The builders' copy was the same function minus the arch gate. The
  gate stays and raises for a non-default mask off the band arches,
  which the tiled gates (_reject_mask_on_arch) already refuse before a
  builder runs.
- _BAND_KERNEL_ARCHES sits beside _reject_mask_on_arch. That gate,
  band_spec_fields and dispatch's _has_no_mask_kernel all read it,
  replacing three literal ("gfx950", "gfx942") lists.
- dense_band / is_ragged move from gfx950_dense's private helpers into
  dispatch/attention/dense_rules.py. Both dense candidates import them
  from there, so gfx942_dense no longer reaches into gfx950_dense.
- gfx942_dense passes right_bound unconditionally. The spec declares it
  with default 0, so its identity is the same.

Behaviour is unchanged. The routing differential (18144 requests, auto
and layout='dense', gfx942/gfx950/gfx1250) is byte-identical before and
after, spec_hash included. Kernel names and IR sha256 of 919 sampled
attention kernels are unchanged, and test_library_layering passes.

Co-Authored-By: Claude <noreply@anthropic.com>
Unpinned contiguous-layout chunked prefill at non-tile-multiple lengths
(e.g. BOTTOM_RIGHT_CAUSAL Sq=200 Sk=1000) is auto-pinned to the gfx942
dense kernel on its ragged bottom-right body, and nothing tested it.

- Add 200x1000 and 197x400 rows, batch 2, both grids, to the dense
  bottom-right numeric cohort (bf16/fp16 D128, bf16 D64) and to the
  torch-free band suite. With batch 2, batch 0's padded key tail would
  read batch 1's K/V if the kernel did not mask it.
- Assert in the dense wiring tests that such a request is the default
  dense route and its spec is ragged and bottom-right.
- Update the coverage inventory counts.

Co-Authored-By: Claude <noreply@anthropic.com>
The suite carried its own device-array wrapper, bf16 codec and band
reference. Use the shared DeviceArray (_attention_device_array), the
rocke.numeric.bf16 codec and the float64 rocke.numeric.sdpa_reference
oracle (ragged THD layout for the paged cases, dense for the dense
ones) instead. The file stays torch-free.

Co-Authored-By: Claude <noreply@anthropic.com>
- Import dense_band from dispatch.attention.dense_rules, its new home.
- normalize_band reads NO_MASK + window at bottom-right alignment and
  seqlen_q == 1 as the bottom-right causal window (dense keeps the band),
  and strided decode serves it on gfx942 and gfx950, including the
  unpinned dense-layout route.
- Strided decode refuses NO_MASK / SLIDING_WINDOW + right_bound=0 at
  top-left alignment on both arches, with its reason.

Co-Authored-By: Claude <noreply@anthropic.com>
Cover kernels/common/_attention_band.py on the CPU: the kernel-name parts
(tl, rbu, rb<R>), the tiled and dense right_bound refusals and their
messages, and that the causal band (right_bound 0) emits no op beyond
the causal IR while a lookahead adds one constant and one add per limit.
Enroll it in rocke_library_attention_cpu_pytest.

Co-Authored-By: Claude <noreply@anthropic.com>
@AnaghaRaoAMD
AnaghaRaoAMD requested a review from a team as a code owner November 3, 2026 19:19
@AnaghaRaoAMD

Copy link
Copy Markdown
Contributor Author

@dlangbe on the test_attention_dense_gfx942_golden.py nit: fixed in 9a09c82. The sentence now says only varlen is rejected. The golden now pins the ragged body, following the gfx950 sibling: ragged_causal_sq500 (self-attention) and ragged_full_cross_197x400 (cross-length), at llvm20 / 22 / 23 / 24. No existing hash changed.

@AnaghaRaoAMD

Copy link
Copy Markdown
Contributor Author

@cgmillette thanks. Everything is addressed at b16fffa, on top of a fresh develop merge (108bddc):

  • N1, product names: 3e3f1f7 replaces them with device identifiers in the coverage map and the dense numeric test. The PR body is fixed too. Product names left in other files predate this PR.

  • N2, checks that run in no CI lane: 479b297.

    • TestKernelIdentity moved into test_unified_mask_wiring.py as TestGfx942KernelIdentity, which runs without torch.
    • A new C++ CTest, rocke_attention_tiled_band_rejects, calls the gfx950 tiled 2D, fastKV and 3D C entry points directly. It asserts the exact causal_top_left rejection and that the default spec still lowers. It runs with no pybind and is in the quick tiers of the category YAML.
    • test_attention_dense_gfx942.py and its golden are now in rocke_library_attention_cpu_pytest.
  • N3, duplicated band code: 164194b adds one shared kernels/common/_attention_band.py, used by the gfx942 and gfx950 tiled 2D / 3D bodies (4-warp included), the tiled and dense specs, and the scalar kernel. There is zero IR change:

    • the attention golden shards regenerate with no diff;
    • byte identity is green at llvm20 / 22 / 23 / 24;
    • name plus IR sha256 match for 919 sampled kernels.

    The dense right_bound field stays on each arch's spec, because attention_dense_cache_key is field-order sensitive and moving it would change 1,696 dense spec hashes, gfx950's included. The checks and name parts are shared.

  • N4, two homes for band fields and arches: 6a3e4d2. There is one band_spec_fields(problem, arch) and one _BAND_KERNEL_ARCHES, which sits next to _reject_mask_on_arch and is read by _has_no_mask_kernel. dense_band / is_ragged moved to dispatch/attention/dense_rules.py. Routing is identical before and after.

  • N5, re-implemented helpers: d04d88a. The torch-free suite now uses the shared DeviceArray, the rocke.numeric.bf16 codec and sdpa_reference. On gfx942 it passes 34/34 with import torch blocked. A unit test for the shared band helpers is in b16fffa.

  • N6, ABI: bc02342 moves ROCKE_ABI_VERSION / BINARY_ABI from 6 to 7, and test_abi.py passes.

G.33, your routing note was right. My table missed layout='dense' requests. The extended differential (18,144 rows) found:

  • Fixed (0809358): 24 gfx942 requests develop served correctly: bottom-right NO_MASK + window, Sq = 1. They get develop's kernel back.
  • Refused on purpose: 72 gfx942 top-left NO_MASK + window decode requests, which develop ran with the legacy bottom-right window. That is wrong whenever Sk > W. Your 6 are in this class.
  • Refused, changing gfx950 behaviour: 192 requests. gfx950 strided decode dropped causal_top_left, so top-left NO_MASK / SLIDING_WINDOW with right_bound = 0 ran bottom-right. That bug already exists on develop. Strided decode now refuses those problems on both arches, and 902c9b6 pins this.

The PR body is corrected to match.

@AnaghaRaoAMD
AnaghaRaoAMD requested a review from dlangbe November 3, 2026 19:36
)
if sq == sk and (uniform_lengths or not _has_no_mask_kernel(req.arch)):
align = Alignment.BOTTOM_RIGHT
if (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the fold should cover any right side, not just an unbounded one. At seqlen_q == 1 a bottom-right row sits at its last key, so a lookahead (right_bound = R > 0) is also exactly causal: there is no key past the diagonal. As written, R > 0 is left alone, and the new strided decode check (strided_decode.py:119, problem.right_bound != 0) refuses it; strided _problem gets that R through _attention_band -> band.kernel_right_bound.

On develop, gfx950 admits R > 0 bands, and strided decode had no band check, so e.g. arch="gfx950", layout="dense", seqlen_q=1, mask_type=NO_MASK, right_bound=16, diagonal_alignment=1 (or SLIDING_WINDOW with a window and right_bound=16) ran the causal strided body, which is the correct answer for one query row. Dense-layout decode has no other candidate, so that request is now refused. It's the same class as the 24 gfx942 rows 0809358 restored, and it isn't in the PR body's differential. 902c9b6 pins the unbounded and top-left cases but not this one.

Fold right_bound is None or right_bound > 0 to 0 here, and add the R > 0 case next to test_strided_decode_serves_the_bottom_right_decode_window. Separately, the refusal reason "strided decode uses bottom-right causal alignment" (L119) reads oddly for a bottom-right request it refuses; something like "strided decode computes only the causal bottom-right band" would cover both branches.

* 7: rocke_attention_tiled_2d_spec_t gained causal_top_left before emit_lse,
* which moves emit_lse; rocke_unified_attention_3d_tiled_spec_t (gfx942,
* gfx950) gained causal_top_left at its end, which changes its size. */
#define ROCKE_ABI_VERSION 7

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

develop took ABI 7 for a different change: #14328 (merged 11-03) bumped 6 -> 7 for rocke_mma_op_t.dst_dtype and the mma catalog signatures. This PR's bc02342 also bumps 6 -> 7, for the attention spec layouts. abi.h conflicts (it's the only conflict with develop right now), but platform/python/rocke/portable_ir/src/abi.py merges silently, because both sides wrote BINARY_ABI = 7. Resolving abi.h by keeping either side leaves two different layouts under one ABI number. When merging develop, take 8 in both files and add the band layout change as entry 8 after develop's entry 7.

@cgmillette

Copy link
Copy Markdown
Contributor

Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max

PR #13211 — attention band coverage on gfx942 (AnaghaRaoAMD, AICK-2520) — 🟠 Follow-up: rebase over develop's attention changes

Prior item Status
dlangbe (10-20) ×5: ragged Q/O voffset; t3d_build drops right_bound; gfx950 reject text; do_mask comment; band suites missing from CTest 🟢 FIXED 144200e8, e857e057
• all 5 intact at head (read)
dlangbe (11-03) ×2: dense golden sentence; ragged bottom-right uncovered 🟢 FIXED 9a09c824, cd74726b
• golden pins both ragged bodies; a ragged key-bound mutant fails it (ran)
• ragged bottom-right rows pass in both gfx94X shards (read: logs)
dlangbe (11-03) common.py:764: fold any right side at Sq = 1 🟡 OPEN
• fold is exact: a bottom-right lookahead at Sq = 1 is causal (ran)
• the example is refused on develop too (band feature): an extension, not a regression (ran)
• the same fold serves the 72 refused top-left decodes exactly: full attention at Sq = 1 (ran)
Ours N6, dlangbe (11-03) abi.h:96: spec layouts need an ABI bump 🟡 OPEN bc02342d
• bumps 6 → 7, but develop is 9 (read)
• takes develop + 1 when it lands: renumber abi.h and abi.py at the rebase, history line after develop's 9
yraparti (10-31) ×13: C++ band twin ×3; checks re-run; routing table; §H.2; §K.3; §Q.1 YAML; §E.2, §E.8, §Q.7, §S.7 🟡 DEFERRED (C++ twin)
• gfx950 top-left refusal now has a CI CTest; refusal mutants 3/3 caught (ran)
• §G.33 classes reproduced; develop's strided spec drops top-left, so its gfx950 answers were wrong, as the body says (ran)
• right_bound refusal still tested only through the binding CI skips (read)
CongMa13 (10-21) APPROVED at 412a42fc 🟢 APPROVED 412a42fc
• older head; still counts on develop
• predates the 10-28 rework, the develop merges and 164194b7's gfx950 band refactor
Ours (10-21…10-31) ×6: conflicts, golden drift ×2, band numerics in CI, PR number 🟢 FIXED
• fixed at earlier heads; conflicts and golden drift recur on today's develop (N1)
Ours N1 (11-03): product name in place of the device id 🟢 FIXED 3e3f1f75
• none in the added lines or the body (ran)
Ours N2 (11-03): band tests in no CI lane 🟢 FIXED 479b2972
• new CTest and moved tests pass in both gfx94X shards (read: logs)
• installed-layout mutants caught: Python 4/4, C++ refusals 3/3 (ran)
• no Windows shard ran at head
Ours N3 (11-03): gfx950 band code copied into gfx942 🟢 FIXED 164194b7
• one _attention_band.py; IR identical before and after on 667 kernels (ran)
• byte identity GREEN ×4, golden ✓ ×4 at head (ran)
• leftover: ragged key-pad rule ×3 (Code quality)
Ours N4 (11-03): band fields in two homes, arches in three 🟢 FIXED 6a3e4d29
• one band_spec_fields, one _BAND_KERNEL_ARCHES; dense helpers in dense_rules.py (read)
Ours N5 (11-03): torch-free test re-implements helpers 🟢 FIXED d04d88a4
• imports DeviceArray, the bf16 codec and sdpa_reference; 34 ✓ in CI (read: logs)
• codec wrappers remain (Code quality)
Bots / CI 🟡 Infra failure
• fails: Windows compiler-runtime stage — external checkout timed out, both attempts; summary red (read: logs)
• gfx94X hipkernelprovider ×2 ✓; no Windows rocKE shard ran
• 36 ✓ · 2 ✗ · 53 skipped; merge: dirty, so no new CI until rebased
Our checks 🟠 Conflicts
• merge vs 60b43a5a52: 12 conflicts; resolved trial merge: golden FAIL ×4 (ran)
• head: byte identity GREEN ×4; golden ✓ ×4; IR sweep 0 diffs (ran)
• tests 991 ✓ / 46 skip; routing differential; hipDNN admission table unchanged (ran)
Code quality 🟡 Minor
• one band module; per-arch dense right_bound is sound (read, ran)
• new lanes run in CI; the CTest misses a status-code mutant (N3) (ran)
• leftovers: ragged key-pad rule ×3, double strided check, codec copy ×4 (read)
# Sev file:line Finding
N1 🟠 kernels/gfx942/attention_tiled_2d.py:6421, 11 more files 12 conflicts with develop; the PR side drops develop's work
• impact: loses 14929's 4-warp Q and page clamps, 15024's head-count check, 14259's parity cases 23–32, ABI 8–9; a resolved trial merge fails the golden ×4 (ran)
• fix: take 14929's clamps; keep both fastKV checks as whole blocks; PR case after 32; recompute LSE pins; _sw name tags; re-record on the merge
N2 🟠 kernels/gfx942/attention_tiled_2d.py:6796 On the merge, the 4-warp empty-row guard bakes the window size
• impact: 15024 caches one kernel for every window; windows 64 and 128 lower different IR under one key, so top-left rows past kv_len get the first window's zeroing (ran: trial merge)
• fix: build _lo from the runtime window_p
N3 🟡 platform/cpp/instances/gfx950/attention_tiled_2d_fastkv_regp_helpers.cpp:312 C++ top-left refusal: three sites, two texts, two codes; the CTest checks only texts
• impact: a NOTIMPL → VALUE mutant passes the CTest (ran); known_gaps.md credits the binding test CI skips
• fix: one message and NOTIMPL in a shared gfx950 header; assert the status; name the CTest there

Utility: 🟢 Positive — band refactor verified IR-neutral; rebase over develop's attention changes remains


Legend: 🔴 blocking · 🟠 should-fix · 🟡 minor nit · 🟢 info/good · ⚪ retracted

Follow-up review: first pass by a local qwen3.6 model, verified by the orchestrator (Claude Opus 5.5). Checks: merge with develop 60b43a5a52 (12 conflicts; resolved trial merge with golden gate), byte identity, IR sweep across the refactor, goldens, routing differential vs develop, installed-layout mutants (Python, C++), hipDNN admission table, CI logs at head, code quality. Flags whether a closer look is needed; not an approval or rejection.

@AnaghaRaoAMD
AnaghaRaoAMD marked this pull request as draft November 6, 2026 17:39
@AnaghaRaoAMD

Copy link
Copy Markdown
Contributor Author

will resume post initial gfx942 AOT packages are in

This branch has not been deployed

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants