Repository navigation
perf(rocke): add opt-in gfx942 BF16 D128 K LDS double buffering and V register prefetch - #13205
Merged
AviralGoelAMD merged 9 commits intoNov 2, 2026
Conversation
github-actions
Bot
added
documentation
project: hip-kernel-provider
rocke-lib: attention
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
assistant-librarian
Bot
added
the
external contribution
Contributor
|
@Parthiv911 Awesome, work! Let us use ignore the missing ticket id failure for now. |
aosewski
previously requested changes
Oct 7, 2026
aosewski
left a comment
aosewski
left a comment
Contributor
There was a problem hiding this comment.
Thanks for this. The K double-buffer plus V register prefetch is a clean design. Carrying V[j+1] in VGPRs across softmax/PV makes the overlap a data dependency instead of a scheduling hint, and the single [2, BN, LDROW] K allocation means the smem_pool packer can't fold the two slots onto each other. With the block_n-derived V pad, the bn64 footprint is 2×17,408 + 16,384 = 51,200 B, which fits. I'd like to see this land, but four things need to change first.
1. Rebase onto develop's kernel instead of overwriting it (blocking)
The merge base (8aa09e65b7e, 2026-10-06) is current, but the PR head drops develop features unrelated to KDB/VPF. It looks like an older copy of kernels/gfx942/attention_dense.py was used as the starting point. Reverted:
- #12284: runtime batch/seqlen (
runtime_shape,runtime_param_fields, the shape kernel params, runtimenum_records). Develop still readsspec.runtime_shapeinbuilders/gfx942/attention/prefill/attention_dense_prefill.py:359andbenchmarks/gfx942/attention/prefill/benchmark_dense_prefill_live.py:203. Both raiseAttributeErrorwith this PR. - #11803: sliding-window support. It is rejected again, and the 5 SWA goldens (
swa_*,dispatch_swa_*,persist_swa_*) are deleted. - #12719: the codegen knobs (
lds_num_buffers,iglp_mode,pv_loop_order,o_store_width,pv_priority,pv_sched_fence_mask,causal_diag_split).dispatch/attention/axes.pyanddense_rules.pystill sweep them. - The gfx942
causal_bottom_rightrejection guard. _use_exp2_fastgoes back to theseqlen < 4096gate. Develop replaced it with a compile-time persistent-grid gate because a shape-dependent body collides with the runtime-shape cache key (see the develop docstring).
About 13 tests covering these features are deleted with them. Please re-apply only the CFVST / KDB / VPF changes on top of develop's file and restore the removed tests and goldens.
2. Gate the pipeline so the fp16 D128 path stays byte-identical
_tuning_name_tags adds kdbvpf1 whenever CFVST is on, so the fp16 D128 kernel also changes codegen and its goldens were re-blessed. All measurements are bf16. Please do one of the following:
- put KDB/VPF behind the existing
lds_num_buffersknob (=2meaning K-only double buffer). Its current comment, "NBUF=2 only fits at block_n=32", assumed K and V both doubled, and this PR shows the K-only case fits at bn64. This also makes it a sweepable dispatch axis instead of always-on; or - limit it to bf16 and keep fp16 byte-identical; or
- add fp16 D128 A/B ratios showing no regression, ideally on both MI300X and MI300A.
3. Measure short sequences and say which GPU part was used
The smallest benchmarked S is 4096. bf16 D128 is furthest behind at S=2048, which is also where _use_exp2_fast turns off. Please add S=2048 (and ideally 1024) to the ratio table and state the GPU part(s). Defaults in this kernel have so far been confirmed on both 228-CU and 304-CU gfx942 before adoption. Keep it to ratios only; no absolute numbers.
4. Correctness and synchronization evidence, plus the ticket reference
test_attention_dense_gfx942_numeric.pychecks bf16 at4e-2, which is loose enough to hide a dropped KV tile. Please validate the new pipeline at bf16 below1e-2(fp16 below2e-3) against the fp32 SDPA reference, includingn_up == 1(zero-trip main loop) and the persistent grid.- gfx942 sets
FeatureBackOffBarrier, so the lgkm drain before eachs_barrierhas to be checked in the ISA, not only in the emitter. Please attach (or describe) anllvm-objdumpexcerpt of the loop showings_waitcnt lgkmcnt(0)before both in-loop barriers and before the epilogue barrier. - Please add
JIRA ID: AICK-1900to the description so the policy check passes and the work is tracked.
Happy to re-review once rebased.
Parthiv911
force-pushed
the
users/Parthiv911/rocke/gfx942-dense-cfvst-kdb-vpf
branch
from
October 9, 2026 04:56
ecc1c70 to
072965b
Compare
Contributor
Author
|
@aosewski Thank you for the detailed review! I have addressed the 4 points and highlighted a few details. Few details worth highlighting:
1. Rebase onto develop's kernel instead of overwriting it (blocking)Pulled from develop and applied changes on top of it. 2. Gate the pipeline so the fp16 D128 path stays byte-identicalFixed kernel naming. 'kdbvpf1' kernel-name suffix is now emitted only when lds_num_buffers=2. FP16 goldens unchanged. Selected options a and b. 3. Measure short sequences and say which GPU part was usedAdded S=1024 and 2048. Hardware: AMD Instinct MI300X VF (gfx942, 304 CUs). Results are from the 304-CU part only; no 228-CU measurements are reported. Updated case study.md 4. Correctness and synchronization evidenceCorrectnessTightened tolerance in test_dense_numeric_vs_fp32_sdpa, test_attention_dense_gfx942_numeric.py. BF16 1e-2, FP16 2e-3. Added test_cfvst_pipeline_strict_numeric. Synchronization EvidenceAnalyzed ISA of kernel . assembly.txt
First in-loop, second in-loop, prologue, and epilogue The barrier at Checking for unfinished LDS operations that could conflict with the next work itemThere are 2 paths that reach Path A — Normal completion of an attention work item Path B — Initial entry into the persistent-work-item loop Path A had the epilogue LGKM drain at The additional branch at |
Contributor
Author
|
The PR description and the response to review have been updated to reflect the latest changes. |
aosewski
reviewed
Oct 14, 2026
Contributor
|
@Parthiv911 on your offer to split
39416d4 already had most of this wiring ( |
Contributor
|
@aosewski, follow-up on point 1 (rebase): the branch now merges current Point 3: all numbers are from the 304-CU gfx942 part. No 228-CU measurement exists yet. The configuration is opt-in ( Inline replies follow. |
cgmillette
reviewed
Oct 15, 2026
cgmillette
left a comment
cgmillette
left a comment
Contributor
There was a problem hiding this comment.
PR #13205 — bf16 D128 CFVST, K double buffering, and V prefetch in gfx942 dense prefill (Parthiv911, AICK-1900) — 🟠 REJECT: evidence open, spec-only
| Prior item | Status |
|---|---|
| aosewski R1 #1: rebase dropped develop features, tests, goldens | 🟢 FIXED — develop merged; runtime shape, SWA, knobs, goldens back; merges clean today |
| aosewski R1 #2: keep fp16 D128 byte-identical | 🟢 FIXED — bf16-only opt-in; golden +4 cases, 0 hashes changed (ran ×3 flavors) |
| aosewski R1 #3: add S=1024/2048, name the GPU part | 🟢 FIXED — rows added; 304-CU only (opt-in, 228-CU deferred) |
| aosewski R1 #4: strict tol (n_up=1, persistent), ISA lgkm drain, JIRA ID | 🟢 FIXED — strict tests added; ISA barrier table posted; AICK-1900 in body |
aosewski R2 code/test threads (×11: LSE m_i, sinks gate, spill table, NP=3, _COHORT, odd n_up, injectivity, bn64 gate, docs, wording) |
🟢 FIXED — verified in code; dense CPU + golden pass (ran) |
| aosewski R2: case-study scope (tolerance, baseline spec) | 🟠 PARTIAL — H1 + bounded claim done; both still "not yet recorded" |
| aosewski R2: sweep provenance, ablation arms/shape, baseline build, replay/stack (×4) | 🟠 OPEN — unanswered, text unchanged; "ROCm 10" stack now also in register table |
| aosewski R2 + 10-14: dispatch reachability; split knob, wire axes | 🟠 OPEN — still spec-only; bf16 CFVST undocumented; dense_rules.py:124 comment stale; no follow-up issue |
| Bots / CI | 🟡 pre-commit + superbuild green; TheRock Windows = develop's gfx1201 golden; Linux tests skipped → GPU tests unrun at head |
| Our checks | 🟢 Merge clean; failures = develop's (277 shared, 0 new); dense + dispatch 1007 pass (ran) |
| # | Sev | file:line | Finding |
|---|---|---|---|
| N1 | 🟠 | tests/test_attention_dense_gfx942_numeric.py:1309,1386 |
Pipeline tests keep raw max-abs <1e-2 (~1e-3 over bf16 rounding; _COHORT fix skipped); perf-only → assert bit-identity vs nbuf=1 |
| N2 | 🟡 | kernels/gfx942/attention_dense.py:900,512 |
_use_v_swizzle now dead; docstrings still name it the single policy point |
Utility: 🟠 Mixed — sound opt-in pipeline; unreachable from dispatch, shipped-baseline gain unproven.
Legend: 🔴 blocking · 🟠 should-fix · 🟡 minor nit · 🟢 info/good
yraparti
reviewed
Oct 18, 2026
yraparti
left a comment
yraparti
left a comment
Contributor
There was a problem hiding this comment.
rocKE checklist review
ROCm/rocm-libraries#13205 · 7d794701d366 · model gemini-4-argon, fallback gpt-5.5
FAIL · graded 220/220 · BLOCKER 3, HIGH 11, MEDIUM 9, LOW 1, NEEDS-INFO 7
The diff was truncated before grading. Items that needed a missing hunk are NEEDS-INFO.
Findings
- BLOCKER §E ·
dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_golden.py:284· Update the test specs, expected kernel-name tags, and golden hashes in test_attention_dense_gfx942.py, test_attention_dense_gfx942_golden.py, test_attention_dense_gfx942_numeric.py, and attention_dense_gfx942_ir_sha256.json to pass v_reg_prefetch=True where the full pipeline is intended and re-run the test suites. - BLOCKER §G ·
dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1008· Updatetest_attention_dense_gfx942.pyto add"v_reg_prefetch": (True, False)to_SPEC_PERTURBATIONS, testkdb1/vpf1tags andv_reg_prefetch=Trueintest_bf16_cfvst_kdb_vpf_are_separate_opt_in_variants, and re-runtools/run_checks.pyplusROCKE_LLVM_FLAVOR=llvm23golden checks. - BLOCKER §K ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443· Classifyv_reg_prefetchinlibrary/dispatch/attention/dense_rules.py(e.g. add it toDENSE_UNTUNABLE_KNOBSalongsidelds_num_buffersor expose it as aKnobAxis) and updatelibrary/tests/dispatch/attention/test_tuning_space.pyandlibrary/dispatch/AGENTS.mdto match. - HIGH §A ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443· Add v_reg_prefetch=True to the kdbvpf golden cases (or add dedicated vpf golden cases) in test_attention_dense_gfx942_golden.py and record their hashes in attention_dense_gfx942_ir_sha256.json for all LLVM flavors. - HIGH §B ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443· Updatetest_attention_dense_gfx942.py,test_attention_dense_gfx942_golden.py, andtest_attention_dense_gfx942_numeric.pyto match the decoupledlds_num_buffersandv_reg_prefetchknobs (testing(2, False),(1, True), and(2, True)as well as'kdb1'/'vpf1'tags), or gatesupports_attention_denseto require_use_kdb(spec) == _use_vpf(spec)if only the combined mode is intended to be supported. - HIGH §C ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:735· Updatetest_attention_dense_gfx942.py(andtest_attention_dense_gfx942_golden.py/test_attention_dense_gfx942_numeric.py/README.md/AGENTS.md) to reflect the splitkdb1/vpf1tags and includev_reg_prefetchin_SPEC_PERTURBATIONS, type/mode rejection tests, and pipeline test cases. - HIGH §D ·
dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:114· Put the knob on the KernelSpec and delete the environment read. - HIGH §E ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:735· Update all stale kdbvpf1 / _use_kdb_vpf references across tests and docs to use kdb1, vpf1, and v_reg_prefetch=True, remove the dead _use_kdb_vpf helper, and fix the replay path in gfx942_dense_cfvst_kdb_vprefetch_case_study.md:241. - HIGH §E ·
dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:7· Reconcile the PR description tables and gfx942_dense_cfvst_kdb_vprefetch_case_study.md:3-10 so every reported ratio comes from a single provenance-complete run with bounded wording. - HIGH §G ·
dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1282· Addv_reg_prefetch=Truevariants (both alone withlds_num_buffers=1and combined withlds_num_buffers=2) totest_cfvst_pipeline_strict_numeric,_CFVST_KNOB_CASES, andtest_kdbvpf_pipeline_lseintest_attention_dense_gfx942_numeric.py. - HIGH §G ·
dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:7· Reconcile all prose and tables across the PR body,gfx942_dense_cfvst_kdb_vprefetch_case_study.md,README.md, and test docstrings/cases so the numbers match a single measurement record, remove the stale "Not yet recorded" sentence, and accurately reflect thatlds_num_buffers=2andv_reg_prefetch=Trueare separate knobs. - HIGH §G ·
dnn-providers/hip-kernel-provider/rocke/library/dispatch/AGENTS.md:187· Wire the winning configuration (use_cfvst=True, lds_num_buffers=2, v_reg_prefetch=True) into the gfx942 dense selector/dispatch policy (and C++ twin if applicable) for the validated BF16 D128 causal shapes, or scope the PR title/claims strictly to adding experimental spec-only plumbing without claiming a shipped perf gain. - HIGH §G ·
dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1022· Updatetest_attention_dense_gfx942.py:1002-1024,_INJECTIVITY_BASES(353-380),test_attention_dense_gfx942_golden.py:284-298, andtest_attention_dense_gfx942_numeric.py:1286,1344-1386,1436to passv_reg_prefetch=Trueon the KDB+VPF cases and assert thekdb1/vpf1tags produced by_tuning_name_tags. - HIGH §K ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443· UpdateGfx942DenseKnobSpace.defaults()inlibrary/dispatch/attention/dense_rules.pyifv_reg_prefetchis untunable/defaulted, and document whetherconfig_key/TUNING_ID_VERSIONis affected. - MEDIUM §D ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:893· Updatednn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.pyto add'v_reg_prefetch': (True, False)to_SPEC_PERTURBATIONS, setv_reg_prefetch=Trueon the*_kdbvpfbases/test cases, and assert on the'kdb1'and'vpf1'kernel-name tags instead of'kdbvpf1'. - MEDIUM §E ·
dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:473· Query the device throughrocke.runtime.hip_moduleinstead of importing torch. - MEDIUM §E ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443· Document v_reg_prefetch alongside lds_num_buffers in library/builders/gfx942/attention/prefill/README.md and library/dispatch/AGENTS.md (and update runbook knob documentation if applicable). - MEDIUM §F · summary · Retitle as
type(scope): summary, 10–80 characters, scope[a-z0-9-]+. - MEDIUM §G ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443· Add unit, golden, and numeric test cases that explicitly setv_reg_prefetch=True(withlds_num_buffers=1andlds_num_buffers=2) and verify type/mode rejection forv_reg_prefetch. - MEDIUM §G ·
dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:126· Fixplatform/Pythontoplatform/pythoninbench_rocke_5way_specs.py:126, update the replay command path ingfx942_dense_cfvst_kdb_vprefetch_case_study.md:240-242to match the shipped script location, and run the script end-to-end from the repo root without pre-populatedPYTHONPATH. - MEDIUM §G ·
dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:74· Add SGPR counts and static ISA instruction-count deltas (e.g.,ds_read/ds_write,s_waitcnt,s_barrier) per variant to the register/codegen table ingfx942_dense_cfvst_kdb_vprefetch_case_study.md:82-89. - MEDIUM §J ·
dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:194· UpdateREADME.md:194andgfx942_dense_cfvst_kdb_vprefetch_case_study.md:3to name bothlds_num_buffers=2andv_reg_prefetch=True(or add a separate row inREADME.mdforv_reg_prefetch=True) so the docs match the independent symbols inGfx942AttentionDenseSpec. - MEDIUM §K ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443· Add"v_reg_prefetch": FalsetoGfx942DenseKnobSpace.defaults()inlibrary/dispatch/attention/dense_rules.py(or declare it as a knob axis with its base default). - LOW §J ·
dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:7· Remove the stale 'Not yet recorded' sentence atgfx942_dense_cfvst_kdb_vprefetch_case_study.md:7-10and fix the replay command path at line 241-243 tolibrary/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py.
Needs info
- §A.3 dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443 adds v_reg_prefetch and updates supports_attention_dense (lines 1218-1267), while C++ selector/spec parity files are not in the diff and it is unclear from the diff if Gfx942AttentionDenseSpec has a C++ selector twin.
- §E.14 The diff of dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py is truncated at line 2336 before the LSE epilogue merge resolution mentioned in the PR description.
- §F.7 Diff is truncated and local
pre-commitexecution status is not included in the provided context (note alsodnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:194has 3 blank lines beforedef _command_output). - §F.8 GitHub review thread states (bot/Copilot findings and resolution status) are not included in the provided diff context.
- §G.12 The diff touches only the Python kernel builder
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.pyand does not show whether any C++ builder or selector exists forattention_denseon gfx942. - §K.5
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1218-1267relaxessupports_attention_denseto allowuse_cfvst=Trueon bf16 D128 standard causal attention (withoutsliding_window,causal_diag_split, oruse_sinks), butlibrary/dispatch/attention/dense_rules.pyis not in the diff to verify whetherGfx942DenseKnobSpacedrop/refuse/validate rules foruse_cfvst,use_v_swizzle, andv_row_padon bf16 D128 remain consistent withassert_tuning_contract. - §K.6
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:506-527and:1218-1237changeuse_cfvst,v_row_pad, anduse_v_swizzleresolution and support for bf16 D128, butlibrary/dispatch/attention/dense_rules.pyandlibrary/dispatch/attention/tuning_specs.pyare not in the diff to verify whetherKnobSpace.validateand policy-free spec construction match the new kernel support rules.
Section notes
- §A: Note: test_bf16_cfvst_kdb_vpf_are_separate_opt_in_variants (test_attention_dense_gfx942.py:1022) and test_attention_dense_gfx942_golden.py:284-298 only set lds_num_buffers=2 without v_reg_prefetch=True after decoupling the two knobs.
- §B: Test files (
test_attention_dense_gfx942.py,test_attention_dense_gfx942_numeric.py) appear out of sync withattention_dense.py(e.g., expecting'kdbvpf1'in kernel names when the emitter produces'kdb1'/'vpf1', and never settingv_reg_prefetch=True). - §D: Part 1 of 2 (items 1-11). Note: test_bf16_cfvst_kdb_vpf_are_separate_opt_in_variants in test_attention_dense_gfx942.py is out of sync with the split kdb1/vpf1 tags and missing v_reg_prefetch coverage.
- §D: Items 12-21 of Section D: PR adds opt-in spec-level knobs on Gfx942AttentionDenseSpec without changing dispatcher registration, priority, or default routing.
- §E: Stale references and broken assertions remain after splitting kdb_vpf into separate lds_num_buffers and v_reg_prefetch knobs.
- §F: PR metadata (branch, title, body) passes policy checks, and unit tests are updated with a small inconsistency between
_tuning_name_tags(kdb1/vpf1) andtest_attention_dense_gfx942.py(kdbvpf1) visible in the diff.
Graded with the rocKE PR checklist, one model pass per section. FAIL rows that sit on a diff line are inline comments.
§A graded 14/14
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1383 modifies only the Python-only gfx942 dense attention kernel emitter (C++ attention builder is only for unified_attention_2d_tiled_scalar). |
| 2 | N/A | No core lowering-op or op-handler files (core/lower_*, core/ir) are modified in this PR. |
| 3 | NEEDS-INFO | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443 adds v_reg_prefetch and updates supports_attention_dense (lines 1218-1267), while C++ selector/spec parity files are not in the diff and it is unclear from the diff if Gfx942AttentionDenseSpec has a C++ selector twin. |
| 4 | N/A | No C++ kernel or selector twin is modified or exists for kernels/gfx942/attention_dense.py. |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:9 leaves existing golden hashes (such as default_d128_bf16_causal) unchanged while only adding new opt-in entries. |
| 6 | N/A | This PR modifies a Python-only kernel family (gfx942 attention_dense) that has no C++ kernel emitter twin for cross-engine… |
| 7 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1571 and 2199 guard the new pipeline behind USE_PIPELINE (USE_KDB or USE_VPF) and guard kbuf offsets behind kbuf is not None at line 1827, keeping covered default shapes byte-identical. |
| 8 | N/A | No C++ files or C++-mirrored emitters are touched in this PR. |
| 9 | N/A | No shared op-handler fix or lowerer-only architecture divergence is introduced in this PR. |
| 10 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443 and 737 add v_reg_prefetch=True (emitting tag vpf1 and V-prefetch IR at line 2254) as an independent knob from lds_num_buffers=2, but dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_golden.py:284-298 only sets lds_num_buffers=2 without v_reg_prefetch=True, leaving the v_reg_prefetch=True flag-on IR uncovered in attention_dense_gfx942_ir_sha256.json. |
| 11 | N/A | No new IR ops are added in this PR. |
| 12 | N/A | core/backend.py and ROCKE_BACKEND=both handling are not modified in this PR. |
| 13 | N/A | LLVM IR lowering (.ll emission) and ROCKE_DEBUG_LOC handling are not modified in this PR. |
| 14 | N/A | No classic helpers under helpers/ or platform/cpp/helpers/ are modified in this PR. |
§B graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1218-1267 classifies unvalidated feature combinations on BF16 CFVST and lds_num_buffers=2/v_reg_prefetch=True with explicit NOT_YET_IMPLEMENTED: prefixes, distinguishing them from… |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1218-1267 and 1383-1395 keep supports_attention_dense as the single functional gate consulted by build_attention_dense while Gfx942AttentionDenseSpec.__post_init__ (lines 478-495)… |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:440-443 and 867 keep default routing and policies unchanged (lds_num_buffers=1, v_reg_prefetch=False, _use_cfvst False for bf16), so no model-zoo shape is demoted to a fallback. |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:478-491 validates type(self.use_cfvst) is not bool, type(self.lds_num_buffers) is not int, and type(self.v_reg_prefetch) is not bool in Gfx942AttentionDenseSpec.__post_init__… |
| 5 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443 and 1253-1267 open v_reg_prefetch=True (both standalone with lds_num_buffers=1 and combined with lds_num_buffers=2), and lines 735-738 emit 'kdb1'/'vpf1' kernel name tags, but dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:325,1021-1023, dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_golden.py:284-298, and… |
| 6 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1218-1267 updates supports_attention_dense to remove the old bf16 D128 CFVST spill policy rejection and instead gates only on hardware/algorithmic invariants (rows-per-DMA==1 at lines… |
§C graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Shared logic and predicates are factored into named helpers _use_kdb, _use_vpf, _use_kdb_vpf, and softmax_pv in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:883-899 and… |
| 2 | PASS | The new pipeline variants are controlled via spec knobs (use_cfvst, lds_num_buffers, v_reg_prefetch) inside the single build_attention_dense(spec) builder in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1379-1456, and the… |
| 3 | PASS | Kernel naming uses codegen knob tags (kdb1, vpf1, cfvst1, vswz1) in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:735-749 without embedding workload sizes. |
| 4 | PASS | All kernel changes are properly scoped inside the gfx942-specific module dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:345-445 without touching shared common/ or other architectures' files. |
| 5 | PASS | The diff touches only the gfx942 attention_dense kernel (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:114), its corresponding tests (dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:56)… |
| 6 | PASS | softmax_pv is cleanly extracted in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2053-2127 so the existing single-buffer tile_body stays byte-identical (existing hashes unchanged in… |
| 7 | N/A | This PR is a targeted performance optimization adding opt-in spec knobs rather than a standalone cleanup refactor. |
| 8 | PASS | The PR extends existing Gfx942AttentionDenseSpec fields (use_cfvst, lds_num_buffers) and reuses existing DSL primitives (b.smem_alloc, b.sync_lds_only, _cfvst_load_v, _cfvst_store_v) in… |
| 9 | PASS | Type-coercion validation (type(...) is not bool/int) is applied across all new/modified spec knobs (use_cfvst, lds_num_buffers, v_reg_prefetch) in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:478-491, and use_sinks… |
| 10 | FAIL | In dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:735-738, _tuning_name_tags was widened to emit independent kdb1 and vpf1 tags instead of kdbvpf1, and v_reg_prefetch was added to Gfx942AttentionDenseSpec (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443), but dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1021-1023 still asserts "kdbvpf1" in gfx942_kernel_name(pipeline) (where pipeline only sets… |
§D graded 21/21
| # | Verdict | Evidence |
|---|---|---|
| 1 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:114 reads the environment |
| 2 | N/A | No Capability declarations or candidate registrations are added or modified in this PR. |
| 3 | N/A | No candidate _supports or admits methods in the dispatcher layer are touched by this PR. |
| 4 | PASS | Spec helpers and supports_attention_dense(spec, arch) remain pure functions of the spec and explicit arch parameter with no host device probes (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1213-1267). |
| 5 | FAIL | In dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:893, _use_kdb_vpf(spec) checks _use_kdb(spec) and _use_vpf(spec), and _tuning_name_tags (lines 735-738) emits 'kdb1' and 'vpf1', yet dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:999-1024 constructs pipeline = _spec(use_cfvst=True, lds_num_buffers=2) (where v_reg_prefetch defaults to False) and asserts _use_kdb_vpf(pipeline) and 'kdbvpf1' in gfx942_kernel_name(pipeline), which will fail… |
| 6 | N/A | No cohort routing changes are made in this PR; the new pipeline modes are spec-level opt-ins that leave grid and block launch… |
| 7 | N/A | No selector heuristics or hand-pinned routing overrides are added or modified in this PR. |
| 8 | N/A | No cohort routing predicates are introduced or changed in this PR. |
| 9 | N/A | No new KernelCandidate or architecture is registered in ATTENTION_ROUTE_REGISTRY or ATTENTION_EXECUTION_REGISTRY. |
| 10 | PASS | The new fields (lds_num_buffers=2, v_reg_prefetch=True) are internal codegen/scheduling knobs rather than user-facing problem/request features, and dnn-providers/hip-kernel-provider/rocke/library/dispatch/AGENTS.md:187-193 explicitly documents them as spec-only… |
| 11 | PASS | The BF16 CFVST, KDB, and VPF changes are strictly opt-in (use_cfvst=None resolves to False for BF16, lds_num_buffers=1, v_reg_prefetch=False by default) and do not alter the auto dispatch default… |
| 12 | N/A | This PR does not add or rename any dispatcher candidate, change candidate priority, or introduce a custom ranker. |
| 13 | N/A | This PR does not modify CandidateRegistry.select, pin resolution, or any dispatch wrapper entry point. |
| 14 | N/A | This PR does not modify KernelId caching keys or change the kernel argument ABI layout. |
| 15 | N/A | This PR does not add a new dispatcher candidate or architecture module to the registry. |
| 16 | N/A | This PR does not introduce a new operator family. |
| 17 | N/A | This PR does not add a new dispatcher candidate or change problem-to-spec routing rules. |
| 18 | N/A | This PR does not modify registry ratchets, torch bindings, or test exempt sets. |
| 19 | PASS | dnn-providers/hip-kernel-provider/rocke/library/dispatch/AGENTS.md:187-192 and dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:186-195 accurately document that lds_num_buffers=2 and bf16 use_cfvst=True are explicit opt-ins… |
| 20 | N/A | This PR does not modify a problem-to-spec selector or a kernel family with a C++ selector/builder twin. |
| 21 | N/A | This PR implements new opt-in kernel pipeline features (BF16 CFVST, K LDS double buffering, and V register prefetch) rather than… |
§E graded 17/17
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only modifies files under library/ and touches no files under platform/. |
| 2 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:473 adds import torch |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:224 imports torch lazily inside @requires_gfx942_gpu test functions while non-GPU unit tests do not import torch. |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:474 imports downward from kernels and dispatch, and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py adds no upward imports. |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:29 is a Python script using pathlib.Path and contains no hardcoded /tmp, nproc, or sudo calls. |
| 6 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2009 preserves unused tuple unpacking with underscore prefixes (_d0, _t0) and golden hashes in… |
| 7 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443 adds the v_reg_prefetch spec knob, but dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:194 and dnn-providers/hip-kernel-provider/rocke/library/dispatch/AGENTS.md:187 omit v_reg_prefetch and describe lds_num_buffers=2 as enabling both K double buffering and V prefetch. |
| 8 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:134 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:996 use descriptive references without Jira IDs, PR numbers, or GitHub handles in code or… |
| 9 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:436 and accompanying test/doc files are all scoped to the gfx942 BF16 D128 CFVST, K double-buffering, and V prefetch change. |
| 10 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_golden.py:284 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1022 were not re-run or updated after dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:735 split kdbvpf1 into kdb1 and vpf1 and gated V prefetch on v_reg_prefetch=True. |
| 11 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:1 and dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:186 are process-required deliverables… |
| 12 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1218 explicitly scopes the opt-in knobs to the gfx942 dense kernel. |
| 13 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:735 replaced the kdbvpf1 tag with kdb1 and vpf1 and split _use_kdb_vpf into _use_kdb and _use_vpf, but dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1002 and :1022 still construct pipeline = _spec(use_cfvst=True, lds_num_buffers=2) and assert _use_kdb_vpf(pipeline) and 'kdbvpf1' in gfx942_kernel_name(pipeline), and… |
| 14 | NEEDS-INFO | The diff of dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py is truncated at line 2336 before the LSE epilogue merge resolution mentioned in the PR description. |
| 15 | PASS | All parameters in modified helpers in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1824 (_async_load, load_pipeline_k, do_qk, softmax_pv, publish_prefetched_v) are used in their function bodies. |
| 16 | N/A | This item governs reviewer severity grading rather than code in the pull request diff. |
| 17 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:7 states that the baseline arm's full spec and pre-timing correctness tolerance are 'Not yet recorded' (contradicting lines 148 and 166), and dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:436 notes that the 5-way ablation table in the PR body cannot be attributed to a recorded shape. |
§F graded 12/12
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Branch users/Parthiv911/rocke/gfx942-dense-cfvst-kdb-vpf matches the recognized users/<user>/... prefix (and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:114). |
| 2 | PASS | branch users/Parthiv911/rocke/gfx942-dense-cfvst-kdb-vpf has a rocke path segment |
| 3 | FAIL | title perf(rocke): bf16 D128 CFVST, K double buffering, and V prefetch in gfx942 dense prefill is 88 characters and does not match type(scope): summary with a 10–80 character length |
| 4 | PASS | description is at least 30 characters and contains an issue reference |
| 5 | PASS | test file dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py is in the diff |
| 6 | PASS | All 10 files in the diff are Python, Markdown, or JSON golden hash files (e.g., dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:5); no keys, certs, or secrets are added. |
| 7 | NEEDS-INFO | Diff is truncated and local pre-commit execution status is not included in the provided context (note also dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:194 has 3 blank lines before def _command_output). |
| 8 | NEEDS-INFO | GitHub review thread states (bot/Copilot findings and resolution status) are not included in the provided diff context. |
| 9 | N/A | Base branch is develop, so this is not a stacked PR targeting a feature branch. |
| 10 | PASS | The PR includes the complete kernel changes, tests, benchmark script, and case study together without artificial size-cap splitting (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:114). |
| 11 | PASS | Added tests exercise real functionality (LDS budget, spec validation, golden IR hashes, and numeric accuracy) in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:999 and… |
| 12 | PASS | The PR title and body are populated in the PR metadata above and match the files in the diff (dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:1). |
§G graded 34/34
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1252 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_golden.py:283 pair IR golden hashes with independent on-GPU numeric validation… |
| 2 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1282 tests only lds_num_buffers in {1, 2} and never sets v_reg_prefetch=True, leaving the v_reg_prefetch code path (USE_VPF at dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1570) without on-silicon numeric coverage in the test suite. |
| 3 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1008 asserts assert _use_kdb_vpf(pipeline) and line 1022 asserts assert "kdbvpf1" in gfx942_kernel_name(pipeline) for pipeline = _spec(use_cfvst=True, lds_num_buffers=2), which fails because _use_kdb_vpf requires v_reg_prefetch=True (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:899) and _tuning_name_tags emits "kdb1" and "vpf1" (attention_dense.py:736-738); additionally _SPEC_PERTURBATIONS… |
| 4 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443 adds the independent v_reg_prefetch: bool = False spec field and USE_VPF codegen branch (line 2254), but dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:999 and test_attention_dense_gfx942_golden.py:283 never pass v_reg_prefetch=True. |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1226 uses _MULTI_ITEM_NP = 3 and sq=256, skv=192 (line 1248) with docstrings explaining how multi-item persistent grids and odd n_up > 1 reach the epilogue K-slot-0 read… |
| 6 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:7-10 claims the baseline arm's full spec and pre-timing correctness tolerance are "Not yet recorded", contradicting lines 165 and 250-266 of the same file which record atol=1e-2, rtol=1e-2 and the full per-shape spec table; furthermore, the PR body's Performance Gains table (e.g., B1 S1024 Hq32 Hkv8 = 1.1342x / 11.83%) disagrees with the case study Results table at line 106 (1.1203x / 10.74%), and… |
| 7 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:164-182 and bench_rocke_5way_specs.py:131-163 specify and implement the chunked FP32 causal GQA SDPA reference with… |
| 8 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:101-102 explicitly rejects any dtype other than bf16 or head dim other than 128 via argparse.ArgumentTypeError with no silent dtype fallback. |
| 9 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:76-88 sources VGPR count, spill count, and scratch bytes directly from compiled code-object notes (.vgpr_count, .vgpr_spill_count… |
| 10 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:102-118 and 218-229 lock hardware (gfx942, 304 CUs), software stack, commit SHA, relative speedup ratios (no absolute latencies), and… |
| 11 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/dispatch/AGENTS.md:187-192 and dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:191-194 leave use_cfvst=True (for bf16 D128), lds_num_buffers=2, and v_reg_prefetch=True off by default and untunable in dispatch/selectors, so the optimized kernel reported to give 10-16% latency reduction is unreachable through the production routing/selector path for the benchmarked shapes. |
| 12 | NEEDS-INFO | The diff touches only the Python kernel builder dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py and does not show whether any C++ builder or selector exists for attention_dense on gfx942. |
| 13 | N/A | This PR does not introduce or recommend any new LLVM intrinsic .td overload classes or mangled intrinsic symbols. |
| 14 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:19 and 98 state that bottlenecks and sync-point stalls were classified using profiling and threadtrace alongside empirical GPU-timed lever… |
| 15 | N/A | The review is graded directly from the provided PR diff against head 7d794701d366ae405e38c6eb6548335d7e7b4c6b. |
| 16 | N/A | This PR ships a completed implementation and benchmark case study rather than a pre-implementation plan. |
| 17 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:126 adds rocke / "platform/Python" (capital P) to sys.path whereas the repository directory is platform/python (lowercase p, cf. test_attention_dense_gfx942_lds.py:32), and gfx942_dense_cfvst_kdb_vprefetch_case_study.md:240 invokes a non-existent path library/benchmarks/gfx942/attention/prefill/rocke_vs_aiter/bench_rocke_5way_specs.py instead of the shipped path… |
| 18 | PASS | The validation guard in dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:669-684 and 710-722 wires the torch.testing.assert_close verdict to abort the run immediately, takes median over 5 rounds of 50… |
| 19 | PASS | Fixtures in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1226-1237 (_multi_item_persistent with _MULTI_ITEM_NP = 3 so workgroups run >2 items with a ragged tail to expose LDS hand-off) and 1246-1250 (odd n_up=3… |
| 20 | N/A | This PR does not add or modify a text detector or linter enforcing a ban on claims. |
| 21 | FAIL | In dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1022-1023, test_bf16_cfvst_kdb_vpf_are_separate_opt_in_variants sets pipeline = _spec(use_cfvst=True, lds_num_buffers=2) (leaving v_reg_prefetch=False), which makes _use_kdb_vpf(pipeline) at 1008 return False (attention_dense.py:899 requires both _use_kdb and _use_vpf) and gfx942_kernel_name(pipeline) emit kdb1 (attention_dense.py:736) rather than kdbvpf1, so this test and test_cfvst_knob_matrix_numeric… |
| 22 | PASS | Every new golden fixture entry in dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:5-8,61-64,69-72,101-104,153-156,209-212,217-220,249-252 records both bytes and sha256 across both llvm20 and llvm22 flavors… |
| 23 | PASS | The case study dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:105-118,251-267 and benchmark harness… |
| 24 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:5-104,153-252, no pre-existing golden hashes were modified; only 4 new opt-in cases (cfvst_d128_bf16_causal, kdbvpf_d128_bf16_causal… |
| 25 | FAIL | In dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:74-96, VGPR, spill count, and scratch bytes before and after are reported alongside the 12-shape corpus A/B (105-139), but static ISA instruction-count deltas (waits, barriers, LDS reads/writes) and SGPR counts per kernel touched are not reported. |
| 26 | PASS | The case study dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:251-267 and benchmark harness… |
| 27 | PASS | Both dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:45-51 and dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:122-157… |
| 28 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:186-195 and dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:3-11,251-267 explicitly… |
| 29 | N/A | This PR does not swap any guarded primitive for a fast primitive (exp2_fast policy is untouched). |
| 30 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:76-95, spill and scratch claims are backed by reading .vgpr_count, .vgpr_spill_count, and .private_segment_fixed_size from the… |
| 31 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:517-561, the baseline is the exact unmodified attention_tuning_spec(req, 'gfx942_dense').kernel_spec shipped on develop, and the V-LDS pad (0 at… |
| 32 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:76-89,215-248 and dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:259-323… |
| 33 | PASS | The changed spec resolvers (resolved_use_cfvst, resolved_v_row_pad, resolved_use_v_swizzle, _lds_bytes, kernel_name) are evaluated and differential-tabled across their input domain in… |
| 34 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1426-1449, test_kdbvpf_pipeline_lse runs the new pipeline with emit_lse both on and off via _run_lse_and_off and asserts torch.equal(out, off_out) at line 1449. |
§H graded 5/5
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only modifies the dense prefill kernel (attention_dense.py) to add opt-in CFVST, K LDS double buffering, and V register… |
| 2 | N/A | This PR applies to dense prefill attention and does not use page tables, block_tables, or any device-side indirection values. |
| 3 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2210-2211 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2247, the hot-loop b.div/b.mod divisors are V_DIM_PAIRS (64, since `head_size… |
| 4 | N/A | This PR does not construct or modify any device-side mask or bound from runtime sequence-length arrays (kv_lens, cu_seqlens)… |
| 5 | N/A | No new launch runtime inputs that feed device-side bounds or masks are introduced or modified in this PR (lds_num_buffers… |
§I graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:26 |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:478 |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:348 |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:105 |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:305 |
| 6 | N/A | This PR does not add or modify any DSL primitive or hardware instruction interface; it only adds opt-in kernel spec knobs… |
§J graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Scalar latency-reduction formulas in dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:188-209 and LDS layout descriptions in… |
| 2 | PASS | All formula symbols (T_variant, t_variant,r, T_after, T_before, R_r, Min, Max) are defined in-place with consistent domains in `dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.m… |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:189-195 links to gfx942_dense_cfvst_kdb_vprefetch_case_study.md and states only the summary table delta without duplicating the methodology or algorithm. |
| 4 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:194 and dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:3 claim lds_num_buffers=2 enables both K LDS double buffering AND V register prefetch, whereas dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:436-443 defines lds_num_buffers and v_reg_prefetch as independent spec knobs. |
| 5 | N/A | The PR does not make asymptotic scaling or big-O complexity claims in its documentation or comments. |
| 6 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:7-10 contains a contradictory stale placeholder ('Not yet recorded: the baseline arm's full spec ... and the tolerance') even though those exact constants and provenance tables are recorded later at lines 166 and 251-266, and line 241 gives a broken replay path (rocke_vs_aiter/bench_rocke_5way_specs.py) that does not match the actual file path at line 231. |
| 7 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:188-190 serves as the directory entry point and links to the companion case study… |
§K graded 13/13
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR does not add or modify a tuned candidate or KnobSpace subclass. |
| 2 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443 adds a new dataclass field v_reg_prefetch: bool = field(default=False, kw_only=True) to Gfx942AttentionDenseSpec, and :268 removes lds_num_buffers from _DEAD_SPEC_FIELDS in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:265, without classifying v_reg_prefetch (e.g. in DENSE_UNTUNABLE_KNOBS or as a KnobAxis) in library/dispatch/attention/dense_rules.py or updating… |
| 3 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443 adds a new field v_reg_prefetch with default False to Gfx942AttentionDenseSpec, and :509-527 changes resolved_v_row_pad() and resolved_use_v_swizzle() when use_cfvst=True on bf16 D128, but library/dispatch/attention/dense_rules.py (defaults() / TUNING_ID_VERSION) is not updated in the PR. |
| 4 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443 adds v_reg_prefetch: bool = field(default=False, kw_only=True) to Gfx942AttentionDenseSpec, which is a problem-independent dataclass default on the tuned spec, but KnobSpace.defaults() in library/dispatch/attention/dense_rules.py is not updated to cover it. |
| 5 | NEEDS-INFO | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1218-1267 relaxes supports_attention_dense to allow use_cfvst=True on bf16 D128 standard causal attention (without sliding_window, causal_diag_split, or use_sinks), but library/dispatch/attention/dense_rules.py is not in the diff to verify whether Gfx942DenseKnobSpace drop/refuse/validate rules for use_cfvst… |
| 6 | NEEDS-INFO | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:506-527 and :1218-1237 change use_cfvst, v_row_pad, and use_v_swizzle resolution and support for bf16 D128, but library/dispatch/attention/dense_rules.py and library/dispatch/attention/tuning_specs.py are not in the diff to verify whether KnobSpace.validate and policy-free spec construction match the new kernel support… |
| 7 | N/A | KnobSpace.base(req) is not modified in this PR. |
| 8 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:478-491 validates use_cfvst, lds_num_buffers, and v_reg_prefetch with exact type checks (type(...) is not bool / int) and raises ValueError rather than coercing with… |
| 9 | N/A | The benchmark added in `dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.… |
| 10 | N/A | KnobSpace.find and bare tuning_id resolution are not touched by this PR. |
| 11 | N/A | No sweep entry points (iter_dispatch_*, pin_to_spec, sweep_space) are modified or called in this PR. |
| 12 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:440-443 and :867 keep lds_num_buffers=1, v_reg_prefetch=False, and _use_cfvst (False for bf16) as the defaults, and… |
| 13 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:583-586 delegates legality checking to dense.supports_attention_dense(spec, arch='gfx942') and aborts on any failure rather than silently dropping… |
§L graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR does not add or modify a gfx target in arch_specs.json or the C++ arch/ISA registries. |
| 2 | N/A | The PR does not add or change any MMA op_id or fragment layout in core/arch/target.py or cpp/core/lower_llvm/mma.cpp. |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1960 continues to use the existing mfma_32x32x8_for_dtype(b, dtype, k_pack, q_packs[ks], acc) helper without hardcoding new intrinsic names or altering atom shapes/lane maps. |
| 4 | N/A | The PR is scoped to the existing gfx942 kernel and does not modify wave-size definitions or target wave-size logic. |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1824 implements gfx942 K LDS double-buffering and V register prefetching directly using standard gfx942 buffer loads and perm_b32 transpose without leaking gfx950 ds_read_tr* or… |
| 6 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:440 keeps the pipeline knobs (lds_num_buffers, v_reg_prefetch) on Gfx942AttentionDenseSpec in the library kernel layer without touching ArchTarget or importing CK Tile's… |
| 7 | N/A | The PR does not add or modify logical dtypes, bit-packing contracts, host pack/unpack routines, or FP8 dialect handling. |
| 8 | N/A | The PR operates on unscaled bf16/fp16 attention and does not touch scaled-MMA contracts or WMMA scale ids. |
| 9 | N/A | The PR does not introduce a new architecture target. |
| 10 | N/A | The PR does not modify intrinsic_arch_domain.<flavor>.json artifacts or LLVM flavor membership lists. |
§M graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Host booleans (USE_PIPELINE, USE_KDB, USE_VPF, masked) drive Python control flow while SSA loop carries (m_new, l_new, *o_acc) are yielded through b.scf_yield and read from loop.results in… |
| 2 | N/A | This PR does not touch legalize_for_target or any target legalization passes. |
| 3 | N/A | This PR does not modify print_ir or the ck.dsl.ir/v1 serialization implementation. |
| 4 | N/A | This PR does not modify portable IR schemas, importers, magic division helpers, or builder recording. |
| 5 | PASS | Async K tile loads compute an unswizzled wave-uniform destination offset per row and consumers drain VMEM via b.s_waitcnt(vmcnt=0) followed by b.sync_lds_only() before reading LDS in… |
| 6 | PASS | All barriers (b.sync_lds_only(), b.s_waitcnt(vmcnt=0)) are emitted unconditionally in the workgroup-uniform prologue, scf_for_iter body, and epilogue in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2222-2295. |
| 7 | N/A | This PR does not touch ISABackend.buffer_rsrc_word3, hard-code buffer resource word 3, or use TemporalHint.STREAMING. |
| 8 | N/A | This PR does not add intrinsic declarations or modify lowering backend selection precedence. |
| 9 | N/A | This PR does not touch C entry points, headers under cpp/include/rocke/, or C++ host launch code. |
| 10 | N/A | This PR does not modify !dbg emission, module debug info flags, or DWARF consumers. |
§N graded 18/18
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR does not add or promote any helpers under platform/python/rocke/helpers/. |
| 2 | N/A | The PR modifies an existing lower-level DSL attention kernel and does not introduce new helper abstractions or modify… |
| 3 | N/A | The PR implements a custom K-LDS double-buffer and V-register-prefetch loop in attention_dense.py rather than modifying or using… |
| 4 | N/A | The PR optimizes an existing attention prefill kernel and does not add or modify epilogue/graph fusion kernels. |
| 5 | N/A | The PR does not add or modify any public API in rocke.helpers.tiling. |
| 6 | N/A | The PR does not add or modify front-door/toolbox tiling helpers or LayoutStyle classes. |
| 7 | N/A | The PR does not use helpers/tiling layout overrides or modify tiling MMA soundness gates. |
| 8 | N/A | The PR does not touch helpers/tiling memory verbs (lds_tile_alloc, lds_store, lds_read) or emit.py. |
| 9 | N/A | The PR does not use helpers/tiling N-D tensor axis roles, FlowStage, or label transforms. |
| 10 | N/A | The PR does not modify helpers/tiling emit, LDS staging, or MMA issue logic. |
| 11 | N/A | The PR does not use TileMma, Tiling, lds_swizzle, or derive_sched_group_counts from helpers/tiling. |
| 12 | N/A | The PR does not make helpers/tiling bank-conflict simulator or coalescing_probe gate claims. |
| 13 | N/A | The PR does not construct WarpDistributionEncoding, call make_tile_desc, or touch RegisterMapper. |
| 14 | PASS | The new double-buffered K / V-prefetch pipeline explicitly places b.sync_lds_only() and b.s_waitcnt(vmcnt=0) around LDS reads and writes (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2224… |
| 15 | PASS | LDS allocation and budget checks account for the padded/swizzled row stride and buffer count (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1075… |
| 16 | N/A | The PR does not use optimize_layout or evaluate_transform from helpers/tiling. |
| 17 | N/A | The PR does not touch tiling_recorder.py, analysis/roundtrip.py, or tiling visualization. |
| 18 | N/A | The PR does not touch InterleavedStyle or demo kernels under helpers/tiling/kernels/. |
§O graded 16/16
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only modifies files under library/ and does not touch platform core verifier or lowerer files. |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2229 — the new scf_for_iter loop uses i32 bounds/step, yields matching iter_args via b.scf_yield(m_new, l_new, *o_acc) at line 2282, and reads loop.results only after the loop exits at… |
| 3 | N/A | This PR does not modify IRBuilder, intrinsic declaration tables, or metadata numbering in platform core. |
| 4 | N/A | This PR does not touch IR type definitions, SmemType, or IR parsers in platform core. |
| 5 | N/A | This PR does not touch core/passes.py, cpp/core/passes.cpp, or opcode purity tables. |
| 6 | N/A | This PR does not modify constant folding passes in Python or C++ core. |
| 7 | N/A | This PR does not modify kernel attribute lowering, CodegenPolicy validation, or COMGR options in platform core. |
| 8 | N/A | This PR does not set or modify SchedulerStrategy or CodegenPolicy. |
| 9 | N/A | This PR does not modify platform waitcnt or barrier lowering logic (and targets gfx942 rather than gfx950). |
| 10 | N/A | This PR does not modify platform lowerer attribute validation or parameter LLVM type helpers. |
| 11 | N/A | This PR does not touch ArchTarget or target resolution in platform core. |
| 12 | N/A | This PR does not modify platform core width/size caps or arch default resolution. |
| 13 | N/A | This PR does not touch any C headers or structs under cpp/include/rocke/. |
| 14 | N/A | This PR does not add or modify any C/C++ files or CMake files. |
| 15 | N/A | This PR does not touch C++ engine fallback logic or backend flavor resolution in platform core. |
| 16 | N/A | This PR does not touch C++ runtime/codegen gap areas or gfx1250 builders. |
§P graded 14/14
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:440-443 adds defaulted keyword-only fields (lds_num_buffers: int = field(default=1, kw_only=True) and v_reg_prefetch: bool = field(default=False, kw_only=True)) on the arch subclass… |
| 2 | N/A | This PR does not modify kernarg ordering, signature(), manifest args_signature, or GEMM/conv ABI definitions. |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:553-556 takes grid=dense.attention_dense_grid(spec) and block=dense.attention_dense_block(spec) directly from the spec helpers rather than recomputing… |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1075 charges k_bytes *= spec.lds_num_buffers in _lds_bytes matching K_lds = b.smem_alloc(dtype, [K_BUFS, BN, LDROW], name_hint="Klds") at line 1584, and… |
| 5 | N/A | This PR does not touch WMMA/RDNA GEMM or GemmMultiD/GemmMultiAbd platform instances. |
| 6 | N/A | This PR does not use or modify caller-zeroed workspaces (Stream-K, MoE sorting, fused MoE, or split-K). |
| 7 | N/A | This PR does not touch Stream-K, persistent GEMM persistent_max_iters, GroupedGemmLauncher, or SUPPORT_MATRIX.md. |
| 8 | N/A | This PR does not touch quantization helpers or MX GEMM. |
| 9 | N/A | This PR does not introduce numeric emulation or quantized decode paths. |
| 10 | N/A | This PR does not add or modify manifest kinds, emitters, or manifest runners. |
| 11 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:534-556 compiles all kernels ahead of timing and constructs KernelLauncher instances, and lines 163-172 time repeated launches using GPU… |
| 12 | N/A | This PR does not touch COMGR or HIP library loading logic or fix a hipError launch failure. |
| 13 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:735-738 and 1248-1267 add cache-name tags (kdb1, vpf1) and admission guards in supports_attention_dense, which are tested in… |
| 14 | N/A | This PR does not touch examples/, expected.json, platform/python/rocke/instances/, or helpers/. |
§Q graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | No new test files are added or removed in this PR; only existing test files under library/tests/ are modified. |
| 2 | N/A | This PR modifies the Python-only library/kernels/gfx942/attention_dense.py builder and does not add a platform kernel family or… |
| 3 | PASS | Golden hashes in dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:5 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_golden.py:283 are used as an IR drift tripwire alongside GPU… |
| 4 | PASS | New golden entries are recorded for both llvm20 (dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:5) and llvm22… |
| 5 | PASS | CPU tests in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1059 use values the parser genuinely rejects (e.g., use_cfvst='false'), and GPU tests in… |
| 6 | N/A | This PR does not refactor any platform gate or test harness. |
| 7 | N/A | This PR only updates library spec validation in supports_attention_dense and does not touch platform backend resolution or… |
yraparti
reviewed
Oct 19, 2026
yraparti
left a comment
yraparti
left a comment
Contributor
There was a problem hiding this comment.
rocKE checklist re-review of the fixes since 7d794701d366
ROCm/rocm-libraries#13205 · 4072b4ae138a · model gemini-4-argon, fallback gpt-5.5
FAIL · graded 220/220 · BLOCKER 3, HIGH 11, MEDIUM 9, LOW 1, NEEDS-INFO 7
The diff was truncated before grading. Items that needed a missing hunk are NEEDS-INFO.
Since the last review (7d794701d366)
Resolved 0 · still open 24 · new 0
- ⏳ §A.10 still FAIL (HIGH): Add v_reg_prefetch=True to the kdbvpf golden cases (or add dedicated vpf golden cases) in test_attention_dense_gfx942_golden.py and record their hashes in attention_dense_gfx942_ir_sha256.json for all LLVM flavors.
- ⏳ §B.5 still FAIL (HIGH): Update
test_attention_dense_gfx942.py,test_attention_dense_gfx942_golden.py, andtest_attention_dense_gfx942_numeric.pyto test the decoupledlds_num_buffersandv_reg_prefetchknobs ((2, False),(1, True), and(2, True)along with'kdb1'/'vpf1'tags), or gatesupports_attention_denseto require matching knobs if only the combined mode is intended. - ⏳ §C.10 still FAIL (HIGH): Update
test_attention_dense_gfx942.py,test_attention_dense_gfx942_golden.py,test_attention_dense_gfx942_numeric.py,README.md, andAGENTS.mdto reflect the splitkdb1/vpf1tags and includev_reg_prefetchin_PRIVATE_PERTURBATIONS, type/mode rejection tests, and pipeline test cases. - ⏳ §D.1 still FAIL (HIGH): Put the knob on the KernelSpec and delete the environment read.
- ⏳ §D.5 still FAIL (MEDIUM): Update
dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.pyto add'v_reg_prefetch': (True, False)to_SPEC_PERTURBATIONS, setv_reg_prefetch=Trueon the*_kdbvpfbases/test cases, and assert on the'kdb1'and'vpf1'kernel-name tags instead of'kdbvpf1'. - ⏳ §E.2 still FAIL (MEDIUM): Query the device through
rocke.runtime.hip_moduleinstead of importing torch. - ⏳ §E.7 still FAIL (MEDIUM): Document
v_reg_prefetchalongsidelds_num_buffersinlibrary/builders/gfx942/attention/prefill/README.mdandlibrary/dispatch/AGENTS.md. - ⏳ §E.10 still FAIL (BLOCKER): Update the test specs, expected kernel-name tags (
kdb1,vpf1), and golden hashes intest_attention_dense_gfx942.py,test_attention_dense_gfx942_golden.py,test_attention_dense_gfx942_numeric.py, andattention_dense_gfx942_ir_sha256.jsonto passv_reg_prefetch=Truewhere the full pipeline is intended and re-run the test suites. - ⏳ §E.13 still FAIL (HIGH): Update all stale
kdbvpf1/_use_kdb_vpfreferences across tests and docs to usekdb1,vpf1, andv_reg_prefetch=True, remove_use_kdb_vpf, and remove the nonexistentrocke_vs_aiter/subdirectory fromgfx942_dense_cfvst_kdb_vprefetch_case_study.md. - ⏳ §E.17 still FAIL (HIGH): Reconcile the PR description and
gfx942_dense_cfvst_kdb_vprefetch_case_study.md:3-15with the recorded tables (10.74%-15.58% at the 12 measured configurations) and remove the stale 'Not yet recorded' text. - ⏳ §F.3 still FAIL (MEDIUM): Retitle as
type(scope): summary, 10–80 characters, scope[a-z0-9-]+. - ⏳ §G.2 still FAIL (HIGH): Add
v_reg_prefetch=Truevariants (both alone withlds_num_buffers=1and combined withlds_num_buffers=2) totest_cfvst_pipeline_strict_numeric,_CFVST_KNOB_CASES, andtest_kdbvpf_pipeline_lseintest_attention_dense_gfx942_numeric.py. - ⏳ §G.3 still FAIL (BLOCKER): Update
test_attention_dense_gfx942.pyto add"v_reg_prefetch": (True, False)to_SPEC_PERTURBATIONS, testkdb1/vpf1tags andv_reg_prefetch=Trueintest_bf16_cfvst_kdb_vpf_are_separate_opt_in_variants, and re-runtools/run_checks.pyplusROCKE_LLVM_FLAVOR=llvm23golden checks. - ⏳ §G.4 still FAIL (MEDIUM): Add unit, golden, and numeric test cases that explicitly set
v_reg_prefetch=True(withlds_num_buffers=1andlds_num_buffers=2) and verify type/mode rejection forv_reg_prefetch. - ⏳ §G.6 still FAIL (HIGH): Remove the stale "Not yet recorded" sentence and update the
10.1-16.3%range to10.7-15.6%ingfx942_dense_cfvst_kdb_vprefetch_case_study.md:3-15, and updateREADME.md:194andtest_attention_dense_gfx942_golden.py:19-20to state thatlds_num_buffers=2andv_reg_prefetch=Trueare separate knobs.
Findings
- BLOCKER §E ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:735· Update the test specs, expected kernel-name tags (kdb1,vpf1), and golden hashes intest_attention_dense_gfx942.py,test_attention_dense_gfx942_golden.py,test_attention_dense_gfx942_numeric.py, andattention_dense_gfx942_ir_sha256.jsonto passv_reg_prefetch=Truewhere the full pipeline is intended and re-run the test suites. - BLOCKER §G ·
dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1008· Updatetest_attention_dense_gfx942.pyto add"v_reg_prefetch": (True, False)to_SPEC_PERTURBATIONS, testkdb1/vpf1tags andv_reg_prefetch=Trueintest_bf16_cfvst_kdb_vpf_are_separate_opt_in_variants, and re-runtools/run_checks.pyplusROCKE_LLVM_FLAVOR=llvm23golden checks. - BLOCKER §K ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443· Classifyv_reg_prefetchinlibrary/dispatch/attention/dense_rules.py(e.g. add it toDENSE_UNTUNABLE_KNOBSalongsidelds_num_buffersor declare it as aKnobAxis), updatelibrary/tests/dispatch/attention/test_tuning_space.pyandlibrary/dispatch/AGENTS.md:187, and addv_reg_prefetchto_SPEC_PERTURBATIONSinlibrary/tests/test_attention_dense_gfx942.py. - HIGH §A ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443· Add v_reg_prefetch=True to the kdbvpf golden cases (or add dedicated vpf golden cases) in test_attention_dense_gfx942_golden.py and record their hashes in attention_dense_gfx942_ir_sha256.json for all LLVM flavors. - HIGH §B ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443· Updatetest_attention_dense_gfx942.py,test_attention_dense_gfx942_golden.py, andtest_attention_dense_gfx942_numeric.pyto test the decoupledlds_num_buffersandv_reg_prefetchknobs ((2, False),(1, True), and(2, True)along with'kdb1'/'vpf1'tags), or gatesupports_attention_denseto require matching knobs if only the combined mode is intended. - HIGH §C ·
gfx942_dense_cfvst_kdb_vprefetch_case_study.md:249· Updatetest_attention_dense_gfx942.py,test_attention_dense_gfx942_golden.py,test_attention_dense_gfx942_numeric.py,README.md, andAGENTS.mdto reflect the splitkdb1/vpf1tags and includev_reg_prefetchin_PRIVATE_PERTURBATIONS, type/mode rejection tests, and pipeline test cases. - HIGH §D ·
dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:114· Put the knob on the KernelSpec and delete the environment read. - HIGH §E ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:735· Update all stalekdbvpf1/_use_kdb_vpfreferences across tests and docs to usekdb1,vpf1, andv_reg_prefetch=True, remove_use_kdb_vpf, and remove the nonexistentrocke_vs_aiter/subdirectory fromgfx942_dense_cfvst_kdb_vprefetch_case_study.md. - HIGH §E ·
dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:3· Reconcile the PR description andgfx942_dense_cfvst_kdb_vprefetch_case_study.md:3-15with the recorded tables (10.74%-15.58% at the 12 measured configurations) and remove the stale 'Not yet recorded' text. - HIGH §G ·
dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1282· Addv_reg_prefetch=Truevariants (both alone withlds_num_buffers=1and combined withlds_num_buffers=2) totest_cfvst_pipeline_strict_numeric,_CFVST_KNOB_CASES, andtest_kdbvpf_pipeline_lseintest_attention_dense_gfx942_numeric.py. - HIGH §G ·
gfx942_dense_cfvst_kdb_vprefetch_case_study.md:106· Remove the stale "Not yet recorded" sentence and update the10.1-16.3%range to10.7-15.6%ingfx942_dense_cfvst_kdb_vprefetch_case_study.md:3-15, and updateREADME.md:194andtest_attention_dense_gfx942_golden.py:19-20to state thatlds_num_buffers=2andv_reg_prefetch=Trueare separate knobs. - HIGH §G ·
dnn-providers/hip-kernel-provider/rocke/library/dispatch/AGENTS.md:187· Wire the winning configuration (use_cfvst=True, lds_num_buffers=2, v_reg_prefetch=True) into the gfx942 dense selector/dispatch policy (and C++ twin if applicable) for the validated BF16 D128 causal shapes, or scope the PR title/claims strictly to adding experimental spec-only plumbing without claiming a shipped perf gain. - HIGH §G ·
dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1002· Updatetest_attention_dense_gfx942.py:353-380,1002-1024,test_attention_dense_gfx942_golden.py:284-298, andtest_attention_dense_gfx942_numeric.py:1286,1344-1386,1436to passv_reg_prefetch=Trueon the KDB+VPF cases and assert thekdb1/vpf1tags produced by_tuning_name_tags. - HIGH §K ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443· UpdateGfx942DenseKnobSpace.defaults()inlibrary/dispatch/attention/dense_rules.pyforv_reg_prefetch(and bumpTUNING_ID_VERSIONor document theconfig_keyimpact ofuse_cfvst=Trueon bf16 D128). - MEDIUM §D ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:735· Updatednn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.pyto add'v_reg_prefetch': (True, False)to_SPEC_PERTURBATIONS, setv_reg_prefetch=Trueon the*_kdbvpfbases/test cases, and assert on the'kdb1'and'vpf1'kernel-name tags instead of'kdbvpf1'. - MEDIUM §E ·
dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:473· Query the device throughrocke.runtime.hip_moduleinstead of importing torch. - MEDIUM §E ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443· Documentv_reg_prefetchalongsidelds_num_buffersinlibrary/builders/gfx942/attention/prefill/README.mdandlibrary/dispatch/AGENTS.md. - MEDIUM §F · summary · Retitle as
type(scope): summary, 10–80 characters, scope[a-z0-9-]+. - MEDIUM §G ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443· Add unit, golden, and numeric test cases that explicitly setv_reg_prefetch=True(withlds_num_buffers=1andlds_num_buffers=2) and verify type/mode rejection forv_reg_prefetch. - MEDIUM §G ·
dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:126· Fixplatform/Pythontoplatform/pythoninbench_rocke_5way_specs.py:126andgfx942_dense_cfvst_kdb_vprefetch_case_study.md:394, align the directory paths ingfx942_dense_cfvst_kdb_vprefetch_case_study.md:257-407withlibrary/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py, and run the script end-to-end from a clean shell. - MEDIUM §G ·
dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:74· Add SGPR counts and static ISA instruction-count deltas (e.g.,ds_read/ds_write,s_waitcnt,s_barrier) per variant to the register/codegen table ingfx942_dense_cfvst_kdb_vprefetch_case_study.md:82-89. - MEDIUM §J ·
dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:194· UpdateREADME.md:194andgfx942_dense_cfvst_kdb_vprefetch_case_study.md:3to name bothlds_num_buffers=2andv_reg_prefetch=True(or add a separate row inREADME.mdforv_reg_prefetch=True) so the docs match the independent symbols inGfx942AttentionDenseSpec. - MEDIUM §K ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443· Add"v_reg_prefetch": FalsetoGfx942DenseKnobSpace.defaults()inlibrary/dispatch/attention/dense_rules.py. - LOW §J ·
dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:7· Remove the stale 'Not yet recorded' sentence atgfx942_dense_cfvst_kdb_vprefetch_case_study.md:7-10and change the script storage paths at lines 257 and 313 from.../prefill/rocke_vs_aiter/{setup,run_5way}.shto.../prefill/{setup,run_5way}.shso they match thecddirectory andbench_rocke_5way_specs.pylocation.
Needs info
- §A.3 Not resolved in new commit (which only touched the case study markdown). dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443 adds v_reg_prefetch and updates supports_attention_dense (lines 1218-1267), while C++ selector/spec parity files are not in the diff and it remains unclear if Gfx942AttentionDenseSpec has a C++ selector twin.
- §E.14 The diff of dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py is truncated at line 2336 before the LSE epilogue merge resolution mentioned in the PR description.
- §F.7 Not resolved since previous review: local
pre-commitexecution status is not visible in the diff context, anddnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:194still has 3 blank lines beforedef _command_output. - §F.8 Not resolved since previous review: GitHub review thread states (bot/Copilot findings and resolution status) are not included in the provided PR diff context.
- §G.12 The diff touches only the Python kernel builder
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.pyand does not show whether any C++ builder or selector exists forattention_denseon gfx942. - §K.5 Unresolved from previous review:
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:506-527and:1218-1237allowuse_cfvst=Trueon bf16 D128 standard causal attention (changingresolved_v_row_padandresolved_use_v_swizzle), and we needlibrary/dispatch/attention/dense_rules.pyto verify whetherGfx942DenseKnobSpacedrop/refuse/inert rules foruse_cfvst,use_v_swizzle, andv_row_padon bf16 D128 remain consistent withassert_tuning_contract. - §K.6 Unresolved from previous review:
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:506-527and:1218-1267updateuse_cfvst,v_row_pad, anduse_v_swizzleresolution and support on bf16 D128, and we needlibrary/dispatch/attention/dense_rules.pyandlibrary/dispatch/attention/tuning_specs.pyto verify whetherKnobSpace.validateand policy-free spec construction match the new kernel support rules.
Section notes
- §A: The commit since the previous review only modified gfx942_dense_cfvst_kdb_vprefetch_case_study.md; items 3 and 10 remain unresolved.
- §B: The commit since the previous review (7d79470..4072b4a) only updated reproducibility instructions in the case study markdown. Item 5 remains FAIL because
v_reg_prefetchis still not covered in the unit, golden, or numeric test suites. - §C: The commit since the previous review only updated the case study markdown (
gfx942_dense_cfvst_kdb_vprefetch_case_study.md:249-410) and did not address item 10. - §D: Re-review of head 4072b4a: the commit since 7d79470 only updates gfx942_dense_cfvst_kdb_vprefetch_case_study.md, leaving previous FAIL items 1 and 5 unresolved.
- §E: Only
gfx942_dense_cfvst_kdb_vprefetch_case_study.mdchanged since the previous review; items 2, 7, 10, 13, and 17 remain unresolved, and item 14 remains truncated. - §G: Re-review of head 4072b4a. The single commit since 7d79470 only touched gfx942_dense_cfvst_kdb_vprefetch_case_study.md:228-412; items 2, 3, 4, 6, 11, and 17 remain unresolved.
Graded with the rocKE PR checklist, one model pass per section. FAIL rows that sit on a diff line are inline comments.
§A graded 14/14
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1383 modifies only the Python-only gfx942 dense attention kernel emitter… |
| 2 | N/A | No core lowering-op or op-handler files (core/lower_*, core/ir) are… |
| 3 | NEEDS-INFO | Not resolved in new commit (which only touched the case study markdown). dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443 adds v_reg_prefetch and updates supports_attention_dense (lines 1218-1267), while… |
| 4 | N/A | No C++ kernel or selector twin is modified or exists for… |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:9 leaves existing golden hashes (such as… |
| 6 | N/A | This PR modifies a Python-only kernel family (gfx942 attention_dense) that… |
| 7 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1571 and 2199 guard the new pipeline behind USE_PIPELINE (USE_KDB or… |
| 8 | N/A | No C++ files or C++-mirrored emitters are touched in this PR. |
| 9 | N/A | No shared op-handler fix or lowerer-only architecture divergence is… |
| 10 | FAIL | Not resolved in the latest commit (which only modified the case study doc). dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443 and 737 add v_reg_prefetch=True (emitting tag vpf1 and V-prefetch IR at line 2254) as an independent knob from lds_num_buffers=2, but… |
| 11 | N/A | No new IR ops are added in this PR. |
| 12 | N/A | core/backend.py and ROCKE_BACKEND=both handling are not modified in this PR. |
| 13 | N/A | LLVM IR lowering (.ll emission) and ROCKE_DEBUG_LOC handling are not… |
| 14 | N/A | No classic helpers under helpers/ or platform/cpp/helpers/ are modified in… |
§B graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1218-1267 classifies unvalidated feature combinations on BF16 CFVST and… |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1218-1267 and 1383-1395 keep supports_attention_dense as the single… |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:440-443 and 867 keep default routing and policies unchanged… |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:478-491 validates type(self.use_cfvst) is not bool… |
| 5 | FAIL | Not resolved in 7d79470..4072b4a (which only touched the case study markdown). dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443,735-738,1253-1267 opens v_reg_prefetch=True independently of lds_num_buffers=2 and emits 'kdb1'/'vpf1' tags, but… |
| 6 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1218-1267 updates supports_attention_dense to remove the old bf16 D128… |
§C graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Shared logic and predicates are factored into named helpers _use_kdb, _use_vpf, _use_kdb_vpf, and softmax_pv in… |
| 2 | PASS | The new pipeline variants are controlled via spec knobs (use_cfvst, lds_num_buffers, v_reg_prefetch) inside the single build_attention_dense(spec)… |
| 3 | PASS | Kernel naming uses codegen knob tags (kdb1, vpf1, cfvst1, vswz1) in `dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.… |
| 4 | PASS | All kernel changes are properly scoped inside the gfx942-specific module `dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py… |
| 5 | PASS | The diff touches only the gfx942 attention_dense kernel (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:114), its… |
| 6 | PASS | softmax_pv is cleanly extracted in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2053-2127 so the existing… |
| 7 | N/A | This PR is a targeted performance optimization adding opt-in spec knobs… |
| 8 | PASS | The PR extends existing Gfx942AttentionDenseSpec fields (use_cfvst, lds_num_buffers) and reuses existing DSL primitives (b.smem_alloc… |
| 9 | PASS | Type-coercion validation (type(...) is not bool/int) is applied across all new/modified spec knobs (use_cfvst, lds_num_buffers, v_reg_prefetch) in… |
| 10 | FAIL | Unresolved from previous review (the commit since 7d79470 only touched gfx942_dense_cfvst_kdb_vprefetch_case_study.md:249-410): in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:735-738 and :893-899, _tuning_name_tags was widened to emit independent kdb1 and… |
§D graded 21/21
| # | Verdict | Evidence |
|---|---|---|
| 1 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:114 reads the environment |
| 2 | N/A | No Capability declarations or candidate registrations are added or… |
| 3 | N/A | No candidate _supports or admits methods in the dispatcher layer are… |
| 4 | PASS | Spec helpers and supports_attention_dense(spec, arch) remain pure functions of the spec and explicit arch parameter with no host device probes… |
| 5 | FAIL | Unresolved since 7d79470 (test_attention_dense_gfx942.py was not modified): in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:735-738 and :893-899, _use_kdb and _use_vpf are independent knobs emitting 'kdb1' and 'vpf1' tags and _use_kdb_vpf(spec) requires… |
| 6 | N/A | No cohort routing changes are made in this PR; the new pipeline modes are… |
| 7 | N/A | No selector heuristics or hand-pinned routing overrides are added or… |
| 8 | N/A | No cohort routing predicates are introduced or changed in this PR. |
| 9 | N/A | No new KernelCandidate or architecture is registered in… |
| 10 | PASS | The new fields (lds_num_buffers=2, v_reg_prefetch=True) are internal codegen/scheduling knobs rather than user-facing problem/request features, and… |
| 11 | PASS | The BF16 CFVST, KDB, and VPF changes are strictly opt-in (use_cfvst=None resolves to False for BF16, lds_num_buffers=1, v_reg_prefetch=False by… |
| 12 | N/A | This PR does not add or rename any dispatcher candidate, change candidate… |
| 13 | N/A | This PR does not modify CandidateRegistry.select, pin resolution, or any… |
| 14 | N/A | This PR does not modify KernelId caching keys or change the kernel argument… |
| 15 | N/A | This PR does not add a new dispatcher candidate or architecture module to… |
| 16 | N/A | This PR does not introduce a new operator family. |
| 17 | N/A | This PR does not add a new dispatcher candidate or change problem-to-spec… |
| 18 | N/A | This PR does not modify registry ratchets, torch bindings, or test exempt… |
| 19 | PASS | dnn-providers/hip-kernel-provider/rocke/library/dispatch/AGENTS.md:187-192 and dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/pre… |
| 20 | N/A | This PR does not modify a problem-to-spec selector or a kernel family with… |
| 21 | N/A | This PR implements new opt-in kernel pipeline features (BF16 CFVST, K LDS… |
§E graded 17/17
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only modifies files under library/ and touches no files under… |
| 2 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:473 adds import torch |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:224 imports torch lazily inside @requires_gfx942_gpu test… |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:474 imports downward from kernels and… |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:29 is a Python script using pathlib.Path… |
| 6 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2258 preserves unused tuple unpacking with underscore prefixes (`_d0… |
| 7 | FAIL | Unresolved from previous review: dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443 adds the v_reg_prefetch spec knob, but dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:194 and… |
| 8 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:134 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attenti… |
| 9 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:440 and accompanying test/doc files are all scoped to the gfx942 BF16… |
| 10 | FAIL | Unresolved from previous review: dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:735-738 emits kdb1 and vpf1 tags and gates V prefetch on v_reg_prefetch=True, but dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1002-1022 (`pipeline =… |
| 11 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:1 and… |
| 12 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1218 explicitly scopes the opt-in knobs to the gfx942 dense kernel. |
| 13 | FAIL | Unresolved from previous review: dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:735 replaced kdbvpf1 with kdb1/vpf1, but dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1002-1022 still asserts _use_kdb_vpf(pipeline) and `'kdbvpf1' in… |
| 14 | NEEDS-INFO | The diff of dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py is truncated at line 2336 before the LSE epilogue merge resolution mentioned in the PR description. |
| 15 | PASS | All parameters in modified helpers in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1824 (_async_load… |
| 16 | N/A | This item governs reviewer severity grading rather than code in the pull… |
| 17 | FAIL | Unresolved from previous review: dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:3-11 still omits v_reg_prefetch=True, states that the baseline arm's full spec and pre-timing correctness tolerance are 'Not yet recorded'… |
§F graded 12/12
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Branch users/Parthiv911/rocke/gfx942-dense-cfvst-kdb-vpf matches the recognized users/<user>/... prefix… |
| 2 | PASS | branch users/Parthiv911/rocke/gfx942-dense-cfvst-kdb-vpf has a rocke path segment |
| 3 | FAIL | title perf(rocke): bf16 D128 CFVST, K double buffering, and V prefetch in gfx942 dense prefill is 88 characters and does not match type(scope): summary with a 10–80 character length |
| 4 | PASS | description is at least 30 characters and contains an issue reference |
| 5 | PASS | test file dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py is in the diff |
| 6 | PASS | All 10 changed files are Python, Markdown, or JSON golden hash files (e.g., `dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gf… |
| 7 | NEEDS-INFO | Not resolved since previous review: local pre-commit execution status is not visible in the diff context, and dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:194 still has 3… |
| 8 | NEEDS-INFO | Not resolved since previous review: GitHub review thread states (bot/Copilot findings and resolution status) are not included in the provided PR diff context. |
| 9 | N/A | The PR base branch is develop, so this is not a stacked PR targeting a… |
| 10 | PASS | The PR keeps the kernel changes, tests, benchmark script, and case study together without artificial size-cap splitting… |
| 11 | PASS | Added tests exercise real functionality (spec validation, LDS budget, golden IR hashes, and numeric accuracy) in… |
| 12 | PASS | The PR title and body are populated in the PR metadata above and match the files in the diff (`dnn-providers/hip-kernel-provider/rocke/library/builders/gfx9… |
§G graded 34/34
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1252 and… |
| 2 | FAIL | Unresolved since previous review: dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1282-1288 and 1344-1348 test only lds_num_buffers in {1, 2} and never set v_reg_prefetch=True, leaving the v_reg_prefetch code path (USE_VPF at… |
| 3 | FAIL | Unresolved since previous review: dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1008 asserts assert pipeline.resolved_use_cfvst() and _use_kdb_vpf(pipeline) and line 1022 asserts assert "kdbvpf1" in gfx942_kernel_name(pipeline) for `pipeline = _spec(use_cfvst=True… |
| 4 | FAIL | Unresolved since previous review: dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443 adds the independent v_reg_prefetch: bool = False spec field and USE_VPF codegen branch (line 2254), but… |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1226 uses _MULTI_ITEM_NP = 3 and sq=256, skv=192 (line… |
| 6 | FAIL | Partially resolved (PR body table now matches gfx942_dense_cfvst_kdb_vprefetch_case_study.md:106-138), but dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:7-10 still claims the baseline arm's full spec and pre-timing correctness… |
| 7 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:164-182 and… |
| 8 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:101-102 explicitly rejects any dtype other… |
| 9 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:76-88 sources VGPR count… |
| 10 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:102-118 and 218-231 lock… |
| 11 | FAIL | Unresolved since previous review: dnn-providers/hip-kernel-provider/rocke/library/dispatch/AGENTS.md:187-193 and dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:191-194 leave use_cfvst=True (for bf16 D128), lds_num_buffers=2, and v_reg_prefetch=True off by… |
| 12 | NEEDS-INFO | The diff touches only the Python kernel builder dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py and does not show whether any C++ builder or selector exists for attention_dense on gfx942. |
| 13 | N/A | This PR does not introduce or recommend any new LLVM intrinsic .td… |
| 14 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:19 and 98 state that… |
| 15 | N/A | The review is graded directly from the provided PR diff against head… |
| 16 | N/A | This PR ships a completed implementation and benchmark case study rather… |
| 17 | FAIL | Not resolved by the latest commit: dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:126 and the newly added run_5way.sh snippet in… |
| 18 | PASS | The validation guard in dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:669-684 and… |
| 19 | PASS | Fixtures in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1226-1237 (_multi_item_persistent with… |
| 20 | N/A | This PR does not add or modify a text detector or linter enforcing a ban on… |
| 21 | FAIL | Unresolved from previous review: in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1002-1023, test_bf16_cfvst_kdb_vpf_are_separate_opt_in_variants sets pipeline = _spec(use_cfvst=True, lds_num_buffers=2) (leaving v_reg_prefetch=False), which makes… |
| 22 | PASS | Every new golden fixture entry in `dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:5-8,61-64,69-72,101-10… |
| 23 | PASS | The case study `dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:105-118,233… |
| 24 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:5-104,153-252, no pre-existing golden hashes were… |
| 25 | FAIL | Unresolved from previous review: in dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:74-96, VGPR, spill count, and scratch bytes before and after are reported alongside the 12-shape corpus A/B (105-139), but static ISA… |
| 26 | PASS | The case study dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:233-249… |
| 27 | PASS | Both dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:45-51 and… |
| 28 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:186-195 and… |
| 29 | N/A | This PR does not swap any guarded primitive for a fast primitive… |
| 30 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:76-95, spill and… |
| 31 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:517-561, the baseline is the exact… |
| 32 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:76-89,215-231,250-409… |
| 33 | PASS | The changed spec resolvers (resolved_use_cfvst, resolved_v_row_pad, resolved_use_v_swizzle, _lds_bytes, kernel_name) are evaluated and… |
| 34 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1426-1449, test_kdbvpf_pipeline_lse runs the new… |
§H graded 5/5
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only modifies the dense prefill kernel (attention_dense.py) to… |
| 2 | N/A | This PR applies to dense prefill attention and does not use page tables… |
| 3 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2210-2211 and… |
| 4 | N/A | This PR does not construct or modify any device-side mask or bound from… |
| 5 | N/A | No new launch runtime inputs that feed device-side bounds or masks are… |
§I graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:26 |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:478 |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:348 |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:105 |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:305 |
| 6 | N/A | This PR does not add or modify any DSL primitive or hardware instruction… |
§J graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Scalar latency-reduction formulas in `dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_cas… |
| 2 | PASS | All formula symbols (T_variant, t_variant,r, T_after, T_before, R_r, Min, Max) are defined in-place with consistent domains in… |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:188-195 links to… |
| 4 | FAIL | Not fixed in the new commit: dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:194 and dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:3 were not modified since the previous review and… |
| 5 | N/A | The PR does not make asymptotic scaling or big-O complexity claims in its… |
| 6 | FAIL | Still not fixed: dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:7-10 still retains the contradictory stale sentence ('Not yet recorded: the baseline arm's full spec ... and the tolerance') even though those exact constants and… |
| 7 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:188-190 serves as the directory entry point and links to the… |
§K graded 13/13
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR does not add or modify a tuned candidate or KnobSpace subclass. |
| 2 | FAIL | Unresolved from previous review (the commit since 7d794701d366 only touched gfx942_dense_cfvst_kdb_vprefetch_case_study.md): dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443 adds a new dataclass field v_reg_prefetch: bool = field(default=False, kw_only=True) to… |
| 3 | FAIL | Unresolved from previous review: dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443 adds v_reg_prefetch: bool = field(default=False, kw_only=True) to Gfx942AttentionDenseSpec and :509-527 changes resolved_v_row_pad() and resolved_use_v_swizzle() when… |
| 4 | FAIL | Unresolved from previous review: dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443 adds the problem-independent default field v_reg_prefetch: bool = field(default=False, kw_only=True) on Gfx942AttentionDenseSpec, which is not added to Gfx942DenseKnobSpace.defaults() in… |
| 5 | NEEDS-INFO | Unresolved from previous review: dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:506-527 and :1218-1237 allow use_cfvst=True on bf16 D128 standard causal attention (changing resolved_v_row_pad and… |
| 6 | NEEDS-INFO | Unresolved from previous review: dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:506-527 and :1218-1267 update use_cfvst, v_row_pad, and use_v_swizzle resolution and support on bf16 D128, and we… |
| 7 | N/A | KnobSpace.base(req) is not modified in this PR. |
| 8 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:478-491 validates use_cfvst, lds_num_buffers, and v_reg_prefetch… |
| 9 | N/A | The benchmark added in `dnn-providers/hip-kernel-provider/rocke/library/benc… |
| 10 | N/A | KnobSpace.find and bare tuning_id resolution are not touched by this PR. |
| 11 | N/A | No sweep entry points (iter_dispatch_*, pin_to_spec, sweep_space) are… |
| 12 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:440-443 and :867 keep lds_num_buffers=1, v_reg_prefetch=False… |
| 13 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:583-586 delegates legality checking to… |
§L graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR does not add or modify a gfx target in arch_specs.json or the C++… |
| 2 | N/A | The PR does not add or change any MMA op_id or fragment layout in… |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1960 continues to use the existing `mfma_32x32x8_for_dtype(b, dtype… |
| 4 | N/A | The PR is scoped to the existing gfx942 kernel and does not modify… |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1824 implements gfx942 K LDS double-buffering and V register prefetching… |
| 6 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:440 keeps the pipeline knobs (lds_num_buffers, v_reg_prefetch) on… |
| 7 | N/A | The PR does not add or modify logical dtypes, bit-packing contracts, host… |
| 8 | N/A | The PR operates on unscaled bf16/fp16 attention and does not touch… |
| 9 | N/A | The PR does not introduce a new architecture target. |
| 10 | N/A | The PR does not modify intrinsic_arch_domain.<flavor>.json artifacts or… |
§M graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Host booleans (USE_PIPELINE, USE_KDB, USE_VPF, masked) drive Python control flow while SSA loop carries (m_new, l_new, *o_acc) are yielded… |
| 2 | N/A | This PR does not touch legalize_for_target or any target legalization… |
| 3 | N/A | This PR does not modify print_ir or the ck.dsl.ir/v1 serialization… |
| 4 | N/A | This PR does not modify portable IR schemas, importers, magic division… |
| 5 | PASS | Async K tile loads compute an unswizzled wave-uniform destination offset per row and consumers drain VMEM via b.s_waitcnt(vmcnt=0) followed by… |
| 6 | PASS | All barriers (b.sync_lds_only(), b.s_waitcnt(vmcnt=0)) are emitted unconditionally in the workgroup-uniform prologue, scf_for_iter body, and epilogue… |
| 7 | N/A | This PR does not touch ISABackend.buffer_rsrc_word3, hard-code buffer… |
| 8 | N/A | This PR does not add intrinsic declarations or modify lowering backend… |
| 9 | N/A | This PR does not touch C entry points, headers under cpp/include/rocke/… |
| 10 | N/A | This PR does not modify !dbg emission, module debug info flags, or DWARF… |
§N graded 18/18
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR does not add or promote any helpers under… |
| 2 | N/A | The PR modifies an existing lower-level DSL attention kernel and does not… |
| 3 | N/A | The PR implements a custom K-LDS double-buffer and V-register-prefetch loop… |
| 4 | N/A | The PR optimizes an existing attention prefill kernel and does not add or… |
| 5 | N/A | The PR does not add or modify any public API in rocke.helpers.tiling. |
| 6 | N/A | The PR does not add or modify front-door/toolbox tiling helpers or… |
| 7 | N/A | The PR does not use helpers/tiling layout overrides or modify tiling MMA… |
| 8 | N/A | The PR does not touch helpers/tiling memory verbs (lds_tile_alloc… |
| 9 | N/A | The PR does not use helpers/tiling N-D tensor axis roles, FlowStage, or… |
| 10 | N/A | The PR does not modify helpers/tiling emit, LDS staging, or MMA issue… |
| 11 | N/A | The PR does not use TileMma, Tiling, lds_swizzle, or… |
| 12 | N/A | The PR does not make helpers/tiling bank-conflict simulator or… |
| 13 | N/A | The PR does not construct WarpDistributionEncoding, call… |
| 14 | PASS | The new double-buffered K / V-prefetch pipeline explicitly places b.sync_lds_only() and b.s_waitcnt(vmcnt=0) around LDS reads and writes… |
| 15 | PASS | LDS allocation and budget checks account for the padded/swizzled row stride and buffer count (`dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx94… |
| 16 | N/A | The PR does not use optimize_layout or evaluate_transform from… |
| 17 | N/A | The PR does not touch tiling_recorder.py, analysis/roundtrip.py, or… |
| 18 | N/A | The PR does not touch InterleavedStyle or demo kernels under… |
§O graded 16/16
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only modifies files under library/ and does not touch platform core… |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2229 — the new scf_for_iter loop uses i32 bounds/step, yields matching… |
| 3 | N/A | This PR does not modify IRBuilder, intrinsic declaration tables, or… |
| 4 | N/A | This PR does not touch IR type definitions, SmemType, or IR parsers in… |
| 5 | N/A | This PR does not touch core/passes.py, cpp/core/passes.cpp, or opcode… |
| 6 | N/A | This PR does not modify constant folding passes in Python or C++ core. |
| 7 | N/A | This PR does not modify kernel attribute lowering, CodegenPolicy… |
| 8 | N/A | This PR does not set or modify SchedulerStrategy or CodegenPolicy. |
| 9 | N/A | This PR does not modify platform waitcnt or barrier lowering logic (and… |
| 10 | N/A | This PR does not modify platform lowerer attribute validation or parameter… |
| 11 | N/A | This PR does not touch ArchTarget or target resolution in platform core. |
| 12 | N/A | This PR does not modify platform core width/size caps or arch default… |
| 13 | N/A | This PR does not touch any C headers or structs under cpp/include/rocke/. |
| 14 | N/A | This PR does not add or modify any C/C++ files or CMake files. |
| 15 | N/A | This PR does not touch C++ engine fallback logic or backend flavor… |
| 16 | N/A | This PR does not touch C++ runtime/codegen gap areas or gfx1250 builders. |
§P graded 14/14
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:440-443 adds defaulted keyword-only fields (`lds_num_buffers: int =… |
| 2 | N/A | This PR does not modify kernarg ordering, signature(), manifest… |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:553-556 takes… |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1075 charges k_bytes *= spec.lds_num_buffers in _lds_bytes matching… |
| 5 | N/A | This PR does not touch WMMA/RDNA GEMM or GemmMultiD/GemmMultiAbd… |
| 6 | N/A | This PR does not use or modify caller-zeroed workspaces (Stream-K, MoE… |
| 7 | N/A | This PR does not touch Stream-K, persistent GEMM persistent_max_iters… |
| 8 | N/A | This PR does not touch quantization helpers or MX GEMM. |
| 9 | N/A | This PR does not introduce numeric emulation or quantized decode paths. |
| 10 | N/A | This PR does not add or modify manifest kinds, emitters, or manifest runners. |
| 11 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:534-556 compiles all kernels ahead of timing… |
| 12 | N/A | This PR does not touch COMGR or HIP library loading logic or fix a… |
| 13 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:735-738 and 1248-1267 add cache-name tags (kdb1, vpf1) and admission… |
| 14 | N/A | This PR does not touch examples/, expected.json… |
§Q graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | No new test files are added or removed in this PR; only existing test files… |
| 2 | N/A | This PR modifies the Python-only library/kernels/gfx942/attention_dense.py… |
| 3 | PASS | Golden hashes in dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:5 and… |
| 4 | PASS | New golden entries are recorded for both llvm20 (dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:5)… |
| 5 | PASS | CPU tests in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1052 use values the parser genuinely rejects (e.g.… |
| 6 | N/A | This PR does not refactor any platform gate or test harness. |
| 7 | N/A | This PR only updates library spec validation in supports_attention_dense… |
This was referenced Oct 19, 2026
yraparti
reviewed
Oct 19, 2026
yraparti
left a comment
yraparti
left a comment
Contributor
There was a problem hiding this comment.
rocKE checklist re-review of the fixes since 4072b4ae138a
ROCm/rocm-libraries#13205 · bbf25121809c · model gemini-4-argon, fallback gpt-5.5
PASS · graded 220/220 · BLOCKER 0, HIGH 2, MEDIUM 3, LOW 1, NEEDS-INFO 7
The diff was truncated before grading. Items that needed a missing hunk are NEEDS-INFO.
Since the last review (4072b4ae138a)
Resolved 19 · still open 5 · new 1
- ✅ §A.10 was FAIL, now PASS — had asked: Add v_reg_prefetch=True to the kdbvpf golden cases (or add dedicated vpf golden cases) in test_attention_dense_gfx942_golden.py and record their hashes in attention_dense_gfx942_ir_sha256.json for all LLVM flavors.
- ✅ §B.5 was FAIL, now PASS — had asked: Update
test_attention_dense_gfx942.py,test_attention_dense_gfx942_golden.py, andtest_attention_dense_gfx942_numeric.pyto test the decoupledlds_num_buffersandv_reg_prefetchknobs ((2, False),(1, True), and(2, True)along with'kdb1'/'vpf1'tags), or gate… - ✅ §C.10 was FAIL, now PASS — had asked: Update
test_attention_dense_gfx942.py,test_attention_dense_gfx942_golden.py,test_attention_dense_gfx942_numeric.py,README.md, andAGENTS.mdto reflect the splitkdb1/vpf1tags and includev_reg_prefetchin_PRIVATE_PERTURBATIONS, type/mode rejection tests, and pipeline test… - ✅ §D.5 was FAIL, now PASS — had asked: Update
dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.pyto add'v_reg_prefetch': (True, False)to_SPEC_PERTURBATIONS, setv_reg_prefetch=Trueon the*_kdbvpfbases/test cases, and assert on the'kdb1'and'vpf1'kernel-name tags instead of… - ✅ §E.7 was FAIL, now PASS — had asked: Document
v_reg_prefetchalongsidelds_num_buffersinlibrary/builders/gfx942/attention/prefill/README.mdandlibrary/dispatch/AGENTS.md. - ✅ §E.10 was FAIL, now PASS — had asked: Update the test specs, expected kernel-name tags (
kdb1,vpf1), and golden hashes intest_attention_dense_gfx942.py,test_attention_dense_gfx942_golden.py,test_attention_dense_gfx942_numeric.py, andattention_dense_gfx942_ir_sha256.jsonto passv_reg_prefetch=Truewhere the full… - ✅ §E.13 was FAIL, now PASS — had asked: Update all stale
kdbvpf1/_use_kdb_vpfreferences across tests and docs to usekdb1,vpf1, andv_reg_prefetch=True, remove_use_kdb_vpf, and remove the nonexistentrocke_vs_aiter/subdirectory fromgfx942_dense_cfvst_kdb_vprefetch_case_study.md. - ✅ §E.17 was FAIL, now PASS — had asked: Reconcile the PR description and
gfx942_dense_cfvst_kdb_vprefetch_case_study.md:3-15with the recorded tables (10.74%-15.58% at the 12 measured configurations) and remove the stale 'Not yet recorded' text. - ✅ §G.2 was FAIL, now PASS — had asked: Add
v_reg_prefetch=Truevariants (both alone withlds_num_buffers=1and combined withlds_num_buffers=2) totest_cfvst_pipeline_strict_numeric,_CFVST_KNOB_CASES, andtest_kdbvpf_pipeline_lseintest_attention_dense_gfx942_numeric.py. - ✅ §G.3 was FAIL, now PASS — had asked: Update
test_attention_dense_gfx942.pyto add"v_reg_prefetch": (True, False)to_SPEC_PERTURBATIONS, testkdb1/vpf1tags andv_reg_prefetch=Trueintest_bf16_cfvst_kdb_vpf_are_separate_opt_in_variants, and re-runtools/run_checks.pyplusROCKE_LLVM_FLAVOR=llvm23golden checks. - ✅ §G.4 was FAIL, now PASS — had asked: Add unit, golden, and numeric test cases that explicitly set
v_reg_prefetch=True(withlds_num_buffers=1andlds_num_buffers=2) and verify type/mode rejection forv_reg_prefetch. - ✅ §G.6 was FAIL, now PASS — had asked: Remove the stale "Not yet recorded" sentence and update the
10.1-16.3%range to10.7-15.6%ingfx942_dense_cfvst_kdb_vprefetch_case_study.md:3-15, and updateREADME.md:194andtest_attention_dense_gfx942_golden.py:19-20to state thatlds_num_buffers=2andv_reg_prefetch=Trueare… - ✅ §G.17 was FAIL, now PASS — had asked: Fix
platform/Pythontoplatform/pythoninbench_rocke_5way_specs.py:126andgfx942_dense_cfvst_kdb_vprefetch_case_study.md:394, align the directory paths ingfx942_dense_cfvst_kdb_vprefetch_case_study.md:257-407with… - ✅ §G.21 was FAIL, now PASS — had asked: Update
test_attention_dense_gfx942.py:353-380,1002-1024,test_attention_dense_gfx942_golden.py:284-298, andtest_attention_dense_gfx942_numeric.py:1286,1344-1386,1436to passv_reg_prefetch=Trueon the KDB+VPF cases and assert thekdb1/vpf1tags produced by_tuning_name_tags. - ✅ §J.4 was FAIL, now PASS — had asked: Update
README.md:194andgfx942_dense_cfvst_kdb_vprefetch_case_study.md:3to name bothlds_num_buffers=2andv_reg_prefetch=True(or add a separate row inREADME.mdforv_reg_prefetch=True) so the docs match the independent symbols inGfx942AttentionDenseSpec. - ⏳ §D.1 still FAIL (HIGH): Put the knob on the KernelSpec and delete the environment read.
- ⏳ §E.2 still FAIL (MEDIUM): Query the device through
rocke.runtime.hip_moduleinstead of importing torch. - ⏳ §F.3 still FAIL (MEDIUM): Retitle as
type(scope): summary, 10–80 characters, scope[a-z0-9-]+. - ⏳ §G.11 still FAIL (HIGH): Include the (use_cfvst=True, lds_num_buffers=2, v_reg_prefetch=True) combination in the production tuning sweep or default selector policy for validated BF16 D128 causal shapes, or explicitly scope the PR title and summary to opt-in tuning/spec plumbing rather than a shipped default latency reduction.
- ⏳ §G.25 still FAIL (MEDIUM): Add SGPR counts and static ISA instruction-count deltas (e.g.,
ds_read/ds_write,s_waitcnt,s_barrier) per variant to the register/codegen table ingfx942_dense_cfvst_kdb_vprefetch_case_study.md:137-144. - 🆕 §F.7 new FAIL (LOW): Run
pre-commit run --all-files(orblackandend-of-file-fixer) on all touched files and commit the formatting and trailing-newline fixes.
Findings
- HIGH §D ·
dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:282· Put the knob on the KernelSpec and delete the environment read. - HIGH §G ·
dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:265· Include the (use_cfvst=True, lds_num_buffers=2, v_reg_prefetch=True) combination in the production tuning sweep or default selector policy for validated BF16 D128 causal shapes, or explicitly scope the PR title and summary to opt-in tuning/spec plumbing rather than a shipped default latency reduction. - MEDIUM §E ·
dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:471· Query the device throughrocke.runtime.hip_moduleinstead of importing torch. - MEDIUM §F · summary · Retitle as
type(scope): summary, 10–80 characters, scope[a-z0-9-]+. - MEDIUM §G ·
dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:129· Add SGPR counts and static ISA instruction-count deltas (e.g.,ds_read/ds_write,s_waitcnt,s_barrier) per variant to the register/codegen table ingfx942_dense_cfvst_kdb_vprefetch_case_study.md:137-144. - LOW §F ·
dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:532· Runpre-commit run --all-files(orblackandend-of-file-fixer) on all touched files and commit the formatting and trailing-newline fixes.
Needs info
- §A.3 dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443 adds v_reg_prefetch and updates supports_attention_dense (lines 1209-1258); while library/dispatch/ changes are Python-only by design, C++ selector/spec parity files are not in the diff to confirm whether Gfx942AttentionDenseSpec has a C++ selector twin.
- §B.6 dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1209-1258 updates
supports_attention_denseto remove the old bf16 D128 CFVST spill policy rejection and instead gates only on hardware/algorithmic invariants (rows-per-DMA==1 at lines 1223-1228) andNOT_YET_IMPLEMENTEDunvalidated feature combinations, while tuning scope lives indnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/dense_rules.py:94-105. — no diff line supports this verdict - §C.1 Shared predicates and pipeline logic are factored into named helpers
_use_kdb,_use_vpf,_gfx942_cfvst_tuning_scope, andsoftmax_pvindnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:883-890,dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2044-2118, anddnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/dense_rules.py:94-105. — no diff line supports this verdict - §C.9 Exact-type validation (
type(...) is not bool/int) is applied across all three spec knobs (use_cfvst,lds_num_buffers,v_reg_prefetch) indnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:478-491, andsliding_window,causal_diag_split, anduse_sinksrejections are enforced across both CFVST and KDB/VPF gates indnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1209-1258anddnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/dense_rules.py:94-105. — no diff line supports this verdict - §E.14 The diff of dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py is truncated at line 2333 right before the LSE epilogue merge resolution.
- §F.8 Not resolved since previous review: GitHub review thread states (bot/Copilot findings and resolution status) are not included in the provided PR diff context.
- §G.12 The diff touches only the Python kernel builder dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py and Python dispatch files, and does not show whether any C++ builder or selector exists for attention_dense on gfx942.
Section notes
- §A: Resolved item 10 by adding independent KDB, VPF, and combined KDB+VPF golden cases across default, persistent, and LSE paths.
- §C: All items in Section C pass; the previous FAIL on item 10 has been resolved by updating all tests, golden hashes, dispatch tuning axes/rules, and documentation for the independent
lds_num_buffers(_kdb1) andv_reg_prefetch(_vpf1) knobs. - §E: Re-review of head bbf2512: items 7, 10, 13, and 17 were resolved in the new commits; item 2 remains FAIL and item 14 remains NEEDS-INFO due to diff truncation.
- §H: Trace of device-side bounds/masks in the diff: 1. Pipeline KV tile loop bound (
n_up/last_j = n_up - 1): extent comes from the existing causal tile boundn_up(unchanged in this PR). The pipeline visits tile0in the prologue, tiles[0, n_up - 1)in thescf_for_iterloop (attention_dense.py:2220-2227), and tilen_up - 1in the epilogue (attention_dense.py:2281-2285), issuing K/V loads only for0..n_up - 1. Each tilejappliesdo_mask(s, j, lower=False, upper=True)viasoftmax_pv(attention_dense.py:2057,2251,2283), matching the visited tile range. 2. LDS K buffer slot index (kbuf = j % K_BUFS,next_kbuf = (j + 1) % K_BUFS,last_kbuf = last_j % K_BUFSatattention_dense.py:2238,2241,2281): extent isK_BUFS = spec.lds_num_buffers(attention_dense.py:1563), matching theK_ldsallocation[K_BUFS, BN, LDROW](attention_dense.py:1575). 3. No paged/indirection tables (block_tables) or runtime varlen arrays (kv_lens… - §J: Shapes: | Equation / Allocation | Operands & Inner Dimensions | Result vs Declared | |---|---|---| |
T_variant = median(t_variant,1, ..., t_variant,5)(case_study.md:245) | 5 scalar round latency samples -> scalar median | Scalar latency | |Reduction (%) = 100 * (1 - T_after / T_before)(case_study.md:256) | Scalar / scalar -> dimensionless percentage | Scalar percentage | |R_r = 100 * (1 - T_full,r / T_baseline,r)(case_study.md:260) | Per-round scalar / scalar -> dimensionless percentage | Scalar percentage | Symbols: | Symbol | Defined at | Domain | |---|---|---| |B, S, Sq, Skv, Hq, Hkv, D|case_study.md:29-40| Positive integers (tensor extents) | |T_variant, t_variant,r|case_study.md:242-246| Positive real GPU latency per launch | |R_r, Min, Max|case_study.md:258-266| Real percentage latency reduction (-inf, 100] | |use_cfvst, lds_num_buffers, v_reg_prefetch|README.md:193-195,case_study.md:3,207,AGENTS.md:183-197| Spec knobs… - §K: Identity table for gfx942_dense tuning changes: -
v_reg_prefetch: added toGfx942AttentionDenseSpec(defaultFalse) and_GFX942_DENSE_AXES(_flag("v_reg_prefetch")); adds"v_reg_prefetch": Falsetodefaults(), updating the defaults fingerprint and re-keyinggfx942_denseconfig_keys. -lds_num_buffers: moved fromDENSE_UNTUNABLE_KNOBS["gfx942"]to_GFX942_DENSE_AXES(_values("lds_num_buffers", 1, (2,))); default remains1. -use_cfvst,use_v_swizzle,v_row_pad: scope in_DENSE_ARCH["gfx942"]updated via_gfx942_cfvst_tuning_scopeto include BF16 D128 standard causal attention; defaults remainNone(resolving toFalse/False/8on BF16 D128).
Graded with the rocKE PR checklist, one model pass per section. FAIL rows that sit on a diff line are inline comments.
§A graded 14/14
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1374 modifies only the Python-only gfx942 dense attention kernel emitter (the C++ attention builder is for unified_attention_2d_tiled_scalar). |
| 2 | N/A | No core lowering-op or op-handler files (core/lower_*, core/ir) are modified in this PR. |
| 3 | NEEDS-INFO | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:443 adds v_reg_prefetch and updates supports_attention_dense (lines 1209-1258); while library/dispatch/ changes are Python-only by design, C++ selector/spec parity files are not in the diff to confirm whether Gfx942AttentionDenseSpec has a C++ selector twin. |
| 4 | N/A | No C++ kernel or selector twin is modified in this PR. |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:9 leaves existing golden hashes (such as default_d128_bf16_causal) unchanged while adding new opt-in entries. |
| 6 | N/A | This PR modifies a Python-only kernel family (gfx942 attention_dense) that has no C++ kernel emitter twin for cross-engine… |
| 7 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1562 and 2190 guard the new pipeline behind USE_PIPELINE (USE_KDB or USE_VPF) and guard kbuf offsets behind kbuf is not None at line 1818, keeping covered default shapes byte-identical. |
| 8 | N/A | No C++ files or C++-mirrored emitters are touched in this PR. |
| 9 | N/A | No shared op-handler fix or lowerer-only architecture divergence is introduced in this PR. |
| 10 | PASS | Resolved since previous review: dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_golden.py:284-327 and dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:5-169 now record golden hashes… |
| 11 | N/A | No new IR ops are added in this PR. |
| 12 | N/A | core/backend.py and ROCKE_BACKEND=both handling are not modified in this PR. |
| 13 | N/A | LLVM IR lowering (.ll emission) and ROCKE_DEBUG_LOC handling are not modified in this PR. |
| 14 | N/A | No classic helpers under helpers/ or platform/cpp/helpers/ are modified in this PR. |
§B graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1209-1258 classifies unvalidated feature combinations on BF16 CFVST and lds_num_buffers=2/v_reg_prefetch=True with explicit NOT_YET_IMPLEMENTED: prefixes, distinguishing them from… |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1209-1258 and 1374-1386 keep supports_attention_dense as the single functional gate consulted by build_attention_dense, while Gfx942AttentionDenseSpec.__post_init__ (lines 478-495)… |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:440-443 and 867 keep default routing and policies unchanged (lds_num_buffers=1, v_reg_prefetch=False, _use_cfvst False for bf16), as verified in… |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:478-491 validates type(self.use_cfvst) is not bool, type(self.lds_num_buffers) is not int, and type(self.v_reg_prefetch) is not bool in Gfx942AttentionDenseSpec.__post_init__… |
| 5 | PASS | Resolved since previous review: dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:326-327,1005-1064,1120-1142,2091-2093, dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_golden.py:284-327… |
| 6 | NEEDS-INFO | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1209-1258 updates supports_attention_dense to remove the old bf16 D128 CFVST spill policy rejection and instead gates only on hardware/algorithmic invariants (rows-per-DMA==1 at lines 1223-1228) and NOT_YET_IMPLEMENTED unvalidated feature combinations, while tuning scope lives in… |
§C graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | NEEDS-INFO | Shared predicates and pipeline logic are factored into named helpers _use_kdb, _use_vpf, _gfx942_cfvst_tuning_scope, and softmax_pv in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:883-890, dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2044-2118, and… |
| 2 | PASS | All pipeline variants are controlled via spec knobs (use_cfvst, lds_num_buffers, v_reg_prefetch) inside the single build_attention_dense(spec) builder in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1374-1444, and the… |
| 3 | PASS | Kernel naming emits codegen knob tags (kdb1, vpf1, cfvst1, vswz1) in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:735-749 without embedding workload dimensions. |
| 4 | PASS | All kernel and dispatch changes are scoped to gfx942-specific modules/entries (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:353-443 and dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:264-270) without… |
| 5 | PASS | The diff is tightly scoped to the gfx942 dense attention kernel (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:114), gfx942 tuning axes/rules (dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:171)… |
| 6 | PASS | softmax_pv is factored out in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2044-2118 so the existing single-buffer tile_body remains byte-identical (existing golden hashes unchanged in… |
| 7 | N/A | This PR is a targeted performance optimization adding opt-in spec knobs rather than a standalone cleanup refactor. |
| 8 | PASS | The PR extends Gfx942AttentionDenseSpec (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:440-443), reuses existing DSL primitives (b.smem_alloc, b.sync_lds_only, _cfvst_load_v, _cfvst_store_v) in… |
| 9 | NEEDS-INFO | Exact-type validation (type(...) is not bool/int) is applied across all three spec knobs (use_cfvst, lds_num_buffers, v_reg_prefetch) in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:478-491, and sliding_window, causal_diag_split, and use_sinks rejections are enforced across both CFVST and KDB/VPF gates in… |
| 10 | PASS | Resolved from previous review: commits 4072b4ae138a..bbf25121809c removed the dead _use_kdb_vpf helper and updated all code, tests, goldens, dispatch rules, and docs for the widened spec (v_reg_prefetch and split _kdb1/_vpf1 tags) in… |
§D graded 21/21
| # | Verdict | Evidence |
|---|---|---|
| 1 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:282 reads the environment |
| 2 | N/A | No Capability declarations or candidate registrations are added or modified in this PR. |
| 3 | N/A | No candidate _supports or admits methods in the dispatcher layer are touched by this PR. |
| 4 | PASS | Spec helpers and supports_attention_dense(spec, arch) remain pure functions of the spec and explicit arch parameter with no host device probes (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1207-1258). |
| 5 | PASS | Resolved since previous review: _use_kdb_vpf was removed from dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:883-890, _use_kdb and _use_vpf emit distinct 'kdb1' and 'vpf1' tags in _tuning_name_tags… |
| 6 | N/A | No cohort routing changes are made in this PR; the new pipeline modes are spec-level opt-ins that leave grid and block launch… |
| 7 | N/A | No selector heuristics or hand-pinned routing overrides are added or modified in this PR. |
| 8 | N/A | No cohort routing predicates are introduced or changed in this PR. |
| 9 | N/A | No new KernelCandidate or architecture is registered in ATTENTION_ROUTE_REGISTRY or ATTENTION_EXECUTION_REGISTRY. |
| 10 | PASS | The new fields (lds_num_buffers=2, v_reg_prefetch=True) are internal codegen/scheduling knobs rather than user-facing problem/request features, and are reachable through dispatcher tuning axes… |
| 11 | PASS | The BF16 CFVST, KDB, and VPF changes remain strictly opt-in (use_cfvst=None resolves to False for BF16, lds_num_buffers=1, v_reg_prefetch=False by default in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:374, :440… |
| 12 | N/A | This PR does not add or rename any dispatcher candidate, change candidate priority, or introduce a custom ranker. |
| 13 | N/A | This PR does not modify CandidateRegistry.select, pin resolution, or any dispatch wrapper entry point. |
| 14 | N/A | This PR does not modify KernelId caching keys or change the kernel argument ABI layout. |
| 15 | N/A | This PR does not add a new dispatcher candidate or architecture module to the registry. |
| 16 | N/A | This PR does not introduce a new operator family. |
| 17 | N/A | This PR does not add a new dispatcher candidate or change problem-to-spec candidate routing rules. |
| 18 | N/A | This PR does not modify registry ratchets, torch bindings, or test exempt sets. |
| 19 | PASS | dnn-providers/hip-kernel-provider/rocke/library/dispatch/AGENTS.md:191-208 and dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:186-195 accurately document that use_cfvst=True, lds_num_buffers=2, and v_reg_prefetch=True on… |
| 20 | N/A | This PR does not modify a problem-to-spec selector or a kernel family with a C++ selector/builder twin. |
| 21 | N/A | This PR implements new opt-in kernel pipeline features (BF16 CFVST, K LDS double buffering, and V register prefetch) rather than… |
§E graded 17/17
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only modifies files under library/ and touches no files under platform/. |
| 2 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:471 adds import torch |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:224 imports torch lazily inside @requires_gfx942_gpu test functions while non-GPU unit tests do not import torch. |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:472 imports downward from kernels and dispatch, and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py adds no upward… |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:29 is a Python script using pathlib.Path with no committed .sh files or /tmp, nproc, or sudo calls. |
| 6 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2249 preserves unused tuple unpacking with underscore prefixes (_d0, _t0) and existing golden hashes in… |
| 7 | PASS | Resolved since previous review: dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:194-195 and dnn-providers/hip-kernel-provider/rocke/library/dispatch/AGENTS.md:184-198 now document v_reg_prefetch alongside lds_num_buffers… |
| 8 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:134 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1005 use descriptive references without Jira IDs, PR numbers, or GitHub handles in code or… |
| 9 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:440 and accompanying test/doc files are all scoped to the gfx942 BF16 D128 CFVST, K double-buffering, and V prefetch change. |
| 10 | PASS | Resolved since previous review: dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1005-1043, dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_golden.py:285-327… |
| 11 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:1 and dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:186 are process-required deliverables… |
| 12 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1209 explicitly scopes the opt-in knobs to the gfx942 dense kernel. |
| 13 | PASS | Resolved since previous review: _use_kdb_vpf was removed in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:883-890, all test references in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:57-58… |
| 14 | NEEDS-INFO | The diff of dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py is truncated at line 2333 right before the LSE epilogue merge resolution. |
| 15 | PASS | All parameters in modified helpers in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1815 (_async_load, load_pipeline_k, do_qk, softmax_pv, publish_prefetched_v) are used in their function bodies. |
| 16 | N/A | This item governs reviewer severity grading rather than code in the pull request diff. |
| 17 | PASS | Resolved since previous review: dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:3-12 and :158-193 now consistently report relative ratios (10.74-15.58% lower latency at the 12 measured… |
§F graded 12/12
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Branch users/Parthiv911/rocke/gfx942-dense-cfvst-kdb-vpf matches the recognized users/<user>/... prefix (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:114). |
| 2 | PASS | branch users/Parthiv911/rocke/gfx942-dense-cfvst-kdb-vpf has a rocke path segment |
| 3 | FAIL | title perf(rocke): bf16 D128 CFVST, K double buffering, and V prefetch in gfx942 dense prefill is 88 characters and does not match type(scope): summary with a 10–80 character length |
| 4 | PASS | description is at least 30 characters and contains an issue reference |
| 5 | PASS | test file dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_tuning_space.py is in the diff |
| 6 | PASS | All 13 changed files are Python, Markdown, or JSON golden hash files (e.g., dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:5); no keys, certs, or secrets are added. |
| 7 | FAIL | Was NEEDS-INFO; the new commits in 4072b4ae138a..bbf25121809c introduced missing trailing newlines (\ No newline at end of file) across 9 files (e.g., dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:532, dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/dense_rules.py:370, dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_tuning_space.py:1082), and… |
| 8 | NEEDS-INFO | Not resolved since previous review: GitHub review thread states (bot/Copilot findings and resolution status) are not included in the provided PR diff context. |
| 9 | N/A | The PR base branch is develop, so this is not a stacked PR targeting a feature branch. |
| 10 | PASS | The PR keeps the kernel changes, dispatch rules, tests, benchmark script, and case study together without artificial size-cap splitting (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:114). |
| 11 | PASS | Added tests exercise real functionality (spec validation, dispatch tuning space, LDS budget, golden IR hashes, and numeric accuracy) in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1005… |
| 12 | PASS | The PR title and body are populated in the PR metadata above and match the files in the diff (dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:1). |
§G graded 34/34
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1253 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_golden.py:284 pair IR golden hashes with independent on-GPU numeric validation… |
| 2 | PASS | Resolved since previous review: dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1285-1291, 1347-1352, and 1443-1450 now exercise all independent pipeline modes ((1, False), (2, False), (1, True), (2, True)) on GPU against… |
| 3 | PASS | Resolved since previous review: dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:326-327 adds v_reg_prefetch to _SPEC_PERTURBATIONS, lines 1005-1043 test _use_kdb/_use_vpf and _kdb1/_vpf1 name tags independently, and… |
| 4 | PASS | Resolved since previous review: dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1005-1145, dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_golden.py:284-327, and… |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1227 uses _MULTI_ITEM_NP = 3 and sq=256, skv=192 (line 1249) with docstrings explaining how multi-item persistent grids and odd n_up > 1 reach the epilogue K-slot-0 read and… |
| 6 | PASS | Resolved since previous review: dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:3-12 removes the stale 'Not yet recorded' claim and updates the range to 10.74-15.58% matching lines… |
| 7 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:155-172 and dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:129-161 specify… |
| 8 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:100-101 explicitly rejects any dtype other than bf16 or head dim other than 128 via argparse.ArgumentTypeError with no silent dtype fallback. |
| 9 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:64-77 sources VGPR count, spill count, and scratch bytes directly from compiled code-object notes (.vgpr_count, .vgpr_spill_count… |
| 10 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:90-105 and 204-218 lock hardware (gfx942, 304 CUs), software stack, commit SHA, relative speedup ratios (no absolute latencies), and FP32… |
| 11 | FAIL | Partially addressed in dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:265-269 and dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/dense_rules.py:94-145 by exposing use_cfvst, lds_num_buffers, and v_reg_prefetch in the tuning space, but dnn-providers/hip-kernel-provider/rocke/library/dispatch/AGENTS.md:199-207 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_tuning_space.py:921-945 show that the default spec still leaves all three off and the production… |
| 12 | NEEDS-INFO | The diff touches only the Python kernel builder dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py and Python dispatch files, and does not show whether any C++ builder or selector exists for attention_dense on gfx942. |
| 13 | N/A | This PR does not introduce or recommend any new LLVM intrinsic .td overload classes or mangled intrinsic symbols. |
| 14 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:16 and 86 state that bottlenecks and sync-point stalls were classified using profiling and threadtrace alongside empirical GPU-timed lever… |
| 15 | N/A | The review is graded directly from the provided PR diff against head bbf2512. |
| 16 | N/A | This PR ships a completed implementation and benchmark case study rather than a pre-implementation plan. |
| 17 | PASS | Resolved since previous review: dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:124 and dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case… |
| 18 | PASS | The validation guard in dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:667-682 and 708-720 wires the torch.testing.assert_close verdict to abort the run immediately, takes median over 5 rounds of 50… |
| 19 | PASS | Fixtures in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1227-1238 (_multi_item_persistent with _MULTI_ITEM_NP = 3 so workgroups run >2 items with a ragged tail to expose LDS hand-off) and 1248-1251 (odd n_up=3… |
| 20 | N/A | This PR does not add or modify a text detector or linter enforcing a ban on claims. |
| 21 | PASS | Resolved since previous review: dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:883-890 removes the coupled _use_kdb_vpf helper in favor of independent _use_kdb and _use_vpf, and… |
| 22 | PASS | Every new golden fixture entry in dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:5-8,61-68,73-80,89-92,113-120,137-140 records both bytes and sha256 with no build-error skips, and… |
| 23 | PASS | The case study dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:159-195,291-306 and benchmark harness… |
| 24 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:5-140 and test_attention_dense_gfx942_golden.py:284-327, no pre-existing golden hashes were modified; only 10 new opt-in cases (cfvst, kdb, vpf, kdbvpf… |
| 25 | FAIL | Unresolved from previous review: in dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:129-151, VGPR, spill count, and scratch bytes before and after are reported alongside the 12-shape corpus A/B (159-195), but static ISA instruction-count deltas (waits, barriers, LDS reads/writes) and SGPR counts per kernel touched are still not reported. |
| 26 | PASS | The case study dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:291-306 and benchmark harness… |
| 27 | PASS | Both dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:45-51 and dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:180-215… |
| 28 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:186-196 and dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:3-8,291-306 explicitly document… |
| 29 | N/A | This PR does not swap any guarded primitive for a fast primitive (exp2_fast policy is untouched). |
| 30 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:129-151, spill and scratch claims are backed by reading .vgpr_count, .vgpr_spill_count, and .private_segment_fixed_size from the… |
| 31 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:515-559, the baseline is the exact unmodified attention_tuning_spec(req, 'gfx942_dense').kernel_spec shipped on develop, and the V-LDS pad (0 at… |
| 32 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:129-144,272-288,308-469 and `dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py… |
| 33 | PASS | The changed spec resolvers (resolved_use_cfvst, resolved_v_row_pad, resolved_use_v_swizzle, _lds_bytes, kernel_name, and _gfx942_cfvst_tuning_scope) are evaluated and differential-tabled across their input domain in… |
| 34 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1437-1476, test_cfvst_pipeline_lse_modes runs all four independent pipeline modes (cfvst_only, kdb_only, vpf_only, kdb_vpf) with emit_lse both on and off via… |
§H graded 5/5
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only modifies the dense prefill kernel (attention_dense.py) and tuning axes to add opt-in CFVST, K LDS double buffering… |
| 2 | N/A | This PR applies to dense prefill attention and does not use page tables, block_tables, or any device-side indirection values. |
| 3 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2201-2202, 2238, 2241, and 2281, the hot-loop b.div/b.mod divisors are V_DIM_PAIRS (64, since head_size == 128 is enforced at attention_dense.py:1246) and K_BUFS… |
| 4 | N/A | This PR does not construct or modify any device-side mask or bound from runtime sequence-length arrays (kv_lens, cu_seqlens)… |
| 5 | N/A | No new launch runtime inputs that feed device-side bounds or masks are introduced or modified in this PR (lds_num_buffers… |
§I graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:23 documents the Step-0 lever sweep across the tuning space and compares achievable winning configurations against the shipped… |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:478 loudly rejects non-exact knob types in post_init, and supports_attention_dense at line 1209 loudly rejects unsupported knob combinations with explicit diagnostic reasons. |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:351 extends _INJECTIVITY_BASES for generated knob-perturbation coverage and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_tuning_space.py:916 adds… |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:105 records both the shipped Baseline kernel and the achievable Winning kernel columns alongside their relative speedup and… |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:304 generates an explicit replay_command.sh script with provenance tracking, and… |
| 6 | N/A | This PR does not add or modify any DSL primitive or hardware instruction interface; it only adds opt-in kernel spec knobs… |
§J graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Scalar latency-reduction equations in dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:245-266 and LDS slot descriptions in… |
| 2 | PASS | All formula symbols (T_variant, t_variant,r, T_after, T_before, R_r, Min, Max) are defined in-place with consistent domains in `dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.m… |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:186-196 links to gfx942_dense_cfvst_kdb_vprefetch_case_study.md and states only the summary table delta without duplicating the methodology or algorithm. |
| 4 | PASS | Resolved in the latest commits: dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:194-195,212, `dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md… |
| 5 | N/A | The PR does not make asymptotic scaling or big-O complexity claims in its documentation or comments. |
| 6 | PASS | Resolved in the latest commits: dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:3-8 removed the stale 'Not yet recorded' sentence, lines 274-309 record complete environment and baseline… |
| 7 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:186-189 serves as the directory entry point and links to the companion case study… |
§K graded 13/13
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR updates axes and scope rules for the existing gfx942_dense KnobSpace and does not add a new tuned candidate or replace… |
| 2 | PASS | Resolved since the previous review: dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:171 and :265-269 classify lds_num_buffers and v_reg_prefetch as KnobAxis entries in _GFX942_DENSE_AXES after enabler use_cfvst (leaving only… |
| 3 | PASS | Resolved since the previous review: declaring _flag("v_reg_prefetch") in dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:269 adds v_reg_prefetch=False to the variant's declared defaults fingerprint (re-keying gfx942_dense… |
| 4 | PASS | Resolved since the previous review: dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:268-269 declares lds_num_buffers (default 1) and v_reg_prefetch (default False) on _GFX942_DENSE_AXES, so DenseKnobSpace.defaults() includes… |
| 5 | PASS | Resolved since the previous review: dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/dense_rules.py:94-105 and :143-145 update the scope predicates via _gfx942_cfvst_tuning_scope (evaluating only base-spec fields) so use_cfvst… |
| 6 | PASS | Resolved since the previous review: dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1209-1258 enforces BF16 D128 CFVST, KDB, and VPF legality in supports_attention_dense, which gates both sweeps and explicit pins in… |
| 7 | N/A | KnobSpace.base(req) is not modified in this PR. |
| 8 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:478-491 validates use_cfvst, lds_num_buffers, and v_reg_prefetch with exact type checks (type(...) is not bool / int) and raises ValueError rather than coercing with… |
| 9 | N/A | The benchmark added in `dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.… |
| 10 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_tuning_space.py:970-976 and dnn-providers/hip-kernel-provider/rocke/library/dispatch/AGENTS.md:207-208 preserve bare tuning_id resolution within the production sweep only, while the… |
| 11 | N/A | No sweep entry points (iter_dispatch_*, pin_to_spec, sweep_space) are modified in this PR. |
| 12 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:440-443 and :867 keep lds_num_buffers=1, v_reg_prefetch=False, and _use_cfvst (False for bf16) as defaults, and… |
| 13 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:581-584 delegates legality checking to dense.supports_attention_dense(spec, arch="gfx942") and aborts on any failure rather than silently dropping… |
§L graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR does not add or modify a gfx target in arch_specs.json or the C++ arch/ISA registries. |
| 2 | N/A | The PR does not add or change any MMA op_id or fragment layout in core/arch/target.py or cpp/core/lower_llvm/mma.cpp. |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1951 continues to use the existing mfma_32x32x8_for_dtype(b, dtype, k_pack, q_packs[ks], acc) helper without hardcoding new intrinsic names or altering atom shapes/lane maps. |
| 4 | N/A | The PR is scoped to the existing gfx942 kernel and does not modify wave-size definitions or target wave-size logic. |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1815 implements gfx942 K LDS double-buffering and V register prefetching directly using standard gfx942 buffer loads and perm_b32 transpose without leaking gfx950 ds_read_tr* or… |
| 6 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:440 and dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:268 keep the pipeline knobs (lds_num_buffers, v_reg_prefetch) in the library kernel and dispatch… |
| 7 | N/A | The PR does not add or modify logical dtypes, bit-packing contracts, host pack/unpack routines, or FP8 dialect handling. |
| 8 | N/A | The PR operates on unscaled bf16/fp16 attention and does not touch scaled-MMA contracts or WMMA scale ids. |
| 9 | N/A | The PR does not introduce a new architecture target. |
| 10 | N/A | The PR does not modify intrinsic_arch_domain.<flavor>.json artifacts or LLVM flavor membership lists. |
§M graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Host booleans (USE_PIPELINE, USE_KDB, USE_VPF, masked) drive Python control flow while SSA loop carries (m_new, l_new, *o_acc) are yielded through b.scf_yield and read from loop.results in… |
| 2 | N/A | This PR does not touch legalize_for_target or any target legalization passes. |
| 3 | N/A | This PR does not modify print_ir or the ck.dsl.ir/v1 serialization implementation. |
| 4 | N/A | This PR does not modify portable IR schemas, importers, magic division helpers, or builder recording. |
| 5 | PASS | Async K tile loads compute an unswizzled wave-uniform destination offset per row and consumers drain VMEM via b.s_waitcnt(vmcnt=0) followed by b.sync_lds_only() before reading LDS in… |
| 6 | PASS | All barriers (b.sync_lds_only(), b.s_waitcnt(vmcnt=0)) are emitted unconditionally in the workgroup-uniform prologue, scf_for_iter body, and epilogue in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2213-2286. |
| 7 | N/A | This PR does not touch ISABackend.buffer_rsrc_word3, hard-code buffer resource word 3, or use TemporalHint.STREAMING. |
| 8 | N/A | This PR does not add intrinsic declarations or modify lowering backend selection precedence. |
| 9 | N/A | This PR does not touch C entry points, headers under cpp/include/rocke/, or C++ host launch code. |
| 10 | N/A | This PR does not modify !dbg emission, module debug info flags, or DWARF consumers. |
§N graded 18/18
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR does not add or promote any helpers under platform/python/rocke/helpers/. |
| 2 | N/A | The PR modifies an existing lower-level DSL attention kernel and does not introduce new helper abstractions or modify… |
| 3 | N/A | The PR implements a custom K-LDS double-buffer and V-register-prefetch loop in attention_dense.py rather than modifying or using… |
| 4 | N/A | The PR optimizes an existing attention prefill kernel and does not add or modify epilogue/graph fusion kernels. |
| 5 | N/A | The PR does not add or modify any public API in rocke.helpers.tiling. |
| 6 | N/A | The PR does not add or modify front-door/toolbox tiling helpers or LayoutStyle classes. |
| 7 | N/A | The PR does not use helpers/tiling layout overrides or modify tiling MMA soundness gates. |
| 8 | N/A | The PR does not touch helpers/tiling memory verbs (lds_tile_alloc, lds_store, lds_read) or emit.py. |
| 9 | N/A | The PR does not use helpers/tiling N-D tensor axis roles, FlowStage, or label transforms. |
| 10 | N/A | The PR does not modify helpers/tiling emit, LDS staging, or MMA issue logic. |
| 11 | N/A | The PR does not use TileMma, Tiling, lds_swizzle, or derive_sched_group_counts from helpers/tiling. |
| 12 | N/A | The PR does not make helpers/tiling bank-conflict simulator or coalescing_probe gate claims. |
| 13 | N/A | The PR does not construct WarpDistributionEncoding, call make_tile_desc, or touch RegisterMapper. |
| 14 | PASS | The double-buffered K and V-prefetch pipeline explicitly places b.s_waitcnt(vmcnt=0) and b.sync_lds_only() around prologue, WAR, and epilogue LDS reads and writes (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2213… |
| 15 | PASS | LDS allocation and budget checks account for the padded/swizzled row stride and independent K buffer count (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1066… |
| 16 | N/A | The PR does not use optimize_layout or evaluate_transform from helpers/tiling. |
| 17 | N/A | The PR does not touch tiling_recorder.py, analysis/roundtrip.py, or tiling visualization. |
| 18 | N/A | The PR does not touch InterleavedStyle or demo kernels under helpers/tiling/kernels/. |
§O graded 16/16
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only modifies files under library/ and does not touch platform core verifier or lowerer files. |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2220 — the new scf_for_iter loop uses i32 bounds/step, yields matching iter_args via b.scf_yield(m_new, l_new, *o_acc) at line 2273, and reads loop.results only after the loop exits at… |
| 3 | N/A | This PR does not modify IRBuilder, intrinsic declaration tables, or metadata numbering in platform core. |
| 4 | N/A | This PR does not touch IR type definitions, SmemType, or IR parsers in platform core. |
| 5 | N/A | This PR does not touch core/passes.py, cpp/core/passes.cpp, or opcode purity tables. |
| 6 | N/A | This PR does not modify constant folding passes in Python or C++ core. |
| 7 | N/A | This PR does not modify kernel attribute lowering, CodegenPolicy validation, or COMGR options in platform core. |
| 8 | N/A | This PR does not set or modify SchedulerStrategy or CodegenPolicy. |
| 9 | N/A | This PR does not modify platform waitcnt or barrier lowering logic (and targets gfx942 rather than gfx950). |
| 10 | N/A | This PR does not modify platform lowerer attribute validation or parameter LLVM type helpers. |
| 11 | N/A | This PR does not touch ArchTarget or target resolution in platform core. |
| 12 | N/A | This PR does not modify platform core width/size caps or arch default resolution. |
| 13 | N/A | This PR does not touch any C headers or structs under cpp/include/rocke/. |
| 14 | N/A | This PR does not add or modify any C/C++ files or CMake files. |
| 15 | N/A | This PR does not touch C++ engine fallback logic or backend flavor resolution in platform core. |
| 16 | N/A | This PR does not touch C++ runtime/codegen gap areas or gfx1250 builders. |
§P graded 14/14
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:440-443 adds defaulted keyword-only fields (lds_num_buffers: int = field(default=1, kw_only=True) and v_reg_prefetch: bool = field(default=False, kw_only=True)) on… |
| 2 | N/A | This PR does not modify kernarg ordering, signature(), manifest args_signature, or GEMM/conv ABI definitions. |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:551-554 takes grid=dense.attention_dense_grid(spec) and block=dense.attention_dense_block(spec) directly from the spec helpers rather than recomputing… |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1066 charges k_bytes *= spec.lds_num_buffers in _lds_bytes matching K_lds = b.smem_alloc(dtype, [K_BUFS, BN, LDROW], name_hint="Klds") at line 1575, and… |
| 5 | N/A | This PR does not touch WMMA/RDNA GEMM or GemmMultiD/GemmMultiAbd platform instances. |
| 6 | N/A | This PR does not use or modify caller-zeroed workspaces (Stream-K, MoE sorting, fused MoE, or split-K). |
| 7 | N/A | This PR does not touch Stream-K, persistent GEMM persistent_max_iters, GroupedGemmLauncher, or SUPPORT_MATRIX.md. |
| 8 | N/A | This PR does not touch quantization helpers or MX GEMM. |
| 9 | N/A | This PR does not introduce numeric emulation or quantized decode paths. |
| 10 | N/A | This PR does not add or modify manifest kinds, emitters, or manifest runners. |
| 11 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_5way_specs.py:532-554 compiles all kernels ahead of timing and constructs KernelLauncher instances, and lines 161-170 time repeated launches using GPU… |
| 12 | N/A | This PR does not touch COMGR or HIP library loading logic or fix a hipError launch failure. |
| 13 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:735-738 and 1239-1258 add cache-name tags (kdb1, vpf1) and admission guards in supports_attention_dense, which are tested in… |
| 14 | N/A | This PR does not touch examples/, expected.json, platform/python/rocke/instances/, or helpers/. |
§Q graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | No new test files are added or removed in this PR; only existing test files under library/tests/ are modified. |
| 2 | N/A | This PR modifies the Python-only library/kernels/gfx942/attention_dense.py builder and does not add a platform kernel family or… |
| 3 | PASS | Golden hashes in dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:5 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_golden.py:284 are used as an IR drift tripwire alongside GPU… |
| 4 | PASS | New golden entries for the independent use_cfvst, lds_num_buffers, and v_reg_prefetch variants are recorded in dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:5 and… |
| 5 | PASS | CPU tests in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1070 use values the parser genuinely rejects (e.g., use_cfvst='false', v_reg_prefetch='false'), and GPU tests in… |
| 6 | N/A | This PR does not refactor any platform gate or test harness. |
| 7 | N/A | This PR only updates library spec validation in supports_attention_dense and does not touch platform backend resolution or… |
AviralGoelAMD
enabled auto-merge (squash)
October 30, 2026 21:23
AviralGoelAMD
disabled auto-merge
October 30, 2026 21:23
AviralGoelAMD
enabled auto-merge (squash)
October 30, 2026 21:26
cgmillette
disabled auto-merge
October 30, 2026 21:51
Contributor
|
Automated merge-queue status (rocKE merge monitor) Hi @Parthiv911, I turned auto-merge off on this PR for now. Before a rocKE PR lands, the rocKE merge monitor checks it on top of today's develop branch, and this version would not land cleanly:
What to do: address the open items in our latest review (#13205 (comment)). After that, turn auto-merge back on; the monitor re-checks every push. Nothing else changed: the monitor does not approve, reject or merge PRs. It only turned auto-merge off to keep develop green. This comment is updated in place. |
Resolve conflicts with develop's gfx942 dense changes, which enable BF16 D128 CFVST by default. Keep develop's CFVST policy and layer the opt-in K double buffer and V register prefetch on top; the pipeline now also rejects bottom-right causal. Drop the BF16 CFVST opt-in tuning scope and the CFVST-only golden cases, which now match the default, and re-bless the gfx942 dense golden (develop's entries unchanged).
Contributor
Author
|
@cgmillette Please run your checks. Merge conflicts are resolved. |
Contributor
|
Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max PR #13205 — add opt-in gfx942 BF16 CFVST and KDB/VPF (Parthiv911, AICK-1900) — 🟠 Follow-up: bench, docs assume CFVST off
Utility: 🟢 Positive — opt-in KDB/VPF on develop's CFVST default; default path unchanged; numerics author-run. 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, build + ctest, C++/Python parity, goldens, PR tests, default path vs develop, C++-engine lowering of the opt-in cases, the bench's pre-timing guards (CPU), history scan, CI at head. Flags whether a closer look is needed; not an approval or rejection. |
yraparti
reviewed
Oct 31, 2026
yraparti
left a comment
yraparti
left a comment
Contributor
There was a problem hiding this comment.
rocKE checklist re-review of the fixes since 0f1fb4741a3f
ROCm/rocm-libraries#13205 · 1529294c6010 · model gemini-4-argon, fallback gpt-5.5
PASS · graded 238/238 · BLOCKER 0, HIGH 3, MEDIUM 0, LOW 0, NEEDS-INFO 9
The diff was truncated before grading. Items that needed a missing hunk are NEEDS-INFO.
Since the last review (0f1fb4741a3f)
Resolved 0 · still open 1 · new 2
- ⏳ §P.4 still FAIL (HIGH): Update the C++ gfx942 dense attention validator/builder to mirror
lds_num_buffersin the LDS calculation and allocation, and cover it with a C++/Python parity test. - 🆕 §D.11 new FAIL (HIGH): Keep the BF16 D128 default policy resolving CFVST off, and make CFVST/KDB/VPF reachable only through explicit tuning knobs or an explicitly documented promotion decision.
- 🆕 §G.6 new FAIL (HIGH): Update the comments and tests in
axes.py,test_tuning_space.py,test_attention_dense_gfx942.py, andtest_attention_dense_gfx942_golden.pyto match the off-by-default BF16 CFVST policy (assertFalse(base.resolved_use_cfvst())on the unpinned BF16 default and passuse_cfvst=Truewhere CFVST/KDB/VPF are intended).
Findings
- HIGH §D ·
dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_tuning_space.py:1073· Keep the BF16 D128 default policy resolving CFVST off, and make CFVST/KDB/VPF reachable only through explicit tuning knobs or an explicitly documented promotion decision. - HIGH §G ·
dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:3· Update the comments and tests inaxes.py,test_tuning_space.py,test_attention_dense_gfx942.py, andtest_attention_dense_gfx942_golden.pyto match the off-by-default BF16 CFVST policy (assertFalse(base.resolved_use_cfvst())on the unpinned BF16 default and passuse_cfvst=Truewhere CFVST/KDB/VPF are intended). - HIGH §P ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1127· Update the C++ gfx942 dense attention validator/builder to mirrorlds_num_buffersin the LDS calculation and allocation, and cover it with a C++/Python parity test.
Needs info
- §A.3 dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:461 adds the v_reg_prefetch spec/codegen knob and dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:186 makes lds_num_buffers tunable on gfx942; the new commits since the previous review do not resolve whether Gfx942AttentionDenseSpec has a C++ selector/spec-builder twin outside the diff that requires mirroring.
- §D.8 Still NEEDS-INFO: the diff defines
bf16_cfvst_supportedand uses it insupports_attention_denseat dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:925, but the dispatch dense_rules/spec-override predicate hunk showing it imports the same cohort definition is missing. - §F.7 Unresolved since previous review (
0f1fb4741a3f..1529294c6010): GitHub bot and Copilot review thread states, resolution statuses, and any deferred ticket IDs in review threads are not visible in the provided PR diff and metadata context. - §G.3 Unresolved from the previous review: the diff updates golden hashes in dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:84 and the PR body reports test counts, but the diff cannot by itself prove that tools/run_checks.py, the installed-golden IR gate, check_ir_validity.py, the pytest suites, GPU numeric tests, and the llvm23 byte-identity check were executed and passed on current head 1529294 (especially given the contradiction noted in item 6).
- §G.12 Unresolved from the previous review: the diff does not include caller grep output or C++ files confirming that no live C++ selector or builder calls or mirrors gfx942 attention_dense.
- §P.3 The diff for
dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_7way_specs.pyis still truncated at line 825 beforeLaunchConfigconstruction, so I cannot verify whether the benchmark harness obtains launch geometry from the spec/candidate or recomputes it. - §R.3 Unresolved in the current diff: library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:455-469 maps the levers to the runbook and gfx942 architecture docs, but the PR still does not show checks against the matching utilities skills such as lds-optimization-rocke.md, lds-bank-conflict-expert/SKILL.md, prefetch-data-load-rocke.md, isa-inspection-rocke.md, or kernel-launch-guide.md.
- §R.7 Unresolved in the current diff: library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:5-13 states bottlenecks and library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:151-166 gives ATT stall evidence, but the PR still lacks the runbook §3 arithmetic-intensity / §3.1a top-level bottleneck classification counters or a stage4_analyze/probe_rocprof_single summary tying the bottleneck claim to that classification.
Section notes
- §G: Re-review of head 1529294. Items 1, 2, 4, 5, 7, 8, 9, 10, 11, 14, and 17 pass. Item 6 fails due to a stale claim in the test suite that
use_cfvst=Trueis default for BF16 D128 whileattention_dense.pyand the documentation state it is off by default for BF16. Items 3 and 12 remain NEEDS-INFO because the diff cannot prove local gate execution on the current head or C++ caller absence. - §H: Trace of device-side masks and bounds in this PR: 1. Pipeline KV tile loop (
USE_PIPELINE): extent source islast_j = n_up - 1(derived from query blockqband KV tile countn_kvfor top-left causal attention without sliding window or sinks); the kernel visits0in prologue,[0, last_j)in main loop(s), andlast_jin epilogue, issuing K/V loads only for0 .. n_up - 1. Incausal_diag_splitmode,body_upper = min(qb * n_per, last_j)splits[0, last_j)into unmasked[0, body_upper)(where every key precedes every query row of the block) and masked[body_upper, last_j), with epilogue tilelast_jalways masked (masked=True). Extent and visited range agree. 2. K LDS slot index (kbuf/next_kbuf/last_kbuf): extent source isb.mod(j, b.const_i32(K_BUFS))whereK_BUFS = spec.lds_num_buffersin{1, 2};K_ldsis allocated with[K_BUFS, BN, LDROW]. Extent and visited range agree. 3. No paged KV indirection (block_tables) or runtime variable… - §J: Shapes table: | Equation/site | Operands | Result | |---|---|---| |
T_variant = median(t_variant,1, ..., t_variant,5)| scalar per-round samples for one variant | scalarT_variant| |Speedup_A_to_candidate = T_A / T_candidate| scalar medians with same unit | scalar ratio | |Reduction_A_to_candidate (%)andR_r| scalar ratios of matched arms/rounds | scalar percentage reduction | |Min/Max| scalarR_rvalues | scalar range endpoints | Symbols table: | Symbol | Defined at | Domain | |---|---|---| |A,B1–B7| case-study arm table | benchmark arm identifiers | |t_variant,r,T_variant,T_A,T_candidate| case-study methodology formulas | scalar per-arm samples/medians | |R_r,Min,Max| case-study range formulas | scalar percentage reductions | Novelty table: | Claim | Status in doc | Boundary | |---|---|---| | BF16 D128 CFVST | inherited feature, now opt-in | not a new mechanism | | K LDS double buffering | added opt-in | K LDS only | | V… - §K: Identity table: (1)
lds_num_buffers: removed fromDENSE_UNTUNABLE_KNOBS['gfx942']and added as aKnobAxiswith default1(unchanged); (2)v_reg_prefetch: new boolean spec field onGfx942AttentionDenseSpecandKnobAxiswith defaultFalse— adds('v_reg_prefetch', False)to thegfx942_densedefaults fingerprint, intentionally re-keying all existinggfx942_denseconfig_keys without bumpingTUNING_ID_VERSION, with migration documented inlibrary/dispatch/AGENTS.md:217-224and verified intest_tuning_space.py:1130-1180; (3)use_cfvst: BF16 D128 causal CFVST remains default-resolved (None->Trueon D128), while KDB (lds_num_buffers=2) and VPF (v_reg_prefetch=True) are opt-in and gated bysupports_attention_dense. - §O: This PR only modifies library-level gfx942 attention kernels, dispatch, benchmarks, tests, and documentation; it does not touch platform core internals, C++ engine files, C ABI headers, or platform CMake.
- §R: Lever table: use_cfvst=True / V LDS transpose and wider LDS reads -> runbook §6/§7, attention §13.3, gfx942 §21 -> utilities skill: lds-optimization-rocke.md + lds-bank-conflict-expert/SKILL.md + isa-inspection-rocke.md (explicit skill citation missing) -> probe evidence: static ISA/resource and rocprof LDS instruction evidence in library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:111-147 -> tiling gate: analyze_coalescing/assert_asm_backed in library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:435-453. lds_num_buffers=2 / K-only LDS double buffering -> runbook §6/§8, attention §13.3, gfx942 §21 -> utilities skill: lds-optimization-rocke.md + prefetch-data-load-rocke.md + scheduler-policy.md (explicit skill citation missing) -> probe evidence: static resources/ISA and ATT wait/barrier evidence in library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:111-166 -> tiling…
Graded with the rocKE PR checklist, one model pass per section. FAIL rows that sit on a diff line are inline comments.
§A graded 14/14
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1432 modifies the existing Python-only gfx942 dense attention kernel emitter rather than adding a new Python-only family or changing a kernel with a C++ emitter twin. |
| 2 | N/A | No core lowering or IR files under core/lower_* or core/ir are modified by this PR. |
| 3 | NEEDS-INFO | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:461 adds the v_reg_prefetch spec/codegen knob and dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:186 makes lds_num_buffers tunable on gfx942; the new commits since the previous review do not resolve whether Gfx942AttentionDenseSpec has a C++ selector/spec-builder twin outside the diff that requires mirroring. |
| 4 | N/A | No C++ twin emitter or C++ selector files are modified in this PR's diff. |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:81 keeps existing golden hash entries unchanged while dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:84 adds entries… |
| 6 | N/A | The modified gfx942 dense kernel path is Python-only and has no C++ cross-engine emitter twin in this diff. |
| 7 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1620 guards the scalarized wave index computation behind _use_kdb(spec) or _use_vpf(spec) and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2397 gates… |
| 8 | N/A | No C++ files or C++-mirrored emitters are touched in this PR's diff. |
| 9 | N/A | This PR does not contain a shared op-handler fix or lowerer-only architecture divergence requiring a parity mini-pair. |
| 10 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_golden.py:338 adds golden test cases for the flag-on KDB, VPF, combined, LSE, and pad4+dsplit variants, and… |
| 11 | N/A | No new IR opcode or builder operation is introduced in this PR. |
| 12 | N/A | core/backend.py and ROCKE_BACKEND=both handling are not modified in this PR. |
| 13 | N/A | LLVM .ll emission and ROCKE_DEBUG_LOC handling are not modified in this PR. |
| 14 | N/A | No classic helpers under helpers/ or platform/cpp/helpers/ are modified in this PR. |
§B graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1296 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1308 classify invalid lds_num_buffers values vs unimplemented KDB/VPF combinations with an… |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:522 keeps __post_init__ to exact type validation on use_cfvst, lds_num_buffers, and v_reg_prefetch, while… |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_tuning_space.py:1075 verifies that default BF16 D128 dispatch preserves use_cfvst=None, lds_num_buffers=1, and v_reg_prefetch=False, so no model-zoo shape is silently demoted to a slow… |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:522 rejects non-exact types for use_cfvst, lds_num_buffers, and v_reg_prefetch in __post_init__ without coercion, and… |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1465 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:2073 numerically verify the newly opened BF16 D128 CFVST, KDB, VPF, and… |
| 6 | PASS | Resolved the previous NEEDS-INFO: dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1301 restricts supports_attention_dense to functional and NOT_YET_IMPLEMENTED emit invariants for lds_num_buffers=2 and v_reg_prefetch=True… |
§C graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Shared predicates and pipeline stages are factored into single-source helpers: _use_kdb (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:928), _use_vpf… |
| 2 | PASS | All pipeline variants are controlled via spec knobs (use_cfvst, lds_num_buffers, v_reg_prefetch, causal_diag_split) inside the single build_attention_dense(spec) builder in… |
| 3 | PASS | Kernel naming emits codegen knob tags (kdb1, vpf1) in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:768 without embedding workload dimensions. |
| 4 | PASS | All kernel and dispatch changes are scoped to gfx942-specific modules and entries (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:452 and dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:295) without… |
| 5 | PASS | The diff is tightly scoped to the gfx942 dense attention kernel (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:114), gfx942 tuning axes (dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:185)… |
| 6 | PASS | softmax_pv is factored out in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2195 so existing single-buffer tile_compute (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2279) and pipeline… |
| 7 | N/A | This PR is a targeted performance optimization adding opt-in spec knobs rather than a standalone cleanup refactor. |
| 8 | PASS | The PR extends Gfx942AttentionDenseSpec (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:457), reuses existing DSL primitives (b.smem_alloc, b.sync_lds_only, b.buffer_load_vN, _cfvst_store_v) in… |
| 9 | PASS | bf16_cfvst_supported in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:938 defines the single KDB/VPF cohort consumed by supports_attention_dense… |
| 10 | PASS | dnn-providers/hip-kernel-provider/rocke/library/dispatch/AGENTS.md:198 and dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:477 match the widened supports_attention_dense gating… |
§D graded 21/21
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | The new controls are explicit spec fields with validation in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:449 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:517, and they are declared as… |
| 2 | N/A | No Capability declaration or KernelCandidate architecture coverage is added or modified in the shown diff. |
| 3 | N/A | No dispatcher candidate _supports predicate or direct _supports caller path is added or modified in the shown diff. |
| 4 | PASS | The new dispatch-selection surface is static tuning-axis data for the request-driven gfx942 dense path at dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:295, and the dispatcher tests select via `tuning_spec_with_knobs(req, "gfx942_dense"… |
| 5 | PASS | The binary-changing KDB and VPF controls are folded into kernel-name/cache-key tags through _use_kdb(spec) and _use_vpf(spec) at dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:763. |
| 6 | PASS | The full-pipeline dispatcher test selects the spec through tuning_spec_with_knobs, checks launch-relevant resolved fields, verifies kernel-name markers, and replays the selected knobs at… |
| 7 | N/A | No selector heuristic ordering or hand-pinned override layer is added or reordered in the shown diff. |
| 8 | NEEDS-INFO | Still NEEDS-INFO: the diff defines bf16_cfvst_supported and uses it in supports_attention_dense at dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:925, but the dispatch dense_rules/spec-override predicate hunk showing it imports the same cohort definition is missing. |
| 9 | N/A | No new kernel candidate or architecture registration is added; the change adds tuning variants on the existing gfx942 dense… |
| 10 | PASS | The new spec fields are reachable from the dispatcher tuning request through declared axes at dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:295 and exercised by dispatcher-level explicit selection in… |
| 11 | FAIL | The dispatcher test now requires the default BF16 D128 gfx942 dense spec to resolve CFVST on at dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_tuning_space.py:1073, which promotes the feature onto the default path instead of keeping it opt-in. |
| 12 | N/A | This PR does not add or rename dispatcher candidates, change candidate priority, or introduce a custom ranker. |
| 13 | PASS | Adding the defaulted v_reg_prefetch field rekeys the gfx942_dense defaults fingerprint so stale tuning_id pins are refused (admits returns False, causing CandidateRegistry.select to raise PinRefused), documented at… |
| 14 | PASS | The new IR-changing KDB and VPF controls receive independent kernel-name/cache identity tags at dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:766-771, and the defaults fingerprint change rekeys config_key / tuning_id while… |
| 15 | N/A | This PR does not add a new dispatcher candidate or architecture module to the registry. |
| 16 | N/A | This PR does not introduce a new operator family. |
| 17 | N/A | This PR does not add a new dispatcher candidate whose CPU routing cohort must be tested through admits(). |
| 18 | N/A | This PR does not modify registry ratchets, torch-binding requirements, route-only registration, or ratchet exempt sets. |
| 19 | PASS | The dispatch documentation explicitly states that the BF16 D128 default remains off (use_cfvst=None resolving to off, lds_num_buffers=1, v_reg_prefetch=False) and that these opt-in dense tuning choices do not alter the shipped default or make the opt-in dense… |
| 20 | N/A | This PR changes Python dispatch tuning and the Python gfx942 dense kernel path, which has no C++ emitter or C++-consumed selector… |
| 21 | N/A | This PR implements new opt-in kernel pipeline features and tuning exposure rather than patching an emitted artifact to work around… |
§E graded 17/17
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/platform/dsl_docs/optimization/optimization_runbook.md:2543-2550 is the only platform file touched by this PR and adds documentation only without importing library. |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_7way_specs.py:591-603 and :681-686 query the device through rocke.runtime.hip_module (get_device_name, get_device_num_cus, get_device_target_id, current_device… |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_7way_specs.py:59 uses NumPy with no torch dependency, and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:233 and :1466 import… |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_7way_specs.py:867-876 imports downward from kernels and dispatch, and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:730 adds no… |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_7way_specs.py:60 is a Python script using pathlib.Path with no committed .sh files or /tmp, nproc, or sudo calls. |
| 6 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2011-2051 emits only used value pairs in _cfvst_load_v_pipeline, and existing golden hashes in… |
| 7 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:193-197, dnn-providers/hip-kernel-provider/rocke/library/dispatch/AGENTS.md:187-224, and… |
| 8 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:114-127 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1102-1105 use descriptive references without Jira IDs, PR numbers, or GitHub handles in… |
| 9 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:452-461 and accompanying test/doc/benchmark files (including dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.… |
| 10 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2632-2706, dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1106-1565… |
| 11 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:1 and dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:188 are process-required deliverables. |
| 12 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1296-1311 explicitly scopes the opt-in knobs to the gfx942 dense kernel. |
| 13 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:315 and :421 reference bench_rocke_7way_specs.py, and… |
| 14 | PASS | Resolved in the full diff: dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2632-2706 preserves upstream emit_segment_store and sinks-aware lse_store(..., row_has_mass=row_has_mass) while updating m to m_i, and… |
| 15 | PASS | All parameters in modified helpers in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1908-2277 (_async_load, load_pipeline_k, _cfvst_load_v_pipeline, do_qk, softmax_pv, pipeline_step) and… |
| 16 | N/A | This item governs reviewer severity grading rather than code in the pull request diff. |
| 17 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:155-225 and :287-297 report relative ratios, static coalescing/ISA verification, and provenance without absolute software-achieved… |
§F graded 11/11
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Branch users/Parthiv911/rocke/gfx942-dense-cfvst-kdb-vpf matches the recognized users/<user>/... prefix (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:114). |
| 2 | PASS | title perf(rocke): add opt-in gfx942 BF16 CFVST and KDB/VPF matches type(scope): summary and is 53 characters |
| 3 | PASS | description is at least 30 characters and contains an issue reference |
| 4 | PASS | test file dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_tuning_space.py is in the diff |
| 5 | PASS | All 13 changed files in the PR are Python source/test files, Markdown docs, or JSON golden hash files (dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:2); no keys, certs, or secrets are added. |
| 6 | PASS | Changed files such as dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:522 and dnn-providers/hip-kernel-provider/rocke/platform/dsl_docs/optimization/optimization_runbook.md:2543 are cleanly formatted without lint or formatting… |
| 7 | NEEDS-INFO | Unresolved since previous review (0f1fb4741a3f..1529294c6010): GitHub bot and Copilot review thread states, resolution statuses, and any deferred ticket IDs in review threads are not visible in the provided PR diff and metadata context. |
| 8 | N/A | The PR base branch is develop, so this is not a stacked PR targeting a feature branch. |
| 9 | PASS | The PR keeps the kernel changes, dispatch axes, tests, benchmark script, and case study together in one cohesive change (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:114). |
| 10 | PASS | Added tests exercise real functionality including spec validation, dispatch tuning space, LDS budgets, golden IR hashes, and numeric accuracy in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1106… |
| 11 | PASS | The PR title and body in the metadata are populated and match the benchmark run and case study in dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:191. |
§G graded 34/34
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_golden.py:338 pins LLVM IR SHA-256 golden hashes for the opt-in pipeline modes, while dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1465… |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1465, dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1702, and… |
| 3 | NEEDS-INFO | Unresolved from the previous review: the diff updates golden hashes in dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:84 and the PR body reports test counts, but the diff cannot by itself prove that tools/run_checks.py, the installed-golden IR gate, check_ir_validity.py, the pytest suites, GPU numeric tests, and the llvm23 byte-identity check were executed and… |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1158 tests the persistent=True requirement for KDB/VPF, dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1326 tests bf16_cfvst_supported, and… |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.py:1442 documents and enforces multi-item persistent reachability preconditions, and… |
| 6 | FAIL | Contradictory load-bearing prose and assertions on whether BF16 D128 CFVST is on or off by default: dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:3 and :392 and dnn-providers/hip-kernel-provider/rocke/library/dispatch/AGENTS.md:211 state that BF16 D128 default use_cfvst=None resolves to off (matching _use_cfvst in attention_dense.py which is dtype == 'fp16' and head_size == 128), whereas… |
| 7 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:223 specifies the independent chunked NumPy FP32 causal GQA SDPA reference as a pre- and post-batch gate, and… |
| 8 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_7way_specs.py:290 rejects any non-bf16 or non-D128 shape in parse_shape instead of silently falling back to another dtype or head dimension. |
| 9 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:128 and dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_7way_specs.py:493 extract VGPR… |
| 10 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:173 and dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:269… |
| 11 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_tuning_space.py:1269 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1532 verify explicit tuning reachability (including the full pad4+dsplit… |
| 12 | NEEDS-INFO | Unresolved from the previous review: the diff does not include caller grep output or C++ files confirming that no live C++ selector or builder calls or mirrors gfx942 attention_dense. |
| 13 | N/A | This PR does not add, modify, or recommend LLVM intrinsic .td overload classes or mangled intrinsic symbols. |
| 14 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:122 and dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:151… |
| 15 | N/A | This review grades the provided diff for head commit 1529294 directly, so there is no local branch-checkout ambiguity. |
| 16 | N/A | This PR delivers concrete kernel, dispatch, test, and benchmark changes rather than an unverified pre-implementation plan. |
| 17 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_7way_specs.py:699 and dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:269 show that the… |
| 18 | PASS | The benchmark validates every variant against the FP32 reference before timing and after every timed batch (aborting on failure), takes medians over 5 rounds of 50 launches, and documents the blind spot that post-batch validation checks only the final output of each… |
| 19 | PASS | The numeric test fixtures construct a small persistent grid (_MULTI_ITEM_NP = 3) with more than two work items per workgroup and ragged handoff, plus zero-trip (256, 64), odd-tile (256, 192 / 512, 192), and multi-tile (512, 512) shapes at… |
| 20 | N/A | This PR does not add or modify a text detector or linter enforcing a ban on claims. |
| 21 | PASS | The benchmark deliverable records full environment and run metadata, validates every variant against a chunked NumPy FP32 reference using fixed atol=1e-2 / rtol=1e-2 bounds rather than noise-floor-relative tolerances, and labels modeled vs measured quantities in the… |
| 22 | PASS | All added golden entries carry both bytes (> 0) and 64-character sha256 digests across all GOLDEN_PROFILE_FLAVORS rather than build errors or skips, verified by test_pipeline_golden_entries_cover_each_independent_mode_and_llvm_flavor at… |
| 23 | PASS | Performance claims enumerate dtype, B, S, Hq, Hkv, D, causal mode, persistent grid, target arch, CU count, and exact dispatch-resolved baseline kernel identity, and validate all arms against the FP32 reference at… |
| 24 | PASS | Existing golden hashes are unchanged and every newly added opt-in golden case (including persist_bthkvmin_kdbvpf_pad4_dsplit and LSE variants) has corresponding GPU numeric coverage at… |
| 25 | PASS | The case study and benchmark report static VGPR/SGPR/spills/scratch/LDS and static ISA instruction counts across all ablation variants (extracted via static_resources from the timed HSACOs) and document the SGPR V-addressing micro-optimization and sibling kernel check… |
| 26 | PASS | _capture_git_checkout refuses dirty or untracked checkouts unless --allow-dirty is explicitly passed, verify_provenance verifies clean checkout state, source SHA-256 hashes, and dispatch replay, and the case study records clean-checkout provenance at… |
| 27 | PASS | The benchmark script defines single-knob transitions across B-stages via STAGE_OVERRIDES and changed_fields and tables each knob increment separately against both the production baseline and Step-0 winner at… |
| 28 | PASS | The documentation states that all new levers are opt-in and off by default (0-delta on shipped default dispatch), and the benchmark resolves its B1 baseline directly from production dispatch via attention_tuning_spec(req, 'gfx942_dense').kernel_spec rather than a… |
| 29 | N/A | This PR does not swap a guarded primitive such as exp2 for a fast primitive. |
| 30 | PASS | VGPR counts, spill counts, and scratch bytes are read directly from compiled HSACO code-object notes via static_resources (using llvm-readelf and llvm-objdump) and reported in the case study at… |
| 31 | PASS | The benchmark resolves the baseline from attention_tuning_spec for the production request, verifies that none of the opt-in pipeline knobs are already enabled in the shipped baseline, and also compares against the rebuilt Step-0 existing-lever sweep winner (arm A) on… |
| 32 | PASS | The benchmark and case study record hardware target ID (gfx942, 304 CUs), loaded HIP/COMGR DSOs, LLVM flavor (llvm23), git commit, source SHA-256 hashes, and exact reproduction scripts at… |
| 33 | PASS | The changed tuning and support decision functions are tested across default, CFVST, KDB, VPF, combined, causal_diag_split, and rejected configurations (including non-persistent KDB/VPF, non-bf16, D64, block_n=32, sliding window, and sinks) and the case study tables… |
| 34 | PASS | TestDenseLseNumericGfx942.test_cfvst_pipeline_lse_modes and test_cfvst_pipeline_pad4_dsplit_lse_numeric verify that enabling emit_lse produces bit-identical output tensor O to emit_lse=False (torch.equal(out, off_out)) across CFVST, KDB, VPF, combined, and pad4+dsplit… |
§H graded 5/5
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only adds opt-in CFVST, K LDS double buffering, and V register prefetch to the dense prefill kernel (attention_dense.py)… |
| 2 | N/A | This PR applies to dense prefill attention and does not use page tables, block_tables, or any device-side indirection values. |
| 3 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1621-1626, b.div and b.mod use compile-time power-of-two constant 64 (and 32), _cfvst_load_v_pipeline at attention_dense.py:2011-2051 and publish_prefetched_v at… |
| 4 | N/A | This PR does not construct or modify any device-side mask or bound from runtime sequence-length arrays (kv_lens, cu_seqlens)… |
| 5 | N/A | No new launch runtime inputs that feed device-side bounds or masks are introduced or modified in this PR (use_cfvst… |
§I graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:15 documents the Step-0 lever sweep across the tuning space and compares achievable winning configurations against both the Step-0 winner… |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:522 loudly rejects non-exact knob types in post_init, and supports_attention_dense at lines 1296 and 1301 loudly rejects unsupported knob combinations (including non-persistent grids… |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:360 extends _INJECTIVITY_BASES for generated knob-perturbation coverage and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_tuning_space.py:1069 adds… |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:97 and line 209 record both the shipped Baseline/Production Dispatch kernel and the achievable Step-0 and Full Pipeline winning kernel… |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_7way_specs.py:123 explicitly distinguishes the recorded Step-0 sweep snapshot from the live re-measured current-checkout specs (lines 282 and 686), and… |
| 6 | N/A | This PR does not add or modify any DSL primitive or hardware instruction interface; it only adds opt-in kernel spec knobs… |
§J graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | The case study defines the arm/result columns before the formulas and every added equation is scalar-to-scalar: arm variants are declared in dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md… |
| 2 | PASS | The formula symbols are introduced with one meaning in the methodology: A/B1-B7 at dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:239-248, T_variant, T_A, and T_candidate at… |
| 3 | PASS | The sibling README links to the case study and summarizes only the opt-in lever deltas in dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:188-198, and the optimization runbook adds a short heuristic plus the same link in… |
| 4 | PASS | The docs separate inherited from added scope: CFVST is described as already present but newly opt-in for BF16 in dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:112, while KDB and VPF are… |
| 5 | N/A | The Markdown diff makes no asymptotic or big-O scaling/complexity claims, only measured ablation, provenance, and tuning-scope… |
| 6 | PASS | The case study anchors constants and scope to a configuration/source table in dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:20-34, records static-resource provenance in… |
| 7 | PASS | The existing directory README remains the entry point and links to the companion case study in dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/README.md:188-190, while the new case study declares its specific scope in… |
§K graded 13/13
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR updates axes and rules on the existing gfx942_dense tuned candidate rather than introducing a new tuned candidate or… |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:186 removes lds_num_buffers from DENSE_UNTUNABLE_KNOBS['gfx942'], dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:294-299 orders enabler use_cfvst before… |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/dispatch/AGENTS.md:217-224 documents the gfx942_dense tuning-id migration caused by the new v_reg_prefetch default fingerprint, and… |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:298-299 declares problem-independent defaults for lds_num_buffers (1) and v_reg_prefetch (False), matching the dataclass defaults in… |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_tuning_space.py:1182-1270 verifies CFVST axes remain reachable on diagonal-split and non-persistent bases while out-of-scope KDB/VPF pins are refused by tuning_spec_with_knobs. |
| 6 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1296-1311 centralizes BF16 CFVST, KDB, and VPF legality in supports_attention_dense, and… |
| 7 | N/A | KnobSpace.base(req) and request normalization for candidate base specs are not modified in this PR. |
| 8 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:522-535 rejects non-exact bool/int types for use_cfvst, lds_num_buffers, and v_reg_prefetch, and… |
| 9 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_7way_specs.py:258-305 selects every arm through tuning_spec_with_knobs, records candidate/algorithm/spec_id/tuning_id/config_key/knobs/spec_hash via… |
| 10 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_tuning_space.py:1116-1122 replays bare tuning_id only for single-lever production points while full-space combinations replay with explicit knobs in… |
| 11 | N/A | No dispatcher sweep entry points (iter_dispatch_*_all, pin_to_spec, sample handling, or sweep_level scoping) are modified… |
| 12 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_tuning_space.py:1069-1099 verifies the default base spec keeps use_cfvst=None (resolved True on D128), lds_num_buffers=1, and v_reg_prefetch=False, while production sweep includes… |
| 13 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_7way_specs.py:258-274 constructs benchmark arms via tuning_spec_with_knobs and dnn-providers/hip-kernel-provider/rocke/library/dispatch/AGENTS.md:206-215 states the… |
§L graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR does not add or modify a gfx target in arch_specs.json or the C++ arch/ISA registries. |
| 2 | N/A | The PR does not add or change any MMA op_id or fragment layout in core/arch/target.py or cpp/core/lower_llvm/mma.cpp. |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2086 continues to use the existing mfma_32x32x8_for_dtype(b, dtype, k_pack, q_packs[ks], acc) helper without hardcoding new intrinsic names or altering atom shapes/lane maps. |
| 4 | N/A | The PR is scoped to the existing gfx942 kernel and does not modify wave-size definitions or target wave-size logic. |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1975 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2011 implement gfx942 K LDS double-buffering and V register prefetching directly using standard… |
| 6 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:457 and dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:298 keep the pipeline knobs (lds_num_buffers, v_reg_prefetch) in the library kernel and dispatch… |
| 7 | N/A | The PR does not add or modify logical dtypes, bit-packing contracts, host pack/unpack routines, or FP8 dialect handling. |
| 8 | N/A | The PR operates on unscaled bf16/fp16 attention and does not touch scaled-MMA contracts or WMMA scale ids. |
| 9 | N/A | The PR does not introduce a new architecture target. |
| 10 | N/A | The PR does not modify intrinsic_arch_domain.<flavor>.json artifacts or LLVM flavor membership lists. |
§M graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Host booleans (USE_CFVST, USE_KDB, USE_VPF, USE_PIPELINE, DIAG_SPLIT, masked) drive Python control flow in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1641-1658 and… |
| 2 | N/A | This PR does not touch legalize_for_target or any target legalization passes. |
| 3 | N/A | This PR does not modify print_ir or the ck.dsl.ir/v1 serialization implementation. |
| 4 | N/A | This PR does not modify portable IR schemas, importers, magic division helpers, or builder recording. |
| 5 | PASS | Async K tile loads compute an unswizzled wave-uniform destination offset per row (row_base = b.smem_ptr_add(lds_base, row_lds_off)) in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1908-1923, and consumers drain VMEM via… |
| 6 | PASS | All barriers and waits (b.s_waitcnt(vmcnt=0), b.sync_lds_only()) in the pipeline prologue, loop body, and epilogue are emitted unconditionally in workgroup-uniform control flow in… |
| 7 | N/A | This PR does not touch ISABackend.buffer_rsrc_word3, hard-code buffer resource word 3, or use TemporalHint.STREAMING. |
| 8 | N/A | This PR does not add intrinsic declarations or modify lowering backend selection precedence. |
| 9 | N/A | This PR does not touch C entry points, headers under cpp/include/rocke/, or C++ host launch code. |
| 10 | N/A | This PR does not modify !dbg emission, module debug info flags, or DWARF consumers. |
§N graded 18/18
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The diff does not add or promote any helper under platform/python/rocke/helpers/. |
| 2 | N/A | The diff modifies a gfx942 attention kernel, dispatch tuning axes, benchmarks, and tests, not a kernel use of shared helper… |
| 3 | N/A | The diff does not modify or use the helpers SoftwarePipeline abstraction. |
| 4 | N/A | The diff does not add or modify fusion, EpilogueOp, pattern tables, or fused GEMM/epilogue builders. |
| 5 | N/A | The diff does not add or modify any public API in rocke.helpers.tiling. |
| 6 | N/A | The diff does not add or modify front-door/toolbox tiling helpers or LayoutStyle classes. |
| 7 | N/A | The diff does not use helpers/tiling layout overrides or modify tiling MMA soundness gates. |
| 8 | N/A | The diff does not touch helpers/tiling memory verbs such as lds_tile_alloc, lds_store, lds_read, or emit.py. |
| 9 | N/A | The diff does not use helpers/tiling N-D tensor axis roles, at_index, squeeze, FlowStage labels, or AB-swap routing. |
| 10 | N/A | The diff does not modify helpers/tiling emit, LDS staging, MMA issue logic, or tiling analysis gates. |
| 11 | N/A | The diff does not use TileMma, Tiling, lds_swizzle, derive_sched_group_counts, or helpers/tiling trait loading. |
| 12 | PASS | The case study explicitly states that CFVST improves performance through wider V LDS reads and reduced LDS instruction traffic rather than as a bank-conflict reduction claim… |
| 13 | N/A | The diff does not construct WarpDistributionEncoding, call make_tile_desc, re-export RegisterMapper, or change store cast rules in… |
| 14 | PASS | The new pipeline places explicit waits and LDS synchronization around prologue/loop WAR and publication points (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:2422… |
| 15 | N/A | The diff does not use helpers/tiling transformed descriptors or tiling allocation/bounds checks. |
| 16 | N/A | The diff does not use optimize_layout, recommend, or evaluate_transform from helpers/tiling. |
| 17 | N/A | The diff does not touch tiling_recorder.py, analysis/roundtrip.py, or tiling visualization. |
| 18 | N/A | The diff does not touch InterleavedStyle or demo kernels under helpers/tiling/kernels/. |
§O graded 16/16
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR does not modify the platform core verifier, C++ verifier, pointer-space definitions, or lowerer pointer type helpers. |
| 2 | N/A | This PR does not touch platform/python/rocke/core/ or platform/cpp/core/ where scf.for verification and lowering internals… |
| 3 | N/A | This PR does not modify IRBuilder SSA naming, intrinsic declaration tables, emission metadata numbering, or platform core emission… |
| 4 | N/A | This PR does not touch IR type definitions, SmemType naming, or IR parsers in either engine. |
| 5 | N/A | This PR does not edit core optimization passes or opcode purity tables in Python or C++. |
| 6 | N/A | This PR does not modify Python or C++ constant folding logic. |
| 7 | N/A | This PR does not modify platform kernel attribute validation, codegen policy attribute emission, cluster-dim handling, or COMGR… |
| 8 | N/A | This PR does not add or modify SchedulerStrategy, CodegenPolicy, apply_codegen_policy, or hipcc scheduler handling. |
| 9 | N/A | This PR does not modify platform waitcnt encoding or barrier lowering logic. |
| 10 | N/A | This PR does not modify platform lowerer attribute revalidation or LLVM parameter type helpers. |
| 11 | N/A | This PR does not touch ArchTarget, target-id resolution, target catalogs, or platform hardware-query internals. |
| 12 | N/A | This PR does not modify platform core width or size-cap derivation or codegen arch defaults. |
| 13 | N/A | This PR does not touch public C structs, enums, ABI version definitions, or Python ABI mirrors under cpp/include/rocke/. |
| 14 | N/A | This PR does not add or modify C++ source, headers, bindings, or platform CMake files. |
| 15 | N/A | This PR does not modify C++ engine fallback visibility, fallback records, backend flavor resolution, or debug lowerer authority… |
| 16 | N/A | This PR does not touch known_gaps.md debt areas, C++ codegen-only runtime behavior, or gfx1250 builder parity claims. |
§P graded 14/14
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:457-461 keeps lds_num_buffers and v_reg_prefetch as defaulted keyword-only spec fields on Gfx942AttentionDenseSpec, and… |
| 2 | N/A | This PR does not modify kernarg ordering, signature(), manifest args_signature, or GEMM/conv ABI definitions. |
| 3 | NEEDS-INFO | The diff for dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_7way_specs.py is still truncated at line 825 before LaunchConfig construction, so I cannot verify whether the benchmark harness obtains launch geometry from the spec/candidate or recomputes it. |
| 4 | FAIL | The changes since the previous review do not resolve this: dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1127 updates Python LDS accounting (k_bytes *= spec.lds_num_buffers) and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1657 updates the Python LDS allocation (b.smem_alloc(dtype, [K_BUFS, BN, LDROW], ...)), while the diff still has no corresponding C++ validator/builder update mirroring the LDS gate and K-buffer count. |
| 5 | N/A | This PR does not touch WMMA/RDNA GEMM or GemmMultiD/GemmMultiAbd platform instances. |
| 6 | N/A | This PR does not use or modify caller-zeroed workspaces such as Stream-K, MoE sorting, fused MoE, or split-K. |
| 7 | N/A | This PR does not touch Stream-K, persistent GEMM persistent_max_iters, GroupedGemmLauncher, or SUPPORT_MATRIX.md. |
| 8 | N/A | This PR does not touch quantization helpers, quantization decode, or MX GEMM. |
| 9 | N/A | This PR does not introduce numeric emulation or quantized decode paths. |
| 10 | N/A | This PR does not add or modify manifest kinds, manifest emitters, or manifest runners. |
| 11 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_7way_specs.py:424-451 manages an explicit HIP stream with wait_stream, and… |
| 12 | PASS | dnn-providers/hip-kernel-provider/rocke/library/benchmarks/gfx942/attention/prefill/bench_rocke_7way_specs.py:524-538 locates LLVM tools via loaded_compiler_info() from the already loaded COMGR library, and… |
| 13 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:768-771 adds distinct kdb1 and vpf1 cache/kernel-name tags, dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1296-1311 updates admission guards in… |
| 14 | N/A | This PR does not touch examples/, expected.json, platform/python/rocke/instances/, or helpers/. |
§Q graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | No new pytest test files are added or removed in this PR; only existing test files under library/tests/ are modified (and… |
| 2 | N/A | This PR modifies the library/kernels/gfx942/attention_dense.py builder and does not add a platform kernel family or C++ parity… |
| 3 | PASS | Golden hashes in dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json:84 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_golden.py:338 serve as an IR drift tripwire alongside GPU… |
| 4 | PASS | New golden entries for the independent use_cfvst, lds_num_buffers, v_reg_prefetch, and persist_bthkvmin_kdbvpf_pad4_dsplit variants are recorded across GOLDEN_PROFILE_FLAVORS in… |
| 5 | PASS | CPU negative tests in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1256 use values the parser genuinely rejects (e.g., use_cfvst='false', v_reg_prefetch='false'), active flavor is queried dynamically in… |
| 6 | N/A | This PR does not refactor any platform gate or test harness. |
| 7 | N/A | This PR only updates library spec validation in supports_attention_dense and does not touch platform backend resolution or… |
§R graded 11/11
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:43-107 enumerates the existing spec levers, states which were swept or excluded, describes the staged sweep with FP32 correctness pruning, and names the Step-0 winners used as… |
| 2 | PASS | library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:455-490 maps CFVST, KDB, and VPF to runbook §6/§7/§8, the §13.3 attention checklist, gfx942 §21, and gfx942-only eligibility; library/kernels/gfx942/attention_dense.py:1293-1306… |
| 3 | NEEDS-INFO | Unresolved in the current diff: library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:455-469 maps the levers to the runbook and gfx942 architecture docs, but the PR still does not show checks against the matching utilities skills such as lds-optimization-rocke.md, lds-bank-conflict-expert/SKILL.md, prefetch-data-load-rocke.md, isa-inspection-rocke.md, or kernel-launch-guide.md. |
| 4 | PASS | library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:111-147 reports static resources, spills, scratch, occupancy, MFMA, load/store, barrier, waitcnt, VALU, and SALU counts from the exact timed HSACOs… |
| 5 | PASS | library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:435-453 records analyze_coalescing with vector_transactions, addr_fn_from_strides, line_bytes_for("gfx942"), CoalescingReport-style summaries, and assert_asm_backed for the… |
| 6 | PASS | library/tests/test_attention_dense_gfx942_numeric.py:1431-1653 adds FP32-reference and bit-identical CFVST/KDB/VPF numeric coverage, and library/tests/test_attention_dense_gfx942_lds.py:334-368 checks the independent KDB/VPF LDS footprints against the lowered LDS pool. |
| 7 | NEEDS-INFO | Unresolved in the current diff: library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:5-13 states bottlenecks and library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:151-166 gives ATT stall evidence, but the PR still lacks the runbook §3 arithmetic-intensity / §3.1a top-level bottleneck classification counters or a… |
| 8 | PASS | library/kernels/gfx942/attention_dense.py:2394-2526 documents the KDB/VPF pipeline’s load, wait, and LDS WAR/publication barriers, and library/tests/test_attention_dense_gfx942.py:1390-1448 verifies the emitted loop operations keep tile.sync_lds_only and… |
| 9 | PASS | library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:167-226 reports the shape sweep on gfx942 and records missing Step-0 winners rather than fabricating A/B comparisons, while… |
| 10 | NEEDS-INFO | Unresolved in the current diff: library/benchmarks/gfx942/attention/prefill/bench_rocke_7way_specs.py:28-35 and library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md:295-312 document warmup, repeated randomized paired rounds, provenance, and environment, but still do not document GPU clock lock/state or otherwise show the same clocks for both benchmark arms as required by runbook… |
| 11 | PASS | platform/dsl_docs/optimization/optimization_runbook.md:2540-2547 adds the new gfx942 BF16 D128 CFVST/KDB/VPF heuristic to the runbook and points to the case study without adding new figures to measured_results.md. |
§S graded 8/8
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | New public symbols v_reg_prefetch on Gfx942AttentionDenseSpec (dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:461) and bf16_cfvst_supported in __all__… |
| 2 | PASS | Existing API behavior changes for lds_num_buffers and DENSE_UNTUNABLE_KNOBS["gfx942"] (dnn-providers/hip-kernel-provider/rocke/library/dispatch/attention/axes.py:186, :298) and supports_attention_dense… |
| 3 | N/A | No public API symbol is removed or renamed in this pull request. |
| 4 | PASS | bf16_cfvst_supported is added to __all__ at dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:734, resolves to the function defined at dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:938, and is… |
| 5 | PASS | Tests assert behavioral contracts and exact negative exceptions: exact ValueError types and messages for invalid use_cfvst, lds_num_buffers, and v_reg_prefetch types are checked at… |
| 6 | PASS | CPU-checkable contracts (spec validation, support gating, IR structure, golden LLVM IR digests, LDS byte sizing, and dispatch tuning space) are tested without a GPU in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:1106… |
| 7 | N/A | The modified gfx942 dense attention kernel builder and Python dispatch tuning layer do not have a C++ spec-builder/selector twin… |
| 8 | N/A | This PR does not modify rocke/platform/ code, runtime/manifest APIs, or the platform README minimal-build entry points. |
Resolve conflict in attention_dense_gfx942_ir_sha256.json: develop (ROCm#14692) qualified the LLVM 24 lane (aliases {} + explicit llvm24 hashes). Re-blessed with test_attention_dense_gfx942_golden.py --write: 0 existing hashes changed; adds llvm24 entries for the 12 KDB/VPF cases this PR introduced.
Contributor
|
Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max PR #13205 — add opt-in gfx942 BF16 CFVST and KDB/VPF (Parthiv911, AICK-1900) — 🟠 Follow-up: bench, docs assume CFVST off
Net-new: none. Utility: 🟢 Positive — opt-in KDB/VPF on develop's CFVST body; default path unchanged; numerics author-run. 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, build + ctest, C++/Python parity, goldens (dense golden re-recorded at all 4 flavors), PR tests, default path vs develop, C++-engine lowering of the opt-in cases, the bench's pre-timing guards, history and body scan, CI at head. Flags whether a closer look is needed; not an approval or rejection. |
_use_cfvst returns True for every D128 spec regardless of dtype (since ROCm#14367), so use_cfvst=None resolves ON for BF16 D128. Correct the case study, dispatch AGENTS.md, prefill README, module docstring and a test comment that said it resolves off / is opt-in. Only K double buffering and V register prefetch are new opt-in levers in this PR. Tests are unchanged; they already assert the real default.
CFVST is the existing D128 default (use_cfvst=None resolves on); describe it as the prerequisite body, and list the production modes as CFVST off, CFVST + KDB, CFVST + VPF.
AviralGoelAMD
enabled auto-merge (squash)
November 1, 2026 09:13
…PF C++ parity Since ROCm#14367 production resolves BF16 D128 CFVST on, so the bench aborted on all 12 shapes before timing. B1 now pins use_cfvst=False (the CFVST-off reference), B2 is production dispatch, downstream stages inherit the policy, and A pins use_cfvst=False to rebuild the pre-ROCm#14367 Step-0 winners (symbol gains the cfvst0 token). All 12 shapes pass the pre-timing guards. Date the case-study results to cc84e26 (pre-ROCm#14367) instead of implying they were measured at the PR head. test_attention_ir_cpp_parity now also lowers the 12 gfx942 KDB/VPF golden cases through both engines.
AviralGoelAMD
changed the title
Contributor
|
@cgmillette thanks — addressed the follow-up at
Local, at |
…t-A guard EXPECTED_A_KERNEL again holds the seven recorded symbols verbatim, so A_original_expected_kernel / A_kernel_name_matches_original stay truthful (the rebuilt symbol now differs by the cfvst0 token). The guard compares against CURRENT_EXPECTED_A_KERNEL (historical + cfvst0), which is also exported alongside a separate match flag.
Contributor
|
Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max PR #13205 — add opt-in gfx942 BF16 D128 K LDS double buffering and V register prefetch (Parthiv911, AICK-1900) — 🟡 Follow-up: bench fixed; squash message stale
Net-new: none. Utility: 🟢 Positive — opt-in KDB/VPF on develop's CFVST body; default path unchanged; numerics author-run. 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, build + ctest, C++/Python parity at 4 flavors with a mutation check of the KDB/VPF cases, goldens, PR tests, default path vs develop, the bench's own pre-timing path on 12 shapes, history, body and armed merge message scan, threads, CI at head. Flags whether a closer look is needed; not an approval or rejection. |
ex-rzr
added a commit
that referenced
this pull request
Nov 3, 2026
…ke/dense-paged-kv Conflicts with #13205 (gfx942 bf16 cfvst, K LDS double buffering, V register prefetch) and the attention CPU test list (#13953). All hunks keep both sides: - gfx942 _async_load takes develop's kbuf slot offset and this branch's per-wave page lookup. The KDB/VPF pipeline requires persistent + cfvst, and paged K/V takes neither, so paged never reaches it. - Name-injectivity table: lds_num_buffers is buildable on develop and the paged fields are buildable here, so neither stays in the unbuildable set; both new bases (persistent KDB/VPF, paged) are kept. - The gfx942 numeric file keeps the paged class and develop's pipeline tests. - CMake: develop's SDPA host tests plus the non-Windows gfx942 dense block. No golden re-bless needed: the representative and differential goldens pass as merged.
1 task done
AviralGoelAMD
added a commit
that referenced
this pull request
Nov 3, 2026
…ed-3d-hipdnn Brings in 52 develop commits, among them #13972 (kernel slots: SDK runtime, tooling and hkp_pack packing), #13205 and #13652 (gfx942 rocKE changes). Resolution: dnn-providers/integration-tests/SUPPORT_MATRIX_OVERVIEW.md is re-rendered from the merged claims (feature_matrix.py; --check passes). Checked on the merge: the four rocKE answer tables are unchanged; g1-g4 pass (rocKE still selects the shipped 4,566 kernels); launch_surface --check passes; 199 sampled recipes lower to identical segment + reduce IR (398/398); byte identity GREEN; IR parity and differential goldens OK; IngestorGenerator suite 1623 passed; rocKE-on clang-tidy build: Unified3d host tests pass (5 GPU-only skips); the new hkp_pack output passes the census (5/5), verify_variant_sets --mode full and coverage_gate rungs 1-2.
Contributor
TheRock Submodule Bump ActivityNewest first
|
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.
This PR introduces opt-in K LDS double buffering and V register prefetch for gfx942 BF16 D128 dense causal attention, on the CFVST body.
JIRA ID: AICK-1900
Benchmarking, profiling and tracing highlighted bottlenecks in:
We implemented following optimizations:
These optimizations are independently selectable through kernel-spec knobs, with the combined configuration accessible through explicit tuning selection and the full tuning space. Default dispatch behavior and the curated production tuning policy remain unchanged.
Case study at
Performance Gains and Ablation: Each Configuration vs. Production Dispatch
Benchmarked at
cc84e262on a clean checkout (the PR head adds docs only;--verify-provenancepasses), gfx942 with 304 CUs, twelve persistent shapes. All 546 FP32-reference checks passed. Two min–max ranges include a single slow round of one arm, kept as measured (see the case study).Register usage at the benchmarked HEAD
gfx942, B1 S4096 Hq32 Hkv8 bf16 D128 causal, persistent-304,
llvm23, from the HSACOs timed in the benchmark run atcc84e262(static_resources.csv). Thewaves_per_eu=2cap is 256 VGPRs. Every B arm is spill-free with 0 B scratch on all twelve benchmarked shapes.cc84e262)lds_row_pad=4lds_row_pad=4+causal_diag_split=TrueKERNEL_AUTHORING.md checklist
Core DoD -- every change
1. Add test coverage
a. Numeric parity
dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_numeric.pyTest cases added appropriately
b. problem -> spec
Applicable to the explicit opt-in tuning path. This PR does not change the default production dispatch policy.
The optimizations are selectable through tuning_spec_with_knobs(req, "gfx942_dense", knobs), including CFVST off, K LDS double buffering, V register prefetch, and the combined configuration.
Selection, default preservation, legality, and tuning-ID replay are covered by library/tests/dispatch/attention/test_tuning_space.py, particularly TestGfx942DenseTuningSpace.test_bf16_cfvst_kdb_vpf_sweep_and_explicit_selection.
c. spec -> IR
Ran dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942_golden.py for llvm20, llvm22, llvm23 and llvm24.
The test compares SHA-256 hashes of Python-lowered LLVM IR against per-flavor golden fixtures. Coverage includes CFVST, KDB, VPF, their combined configuration, persistent execution, and LSE variants.
The goldens pass through both engines in strict mode (
ROCKE_BACKEND=both), andcheck_byte_identity.pyis green at all four flavors.d. re-blessed golden since emission changed
reblessed and tested by c above.
2. run_checks.py green on the platforms in the testing matrix
dnn-providers/hip-kernel-provider/rocke/tools/run_checks.pyAt the PR head on gfx942, each file run separately:
test_attention_dense_gfx942.py377 passed,_golden.py3 passed,_lds.py182 passed,dispatch/attention/test_tuning_space.py55 passed (264 subtests),_numeric.py238 passed (0 skipped).3. Correctness before speed
Numeric tests already done above in 1.a.
5. Docs
Case study at
dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.md6. Hygiene
No such files in PR
DoD by change type
D. Perf optimization (no new feature)
a. Step 0 exhaustive lever sweep before concluding a gap is structural
Conducted non exhaustive lever sweep. Methodology and results available in case study.md.
b. Same-session A/B ratios (median of ≥3) at production-representative scale; absolute µs treated as illustrative
Methodology followed and numbers provided above and in case study.
c. Replayable case study + runbook/measured-results update
replayable case study: case study at
dnn-providers/hip-kernel-provider/rocke/library/builders/gfx942/attention/prefill/gfx942_dense_cfvst_kdb_vprefetch_case_study.mdrunbook: lever heuristics added to
platform/dsl_docs/optimization/optimization_runbook.md§16.1measured-results update: NA —
measured_results.mdis a frozen archive; results are in the case study.d. Honest losses recorded, not just wins
No regression against production dispatch at the twelve measured shapes. At B=1, S=1024/2048 the earlier twelve-shape Step-0 sweep's winners (non-persistent) outperformed the pipeline; recorded as a loss in the case study.
Historical review fixes (pushed by @AviralGoelAMD,
6e35a8a6d3a..8dec2cd5ce8)Merges
develop(including #12620, gfx942 dense sinks) and addresses @aosewski's review comments 2-6. The changes are scoped to those comments.kernels/gfx942/attention_dense.py(lse_store)m_iplus develop's sinks-awarerow_has_mass. The pipeline'sloop.resultsbinding is renamedloop_res, so a staleres[0]read fails at build time.kernels/gfx942/attention_dense.py(supports_attention_dense)use_sinkson the bf16 CFVST andlds_num_buffers=2paths. #12620 removed the blanket sinks rejection, and sinks x CFVST/KDB-VPF has no golden or numeric coverage.tests/test_attention_dense_gfx942.pytests/golden/*.json,tests/test_attention_dense_gfx942_golden.py_use_cfvstand module docstringstests/test_attention_dense_gfx942.py(_INJECTIVITY_BASES)lds_num_buffers=2: plain,iglp, persistent), so the kernel-name injectivity check perturbs every knob inside the pipeline body.kernels/gfx942/attention_dense.pycomments and docstringstests/test_attention_dense_gfx942_numeric.py(256, 192)in the strict pipeline test, and ablock_m=64knob row.tests/test_attention_dense_gfx942_numeric.pybuilders/gfx942/attention/prefill/README.mdtests/test_attention_dense_gfx942_numeric.py(_COHORT)torch.testing.assert_close(atol = rtol = per-dtype tol) instead of a raw max-abs bound that output rounding mostly consumes in bf16.kernels/gfx942/attention_dense.py(supports_attention_dense), tests, goldenlds_num_buffers=2gated toblock_n=64, where it was measured. Atblock_n=32the second K slot lowers occupancy atblock_m=128on both toolchains. The bn32 pipeline golden case is removed.Register usage: ROCm 7.1 vs ROCm 10
gfx942, B1 S4096 Hq32 Hkv8 bf16 D128 causal. Values come from the code-object notes (
.vgpr_count/.vgpr_spill_count/.private_segment_fixed_size). Thewaves_per_eu=2cap is 256 VGPRs.llvm20: VGPR / spills / scratch Bllvm23: VGPR / spills / scratch Bcc84e262)use_cfvst=Truelds_num_buffers=2emit_lse=Truepersistent=True,num_persistent=304llvm23the KDB/VPF pipeline fits; only the persistent grid spills.llvm22was not measured.Validation of
e3ccb2e37cetest_attention_dense_gfx942_numeric.py -k "cfvst_pipeline or kdbvpf or lse or sink": 165 passedllvm23), gfx942 304-CUkdbvpf_d128_bf16_bn32was removed in 8dec2cdres[0]without the rename:lse_kdbvpf_d128_bf16_causaldrifts in all 3 flavors.res[0]with the rename:UnboundLocalError.row_has_key:lse_sink_persist_empty_window_d128_bf16drifts in all 3 flavors. New rejection rows: 3 fail with the gate removed.pre-commit run --from-ref origin/develop --to-ref HEAD