fix(rocke): emit magic division's sum without signed-overflow poison by cgmillette · Pull Request #13802 · ROCm/rocm-libraries

cgmillette · 2026-10-16T04:51:41Z

Motivation

Magic-number division (do_magic_division / do_magic_division_dynamic and their C++ and hand-written copies) computes (umulhi(n, m) + n) >> s. The quotient is exact for every unsigned dividend below 2^31, but the sum was emitted as add nsw: for large dividends it passes the signed maximum while staying below 2^32, which is signed overflow, so LLVM may treat the result as poison inside the documented input range.

Fixing it needs a no-unsigned-wrap add in the IR builder. The same change adds the unsigned compare and the range marker that the upcoming bound-check code builds on.

ISSUE ID : AICK-2575

Technical Details

IR builder (both engines)

add_nuw(a, b): an arith.add lowered to add nuw. Plain add still emits add nsw, so no existing kernel changes.
cmp_ult(a, b): an unsigned less-than compare (icmp ult); the constant folder compares at operand width.
A private range marker (arith.assume_range): records a proven [lo, hi) fact on a value for the layer's range analysis; it emits no instruction.
add_nuw and cmp_ult take two i32 or two i64 operands, refused otherwise with the same message in both engines.
Both verifiers now reject an unknown wrap or pred.
New opcodes are appended at the end of rocke_opcode_t, so no existing opcode value changes.
HIP lowering: unsigned casts for both ops; the unsigned HIP type map has one home.

Magic division

Every copy emits its sum with add_nuw: the Python helper, its C++ mirror, the conv load phase, and the C++ dgrad instance's hand-written copy.
Magic-number division moves to its own module (pure move).

Test Plan

tests/core/test_unsigned_int_ops.py and the rocke_unsigned_int_ops ctest: lowering, folding, HIP text, verifier messages and builder refusals in both engines; an IR-validity compile; GPU values against numpy.
tests/core/test_magic_division_sum.py: the lowered sum is add nuw with no add nsw (static and runtime forms, all three LLVM flavors), both engines emit the same text, and GPU quotients match numpy for dividends up to 2^31 - 1.
Byte identity at llvm20, llvm22 and llvm23; the representative-IR goldens; the full platform suite; the library attention build tests.
Each commit builds and passes its own tests.

Test Result

Byte identity: green at llvm20, llvm22 and llvm23 for every kernel family.
Representative-IR goldens: re-blessed for the conv, conv_dgrad, conv_wgrad, conv_wgrad_two_stage, conv_direct, fused conv-pool and moe_sort cases. The only change is add nsw -> add nuw in the magic-division sum.
New tests pass in strict both-engines mode; GPU tests ran on gfx90a.
Full suite: no new failures against develop (the remaining failures also fail on develop and are environment-dependent).
pre-commit passes.

Submission Checklist

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

🤖 Generated with Claude Code

therock-pr-bot · 2026-10-16T04:57:49Z

✅ All Checks Passed — Ready for Review

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	✅ Pass	—
🔎 pre-commit	✅ Pass	—
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—
🤖 therock-pr-bot	✅ Pass	—

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

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

Copilot

🟡 Changes recommended

Malformed IR attributes can bypass native validation or crash Python verification, leaving inconsistent behavior between engines.

2 open findings

What changed in this PR

Fixes signed-overflow assumptions in rocKE magic division and adds unsigned integer operations to both IR engines.

Changes:

Adds add_nuw, cmp_ult, and a private range marker, with lowering and folding support.
Extracts magic-division helpers and updates callers to use unsigned sums.
Adds parity and GPU tests, updates test integration, and refreshes affected IR goldens.

Paths below are relative to dnn-providers/hip-kernel-provider/rocke/.

