Repository navigation
feat(hip-kernel-provider): port the rocKE gfx950 tiled-2D V double-buffer, D256 fast path and softmax interleave to C++ - #14525
Conversation
CongMa13
left a comment
There was a problem hiding this comment.
One actionable correctness issue found in an accepted schedule combination; the inline comment identifies the trigger and failure mechanism.
| ROCKE_ERR_NOTIMPL, | ||
| "use_staggered_iter_wait: gfx950 C twin emits only the depth-2 " | ||
| "single/early-V schedule (not yet ported)"); | ||
| if(!spec->use_v_double_buffer) |
There was a problem hiding this comment.
This accepts use_staggered_iter_wait with use_early_v_schedule. On a multi-tile input, the early-V branch issues next-K after QK, while the staggered iter-start wait assumes K was issued before V; the wait can leave current K incomplete when QK reads it. Please reject this combination here and keep the Python spec validation aligned.
|
Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max PR #14525 — port the rocKE gfx950 tiled-2D V double-buffer, D256 fast path and softmax interleave to C++ (amd-khushbu, AICK-2712) — 🟠 First-round: not merge-ready — rebase needed — conflicts inherited from #14523/#14524; CongMa13's early-V + staggered-wait race is real in both engines
Utility: 🟢 Positive — brings three gfx950 prefill schedules to C++ parity; inherits two validator gaps. 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. The PR conflicts with develop, so there is no merged tree yet; the build, byte identity, goldens and test runs wait for the rebase. The orchestrator simulated the merge with |
…ffer, D256 fast path and softmax interleave to C++ JIRA ID : AICK-2712 ## Summary This is PR 4 of a stack of 7 that brings the rocKE C++ unified-attention engine to parity with Python (see the stack below). It sits on PR 3. Python's gfx950 production prefill specs use three things the C++ builder rejected or left unset: - the V double-buffer and staggered-wait schedules, for the single-batch D64 combo cohort; - the D256 fast path, for bf16 D256 prefill; - the softmax-MFMA interleave, for the single-batch D128 combo cohort. C++ now builds all three the way Python does. The tiled-2D gate no longer rejects these cohorts, and the gfx950 spec built from a problem matches Python's on every field. ## Changes All paths are under `dnn-providers/hip-kernel-provider/rocke/platform/cpp` unless noted. - **Softmax-MFMA interleave.** The C++ spec builder sets `use_softmax_mfma_interleave` (mode 1) for the single-batch D128 combo cohort, with the same predicate that widens it to 4 warps. Before, C++ built the widened kernel without the `iglp_opt` hint. - **V double-buffer and staggered wait.** - `V_lds` gets two slots. V[tile_start] is prefetched before the loop, and each iteration prefetches V[i+1] and K[i+1] into the other slot (K first when staggered). - The staggered iteration-start wait drains only K[i]. - The builder mirrors Python's `__post_init__` checks for both flags instead of rejecting them. The deep ring and Q-reread stay rejected (`known_gaps.md`). - **D256 fast path.** - The spec struct gains `use_kq_lds_pad`, `kq_lds_pad_halves` and `use_mask_phase_split`. They are defaulted, and gfx942 ignores them. - The spec from a problem applies Python's D256 overrides last, as Python does. - The builder ports the direct-to-register Q gather, the K-slab LDS pad, interleave mode 2 (alternating MFMA/VALU `sched_group_barrier` groups) and the mask phase split (a mask-free full-tile loop, then the masked boundary loop). - **Fix: fp8-in-LDS K on the transposed QK.** The transposed QK had its own K reader, which read fp8-in-LDS K as bf16. It now reads K through the shared reader, which applies the fp8 dequant as Python does. Config 69 pins this. With the old reader the gate is RED on configs 64, 65, 67, 68 and 69. - **Not ported:** the right-bound variant of the mask phase split. The C problem has no `right_bound` slot yet (`known_gaps.md`). Co-Authored-By: Claude <noreply@anthropic.com>
f9edddc to
208a0d0
Compare
30d8f00 to
8c8e442
Compare
|
Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max PR #14525 — port the rocKE gfx950 tiled-2D V double-buffer, D256 fast path and softmax interleave to C++ (amd-khushbu, AICK-2712) — 🟠 Follow-up: rebase fixed the conflicts; CongMa13's early-V race is still open
Net-new: none. Utility: 🟢 Positive — C++ now builds gfx950's production prefill specs byte-identically; untested on gfx950 hardware. Legend: 🔴 blocking · 🟠 should-fix · 🟡 minor nit · 🟢 info/good · ⚪ retracted Follow-up review: a local qwen3.6 model made the independent first pass, and the orchestrator (Claude Opus 5.5) verified it. The checks started with a range-diff of the rebased stack against the head we reviewed ( |
8c8e442
into
users/khuagarw/rocke/aick-2712-cpp-parity-1-stopgaps
|
This stack is now combined into one PR, #14522, so it can be reviewed and merged as one change. GitHub shows this PR as merged because its commit is now on #14522's branch; nothing has gone into develop. This PR's change is one commit there, unchanged. Open review comments from here are listed in #14522's description and will be answered there. |
JIRA ID : AICK-2712
Summary
This is PR 4 of a stack of 9 that brings the rocKE C++ unified-attention engine to parity with Python (see the stack below). It sits on PR 3.
Python's gfx950 production prefill specs use three things the C++ builder rejected or left unset:
C++ now builds all three the way Python does. The tiled-2D gate no longer rejects these cohorts, and the gfx950 spec built from a problem matches Python's on every field.
Changes
All paths are under
dnn-providers/hip-kernel-provider/rocke/platform/cppunless noted.use_softmax_mfma_interleave(mode 1) for the single-batch D128 combo cohort, with the same predicate that widens it to 4 warps. Before, C++ built the widened kernel without theiglp_opthint.V_ldsgets two slots. V[tile_start] is prefetched before the loop, and each iteration prefetches V[i+1] and K[i+1] into the other slot (K first when staggered).__post_init__checks for both flags instead of rejecting them. The deep ring and Q-reread stay rejected (known_gaps.md).use_kq_lds_pad,kq_lds_pad_halvesanduse_mask_phase_split. They are defaulted, and gfx942 ignores them.sched_group_barriergroups) and the mask phase split (a mask-free full-tile loop, then the masked boundary loop).right_boundslot yet (known_gaps.md).Stack
Each PR is based on the one before it. Review from 1 upward; each diff shows only its own slice.
Test plan
test_attention_selector_cpp_parity.py: the D256-override and softmax-interleave known-gap rows are removed, so the gfx950 spec has no known gaps lefttest_attention_binding_spec_fields.pyandtest_attention_routing_cpp_parity.pyupdated for the newly ported fields and cohortsgfx950_attention_tiled_2d59–69 blessed; no other attention entry moves🤖 Generated with Claude Code