Repository navigation
fix(rocke): name seqlen and tile when dense attention rejects a partial tile - #13681
Closed
yassinsolim wants to merge 4 commits into
Closed
yassinsolim wants to merge 4 commits into
yassinsolim wants to merge 4 commits into
Conversation
…clines a partial tile The dense _base_spec factories set ragged=True for any self-attention length that is not a tile multiple, so Sq = Skv = 384 or 1152 at block_m 256 is an ordinary graph that takes the on-chip ragged path. The gfx942 predicate then declined with "ragged not yet supported" and the shared spec validator with "ragged is not supported with sliding_window", both of which read as a varlen problem and name nothing the caller can act on. partial_tile_reason(spec) names the length and the tile. The gfx942 predicate now says "gfx942 attention_dense needs whole tiles: seqlen_q 384 is not a multiple of block_m 256 (the partial-tile path is not yet supported)" and the sliding-window check says "sliding_window needs whole tiles: <reason> (the partial-tile path has no window mask)". Varlen messages are unchanged. What could break: anything matching the old "ragged not yet supported" or "ragged is not supported with sliding_window" text. No other caller in the repo matches either string. Checked host only, no GPU: tests/dispatch/attention plus tests/test_attention_dense_gfx942.py give 752 passed, 75 skipped, 2600 subtests passed. With the two messages reverted, the three updated tests fail. black 25.12.0 --check is clean on the changed files.
github-actions
Bot
added
project: hip-kernel-provider
rocke-lib: attention
This was referenced Oct 14, 2026
✅ 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. |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The diagnostic-only changes preserve validation behavior and include focused regression coverage, with no unresolved findings.
0 open findings
What changed in this PR
Improves rocKE dense-attention rejection messages by naming sequence lengths and tile sizes, distinguishing partial tiles from variable-length inputs without changing acceptance behavior.
Changes:
- Adds a shared partial-tile diagnostic helper.
- Updates gfx942 and sliding-window rejection messages.
- Strengthens regression assertions for the new wording.
| File | Description |
|---|---|
dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py |
Checks the specific partial-tile rejection reason. |
dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py |
Verifies sliding-window diagnostics name lengths and tiles. |
dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py |
Covers rejection messages for three partial-tile lengths. |
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py |
Uses the shared diagnostic in gfx942 rejection messages. |
dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py |
Adds the helper and updates sliding-window validation wording. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
yraparti
reviewed
Oct 19, 2026
yraparti
left a comment
yraparti
left a comment
Contributor
There was a problem hiding this comment.
rocKE checklist review
ROCm/rocm-libraries#13681 · 7d9983a503d4 · model gemini-4-argon, fallback gpt-5.5
PASS · graded 220/220 · BLOCKER 0, HIGH 1, MEDIUM 1, LOW 0, NEEDS-INFO 8
Findings
- HIGH §F · summary · Move the branch onto a name with a
rockesegment, such asusers/<user>/rocke/<desc>. - MEDIUM §F · summary · Retitle as
type(scope): summary, 10–80 characters, scope[a-z0-9-]+.
Needs info
- §E.12 dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:565 cover both gfx942 and gfx950. — no diff line supports this verdict
- §E.13 dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:565 update all test names, docstrings, and assertions matching the replaced refusal strings. — no diff line supports this verdict
- §F.8 GitHub review threads and bot/Copilot comments are not included in the provided PR context.
- §F.11 Tests in
dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208anddnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:565directly verify the new partial-tile error messages across multiple sequence lengths. — no diff line supports this verdict - §G.4
dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:233anddnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:573assert exact substring matches (seqlen_q ... is not a multiple of block_m 256) andassertNotIn("ragged", ...)which fail prior to this change. — no diff line supports this verdict - §G.5
dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:228enters via_candidate().admits(_req(...))anddnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:571enters via_spec(req), both exercising reachable dispatch/spec-building paths that fail ifpartial_tile_reasonformatting indnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39is reverted. — no diff line supports this verdict - §G.22 dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208-238 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:565-577 are pure-Python host tests that executed directly (3 new subtests ran and failed under negative control per PR test plan). — no diff line supports this verdict
- §Q.5 The updated unit tests in
dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208anddnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:565run on CPU without a GPU or HIP device using mocked arch contexts. — no diff line supports this verdict
Section notes
- §A: This PR only updates refusal/exception diagnostic message strings in Python dense attention spec validation (
attention_dense_spec.py) and predicate (gfx942/attention_dense.py), along with corresponding unit tests; no IR emission, lowering, or selector acceptance decisions change. - §D: This PR refines diagnostic decline messages for partial-tile sequence lengths in AttentionDenseSpec validation and gfx942 supports_attention_dense.
- §D: Graded items 12-21 of Section D.
- §G: Items 1-17 of 34 in section G.
- §G: Graded items 18-34 for PR #13681 (error message wording update for partial tiles on ragged specs).
- §H: No device-side masks, bounds, indirection tables, or launch inputs are added or modified in this PR; the diff only updates host-side rejection error messages in
AttentionDenseSpec.__post_init__andsupports_attention_dense.
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 | N/A | No kernel-body emitters are added or modified in this PR. |
| 2 | N/A | No core lowering or IR (core/lower_*, core/ir) files are touched. |
| 3 | N/A | The changes in attention_dense_spec.py and gfx942/attention_dense.py only alter diagnostic refusal text without changing what any selector or spec builder computes or accepts. |
| 4 | N/A | No IR emission code in Python or C++ is modified. |
| 5 | N/A | Emitted IR is untouched and no representative golden files are modified. |
| 6 | N/A | No IR, lowering, or cross-engine code is changed, so parity proof gates are not implicated. |
| 7 | N/A | No kernel IR logic or ops are added. |
| 8 | N/A | No C++ files or C++-mirrored components are modified. |
| 9 | N/A | No shared op-handler or lowerer changes are made. |
| 10 | N/A | No new IR feature or codegen flag is introduced. |
| 11 | N/A | No new IR opcode is added. |
| 12 | N/A | core/backend.py and backend fallback logic are not touched. |
| 13 | N/A | .ll emission and debug location handling are not touched. |
| 14 | N/A | No classic helpers under helpers/ are modified. |
§B graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | No new rejection or gate is added; the PR only rewords existing rejection messages for ragged and sliding_window. |
| 2 | N/A | The set of configurations rejected by the spec constructor and preflight gate is unchanged. |
| 3 | N/A | No shapes are demoted or rerouted; only diagnostic rejection strings are updated. |
| 4 | N/A | No input normalization, coercion, clamping, or validation condition changes at any trust boundary. |
| 5 | N/A | No gate scope is widened or narrowed by this change. |
| 6 | PASS | supports_attention_dense in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1081 and AttentionDenseSpec.__post_init__ in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:165 remain functional-only checks and use partial_tile_reason (attention_dense_spec.py:39) to name the specific tile divisibility invariant (seqlen_q % block_m, seqlen_kv % block_n). |
§C graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 extracts partial_tile_reason(spec) as a shared helper used at attention_dense_spec.py:168 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1085. |
| 2 | N/A | This PR does not add or modify a kernel build_* function or shape-class kernel builder. |
| 3 | N/A | This PR does not add or modify kernel builder names or kernel_name() formatting. |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 places the arch-independent partial_tile_reason helper in common/ and imports it in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:198. |
| 5 | PASS | The diff touches only the two rejection message sites (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1084) and the three corresponding test files (test_gfx942_dense_wiring.py:208, test_gfx950_dense_wiring.py:565, test_attention_dense_gfx942.py:755). |
| 6 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 cleanly factors partial_tile_reason with a fallback for explicit ragged=True on aligned lengths rather than duplicating ad-hoc string formatting. |
| 7 | N/A | This PR is a targeted diagnostic message improvement, not a cleanup or refactor no-op. |
| 8 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 adds partial_tile_reason directly in the existing shared spec module alongside AttentionDenseSpec. |
| 9 | PASS | Both places where an auto-flagged ragged=True partial-tile spec was rejected with a generic ragged message are updated together: the shared validator at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:166 and the gfx942 predicate at dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1084. |
| 10 | N/A | This PR does not widen a gate set, shared control-flow branch, or multi-field source of truth. |
§D graded 21/21
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | No new knobs, features, or environment variable reads are added or modified in this PR. |
| 2 | N/A | No candidate Capability declarations or coverage constants are added or modified in this PR. |
| 3 | PASS | supports_attention_dense returns a specific reason naming the invariant via partial_tile_reason(spec) at dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1084, and the wiring test calls _candidate().admits(...) at dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:228. |
| 4 | N/A | No selection-path arch resolution or host device probes are added or modified in this PR. |
| 5 | N/A | No spec cache keys, identity fields, or kernel names are modified in this PR. |
| 6 | N/A | No cohort routing, launch metadata, or cache-key logic is modified in this PR. |
| 7 | N/A | No heuristic or hand-pinned override logic is touched in this PR. |
| 8 | N/A | No cohort predicates or spec overrides are added or modified in this PR. |
| 9 | N/A | No new kernel or architecture candidates are registered in this PR. |
| 10 | N/A | No new spec fields or features are introduced in this PR. |
| 11 | N/A | No candidate registration, opt-in status, or routing precedence is changed in this PR. |
| 12 | N/A | This PR does not add, rename, or reprioritize any candidate or custom ranker. |
| 13 | N/A | This PR does not change pinned request handling, candidate selection, or fallback behavior. |
| 14 | N/A | This PR does not modify cache keys, KernelId definitions, or kernarg layouts / abi_version. |
| 15 | N/A | This PR does not add a new candidate or arch module or alter routing verdicts (only refusal reason strings). |
| 16 | N/A | This PR does not introduce a new operator family or modify request types. |
| 17 | PASS | CPU routing refusal test for partial-tile shapes exercises admits() and checks the updated refusal strings in dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:228. |
| 18 | N/A | This PR does not modify registry ratchet flags (require_build, require_binding, require_torch_binding) or exempt sets. |
| 19 | N/A | This PR only refines dense attention refusal messages and makes no claims about unified attention auto routing. |
| 20 | N/A | This PR only changes Python diagnostic refusal message text in dense attention spec/predicate, which does not change selection outcomes or have a C++ selector twin. |
| 21 | N/A | This PR only updates refusal diagnostic text and does not modify either the spec chooser logic or the kernel emitter. |
§E graded 17/17
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | No files under platform/ are modified in this PR. |
| 2 | N/A | No device queries or torch imports are added or modified. |
| 3 | N/A | No torch dependencies or imports are added in the modified library or test files. |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:198 imports partial_tile_reason from kernels.common.attention_dense_spec, staying within the kernels layer. |
| 5 | N/A | No files under platform/ or helper scripts are added or modified. |
| 6 | N/A | No IR emitter handles are deleted or lint-autofixed; only spec validation and predicate error strings are changed. |
| 7 | N/A | No new env vars, dtypes, public ops, kernel families, manifest kinds, or perf levers are introduced. |
| 8 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1082 contain descriptive comments and strings without any Jira IDs, handles, or PR numbers. |
| 9 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1084 only update partial-tile rejection messages and their corresponding tests. |
| 10 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:166 and the PR description note this was cleanly carved out after #12710 and #12719 landed on develop (8db16c8) without file moves or splits. |
| 11 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208 contain only deliverable kernel spec logic and unit tests. |
| 12 | NEEDS-INFO | dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:565 cover both gfx942 and gfx950. — no diff line supports this verdict |
| 13 | NEEDS-INFO | dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:565 update all test names, docstrings, and assertions matching the replaced refusal strings. — no diff line supports this verdict |
| 14 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:165 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1081 remove only the two targeted error strings and their old test assertions. |
| 15 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 defines partial_tile_reason(spec: "AttentionDenseSpec"), which reads spec.seqlen_q, spec.block_m, spec.seqlen_kv, and spec.block_n at lines 48-60. |
| 16 | N/A | This item applies to review comment grading rather than code changes in the diff. |
| 17 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 and the PR body contain no performance claims or measurements. |
§F graded 12/12
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Branch users/ysoliman/hqs-rocke942-tiles matches the users/<user>/... prefix for changes in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39. |
| 2 | FAIL | branch users/ysoliman/hqs-rocke942-tiles has no rocke path segment |
| 3 | FAIL | title fix(rocke): name the sequence length and tile when dense attention declines a partial tile is 90 characters and does not match type(scope): summary with a 10–80 character length |
| 4 | PASS | description is at least 30 characters and contains an issue reference |
| 5 | PASS | test file dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py is in the diff |
| 6 | PASS | Only .py source and test files are touched, such as dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1084; no keys, certs, or secrets are added. |
| 7 | PASS | Formatting in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1084 conforms to black, and the PR body confirms black 25.12.0 --check is clean. |
| 8 | NEEDS-INFO | GitHub review threads and bot/Copilot comments are not included in the provided PR context. |
| 9 | N/A | This PR targets develop directly (Base: develop), not a stacked feature branch. |
| 10 | PASS | The PR is a small, self-contained change across dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1084 with its matching tests, not artificially split for a size cap. |
| 11 | NEEDS-INFO | Tests in dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:565 directly verify the new partial-tile error messages across multiple sequence lengths. — no diff line supports this verdict |
| 12 | PASS | The PR title and full markdown body are populated and visible in the PR metadata describing dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39. |
§G graded 34/34
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only changes host-side refusal/error message strings in Python spec validation and support predicates without touching IR/kernel emission or byte-identity checks. |
| 2 | N/A | This PR does not modify GPU kernel code, IR emission, or admitted shapes; it only refines diagnostic text for already-rejected partial-tile configurations. |
| 3 | PASS | The PR body documents running tests/dispatch/attention and tests/test_attention_dense_gfx942.py with a baseline comparison against develop (752 passed, 75 skipped, 2600 vs 2597 subtests) covering the updated tests in dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208. |
| 4 | NEEDS-INFO | dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:233 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:573 assert exact substring matches (seqlen_q ... is not a multiple of block_m 256) and assertNotIn("ragged", ...) which fail prior to this change. — no diff line supports this verdict |
| 5 | NEEDS-INFO | dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:228 enters via _candidate().admits(_req(...)) and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:571 enters via _spec(req), both exercising reachable dispatch/spec-building paths that fail if partial_tile_reason formatting in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 is reverted. — no diff line supports this verdict |
| 6 | PASS | All prose claims in docstrings, comments, and PR description match the implementation in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1084, and the claimed shapes (384, 1152, 1000) are directly tested in dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:214. |
| 7 | N/A | This PR does not add or modify a design document or numerical fp32 parity reference. |
| 8 | N/A | This PR does not modify test harnesses or dtype handling. |
| 9 | N/A | This PR does not report or modify LDS usage, VGPR counts, or occupancy numbers. |
| 10 | N/A | This PR makes no performance claims and does not include benchmark scripts or measurements. |
| 11 | N/A | This PR does not introduce or optimize a kernel or alter candidate admission decisions. |
| 12 | N/A | This PR only updates diagnostic strings in Python spec validation and predicates; no cross-engine IR or C++ selector parity behavior is changed. |
| 13 | N/A | This PR does not touch LLVM intrinsics or .td definitions. |
| 14 | N/A | This PR does not classify hardware bottlenecks or make performance claims. |
| 15 | N/A | The PR diff directly reflects the commit object on the head branch without branch-mismatch ambiguity. |
| 16 | N/A | The PR body notes grepping the repo for consumers of the old error strings, and all modified symbols in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:513 are verified in tests. |
| 17 | N/A | This PR does not add or modify standalone shipped scripts. |
| 18 | N/A | No statistical/measurement guard or fault-detection probe is added or modified in this PR. |
| 19 | PASS | dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:214-238 tests shapes where only seqlen_q is a partial tile (384, 1152) and where both seqlen_q and seqlen_kv are partial tiles (1000). |
| 20 | N/A | This PR does not add or modify a claim-ban detector or regex lint. |
| 21 | N/A | This PR does not add or modify any measurement or benchmarking code. |
| 22 | NEEDS-INFO | dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208-238 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:565-577 are pure-Python host tests that executed directly (3 new subtests ran and failed under negative control per PR test plan). — no diff line supports this verdict |
| 23 | N/A | This PR makes no performance or routing speedup claims. |
| 24 | N/A | No golden fixtures or IR hashes are re-blessed or modified in this PR. |
| 25 | N/A | This PR does not contain a codegen micro-optimization or touch kernel ISA generation. |
| 26 | N/A | No benchmarks or performance measurements are included in this PR. |
| 27 | N/A | No A/B performance experiments or kernel tuning knobs are changed in this PR. |
| 28 | N/A | No performance comparisons or counterfactual benchmark tables are included in this PR. |
| 29 | N/A | No math primitives (such as exp2 to exp2_fast) are swapped in this PR. |
| 30 | N/A | No register spill or scratch usage claims are made in this PR. |
| 31 | N/A | No performance baseline or hardware conflict fix is claimed in this PR. |
| 32 | N/A | No hardware or kernel performance measurements are reported in this PR. |
| 33 | N/A | The decision logic (whether a spec is rejected or admitted) is unchanged; only the rejection reason string in attention_dense_spec.py and attention_dense.py is updated. |
| 34 | N/A | No kernel output flags or kernel tensor outputs are modified in this PR. |
§H graded 5/5
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only updates error strings for partial-tile rejection in host spec validation and does not touch hardware/resource bounds or launch tensor validation. |
| 2 | N/A | This PR does not involve paged attention or indirection table lookups. |
| 3 | N/A | This PR does not add or modify device kernel code or hot-loop div/mod operations. |
| 4 | N/A | This PR does not build or modify any device-side masks or bounds from runtime inputs; it only changes error text when rejecting unsupported ragged configurations on the host. |
| 5 | N/A | This PR does not add or modify any launch inputs feeding device-side bounds or masks; the existing compile-time ragged rejections retain negative test coverage in… |
§I graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR refines partial-tile rejection messages and does not perform tuning or routing benchmarks. |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1084 replace cryptic 'ragged' error messages with clear loud-reject messages naming the exact sequence length and tile dimension. |
| 3 | N/A | This PR adds targeted unit subtests for specific error strings rather than changing property-based or generated test coverage. |
| 4 | N/A | This PR does not include benchmark tables or performance reporting. |
| 5 | N/A | This PR does not introduce or modify baked tuning/search results. |
| 6 | N/A | This PR does not add or modify DSL ops or hardware instruction primitives. |
§J graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The diff contains no Markdown or design documents. |
| 2 | N/A | The diff contains no Markdown or design documents. |
| 3 | N/A | The diff contains no Markdown or design documents. |
| 4 | N/A | The diff contains no Markdown or design documents. |
| 5 | N/A | The diff contains no Markdown or design documents. |
| 6 | N/A | The diff contains no Markdown or design documents. |
| 7 | N/A | The diff contains no Markdown or design documents. |
§K graded 13/13
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The diff only changes error message formatting in attention_dense_spec.py and attention_dense.py and does not add or modify any tuned candidates or KnobSpace subclasses. |
| 2 | N/A | No kernel spec fields, KnobAxis declarations, or field classifications are added or modified in the diff. |
| 3 | N/A | No tuning identity payload, canonicalization rules, knob meanings, or spec defaults are changed. |
| 4 | N/A | KnobSpace.defaults() and recorded() are not touched by this PR. |
| 5 | N/A | No knob drop or refuse rules in KnobSpace or tuning/space.py are modified. |
| 6 | N/A | The diff only refines the rejection message text for already-rejected ragged/partial-tile configurations without changing acceptance predicates, KnobSpace.validate, or tuning_specs.py. |
| 7 | N/A | No base(req) implementation or request normalization logic is touched. |
| 8 | N/A | Knob normalization and type coercion are not touched by this diff. |
| 9 | N/A | Benchmark recording, replay, and pin resolution are not touched by this diff. |
| 10 | N/A | KnobSpace.find and tuning_id resolution are not touched by this diff. |
| 11 | N/A | No sweep entry points or sweep harnesses are modified in this diff. |
| 12 | N/A | No production stacks, accept_default rules, or base spec defaults affecting auto selection are changed. |
| 13 | N/A | No benchmark harnesses or ranking logic are touched in this diff. |
§L graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The diff does not add or modify any gfx target or ISA backend. |
| 2 | N/A | The diff does not add or modify any MMA op_id or fragment layout. |
| 3 | N/A | The diff does not change MMA selection, catalogs, lane maps, or lowering. |
| 4 | N/A | The diff does not touch wave size or lane math. |
| 5 | N/A | The diff only updates a refusal string in supports_attention_dense and does not share or alter arch-specific codegen recipes. |
| 6 | N/A | The diff does not touch ArchTarget, core/arch/, policy modules, or CK Tile dispatcher imports. |
| 7 | N/A | The diff does not add or modify dtypes, bit packing, or instruction fragments. |
| 8 | N/A | The diff does not touch scaled-MMA contracts or WMMA/MFMA emission. |
| 9 | N/A | The diff does not introduce a new architecture target. |
| 10 | N/A | The diff does not modify intrinsic arch-domain artifacts or LLVM flavor lists. |
§M graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The diff only modifies host-side spec validation, predicate messages, and unit tests, without emitting any DSL IR or device-time values. |
| 2 | N/A | The diff does not touch legalize_for_target, ISA backends, or kernel builders. |
| 3 | N/A | The diff does not touch IR serialization or print_ir. |
| 4 | N/A | The diff does not touch portable IR schemas, recipes, bundles, or magic division helpers. |
| 5 | N/A | The diff does not touch AsyncTileLoader, LDS layouts, or kernel memory operations. |
| 6 | N/A | The diff does not touch device kernel bodies, barriers, memory loads, or cross-lane shuffles. |
| 7 | N/A | The diff does not touch ISA backends, buffer resource descriptors, or temporal hints. |
| 8 | N/A | The diff does not add or modify LLVM intrinsic declarations or backend engine selection. |
| 9 | N/A | The diff does not touch C++ headers, public C entry points, or C++ library code. |
| 10 | N/A | The diff does not touch debug info emission or DWARF consumers. |
§N graded 18/18
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The diff does not add or modify anything in platform/python/rocke/helpers/. |
| 2 | N/A | The diff only changes spec validation and support-predicate error messages, not kernel addressing, MFMA mapping, loaders, epilogues, or pipelines. |
| 3 | N/A | The diff does not touch SoftwarePipeline or pipeline synchronization. |
| 4 | N/A | The diff does not add or modify fused kernels or EpilogueOp. |
| 5 | N/A | The diff does not touch rocke.helpers.tiling or add any tiling APIs. |
| 6 | N/A | The diff does not add or modify front-door/toolbox tiling helpers or LayoutStyle. |
| 7 | N/A | The diff does not modify tiling MMA layouts, c_desc, or _validate_mma_issue. |
| 8 | N/A | The diff does not touch tiling LDS allocation, lds_store/lds_read, or global load predication. |
| 9 | N/A | The diff does not touch N-D tensor axis roles, at_index/squeeze, or FlowStage label routing. |
| 10 | N/A | The diff does not change tiling emit, LDS staging, or MMA issue. |
| 11 | N/A | The diff does not use or modify tiling MMA traits, lds_swizzle, or derive_sched_group_counts. |
| 12 | N/A | The diff makes no LDS bank-conflict or memory coalescing claims. |
| 13 | N/A | The diff does not touch WarpDistributionEncoding, make_tile_desc, RegisterMapper, or store casts. |
| 14 | N/A | The diff does not touch tiling verbs, barriers, waitcnt, or LDS pipelining. |
| 15 | N/A | The diff does not touch tiling allocation or bounds checks. |
| 16 | N/A | The diff does not touch optimize_layout or evaluate_transform. |
| 17 | N/A | The diff does not touch tiling_recorder, round-trip analysis, or visualization. |
| 18 | N/A | The diff does not touch interleaved layouts, tiling demo kernels, or numeric gates. |
§O graded 16/16
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The diff only modifies Python files under rocke/library/ and does not touch IR verification, lowerers, or platform core files. |
| 2 | N/A | The diff does not construct or modify scf.for regions, control flow terminators, or IR builder internals. |
| 3 | N/A | The diff only updates error message strings in library attention spec validation and does not touch IR or LLVM emission. |
| 4 | N/A | The diff does not touch IR type definitions, SmemType, or IR parsers. |
| 5 | N/A | The diff does not touch core/passes.py, cpp/core/passes.cpp, or opcode purity tables. |
| 6 | N/A | The diff does not modify constant folding passes in Python or C++. |
| 7 | N/A | The diff does not touch kernel attributes, codegen policy, or COMGR options. |
| 8 | N/A | The diff does not touch SchedulerStrategy, CodegenPolicy, or autotune policy variants. |
| 9 | N/A | The diff does not emit or lower waits, barriers, or optimization_barrier primitives. |
| 10 | N/A | The diff does not touch LLVM lowerers or attribute validation in platform core. |
| 11 | N/A | The diff does not touch ArchTarget or target resolution logic in platform core. |
| 12 | N/A | The diff does not touch target catalog width/size caps or arch default resolution. |
| 13 | N/A | The diff does not touch any C headers under cpp/include/rocke/ or ABI version constants. |
| 14 | N/A | The diff does not add or modify any C++ source files, headers, or CMake files. |
| 15 | N/A | The diff does not touch C++ engine fallback handling or backend lowering paths. |
| 16 | N/A | The diff does not touch C++ codegen library capabilities or known_gaps.md coverage areas. |
§P graded 14/14
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The diff does not touch any kernel builder signature or spec REQUIRED_FIELDS. |
| 2 | N/A | The diff does not modify kernarg signatures, argument packing, or ABI definitions. |
| 3 | N/A | The diff does not change launch geometry or grid calculation logic. |
| 4 | N/A | The diff does not touch LDS capacity gates or LDS allocation calculations. |
| 5 | N/A | The diff does not touch WMMA/RDNA GEMM, wave_size rules, or GemmMultiD/GemmMultiAbd instances. |
| 6 | N/A | The diff does not touch Stream-K, MoE, or split-K workspace allocation or clearing. |
| 7 | N/A | The diff only rewords existing rejection messages (attention_dense_spec.py:166, attention_dense.py:1084) and does not alter which configs are accepted or rejected. |
| 8 | N/A | The diff does not involve quantization or MX GEMM scale decoding. |
| 9 | N/A | The diff does not touch numeric emulation, decode paths, or error bounds. |
| 10 | N/A | The diff does not touch manifest generation, schema, or runners. |
| 11 | N/A | The diff does not touch kernel launching, stream synchronization, workspaces, or timing harnesses. |
| 12 | N/A | The diff does not touch COMGR/HIP library loading or fix a HIP launch error. |
| 13 | N/A | The diff only formats error message strings when spec.ragged is rejected and does not change host launch math, cache keys, or admission guards (attention_dense_spec.py:39, attention_dense.py:1081). |
| 14 | N/A | The diff does not touch examples, expected.json, helpers/, or add new files to kernels/common/. |
§Q graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only edits existing test methods under rocke/library/tests/ and does not add or remove any platform/ test files or CMake test registrations. |
| 2 | N/A | This PR refines rejection diagnostic strings in attention_dense_spec.py and attention_dense.py without adding a new kernel family or variant. |
| 3 | N/A | This PR does not touch or make claims about platform IR/lowering gates (check_byte_identity.py, run_diff.py, check_ir_validity.py, or fuzz_diff.py). |
| 4 | N/A | This PR does not modify any IR goldens, lowerers, or builders that widen the valid spec space. |
| 5 | NEEDS-INFO | The updated unit tests in dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:565 run on CPU without a GPU or HIP device using mocked arch contexts. — no diff line supports this verdict |
| 6 | N/A | This PR does not refactor any gate or test harness. |
| 7 | N/A | This PR only changes wording inside Python library AttentionDenseSpec validation and supports_attention_dense predicate strings, not the platform C++/Python backend rejection contract. |
…in the unified mask tests The sliding_window branch of AttentionDenseSpec.__post_init__ no longer says "ragged", so test_ragged_window_is_declined_not_served asserted text that this branch removed. It now checks the same three parts the gfx950 and gfx942 wiring tests check: the query length against block_m, the key length against block_n, and that the reason still names sliding_window and no longer says ragged.
yraparti
reviewed
Oct 20, 2026
yraparti
left a comment
yraparti
left a comment
Contributor
There was a problem hiding this comment.
rocKE checklist re-review of the fixes since 7d9983a503d4
ROCm/rocm-libraries#13681 · 1b17babac34b · model gemini-4-argon, fallback gpt-5.5
PASS · graded 220/220 · BLOCKER 0, HIGH 1, MEDIUM 1, LOW 0, NEEDS-INFO 12
Since the last review (7d9983a503d4)
Resolved 0 · still open 2 · new 0
- ⏳ §F.2 still FAIL (HIGH): Move the branch onto a name with a
rockesegment, such asusers/<user>/rocke/<desc>. - ⏳ §F.3 still FAIL (MEDIUM): Retitle as
type(scope): summary, 10–80 characters, scope[a-z0-9-]+.
Findings
- HIGH §F · summary · Move the branch onto a name with a
rockesegment, such asusers/<user>/rocke/<desc>. - MEDIUM §F · summary · Retitle as
type(scope): summary, 10–80 characters, scope[a-z0-9-]+.
Needs info
- §C.5 The diff touches only the two rejection message sites (
dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39anddnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1084) and the four corresponding test files (dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208,dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:565,dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:357,dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:755). —… - §D.3
supports_attention_densereturns a specific reason naming the invariant viapartial_tile_reason(spec)atdnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1084, and the wiring tests calladmits(...)atdnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:228anddnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:375. — no diff line supports this verdict - §D.17 CPU routing refusal tests for partial-tile shapes exercise
admits()and check the updated refusal strings in dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:228 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:375. — no diff line supports this verdict - §E.12 Both architectures are covered: dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208 tests gfx942 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:565 tests gfx950. — no diff line supports this verdict
- §E.13 The new commit resolves the remaining stale test reference in dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:357, along with dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:565. — no diff line supports this verdict
- §F.8 GitHub review threads and bot/Copilot comments are not included in the provided PR context.
- §F.11 The updated tests in
dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208,dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:565, anddnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:357are real assertions checking the new partial-tile diagnostic strings, not throwaway tests. — no diff line supports this verdict - §G.4 Resolved since previous review with valid diff lines:
dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:235,dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:573, anddnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:377assert exact substring matches (seqlen_q ... is not a multiple of block_m 256) andassertNotIn("ragged", ...)that fail before this change. — no diff line supports this verdict
Section notes
- §G: Items 1–17 of 34 in section G.
- §G: Re-review of items 18-34 against head 1b17bab. The follow-up commit updated test_unified_mask_wiring.py to check the new partial-tile error message instead of 'ragged'.
- §H: No device-side masks or bounds are added or modified in this PR; the diff only changes the error message strings for host-side partial-tile (
ragged=True) rejection inAttentionDenseSpec.__post_init__andsupports_attention_dense, and updates the corresponding unit tests. - §I: Re-reviewed at head 1b17bab; the update to test_unified_mask_wiring.py aligns the remaining test assertion with the loud-reject partial-tile message.
- §J: The diff contains no Markdown files or design documents.
- §K: The PR only refines refusal message strings when
ragged=Trueis rejected inAttentionDenseSpec.__post_init__andsupports_attention_dense, and updates tests accordingly; no tuning spaces, knob axes,config_keypayloads, or sweep entry points are touched.
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 | N/A | No kernel-body emitters are added or modified in this PR. |
| 2 | N/A | No core lowering or IR (core/lower_*, core/ir) files are touched. |
| 3 | N/A | The changes in attention_dense_spec.py and gfx942/attention_dense.py only alter diagnostic refusal text without changing what any selector or spec builder computes or accepts. |
| 4 | N/A | No IR emission code in Python or C++ is modified. |
| 5 | N/A | Emitted IR is untouched and no representative golden files are modified. |
| 6 | N/A | No IR, lowering, or cross-engine code is changed, so parity proof gates are not implicated. |
| 7 | N/A | No kernel IR logic or ops are added. |
| 8 | N/A | No C++ files or C++-mirrored components are modified. |
| 9 | N/A | No shared op-handler or lowerer changes are made. |
| 10 | N/A | No new IR feature or codegen flag is introduced. |
| 11 | N/A | No new IR opcode is added. |
| 12 | N/A | core/backend.py and backend fallback logic are not touched. |
| 13 | N/A | .ll emission and debug location handling are not touched. |
| 14 | N/A | No classic helpers under helpers/ are modified. |
§B graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | No new rejection or gate is added; the PR only rewords existing rejection messages for ragged and sliding_window. |
| 2 | N/A | The set of configurations rejected by the spec constructor and preflight gate is unchanged. |
| 3 | N/A | No shapes are demoted or rerouted; only diagnostic rejection strings are updated. |
| 4 | N/A | No input normalization, coercion, clamping, or validation condition changes at any trust boundary. |
| 5 | N/A | No gate scope is widened or narrowed by this change. |
| 6 | PASS | supports_attention_dense in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1081 and AttentionDenseSpec.__post_init__ in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:165 remain functional-only checks and use partial_tile_reason (attention_dense_spec.py:39) to name the specific tile divisibility invariant (seqlen_q % block_m, seqlen_kv % block_n). |
§C graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 extracts partial_tile_reason(spec) as a shared helper used at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:168 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1085. |
| 2 | N/A | This PR does not add or modify a kernel build_* function or shape-class kernel builder. |
| 3 | N/A | This PR does not add or modify kernel builder names or kernel_name() formatting. |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 places the arch-independent partial_tile_reason helper in common/ and imports it in dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:198. |
| 5 | NEEDS-INFO | The diff touches only the two rejection message sites (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1084) and the four corresponding test files (dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208, dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:565, dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:357, dnn-providers/hip-kernel-provider/rocke/library/tests/test_attention_dense_gfx942.py:755). —… |
| 6 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 cleanly factors partial_tile_reason with a fallback for explicit ragged=True on aligned lengths rather than duplicating ad-hoc string formatting. |
| 7 | N/A | This PR is a targeted diagnostic message improvement, not a cleanup or refactor no-op. |
| 8 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 adds partial_tile_reason directly in the existing shared spec module alongside AttentionDenseSpec. |
| 9 | PASS | Both places where an auto-flagged ragged=True partial-tile spec was rejected with a generic ragged message are updated (dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:166 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1084), and all test sites matching the old message were updated including… |
| 10 | N/A | This PR does not widen a gate set, shared control-flow branch, or multi-field source of truth. |
§D graded 21/21
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | No new knobs, features, or environment variable reads are added or modified in this PR. |
| 2 | N/A | No candidate Capability declarations or coverage constants are added or modified in this PR. |
| 3 | NEEDS-INFO | supports_attention_dense returns a specific reason naming the invariant via partial_tile_reason(spec) at dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1084, and the wiring tests call admits(...) at dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:228 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:375. — no diff line supports this verdict |
| 4 | N/A | No selection-path arch resolution or host device probes are added or modified in this PR. |
| 5 | N/A | No spec cache keys, identity fields, or kernel names are modified in this PR. |
| 6 | N/A | No cohort routing, launch metadata, or cache-key logic is modified in this PR. |
| 7 | N/A | No heuristic or hand-pinned override logic is touched in this PR. |
| 8 | N/A | No cohort predicates or spec overrides are added or modified in this PR. |
| 9 | N/A | No new kernel or architecture candidates are registered in this PR. |
| 10 | N/A | No new spec fields or features are introduced in this PR. |
| 11 | N/A | No candidate registration, opt-in status, or routing precedence is changed in this PR. |
| 12 | N/A | This PR does not add, rename, or reprioritize any candidate or custom ranker. |
| 13 | N/A | This PR does not change pinned request handling, candidate selection, or fallback behavior. |
| 14 | N/A | This PR does not modify cache keys, KernelId definitions, or kernarg layouts / abi_version. |
| 15 | N/A | This PR does not add a new candidate or arch module or alter routing verdicts (only refusal reason strings). |
| 16 | N/A | This PR does not introduce a new operator family or modify request types. |
| 17 | NEEDS-INFO | CPU routing refusal tests for partial-tile shapes exercise admits() and check the updated refusal strings in dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:228 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:375. — no diff line supports this verdict |
| 18 | N/A | This PR does not modify registry ratchet flags (require_build, require_binding, require_torch_binding) or exempt sets. |
| 19 | N/A | This PR only refines dense attention refusal messages and makes no claims about unified attention auto routing. |
| 20 | N/A | This PR only changes Python diagnostic refusal message text in dense attention spec/predicate, which does not change selection outcomes or have a C++ selector twin. |
| 21 | N/A | This PR only updates refusal diagnostic text and does not modify either the spec chooser logic or the kernel emitter. |
§E graded 17/17
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | No files under platform/ are modified in this PR. |
| 2 | N/A | No device queries or torch imports are added or modified. |
| 3 | N/A | No torch dependencies or imports are added in the modified library or test files. |
| 4 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:198 imports partial_tile_reason from kernels.common.attention_dense_spec, staying within the kernels layer. |
| 5 | N/A | No files under platform/ or helper scripts are added or modified. |
| 6 | N/A | No IR emitter handles are deleted or lint-autofixed; only spec validation and predicate error strings are changed. |
| 7 | N/A | No new env vars, dtypes, public ops, kernel families, manifest kinds, or perf levers are introduced. |
| 8 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1082 contain descriptive comments and strings without any Jira IDs, handles, or PR numbers. |
| 9 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1084 only update partial-tile rejection messages and their corresponding tests. |
| 10 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:166 and the PR description note this was cleanly carved out after #12710 and #12719 landed on develop without file moves or splits. |
| 11 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208 contain only deliverable kernel spec logic and unit tests. |
| 12 | NEEDS-INFO | Both architectures are covered: dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208 tests gfx942 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:565 tests gfx950. — no diff line supports this verdict |
| 13 | NEEDS-INFO | The new commit resolves the remaining stale test reference in dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:357, along with dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:565. — no diff line supports this verdict |
| 14 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:166 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1081 replace only the two targeted error strings and their old test assertions. |
| 15 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 defines partial_tile_reason(spec: "AttentionDenseSpec"), which reads spec.seqlen_q, spec.block_m, spec.seqlen_kv, and spec.block_n at lines 48-60. |
| 16 | N/A | This item applies to review comment grading rather than code changes in the diff. |
| 17 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 and the PR body contain no performance claims or measurements. |
§F graded 12/12
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Branch users/ysoliman/hqs-rocke942-tiles matches the users/<user>/... prefix for changes in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39. |
| 2 | FAIL | branch users/ysoliman/hqs-rocke942-tiles has no rocke path segment |
| 3 | FAIL | title fix(rocke): name the sequence length and tile when dense attention declines a partial tile is 90 characters and does not match type(scope): summary with a 10–80 character length |
| 4 | PASS | description is at least 30 characters and contains an issue reference |
| 5 | PASS | test file dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py is in the diff |
| 6 | PASS | Only .py source and test files are modified, such as dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1084; no keys, certs, or secrets are added. |
| 7 | PASS | The formatting in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 and the newly touched dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:377 conforms to black, and the PR body states black 25.12.0 --check is clean. |
| 8 | NEEDS-INFO | GitHub review threads and bot/Copilot comments are not included in the provided PR context. |
| 9 | N/A | This PR targets develop directly (Base: develop), not a stacked feature branch. |
| 10 | PASS | The PR is a focused change across dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1084 together with all affected tests, not artificially split for a size cap. |
| 11 | NEEDS-INFO | The updated tests in dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208, dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:565, and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:357 are real assertions checking the new partial-tile diagnostic strings, not throwaway tests. — no diff line supports this verdict |
| 12 | PASS | The PR title and markdown body (updated in the latest push to mention dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:357 and head 1b17babac3) are populated in the PR metadata. |
§G graded 34/34
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only changes host-side refusal/error message strings in Python spec validation and support predicates without touching IR/kernel emission or byte-identity checks. |
| 2 | N/A | This PR does not modify GPU kernel code, IR emission, or admitted shapes; it only refines diagnostic text for already-rejected partial-tile configurations. |
| 3 | PASS | The PR body documents running tests/dispatch/attention and tests/test_attention_dense_gfx942.py with a baseline comparison against develop (882 passed, 121 skipped, 3296 vs 3293 subtests) covering the updated tests in dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:357. |
| 4 | NEEDS-INFO | Resolved since previous review with valid diff lines: dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:235, dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:573, and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:377 assert exact substring matches (seqlen_q ... is not a multiple of block_m 256) and assertNotIn("ragged", ...) that fail before this change. — no diff line supports this verdict |
| 5 | NEEDS-INFO | Resolved since previous review with valid diff lines: dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:228 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:375 enter via admits(...) and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:571 enters via _spec(req), exercising reachable dispatch/spec-building paths that fail if partial_tile_reason in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 is reverted. — no diff line supports this verdict |
| 6 | PASS | All prose claims in docstrings, comments, and PR description match the implementation in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39, dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1084, and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:357, and the claimed shapes (384, 1152, 1000) are directly tested in… |
| 7 | N/A | This PR does not add or modify a design document or numerical fp32 parity reference. |
| 8 | N/A | This PR does not modify test harnesses or dtype handling. |
| 9 | N/A | This PR does not report or modify LDS usage, VGPR counts, or occupancy numbers. |
| 10 | N/A | This PR makes no performance claims and does not include benchmark scripts or measurements. |
| 11 | N/A | This PR does not introduce or optimize a kernel or alter candidate admission decisions. |
| 12 | N/A | This PR only updates diagnostic strings in Python spec validation and predicates; no cross-engine IR or C++ selector parity behavior is changed. |
| 13 | N/A | This PR does not touch LLVM intrinsics or .td definitions. |
| 14 | N/A | This PR does not classify hardware bottlenecks or make performance claims. |
| 15 | N/A | The PR diff directly reflects the commit object on the head branch without branch-mismatch ambiguity. |
| 16 | N/A | The PR body notes grepping the repo and catching the substring match in dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:357, and all modified symbols in… |
| 17 | N/A | This PR does not add or modify standalone shipped scripts. |
| 18 | N/A | No statistical/measurement guard or fault-detection probe is added or modified in this PR. |
| 19 | NEEDS-INFO | dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:214-238 and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:374-384 test fixtures with partial-tile seqlen_qonly (384, 1152) and both seqlen_q and seqlen_kv (1000/1000, 1000/1050) across the inline checks. — no diff line supports this verdict |
| 20 | N/A | This PR does not add or modify a claim-ban detector or regex lint. |
| 21 | N/A | This PR does not add or modify any measurement or benchmarking code. |
| 22 | NEEDS-INFO | Resolves previous NEEDS-INFO: dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:208-238, dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:565-577, and dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:357-384 are pure-Python unit tests without conditional skip guards or golden build-error entries, and the PR test plan confirms all 3 new subtests executed and failed under negative control. — no diff line supports this verdict |
| 23 | N/A | This PR makes no performance or routing speedup claims. |
| 24 | N/A | No golden fixtures or IR hashes are re-blessed or modified in this PR. |
| 25 | N/A | This PR does not contain a codegen micro-optimization or touch kernel ISA generation. |
| 26 | N/A | No benchmarks or performance measurements are included in this PR. |
| 27 | N/A | No A/B performance experiments or kernel tuning knobs are changed in this PR. |
| 28 | N/A | No performance comparisons or counterfactual benchmark tables are included in this PR. |
| 29 | N/A | No math primitives (such as exp2 to exp2_fast) are swapped in this PR. |
| 30 | N/A | No register spill or scratch usage claims are made in this PR. |
| 31 | N/A | No performance baseline or hardware conflict fix is claimed in this PR. |
| 32 | N/A | No hardware or kernel performance measurements are reported in this PR. |
| 33 | N/A | The decision boolean (whether a spec is rejected or admitted) is unchanged; only the human-readable rejection reason string in attention_dense_spec.py and attention_dense.py is updated. |
| 34 | N/A | No kernel output flags or kernel tensor outputs are modified in this PR. |
§H graded 5/5
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only updates error strings for partial-tile rejection in host spec validation and does not touch hardware/resource bounds or launch tensor validation. |
| 2 | N/A | This PR does not involve paged attention or indirection table lookups. |
| 3 | N/A | This PR does not add or modify device kernel code or hot-loop div/mod operations. |
| 4 | N/A | This PR does not build or modify any device-side masks or bounds from runtime inputs; it only changes error text when rejecting unsupported ragged configurations on the host. |
| 5 | N/A | This PR does not add or modify any launch inputs feeding device-side bounds or masks; the existing compile-time ragged rejections retain negative test coverage in… |
§I graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR refines partial-tile rejection messages and does not perform tuning or routing benchmarks. |
| 2 | PASS | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/attention_dense_spec.py:39 and dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py:1084 replace cryptic 'ragged' error messages with clear loud-reject messages naming the exact sequence length and tile dimension, tested across dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:357. |
| 3 | N/A | This PR adds targeted unit subtests for specific error strings rather than changing property-based or generated test coverage. |
| 4 | N/A | This PR does not include benchmark tables or performance reporting. |
| 5 | N/A | This PR does not introduce or modify baked tuning/search results. |
| 6 | N/A | This PR does not add or modify DSL ops or hardware instruction primitives. |
§J graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The diff contains no Markdown or design documents. |
| 2 | N/A | The diff contains no Markdown or design documents. |
| 3 | N/A | The diff contains no Markdown or design documents. |
| 4 | N/A | The diff contains no Markdown or design documents. |
| 5 | N/A | The diff contains no Markdown or design documents. |
| 6 | N/A | The diff contains no Markdown or design documents. |
| 7 | N/A | The diff contains no Markdown or design documents. |
§K graded 13/13
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The diff only changes error message formatting in attention_dense_spec.py and attention_dense.py and does not add or modify any tuned candidates or KnobSpace subclasses. |
| 2 | N/A | No kernel spec fields, KnobAxis declarations, or field classifications are added or modified in the diff. |
| 3 | N/A | No tuning identity payload, canonicalization rules, knob meanings, or spec defaults are changed. |
| 4 | N/A | KnobSpace.defaults() and recorded() are not touched by this PR. |
| 5 | N/A | No knob drop or refuse rules in KnobSpace or tuning/space.py are modified. |
| 6 | N/A | The diff only refines the rejection message text for already-rejected ragged/partial-tile configurations without changing acceptance predicates, KnobSpace.validate, or tuning_specs.py. |
| 7 | N/A | No base(req) implementation or request normalization logic is touched. |
| 8 | N/A | Knob normalization and type coercion are not touched by this diff. |
| 9 | N/A | Benchmark recording, replay, and pin resolution are not touched by this diff. |
| 10 | N/A | KnobSpace.find and tuning_id resolution are not touched by this diff. |
| 11 | N/A | No sweep entry points or sweep harnesses are modified in this diff. |
| 12 | N/A | No production stacks, accept_default rules, or base spec defaults affecting auto selection are changed. |
| 13 | N/A | No benchmark harnesses or ranking logic are touched in this diff. |
§L graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The diff does not add or modify any gfx target or ISA backend. |
| 2 | N/A | The diff does not add or modify any MMA op_id or fragment layout. |
| 3 | N/A | The diff does not change MMA selection, catalogs, lane maps, or lowering. |
| 4 | N/A | The diff does not touch wave size or lane math. |
| 5 | N/A | The diff only updates refusal message strings and tests, without altering or sharing arch-specific codegen recipes. |
| 6 | N/A | The diff does not touch ArchTarget, core/arch/, policy modules, or CK Tile dispatcher imports. |
| 7 | N/A | The diff does not add or modify dtypes, bit packing, or instruction fragments. |
| 8 | N/A | The diff does not touch scaled-MMA contracts or WMMA/MFMA emission. |
| 9 | N/A | The diff does not introduce a new architecture target. |
| 10 | N/A | The diff does not modify intrinsic arch-domain artifacts or LLVM flavor lists. |
§M graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The diff only modifies host-side spec validation, predicate messages, and unit tests, without emitting any DSL IR or device-time values. |
| 2 | N/A | The diff does not touch legalize_for_target, ISA backends, or kernel builders. |
| 3 | N/A | The diff does not touch IR serialization or print_ir. |
| 4 | N/A | The diff does not touch portable IR schemas, recipes, bundles, or magic division helpers. |
| 5 | N/A | The diff does not touch AsyncTileLoader, LDS layouts, or kernel memory operations. |
| 6 | N/A | The diff does not touch device kernel bodies, barriers, memory loads, or cross-lane shuffles. |
| 7 | N/A | The diff does not touch ISA backends, buffer resource descriptors, or temporal hints. |
| 8 | N/A | The diff does not add or modify LLVM intrinsic declarations or backend engine selection. |
| 9 | N/A | The diff does not touch C++ headers, public C entry points, or C++ library code. |
| 10 | N/A | The diff does not touch debug info emission or DWARF consumers. |
§N graded 18/18
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The diff does not add or modify anything in platform/python/rocke/helpers/. |
| 2 | N/A | The diff only changes spec validation and support-predicate error messages, not kernel addressing, MFMA mapping, loaders, epilogues, or pipelines. |
| 3 | N/A | The diff does not touch SoftwarePipeline or pipeline synchronization. |
| 4 | N/A | The diff does not add or modify fused kernels or EpilogueOp. |
| 5 | N/A | The diff does not touch rocke.helpers.tiling or add any tiling APIs. |
| 6 | N/A | The diff does not add or modify front-door/toolbox tiling helpers or LayoutStyle. |
| 7 | N/A | The diff does not modify tiling MMA layouts, c_desc, or _validate_mma_issue. |
| 8 | N/A | The diff does not touch tiling LDS allocation, lds_store/lds_read, or global load predication. |
| 9 | N/A | The diff does not touch N-D tensor axis roles, at_index/squeeze, or FlowStage label routing. |
| 10 | N/A | The diff does not change tiling emit, LDS staging, or MMA issue. |
| 11 | N/A | The diff does not use or modify tiling MMA traits, lds_swizzle, or derive_sched_group_counts. |
| 12 | N/A | The diff makes no LDS bank-conflict or memory coalescing claims. |
| 13 | N/A | The diff does not touch WarpDistributionEncoding, make_tile_desc, RegisterMapper, or store casts. |
| 14 | N/A | The diff does not touch tiling verbs, barriers, waitcnt, or LDS pipelining. |
| 15 | N/A | The diff does not touch tiling allocation or bounds checks. |
| 16 | N/A | The diff does not touch optimize_layout or evaluate_transform. |
| 17 | N/A | The diff does not touch tiling_recorder, round-trip analysis, or visualization. |
| 18 | N/A | The diff does not touch interleaved layouts, tiling demo kernels, or numeric gates. |
§O graded 16/16
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The diff only modifies Python files under rocke/library/ and does not touch IR verification, lowerers, or platform core files. |
| 2 | N/A | The diff does not construct or modify scf.for regions, control flow terminators, or IR builder internals. |
| 3 | N/A | The diff only updates error message strings and test assertions in library attention spec validation and does not touch IR or LLVM emission. |
| 4 | N/A | The diff does not touch IR type definitions, SmemType, or IR parsers. |
| 5 | N/A | The diff does not touch core/passes.py, cpp/core/passes.cpp, or opcode purity tables. |
| 6 | N/A | The diff does not modify constant folding passes in Python or C++. |
| 7 | N/A | The diff does not touch kernel attributes, codegen policy, or COMGR options. |
| 8 | N/A | The diff does not touch SchedulerStrategy, CodegenPolicy, or autotune policy variants. |
| 9 | N/A | The diff does not emit or lower waits, barriers, or optimization_barrier primitives. |
| 10 | N/A | The diff does not touch LLVM lowerers or attribute validation in platform core. |
| 11 | N/A | The diff does not touch ArchTarget or target resolution logic in platform core. |
| 12 | N/A | The diff does not touch target catalog width/size caps or arch default resolution. |
| 13 | N/A | The diff does not touch any C headers under cpp/include/rocke/ or ABI version constants. |
| 14 | N/A | The diff does not add or modify any C++ source files, headers, or CMake files. |
| 15 | N/A | The diff does not touch C++ engine fallback handling or backend lowering paths. |
| 16 | N/A | The diff does not touch C++ codegen library capabilities or known_gaps.md coverage areas. |
§P graded 14/14
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The diff does not touch any kernel builder signature or spec REQUIRED_FIELDS. |
| 2 | N/A | The diff does not modify kernarg signatures, argument packing, or ABI definitions. |
| 3 | N/A | The diff does not change launch geometry or grid calculation logic. |
| 4 | N/A | The diff does not touch LDS capacity gates or LDS allocation calculations. |
| 5 | N/A | The diff does not touch WMMA/RDNA GEMM, wave_size rules, or GemmMultiD/GemmMultiAbd instances. |
| 6 | N/A | The diff does not touch Stream-K, MoE, or split-K workspace allocation or clearing. |
| 7 | N/A | The diff only rewords existing rejection messages (attention_dense_spec.py:166, attention_dense.py:1084) and does not alter which configs are accepted or rejected. |
| 8 | N/A | The diff does not involve quantization or MX GEMM scale decoding. |
| 9 | N/A | The diff does not touch numeric emulation, decode paths, or error bounds. |
| 10 | N/A | The diff does not touch manifest generation, schema, or runners. |
| 11 | N/A | The diff does not touch kernel launching, stream synchronization, workspaces, or timing harnesses. |
| 12 | N/A | The diff does not touch COMGR/HIP library loading or fix a HIP launch error. |
| 13 | N/A | The diff only formats error message strings when spec.ragged is rejected and does not change host launch math, cache keys, or admission guards (attention_dense_spec.py:39, attention_dense.py:1081). |
| 14 | N/A | The diff does not touch examples, expected.json, helpers/, or add new files to kernels/common/. |
§Q graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only edits existing test methods under rocke/library/tests/ and does not add or remove any platform/ test files or CMake test registrations. |
| 2 | N/A | This PR refines rejection diagnostic strings in attention_dense_spec.py and attention_dense.py without adding a new kernel family or variant. |
| 3 | N/A | This PR does not touch or make claims about platform IR/lowering gates (check_byte_identity.py, run_diff.py, check_ir_validity.py, or fuzz_diff.py). |
| 4 | N/A | This PR does not modify any IR goldens, lowerers, or builders that widen the valid spec space. |
| 5 | NEEDS-INFO | Resolved: the updated unit tests in dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx942_dense_wiring.py:225, dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_gfx950_dense_wiring.py:565, and the newly updated dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/attention/test_unified_mask_wiring.py:357 are pure-Python CPU tests that require neither a GPU/HIP device nor clang/torch. — no diff line supports this verdict |
| 6 | N/A | This PR does not refactor any gate or test harness. |
| 7 | N/A | This PR only changes wording inside Python library AttentionDenseSpec validation and supports_attention_dense predicate strings, not the platform C++/Python backend rejection contract. |
yassinsolim
changed the title
1 task done
Member
Author
|
GitHub closed this PR when I renamed its branch to |
cgmillette
pushed a commit
that referenced
this pull request
Nov 4, 2026
…al tile (#14335) Replaces #13681 with the same commit (1b17bab). GitHub closed #13681 when its branch moved to `users/ysoliman/rocke/` for the rocKE branch-naming rule, and a PR cannot be reopened once its branch is renamed. The review history, including the rocKE checklist reviews, stays on #13681. Carved out of #12928, which is closed. The rest of that PR is already on develop (#12710 landed the nhead_k guard, #12719 replaced the factories this touched and routes `_request_errors` through every dense candidate). ## Motivation The dense `_base_spec` factories set `ragged=True` for any self-attention length that is not a tile multiple, so Sq = Skv = 384 or 1152 at block_m 256 is an ordinary graph taking the on-chip ragged path. The gfx942 predicate then declined with "gfx942 attention_dense: ragged not yet supported" and the shared spec validator with "ragged is not supported with sliding_window". Both read as a varlen problem and name nothing the caller can act on. Found while running the hipDNN kernel-integration runbook on gfx942. ## Technical details - `kernels/common/attention_dense_spec.py`: new `partial_tile_reason(spec)` names the length and the tile, for example "seqlen_q 384 is not a multiple of block_m 256". The sliding-window branch of `__post_init__` now says "sliding_window needs whole tiles: <reason> (the partial-tile path has no window mask)". - `kernels/gfx942/attention_dense.py`: `supports_attention_dense` says "gfx942 attention_dense needs whole tiles: <reason> (the partial-tile path is not yet supported)". - Varlen messages are unchanged. What could break: anything matching the old "ragged not yet supported" or "ragged is not supported with sliding_window" text. A repo grep for those two strings finds only the two lines this PR rewrites, and that was not enough. One test matched on less than the whole string: `tests/dispatch/attention/test_unified_mask_wiring.py` asserted the substring "ragged" in the gfx950 window refusal, and this branch broke it. It is now `test_partial_tile_window_is_declined_not_served` and checks the length, the tile and the window, the same way the gfx942 and gfx950 wiring tests do. Anything outside this repo that keys on the word "ragged" has the same exposure. ## Test plan and result Host only, no GPU, Windows Python 3.12, from `dnn-providers/hip-kernel-provider/rocke/library` with `library` and `platform/python` on PYTHONPATH. - `tests/dispatch/attention` plus `tests/test_attention_dense_gfx942.py` at head `1b17babac3`: 882 passed, 121 skipped, 3296 subtests passed. The merge base, develop at 28af48c, gives 882 passed, 121 skipped, 3293 subtests; the 3 extra subtests are the new 384 / 1152 / 1000 cases. - Negative control: with the two messages reverted, `test_rejects_a_partial_tile_and_names_the_length`, `test_sliding_window_with_partial_tile_rejected_at_dispatch` and the gfx942 `supports` marker all fail. - black 25.12.0 `--check` is clean on the changed files. Refs #12920. ## Submission checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests. Merged develop 3f51737 and updated the three rocke attention wiring tests to match the new refusal message.
cycheng
pushed a commit
that referenced
this pull request
Nov 6, 2026
…al tile (#14335) Replaces #13681 with the same commit (1b17bab). GitHub closed #13681 when its branch moved to `users/ysoliman/rocke/` for the rocKE branch-naming rule, and a PR cannot be reopened once its branch is renamed. The review history, including the rocKE checklist reviews, stays on #13681. Carved out of #12928, which is closed. The rest of that PR is already on develop (#12710 landed the nhead_k guard, #12719 replaced the factories this touched and routes `_request_errors` through every dense candidate). ## Motivation The dense `_base_spec` factories set `ragged=True` for any self-attention length that is not a tile multiple, so Sq = Skv = 384 or 1152 at block_m 256 is an ordinary graph taking the on-chip ragged path. The gfx942 predicate then declined with "gfx942 attention_dense: ragged not yet supported" and the shared spec validator with "ragged is not supported with sliding_window". Both read as a varlen problem and name nothing the caller can act on. Found while running the hipDNN kernel-integration runbook on gfx942. ## Technical details - `kernels/common/attention_dense_spec.py`: new `partial_tile_reason(spec)` names the length and the tile, for example "seqlen_q 384 is not a multiple of block_m 256". The sliding-window branch of `__post_init__` now says "sliding_window needs whole tiles: <reason> (the partial-tile path has no window mask)". - `kernels/gfx942/attention_dense.py`: `supports_attention_dense` says "gfx942 attention_dense needs whole tiles: <reason> (the partial-tile path is not yet supported)". - Varlen messages are unchanged. What could break: anything matching the old "ragged not yet supported" or "ragged is not supported with sliding_window" text. A repo grep for those two strings finds only the two lines this PR rewrites, and that was not enough. One test matched on less than the whole string: `tests/dispatch/attention/test_unified_mask_wiring.py` asserted the substring "ragged" in the gfx950 window refusal, and this branch broke it. It is now `test_partial_tile_window_is_declined_not_served` and checks the length, the tile and the window, the same way the gfx942 and gfx950 wiring tests do. Anything outside this repo that keys on the word "ragged" has the same exposure. ## Test plan and result Host only, no GPU, Windows Python 3.12, from `dnn-providers/hip-kernel-provider/rocke/library` with `library` and `platform/python` on PYTHONPATH. - `tests/dispatch/attention` plus `tests/test_attention_dense_gfx942.py` at head `1b17babac3`: 882 passed, 121 skipped, 3296 subtests passed. The merge base, develop at 28af48c, gives 882 passed, 121 skipped, 3293 subtests; the 3 extra subtests are the new 384 / 1152 / 1000 cases. - Negative control: with the two messages reverted, `test_rejects_a_partial_tile_and_names_the_length`, `test_sliding_window_with_partial_tile_rejected_at_dispatch` and the gfx942 `supports` marker all fail. - black 25.12.0 `--check` is clean on the changed files. Refs #12920. ## Submission checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests. Merged develop 3f51737 and updated the three rocke attention wiring tests to match the new refusal message.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Carved out of #12928, which is closed. The rest of that PR is already on develop (#12710 landed the nhead_k guard, #12719 replaced the factories this touched and routes
_request_errorsthrough every dense candidate).Motivation
The dense
_base_specfactories setragged=Truefor any self-attention length that is not a tile multiple, so Sq = Skv = 384 or 1152 at block_m 256 is an ordinary graph taking the on-chip ragged path. The gfx942 predicate then declined with "gfx942 attention_dense: ragged not yet supported" and the shared spec validator with "ragged is not supported with sliding_window". Both read as a varlen problem and name nothing the caller can act on. Found while running the hipDNN kernel-integration runbook on gfx942.Technical details
kernels/common/attention_dense_spec.py: newpartial_tile_reason(spec)names the length and the tile, for example "seqlen_q 384 is not a multiple of block_m 256". The sliding-window branch of__post_init__now says "sliding_window needs whole tiles: (the partial-tile path has no window mask)".kernels/gfx942/attention_dense.py:supports_attention_densesays "gfx942 attention_dense needs whole tiles: (the partial-tile path is not yet supported)".What could break: anything matching the old "ragged not yet supported" or "ragged is not supported with sliding_window" text. A repo grep for those two strings finds only the two lines this PR rewrites, and that was not enough. One test matched on less than the whole string:
tests/dispatch/attention/test_unified_mask_wiring.pyasserted the substring "ragged" in the gfx950 window refusal, and this branch broke it. It is nowtest_partial_tile_window_is_declined_not_servedand checks the length, the tile and the window, the same way the gfx942 and gfx950 wiring tests do. Anything outside this repo that keys on the word "ragged" has the same exposure.Test plan and result
Host only, no GPU, Windows Python 3.12, from
dnn-providers/hip-kernel-provider/rocke/librarywithlibraryandplatform/pythonon PYTHONPATH.tests/dispatch/attentionplustests/test_attention_dense_gfx942.pyat head1b17babac3: 882 passed, 121 skipped, 3296 subtests passed. The merge base, develop at 28af48c, gives 882 passed, 121 skipped, 3293 subtests; the 3 extra subtests are the new 384 / 1152 / 1000 cases.test_rejects_a_partial_tile_and_names_the_length,test_sliding_window_with_partial_tile_rejected_at_dispatchand the gfx942supportsmarker all fail.--checkis clean on the changed files.Refs #12920.
Submission checklist