File	Description
platform/tests/tools/test_run_all.py	Tests native fixture discovery.
platform/tests/run_all.py	Registers unsigned-op parity fixture.
platform/tests/README.md	Documents fixture configuration.
platform/tests/portable_ir/test_recipe_roller.py	Updates helper import.
platform/tests/golden/rocke_representative_ir_sha256.json	Refreshes affected IR hashes.
platform/tests/core/test_unsigned_int_ops.py	Adds Python, parity, and GPU coverage.
platform/tests/core/test_unsigned_int_ops.cpp	Adds native unsigned-op coverage.
platform/tests/core/test_transforms_dynamic.py	Updates helper imports.
platform/tests/core/test_magic_division_sum.py	Tests unsigned sums and quotients.
platform/tests/CMakeLists.txt	Builds and registers native tests.
platform/python/rocke/portable_ir/utils/recipe_expand.py	Updates helper reference in docstring.
platform/python/rocke/portable_ir/tests/test_roll_nd.py	Updates helper imports.
platform/python/rocke/portable_ir/portable_ir_scaling_plan.md	Updates documented helper location.
platform/python/rocke/instances/common/moe_gemm_fused.py	Uses extracted helper module.
platform/python/rocke/helpers/transforms.py	Delegates magic division to new module.
platform/python/rocke/helpers/magic_division.py	Houses helpers using unsigned sums.
platform/python/rocke/core/verify.py	Adds attribute and marker validation.
platform/python/rocke/core/passes.py	Adds width-aware folding.
platform/python/rocke/core/lower_llvm.py	Lowers unsigned operations and markers.
platform/python/rocke/core/lower_hip.py	Emits unsigned arithmetic casts.
platform/python/rocke/core/ir.py	Adds builders and validation helpers.
platform/dsl_docs/reference/op_vocabulary.md	Documents new operations.
platform/dsl_docs/reference/file_index.md	Lists extracted helper module.
platform/dsl_docs/ir_lowering/ir_model.md	Documents unsigned builder methods.
platform/dsl_docs/architecture/portable_ir_schema.md	Updates helper reference.
platform/dsl_docs/architecture/portable_ir_production_readiness.md	Updates helper reference.
platform/cpp/portable_ir/recipe_vm.cpp	Updates helper reference in comment.
platform/cpp/instances/common/conv_implicit_gemm_dgrad.cpp	Uses unsigned magic-division sum.
platform/cpp/instances/common/conv_implicit_gemm_conv_load_phase.cpp	Uses unsigned magic-division sum.
platform/cpp/include/rocke/lower_hip_internal.h	Declares shared unsigned type mapping.
platform/cpp/include/rocke/ir.h	Declares builders and marker opcode.
platform/cpp/include/rocke/helper_rocke.helpers.transforms.h	Updates helper documentation.
platform/cpp/helpers/transforms.cpp	Uses unsigned sums in both helpers.
platform/cpp/core/verify.cpp	Adds native attribute validation.
platform/cpp/core/passes.cpp	Mirrors width-aware folding.
platform/cpp/core/lower_llvm/core.cpp	Resolves marker operands directly.
platform/cpp/core/lower_llvm/arith.cpp	Lowers unsigned operations and markers.
platform/cpp/core/lower_hip/lower_hip_cast.cpp	Uses shared unsigned type mapping.
platform/cpp/core/lower_hip/lower_hip_arith.cpp	Emits unsigned operations and marker copies.
platform/cpp/core/lower_hip/core.cpp	Centralizes unsigned type mapping.
platform/cpp/core/ir/ir_arith.cpp	Implements native unsigned builders.
platform/cpp/core/ir/core_types.cpp	Registers marker name and purity.
platform/CMakeLists.txt	Configures installed parity fixture.
library/tests/test_attention_dense_gfx942.py	Updates helper import and documentation.
library/tests/test_attention_builds.py	Updates helper import.
library/tests/dispatch/attention/test_gfx942_dense_wiring.py	Updates helper import.
library/kernels/gfx942/attention_dense.py	Uses extracted constants helper.
library/kernels/common/sparse_attention.py	Updates helper imports.
library/kernels/common/sage_attention.py	Updates helper imports.
library/kernels/common/fmha_splitkv_decode.py	Updates helper imports.
library/kernels/common/conv_implicit_gemm_wgrad.py	Updates constants helper import.
library/kernels/common/conv_implicit_gemm_dgrad.py	Updates helper imports.
library/kernels/common/conv_args.py	Updates constants helper import.
library/kernels/common/attention_unified.py	Updates helper imports and documentation.
library/kernels/common/_conv_implicit_gemm_common.py	Updates dynamic-division import.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

therock-pr-bot · 2026-10-16T05:13:03Z

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

AviralGoelAMD · 2026-10-16T05:49:20Z

New tests pass in strict both-engines mode; GPU tests ran on gfx90a.

Thanks for the fix Chris, One thing that stands out to me: this change touches a lot of kernels and updates many IR SHAs, but we are not doing a GPU validation run of the kernels. Since you understand this instruction better please just make the call if we need to do it once or not.

AviralGoelAMD

LGTM! Just left one comment to watch out for.

cgmillette

PR #13802 — emit magic division's sum without signed-overflow poison (cgmillette, AICK-2575) — 🟠 REJECT: rebase, re-bless, attr parity

Prior item	Status
Copilot #1: C++ accepts non-string `wrap` / `pred` (→ `nsw` / `lt`)	🟠 OPEN — valid: `rocke_attr_get_str` returns NULL for non-strings (code-read); Python rejects `wrap=1` / `pred=1` (ran)
Copilot #2: range-marker attrs not type-checked	🟠 OPEN — valid (ran): `multiple_of="4"` crashes the verifier (TypeError); `2.5` / `True` / `hi=10.5` pass
AviralGoelAMD (approved): GPU-validate the re-blessed kernels?	🟡 OPEN — no reply; IR diff is the flag only (ran); in-range values unchanged; an ISA diff of one conv kernel would settle it
Bots / CI	🟠 Conflicts with develop (`tests/CMakeLists.txt`) → pre-commit, superbuild and TheRock never ran
Our checks	🟢 Head golden ×3 flavors clean; new tests 24 pass / 16 skip (no GPU or C++ engine); magic-division move is pure

#	Sev	file:line	Finding
N1	🟠	`platform/tests/golden/rocke_representative_ir_sha256.json`	Develop's new `deep/gfx12_generic/h16w16_k32` (6 magic-division sums) drifts ×3 flavors after merge → `test_ir_cases_match_golden_sha256` fails (ran)
N2	🟡	`platform/tests/CMakeLists.txt:159`	New native `rocke_unsigned_int_ops` is not in the tier YAML → its C++ verifier / folder checks never run in TheRock

Utility: 🟢 Positive — removes signed-overflow poison from magic division; ops mirrored in both engines.

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

CongMa13

Reviewed against AICK-2575. This review found no new findings; the existing feedback on malformed IR attribute validation and golden/test registration already records the actionable observations. No duplicate inline comments are added.

tenpercent

let's get the should-fixes before a human review pass

cgmillette · 2026-10-16T17:38:40Z

Self-review N1 -- golden drift on develop's new gfx12_generic case

Re-blessed after rebasing on develop. The only change in deep/gfx12_generic/h16w16_k32 is its 6 magic-division
sums going from add nsw to add nuw: replacing them back reproduces develop's sha at llvm20, llvm22 and llvm23.

Self-review N2 -- new ctest missing from the tier YAML

Added rocke_unsigned_int_ops to both tiers of ROCKE_ENGINE_test_categories_external.yaml.

cgmillette · 2026-10-16T17:39:01Z

New tests pass in strict both-engines mode; GPU tests ran on gfx90a.

Thanks for the fix Chris, One thing that stands out to me: this change touches a lot of kernels and updates many IR SHAs, but we are not doing a GPU validation run of the kernels. Since you understand this instruction better please just make the call if we need to do it once or not.

