[reble]

JIRA ID : AICK-2839

Motivation

On gfx1250 the decode attention softmax (attention_tiled_3d, DPP softmax on by default) emits each row-reduce step through IRBuilder.vop2_f32_dpp_xor as inline asm (v_max_f32_dpp / v_add_f32_dpp ... row_xmask:N). LLVM does not inspect inline asm for the gfx1250 WMMA co-execution hazard (GCNHazardRecognizer::checkWMMACoexecutionHazards), so an asm DPP write to a WMMA source register gets no wait states. As real instructions the DPP ops are covered by the hazard recognizer.

Technical Details

• core/ir.py::vop2_f32_dpp_xor now emits warp_shuffle_xor_dpp (llvm.amdgcn.update.dpp, row_xmask:N) followed by fmax / fadd. Signature and callers (helpers/attention.py::wave_reduce_max/sum) are unchanged.
• With upstream LLVM, GCNDPPCombine folds the v_mov_b32_dpp back into v_add_f32_dpp / v_max_num_f32_dpp. The docstring claimed this fold does not happen on gfx1250; that is corrected.
• Python only: the C++ engine has no counterpart of this asm.
• The forced wmma_spacing=1 for use_dpp_softmax is left in place; removing it needs a hardware run.

Test Plan

• Golden IR parity (rocke_ir_parity_harness.py --check) on llvm20, llvm22 and llvm23
• platform/tests/test_rocke.py and the library attention tests
• Emitted ISA of the decode_3d_verify seg kernel for 13 configurations (bf16/fp8 KV, kv 512-4096, --invariant-hoist, --num-waves 4, --dtla, --sw-pipeline, --plain, --ds-tr, --register-p, --wide-kv-load), compiled with upstream LLVM (d55a3fd2, -O3): fused DPP count and WMMA co-execution distances against LLVM's wait-state table

Test Result

• Golden IR: parity OK on all three flavors (no golden pins a gfx1250 decode_3d kernel).
• test_rocke.py: 316 passed, 124 skipped. Attention tests: 138 passed, 51 skipped. Both unchanged by the patch (compared at the earlier base 7accefb).
• ISA: all 128 DPP reduce steps stay fused (112 with --wide-kv-load, where 16 adds run under a different EXEC mask than their DPP mov, which GCNDPPCombine does not fold across); hazard violations 4 -> 0 in fp8 kv 512/4096 and 0 everywhere else.

config	insts (before -> after)	fused DPP (before -> after)	WMMA hazard violations (before -> after)
bf16, kv 1024	1522 -> 1504	128 -> 128	0 -> 0
bf16, --num-waves 4	2842 -> 2836	128 -> 128	0 -> 0
fp8, kv 4096	1757 -> 1761	128 -> 128	4 -> 0
fp8, --plain --wide-kv-load	2185 -> 2225	128 -> 112	0 -> 0

Compilers that quiet the shuffled value for llvm.maxnum (a separate v_max_num_f32 v, v, v) leave the max steps unfused until they pick up upstream. Not run: GPU, tools/check_byte_identity.py, pre-commit.

Submission Checklist

• Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests.

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com

[therock-pr-bot]

✅ 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]

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

[CongMa13]

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

• PR description needs an issue reference — therock-pr-bot[bot] — #14613 (comment) (repeated at #14613 (comment))
• Changed source needs an accompanying unit test — therock-pr-bot[bot] — #14613 (comment)

[CongMa13]

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

[cgmillette]

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

PR #14613 — emit the gfx1250 DPP softmax reduce with update.dpp, not inline asm (reble, AICK-2839) — 🟡 First-round: no closer look needed — exact rewrite; the WMMA hazard check now sees the DPP writes; ROCm clang leaves the max steps unfused

