Repository navigation
fix(rocke): honour the block-table stride in the scalar attention fallback - #13889
Conversation
…keys The tiled 2D and 3D cache keys are computed per launch without building the spec, so they must carry the resolved output of every selector the spec builder consults. Three did not reach the key: - binary_search_iters, the seq_idx search trip count every tiled spec bakes into IR from num_seqs. Only the raw num_seqs key field separated it. - use_softmax_mfma_interleave and use_k_single_buffer, gated on the gfx950 single-batch cohort (num_seqs == 1). Also separated only by raw num_seqs. - use_fp8_mfma_qk, which reads max_seqlen_k. Nothing in the key reads that field, so two fp8 problems straddling max_seqlen_k = 16 * T shared one key and the cache served whichever kernel compiled first. Add those outputs, plus the combo predicate and sched_barrier, so the key covers every spec field the builder resolves. The interleave / K single-buffer / sched_barrier entries are gated on the single-batch cohort so other launches skip those selector walks. _enable_fp8_mfma_qk now checks max_seqlen_q before the loader-fit walk; both checks are pure, so the result is unchanged. The fp8 entry is skipped on gfx1250, whose builder never reads the field. Raw num_seqs stays in the key here. The follow-up drops it. test_attention_tiled_cache_key sweeps every num_seqs / max_seqlen_q / max_seqlen_k selector threshold on gfx942, gfx950 and gfx1250, 2D and 3D, and asserts an equal key implies an equal built spec, builder route and compile backend. Co-Authored-By: Claude <noreply@anthropic.com>
With every resolved selector output in the key, raw num_seqs only split problems that build the same kernel. Remove it from _tiled_cache_key and _tiled_3d_cache_key. Batch count now reaches the key only through _binary_search_iters, so 2D and 3D binaries are shared within a log2 batch bucket (and across all batch counts on gfx1250 3D, which floors the search at 32 iterations). The launch-geometry caches still need the batch count: both grids add one padding q-block per sequence. The 2D launch-meta key and the 3D prepared-pipeline key now go through _shape_key, which appends total_q and num_seqs; the 3D prepared pipeline covers both grids and the workspace, not the workspace alone. The 3D CUDA-graph key gains num_seqs too, since the captured launch bakes in the num_seqs kernarg and the segment grid. test_attention_tiled_cache_key now asserts both directions: an equal key implies an equal build, and an equal build implies an equal key, so a raw shape field or a selector the builder never reads fails as over-keying. New pair tests check that same-output shapes share a key and lower to byte-identical IR at llvm20 and llvm22, and that launch meta still separates batch counts under one key. Co-Authored-By: Claude <noreply@anthropic.com>
benchmarks/common/tiled_cache_key_ab.py compares two rocke trees, each imported in its own process, on the tiled 2D/3D attention launch path. --mode ir (CPU) routes a sweep that straddles every num_seqs / max_seqlen_q / max_seqlen_k selector threshold like backend="auto", lowers each distinct full spec to LLVM IR, and checks per problem that the binary the launcher cache serves is the one the problem builds from its own spec, and that this IR matches the baseline tree's. It also reports compiled-binary counts per tree. --mode ab (GPU) runs a smaller sweep end to end with arm order alternated across rounds and a fresh process per arm per round, and records first-launch time, steady-state latency, max_abs against an fp32 reference, cache-key host cost and compiled-binary counts. Measured numbers stay out of the repo. Co-Authored-By: Claude <noreply@anthropic.com>
…e-tiled-cache-key
…cache-key test and harness
The tiled 2D cache key is computed on every launch. Two of its entries re-ran selectors whose outputs the key already holds: _enable_k_single_buffer re-walked the num-warps, rows-per-warp and tile selectors for its block_m <= tile_size check, and _enable_fp8_mfma_qk re-walked the tile and num-warps selectors for its loader-fit check. On short, host-bound launches that recomputation was visible per call. _tiled_cache_key now resolves num-warps, tile size and rows-per-warp once and passes them to both predicates (and to _fp8_qk_loader_fits) through new keyword-only parameters. The parameters default to None, in which case each helper resolves the value itself, so the spec builder and every other caller are unchanged. The generic 2D warp count is only passed when it is the count the key records; on the gfx942 fp16 flash path the predicates still resolve it themselves. This is an exact rewrite: the key tuple is identical for every problem in the cache-key test sweep and its fp8 / single-batch extensions on gfx942, gfx950 and gfx1250, and test_attention_tiled_cache_key still enforces key equality <=> build equality in both directions. Co-Authored-By: Claude <noreply@anthropic.com>
…redicates The tiled cache key evaluates the gfx950 single-batch schedule predicates on every launch in that cohort. _enable_softmax_mfma_interleave and _enable_d128_small_tile read their environment knob before the head-size and fp8 checks, and _enable_sched_barrier walked the V-schedule predicates before checking head size, so every d64 single-batch launch paid for predicates that can only fire at head size 128. Check the problem fields first. The result is unchanged for every input; the cache key is identical across the cache-key test sweep and its fp8 / single-batch extensions on gfx942, gfx950 and gfx1250, with the knobs both unset and disabled.
…e-tiled-cache-key
…e-tiled-cache-key
…e-tiled-cache-key Co-Authored-By: Claude <noreply@anthropic.com> # Conflicts: # dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py
The scalar fallback compiled the block-table row stride as ceil(max_seqlen_k / block_size) and ignored the caller's table stride. A table with rows wider than the batch needs made every sequence after the first read the wrong blocks. Because the stride was baked into the IR, the cache key also had to carry raw max_seqlen_k, and the kernel name carried total_q, so each batch shape compiled its own binary. - Add a trailing block_table_stride kernarg to the scalar 2D and 3D kernels and index block-table rows with it, in both engines. - Launch the scalar kernel with the table's real row stride. - Drop q<total_q> from the 2D, 3D and reduce scalar kernel names. - Drop total_q, num_seqs, max_seqlen_q and max_seqlen_k from the scalar _cache_key; none of them reach the IR any more. - Add CPU tests for key sharing, byte-identical IR across shapes at every LLVM flavor, and the launch signature, plus a GPU test with wide and sliced block tables against an fp32 reference. Emitted IR changes for the scalar kernels only (name, new kernarg, row stride multiply). The representative golden has no scalar cases. Co-Authored-By: Claude <noreply@anthropic.com>
The harness compared only the tiled 2D/3D launcher keys. The scalar
fallback key change needs the same evidence: per-problem IR identity,
compiled binary count, and A/B latency with max_abs against fp32.
- Add --family {tiled,scalar}; scalar routes problems through the
scalar gate, keys them with _cache_key, lowers the 2D scalar kernel
the launcher compiles, and launches with backend="scalar".
- Use smaller scalar sweeps, since every problem is lowered and the
kernel is slow.
- In the scalar family, report but do not fail on own-IR differences
between arms: a key change that also changes the kernel ABI makes
them differ by design.
Co-Authored-By: Claude <noreply@anthropic.com>
✅ 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. |
…e-drop-shape-from-cache-key Brings in #13372 (squash-merged) and later develop work. Resolution: - KERNEL_AUTHORING.md, builders/common/README.md and test_attention_tiled_cache_key.py: develop's version. This branch never edited them. - attention_unified.py: develop's file with this branch's scalar changes re-applied. The automatic merge also kept #13372-era tiled key code that develop has since replaced (a second search-bucket entry in the 3D key and an unused math import); that is dropped. - tiled_cache_key_ab.py: removed, as #13372 did before merging. Co-Authored-By: Claude <noreply@anthropic.com>
The scalar 2D and 3D kernels now take block_table_stride as a kernarg and drop q<total_q> from their names, which moves the representative IR of the scalar cases develop added since this branch was cut. - Update sha256, bytes and kernel_name for 7 scalar cases at llvm20, llvm22 and llvm23. No other entry changes; the stale summary block is left as develop has it. Co-Authored-By: Claude <noreply@anthropic.com>
yraparti
left a comment
There was a problem hiding this comment.
rocKE checklist review
ROCm/rocm-libraries#13889 · ec41895c3412 · model gemini-4-argon, fallback gpt-5.5
PASS · graded 238/238 · BLOCKER 0, HIGH 1, MEDIUM 2, LOW 0, NEEDS-INFO 10
The diff was truncated before grading. Items that needed a missing hunk are NEEDS-INFO.
Findings
- HIGH §Q ·
dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:1· Registertest_attention_scalar_cache_key.pyandtest_attention_scalar_block_table_numeric.pyin the appropriateCMakeLists.txtandROCKE_ENGINE_test_categories_external.yaml, and name the CI lane in the PR description. - MEDIUM §C ·
dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6336· Call _physical_block_and_token in build_unified_attention_2d and rocke_unified_attn_physical_block_and_token in rocke_attn_unified_emit_2d_softmax_loop (matching the align=4 / default align as needed) so the block-table index calculation lives in one helper per engine. - MEDIUM §F · summary · Retitle as
type(scope): summary, 10–80 characters, scope[a-z0-9-]+.
Needs info
- §A.6 Both the cross-engine
check_byte_identity.pygate and the Python representative-IR golden (dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:229) are updated/verified, and C++ twins indnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:191-257,331match Python. — no diff line supports this verdict - §A.8 C++ changes in
dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:191-257,331,387,455,dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_2d.cpp:124,dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_3d_reduce.cpp:155,178, anddnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_selectors.cpp:1309,1346,1447were reviewed by hand and verified withcheck_byte_identity.py. — no diff line supports this verdict - §C.5 dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:757, dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:188, and dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:229-275 restrict the diff to the Python/C++ scalar attention twin files, their headers, two focused tests, and the 7 affected scalar golden entries. — no diff line supports this verdict
- §F.6 Local pre-commit execution status and full file formatting cannot be verified from the truncated PR diff alone.
- §F.7 Bot, Copilot, and human review threads are not included in the provided PR context.
- §H.2 The diff modifies
physical = b.global_load_i32(block_tables, b.add(b.mul(seq_idx, bt_stride), block_idx))indnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6337, but any host-side range validation ofblock_tablesvalues orbt_strideinrun_unified_attention_torchis outside the visible diff hunks. - §H.5
run_unified_attention_torchpassesbt_stride=bt_stride, include_bt_stride=True(dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6132), but the host validation ofblock_table/bt_strideinrun_unified_attention_torchand existing negative tests are not included in the diff hunks. - §Q.4
dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:229is truncated in the diff after line 277, so the golden updates across all entries inLLVM_FLAVORS(llvm20, llvm22, llvm23) cannot be verified from the diff.
Section notes
- §B: This PR updates the scalar attention fallback to pass block_table_stride as a kernarg and removes dynamic batch shape fields from the scalar launcher cache key and kernel names; it does not modify any rejection gates, routing, or spec validators.
- §D: Scalar unified-attention cache key, kernel name, and block_table_stride kernarg alignment across Python and C++.
- §D: Scalar unified-attention fallback cache-key normalization and block_table_stride kernarg fix (both Python and C++ engines); no dispatcher candidate registration or routing changes.
- §F: PR title violates Conventional Commits scope and length rules (contains slash in scope
rocke/attentionand is 95 chars > 80). - §G: Graded items 1-17 of Section G.
- §G: Items 18–34 of 34 in section G.
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 | Python scalar unified-attention builders in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6279,6338,6509 are mirrored in C++ in dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:387,455 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_2d.cpp:124. |
| 2 | N/A | This PR does not modify core lowering ops or op handlers under core/lower_* or core/ir. |
| 3 | PASS | Kernel-level helpers and signatures in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6863,6943,6966 are mirrored in C++ in dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_selectors.cpp:1309,1346,1447 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:331. |
| 4 | PASS | C++ emit order matches Python: block_table_stride param is declared after num_seqs (dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:387,455 vs dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6279,6509) and rocke_b_mul(b, ctx->seq_idx, ctx->block_table_stride) uses already-bound values so no nested right-to-left evaluation order bug occurs… |
| 5 | PASS | The representative-IR golden is re-blessed only for the intentional scalar attention IR/name changes in dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:229-277. |
| 6 | NEEDS-INFO | Both the cross-engine check_byte_identity.py gate and the Python representative-IR golden (dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:229) are updated/verified, and C++ twins in dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:191-257,331 match Python. — no diff line supports this verdict |
| 7 | PASS | Changes are scoped strictly to the scalar fallback kernels (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6338,6977 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_2d.cpp:124), replacing a constant multiply with the kernarg multiply without adding extra ops or touching tiled paths. |
| 8 | NEEDS-INFO | C++ changes in dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:191-257,331,387,455, dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_2d.cpp:124, dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_3d_reduce.cpp:155,178, and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_selectors.cpp:1309,1346,1447 were reviewed by hand and verified with check_byte_identity.py. — no diff line supports this verdict |
| 9 | N/A | This PR does not contain a shared op-handler fix or a lowerer-only arch divergence. |
| 10 | PASS | The updated scalar 2D and 3D IR is covered by representative golden hash entries across flavors in dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:229-277. |
| 11 | N/A | This PR does not introduce a new IR opcode. |
| 12 | N/A | This PR does not modify core/backend.py or ROCKE_BACKEND fallback behavior. |
| 13 | N/A | This PR does not add timestamps, build IDs, version stamps, or debug locations to emitted .ll. |
| 14 | N/A | This PR does not modify any classic helpers under platform/cpp/helpers/. |
§B graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR does not add or modify any rejection gates or support checks. |
| 2 | N/A | This PR does not touch preflight gates or spec constructor validation logic. |
| 3 | N/A | This PR only modifies the scalar fallback's cache key and block-table stride indexing without changing dispatch routing or demoting any shapes. |
| 4 | N/A | This PR does not add or modify any input validation, normalization, coercion, or spec/request boundary checks. |
| 5 | N/A | No gate scope is opened or modified in this PR; the changes only affect cache key composition and stride indexing within the existing scalar fallback. |
| 6 | N/A | This PR does not modify is_valid_spec or any supports_* functions. |
§C graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6336-6339 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_2d.cpp:121-128 duplicate the magic_div_mod + global_load_i32 block-table lookup instead of calling the existing _physical_block_and_token (attention_unified.py:6962) / rocke_unified_attn_physical_block_and_token (attention_unified_selectors.cpp:1306) helpers. |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:761-775 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:72-81 use the single family builders build_unified_attention_2d/3d/reduce and remove batch-shape specialization from the cache key. |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6172-6177, 6450-6455, 6623-6627 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:191-257 remove the workload-size tag q<total_q> from the 2D, 3D, and reduce scalar kernel_name() implementations. |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:757 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_2d.cpp:120 keep the arch-independent scalar attention logic in common/ without arch-specific branches. |
| 5 | NEEDS-INFO | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:757, dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:188, and dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:229-275 restrict the diff to the Python/C++ scalar attention twin files, their headers, two focused tests, and the 7 affected scalar golden entries. — no diff line supports this verdict |
| 6 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:5594, 6133 reuse the existing include_bt_stride=True signature and launch plumbing already established for tiled kernels rather than introducing a bespoke workaround. |
| 7 | N/A | This PR is a bug fix and cache-key normalization that intentionally changes emitted scalar kernel IR and names, not a no-op cleanup refactor. |
| 8 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:5592-5596 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:331 reuse the existing _attn_signature(include_bt_stride=True) and rocke_signature_builder_scalar infrastructure. |
| 9 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6172, 6279, 6450, 6509, 6623 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:188-257, 387, 455 fix all sibling sites across 2D, 3D, and reduce scalar kernels in both Python and C++ engines, and remove the dead max_blocks field in instance_attention_unified_internal.h:108-111. |
| 10 | N/A | This PR does not widen a gate set, shared arch branch, or introduce a new multi-field source of truth. |
§D graded 21/21
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | No new knobs, features, or environment variable reads are added in this PR. |
| 2 | N/A | No KernelCandidate or Capability definitions are added or modified in this PR. |
| 3 | N/A | No candidate _supports predicates or admits() calls are added or modified in this PR. |
| 4 | N/A | No selector or spec-builder routing logic on the dispatch selection path is modified in this PR. |
| 5 | PASS | Dynamic shape fields (total_q, num_seqs, max_seqlen_q, max_seqlen_k) are removed from _cache_key (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:766) and kernel_name (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6172, 6449, 6622; dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:191) once block_table_stride becomes a… |
| 6 | N/A | No cohort routing is added or modified in this PR. |
| 7 | N/A | No hand-pinned overrides or heuristic tables are touched in this PR. |
| 8 | N/A | No cohort predicates are defined or modified in this PR. |
| 9 | N/A | No new kernel or architecture is registered in the dispatcher registries in this PR. |
| 10 | N/A | No new spec field or feature is added; block_table_stride is an existing runtime tensor stride passed as a kernarg. |
| 11 | N/A | No candidate registration, opt-in flag, or arch default promotion is changed in this PR. |
| 12 | N/A | This PR does not add, rename, or reprioritize any dispatcher candidates or custom rankers. |
| 13 | N/A | This PR does not modify CandidateRegistry.select, pin handling, or add a new dispatch wrapper. |
| 14 | PASS | The scalar launcher compile cache key in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:766-778 drops problem-shape fields (total_q, num_seqs, max_seqlen_q, max_seqlen_k) so compiled binaries key only on arch and static configuration rather than recompiling per shape. |
| 15 | N/A | This PR does not add a dispatcher candidate or arch module. |
| 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 modify candidate routing cohorts. |
| 18 | N/A | This PR does not touch CandidateRegistry ratchets, binding flags, or exempt sets. |
| 19 | PASS | The PR makes no dispatcher registration claims and accurately scopes its changes to the scalar fallback launcher and builders in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:5591-5597 and :6129-6136. |
| 20 | PASS | Kernel-level signature, kernel-name, and builder changes in Python (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6171-6177, :6279) are mirrored in C++ (dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:188-209, :328-334, :383-388). |
| 21 | N/A | This PR fixes a kernel ABI/builder bug where block_table_stride was hardcoded from max_seqlen_k in the scalar kernel rather than a spec-selection issue. |
§E graded 17/17
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:31 imports from platform (rocke.core.lower_llvm) into library, and no platform file imports library. |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:757 modifies library kernel code without adding any torch imports for device queries. |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:30 gates torch usage with pytest.importorskip("torch"), while dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:25 runs without torch. |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:22 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:30 only import downward into kernels.common. |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:190 and other platform changes contain no hardcoded absolute or repo-escaping paths or shell scripts. |
| 6 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6276 and :6507 preserve intentional unused param handles (_out_scale, _softcap) rather than stripping them. |
| 7 | N/A | This PR does not introduce new ROCKE_* environment variables, dtype support, public ops, kernel families, manifest kinds, or tuning knobs. |
| 8 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:761 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:4 use descriptive explanations without Jira IDs, PR numbers, or usernames in code. |
| 9 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:766 and :6279 show all changes are tightly scoped to passing block_table_stride as a kernarg in scalar attention and removing dynamic shapes from its cache key and kernel names. |
| 10 | PASS | dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:231 updates only the 7 affected scalar attention golden entries without reverting unrelated entries. |
| 11 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:1 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:1 are deliverable test suites with no scratch files committed. |
| 12 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:111 tests both gfx942 and gfx950 cache keys, and the scalar builder changes are arch-generic across all targets. |
| 13 | PASS | dnn-providers/hip-kernel-provider/rocke/platform/cpp/include/rocke/instance_attention_unified_internal.h:110 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_prologue.cpp:125 cleanly remove the dead max_blocks field and its initialization. |
| 14 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6337 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_2d.cpp:123 remove only the intended compiled max_blocks stride calculation. |
| 15 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6977 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_selectors.cpp:1326 use the newly added bt_stride parameter in the address calculation. |
| 16 | N/A | This item applies to review grading output rather than code changes in the PR diff. |
| 17 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:761 and the PR description report only qualitative latency observations without any absolute performance numbers. |
§F graded 11/11
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Branch users/dlangbe/rocke-drop-shape-from-cache-key matches users/<user>/… (diff seen at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:760). |
| 2 | FAIL | title fix(rocke/attention): honour the block-table stride in the scalar fallback and drop shape from its key is 102 characters and does not match type(scope): summary with a 10–80 character length |
| 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/test_attention_scalar_block_table_numeric.py is in the diff |
| 5 | PASS | No forbidden files (keys/certs/secrets) are in the file list (only .py, .h, .cpp, and .json files such as dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:760). |
| 6 | NEEDS-INFO | Local pre-commit execution status and full file formatting cannot be verified from the truncated PR diff alone. |
| 7 | NEEDS-INFO | Bot, Copilot, and human review threads are not included in the provided PR context. |
| 8 | N/A | This PR targets develop directly rather than being a stacked PR on a feature branch. |
| 9 | PASS | The PR includes both Python and C++ changes plus tests and golden updates in one cohesive change without artificial splitting (e.g., dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:760 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_2d.cpp:123). |
| 10 | PASS | The added tests in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:85 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:113 test real cache-key, IR-identity, and numeric block-table stride invariants rather than throwaway checks. |
| 11 | PASS | The PR title and body fields are populated in the PR metadata describing changes in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:760. |
§G graded 34/34
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:75 |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:25 |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:231 |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:85 |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:95 |
| 6 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:761 |
| 7 | N/A | This PR does not add or modify a design document. |
| 8 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:30 |
| 9 | N/A | This PR does not make load-bearing LDS or occupancy claims. |
| 10 | N/A | This PR is a correctness and cache-key fix and does not make headline performance ratio claims or ship a benchmark script. |
| 11 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6133 |
| 12 | PASS | dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_2d.cpp:127 |
| 13 | N/A | This PR does not add or recommend LLVM intrinsic symbols. |
| 14 | N/A | This PR does not classify hardware bottlenecks. |
| 15 | N/A | This is a process reminder for git object inspection rather than a diff property. |
| 16 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:5594 |
| 17 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:113 |
| 18 | N/A | This PR does not add a statistical runtime guard or back-pressure detector. |
| 19 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:41-56 allocates an extra poison block filled with 64.0 and points unused wide-row block-table slots at it so a wrong stride corrupts the output instead of passing by luck. |
| 20 | N/A | This PR does not add or modify a text/claim ban detector. |
| 21 | N/A | This PR does not check in measurement or profiling harness deliverables. |
| 22 | PASS | dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:229-275 updates the scalar golden entries with valid bytes, sha256, and status: "ok" (and the PR test plan confirms GPU execution on gfx950 and CPU lowering across llvm20/22/23). |
| 23 | N/A | This PR is a correctness and cache-key fix for the scalar fallback, not a performance or routing speedup claim. |
| 24 | PASS | dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:229-275 re-blesses only the scalar cases whose kernel name and block_table_stride kernarg changed, and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:112-141 + the gfx950 GPU A/B sweep validate the changed scalar behaviour on GPU. |
| 25 | N/A | This PR is not a codegen micro-optimization PR. |
| 26 | N/A | This PR is not a performance benchmarking campaign PR. |
| 27 | N/A | This PR does not ship a bundled performance optimization toggle. |
| 28 | N/A | This PR does not present counterfactual performance tables. |
| 29 | N/A | This PR does not swap guarded math primitives for fast approximations. |
| 30 | N/A | This PR makes no register-spill or scratch-memory claims. |
| 31 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:5593-5594 and 6132-6133 show include_bt_stride=False and compiled max_seqlen_k stride existed in develop, and the PR verified that dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:112-141 fails against the pre-change tree. |
| 32 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:78-98 explicitly binds arch="gfx950" and iterates LLVM_FLAVORS (llvm20, llvm22, llvm23), matching the provenance stated in the PR test plan. |
| 33 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:757-777 changes _cache_key, and the PR differential-tables every dropped and retained field in the PR body and enforces the partition across _DYNAMIC_VARIANTS and _STATIC_VARIANTS in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:49-112. |
| 34 | N/A | This PR does not add or modify an opt-in kernel output flag. |
§H graded 5/5
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The scalar unified-attention kernels use global pointer loads (b.global_load_i32) rather than spec-sized AMDGPU buffer-SRD num_records, and this PR removes max_seqlen_q/max_seqlen_k from the compile-time cache… |
| 2 | NEEDS-INFO | The diff modifies physical = b.global_load_i32(block_tables, b.add(b.mul(seq_idx, bt_stride), block_idx)) in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6337, but any host-side range validation of block_tables values or bt_stride in run_unified_attention_torch is outside the visible diff hunks. |
| 3 | PASS | The scalar kernel uses _magic_div_mod(b, kpos, p.block_size) (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6336 and :6975) and rocke_unified_attn_magic_div_mod (dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_2d.cpp:121), which explicitly handles constant divisors. |
| 4 | N/A | The scalar unified-attention kernels iterate over runtime sequence bounds (kpos) rather than a fixed compile-time Skv/BN tile loop with a page mask. |
| 5 | NEEDS-INFO | run_unified_attention_torch passes bt_stride=bt_stride, include_bt_stride=True (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6132), but the host validation of block_table / bt_stride in run_unified_attention_torch and existing negative tests are not included in the diff hunks. |
§I graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR fixes scalar fallback block-table row stride handling and cache-key normalization without changing dispatch routing or candidate space measurement. |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:5594 and dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6279 repair the scalar kernel to consume the runtime block_table_stride kernarg instead of silently reading wrong rows with a compiled stride. |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:49 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:113 cover dynamic/static problem variations and wide/view block-table layouts. |
| 4 | N/A | This PR does not add or modify tuning/routing benchmark tables with shipped vs achievable columns. |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:231 updates the explicit representative-IR golden snapshot for the modified scalar attention cases. |
| 6 | N/A | This PR does not add any new DSL op or hardware-instruction-named primitive. |
§J graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR does not add or modify any Markdown or design documents. |
| 2 | N/A | This PR does not add or modify any Markdown or design documents. |
| 3 | N/A | This PR does not add or modify any Markdown or design documents. |
| 4 | N/A | This PR does not add or modify any Markdown or design documents. |
| 5 | N/A | This PR does not add or modify any Markdown or design documents, and adds no complexity claims in docstrings. |
| 6 | N/A | This PR does not add or modify any Markdown or design documents. |
| 7 | N/A | This PR does not add any new Markdown or design documents. |
§K graded 13/13
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR only touches the untuned scalar fallback kernels and does not define or modify any tuned candidates or KnobSpace subclasses. |
| 2 | N/A | No tunable kernel fields, spec fields, or KnobAxis definitions are added or modified in this diff. |
| 3 | N/A | The PR only changes the untuned scalar launcher cache key and scalar builder ABI, not any tuned spec defaults, knob meanings, or config_key payloads. |
| 4 | N/A | No KnobSpace.defaults() or recorded() methods are touched by this PR. |
| 5 | N/A | No knob drop, refuse, inert(), or base_value() rules are modified in this diff. |
| 6 | N/A | No KnobSpace.validate or tuning_specs.py policy-free spec construction code is touched by this PR. |
| 7 | N/A | No KnobSpace.base(req) implementation or tuning problem normalization is modified in this diff. |
| 8 | N/A | No knob value normalization or axis type coercion code is touched by this PR. |
| 9 | N/A | No benchmark recording, tuning replay, or PinRefused handling code is touched by this PR. |
| 10 | N/A | No tuning_id resolution or KnobSpace.find logic is touched by this PR. |
| 11 | N/A | No sweep entry points, pin_to_spec logic, or sweep harnesses are modified in this diff. |
| 12 | N/A | No production tuning stacks, accept_default rules, or tuned variant base specs are modified in this diff. |
| 13 | N/A | No sweep benchmarks, legality filters, or auto selection tables are touched by this PR. |
§L graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR does not add or modify any gfx targets or ISA backends. |
| 2 | N/A | This PR does not add or modify any MMA op_ids or fragment info. |
| 3 | N/A | This PR touches only the scalar attention fallback and does not select or modify MMA atoms. |
| 4 | N/A | This PR does not touch wave-size or lane-math logic. |
| 5 | N/A | This PR does not touch arch-specific MMA atoms, LDS transpose reads, or tile loaders. |
| 6 | N/A | This PR does not modify pipeline vocabulary, policy modules, or ArchTarget. |
| 7 | N/A | This PR does not add or change logical dtypes, bit packing formats, or FP8 dialect handling. |
| 8 | N/A | This PR does not touch scaled-MMA contracts or WMMA/MFMA scaled emission. |
| 9 | N/A | This PR does not introduce a new architecture target. |
| 10 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:31 imports LLVM_FLAVORS and line 90 parametrizes over LLVM_FLAVORS instead of using a hand-rolled flavor list; no intrinsic arch-domain artifacts are touched. |
§M graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | The new bt_stride SSA value is passed through IR arithmetic ops (b.mul, b.add) and helper arguments without driving Python host control flow or escaping scf regions (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6338, dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6977). |
| 2 | N/A | The diff does not touch legalize_for_target, tile.mma, or scf.for iter-args. |
| 3 | PASS | Wire serialization (core/ir_serialize.py and cpp/core/ir/serialize.cpp) is untouched, and the trailing block_table_stride parameter order matches identically across Python and C++ builders and launch signature (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6279, dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:331… |
| 4 | N/A | The diff does not touch portable IR schemas, importers, recipe expansion, or magic_division implementations. |
| 5 | N/A | The diff modifies scalar attention kernels and does not use AsyncTileLoader or LDS. |
| 6 | PASS | The modified global_load_i32 indexing uses the dynamic bt_stride kernarg unconditionally inside the existing loop without introducing barriers, divergent control flow, or speculative invalid loads (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6337, dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_2d.cpp:124). |
| 7 | N/A | The diff does not touch ISA backends, buffer-resource word 3, or temporal hints. |
| 8 | N/A | The diff does not add any intrinsic declarations or modify engine selection precedence. |
| 9 | PASS | Headers under cpp/include/rocke/ are not renamed, and the public C entry points in instance_attention_unified.h retain their stable signatures while internal helper headers update for bt_stride (dnn-providers/hip-kernel-provider/rocke/platform/cpp/include/rocke/instance_attention_unified.h:39, dnn-providers/hip-kernel-provider/rocke/platform/cpp/include/rocke/helper_helper_rocke.instances.common.attention_unified_selectors.h:279). |
| 10 | N/A | The diff does not touch debug info (!dbg) emission or DWARF consumers. |
§N graded 18/18
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR does not add or modify any module in platform/python/rocke/helpers/. |
| 2 | N/A | This PR only threads a scalar block_table_stride kernarg into existing scalar unified-attention block-table lookups and does not modify MFMA output mapping, TensorView DAGs, loaders, epilogues, pipelines, or… |
| 3 | N/A | This PR does not touch SoftwarePipeline or any pipelined kernel. |
| 4 | N/A | This PR does not add or modify fused kernels or epilogue fusion. |
| 5 | N/A | This PR does not add or change any public or private API in rocke.helpers.tiling. |
| 6 | N/A | This PR does not add or modify front-door/toolbox tiling helpers or LayoutStyle implementations. |
| 7 | N/A | This PR does not use or modify tiling MMA layout overrides or c_desc derivation. |
| 8 | N/A | This PR modifies only scalar unified-attention kernels and does not use tiling LDS or global memory verbs. |
| 9 | N/A | This PR does not touch N-D tiling tensor axis roles, FlowStage, or batched MMA loops. |
| 10 | N/A | This PR does not change tiling emit, LDS staging, MMA issue, or tiling fail-fast checks. |
| 11 | N/A | This PR does not touch mma_traits.json, TileMma, lds_swizzle, or derive_sched_group_counts. |
| 12 | N/A | This PR makes no LDS bank-conflict or tiling coalescing claims. |
| 13 | N/A | This PR does not construct or modify WarpDistributionEncoding, make_tile_desc, RegisterMapper, or tiling store casts. |
| 14 | N/A | This PR does not use tiling verbs, LDS staging, or pipeline barriers. |
| 15 | N/A | This PR does not touch tiling allocation or swizzled/padded descriptor bounds checks. |
| 16 | N/A | This PR does not touch optimize_layout, recommend(), or evaluate_transform. |
| 17 | N/A | This PR does not touch tiling_recorder.py, round-trip analysis, or tiling visualization. |
| 18 | N/A | This PR does not touch interleaved tiling layouts or demo kernels under helpers/tiling/kernels/. |
§O graded 16/16
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR does not modify verify.py, verify.cpp, address spaces, or required IR attributes. |
| 2 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6335 and dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6590, the scf.for bodies keep their iter-arg structure and end in b.scf_yield(new_m, new_l, new_acc). |
| 3 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:91, test_dynamic_shape_lowers_identical_ir verifies deterministic byte-identical lowering across dynamic shape variants and LLVM flavors. |
| 4 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6279 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:387, the new parameter uses the I32 / rocke_i32() type singletons. |
| 5 | N/A | This PR does not modify core/passes.py, cpp/core/passes.cpp, or op purity tables. |
| 6 | N/A | This PR does not touch constant folding in core/passes.py or cpp/core/passes.cpp. |
| 7 | N/A | This PR does not modify kernel attributes, agpr_alloc, waves_per_eu, or cluster_dims. |
| 8 | N/A | This PR does not touch CodegenPolicy or SchedulerStrategy. |
| 9 | N/A | This PR does not emit or modify waits, barriers, or optimization_barrier. |
| 10 | N/A | This PR does not modify the LLVM lowerers or parameter address-space handling. |
| 11 | N/A | This PR does not add or modify ArchTarget queries or target ID resolution. |
| 12 | N/A | This PR does not introduce hardware vector widths, size caps, or architecture defaults. |
| 13 | PASS | In dnn-providers/hip-kernel-provider/rocke/platform/cpp/include/rocke/instance_attention_unified_internal.h:146, only the internal rocke_attn_unified_build_ctx struct is modified (replacing max_blocks with block_table_stride), while public C structs and enums in instance_attention_unified.h are unchanged. |
| 14 | PASS | In dnn-providers/hip-kernel-provider/rocke/platform/cpp/include/rocke/helper_helper_rocke.instances.common.attention_unified_selectors.h:276 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:387, C++ changes follow rocke_* snake_case and keep the Python↔C correspondence comments updated. |
| 15 | N/A | This PR does not touch C++ engine fallback handling or LLVM flavor resolution. |
| 16 | PASS | In dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:331 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_2d.cpp:127, C++ changes remain strictly in codegen/IR building in parity with Python without fabricating runtime or launch behavior. |
§P graded 14/14
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Builder signatures are unchanged and tested without extra args in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:74. |
| 2 | PASS | The trailing block_table_stride kernarg is added in both Python (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:5594, :6279, :6509) and C++ (dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:331, :387, :455) and checked by test_launch_signature_matches_kernel_params in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:122. |
| 3 | N/A | This PR does not touch GEMM, implicit-GEMM, or block-scaled GEMM launch geometry. |
| 4 | N/A | This PR modifies scalar unified attention and does not touch LDS gates or LDS allocation. |
| 5 | N/A | This PR does not touch WMMA/RDNA GEMM, GemmMultiD, or GemmMultiAbd paths. |
| 6 | N/A | This PR does not add or modify caller-zeroed workspaces for Stream-K, MoE, or split-K. |
| 7 | N/A | This PR does not touch Stream-K, persistent GEMM, GroupedGemmLauncher, or SUPPORT_MATRIX.md. |
| 8 | N/A | This PR does not touch quantization helpers or MX GEMM scale decoding. |
| 9 | N/A | This PR does not add or modify numeric emulation or dtype decode paths. |
| 10 | N/A | This PR does not touch manifest kinds, manifest emitters, or manifest runners. |
| 11 | PASS | Scalar launches continue to go through _get_scalar_launcher and run_unified_attention_torch (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:5590, :6129) and the new GPU test synchronizes the stream before reading output on host (dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:108). |
| 12 | N/A | This PR does not touch COMGR or HIP library loading or fix a hipError launch code. |
| 13 | PASS | The scalar attention cache-key and stride-kernarg refactor is covered by both CPU cache-key/IR tests (dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:85) and GPU numeric tests including multi-batch binary reuse (dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:113, :126). |
| 14 | N/A | This PR does not touch examples, expected.json files, or promote modules to helpers/. |
§Q graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | FAIL | Two new test files dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:1 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:1 are added, but no CMakeLists.txt or ROCKE_ENGINE_test_categories_external.yaml file is modified in the PR file list, and the PR body does not name the CI lane that runs them. |
| 2 | N/A | This PR modifies the existing scalar unified-attention kernel family rather than introducing a new kernel family or variant (`dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.… |
| 3 | PASS | The PR separates cross-engine parity (check_byte_identity.py), IR golden change tripwire (dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:229), and actual GPU numeric verification (dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:113), explicitly noting where 3D C++ parity is not covered by the byte-identity gate. |
| 4 | NEEDS-INFO | dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:229 is truncated in the diff after line 277, so the golden updates across all entries in LLVM_FLAVORS (llvm20, llvm22, llvm23) cannot be verified from the diff. |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:29 skips cleanly via pytest.importorskip("torch") and torch.cuda.is_available(), while dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:90 iterates over LLVM_FLAVORS without requiring torch or a GPU. |
| 6 | N/A | This PR does not refactor any test gate or harness. |
| 7 | N/A | This PR does not change any rejection contracts or error-raising paths. |
§R graded 11/11
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR fixes block-table stride indexing and cache-key normalization in the scalar fallback and introduces no new kernel body, tiling strategy, pipeline variant, or structural performance claim. |
| 2 | N/A | No performance lever, architecture-specific optimization recipe, or heuristic knob is changed in this diff. |
| 3 | N/A | The diff touches no GEMM tile, LDS layout, prefetch pipeline, or launch occupancy lever covered by the optimization utilities skills. |
| 4 | N/A | The PR is a correctness and compile-cache fix that replaces an immediate stride with a scalar kernarg (block_table_stride) rather than a performance optimization claiming primitive changes. |
| 5 | N/A | The diff changes no performance lever or memory layout; the scalar block_tables lookup only replaces a compile-time row stride constant with the runtime block_table_stride kernarg for correctness. |
| 6 | N/A | No LDS or MMA layout or rocke.helpers.tiling pipeline is added or modified. |
| 7 | N/A | The PR makes no bottleneck classification claim or bottleneck-targeted optimization. |
| 8 | N/A | No waits, barriers, or scheduling hints (sched_barrier, sched_group_barrier, iglp_opt) are added, removed, or modified. |
| 9 | N/A | The PR is not a performance regression fix and does not tune a shape-trading performance lever. |
| 10 | N/A | The PR makes no codegen alias-semantics change, assembly parity claim, or occupancy optimization conclusion. |
| 11 | N/A | The PR introduces no new optimization lesson, heuristic, case study, or DSL probe tool. |
§S graded 8/8
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR does not add any new public Python symbols, C entry points, or parameters on compile_kernel/KernelLauncher/time_launches/run_manifest. |
| 2 | PASS | Changed scalar builders, kernel_name methods, signatures, and run_unified_attention_torch update golden expectations in dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:231-276 and add coverage in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:85-129 and… |
| 3 | PASS | No public Python or C API is removed or renamed; the C helper signature changes in dnn-providers/hip-kernel-provider/rocke/platform/cpp/include/rocke/helper_helper_rocke.instances.common.attention_unified_selectors.h:276-318 are updated at their callers in dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_3d_reduce.cpp:152-181 and the kernel ABI change is explicitly documented under 'ABI note' in the PR… |
| 4 | N/A | The PR does not modify any all lists or add public module-level exports. |
| 5 | PASS | Tests assert concrete behavioral contracts and numeric accuracy: IR text and param names in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:85-129 and FP32 reference comparison with poison blocks in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:113-141. |
| 6 | PASS | CPU-checkable contracts (cache key invariance/splitting, LLVM IR lowering, kernarg presence, signature matching) are tested without torch or a GPU in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:25-129, while GPU launch and numerics are separated into dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:28-141. |
| 7 | NEEDS-INFO | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:31,79 calls _lower_kernel_to_llvm_python directly rather than testing both Python and C++ backends, and the PR body notes that the C++ 3D scalar kernel (dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:452-455, dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_selectors.cpp:1306-1460) has no parity emitter and was only compile-checked; please confirm whether an existing test exercises rocke_build_unified_attention_3d_scalar and rocke_unified_attention_2d_scalar_signature through the C++ binding. |
| 8 | NEEDS-INFO | The PR body lists check_byte_identity.py, test_rocke_ci_static.py, and targeted pytest files, and names gfx950 for GPU runs, but does not state whether the README entry points python tests/test_rocke.py and python tests/run_all.py (including ctest) were run from rocke/platform/. |
|
Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max PR #13889 — honour the block-table stride in the scalar attention fallback — 🟠 First-round: closer look recommended — the fix is verified on a GPU, but its regression tests never run in CI
Utility: 🟢 Positive — fixes silently wrong attention output whenever block-table rows are wider than the batch needs (the usual framework layout), and stops compiling one scalar binary per batch shape. Legend: 🔴 blocking · 🟠 should-fix · 🟡 minor nit · 🟢 info/good · ⚪ retracted First-round review: a local qwen3.6 model made the independent first pass. The orchestrator (Claude Opus 5.5) verified it with the new numeric test on a gfx1151 GPU against develop, byte identity, the golden gate, the cache-key and scalar suites, an audit of the scalar kernels' callers, the CTest lists and the CI logs, and owns this merge-worthiness call. It flags whether a closer look is needed; it is not an approval or a rejection. |
…e-drop-shape-from-cache-key
The differential golden now runs in CI and anchors the scalar attention_unified and attention_unified_stress parity families. Their IR changed with the block_table_stride kernarg and the q<total_q> name drop, so their hashes are stale. - Re-record both families with --only attention_unified: 38 configs at llvm20, llvm22, llvm23 and in the canonical section. No other family's hashes change.
yraparti
left a comment
There was a problem hiding this comment.
rocKE checklist re-review of the fixes since 7189edc583ff
ROCm/rocm-libraries#13889 · 56ae95258ca1 · model gemini-4-argon, fallback gpt-5.5
PASS · graded 238/238 · BLOCKER 0, HIGH 0, MEDIUM 1, LOW 0, NEEDS-INFO 22
The diff was truncated before grading. Items that needed a missing hunk are NEEDS-INFO.
Since the last review (7189edc583ff)
Resolved 0 · still open 0 · new 1
- 🆕 §C.7 new FAIL (MEDIUM): Move the cleanup/deletion work to a separate no-op cleanup PR with unchanged parity/golden results, or remove it from this scalar attention bug-fix PR.
Findings
- MEDIUM §C ·
dnn-providers/hip-kernel-provider/rocke/library/builders/gfx1151/attention/bench_v_staging.py:1· Move the cleanup/deletion work to a separate no-op cleanup PR with unchanged parity/golden results, or remove it from this scalar attention bug-fix PR.
Needs info
- §A.8 Resolved from previous review: C++ changes in dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:203,232,255,331,387,455, dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_2d.cpp:122-129, dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_3d_reduce.cpp:155,178, and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_selectors.cpp:1309,1326,1346,1447 match Python in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6514,6571,6743,6794,6820,7211. — no diff line supports this verdict
- §C.5 Not resolved since the previous NEEDS-INFO: the review-response/current diff includes unrelated repo-wide changes such as .github/CODEOWNERS:5 and dnn-providers/hip-kernel-provider/config/ASM_SDPA_ENGINE.toml:22 alongside the scalar attention stride fix, so the blast radius is inflated rather than split. — no diff line supports this verdict
- §F.6 The new commits merge
developinto the branch; localpre-commitexecution status and full-file black formatting still cannot be verified from the PR diff alone. - §F.7 The new commits merge
developinto the branch; bot, Copilot, and human review thread states are still not included in the provided PR context. - §H.2 The new commits since 7189edc are a rebase onto develop;
physical = b.global_load_i32(block_tables, b.add(b.mul(seq_idx, bt_stride), block_idx))indnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:7210-7212has no device-side range check onphysical, and the host-side validation inrun_unified_attention_torchremains outside the visible diff hunks. - §H.5 The new commits since 7189edc are a rebase onto develop and do not touch
run_unified_attention_torchvalidation;bt_stride=bt_stride, include_bt_stride=Trueis passed indnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6367-6368, while the surrounding host validation ofblock_table/bt_strideand negative tests remain outside the visible diff hunks. - §J.1 Missing full Markdown hunks for design/algorithm docs such as dnn-providers/hip-kernel-provider/rocke/library/builders/gfx950/attention/ALGORITHM.md and dnn-providers/hip-kernel-provider/rocke/platform/dsl_docs/architecture/attention_thread_block_mapping_case_study.md, so equations and declared shapes cannot be type-checked.
- §J.2 Missing full Markdown hunks for the changed rocKE documentation, so notation tables, symbol domains, and log-base statements cannot be verified.
Section notes
- §A: Re-evaluated against current head 56ae952 (rebased onto develop). Both engines are updated in lockstep for the scalar unified-attention block_table_stride kernarg and kernel name changes, and both the representative-IR golden and differential golden are updated.
- §C: Re-review grading against the current diff, including changes pushed since the previous review.
- §E: The commit since the previous review is a clean rebase onto develop; all 17 items retain their previous grades against the current head.
- §G: Re-reviewed against current head 56ae952 (rebase onto develop). All previously passing items remain valid.
- §G: Re-reviewed items 18-34 against head 56ae952 (rebase onto develop); all previous grades hold.
- §H: Trace of device-side bounds/indirection in this PR: 1. Scalar unified-attention 2D/3D block-table lookup (
_physical_block_and_tokeninattention_unified.py:7209-7212androcke_unified_attn_physical_block_and_tokeninattention_unified_selectors.cpp:1319-1327): - Extent source:block_tablesindexed byseq_idx * bt_stride + block_idx, wherebt_strideis now passed as runtime kernargblock_table_stride(block_table.stride(0)), andblock_idx = kpos // p.block_size. - Visited range:kposloops over the runtime sequence KV range (no compile-timemax_seqlen_kbound in the kernel any more). - Agreement: Replacing the compiled(max_seqlen_k + block_size - 1) // block_sizestride with runtimebt_stridealigns the device row stride with the actualblock_tabletensor stride(0). 2. Indirection value (physicalblock ID loaded fromblock_tables): - Used directly to indexkeyandvalueglobal pointers via_paged_kv_descriptor(p).offset(...)without…
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 | Python scalar unified-attention builders in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6514,6571,6743,6794,6820 are mirrored in C++ in dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:387,455, dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_2d.cpp:122, and… |
| 2 | N/A | This PR does not modify core lowering ops or op handlers under core/lower_* or core/ir. |
| 3 | PASS | Kernel-level helpers, names, and signatures in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:5749,6368,6407,6683,6856,7097,7177,7200 are mirrored in C++ in dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_selectors.cpp:1309,1346,1447 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:203,232,255,331. |
| 4 | PASS | C++ emit order matches Python: block_table_stride param is declared after num_seqs (dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:387,455 vs dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6514,6743) and rocke_unified_attn_physical_block_and_token emits magic_div_mod then rocke_b_mul(b, seq_idx, bt_stride) with already-bound values before… |
| 5 | PASS | The representative-IR golden is re-blessed only for the intentional scalar attention IR and kernel name changes in dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:261-308. |
| 6 | PASS | Resolved from previous review: both the Python representative-IR golden (dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:261-308) and the cross-engine differential golden / C++ twins (dnn-providers/hip-kernel-provider/rocke/platform/tests/instances/differential/golden/llvm_gfx_all.json:36-73 and… |
| 7 | PASS | Changes are scoped strictly to the scalar fallback kernels (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6571,7211 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_selectors.cpp:1326), replacing the constant multiply with the kernarg multiply without adding extra ops or touching tiled paths. |
| 8 | NEEDS-INFO | Resolved from previous review: C++ changes in dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:203,232,255,331,387,455, dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_2d.cpp:122-129, dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_3d_reduce.cpp:155,178, and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_selectors.cpp:1309,1326,1346,1447 match Python in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6514,6571,6743,6794,6820,7211. — no diff line supports this verdict |
| 9 | N/A | This PR does not contain a shared op-handler fix or a lowerer-only arch divergence. |
| 10 | PASS | The updated scalar 2D and 3D IR is covered by representative golden hash entries in dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:261-308 and differential golden hashes in dnn-providers/hip-kernel-provider/rocke/platform/tests/instances/differential/golden/llvm_gfx_all.json:36-73, and tested across all LLVM_FLAVORS in… |
| 11 | N/A | This PR does not introduce a new IR opcode. |
| 12 | N/A | This PR does not modify core/backend.py or ROCKE_BACKEND fallback behavior. |
| 13 | N/A | This PR does not add timestamps, build IDs, version stamps, or debug locations to emitted .ll. |
| 14 | N/A | This PR does not modify any classic helpers under platform/cpp/helpers/. |
§B graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR does not add or modify any rejection gates or support checks. |
| 2 | N/A | This PR does not touch preflight gates or spec constructor validation logic. |
| 3 | N/A | This PR only modifies the scalar fallback's cache key and block-table stride indexing without changing dispatch routing or demoting any shapes. |
| 4 | N/A | This PR does not add or modify any input validation, normalization, coercion, or spec/request boundary checks. |
| 5 | N/A | No gate scope is opened or modified in this PR; the changes only affect cache key composition and stride indexing within the existing scalar fallback. |
| 6 | N/A | This PR does not modify is_valid_spec or any supports_* functions. |
§C graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | The repeated scalar block-table lookup is centralized in _physical_block_and_token at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:7200-7214 and the 2D loop now delegates to it at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6568-6570; the C++ twin delegates to rocke_unified_attn_physical_block_and_token at… |
| 2 | PASS | The scalar tests exercise the public family builders build_unified_attention_2d, build_unified_attention_3d, and build_unified_attention_reduce directly at dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:76-80, and the cache key removes batch-shape specialization at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:802-816. |
| 3 | PASS | The workload-size q tag is removed from the Python scalar kernel_name implementations at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6406-6415, dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6682-6691, and dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6855-6862; the C++ names likewise drop qbuf at… |
| 4 | PASS | The scalar attention implementation remains in common code, with the Python cache/build logic in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:797-816 and the C++ scalar 2D body in dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_2d.cpp:115-129 without arch-specific structural branching. |
| 5 | NEEDS-INFO | Not resolved since the previous NEEDS-INFO: the review-response/current diff includes unrelated repo-wide changes such as .github/CODEOWNERS:5 and dnn-providers/hip-kernel-provider/config/ASM_SDPA_ENGINE.toml:22 alongside the scalar attention stride fix, so the blast radius is inflated rather than split. — no diff line supports this verdict |
| 6 | PASS | The scalar fix reuses the existing include_bt_stride ABI plumbing at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:5745-5752 and dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6364-6366, and refactors the lookup through the named helper at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:7200-7214 rather than open-coding another patch. |
| 7 | FAIL | The current re-review diff contains large cleanup/deletion work such as deleting dnn-providers/hip-kernel-provider/rocke/library/builders/gfx1151/attention/bench_v_staging.py:1-345, while the scalar attention diff also changes golden hashes at dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:260-299, so the cleanup is not shown as a proven no-op in this PR. |
| 8 | PASS | The scalar path uses the existing _attn_signature include_bt_stride hook at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:5745-5752 and the existing C++ signature builder infrastructure at dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:303-334 rather than introducing a parallel ABI helper. |
| 9 | PASS | The bug is fixed as a class across sibling scalar sites: Python 2D, 3D QK, and 3D V-load call the stride-aware helper at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6568-6570, dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6798-6803, and dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6822-6824, while the C++ 2D and 3D paths pass… |
| 10 | PASS | The widened scalar ABI is propagated through consumers: the Python launch signature and packing include block_table_stride at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:5745-5752 and dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6364-6366, the C++ build context/signature add the field at… |
§D graded 21/21
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | No new knobs, features, or environment variable reads are added in this PR. |
| 2 | N/A | No KernelCandidate or Capability definitions are added or modified in this PR. |
| 3 | N/A | No candidate _supports predicates or admits() calls are added or modified in this PR. |
| 4 | N/A | No selector or spec-builder routing logic on the dispatch selection path is modified in this PR. |
| 5 | PASS | Dynamic shape fields (total_q, num_seqs, max_seqlen_q, max_seqlen_k) are removed from _cache_key (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:797) and kernel_name (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6407, 6683, 6856; dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:191) once block_table_stride becomes a… |
| 6 | N/A | No cohort routing is added or modified in this PR. |
| 7 | N/A | No hand-pinned overrides or heuristic tables are touched in this PR. |
| 8 | N/A | No cohort predicates are defined or modified in this PR. |
| 9 | N/A | No new kernel or architecture is registered in the dispatcher registries in this PR. |
| 10 | N/A | No new spec field or feature is added; block_table_stride is an existing runtime tensor stride passed as a kernarg. |
| 11 | N/A | No candidate registration, opt-in flag, or arch default promotion is changed in this PR. |
| 12 | N/A | This PR does not add, rename, or reprioritize any dispatcher candidates or custom rankers. |
| 13 | N/A | This PR does not modify CandidateRegistry.select, pin handling, or add a new dispatch wrapper. |
| 14 | PASS | The scalar launcher compile cache key in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:806-816 drops problem-shape fields (total_q, num_seqs, max_seqlen_q, max_seqlen_k) so compiled binaries key on arch and static configuration rather than recompiling per shape. |
| 15 | N/A | This PR does not add a dispatcher candidate or arch module. |
| 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 modify candidate routing cohorts. |
| 18 | N/A | This PR does not touch CandidateRegistry ratchets, binding flags, or exempt sets. |
| 19 | PASS | The PR makes no dispatcher registration claims and scopes its changes to the scalar fallback launcher and builders in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:5746-5751 and :6365-6370. |
| 20 | PASS | Kernel-level signature, kernel-name, and builder changes in Python (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6407-6412, :6514, :6743) are mirrored in C++ (dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:188-209, :328-334, :383-388, :452-455). |
| 21 | N/A | This PR fixes a kernel ABI/builder bug where block_table_stride was hardcoded from max_seqlen_k in the scalar kernel rather than a spec-selection issue. |
§E graded 17/17
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:32 imports from platform (rocke.core.lower_llvm) into library, and no platform file imports library. |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:29 uses rocke.runtime.hip_module.get_device_arch for device queries rather than importing torch. |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:24 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:29 run with numpy/pytest and rocke.runtime without requiring torch. |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:27 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:31 only import downward into kernels.common. |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/platform/CMakeLists.txt:632 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:190 use only relative paths and contain no shell scripts. |
| 6 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6511 and :6741 preserve intentional unused param handles (_out_scale, _softcap) rather than stripping them. |
| 7 | N/A | This PR does not introduce new ROCKE_* environment variables, dtype support, public ops, kernel families, manifest kinds, or tuning knobs. |
| 8 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:801 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:4 use descriptive explanations without Jira IDs, PR numbers, or usernames in code. |
| 9 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:806 and :6514 show all PR changes are tightly scoped to passing block_table_stride as a kernarg in scalar attention and removing dynamic shapes from its cache key and kernel names. |
| 10 | PASS | dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:261 and dnn-providers/hip-kernel-provider/rocke/platform/tests/instances/differential/golden/llvm_gfx_all.json:36 update only the affected scalar attention golden entries after rebasing onto current develop. |
| 11 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:1 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:1 are deliverable test suites registered in CMakeLists.txt with no scratch files committed. |
| 12 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:115 tests both gfx942 and gfx950 cache keys, and the scalar builder changes are arch-generic across all targets. |
| 13 | PASS | dnn-providers/hip-kernel-provider/rocke/platform/cpp/include/rocke/instance_attention_unified_internal.h:110 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_prologue.cpp:121 cleanly remove the dead max_blocks field and its initialization. |
| 14 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6571 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_2d.cpp:118 remove only the intended compiled max_blocks stride calculation after rebasing onto develop. |
| 15 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:7211 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_selectors.cpp:1326 use the newly added bt_stride parameter in the address calculation. |
| 16 | N/A | This item applies to review grading output rather than code changes in the PR diff. |
| 17 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:801 and the PR description report only qualitative latency observations without any absolute performance numbers. |
§F graded 11/11
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Branch users/dlangbe/rocke-drop-shape-from-cache-key matches users/<user>/… (diff at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:797). |
| 2 | PASS | title fix(rocke): honour the block-table stride in the scalar attention fallback matches type(scope): summary and is 74 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/test_attention_scalar_block_table_numeric.py is in the diff |
| 5 | PASS | No forbidden files (keys/certs/secrets) are in the PR file list (only .py, .h, .cpp, CMakeLists.txt, and .json files such as dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:797). |
| 6 | NEEDS-INFO | The new commits merge develop into the branch; local pre-commit execution status and full-file black formatting still cannot be verified from the PR diff alone. |
| 7 | NEEDS-INFO | The new commits merge develop into the branch; bot, Copilot, and human review thread states are still not included in the provided PR context. |
| 8 | N/A | This PR targets develop directly rather than being a stacked PR on a feature branch. |
| 9 | PASS | The PR keeps Python, C++, tests, CMake test lists, and golden updates together in one cohesive change without artificial splitting (e.g., dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:797 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_2d.cpp:118). |
| 10 | PASS | The added tests in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:89 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:157 verify real cache-key, IR-identity, and numeric block-table stride invariants rather than throwaway checks. |
| 11 | PASS | The PR title and body fields are populated in the PR metadata describing the changes in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:797. |
§G graded 34/34
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Both engines are updated in lockstep and numeric correctness is verified independently on GPU against an fp32 oracle using poison KV blocks (dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:90, dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:119). |
| 2 | PASS | On-silicon numeric verification against an fp32 reference is added in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:157 and wired into rocke_library_gpu_pytest (dnn-providers/hip-kernel-provider/rocke/platform/CMakeLists.txt:834). |
| 3 | PASS | The representative IR golden hashes and differential golden hashes are updated across flavors (dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:261, dnn-providers/hip-kernel-provider/rocke/platform/tests/instances/differential/golden/llvm_gfx_all.json:36) and the cache-key/IR test checks all LLVM_FLAVORS… |
| 4 | PASS | test_dynamic_shape_shares_key, test_dynamic_shape_lowers_identical_ir, test_block_table_stride_is_a_kernarg, and test_wide_block_table_rows all fail against the pre-change code (dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:89, dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:157). |
| 5 | PASS | Tests exercise the shipped launch entrypoint run_unified_attention_torch(backend="scalar") (dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:140, dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:170) and each tests a single falsifiable property. |
| 6 | PASS | Docstring and comment claims about _cache_key and _physical_block_and_token match the implementation and are enforced by tests (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:801, dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:7204). |
| 7 | N/A | This PR does not add or modify a design document. |
| 8 | PASS | The GPU test harness uses explicit @pytest.mark.skipif(_ARCH not in known_arches(), ...) without silent dtype fallback (dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:39). |
| 9 | N/A | This PR does not make load-bearing LDS or occupancy claims. |
| 10 | N/A | This PR is a correctness and cache-key fix and does not make headline performance ratio claims or ship a benchmark script. |
| 11 | PASS | The scalar kernel path is updated on both the Python launch path (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6368) and C++ builders/signatures (dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:331). |
| 12 | PASS | Both Python and C++ scalar builders/helpers are updated together (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6571, dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_2d.cpp:122). |
| 13 | N/A | This PR does not add or recommend LLVM intrinsic symbols. |
| 14 | N/A | This PR does not classify hardware bottlenecks. |
| 15 | N/A | This item is a review process reminder about reading from git objects rather than a property of the PR diff. |
| 16 | PASS | All callers of _physical_block_and_token, _emit_qk_score, _emit_v_load, and _attn_signature across Python and C++ were located and updated consistently (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:5749, dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_3d_reduce.cpp:155). |
| 17 | PASS | Both new test files were executed end-to-end on CPU and gfx950 GPU and registered in CMake (dnn-providers/hip-kernel-provider/rocke/platform/CMakeLists.txt:635, dnn-providers/hip-kernel-provider/rocke/platform/CMakeLists.txt:834). |
| 18 | N/A | This PR does not add a statistical runtime guard or back-pressure detector. |
| 19 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:85-104 allocates an extra poison block filled with 64.0 and points unused wide-row block-table slots at it so a wrong stride corrupts the output instead of passing by luck. |
| 20 | N/A | This PR does not add or modify a text or claim ban detector. |
| 21 | N/A | This PR does not check in measurement or profiling harness deliverables. |
| 22 | PASS | dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:260-308 updates the scalar golden entries with valid bytes, sha256, and status: "ok", and the PR test plan confirms GPU execution on gfx950 and CPU lowering across llvm20/22/23. |
| 23 | N/A | This PR is a correctness and cache-key fix for the scalar fallback, not a performance or routing speedup claim. |
| 24 | PASS | dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:260-308 and dnn-providers/hip-kernel-provider/rocke/platform/tests/instances/differential/golden/llvm_gfx_all.json:35-72 re-bless the scalar attention cases whose kernel name and block_table_stride kernarg changed, and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:156-185 plus the gfx950 GPU… |
| 25 | N/A | This PR is not a codegen micro-optimization PR. |
| 26 | N/A | This PR is not a performance benchmarking campaign PR. |
| 27 | N/A | This PR does not ship a bundled performance optimization toggle. |
| 28 | N/A | This PR does not present counterfactual performance tables. |
| 29 | N/A | This PR does not swap guarded math primitives for fast approximations. |
| 30 | N/A | This PR makes no register-spill or scratch-memory claims. |
| 31 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:5748-5749 and 6367-6368 show include_bt_stride=False and compiled max_seqlen_k stride existed in develop, and the PR verified that dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:156-185 fails against the pre-change tree. |
| 32 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:76-103 explicitly binds arch="gfx950" and iterates LLVM_FLAVORS (llvm20, llvm22, llvm23), matching the provenance stated in the PR test plan. |
| 33 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:797-818 changes _cache_key, and the PR differential-tables every dropped and retained field in the PR body and enforces the partition across _DYNAMIC_VARIANTS and _STATIC_VARIANTS in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:50-115. |
| 34 | N/A | This PR does not add or modify an opt-in kernel output flag. |
§H graded 5/5
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The scalar unified-attention kernels use global pointer loads rather than spec-sized AMDGPU buffer-SRD num_records, and this PR removes max_seqlen_q and max_seqlen_k from the compile-time cache key. |
| 2 | NEEDS-INFO | The new commits since 7189edc are a rebase onto develop; physical = b.global_load_i32(block_tables, b.add(b.mul(seq_idx, bt_stride), block_idx)) in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:7210-7212 has no device-side range check on physical, and the host-side validation in run_unified_attention_torch remains outside the visible diff hunks. |
| 3 | PASS | The scalar kernel uses _magic_div_mod(b, kpos, p.block_size) in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:7209 and rocke_unified_attn_magic_div_mod(b, kpos, p->block_size, &block_idx, &token_in_block) in dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_selectors.cpp:1319 to lower the constant block_size division/modulo. |
| 4 | N/A | The scalar unified-attention kernels iterate over runtime sequence bounds (kpos) rather than a fixed compile-time Skv/BN tile loop with a page mask. |
| 5 | NEEDS-INFO | The new commits since 7189edc are a rebase onto develop and do not touch run_unified_attention_torch validation; bt_stride=bt_stride, include_bt_stride=True is passed in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6367-6368, while the surrounding host validation of block_table / bt_stride and negative tests remain outside the visible diff hunks. |
§I graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR fixes scalar fallback block-table row stride handling and cache-key normalization without changing dispatch routing or candidate space measurement. |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:5749 and dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6368 repair the scalar kernel launch to pass and consume the runtime block_table_stride kernarg instead of silently reading wrong rows with a compiled stride. |
| 3 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:50 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:156 cover dynamic/static problem variations and wide/view block-table layouts. |
| 4 | N/A | This PR does not add or modify tuning/routing benchmark tables with shipped vs achievable columns. |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:263 and dnn-providers/hip-kernel-provider/rocke/platform/tests/instances/differential/golden/llvm_gfx_all.json:36 update the explicit golden snapshots for the modified scalar attention cases. |
| 6 | N/A | This PR does not add any new DSL op or hardware-instruction-named primitive. |
§J graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | NEEDS-INFO | Missing full Markdown hunks for design/algorithm docs such as dnn-providers/hip-kernel-provider/rocke/library/builders/gfx950/attention/ALGORITHM.md and dnn-providers/hip-kernel-provider/rocke/platform/dsl_docs/architecture/attention_thread_block_mapping_case_study.md, so equations and declared shapes cannot be type-checked. |
| 2 | NEEDS-INFO | Missing full Markdown hunks for the changed rocKE documentation, so notation tables, symbol domains, and log-base statements cannot be verified. |
| 3 | NEEDS-INFO | Missing full Markdown hunks for changed algorithm/design docs, so it is not possible to determine whether they restate algorithms owned by sibling docs instead of linking and stating only deltas. |
| 4 | NEEDS-INFO | Missing full Markdown hunks for changed rocKE docs, so added/new/shared novelty claims and their named symbols cannot be checked against develop. |
| 5 | NEEDS-INFO | Missing full Markdown hunks for changed docs, so scaling or complexity claims and their aggregate tuning quantities cannot be inspected across the edited prose sites. |
| 6 | NEEDS-INFO | Missing full Markdown hunks for changed design docs, so constants, model-table provenance, tuned band edges, and stated assumptions cannot be checked. |
| 7 | NEEDS-INFO | Missing full Markdown hunks and file-level context for newly changed/added rocKE docs, so sibling filename conventions, declared entry points, and ownership boundaries cannot be verified. |
§K graded 13/13
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR only touches the untuned scalar fallback kernels and does not define or modify any tuned candidates or KnobSpace subclasses. |
| 2 | N/A | No tunable kernel fields, spec fields, or KnobAxis definitions are added or modified in this PR's diff. |
| 3 | N/A | The PR only changes the untuned scalar launcher cache key and scalar builder ABI, not any tuned spec defaults, knob meanings, or config_key payloads. |
| 4 | N/A | No KnobSpace.defaults() or recorded() methods are touched by this PR. |
| 5 | N/A | No knob drop, refuse, inert(), or base_value() rules are modified in this PR's diff. |
| 6 | N/A | No KnobSpace.validate or tuning_specs.py policy-free spec construction code is touched by this PR. |
| 7 | N/A | No KnobSpace.base(req) implementation or tuning problem normalization is modified in this PR's diff. |
| 8 | N/A | No knob value normalization or axis type coercion code is touched by this PR. |
| 9 | N/A | No benchmark recording, tuning replay, or PinRefused handling code is touched by this PR. |
| 10 | N/A | No tuning_id resolution or KnobSpace.find logic is touched by this PR. |
| 11 | N/A | No sweep entry points, pin_to_spec logic, or sweep harnesses are modified in this PR's diff. |
| 12 | N/A | No production tuning stacks, accept_default rules, or tuned variant base specs are modified in this PR's diff. |
| 13 | N/A | No sweep benchmarks, legality filters, or auto selection tables are touched by this PR. |
§L graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR does not add or modify any gfx targets or ISA backends. |
| 2 | N/A | This PR does not add or modify any MMA op_ids or fragment info. |
| 3 | N/A | This PR touches only the scalar attention fallback and does not select or modify MMA atoms. |
| 4 | N/A | This PR does not touch wave-size or lane-math logic. |
| 5 | N/A | This PR does not touch arch-specific MMA atoms, LDS transpose reads, or tile loaders. |
| 6 | N/A | This PR does not modify pipeline vocabulary, policy modules, or ArchTarget. |
| 7 | N/A | This PR does not add or change logical dtypes, bit packing formats, or FP8 dialect handling. |
| 8 | N/A | This PR does not touch scaled-MMA contracts or WMMA/MFMA scaled emission. |
| 9 | N/A | This PR does not introduce a new architecture target. |
| 10 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:32 imports LLVM_FLAVORS and line 94 parametrizes over LLVM_FLAVORS instead of using a hand-rolled flavor list; no intrinsic arch-domain artifacts are touched in the PR diff. |
§M graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | The new bt_stride SSA value is passed through IR arithmetic ops (b.mul, b.add) and helper arguments without driving Python host control flow or escaping scf regions (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6514, dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:7211). |
| 2 | N/A | The diff does not touch legalize_for_target, tile.mma, or scf.for iter-args. |
| 3 | PASS | Wire serialization (core/ir_serialize.py and cpp/core/ir/serialize.cpp) is untouched, and the trailing block_table_stride parameter order matches identically across Python and C++ builders and the launch signature (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6514, dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:331… |
| 4 | N/A | The PR diff does not touch portable IR schemas, importers, recipe expansion, or magic_division implementations. |
| 5 | N/A | The diff modifies scalar attention kernels and does not use AsyncTileLoader or LDS. |
| 6 | PASS | The modified global_load_i32 indexing uses the dynamic bt_stride kernarg unconditionally inside the existing loop without introducing barriers, divergent control flow, or speculative invalid loads (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:7211, dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_selectors.cpp:1324). |
| 7 | N/A | The diff does not touch ISA backends, buffer-resource word 3, or temporal hints. |
| 8 | N/A | The diff does not add any intrinsic declarations or modify engine selection precedence. |
| 9 | PASS | Headers under cpp/include/rocke/ are not renamed, and the public C entry points in instance_attention_unified.h retain their stable signatures while internal helper headers update for bt_stride (dnn-providers/hip-kernel-provider/rocke/platform/cpp/include/rocke/instance_attention_unified.h:39, dnn-providers/hip-kernel-provider/rocke/platform/cpp/include/rocke/helper_helper_rocke.instances.common.attention_unified_selectors.h:279). |
| 10 | N/A | The diff does not touch debug info (!dbg) emission or DWARF consumers. |
§N graded 18/18
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR does not add or modify any module in platform/python/rocke/helpers/. |
| 2 | N/A | This PR only threads a scalar block_table_stride kernarg into existing scalar unified-attention block-table lookups and does not modify MFMA output mapping, TensorView DAGs, loaders, epilogues, pipelines, or… |
| 3 | N/A | This PR does not touch SoftwarePipeline or any pipelined kernel. |
| 4 | N/A | This PR does not add or modify fused kernels or epilogue fusion. |
| 5 | N/A | This PR does not add or change any public or private API in rocke.helpers.tiling. |
| 6 | N/A | This PR does not add or modify front-door/toolbox tiling helpers or LayoutStyle implementations. |
| 7 | N/A | This PR does not use or modify tiling MMA layout overrides or c_desc derivation. |
| 8 | N/A | This PR modifies only scalar unified-attention kernels and does not use tiling LDS or global memory verbs. |
| 9 | N/A | This PR does not touch N-D tiling tensor axis roles, FlowStage, or batched MMA loops. |
| 10 | N/A | This PR does not change tiling emit, LDS staging, MMA issue, or tiling fail-fast checks. |
| 11 | N/A | This PR does not touch mma_traits.json, TileMma, lds_swizzle, or derive_sched_group_counts. |
| 12 | N/A | This PR makes no LDS bank-conflict or tiling coalescing claims. |
| 13 | N/A | This PR does not construct or modify WarpDistributionEncoding, make_tile_desc, RegisterMapper, or tiling store casts. |
| 14 | N/A | This PR does not use tiling verbs, LDS staging, or pipeline barriers. |
| 15 | N/A | This PR does not touch tiling allocation or swizzled/padded descriptor bounds checks. |
| 16 | N/A | This PR does not touch optimize_layout, recommend(), or evaluate_transform. |
| 17 | N/A | This PR does not touch tiling_recorder.py, round-trip analysis, or tiling visualization. |
| 18 | N/A | This PR does not touch interleaved tiling layouts or demo kernels under helpers/tiling/kernels/. |
§O graded 16/16
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR does not modify verify.py, verify.cpp, address spaces, or required IR attributes. |
| 2 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6570 and dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6824, the scf.for bodies keep their iter-arg structure and end in b.scf_yield(new_m, new_l, new_acc). |
| 3 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:95, test_dynamic_shape_lowers_identical_ir verifies deterministic byte-identical lowering across dynamic shape variants and LLVM flavors. |
| 4 | PASS | In dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:6514 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:387, the new parameter uses the I32 / rocke_i32() type singletons. |
| 5 | N/A | This PR does not modify core/passes.py, cpp/core/passes.cpp, or op purity tables. |
| 6 | N/A | This PR does not touch constant folding in core/passes.py or cpp/core/passes.cpp. |
| 7 | N/A | This PR does not modify kernel attributes, agpr_alloc, waves_per_eu, or cluster_dims. |
| 8 | N/A | This PR does not touch CodegenPolicy or SchedulerStrategy. |
| 9 | N/A | This PR does not emit or modify waits, barriers, or optimization_barrier. |
| 10 | N/A | This PR does not modify the LLVM lowerers or parameter address-space handling. |
| 11 | N/A | This PR does not add or modify ArchTarget queries or target ID resolution. |
| 12 | N/A | This PR does not introduce hardware vector widths, size caps, or architecture defaults. |
| 13 | PASS | In dnn-providers/hip-kernel-provider/rocke/platform/cpp/include/rocke/instance_attention_unified_internal.h:146, only the internal rocke_attn_unified_build_ctx struct is modified (replacing max_blocks with block_table_stride), while public C structs and enums in instance_attention_unified.h are unchanged. |
| 14 | PASS | In dnn-providers/hip-kernel-provider/rocke/platform/cpp/include/rocke/helper_helper_rocke.instances.common.attention_unified_selectors.h:276 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:387, C++ changes follow rocke_* snake_case and keep the Python↔C correspondence comments updated. |
| 15 | N/A | This PR does not touch C++ engine fallback handling or LLVM flavor resolution. |
| 16 | PASS | In dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:331 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_2d.cpp:122, C++ changes remain strictly in codegen/IR building in parity with Python without fabricating runtime or launch behavior. |
§P graded 14/14
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Builder signatures are unchanged and tested without extra args in dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:78. |
| 2 | PASS | The trailing block_table_stride kernarg is added in both Python (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:5749, :6368, :6514, :6743) and C++ (dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:331, :387, :455) and checked by test_launch_signature_matches_kernel_params in… |
| 3 | N/A | This PR does not touch GEMM, implicit-GEMM, or block-scaled GEMM launch geometry. |
| 4 | N/A | This PR modifies scalar unified attention and does not touch LDS gates or LDS allocation. |
| 5 | N/A | This PR does not touch WMMA/RDNA GEMM, GemmMultiD, or GemmMultiAbd paths. |
| 6 | N/A | This PR does not add or modify caller-zeroed workspaces for Stream-K, MoE, or split-K. |
| 7 | N/A | This PR does not touch Stream-K, persistent GEMM, GroupedGemmLauncher, or SUPPORT_MATRIX.md. |
| 8 | N/A | This PR does not touch quantization helpers or MX GEMM scale decoding. |
| 9 | N/A | This PR does not add or modify numeric emulation or dtype decode paths. |
| 10 | N/A | This PR does not touch manifest kinds, manifest emitters, or manifest runners. |
| 11 | PASS | Scalar launches continue to go through _get_scalar_launcher and run_unified_attention_torch (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_unified.py:5746, :6365) and the new GPU test reads output via Runtime.memcpy_d2h after launch (dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:81, :140). |
| 12 | N/A | This PR does not touch COMGR or HIP library loading or fix a hipError launch code. |
| 13 | PASS | The scalar attention cache-key and stride-kernarg refactor is covered by both CPU cache-key/IR tests (dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:89) and GPU numeric tests including multi-batch binary reuse (dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:157, :170). |
| 14 | N/A | This PR does not touch examples, expected.json files, or promote modules to helpers/. |
§Q graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | test_attention_scalar_cache_key.py is registered in _ROCKE_LIBRARY_ATTENTION_CPU_TEST_RELPATHS at dnn-providers/hip-kernel-provider/rocke/platform/CMakeLists.txt:635, test_attention_scalar_block_table_numeric.py is registered in dnn-providers/hip-kernel-provider/rocke/platform/CMakeLists.txt:834, and the PR body names the CI lanes (rocke_library_attention_cpu_pytest and rocke_library_gpu_pytest). |
| 2 | N/A | This PR modifies the existing scalar unified-attention kernel family rather than introducing a new kernel family or variant. |
| 3 | PASS | The PR separates cross-engine parity (check_byte_identity.py / dnn-providers/hip-kernel-provider/rocke/platform/tests/instances/differential/golden/llvm_gfx_all.json:36), the representative IR golden change tripwire (dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:261), and GPU numeric verification… |
| 4 | NEEDS-INFO | Both dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json:260 and dnn-providers/hip-kernel-provider/rocke/platform/tests/instances/differential/golden/llvm_gfx_all.json:36 are truncated in the diff, so re-blessing across all entries in LLVM_FLAVORS (llvm20, llvm22, llvm23) cannot be verified from the diff alone. |
| 5 | PASS | The GPU test is torch-free using rocke.runtime and skips cleanly when the device target is not in known_arches() (dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_block_table_numeric.py:36), while the CPU test iterates over LLVM_FLAVORS without requiring torch or a GPU (dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:94). |
| 6 | N/A | This PR does not refactor any test gate or harness. |
| 7 | N/A | This PR does not change any rejection contracts or error-raising paths. |
§R graded 11/11
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR fixes block-table stride indexing and cache-key normalization in the scalar fallback and introduces no new kernel body, tiling strategy, pipeline variant, or structural performance claim. |
| 2 | N/A | No performance lever, architecture-specific optimization recipe, or heuristic knob is changed in this diff. |
| 3 | N/A | The diff touches no GEMM tile, LDS layout, prefetch pipeline, or launch occupancy lever covered by the optimization utilities skills. |
| 4 | N/A | The PR is a correctness and compile-cache fix that replaces an immediate stride with a scalar kernarg (block_table_stride) rather than a performance optimization claiming primitive changes. |
| 5 | N/A | The diff changes no performance lever or memory layout; the scalar block_tables lookup only replaces a compile-time row stride constant with the runtime block_table_stride kernarg for correctness. |
| 6 | N/A | No LDS or MMA layout or rocke.helpers.tiling pipeline is added or modified. |
| 7 | N/A | The PR makes no bottleneck classification claim or bottleneck-targeted optimization. |
| 8 | N/A | No waits, barriers, or scheduling hints (sched_barrier, sched_group_barrier, iglp_opt) are added, removed, or modified. |
| 9 | N/A | The PR is not a performance regression fix and does not tune a shape-trading performance lever. |
| 10 | N/A | The PR makes no codegen alias-semantics change, assembly parity claim, or occupancy optimization conclusion. |
| 11 | N/A | The PR introduces no new optimization lesson, heuristic, case study, or DSL probe tool. |
§S graded 8/8
| # | Verdict | Evidence |
|---|---|---|
| 1 | NEEDS-INFO | The current-head delta includes omitted hunks in public rocKE platform modules and C headers, so it is not possible to determine every new public symbol or its CI-running test from the provided diff. |
| 2 | NEEDS-INFO | The visible scalar API/ABI behavior changes do update tests, but omitted current-head hunks in public rocKE platform modules prevent verifying that every changed existing public API kept and updated its tests. |
| 3 | NEEDS-INFO | The visible scalar diff does not remove a public Python/C entry point, but the current-head delta shows omitted deletions/renames such as public-looking rocKE helper files, so import-site and deprecation coverage cannot be verified. |
| 4 | NEEDS-INFO | The visible scalar diff does not edit all, but omitted current-head hunks in public rocKE Python packages prevent verifying whether all changed and whether every listed name resolves. |
| 5 | NEEDS-INFO | Visible scalar tests assert behavior, but omitted current-head public API changes prevent verifying that every new or changed API test asserts contracts and negative cases rather than existence only. |
| 6 | NEEDS-INFO | Visible scalar contracts are split between CPU and GPU tests, but omitted current-head public API changes prevent verifying that all CPU-checkable contracts added or changed by the current head are CPU-tested. |
| 7 | NEEDS-INFO | Changes since the previous review do not resolve the prior gap in the visible diff: the CPU scalar cache-key test still lowers through _lower_kernel_to_llvm_python at dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_scalar_cache_key.py:76-82, while C++ scalar 3D also changes to add block_table_stride at dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_entry.cpp:449-455 and dnn-providers/hip-kernel-provider/rocke/platform/cpp/instances/common/attention_unified_3d_reduce.cpp:152-181; missing evidence of a C++/binding/parity test exercising the scalar 3D C++ path. |
| 8 | NEEDS-INFO | Changes since the previous review do not resolve the prior gap: the provided diff and PR text do not state that the README entry points were run from rocke/platform/, nor do they state the manifest verification command for any manifest/runtime-impacting change. |
…arry Resolve the golden conflicts by regeneration: take develop's files, then re-record on the merged tree. The representative IR golden is rewritten by rocke_ir_parity_harness.py --write; the differential golden re-records the attention_tiled_2d and attention_unified families (ll llvm20/22/23/24 + ir canonical, python-only). Other families keep develop's entries. The scalar unified attention hashes differ from both sides because develop's #13889 (block-table stride) and this branch both change those kernels.
build_unified_attention_3d declares a trailing i32 block_table_stride (#13889), but the test's direct segment launch omitted it, so the argument buffer was one i32 short. With one sequence the block-table row index is 0, which hid the missing stride. Pass the table's row stride, and check that each direct launch's signature names exactly the kernel's params, so a later ABI change fails here instead of reading past the arguments.
TheRock Submodule Bump ActivityNewest first
|
…ime (#14530) JIRA ID : AICK-2676 The gfx942 tiled attention kernels (2D, 4-warp GQA, 3D split-KV and reduce) now read the head counts and the sliding-window size as kernargs and derive the `cu_q` search trip count from `num_seqs`. One compiled binary therefore serves every head pair at a given GQA ratio, every window size and every batch. This is the kernel-side readiness hipDNN AOT packaging needs: a packaged catalog ships one binary per kernel symbol and cannot enumerate head counts, window sizes or batch sizes. Follow-up to #13372 and #13889, which removed raw shape fields from the tiled and scalar cache keys. Builds on #14216 and #14349, merged from develop. The gfx950 counterpart (AICK-2675) reuses the plumbing added here. ## Risk Assessment Risk 4. Every gfx942 tiled kernel changes its argument list and emitted IR, the tiled cache keys change, and the paged K/V descriptor gains runtime strides in both engines. That is a performance-critical path. Mitigations: byte identity is GREEN at llvm20, llvm22 and llvm23 for every family; gfx950 and gfx1250 tiled IR is unchanged (golden); and the gfx942 numerics and an A/B against develop were run on hardware (see Testing). ## ASIC Coverage - **gfx942:** all kernel changes. Verified manually on gfx942: numeric tests, the existing gfx942 GPU suites, and a develop-vs-branch A/B. Superbuild and TheRock CI do not run these GPU numerics on gfx942. - **gfx950 / gfx1250:** host-only changes (signature helpers, cache keys, launch-cache keys). Their tiled spec classes declare no runtime kernargs, so their IR and goldens do not move. PR CI is sufficient. ## Testing Summary - **Byte identity:** full `check_byte_identity.py` GREEN at llvm20, llvm22 and llvm23. The gfx942 2D, 2D stress, 3D and reduce emit configs cover windowed, i64 and sinks specs. - **Goldens:** the representative-IR golden moves only for the gfx942 tiled cases (2D, 3D, reduce and both 4-warp kernels); the strided-KV library golden only for its gfx942 configs. Every gfx950 and gfx1250 entry is unchanged. - **Cache keys (CPU):** - shapes that differ only in head pair (same GQA ratio), window size or batch bucket share one key and lower to byte-identical IR at every flavor, on 2D, 4-warp D256, the 4-warp D128 fold and 3D; - the invariant sweep holds; - gfx950 and gfx1250 still split on the batch bucket. - **Launch signature (CPU):** every production builder route and three gfx942 tuning candidates are checked: the kernel's params equal its launch signature by name and type. - **One binary, many shapes (gfx942 GPU):** `test_attention_gfx942_runtime_shape_numeric.py` runs pairs of shapes that share a key back to back through `run_unified_attention_torch`. The second launch must compile nothing, and both outputs must match an fp32 paged reference (max_abs ≤ 4e-2; bf16/fp16 inputs, randn) with unused block-table slots on a poison block. - Pairs cover 2D default, windowed 2D, D256 4-warp, the D128 window fold, 3D decode and two-token 3D. One pair changes only the head counts at identical token counts. - Checked against mutants: baking the window back in fails the windowed pair (max_abs 0.88); baking the kv head count into the K/V stride faults on the device. - **Existing GPU suites (gfx942):** 69 attention, MLA and strided-KV library test files. Results match develop; the remaining failures fail identically on develop (MLA comgr environment errors, strided-KV and selector-parity cases). **A/B, develop vs this branch (gfx942, 304 CUs)** - **Workload:** 116 problems through `run_unified_attention_torch` with production routing (`backend="auto"`): 98 on the 2D path, 18 on 3D. Cohorts cover d64/d128/d256, bf16/fp16 and fp8 K/V, GQA ratios 1, 4 and 8, sinks, sliding window, the 4-warp D256 and fold paths, and a 64/16 head pair. - **Method:** HIP-event mean over 50 launches after 10 warmup, median over 3 rounds. Arm order alternates per round, and each arm runs in a fresh process each round. - **Toolchain:** torch 2.13.0+rocm10.0.0, LLVM 23 from the ROCm 10 SDK wheels, Python lowering. - **Baseline:** develop at `f37a7056c791`. - **Numeric gate:** every row of both arms is within 4e-2 of an fp32 reference; worst max_abs is 1.24e-2 in both arms. - **Results (branch / develop):** | Measure | Result | |---|---| | Compiled binaries per process | 18 vs 54 | | Total first-launch time | 0.37× | | Steady state, geometric mean over 116 problems | 0.994× (range 0.87–1.06×) | | Per cohort | within 0.98–1.01×, except the two below | | d64 GQA-8 block-32 **sinks** (2 problems, 2D) | 1.05–1.06× | | d64 GQA-8 **fp8 K/V** decode (4 problems, 3D) | 1.01–1.03× | - **Sinks caveat, isolated:** - Timing each commit puts the whole sinks cost in the head-count commit, and only on the narrow 2D kernel. - A timing-only variant with the K/V token stride restored to a constant removes it; restoring the query head count changes nothing. - The per-copy K/V offset now multiplies the token index by the runtime stride inside the KV loop, where the constant stride used to compile to a shift. - Accepted here as a known cost. Hoisting the in-page offsets out of the loop belongs with the planned per-page i32 K/V loader rework. **AOT dry run:** a draft engine manifest pinned to one gfx942 tuning candidate (`transposed_x8`, 4 warps, tile 4× block) expands with `registry_enumerator.py --plan-space`: 24 cells (6 head configurations × bf16/fp16 × block 16/32), production sweep, 20 varying knobs. Result: - 324 instances (within the default per-arch budget of 1000); - 162 distinct kernels on this branch against 324 on develop, because head pairs at one GQA ratio now build the same kernel. ## Testing Checklist - [x] Byte identity - `ROCKE_LLVM_FLAVOR=llvm{20,22,23} python tools/check_byte_identity.py` - Status: Passed - [x] Representative-IR golden - `python tests/instances/rocke_ir_parity_harness.py --check tests/golden/rocke_representative_ir_sha256.json` - Status: Passed - [x] Library host tests (attention, MLA, strided KV, dispatch, layering), GPU hidden - `python -m pytest <file>` per file - Status: Passed (3 files fail identically on develop) - [x] Platform dynamic-descriptor tests - `python -m pytest tests/core/test_transforms_dynamic.py` - Status: Passed - [x] gfx942 GPU: new runtime-shape numeric test - `python -m pytest tests/test_attention_gfx942_runtime_shape_numeric.py` - Status: Passed (14/14) - [x] gfx942 GPU: existing attention/MLA/strided-KV library suites (69 files) - Status: Passed, matching develop - [x] gfx942 GPU: develop-vs-branch A/B, 116 problems, 3 rounds - Status: Passed (numeric gate), caveats above ## Technical Changes - **Runtime-kernarg plumbing.** Tiled spec classes declare `RUNTIME_KERNARG_FIELDS`. `_tiled_2d_signature` / `_tiled_3d_signatures` append those fields to the launch signature, and `_runtime_kernarg_values` packs them. Every gfx942 launch site uses the arch-aware signature. The 4-warp GQA kernels now declare `qq_bias_stride_0`, which the shared 2D signature already passed. - **Search trip count.** `binary_search_seq_idx_trip_count` computes `bit_length(num_seqs)` from the `num_seqs` kernarg. It's exact below 2^24 sequences; past that it adds at most one no-op iteration. Specs declare `RUNTIME_SEARCH_ITERS` and the keys drop the batch bucket. The existing helper and its other callers emit the same IR. - **Sliding window.** The size is a kernarg. Whether a window applies still selects structure, so the name token is `sw` and the keys carry `window > 0`. - **Head counts.** `num_query_heads` and `num_kv_heads` are kernargs. The Q/O, segment-output and paged K/V strides are runtime values through `DynamicTensorDescriptor`, which now takes mixed int/Value strides plus runtime strides in `offset_i64_split` (both engines) and `offset_i64` (Python). Names carry `g<Hq/Hkv>`, and the keys carry the GQA ratio. Every selector that reads absolute head counts feeds an output that's already keyed. - **Launch caches.** `_shape_key` (2D grid, 3D grids and workspace) now carries both head counts, since one binary now serves several head pairs. - **i64 name token.** The gfx942 2D name carries `i64` when `use_i64_kv_addr` is set; i32 and i64 builds used to share a symbol. - **C++ twins.** The 2D, 3D and reduce twins mirror all of the above. The gfx942 2D C++ entry also builds the gfx950 fastKV register-P twin, so its runtime changes are gated to gfx942. - **CI enrollment.** `test_attention_tiled_launch_signature.py` joins the installed attention CPU cohort; `test_attention_gfx942_runtime_shape_numeric.py` joins the library GPU cohort. **ABI note:** in this repo, the gfx942 tiled kernels are launched only through `run_unified_attention_torch`, the dispatch tuning candidates and the harnesses updated here. Anything that launches them by name must append `num_query_heads`, `num_kv_heads` and `sliding_window` (2D and 3D) or `num_query_heads` (reduce). **Notes for the packaging work:** - The gfx942 fast paged-KV descriptor path (`use_fast_paged_kv_desc`, reachable only through the `fastkv` tuning knob) still hard-codes 64/8 heads, pinned by its spec validator. A packaged catalog must serve it only at 64/8 or leave that knob out. - gfx942 routes windowed decode to 2D, so production never takes the windowed 3D path; byte identity covers it. - `AttentionTuningSpec.cache_key()` remains field-complete, so it still splits on the runtime fields. That over-keys but is never wrong. ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests. --------- Co-authored-by: Claude <noreply@anthropic.com>
…r engines (#14242) ## Summary Packages rocKE's gfx950 unified attention as two hipDNN kernel-ingestor engines: `hipkernel:Gfx950AttentionUnified2D` (prefill and the decode shapes 3D does not cover) and `hipkernel:Gfx950AttentionUnified3D` (split-KV decode, a segment and a reduce launch per kernel through #14158's multi-launch `sources`). Both ship AOT, so an SDPA-forward graph on gfx950 can be served by unified attention with no JIT. Both catalogs are reduced to what a correctness-checked MI355X sweep measured as needed: Unified2D ships 222 of its 792-kernel full space and Unified3D 123 of 348, with no graph losing coverage or more than the measured noise (the final reduced-vs-full sweep is below). JIRA ID : AICK-2542 JIRA ID : AICK-2682 ## Risk Assessment Risk 2 of 5. The PR adds two engines and their descriptors. No existing engine's code, catalog or ranking changes: an unpinned SdpaFwd graph still ranks `hipkernel:Gfx950AttentionDense` first (device test `DenseStillRanksAheadWhenUnpinned`). The shared pieces it touches: - generator tooling: `registry_enumerator` full-space and multi-launch sections, the regeneration gates, provenance stamping, and two fixes in `variant_reduce`. Only `provenance.generated_from` is opt-in. The expanded registry output now always writes per-kernel `provenance.rocke_spec_id` / `rocke_tuning_id`, including Dense's 840 catalog kernels (its manifest pins a `spec_id`). The compact `variants:` selections drop it, so no shipped descriptor changes, but any other consumer of the expanded output sees the new field; - one rocKE builder, `build_unified_attention_3d_tiled_module`; - the ingestor save census's list of dispatch symbols. These are covered by their own tests and by the existing dense regeneration gates, which still pass. 3D is the first production user of `launchNames()` (#14158). ## ASIC Coverage gfx950 only: both engines ship only for gfx950, and other architectures' shards carry neither engine. PR CI has no gfx950 device lane (`hkp-probe-gfx950` packs and asserts but launches nothing), so the device evidence comes from MI355X (and, before the rebase, MI350X) runs. ## Testing Summary - Host: unit and census tests for both packs, the arch-neutral geometry, and the generator suite under `hipdnn_strict_skips`, including regeneration gates g1–g7 for all three gfx950 manifests; static descriptor gates on the installed shards (variant sets, coverage rungs 1–2, validator, launch surface). - Device, MI355X (gfx950), on the shipped (reduced) catalogs at this head's code: - engine-pinned SdpaFwd quick and standard suites against the CPU and GPU references, both engines plus a Dense regression run; - the knobs suite: every shipped kernel class forced and checked against the CPU reference, plus declines, precedence and a NaN-prefilled workspace; - a timing comparison of every engine that serves each corpus graph, and PyTorch; - RUNBOOK stage 7's final sweep: the full catalog against the reduced one in one session. - How the catalogs were reduced: a 3-round dnn-benchmark sweep with correctness on, over 188 prefill graphs (2D) and 88 decode graphs (3D), then `variant_reduce`. ## Packaging and performance statistics All numbers below come from tool output (packaging logs, device suite runs, dnn-benchmark sweeps). **Packaging** (gfx950 shard, shipped catalogs; pack time is the build's own `hkp_pack` command per engine, median of 3, relative to Dense's on the same job) | Engine | Kernels (full space) | AOT budget | Code objects, zstd | KDP | Pack time vs Dense, build default workers | Serial time per kernel vs Dense | |---|---:|---:|---:|---:|---:|---:| | Unified2D | 222 (792) | 1000 | 1.55 MiB | 0.29 MB | 0.11× | 1.58× | | Unified3D | 123 (348), 246 launches | 400 | 0.64 MiB | 0.26 MB | 0.41× | 1.06× | | Dense (catalog unchanged, for scale) | 840 (52 distinct binaries since #14059) | 1000 | 6.16 MiB | 1.26 MB | 1 | 1 | - The gfx950 kpack is 8.49 MiB at this head; the two engines are 2.19 MiB, 26% of it. Since #14059 (runtime head counts for Dense), Dense's 840 catalog kernels compile to 52 distinct binaries, but the kpack still stores one frame per kernel, so Dense grew from 5.79 to 6.16 MiB. Collapsing those entries is #14059's stated follow-up. The pack-time ratios and the full-space figure (14.45 MiB) were measured against Dense before #14059. - Provider load (`hipdnn_list_engines`, median of 5) is 1.24× its time without these descriptors. **Correctness and coverage** (MI355X, shipped catalogs) | Suite | Unified2D | Unified3D | Dense | |---|---|---|---| | SdpaFwd quick + standard, CPU and GPU reference (served / declined / failed) | 43 / 6 / 0 | 4 / 45 / 0 | 45 / 4 / 0 | | Knobs suite (forced kernel classes, declines, precedence) | 30 / 30 (both engines) | | 62 / 62 | | Corpus graphs served and validated against PyTorch | 188 / 188 | 82 / 88 | | Every decline is intentional: Unified2D leaves decode to Unified3D and declines Hkv > Hq; Unified3D serves decode only, and its 6 corpus declines are batch-256 decode graphs whose K/V is over the 32-bit addressing cap (ASM SDPA serves those). **Kernel time** (MI355X, autotuned, median of 3 rounds; ratio = Unified / other, below 1 = Unified faster; every engine's round-to-round drift ≤ 0.4%). The Unified2D rows were re-measured at this head, on the kernels develop's tiled-2D changes recompiled (see the checklist). | | Graphs both serve | Geomean ratio | Time-weighted (other / Unified) | Unified faster on | |---|---:|---:|---:|---:| | Unified3D vs ASM SDPA (decode) | 60 | 0.200 | 8.17 | 60 / 60 | | Unified3D vs PyTorch | 82 | 0.062 | 34.2 | 82 / 82 | | Unified2D vs Dense (prefill) | 179 | 2.37 | 0.38 | 0 / 179 | | Unified2D vs PyTorch | 188 | 1.38 | 0.55 | 39 / 188 | - Unified3D serves 22 decode graphs no existing hipDNN engine serves. - Unified2D is slower than Dense wherever Dense serves, and Dense keeps ranking first there; it adds coverage Dense lacks (the 9 bottom-right causal graphs in these corpora). **Variant reduction** | Engine | Kernels | Chosen eps | Measured drift | Final sweep, reduced / full kernel time (geomean, rounds 1–3) | |---|---|---:|---:|---| | Unified2D | 792 → 222 | 0.02 | 0.62% | 1.000, 0.999, 1.000 (u2d_attn); 1.000, 0.999, 1.000 (u2d_rocke) | | Unified3D | 348 → 123 | 0.03 | 1.08% | 1.004, 1.002, 0.999 (u3d_attn); 1.002, 1.007, 1.004 (u3d_rocke) | Both arms of the final sweep served the same graphs and passed correctness on every corpus. ## Testing Checklist - [x] Unit (host, no GPU) - `HIP_VISIBLE_DEVICES=-1 hip_kernel_provider_tests` - 1454 passed, including `TestKernelIngestorEngine.EveryHandlerReportsSupportsSavingExactlyWhenItSavesAndRestores`. Our build host also fails 11 Dense signature/dispatch tests: it has a `/nonexistent` directory, which those tests assume is absent. They pass in CI - Status: Passed - [x] Census - `ctest -R 'hkp-census|rocke_unified'` - 36/36 (the census inventories are the reduced catalogs') - Status: Passed - [x] Generator suite (strict skips) - `pytest -p hipdnn_strict_skips IngestorGenerator/tests` - 1646 passed, 0 skipped - Status: Passed - [x] Regeneration gates on the reduced selections - `tests/test_registry_regeneration.py` (g1–g7) 39 passed; AOT budget tests 12 passed - Status: Passed - [x] rocKE attention CPU tests and packaging guards - 2D/3D mask_type, 3D module, decode split, KV cache policy, spec-predicate parity: 145 passed, plus 6 develop KV-cache tests that need the compiled `rocke_engine`; `test_hkp_pack_producer_guards.py` and `test_gfx950_attention_unified_3d_bundle_coverage.py`: 26 passed - Status: Passed - [x] Golden IR, before the reduction - `ctest -R rocke_golden_static` - the reduction changes no rocKE code - Status: Passed - [x] Launch surface - `launch_surface.py configs/gfx950_attention_unified_{2d,3d}.engine.yaml --check` - Status: Passed - [x] Variant sets - `verify_variant_sets.py --mode structural` and `--mode full --arch gfx950 --manifest …` - 2D 222/222 distinct binaries, 3D 123/123 - Status: Passed - [x] Coverage gate rungs 1–2 - `coverage_gate.py --mode full --expect-engine …` - Status: Passed - [x] Validator - `hipdnn_validate_descriptors <shard> --expect-engine … --json` - success, no warnings - Status: Passed - [x] Packaging probe - gfx942 configure with `HIPKERNELPROVIDER_ENABLE_PACKAGING_PROBES=ON`, `ctest -R '^hkp-probe-gfx950'` - 12/12; `hkp-probe-gfx950` packs 51 UKDs of the current descriptors in a gfx942-only build - Status: Passed - [x] Device suites, MI355X - engine-pinned SdpaFwd quick + standard with GPU reference (`hip_kernel_provider_gfx950_attention_*_gpu_ref_integration_tests`) and CPU reference (`*-external-integration_{quick,standard}_suite`) - 0 failures: Unified2D 43 served / 6 declined, Unified3D 4 / 45, Dense 45 / 4 - Status: Passed - [x] Device knobs suite, MI355X - `hip_kernel_provider_integration_tests --gtest_filter='*Gfx950AttentionUnified*'` - 30/30; Dense knobs 62/62 - Status: Passed - [x] Fault re-check, MI355X - the whole `u2d_rocke` corpus in one autotuned process, twice and then with serialized launches - 93/93 each, 0 errors - Status: Passed - [x] Reduction sweep + `variant_reduce.py` - 3 rounds, correctness on; 2D `SWEEP_DONE`, 3D `SWEEP_DONE`; 2D 792 → 222, 3D 348 → 123 - Status: Passed - [x] Final sweep, reduced against full (RUNBOOK §7) - same coverage, correctness passed on both arms, ratios above - Status: Passed - [x] Unified3D catalog regenerated for #14367's LLC-aware K/V cache policy - all 123 kernels now `kv_cache_policy: "all"` (was `"stream"`), same kernel classes; MI355X re-run: suites and knobs unchanged, 246/246 decode rows correct, kernel time against the `"stream"` catalog geomean 0.998, time-weighted 0.999 - Status: Passed - [x] Device evidence at this head (rebased onto develop 2026-11-03, after the review-round refactors) - Dense's and Unified3D's code objects are byte-identical to the previous device runs; develop's #13652 recompiled 201 of the 222 Unified2D code objects (same kernels and ids), and the Unified2D/3D matchers moved onto a shared header, so both engines' device runs were repeated at this head: suites Unified2D 43 / 6 / 0, Unified3D 4 / 45 / 0, Dense 45 / 4 / 0 (passed / skipped / failed), knobs 30/30 and 62/62; compare 188/188 Unified2D graphs (564/564 rows) and 82 Unified3D graphs (246/246 rows) correct against PyTorch in all 3 rounds. Kernel time vs the previous runs: Unified2D geomean 0.985 (none of 188 graphs >2% slower), Unified3D 1.003 (ASM_SDPA control 1.006). A final rebase picked up #14059, which recompiled Dense's 840 code objects only (Unified2D/3D byte-identical to the run above); the suites and knobs were re-run at this head with the same results, `DenseStillRanksAheadWhenUnpinned` included - Status: Passed - [x] DVC - both reduced KDPs on the `ingestor` remote; `dvc status -c` in sync - Status: Passed - [ ] PR CI - GitHub PR checks - Status: Pending ## Technical Changes - **Unified2D pack** (`Gfx950AttentionUnified2DNative.cpp`): matches BSHD SDPA forward on the identity paged-KV layout and builds block_tables, seq_lens and query_start_len in the workspace. - It declines: batch > 511 (the baked 9-iteration binary search), Skv not a multiple of 16, i32 KV over 0x7FFF0000, any window, and the decode cells Unified3D covers (Sq = 1 with the GQA group dividing 16). - Engine knobs: tile_size, num_warps, block_m_per_warp, use_mfma_32x32 and use_transposed_qk_32x32. - **Unified3D pack** (`Gfx950AttentionUnified3DNative.cpp`): decode only. - `launchNames()` is `{segment, reduce}`, and both definitions are resolved through `launchKernelDefinition` with a signature check before any load. - The segment launch writes `[token, Hq, seg, D]` scratch and the reduce always runs, so the workspace needs no zeroing. - Knob: num_segments. No batch limit. - **Arch-neutral geometry** (`UnifiedAttentionGeometry.hpp`): the launch geometry and workspace layouts take a `UnifiedAttentionArchTraits`, and the gfx950 headers are thin wrappers, so a gfx942 pack (AICK-2544) only supplies traits. - **rocKE**: `build_unified_attention_3d_tiled_module` packs the segment and its derived reduce (`reduce_spec_for`) as one `KernelModule`. The module predicate gates both halves. The 3D pack compiles develop's running-max fix for split-KV (#14276). - **Generator**: - `enumerate.also` sections may use `space: full` and carry manifest `launches`. The enumerator refuses a registry that pairs a reduce when the manifest declares no launches, and keeps one default per cell. - The gates are launch-aware: g1 keys on launches, g3 uses `--subset` for full spaces, g4 compares launch sets, and g6 walks every launch and checks module roles. - g2 and g3 run per engine with the shipped ids reused (#14109), comparing every descriptor byte for byte. - `generate.py --stamp-provenance` stamps `provenance.generated_from` (config, manifest, generator and rocKE commits). It is opt-in, so a default regeneration stays byte-identical. - `variant_reduce` fixes: - it reads the format-2 winner lines the runtime has written since #13028; - it keys a graph on its content with `Graph.id` (minted fresh at each finalization) zeroed, so sweep rounds match. - **Catalogs** (the manifests enumerate the full spaces; the selections are `variant_reduce`'s): - Unified2D: 222 kernels over 198 cells (33 head configs × bf16/fp16 × 3 masks): the narrow nw4/mw16/t64 geometry in every cell, transposed 32x32 nw4/mw32/t64 in 21 and narrow nw2/mw16/t32 in 3. The wide 32x32 geometry won no graph and is not shipped. - Unified3D: 123 kernels: num_segments 16 in all 116 decode cells, 32 or 64 in 7 bf16 bottom-right D128 variants. The K/V cache policy follows #14367's LLC-aware rule, which picks `"all"` for every shipped decode cell. - **Descriptors**: both are generated with develop's generator (post-#13028 UED/UHD, UKD `sources` for 3D), every kernel id reused from the full catalog. A regeneration with the shipped ids reused reproduces them byte for byte. The KDPs live in DVC (`ingestor` remote). - **Shared code** (one home per fact): - `packs/UnifiedAttentionGraphMatch.hpp`: the graph-match helpers both unified packs use (operand/BSHD checks, SDPA node, mask classification, dtype, metadata readers); the tests' graph builder is `UnifiedAttentionTestGraphs.hpp`. - rocKE `kernels/common/attention_mask_type.py` (mask ordinals and band -> kind, used by both tiled specs and re-exported by dispatch) and `attention_split_kv_reduce.py` (the segment -> reduce derivation the dispatcher and the module export share). - CMake registers Dense, Unified2D and Unified3D by one loop per lane (external integration, gpu_ref, census). - **Tests**: - The device knobs suite (`IntegrationGpuGfx950AttentionUnifiedKnobs.cpp`) adds a harness `prepareWorkspace` hook. It forces the default geometry/segment count across the head configurations and each other shipped class on a cell that carries it, and `CoversEveryCatalogKernelClass` holds those tables to the catalog. - The engine-pinned `gpu_ref` targets and external-integration lanes cover both engines. - The ingestor save census names all seven pack dispatch symbols. - The Unified2D/3D integration TOMLs are reserved with the gfx950 census shard (`hkp_reserve_census_shard(... FILES ...)`, as #14573 requires of every shard entry). **Constraints and follow-ups** - #13372 and #13889 are both in develop; this branch no longer carries a copy of #13889. - Heads are catalog cells (baked), window = 0, 2D batch ≤ 511, i32 KV ≤ 0x7FFF0000. D256 is deferred for both engines. The kernarg work is tracked in AICK-2675. - There is no 2D/3D `select_path` crossover replay: the split is the static decode rule above. - Runtimes older than UKD 1.1 skip the 3D descriptors. Strided-KV 3D is a follow-up. - Follow-up: execution-plan save/restore (#13315) for both handlers. Today they report `supportsSaving() == false`, as the RDNA handler does. - One earlier MI355X sweep hit a single GPU fault in Unified2D on `u2d_rocke/rocke_sdpa_035` (bf16, D 128, Hq 64 / Hkv 8, causal, S 512). It has not reproduced since: not in the forced geometries on that cell, 15 single-graph runs, 4 full-corpus runs on MI350X, or the MI355X full-corpus re-check above. - Unified2D kernel time moved with develop's tiled-2D changes (#14542, #14616), not with this PR: at this head it is 1.035× (geomean) its pre-rebase time on the same 188 graphs, up to 1.36× on long causal bf16 D128 prefill (S 8192), while Dense in the same runs is flat (0.997). Dense still ranks first wherever it serves; the table above is at this head. - Measurement note: the pinned dnn-benchmarking (v1 result schema, which `sweep.py` reads) has no bottom-right causal PyTorch reference. The sweeps used upstream dnn-benchmarking #84's reference, with the 9 bottom-right corpus graphs written as `causal_mask: false, right_bound: 0, diagonal_alignment: BOTTOM_RIGHT` (the same band under hipDNN's `resolveDiagonalBand`): #84 reads `causal_mask: true` as top-left regardless of the alignment. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Christopher Millette <63608002+cgmillette@users.noreply.github.com>
…r engines (#14242) ## Summary Packages rocKE's gfx950 unified attention as two hipDNN kernel-ingestor engines: `hipkernel:Gfx950AttentionUnified2D` (prefill and the decode shapes 3D does not cover) and `hipkernel:Gfx950AttentionUnified3D` (split-KV decode, a segment and a reduce launch per kernel through #14158's multi-launch `sources`). Both ship AOT, so an SDPA-forward graph on gfx950 can be served by unified attention with no JIT. Both catalogs are reduced to what a correctness-checked MI355X sweep measured as needed: Unified2D ships 222 of its 792-kernel full space and Unified3D 123 of 348, with no graph losing coverage or more than the measured noise (the final reduced-vs-full sweep is below). JIRA ID : AICK-2542 JIRA ID : AICK-2682 ## Risk Assessment Risk 2 of 5. The PR adds two engines and their descriptors. No existing engine's code, catalog or ranking changes: an unpinned SdpaFwd graph still ranks `hipkernel:Gfx950AttentionDense` first (device test `DenseStillRanksAheadWhenUnpinned`). The shared pieces it touches: - generator tooling: `registry_enumerator` full-space and multi-launch sections, the regeneration gates, provenance stamping, and two fixes in `variant_reduce`. Only `provenance.generated_from` is opt-in. The expanded registry output now always writes per-kernel `provenance.rocke_spec_id` / `rocke_tuning_id`, including Dense's 840 catalog kernels (its manifest pins a `spec_id`). The compact `variants:` selections drop it, so no shipped descriptor changes, but any other consumer of the expanded output sees the new field; - one rocKE builder, `build_unified_attention_3d_tiled_module`; - the ingestor save census's list of dispatch symbols. These are covered by their own tests and by the existing dense regeneration gates, which still pass. 3D is the first production user of `launchNames()` (#14158). ## ASIC Coverage gfx950 only: both engines ship only for gfx950, and other architectures' shards carry neither engine. PR CI has no gfx950 device lane (`hkp-probe-gfx950` packs and asserts but launches nothing), so the device evidence comes from MI355X (and, before the rebase, MI350X) runs. ## Testing Summary - Host: unit and census tests for both packs, the arch-neutral geometry, and the generator suite under `hipdnn_strict_skips`, including regeneration gates g1–g7 for all three gfx950 manifests; static descriptor gates on the installed shards (variant sets, coverage rungs 1–2, validator, launch surface). - Device, MI355X (gfx950), on the shipped (reduced) catalogs at this head's code: - engine-pinned SdpaFwd quick and standard suites against the CPU and GPU references, both engines plus a Dense regression run; - the knobs suite: every shipped kernel class forced and checked against the CPU reference, plus declines, precedence and a NaN-prefilled workspace; - a timing comparison of every engine that serves each corpus graph, and PyTorch; - RUNBOOK stage 7's final sweep: the full catalog against the reduced one in one session. - How the catalogs were reduced: a 3-round dnn-benchmark sweep with correctness on, over 188 prefill graphs (2D) and 88 decode graphs (3D), then `variant_reduce`. ## Packaging and performance statistics All numbers below come from tool output (packaging logs, device suite runs, dnn-benchmark sweeps). **Packaging** (gfx950 shard, shipped catalogs; pack time is the build's own `hkp_pack` command per engine, median of 3, relative to Dense's on the same job) | Engine | Kernels (full space) | AOT budget | Code objects, zstd | KDP | Pack time vs Dense, build default workers | Serial time per kernel vs Dense | |---|---:|---:|---:|---:|---:|---:| | Unified2D | 222 (792) | 1000 | 1.55 MiB | 0.29 MB | 0.11× | 1.58× | | Unified3D | 123 (348), 246 launches | 400 | 0.64 MiB | 0.26 MB | 0.41× | 1.06× | | Dense (catalog unchanged, for scale) | 840 (52 distinct binaries since #14059) | 1000 | 6.16 MiB | 1.26 MB | 1 | 1 | - The gfx950 kpack is 8.49 MiB at this head; the two engines are 2.19 MiB, 26% of it. Since #14059 (runtime head counts for Dense), Dense's 840 catalog kernels compile to 52 distinct binaries, but the kpack still stores one frame per kernel, so Dense grew from 5.79 to 6.16 MiB. Collapsing those entries is #14059's stated follow-up. The pack-time ratios and the full-space figure (14.45 MiB) were measured against Dense before #14059. - Provider load (`hipdnn_list_engines`, median of 5) is 1.24× its time without these descriptors. **Correctness and coverage** (MI355X, shipped catalogs) | Suite | Unified2D | Unified3D | Dense | |---|---|---|---| | SdpaFwd quick + standard, CPU and GPU reference (served / declined / failed) | 43 / 6 / 0 | 4 / 45 / 0 | 45 / 4 / 0 | | Knobs suite (forced kernel classes, declines, precedence) | 30 / 30 (both engines) | | 62 / 62 | | Corpus graphs served and validated against PyTorch | 188 / 188 | 82 / 88 | | Every decline is intentional: Unified2D leaves decode to Unified3D and declines Hkv > Hq; Unified3D serves decode only, and its 6 corpus declines are batch-256 decode graphs whose K/V is over the 32-bit addressing cap (ASM SDPA serves those). **Kernel time** (MI355X, autotuned, median of 3 rounds; ratio = Unified / other, below 1 = Unified faster; every engine's round-to-round drift ≤ 0.4%). The Unified2D rows were re-measured at this head, on the kernels develop's tiled-2D changes recompiled (see the checklist). | | Graphs both serve | Geomean ratio | Time-weighted (other / Unified) | Unified faster on | |---|---:|---:|---:|---:| | Unified3D vs ASM SDPA (decode) | 60 | 0.200 | 8.17 | 60 / 60 | | Unified3D vs PyTorch | 82 | 0.062 | 34.2 | 82 / 82 | | Unified2D vs Dense (prefill) | 179 | 2.37 | 0.38 | 0 / 179 | | Unified2D vs PyTorch | 188 | 1.38 | 0.55 | 39 / 188 | - Unified3D serves 22 decode graphs no existing hipDNN engine serves. - Unified2D is slower than Dense wherever Dense serves, and Dense keeps ranking first there; it adds coverage Dense lacks (the 9 bottom-right causal graphs in these corpora). **Variant reduction** | Engine | Kernels | Chosen eps | Measured drift | Final sweep, reduced / full kernel time (geomean, rounds 1–3) | |---|---|---:|---:|---| | Unified2D | 792 → 222 | 0.02 | 0.62% | 1.000, 0.999, 1.000 (u2d_attn); 1.000, 0.999, 1.000 (u2d_rocke) | | Unified3D | 348 → 123 | 0.03 | 1.08% | 1.004, 1.002, 0.999 (u3d_attn); 1.002, 1.007, 1.004 (u3d_rocke) | Both arms of the final sweep served the same graphs and passed correctness on every corpus. ## Testing Checklist - [x] Unit (host, no GPU) - `HIP_VISIBLE_DEVICES=-1 hip_kernel_provider_tests` - 1454 passed, including `TestKernelIngestorEngine.EveryHandlerReportsSupportsSavingExactlyWhenItSavesAndRestores`. Our build host also fails 11 Dense signature/dispatch tests: it has a `/nonexistent` directory, which those tests assume is absent. They pass in CI - Status: Passed - [x] Census - `ctest -R 'hkp-census|rocke_unified'` - 36/36 (the census inventories are the reduced catalogs') - Status: Passed - [x] Generator suite (strict skips) - `pytest -p hipdnn_strict_skips IngestorGenerator/tests` - 1646 passed, 0 skipped - Status: Passed - [x] Regeneration gates on the reduced selections - `tests/test_registry_regeneration.py` (g1–g7) 39 passed; AOT budget tests 12 passed - Status: Passed - [x] rocKE attention CPU tests and packaging guards - 2D/3D mask_type, 3D module, decode split, KV cache policy, spec-predicate parity: 145 passed, plus 6 develop KV-cache tests that need the compiled `rocke_engine`; `test_hkp_pack_producer_guards.py` and `test_gfx950_attention_unified_3d_bundle_coverage.py`: 26 passed - Status: Passed - [x] Golden IR, before the reduction - `ctest -R rocke_golden_static` - the reduction changes no rocKE code - Status: Passed - [x] Launch surface - `launch_surface.py configs/gfx950_attention_unified_{2d,3d}.engine.yaml --check` - Status: Passed - [x] Variant sets - `verify_variant_sets.py --mode structural` and `--mode full --arch gfx950 --manifest …` - 2D 222/222 distinct binaries, 3D 123/123 - Status: Passed - [x] Coverage gate rungs 1–2 - `coverage_gate.py --mode full --expect-engine …` - Status: Passed - [x] Validator - `hipdnn_validate_descriptors <shard> --expect-engine … --json` - success, no warnings - Status: Passed - [x] Packaging probe - gfx942 configure with `HIPKERNELPROVIDER_ENABLE_PACKAGING_PROBES=ON`, `ctest -R '^hkp-probe-gfx950'` - 12/12; `hkp-probe-gfx950` packs 51 UKDs of the current descriptors in a gfx942-only build - Status: Passed - [x] Device suites, MI355X - engine-pinned SdpaFwd quick + standard with GPU reference (`hip_kernel_provider_gfx950_attention_*_gpu_ref_integration_tests`) and CPU reference (`*-external-integration_{quick,standard}_suite`) - 0 failures: Unified2D 43 served / 6 declined, Unified3D 4 / 45, Dense 45 / 4 - Status: Passed - [x] Device knobs suite, MI355X - `hip_kernel_provider_integration_tests --gtest_filter='*Gfx950AttentionUnified*'` - 30/30; Dense knobs 62/62 - Status: Passed - [x] Fault re-check, MI355X - the whole `u2d_rocke` corpus in one autotuned process, twice and then with serialized launches - 93/93 each, 0 errors - Status: Passed - [x] Reduction sweep + `variant_reduce.py` - 3 rounds, correctness on; 2D `SWEEP_DONE`, 3D `SWEEP_DONE`; 2D 792 → 222, 3D 348 → 123 - Status: Passed - [x] Final sweep, reduced against full (RUNBOOK §7) - same coverage, correctness passed on both arms, ratios above - Status: Passed - [x] Unified3D catalog regenerated for #14367's LLC-aware K/V cache policy - all 123 kernels now `kv_cache_policy: "all"` (was `"stream"`), same kernel classes; MI355X re-run: suites and knobs unchanged, 246/246 decode rows correct, kernel time against the `"stream"` catalog geomean 0.998, time-weighted 0.999 - Status: Passed - [x] Device evidence at this head (rebased onto develop 2026-11-03, after the review-round refactors) - Dense's and Unified3D's code objects are byte-identical to the previous device runs; develop's #13652 recompiled 201 of the 222 Unified2D code objects (same kernels and ids), and the Unified2D/3D matchers moved onto a shared header, so both engines' device runs were repeated at this head: suites Unified2D 43 / 6 / 0, Unified3D 4 / 45 / 0, Dense 45 / 4 / 0 (passed / skipped / failed), knobs 30/30 and 62/62; compare 188/188 Unified2D graphs (564/564 rows) and 82 Unified3D graphs (246/246 rows) correct against PyTorch in all 3 rounds. Kernel time vs the previous runs: Unified2D geomean 0.985 (none of 188 graphs >2% slower), Unified3D 1.003 (ASM_SDPA control 1.006). A final rebase picked up #14059, which recompiled Dense's 840 code objects only (Unified2D/3D byte-identical to the run above); the suites and knobs were re-run at this head with the same results, `DenseStillRanksAheadWhenUnpinned` included - Status: Passed - [x] DVC - both reduced KDPs on the `ingestor` remote; `dvc status -c` in sync - Status: Passed - [ ] PR CI - GitHub PR checks - Status: Pending ## Technical Changes - **Unified2D pack** (`Gfx950AttentionUnified2DNative.cpp`): matches BSHD SDPA forward on the identity paged-KV layout and builds block_tables, seq_lens and query_start_len in the workspace. - It declines: batch > 511 (the baked 9-iteration binary search), Skv not a multiple of 16, i32 KV over 0x7FFF0000, any window, and the decode cells Unified3D covers (Sq = 1 with the GQA group dividing 16). - Engine knobs: tile_size, num_warps, block_m_per_warp, use_mfma_32x32 and use_transposed_qk_32x32. - **Unified3D pack** (`Gfx950AttentionUnified3DNative.cpp`): decode only. - `launchNames()` is `{segment, reduce}`, and both definitions are resolved through `launchKernelDefinition` with a signature check before any load. - The segment launch writes `[token, Hq, seg, D]` scratch and the reduce always runs, so the workspace needs no zeroing. - Knob: num_segments. No batch limit. - **Arch-neutral geometry** (`UnifiedAttentionGeometry.hpp`): the launch geometry and workspace layouts take a `UnifiedAttentionArchTraits`, and the gfx950 headers are thin wrappers, so a gfx942 pack (AICK-2544) only supplies traits. - **rocKE**: `build_unified_attention_3d_tiled_module` packs the segment and its derived reduce (`reduce_spec_for`) as one `KernelModule`. The module predicate gates both halves. The 3D pack compiles develop's running-max fix for split-KV (#14276). - **Generator**: - `enumerate.also` sections may use `space: full` and carry manifest `launches`. The enumerator refuses a registry that pairs a reduce when the manifest declares no launches, and keeps one default per cell. - The gates are launch-aware: g1 keys on launches, g3 uses `--subset` for full spaces, g4 compares launch sets, and g6 walks every launch and checks module roles. - g2 and g3 run per engine with the shipped ids reused (#14109), comparing every descriptor byte for byte. - `generate.py --stamp-provenance` stamps `provenance.generated_from` (config, manifest, generator and rocKE commits). It is opt-in, so a default regeneration stays byte-identical. - `variant_reduce` fixes: - it reads the format-2 winner lines the runtime has written since #13028; - it keys a graph on its content with `Graph.id` (minted fresh at each finalization) zeroed, so sweep rounds match. - **Catalogs** (the manifests enumerate the full spaces; the selections are `variant_reduce`'s): - Unified2D: 222 kernels over 198 cells (33 head configs × bf16/fp16 × 3 masks): the narrow nw4/mw16/t64 geometry in every cell, transposed 32x32 nw4/mw32/t64 in 21 and narrow nw2/mw16/t32 in 3. The wide 32x32 geometry won no graph and is not shipped. - Unified3D: 123 kernels: num_segments 16 in all 116 decode cells, 32 or 64 in 7 bf16 bottom-right D128 variants. The K/V cache policy follows #14367's LLC-aware rule, which picks `"all"` for every shipped decode cell. - **Descriptors**: both are generated with develop's generator (post-#13028 UED/UHD, UKD `sources` for 3D), every kernel id reused from the full catalog. A regeneration with the shipped ids reused reproduces them byte for byte. The KDPs live in DVC (`ingestor` remote). - **Shared code** (one home per fact): - `packs/UnifiedAttentionGraphMatch.hpp`: the graph-match helpers both unified packs use (operand/BSHD checks, SDPA node, mask classification, dtype, metadata readers); the tests' graph builder is `UnifiedAttentionTestGraphs.hpp`. - rocKE `kernels/common/attention_mask_type.py` (mask ordinals and band -> kind, used by both tiled specs and re-exported by dispatch) and `attention_split_kv_reduce.py` (the segment -> reduce derivation the dispatcher and the module export share). - CMake registers Dense, Unified2D and Unified3D by one loop per lane (external integration, gpu_ref, census). - **Tests**: - The device knobs suite (`IntegrationGpuGfx950AttentionUnifiedKnobs.cpp`) adds a harness `prepareWorkspace` hook. It forces the default geometry/segment count across the head configurations and each other shipped class on a cell that carries it, and `CoversEveryCatalogKernelClass` holds those tables to the catalog. - The engine-pinned `gpu_ref` targets and external-integration lanes cover both engines. - The ingestor save census names all seven pack dispatch symbols. - The Unified2D/3D integration TOMLs are reserved with the gfx950 census shard (`hkp_reserve_census_shard(... FILES ...)`, as #14573 requires of every shard entry). **Constraints and follow-ups** - #13372 and #13889 are both in develop; this branch no longer carries a copy of #13889. - Heads are catalog cells (baked), window = 0, 2D batch ≤ 511, i32 KV ≤ 0x7FFF0000. D256 is deferred for both engines. The kernarg work is tracked in AICK-2675. - There is no 2D/3D `select_path` crossover replay: the split is the static decode rule above. - Runtimes older than UKD 1.1 skip the 3D descriptors. Strided-KV 3D is a follow-up. - Follow-up: execution-plan save/restore (#13315) for both handlers. Today they report `supportsSaving() == false`, as the RDNA handler does. - One earlier MI355X sweep hit a single GPU fault in Unified2D on `u2d_rocke/rocke_sdpa_035` (bf16, D 128, Hq 64 / Hkv 8, causal, S 512). It has not reproduced since: not in the forced geometries on that cell, 15 single-graph runs, 4 full-corpus runs on MI350X, or the MI355X full-corpus re-check above. - Unified2D kernel time moved with develop's tiled-2D changes (#14542, #14616), not with this PR: at this head it is 1.035× (geomean) its pre-rebase time on the same 188 graphs, up to 1.36× on long causal bf16 D128 prefill (S 8192), while Dense in the same runs is flat (0.997). Dense still ranks first wherever it serves; the table above is at this head. - Measurement note: the pinned dnn-benchmarking (v1 result schema, which `sweep.py` reads) has no bottom-right causal PyTorch reference. The sweeps used upstream dnn-benchmarking #84's reference, with the 9 bottom-right corpus graphs written as `causal_mask: false, right_bound: 0, diagonal_alignment: BOTTOM_RIGHT` (the same band under hipDNN's `resolveDiagonalBand`): #84 reads `causal_mask: true` as top-left regardless of the alignment. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Christopher Millette <63608002+cgmillette@users.noreply.github.com>
JIRA ID : AICK-2424
Motivation
The scalar unified-attention fallback keyed its launcher cache on raw
total_q,num_seqs,max_seqlen_qandmax_seqlen_k, so every batch shape compiled its own binary. Lowering the scalar kernels at two values of each field, with everything else fixed, showed which ones actually reach the IR:max_seqlen_qnum_seqstotal_qq<total_q>tag in the 2D / 3D / reduce kernel namesmax_seqlen_kceil(max_seqlen_k / block_size)The last row is also a correctness bug. The scalar launch computed the caller's
block_table.stride(0)and then dropped it (include_bt_stride=False), so the kernel indexed rows with its compiled stride instead. When block-table rows are wider than the longest sequence in the batch needs, which is usual when a framework sizes rows for its maximum context, every sequence after the first reads another sequence's KV blocks. The tiled paths already pass the stride as a kernarg and are not affected.Follow-up to #13372, which did the same normalization for the tiled keys and left the scalar key out of scope.
Technical Details
Stride as a kernarg (both engines)
i32 block_table_stridekernarg, matching the tiled signature, and index block-table rows with it.build_unified_attention_2d,build_unified_attention_3d, and_physical_block_and_token/_emit_qk_score/_emit_v_load.attention_unified_entry.cpp,attention_unified_2d.cpp,attention_unified_3d_reduce.cppandattention_unified_selectors.cpp, plus the matching headers. The unusedmax_blocksbuild-context field is removed._get_scalar_launcherand the scalar launch inrun_unified_attention_torchnow useinclude_bt_stride=True, so the kernel gets the realblock_table.stride(0). That also covers a narrow column view of a wider table.Kernel names.
q<total_q>is removed from the 2D, 3D and reduce scalar kernel names, in both engines. The name is part of the emitted module, so keeping it would give shapes that share a key different IR.Cache key.
_cache_keykeeps arch, the static fields the scalar build reads, and the mask suffix (_mask_key).total_q,num_seqs,max_seqlen_qandmax_seqlen_kare dropped, since none of them reach the IR any more. The launch grid still comes fromtotal_qat launch time and is not cached by key.Emitted IR. For each scalar kernel the diff is the name, the new trailing kernarg, and
mul nsw i32 %seq, <const>becomingmul nsw i32 %seq, %block_table_stride. Nothing else changes..py/.cparity emitters call the builders directly and need no change.sha256,bytesandkernel_namechange in those entries, and no other entry moves.ABI note. Within this repo, the scalar kernels are launched only through
run_unified_attention_torch, which is updated here. Anything outside that launches them by name with the old argument list must appendblock_table_stride.Test Plan
library/tests/test_attention_scalar_cache_key.py(CPU):total_q,num_seqs,max_seqlen_qormax_seqlen_kshare a key. That includes a change in block count, which used to change the compiled stride.library/tests/test_attention_scalar_block_table_numeric.py(GPU), each against an fp32 reference:check_byte_identity.py) at llvm20, llvm22 and llvm23.test_rocke_ci_static.py) at every flavor.max_absagainst an fp32 reference on every row.test_verify_dsl_docs_attention,test_builder_signature_contract,test_attention_tiled_cache_key;test_host_independent_dispatch,test_library_layering;test_gfx1250_attention;test_attention_builds.Test Result
max_absagainst the fp32 reference is identical between the pre-change tree and this branch on every row.Submission Checklist