Thanks -- I ran it once on gfx90a. The only IR change in the re-blessed kernels is the flag on the
magic-division sum (add nsw -> add nuw; putting nsw back reproduces every previous golden sha), and the
magic-division GPU test checks quotients against numpy for dividends up to 2^31 - 1. On gfx90a: the magic-division
and unsigned-op GPU tests pass; the library conv correctness tests give 43 passed, 116 skipped (configs that need
gfx942 or newer) and 1 failure, fp32 dgrad stride 1, which fails identically on develop ("No valid dgrad
configurations found" for fp32 on gfx90a), so it is not from this change.

Copilot

🟡 Changes recommended

Incorrect constant folding and incomplete unsigned-operand validation remain unresolved in both engines.

2 open findings

2 resolved since last review

🧠 Review effort: Balanced

CongMa13

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

Reject malformed non-integer range marker attributes — copilot-pull-request-reviewer, #13802 (comment) (thread PRRT_kwDOOej8qc6tEfsO)
Reject non-string wrap and predicate attributes — copilot-pull-request-reviewer, #13802 (comment) (thread PRRT_kwDOOej8qc6tEfr4)
Restrict unsigned operations to supported operand widths — copilot-pull-request-reviewer, #13802 (comment) (thread PRRT_kwDOOej8qc6tUh9t)
Read folded constants at their declared width — copilot-pull-request-reviewer, #13802 (comment) (thread PRRT_kwDOOej8qc6tUh-4)
Re-bless the representative golden after rebasing — cgmillette, #13802 (comment)
Register the new native test in both test tiers — cgmillette, #13802 (comment)
GPU validation for the changed kernel families — AviralGoelAMD, #13802 (comment) (author response: #13802 (comment))

CongMa13

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

tenpercent

One confirmed constant-folding regression when IR optimization is enabled; details inline.

fixed

…nge marker to the IR builder Adds three integer IR pieces to both engines: - cmp_ult(a, b): arith.cmp with pred "ult", lowered to icmp ult. The constant folder compares the operands as unsigned at their width, and the HIP lowering casts both operands to uint32_t / uint64_t. - add_nuw(a, b): arith.add with wrap "nuw", lowered to add nuw. Plain add keeps nsw, so no existing IR changes. The HIP lowering adds as unsigned and casts back, since a signed C++ sum past the signed maximum is undefined behaviour that clang emits as nsw. - IRBuilder._assume_range: a private marker op, arith.assume_range, that returns its operand and records a checked range and congruence in its attrs for the tiling layer's range walk. It emits no instruction: both LLVM lowerings print the marked value at every use, the HIP lowerings emit a copy, and the folder folds it on a constant. Both constant folders wrap every folded integer result to its width as two's complement, read every integer constant at its width the same way (so the literal 0x80000000 and a folded i32 sum with those bits are one value), and fold zext as the operand read unsigned at its own width, so a folded add_nuw past the signed maximum reaches later signed folds and widenings as the value the register holds. Both builders refuse operands other than two i32 or two i64 values. Both verifiers check the new attrs with identical texts: the add wrap and the compare predicate (each a known string; nuw and ult, the forms of add_nuw and cmp_ult, only on i32 or i64 operands), and the marker's attrs (each an integer), multiple_of, residue, the choice of an int or a runtime upper end, and lo below hi. Both lowerings of every engine refuse a wrap that is not a known string rather than reading it as nsw, and a nuw add on other operand types. The new opcode is appended to rocke_opcode_t. The C API gains rocke_b_add_nuw and rocke_b_cmp_ult; the unsigned HIP type spelling moves to the shared HIP lowering core so the cast and arith handlers use one table. Tests: tests/core/test_unsigned_int_ops.py (IR text, engine agreement, C API against the Python builder, verifier and builder refusals, folding, HIP text and a hipcc compile without nsw, GPU values against numpy) and the rocke_unsigned_int_ops ctest. The C API comparison gets the built program through ROCKE_UNSIGNED_INT_OPS_CLI, which tests/run_all.py resolves from CTest and the installed rocke_pytest ctest sets. The op vocabulary and IR model docs list the new ops. Co-Authored-By: Claude <noreply@anthropic.com>

Magic division computes (umul_hi(n, multiplier) + n) >> shift. For a dividend below 2^31 the sum can pass the signed i32 maximum while staying below 2^32 (from 1,610,612,736 for divisor 3), and it was emitted as add nsw, so LLVM was free to treat the quotient as poison inside the documented range. The sum is now a no-unsigned-wrap add (add nuw) everywhere it is built: do_magic_division and do_magic_division_dynamic in helpers/transforms.py, their C++ mirrors in cpp/helpers/transforms.cpp, and the two hand-written C++ copies in the conv forward load phase and the conv dgrad kernel. The C++ helper header comment is updated to match. The LLVM IR of the conv, conv dgrad, conv wgrad, direct conv, fused conv-pool and MoE sorting families changes accordingly; the representative IR golden is re-blessed for all three LLVM flavors. Tests: tests/core/test_magic_division_sum.py checks that the lowered sum is add nuw with no add nsw, that both engines emit the same text, and, on a GPU, the static and runtime-multiplier quotients of dividends 2^30 - 1, 2^30, 1,610,612,736 and 2^31 - 1 by divisors 3, 9, 1025 and 64 against numpy. Co-Authored-By: Claude <noreply@anthropic.com>

calculate_magic_numbers, do_magic_division and do_magic_division_dynamic move verbatim from helpers/transforms.py to the new helpers/magic_division.py. Magic-number division is a general integer primitive, not part of the coordinate-transform DAG; the DAG's magic unmerge transforms now call it through the new module. Every importer imports from rocke.helpers.magic_division: the conv, conv dgrad, conv wgrad, conv argument, unified attention, dense attention, split-KV decode, sage and sparse attention kernels, the fused MoE GEMM instance, and their tests. helpers/transforms.py no longer exports or re-exports the three names, so an old import fails instead of silently resolving. Comments and docs that named the old location now name the new one, including the C++ helper header and source, the recipe VM, the portable IR docs and the file index. No emitted IR changes. Co-Authored-By: Claude <noreply@anthropic.com>

arith.div and arith.mod lower to sdiv and srem, which round the quotient toward zero, but the constant folder used Python's floored modulo (mirrored in C++) and a float division. A folded mod of a negative dividend or divisor, such as 3 mod 0x80000000 read at i32, disagreed with the unfolded kernel, and large i64 quotients lost precision. Both engines now fold with integer division rounded toward zero, and leave a zero divisor or the minimum value divided by -1 unfolded, as LLVM leaves them undefined. Tests in both engines cover high-bit and negative divisors, exact i64 quotients, and the unfolded cases. Co-Authored-By: Claude <noreply@anthropic.com>

The differential golden had gone stale: 627 of its 1240 LLVM IR entries and 537 of its 1206 canonical IR entries no longer matched the tree, because nothing ran the check. Recorded afresh in the new format, with the Python reference emitters, from a tree whose two-engine gate is green for all 82 families: - ll at llvm20, llvm22 and llvm23: 82 families, 1518 configs per flavor; 1615 at llvm23, where gfx1250_lowering has configs that need LLVM 23 and are refused below it. - ir_canonical: 81 families, 1598 configs (one family's emitter has no IR mode). Canonical IR was checked to be identical at llvm20 and llvm23. What the re-bless absorbs: every mismatch that could be traced is an intended code generation change that did not re-bless this file -- the shared LDS pool (#8844, about 425 configs), the ds.swizzle declaration (#9583, about 177), renamed values in the attention transpose reads (#9133, 57), the smaller LDS pool (#10112, about 39), the flat work group size of one-wave wave32 kernels (#12710, about 39), #9220 (6) and the conv changes of #13109 (6). 441 entries were already stale at the start of the history available here and cannot be attributed; no unexplained change was found after it. The two repeated keys (conv_direct_grouped, conv_wgrad_workspace_reduce), which hid their newer copies, are gone. It also takes in the code generation changes merged since then, each traced by recording the file at the commit and at its parent: - the magic division sum emitted without signed-overflow poison (#13802): 215 configs in each of the four sections, in 19 families -- attention_unified 6, attention_unified_stress 32, conv_direct_grouped 16, conv_implicit_gemm 26, conv_implicit_gemm_dgrad 31, conv_implicit_gemm_wgrad 42, deep_fused_conv_pool 2, gfx1201_deep_fused_conv_pool 5, gfx950_deep_fused_conv_pool 5, dynamic_helpers 2, fmha_splitkv_decode 6, img2col 5, moe_fused_mega 5, moe_gemm_fused 6, moe_sorting 6, pooling 4, sage_attention 4, sparse_attention 6, transpose 6; - attention decoding on gfx942 and gfx950 (#13492): 23 changed and 25 new configs per section in attention_strided_kv, gfx942_attention_tiled_3d and gfx950_attention_tiled_3d; - wave_all on wave32 (#13896): 18 gfx1151_wmma_fmha_fwd configs per LLVM flavor, canonical IR unchanged; - new configs: the barrier_fence family (#13575, 10) and one conv_implicit_gemm_dgrad config (#13782). Co-Authored-By: Claude <noreply@anthropic.com>

…nd checks and split addresses (#13876) ## Motivation Kernels need one way to say "these loop coordinates become these tensor coordinates": implicit-GEMM convolution, halos, K splitting and swizzles. They also need bound checks emitted only where a coordinate can actually leave its tensor, and addresses split so the per-wave part is computed once. Today each kernel does this by hand, and the bound checks are either missing or emitted everywhere. This PR adds `helpers/tiling/tensor_transforms`, a coordinate-graph package for any workload (copy, prefetch, reduction, attention, convolution, GEMM). It is new files only; no existing kernel or API changes. Stacked on #13802 (it uses that PR's `cmp_ult`, `add_nuw` and range marker); it will be retargeted to `develop` when #13802 merges. ISSUE ID : AICK-2578 ## Technical Details **The graph** (`graph.py`) - `make_transform_graph(inputs(...), steps..., outputs(...))` with eight kinds: `embed`, `merge` (N -> 1), `unmerge` (1 -> N), `offset`, `slice_window`, `freeze`, `xor`, `broadcast`. - A graph holds no tensor and no bound; a tensor's lengths live only on its `TensorDesc`. A halo is an `offset` plus the tensor's own length, not a pad kind. - Positional: `evaluate` / `resolve` take a tuple in `inputs(...)` order; graph input i is window coordinate i. Strings name only the coordinates the author declares. **Ranges and required checks** (`ranges.py`) - `Interval`, `Congruence` and their range arithmetic; `RuntimeValue`, a runtime integer carrying its stated range and multiple-of fact. - `value_ranges`, `required_checks` (which side of which coordinate can leave its domain or tensor bound, with the reason) and `graph_injective`. **The IR range walk** (`walk.py`) - `value_facts` derives a value's range and residue from how it was computed (constants, thread / block ids, loop counters, range markers, add / sub / mul / div / mod / shifts / masks / min / max); anything else is unknown and partial facts survive. - `split_coordinate` splits a plain i32 value into a constant, wave-uniform parts and one per-lane part. - It emits nothing. **Emission** (`emit.py`, `runs.py`, `address.py`) - `emit_accesses` returns, per access, the element address split into wave-uniform terms, one per-lane part and a constant, and its validity against the tensor's own lengths and the graph's domains, all over one shared table. - `emit_bound_check`: the one predicate, `lo <= value < hi`, folded so a side the walk proves emits nothing and the rest is one unsigned compare; widths are chosen so no subtraction can wrap. - Vector accesses keep one validity when every check has one answer for the whole vector, otherwise the first and last element's checks plus the span of valid elements. - `address_of` / `transform_address_of`: the pure-Python address every emitted address is tested against. **Docs** - `tensor_transforms_api.md` (the package's facts, every error message verbatim) and `tensor_transforms_guide.md` (runnable examples: halo, K split, swizzle, addresses, required checks, and CK's pad transform expressed here). - Pointers from the tiling API docs; the tiling skill and expert files gain the coordinate-graph step. ## Test Plan - Each kind against numpy over its whole domain; a convolution graph against a numpy im2col index. - Every range operation against exhaustive enumeration; required checks and injectivity on worked cases. - Every walk rule, including soundness against i32 evaluation and the loop-counter cases. - Emitted IR evaluated on the host for every lane against `transform_address_of`, failing on any overflowing op; golden IR text; compiled for gfx90a, gfx942 and gfx950. - On gfx90a: per-lane address parts, bound-check folds against every value they admit, and validity of convolution, halo and ragged vector accesses. - Every error message in its test. ## Test Result - 348 new tests pass in strict both-engines mode; GPU tests ran on gfx90a. - Byte identity green at llvm20, llvm22 and llvm23; ctest passes; the representative-IR golden is unchanged. - Full platform suite: no new failures against the base (the remaining failures also fail on the base and are environment-dependent). - Each commit builds and passes its own tests. - pre-commit passes. ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com>

Four of the new gfx1250 conv golden cases (g4_cshuffle, dw_gm4_direct, dw_gm8_cshuffle, dw_unmerged) drifted at llvm20, llvm22 and llvm23 after the develop merge 24e0c66. The drift comes from develop's #13802 (magic division's sum is now `add nuw` instead of `add nsw`), which could not re-bless cases that exist only on this branch. 8eded88 itself leaves the golden green. The dumped IR before and after the merge differs only in that flag on the umulhi sums, at every flavor. The summary counters pick up the cases the develop merges added. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

assistant-librarian · 2026-10-22T19:21:30Z

TheRock Submodule Bump Activity

Newest first

2026-10-22 — Included in TheRock via ROCm/TheRock#9426.

## Summary Bring depthwise and grouped forward convolution on gfx1250 to the gfx950 model. On gfx950, depthwise forward is served from the AOT kernel caches (group-merged implicit GEMM and the direct depthwise kernels) and from the grouped-conv dispatcher's registered instance list. On gfx1250 none of this was reachable: forward group merging was gated to wave64, the direct-conv cache built mismatched or no depthwise kernels, and the dispatcher had no gfx1250 list, so it refused every gfx1250 forward request (dense, grouped and depthwise). ISSUE ID : AICK-2317, AICK-2319 ## Changes **Merged-groups forward on WMMA (Python and C++ engines, byte-identical)** - Lift the wave64-only gate on forward `group_merge`. The merge is in the A/B addressing, the diagonal mask on the B load and the epilogue's N bound, none of which depend on the MMA family. wgrad group merging stays wave64-only. - C++ fix: the `wavelet` load waves fetched B through the unmerged descriptor, so a merged wavelet kernel would have accumulated the masked-off weights. They now take the merged descriptor, as the sync path and the Python engine do. **Direct depthwise kernels on wave32** - The direct-conv AOT cache (`direct_kernel_sweep.py`) and the JIT sweep (`benchmark_direct_conv.py`) build the depthwise forward variants at the target's wave size. Previously the gfx1250 cache recorded `wave_size=32` in each kernel identity while building the binary at the wave64 spec default, and rejected the tiled and column-streamed variants outright. - Builds record the wave size each wave-parametric binary was built at (`spec_wave_size`), and the run side refuses an entry whose recorded size differs from its identity's, so an older gfx1250 cache holding wave64 binaries is rejected with a rebuild hint instead of launched with wave32 geometry. An explicit `wave_size` knob that conflicts with the target's is rejected rather than overwritten. - The preloading and spatial kernels use no cross-lane operation, so a wave64 spec remains valid on a wave32 target (a logical 64-thread wave). This is documented and pinned by a parity config and a GPU test rather than restricted. **Dispatch: gfx1250 forward instance list** - Register `dispatch/grouped_convolution_instances/gfx1250_fwd.json`: 33 configs, each listed for fp16 and bf16 (66 instances), all wave32 at `llvm22` like the other lists. gfx1250 dgrad stays unregistered and is refused with a reason. - Selected on gfx1250 from the forward AOT grid (17,804 implicit-GEMM and direct depthwise instances; no merged groups, no `wavelet`), timed with `benchmark_grouped_conv.py` over the 84 2-D fp16/bf16 forward cases of `bench_cases_conv.json`: - Coverage: greedy set cover, so every case's admitted set holds an instance within 95% of the grid's best for it. - Order: configs sorted by the geomean of their share of each admitting case's best, so the direct depthwise kernels come ahead of generic tiles; the most generic tile per vector width goes last, so odd channel counts (stem `C=3`, `cpg=4`) still dispatch. - The dispatcher already forwards the identity's wave size into the direct specs, so dispatched depthwise kernels launch wave32 blocks. **Tooling fix:** `benchmark_grouped_conv.py --compile-all` (and its build-then-run mode) imported `build_kernel` / `build_direct_job`, which do not exist, so it failed at import on develop. It now uses `kernel_for_job` / `direct_kernel_for_job`, the builders the sweeps use. **Docs:** `SUPPORT_MATRIX.md`, optimization runbook knob catalog, kernel/sweep comments, the dispatcher's module docstring. ## Correctness evidence - GPU numerics on gfx1250 against the existing references: merged forward at Gm 2-64 on `mem` and `wavelet`, both epilogues, fp16 and bf16; the direct depthwise preloading, spatial, tiled and column-streamed sweeps at wave32, and preloading/spatial at a logical wave64. - Every instance of the gfx1250 list, run through `launch_plan_for` and verified against the torch reference on gfx1250 (`benchmark_grouped_conv.py --verify`) in the 82 forward cases the reference can hold; the other 2 exceed its 2**31-byte limit and were timed only. One run of one case (`N1H56W56C64_K64Y3X3` bf16) failed all 24 admitted instances with an identical `rel_err`; it passed in 12 reruns, so it has not been reproduced. - gfx11 WMMA targets (gfx1151/gfx1201) are covered by the parity gate only; no hardware was available. ## Performance (gfx1250, gfx1250 list vs the full 17,804-instance forward grid) | | geomean of grid best | min | cases ≥ 95% | cases ≥ 90% | |---|---|---|---|---| | Best instance in the list (what a tuner sweeping the list finds) | 95.4% | 82.6% | 52 / 84 | 70 / 84 | | Untuned default pick (first admitted instance) | 79.6% | 42.6% | — | — | Re-timed separately from the grid run; the grid's best is the maximum over thousands of noisy timings, so these ratios are conservative. On develop every one of these requests is refused. ## Known limitations / follow-ups - wgrad group merging remains wave64-only (needs the two-stage split-K path on WMMA). - The dispatcher does not select a forward merge degree on any arch; merged kernels are reached through the AOT cache, as on gfx950. - gfx1250 dgrad has no instance list. - The untuned default pick is a single priority order; large dense 3x3 shapes default to a mid-size tile (as low as ~43% of best) until a tuner sweeps the list. - The gfx1250-only 512 tile / 16 warp configurations are still not in the implicit-GEMM AOT grid. - The depthwise dgrad streaming kernel has the same identity/spec wave-size mismatch in the backward direct cache; out of scope for forward. ## Test plan - [x] `check_byte_identity.py` (conv families), llvm20 / llvm22 / llvm23: GREEN; new configs `conv_implicit_gemm` 29-35 and `conv_direct_grouped` 95-99 IDENTICAL - [x] Golden IR: 8 new gfx1250 cases at llvm20/22/23; 4 of them re-blessed after the develop merge for #13802 (magic division's sum is now `add nuw`; the dumped IR differs only in that flag); `rocke_ir_parity_harness.py --check` passes at every flavor - [x] `tests/test_conv_fwd_correctness.py`, `tests/test_direct_conv_correctness.py` on gfx1250 - [x] `tests/dispatch/test_grouped_conv_dispatch.py`: gfx1250 forward runs every per-list check (admission and launch plans match the AOT run path, every instance has its compile-all job); every registered list must run every test shape; grouped and depthwise forward on gfx1250 dispatch wave32 instances - [x] `tests/test_direct_kernel_cache.py`, now in the `rocke_library_conv_pytest` CTest list (it was in none). The develop failure `test_dgrad_rtc_serves_any_channels_its_tiles_fit[1-48-192]` is fixed: the test now accepts the 384-VGPR preload budget the enumerator applies. Its cache fixture is module-scoped (~140 s → a few seconds) - [x] `tests/test_conv_fwd_group_merge_gate.py`, `test_library_layering.py`, `test_builder_signature_contract.py`, `test_kernel_label.py` - [x] GPU: `benchmark_grouped_conv.py --verify` over `bench_cases_conv.json` on gfx1250 with the registered list - [x] `pre-commit` on changed files 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

The differential golden had gone stale, because nothing ran the check: 675 of its 1116 LLVM IR entries (compared at llvm20) and 608 of its 1089 canonical IR entries no longer match the tree. Recorded afresh in the new format, with the Python reference emitters, from a tree whose two-engine gate is green at llvm20, llvm22 and llvm23: - ll at llvm20 and llvm22: 82 families, 1474 configs per flavor. At llvm23: 83 families, 1578 configs, where gfx1250_lowering has configs that need LLVM 23 and are refused below it, and every conv_direct_dgrad_wmma config needs LLVM 23. - ir_canonical: 82 families, 1561 configs (conv_abi's emitter has no IR mode). Canonical IR was checked to be identical at llvm20 and llvm23. What the re-bless absorbs: every mismatch that could be traced is an intended code generation change that did not re-bless this file -- the shared LDS pool (#8844, about 425 configs), the ds.swizzle declaration (#9583, about 177), renamed values in the attention transpose reads (#9133, 57), the smaller LDS pool (#10112, about 39), the flat work group size of one-wave wave32 kernels (#12710, about 39), #9220 (6) and the conv changes of #13109 (6). 441 entries were already stale at the start of the history available here and cannot be attributed; no unexplained change was found after it. The two repeated keys (conv_direct_grouped, conv_wgrad_workspace_reduce), which hid their newer copies, are gone. It also takes in the code generation changes merged since then, each traced by recording the file at the commit and at its parent; the count is per section unless a flavor is named: - the magic division sum emitted without signed-overflow poison (#13802): 215 configs in 19 families -- attention_unified 6, attention_unified_stress 32, conv_direct_grouped 16, conv_implicit_gemm 26, conv_implicit_gemm_dgrad 31, conv_implicit_gemm_wgrad 42, deep_fused_conv_pool 2, gfx1201_deep_fused_conv_pool 5, gfx950_deep_fused_conv_pool 5, dynamic_helpers 2, fmha_splitkv_decode 6, img2col 5, moe_fused_mega 5, moe_gemm_fused 6, moe_sorting 6, pooling 4, sage_attention 4, sparse_attention 6, transpose 6; - attention decoding on gfx942 and gfx950 (#13492): 23 changed and 25 new configs in attention_strided_kv, gfx942_attention_tiled_3d and gfx950_attention_tiled_3d; - the f64 follow-ups (#13887): 3 changed and 1 new elementwise config, 5 changed reduce configs; - MX scale and fp6 decoding (#13885): 6 mx_gemm configs; - the gfx950 conv dgrad speed-up (#13075): 36 new conv_direct_dgrad_ext configs; 22 changed, 15 new and 2 removed conv_implicit_gemm_dgrad configs; 2 changed and 3 new conv_direct_grouped configs; 2 new dynamic_helpers and 3 new target_intrinsics configs; - removing the ds.read.tr16.b128 declaration (#14054): gfx1250_lowering, 100 changed and 2 removed configs at llvm23 and in canonical IR, 5 changed, 2 new and 4 removed at llvm20 and llvm22; - optional LSE output on unified split-KV attention (#13955): 18 gfx942_attention_tiled_3d and 16 gfx950_attention_tiled_3d configs per LLVM flavor, canonical IR unchanged; - the running softmax max in 3D split-KV (#14276): 14 attention_strided_kv, 18 gfx942_attention_tiled_3d and 16 gfx950_attention_tiled_3d configs; - depthwise 7x7 and grouped wgrad on batched MFMA (#14157): 7 changed, 10 new and 1 removed conv_direct_grouped configs; - the retirement of wmma_fmha_fwd (#13897): the gfx1151_wmma_fmha_fwd (157 configs) and gfx1201_wmma_fmha_fwd (12) families are gone; - new configs, every existing hash unchanged: the barrier_fence family (#13575, 10), one conv_implicit_gemm_dgrad config (#13782), 14 conv_direct_grouped configs for the split-C and grouped LDS-halo kernels (#13993), one config each in fmha_splitkv_decode, sage_attention and sparse_attention (#13876), 23 conv_direct_grouped configs for the direct wgrad kernels (#13990), 2 conv_implicit_gemm_dgrad configs for patch-embedding dgrad (#13988), 6 f64_mma configs (#13794), 5 conv_direct_grouped and 7 conv_implicit_gemm configs for depthwise and grouped conv forward (#13987), and the conv_direct_dgrad_wmma family (7 configs, llvm23 and canonical IR only, #14156). #13885, #13955 and #14276 updated the old file's entries for their changes; the new llvm20 and canonical IR sections reproduce every one of those hashes. Co-Authored-By: Claude <noreply@anthropic.com>

…13974) ## Motivation The differential golden (`tests/instances/differential/golden/llvm_gfx_all.json`) is the only tripwire for an output change in about 55 parity families that the representative golden does not cover (norms, reduce, transpose, fmha, wmma GEMMs, flatmm, mx_gemm, ...). The byte-identity gate cannot see a change made the same way in both engines; this file can. But nothing ran its check, and on develop 627 of its 1240 LLVM IR entries and 537 of its 1206 canonical IR entries no longer matched. The checker itself made it easy to go stale: - Entries carried no LLVM flavor, so blesses from hosts of different flavors mixed and no flavor matched the file. - A re-bless limited with `--only` replaced the whole section; one did, and the pasted-back copy left two repeated JSON keys whose older copies silently won. - A check limited with `--only` reported every other family missing. - The check needed the C++ build although it compares Python reference hashes only. ISSUE ID : AICK-2654 ## Technical Details Three commits. 1. **The checker** (`fix`) - New `golden_store.py` owns the file (schema 2): LLVM IR hashes in one section per flavor (`llvm20`, `llvm22`, `llvm23`), canonical IR in one section (produced before lowering; identical across flavors, checked). Loading refuses repeated keys and the old format. A filtered record replaces only the families it ran; a filtered check compares only those. - `run_diff.py` pins the flavor for every golden run (`--flavor`, else `ROCKE_LLVM_FLAVOR`, else the host's) and exports it to both emitters. - `--python-only` records or checks from the Python reference emitters alone: no C++ engine, any flavor on any host, a list of flavors in one run. It produces the same hashes as the two-engine run (all 82 families, checked). - The canonical hash has one definition (`python_batch_worker.canonical_digest`), shared by the batch worker, which now computes it in parallel, and `run_diff.py`. - `run_diff.py` finds the rocke and library packages in an installed test tree too. 2. **The re-bless** (`test`, data only): recorded afresh at llvm20, llvm22, llvm23 and canonical IR from a tree whose two-engine gate is green for all 82 families (re-recorded after rebasing onto develop with #13802, whose magic-division change moved 215 configs in 19 families in every section). Every mismatch that could be traced is an intended code generation change that did not re-bless this file (#8844, #9583, #9133, #10112, #12710, #9220, #13109); 441 entries were already stale at the start of the history available and cannot be attributed; no unexplained change was found after it. The commit message has the breakdown. 3. **CI** (`test`): two quick-tier CTest entries run the Python-only check from the installed test tree, `rocke_differential_golden_ll` (all three flavors) and `rocke_differential_golden_canonical`. The install ships only what that check needs from `tests/instances/differential`; the rest still needs the C++ build and stays out. The golden README and the troubleshooting entry describe the check and re-bless commands. No emitter or IR changes. ## Test Plan - `test_golden_store.py`: repeated keys refused by name; the old format refused with the way out; flavor sections separate; a full record replaces, a filtered record merges; a filtered check ignores other families; new configs alone pass. - The existing differential tests (gate verdict, scaled MFMA batch) still pass. - Python-only vs two-engine record at llvm20: identical hashes for all 82 families; canonical IR identical at llvm20 and llvm23. - The check passes on the re-blessed tree and fails (1058 changed configs) when the lowerer's output is altered. - Installed tree (provider layout, staged install): both CTest entries pass, and fail when the installed lowerer's output is altered. - Byte identity at llvm20, llvm22, llvm23; ctest; full platform suite. ## Test Result - All of the above pass. - Installed CTest time: about 15 s (LLVM IR, three flavors) and 13 s (canonical IR). - Byte identity green at llvm20, llvm22 and llvm23; ctest passes; full platform suite passes with no failures; pre-commit passes. ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com>

Copilot AI balanced review requested due to automatic review settings October 16, 2026 04:51

cgmillette requested review from a team as code owners October 16, 2026 04:51

github-actions Bot added documentation project: hip-kernel-provider rocke-lib: attention Touches rocKE library attention/FMHA kernels rocke: platform Touches rocKE platform labels Oct 16, 2026

Copilot started reviewing on behalf of cgmillette October 16, 2026 04:52

View session

cgmillette changed the title ~~Users/cgmillette/rocke/transforms core ops~~ fix(rocke): emit magic division's sum without signed-overflow poison Oct 16, 2026

Copilot AI reviewed Oct 16, 2026

View reviewed changes

Comment thread dnn-providers/hip-kernel-provider/rocke/platform/cpp/core/verify.cpp Resolved

Comment thread dnn-providers/hip-kernel-provider/rocke/platform/python/rocke/core/ir.py Resolved

assistant-librarian Bot added the organization: ROCm label Oct 16, 2026

AviralGoelAMD approved these changes Oct 16, 2026

View reviewed changes

cgmillette commented Oct 16, 2026

View reviewed changes

CongMa13 reviewed Oct 16, 2026

View reviewed changes

tenpercent previously requested changes Oct 16, 2026

View reviewed changes

cgmillette force-pushed the users/cgmillette/rocke/transforms-core-ops branch from 1b60c20 to 2a8800b Compare October 16, 2026 17:32

cgmillette requested a review from tenpercent October 16, 2026 17:39

tenpercent requested a balanced review from Copilot October 16, 2026 17:54

Copilot started reviewing on behalf of tenpercent October 16, 2026 17:55

View session

Copilot AI reviewed Oct 16, 2026

View reviewed changes

Comment thread dnn-providers/hip-kernel-provider/rocke/platform/python/rocke/core/verify.py Resolved

Comment thread dnn-providers/hip-kernel-provider/rocke/platform/python/rocke/core/passes.py Resolved

cgmillette force-pushed the users/cgmillette/rocke/transforms-core-ops branch from 005874e to ef27749 Compare October 16, 2026 21:17

cgmillette requested a review from a team as a code owner October 16, 2026 21:17

cgmillette mentioned this pull request Oct 16, 2026

feat(rocke): add tensor_transforms -- coordinate graphs, required bound checks and split addresses #13876

Merged

1 task

CongMa13 reviewed Oct 16, 2026

View reviewed changes

cgmillette enabled auto-merge (squash) October 16, 2026 22:28

CongMa13 approved these changes Oct 16, 2026

View reviewed changes

tenpercent reviewed Oct 17, 2026

View reviewed changes

Comment thread dnn-providers/hip-kernel-provider/rocke/platform/python/rocke/core/passes.py Resolved

cgmillette force-pushed the users/cgmillette/rocke/transforms-core-ops branch 3 times, most recently from 2add300 to 4376a74 Compare October 20, 2026 18:02

cgmillette and others added 4 commits October 20, 2026 14:11

cgmillette force-pushed the users/cgmillette/rocke/transforms-core-ops branch from 4376a74 to 72dd160 Compare October 20, 2026 19:44

cgmillette merged commit e6157a0 into develop Oct 20, 2026
100 checks passed

cgmillette deleted the users/cgmillette/rocke/transforms-core-ops branch October 20, 2026 21:54

cgmillette mentioned this pull request Oct 20, 2026

fix(rocke): repair the differential golden and run its check in CI #13974

Merged

1 task

This was referenced Oct 21, 2026

fix(rocke): core dtype review follow-ups: uitofp_f32 source check and native test fixtures in every lane #14050

Merged

fix(rocke): one valid K=128 f8f6f4 MFMA lowering for fp8, fp6 and fp4 #13976

Merged

jakpiase mentioned this pull request Oct 21, 2026

feat(rocke): depthwise and grouped conv forward on gfx1250 #13987

Merged

7 tasks done

cgmillette mentioned this pull request Oct 29, 2026

fix(rocke): name seqlen and tile when dense attention rejects a partial tile #14335

Merged

1 task done

Conversation

cgmillette commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Technical Details

Test Plan

Test Result

Submission Checklist

Uh oh!

therock-pr-bot Bot commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ All Checks Passed — Ready for Review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

therock-pr-bot Bot commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AviralGoelAMD commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AviralGoelAMD left a comment

Choose a reason for hiding this comment

Uh oh!

cgmillette left a comment

Choose a reason for hiding this comment

PR #13802 — emit magic division's sum without signed-overflow poison (cgmillette, AICK-2575) — 🟠 REJECT: rebase, re-bless, attr parity

Uh oh!

CongMa13 left a comment

Choose a reason for hiding this comment

Uh oh!

tenpercent left a comment

Choose a reason for hiding this comment

Uh oh!

cgmillette commented Oct 16, 2026

Self-review N1 -- golden drift on develop's new gfx12_generic case

Self-review N2 -- new ctest missing from the tier YAML

Uh oh!

cgmillette commented Oct 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

CongMa13 left a comment

Choose a reason for hiding this comment

Uh oh!

CongMa13 left a comment

Choose a reason for hiding this comment

Uh oh!

tenpercent left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

assistant-librarian Bot commented Oct 22, 2026

TheRock Submodule Bump Activity

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cgmillette commented Oct 16, 2026 •

edited

Loading

therock-pr-bot Bot commented Oct 16, 2026 •

edited

Loading

therock-pr-bot Bot commented Oct 16, 2026 •

edited

Loading

AviralGoelAMD commented Oct 16, 2026 •

edited

Loading