Prior item	Status
Author (body, 10-28): not run: GPU, check_byte_identity.py, pre-commit	🟢 Mostly — byte identity GREEN here (ran) and pre-commit passes in CI. GPU is still not run anywhere: there is no gfx1250 device here or in PR CI
therock-pr-bot (10-28) ×2, echoed by CongMa13 (10-29 01:21Z): the body has no issue reference; the changed source has no unit test	🟢 FIXED 715a807a — the body cites AICK-2839, and 715a807a adds test_vop2_f32_dpp_xor_is_update_dpp_then_fmax_or_fadd plus an argument-check test. The bot re-checked and passed (10-29 02:32Z). Both tests pass on all four hipkernelprovider shards (read: CI logs) and here (ran)
CongMa13 (10-29 01:31Z) APPROVED at 3af7245c	🟢 Stands — the only later commit, 715a807a, adds tests and changes no code (read)
Bots / CI	🟢 • Policy bot pass. At head 715a807a, the TheRock Multi-Arch CI run started by the bot's label change (02:32Z) passes, summary included: the Linux gfx94X and Windows gfx110X hipkernelprovider shards 1/2 and 2/2, plus the hipDNN samples and integration tests. The hipDNN superbuild (Linux, Windows), pre-commit and Component CI pass too. The gfx950 and gfx125X test matrices were configured, but their test jobs were skipped, so nothing ran on gfx1250 hardware. • The one red check, Multi-Arch CI Summary, belongs to the first Multi-Arch run at the same head (02:24Z). The second run cancelled it about 9 minutes in, and its summary failed only because its Linux and Windows build jobs were cancelled. That run and a superseded PR-bot run hold all 7 cancelled checks; none of them is this PR's. • In the shard logs, rocke_pytest passes with both new tests, but the first one's 8 cross-engine subtests skip (no rocke_engine in CI). test_gfx1250_attention.py lowers the gfx1250 3D seg kernel and compiles a bf16 DTLA seg kernel (DPP softmax on) through COMGR, and both pass; its 4 gfx1250 GPU cases skip (no device). The differential-golden CTests pass (read: CI logs)
Our checks	🟢 • Merge: clean with develop 2304e8d0, still develop's head (55 behind, 2 ahead). Native build OK, ctest 33/33 (1 skip). Byte identity GREEN at llvm22 for every family, but no family covers this kernel: the C++ engine has no gfx1250 instances and no counterpart of the old asm, and it lowers tile.dpp_xor with the same control word (lower_llvm/crosslane.cpp:417) (read). Representative golden PASS ×3. Differential golden OK at llvm20/22/23/24 and canonical (0 changed, 0 missing). Its 180 NEW (unblessed) configs (115 at llvm20/22) are develop's: #14542 and #14626 report the same counts on the same develop (read). No golden pins a gfx1250 decode_3d kernel, so none of these gates sees this change (ran: grep). test_rocke.py: 358 passed, 14 skipped, with the native engine built, so the new test's cross-engine half ran here. Rules 1–3 clean (0/0/0); no links or tracker IDs in the added lines or the commit messages (ran). • Semantics (ran: IR; read): each step now lowers to update.dpp.i32(src, src, 0x160 | mask, 15, 15, true), a bitcast, then maxnum or fadd with the shuffled value first, which is the old asm's encoding. With row_xmask 1..15 every source lane is in its row, and with full row/bank masks and bound_ctrl the old value never shows; an inactive source lane reads 0 in both forms. update.dpp is convergent like the asm, so the shuffle stays where it was. Only the plain fadd can sink under a narrower EXEC (--wide-kv-load), and there it still adds the value shuffled under the wider mask, so no lane value changes. llvm.maxnum and the old v_max_num_f32_dpp both return the non-NaN operand, and the added quiet only turns sNaN into qNaN. So NaN, -inf and fully masked rows (the -1e30 fill) reduce exactly as before. The step's IR is identical in llvm20/22/23/24. • ISA (ran: ROCm clang 24, -O3; merge vs develop): fp8 KV seg kernel (the decode_3d_verify defaults): on develop, an asm v_add_f32_dpp overwrites a B-operand VGPR of the WMMA issued right before it, with no gap. On the merge, no asm DPP is left, and LLVM puts a v_nop before a DPP write into the B operand of a WMMA issued a few instructions earlier. The 64 add steps fuse; the 64 max steps do not (N1). bf16 KV with 4 waves: the same split, and a linear scan finds no unguarded DPP write on either tree. In both kernels the forced wmma_spacing=1 v_nops stay, nothing spills, and occupancy is unchanged. • Coverage (read: CI logs): CI would catch a lowering or compile break of this kernel (test_gfx1250_attention.py) and a change to the step's IR (the new test pins the Python lowering: no asm, one update.dpp with 0x160 | mask, masks 15/15 and bound_ctrl, and the shuffled value as the first operand). Nothing in CI would catch wrong gfx1250 numerics or a hazard in the kernel's ISA: no gfx1250 test job runs and no golden pins the kernel

#	Sev	file:line	Finding
N1	🟡	core/ir.py:4419, gfx1250/attention_tiled_3d.py:136	With ROCm clang 24 (the only gfx1250 compiler here; the shipped path compiles through ROCm's COMGR), each max step compiles to v_mov_b32_dpp + a quieting v_max_num_f32 v, v, v + the max. Only the 64 add steps fuse (develop's asm: all 128), and the static instruction count rises ×1.04 (fp8 KV) and ×1.02 (bf16, 4 waves) (ran). Fusion depends on the compiler, not the IR flavour. The body's ISA table covers upstream LLVM only, and the spec comment still promises one fused VALU op per stage. The shipped decode keeps the hazard fix but gives back part of the DPP softmax's critical-path gain until ROCm picks up the upstream change; how much is unmeasured (no gfx1250 here) → time the low-wave decode on gfx1250 with a ROCm compiler and put the result in the body. If the loss shows, nnan on this one maxnum gets the single v_max_num_f32_dpp back on ROCm clang 24 (ran: probe), though a NaN score then becomes poison. Update the comment either way

Utility: 🟢 Positive — closes an unguarded WMMA operand overwrite in gfx1250 decode; exact rewrite.

---

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

First-round review: a local qwen3.6 model made the independent first pass, and the orchestrator (Claude Opus 5.5) verified it. On a clean merge with develop, it ran the native build and ctest, byte identity (every family at llvm22), the representative and differential goldens, and the PR's test file. It checked the new lowering against the old asm, lowered two gfx1250 decode_3d seg kernels on the merge and on develop, compared their ROCm clang 24 ISA, probed the max-step fusion, and read the four hipkernelprovider CI logs at head. The orchestrator owns this merge-worthiness call. It flags whether a closer look is needed; it is not an approval or a rejection.

[cgmillette]

lgtm

[assistant-librarian]

TheRock Submodule Bump Activity

Newest first

• 2026-10-30 — Included in TheRock via ROCm/TheRock#9626.