Skip to content

feat(rocke): make the C++ engine consult the arch-domain table, and add error mode - #13723

Merged
CongMa13 merged 12 commits into
developfrom
users/congma/ck/rocke-arch-domain-binding
Oct 27, 2026
Merged

CongMa13 merged 12 commits into
developfrom
users/congma/ck/rocke-arch-domain-binding

Conversation

@CongMa13

@CongMa13 CongMa13 commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

JIRA ID : AICK-2274

feat(rocke): make the C++ engine consult the arch-domain table, and add error mode

Both engines now act on the same answers at their demand chokepoint.
In error mode they refuse, with the same message, a kernel that demands
an intrinsic its target provably cannot lower. The default stays warn;
promoting it is a separate change, gated on a recorded judgement that the
warning stream is quiet on real kernels.

C++ engine

  • rocke_ll_need looks up (flavor, key, gfx) in the generated table. In
    error mode it fails a non-waived negative with a new status,
    ROCKE_ERR_ARCH_DOMAIN, appended to rocke_status_t (no ABI bump), and a
    matching ckc::ArchDomainError.
  • It parses ROCKE_ARCH_DOMAIN by Python's rules, once per lowering at entry,
    so a misspelled value fails every lowering -- as lower_kernel_to_llvm does on
    the Python side -- and not only a kernel that demands an intrinsic. It has no
    warning channel, so warn and off are silent there.
  • The binding raises rocke_engine.ArchDomainError, a
    NotImplementedError, carrying the engine's message verbatim. It comes
    from lower_serialized_ir and from the shared take_* helpers.

Policy, still in Python only: rocke.core.arch.domain

  • The mode vocabulary and DEFAULT_MODE. The generator emits the default
    into the C++ table, so changing it moves both engines.
  • message(): the one sentence both engines print, cut at the C++ error
    buffer so the bytes match. parse_message() reads it back.
  • The waivers in arch_domain_waivers.json, validated strictly.

The lowerer and the backend

  • _Lowerer._check_arch_domain raises ArchDomainError in error mode
    unless the cell is waived, and warns otherwise.
  • ArchDomainError is a NotImplementedError, the convention for a kernel
    the lowerer declines on its target.
  • The cpp backend re-raises a refusal instead of falling back. The Python
    lowerer reads the same table and would refuse too.
  • A refusal this checkout's table would not make, from an extension built
    from other columns, is reported as BackendMismatch.
  • both mode requires both engines to refuse, over the same intrinsic.
  • The post-pass over C++ output still reports what the engine emitted. In
    error mode it raises on an unwaived negative the engine let through.

The escape hatch is one cell. A waiver names (key, arch, flavor) and the
reason the measurement is wrong. Both engines then report that cell
instead of refusing it, never silently. It is for a wrong measurement,
not a kernel bug. The file starts empty.

@github-actions 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 15, 2026
@therock-pr-bot

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

Copy link
Copy Markdown

✅ 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?

@therock-pr-bot

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

Copy link
Copy Markdown

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

CongMa13 and others added 6 commits October 16, 2026 21:28
…lumns

The committed arch-domain columns say, per (declaration key, gfx target,
LLVM flavor), whether a target can lower an intrinsic. The Python engine
reads them when it lowers. The C++ engine had no table at all, so the lane
could only run on C++ output after the fact, from Python. Making the
diagnostic binding needs both engines to refuse the same kernels, which
needs both to read the same answers.

A second, hand-maintained table would drift, and the first drift would
show up as a byte-identity failure caused by a data file. So the C++ copy
is generated at build time instead:

- `tools/gen_arch_domain_cpp.py` loads `rocke/core/arch/domain.py` by
  path (it imports only the standard library) and emits every cell
  `domain.is_negative()` accepts, sorted for a binary search. The policy
  stays one Python function; the C++ side gets answers only.
- CMake runs it with the columns, `domain.py` and the generator as
  dependencies, and compiles the result into `rocke_core`. The output
  lives in `<build>/generated/` and is never committed. Regenerating a
  column needs no other step: the next build rewrites it.
- `rocke/arch_domain.h` + `cpp/core/arch/domain.cpp` are the lookup. They
  are not wired into lowering yet; nothing changes for any kernel.
- `rocke_engine.arch_domain_column(flavor)` exposes the compiled table, so
  a test can hold it to the Python reader cell for cell.

Two consequences outside the engine:

- Configuring the platform C++ build now requires a Python 3 interpreter
  (it was optional). The README and BUILD.md say why.
- The columns, `domain.py` and the generator now feed the build id. The
  platform wheel's rebuild dependencies cover `python/**/*.json`, which
  was `*.py` only. A JSON-only regeneration did not rebuild the wheel.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`build_attention_3d` and `build_attention_reduce` in the parity harness
took an `arch` argument and dropped it. They imported the builder from the
top-level `kernels` package, which re-exports the gfx950 one, so the four
`attention/gfx942/{3d,reduce}_*` cases lowered the gfx950 kernels for
gfx942. `build_attention_2d` already routes through `_tiled_2d_impl(arch)`;
both now do the same through `_tiled_3d_impl(arch)`. Shipped gfx942
dispatch has always used the gfx942 builders, so this is a test fix.

It accounts for all nine entries in the arch-domain lane's allowlist:
`ds.read.tr16.b64` and the two 16x16x32 MFMAs are CDNA4 intrinsics, and
gfx942 is CDNA3. The gfx942 kernels demand none of them, so the corpus
now warns on nothing at any flavor and EXPECTED_WARNINGS is empty --
which is what making the diagnostic binding needs.

The golden moves for exactly those four cases at all three flavors (12
entries; the new kernel names carry the gfx942 tag). Every other case,
including the gfx950 3D and reduce twins, is byte-identical.

The two 3D cases keep their KNOWN_BAD entries in check_ir_validity.py,
but for a different reason. Compiled in the llvm20, llvm22 and llvm23
images, the gfx942 kernel stops at "local memory (71680) exceeds limit
(65536)": at d128/b64 it needs more LDS than gfx942 has, and
`supports_tiled_3d` admits the config without checking. The old reason,
an llvm23 backend fatal, belonged to the gfx950 kernel.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…dd error mode

Both engines now act on the same answers at their demand chokepoint.
In `error` mode they refuse, with the same message, a kernel that demands
an intrinsic its target provably cannot lower. The default stays `warn`;
promoting it is a separate change, gated on a recorded judgement that the
warning stream is quiet on real kernels.

C++ engine
- `rocke_ll_need` looks up (flavor, key, gfx) in the generated table. In
  error mode it fails a non-waived negative with a new status,
  ROCKE_ERR_ARCH_DOMAIN, appended to `rocke_status_t` (no ABI bump), and a
  matching `ckc::ArchDomainError`.
- It parses ROCKE_ARCH_DOMAIN by Python's rules, once per lowering at entry,
  so a misspelled value fails every lowering -- as lower_kernel_to_llvm does on
  the Python side -- and not only a kernel that demands an intrinsic. It has no
  warning channel, so `warn` and `off` are silent there.
- The binding raises `rocke_engine.ArchDomainError`, a
  NotImplementedError, carrying the engine's message verbatim. It comes
  from `lower_serialized_ir` and from the shared `take_*` helpers.

Policy, still in Python only: `rocke.core.arch.domain`
- The mode vocabulary and `DEFAULT_MODE`. The generator emits the default
  into the C++ table, so changing it moves both engines.
- `message()`: the one sentence both engines print, cut at the C++ error
  buffer so the bytes match. `parse_message()` reads it back.
- The waivers in `arch_domain_waivers.json`, validated strictly.

The lowerer and the backend
- `_Lowerer._check_arch_domain` raises `ArchDomainError` in error mode
  unless the cell is waived, and warns otherwise.
- `ArchDomainError` is a NotImplementedError, the convention for a kernel
  the lowerer declines on its target.
- The cpp backend re-raises a refusal instead of falling back. The Python
  lowerer reads the same table and would refuse too.
- A refusal this checkout's table would not make, from an extension built
  from other columns, is reported as BackendMismatch.
- `both` mode requires both engines to refuse, over the same intrinsic.
- The post-pass over C++ output still reports what the engine emitted. In
  error mode it raises on an unwaived negative the engine let through.

The escape hatch is one cell. A waiver names (key, arch, flavor) and the
reason the measurement is wrong. Both engines then report that cell
instead of refusing it, never silently. It is for a wrong measurement,
not a kernel bug. The file starts empty.

check_arch_domain.py drops EXPECTED_WARNINGS for the waiver list:
- an unwaived negative is red;
- a waiver whose cell no longer measures negative is red;
- with the extension importable, it lowers the corpus through the C++
  engine in error mode and requires the same verdicts.

Things review turned up, now part of the design rather than patched on:
- Only absence is "no data". The loaders for the columns and for the waivers
  catch FileNotFoundError and nothing else, so a file that exists but cannot
  be read raises instead of switching the lane off, or the waivers, in silence.
- ArchDomainError survives pickle and copy. Its constructor takes the measured
  cell and the base class keeps only the message, so a refusal raised in a
  worker process used to fail to unpickle in its parent.
- The generator refuses non-ASCII evidence: the engines cut a message at a
  byte count and at a character count, and agree only on ASCII.
- A BackendMismatch from the gate's C++ pass -- an extension built from other
  columns than this checkout -- is a disagreement, counted, not an unjudged case.

Tests:
- New ctest rocke_arch_domain_refusal covers mode parsing, the message
  cap, and refusal / warn / off on a real gfx950 demand, without the
  extension.
- The lane tests cover error mode, waivers, the post-pass, no-fallback,
  `both` verdicts and, with the extension, a C++ refusal message
  byte-identical to Python's.

Docs: AGENTS.md and env_flags.md (which lacked the flag), TESTING.md
§4.5, and the proposal's S4/S2 status.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Three test sites lowered RDNA and gfx1250 intrinsics with arch=gfx950,
because the lowering text was all they checked: permlane16, dpp8,
s.wait.event, s.alloc.vgpr, s.prefetch.inst and the async LDS copies. The
committed columns measure every one of them arch_absent on gfx950, so with
ROCKE_ARCH_DOMAIN=error both engines refuse them. In the parity family
`run_diff` would then score both refusals as BOTH_REJECTED, a pass that
compares nothing.

Each site now names a target that has the intrinsics, taken from the
columns:

- target_intrinsics_emit.{py,c}: a per-config ARCHES table, identical on
  both sides. gfx1250 for the async copies, s.prefetch.inst and the async
  marks; gfx1201 for permlane16/permlane64; gfx950 where it was already
  right. The scheduler-hints config mixed `s.wait.event`, which gfx1250
  lacks, with async marks that exist only there, so no target could lower
  it. `s.wait.event` moved to a ninth config at gfx1201.
- TestNewTargetIntrinsics: `_lower` takes an `arch`, and the ten tests
  error mode refused name gfx1201 or gfx1250. `_assert_engines_agree`
  stops defaulting a missing arch to gfx950 and requires it.
- future_intrinsic_lowering.cpp: the same nine cases name the same targets.

Expected strings are unchanged: they pin the declare and the call, which do
not depend on the target. The parity family has no golden; the gate
compares the two engines, which still agree byte for byte.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… from

The post-pass recovers demanded intrinsics from emitted IR by inverting the
key -> declare map, and that map is not injective: at llvm23
mfma.scale.f32.16x16x128.f8f6f4 and mfma.f32.16x16x128.fp8.hero are two keys
for one LLVM intrinsic and so have byte-identical declare text. A dict
comprehension kept whichever came last, so a kernel demanding the other was
reported under a key it never named -- a different table row, and a different
key than the Python lowerer warns under for the same kernel.

The IR cannot say which alias was demanded; nothing in a declare line
distinguishes them. So report every alias that is itself a measured negative
rather than guess one. That is also what keeps the cells independent: a
waiver names (key, arch, flavor), and collapsing the keys would let a waiver
on one alias silence a refusal that was never waived.

Refusing is judged more carefully than reporting, because refusing wrongly
blocks a kernel. The engine lowered the kernel, so whichever alias it demanded
was acceptable to it: if any alias sharing the declare is not an unwaived
negative, that one may be the demanded key, and raising over its sibling would
overrule a waiver on a key the kernel never asked for. Only when every alias is
an unwaived negative could none of them account for the engine having lowered
it, and only then does the post-pass refuse. Both are still reported, each
against its own cell.

The new tests find the aliased pair by scanning the committed columns
instead of naming it, so they keep testing this when the data moves and skip
when no column has such a pair.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…lives in

An arch-domain refusal carries the intrinsic, the target and the flavor in
its message and nowhere else -- the status says only that the target cannot
lower the kernel, and the message is all backend.py has to rebuild the Python
error from. Most family bindings lowered through rocke_lower_kernel_to_llvm,
which has no err out-parameter, and then passed nullptr to their take_*
helper, so the new refusal reached Python as an ArchDomainError with an empty
message: nothing to act on and nothing to look up.

Lower through the _ex entry everywhere. The reported site
(gfx1151_wmma_gemm) was one of 26 across four translation units, and two of
them had no arch-domain handling at all: rocke_engine_fmha_extra's take_ll
never called throw_if_arch_domain, and moe_gemm_fused threw a plain
runtime_error, so both turned a refusal into a generic RuntimeError.

Two more bindings had the same defect through a different door. gemm_lower_llvm
and img2col's lower_llvm lower through a family convenience wrapper rather than
calling the lowerer, formatted their own RuntimeError, and never reached
throw_if_arch_domain, so a refusal arrived as a plain RuntimeError. A second
scan now looks at every call whose name ends in to_llvm and requires a
conversion in the same function.

The helpers now take err as a reference to the array rather than a pointer,
which is what makes this stay fixed: the call that regressed it cannot be
written any more. A test scans the bindings for the err-less entry as well,
since the type system has nothing to say about which entry you call.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@CongMa13
CongMa13 force-pushed the users/congma/ck/rocke-arch-domain-binding branch from 7ceba66 to d04fdae Compare October 17, 2026 02:21

@CongMa13 CongMa13 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One new issue is described in the inline comment.

rocke_compute_build_id() runs while CMake configures, and the glob in it only
sees a column that is added or removed. An edit to a column, to the waivers,
or to the reader or the generator left ROCKE_BUILD_ID at its old value in an
existing build directory, while the build still regenerated the C++ table from
the new bytes. Output from the new table then carried the old engine id.

Name the table inputs as configure dependencies of the calling directory. An
edit to one now makes the next build re-run CMake, which computes the id
again. The engine sources keep their behavior: an edit to one still does not
re-run CMake, as before.

The new test configures a small project that includes the real module, edits
each input in turn, builds, and compares the id with the one that a new build
directory computes. It fails on the old module for all four inputs. It needs
CMake and the source tree, so the install leaves it out, like the other tests
that configure source CMake modules.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@CongMa13
CongMa13 marked this pull request as ready for review October 20, 2026 00:09
@CongMa13
CongMa13 requested review from a team as code owners October 20, 2026 00:09
@cgmillette

Copy link
Copy Markdown
Contributor

Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max

PR #13723 — C++ engine consults the arch-domain table, plus an error mode — 🟠 First-round: closer look — sound at its head, but it conflicts with develop in 10 files and needs a rebase and fresh verification

Prior item Status
CongMa13 (10-19, own pass) P2: editing a table input does not recompute ROCKE_BUILD_ID 🟢 FIXED 3edcaea0 — the columns, waivers, domain.py and the generator are CMAKE_CONFIGURE_DEPENDS; test_build_id_cmake.py passes (4) (ran)
Bots / CI 🟢 Pre-commit, clang-tidy pass; TheRock CI hipkernelprovider green on Linux gfx94X and Windows gfx110X (both shards each) at head 3edcaea0 (10-20); two internal CK checks (gfx1250 build, summary) fail, not visible here
Our checks 🟢 At the head, before any rebase: native build of the engine; ctest including the new rocke_arch_domain_refusal passes; byte identity GREEN for all families at llvm20 / 22 in the default warn mode; the lane and generator tests 92 passed (ran). The representative golden passes in warn and in error mode, so no representative kernel is refused, which supports promoting the default later (ran). The arch-domain gate is GREEN (ran). ROCKE_ERR_ARCH_DOMAIN is appended after ROCKE_ERR_NOTIMPL, so existing status values keep their numbers (read)
# Sev file:line Finding
N1 🟠 10 files Does not merge with develop (123 behind): BUILD.md, platform/CMakeLists.txt, cmake/rocke_build_id.cmake, cpp/bindings/rocke_engine.cpp, tests/CMakeLists.txt, test_arch_domain_lane.py, the representative golden, rocke_ir_parity_harness.py, check_arch_domain.py, check_ir_validity.py. Since the base, develop added 70–107 lines to each intrinsic_arch_domain.llvm*.json column (the table this PR compiles into the engine), and #13553 changed how stale engine artifacts are rejected (ran) → rebase, regenerate the table from develop's columns, re-bless the goldens, and re-run byte identity and the lane tests on the rebased tree.
N2 🟡 platform/tests/core/test_arch_domain_lane.py:1202 test_the_cpp_engine_refuses_with_the_python_message is the one check that both engines print the same bytes, and it importorskips rocke_engine, which CI never builds, so in CI only the C++ side (test_arch_domain_refusal) runs; it passes with the extension (ran) → say so in the description, or add a ctest that compares the C++ message with the Python message() output.

Utility: 🟢 Positive — one refusal rule and one message in both engines, with a single-cell waiver as the escape hatch; the right foundation for making unavailable intrinsics a hard error.


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

First-round review: a local qwen3.6 model made the independent first pass; the orchestrator (Claude Opus 5.5) verified it — the thread against the code, merge simulation, native build + ctest + byte identity at the head, the PR's tests, the representative corpus in both modes, the arch-domain gate, and the CI state — and owns this merge-worthiness call. It flags whether a closer look is needed; it is not an approval or a rejection.

CongMa13 and others added 3 commits October 21, 2026 16:33
Develop rewrote the build id as a build-time step (#13553) and fixed the
attention harness to build the arch-keyed kernel, which removes the two
gfx942 KNOWN_BAD entries. Keep the binding's side of the arch-domain lane
and fold the table inputs into the shared build id inputs, so a table edit
refreshes the id on every build. This replaces the configure-dependency
fix and its CMake test.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… bindings with a buffer

The C++ refusal test pins the message sentence, and a Python test with no
extension pins the same sentence, so CI compares the two engines.

The merged gfx1151 fmha bindings lowered through the entry that has no
diagnostic buffer. They use the entry with a buffer, as every other
binding does.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Develop compiles the engine once in rocke_core_objects. The table
belongs to that target, or no object can include it.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@CongMa13

Copy link
Copy Markdown
Contributor Author

N1 Does not merge with develop (123 behind) ... rebase, regenerate the table from develop's columns, re-bless the goldens, and re-run byte identity and the lane tests on the rebased tree.

Fixed in b7a6fc2. I merged develop into the branch. I did not rebase, so the push is a fast-forward. The PR squashes on merge.

  • The C++ table is generated from develop's columns at build time. No file is committed.
  • Develop's golden matches the merged tree at llvm20, llvm22 and llvm23. It also matches in error mode at llvm22 and llvm23. I did not re-bless it.
  • Develop now recomputes the build ID on every build. The table inputs are now in that input list and in its Python mirror. This replaces the CMAKE_CONFIGURE_DEPENDS change and its test.
  • Two new gfx1151 fmha bindings from develop lowered without an error buffer. They now use rocke_lower_kernel_to_llvm_ex.
  • The generated table is now a source of rocke_core_objects, which develop shares between the static and shared libraries.

Verification at b7a6fc2, in the ROCm 7.1.1 (llvm20), 7.2 (llvm22) and 7.13 (llvm23) images: the engine builds, byte identity is GREEN, the IR validity gate is GREEN, the arch-domain gate is GREEN, and ctest passes. I ran no GPU test.

N2 test_the_cpp_engine_refuses_with_the_python_message ... importorskips rocke_engine, which CI never builds ... add a ctest that compares the C++ message with the Python message() output.

Fixed in b7a6fc2. test_arch_domain_refusal.cpp pins two message sentences. The new Python test test_the_message_text_is_the_one_the_cpp_engine_pins pins the same two sentences against message(). Neither test needs rocke_engine, so CI fails when either engine drifts. The extension-based test stays as the end-to-end check.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@cgmillette

Copy link
Copy Markdown
Contributor

Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max

PR #13723 — C++ engine consults the arch-domain table, plus an error mode — 🟠 Follow-up: rebase needed — N1 and N2 are fixed, and a scratch merge with today's develop passes everything once its one code hunk takes develop's side

Prior item Status
Our N1 🟠 (10-21): 10-file conflict, 123 behind 🟢 FIXED by the develop merges 94412eaf and 2d8373ba, with b7a6fc27 moving the table into the shared object library. The C++ table is now generated from develop's columns at build time, and CI at 2d8373ba is green on every lane (read). It conflicts again with today's develop; see the Merge row
Our N2 🟡: the C++ / Python message comparison needed the extension, which CI never builds 🟢 FIXED in a9af8da2. test_arch_domain_refusal.cpp and the extension-free test_the_message_text_is_the_one_the_cpp_engine_pins pin the same two sentences. Negative control: drifting the Python sentence fails the pytest, and restoring it passes (ran)
CongMa13's P2 (10-19): the build ID ignored table-input edits 🟢 Superseded, as the author's reply says: develop now recomputes the build ID on every build, and the table inputs are in that list (read)
Bots / CI 🟢 Green at 2d8373ba (10-23): TheRock Multi-Arch, ASAN, the hipDNN superbuild, pre-commit and clang-tidy (read)
Merge 🟠 It conflicts with today's develop (102 behind) in three files:
• rocke_engine.cpp: #13897 retired the gfx1151 wmma_fmha_fwd binding that this PR rewired to the buffered entry.
• The lane-test docstring: #14160 moved test_arch_domain_artifact.py under tests/tools/.
• arch_axis_proposal.md.
Take develop's deletion in the binding and the PR's text elsewhere. Every binding then still lowers through the buffered entry, with 0 no-buffer calls left (ran). On that scratch merge, all of these pass (ran):
• The native build and 32 ctests, rocke_arch_domain_refusal among them.
• Byte identity for every family at llvm20 / 22 / 23 in warn mode.
• The representative corpus in both warn and error mode.
• The lane and generator tests: 93 with the engine; 69 without it, plus 9 engine-only skips.
• The arch-domain gate
Cross-PR 🟢 The merge with #13976 is clean. It conflicts with #14328 in domain.py (ran)

No net-new findings beyond the rebase.

Utility: 🟢 Positive — one refusal rule and one message in both engines, with a per-cell waiver as the escape hatch. The corpus already passes in error mode, which supports making that mode the default later.


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

Follow-up review: a local qwen3.6 model made the independent first pass. The orchestrator (Claude Opus 5.5) verified it. The checks were the author's replies against the code and a scratch merge with today's develop with the conflicts resolved as described. On that merge they covered the native build and ctests, byte identity for every family, and the representative corpus in both modes. They also included the lane tests with and without the engine, a negative control on the pinned message, the arch-domain gate, scratch merges with #13976 and #14328, and the CI logs at head. The orchestrator owns this merge-worthiness call. It flags whether a closer look is needed; it is not an approval or a rejection.

@cgmillette cgmillette left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@CongMa13
CongMa13 enabled auto-merge (squash) October 26, 2026 17:51
# Conflicts:
#	dnn-providers/hip-kernel-provider/rocke/platform/cpp/bindings/rocke_engine.cpp
#	dnn-providers/hip-kernel-provider/rocke/platform/dsl_docs/development/arch_axis_proposal.md
#	dnn-providers/hip-kernel-provider/rocke/platform/tests/core/test_arch_domain_lane.py
@cgmillette

Copy link
Copy Markdown
Contributor

Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max

PR #13723 — C++ engine consults the arch-domain table, plus an error mode — 🟢 Follow-up: no closer look needed — the rebase we asked for is done as described, and the merge with today's develop passes everything we ran

Prior item Status
Ours 🟠 (10-26): rebase needed; conflicts in rocke_engine.cpp, the lane docstring and arch_axis_proposal.md 🟢 DONE in the develop merge 6dd530ef. rocke_engine.cpp takes develop's retirement of the gfx1151 wmma_fmha_fwd binding, and every binding still lowers through the buffered entry, with 0 no-buffer calls (ran). The lane docstring names the moved tests/tools/test_arch_domain_artifact.py (ran)
Ours N1 / N2 (10-21), fixed on 10-26 🟢 Still fixed. rocke_arch_domain_refusal passes (ran). The lane and generator tests pass 93 with the engine, and 69 without it, plus 9 engine-only skips (ran)
Bots / CI 🟢 At 6dd530ef, the hipDNN superbuilds fail only on develop's ingestor break (KeyError: 'kernel_source', EveryHandlerReportsSupportsSaving…). This merge predates #14412, which fixed it today (read). The gfx94X tests are still running (read)
Our checks 🟢
• Range-diff: all 9 commits are unchanged, plus the develop merge (ran).
• On a clean merge with today's develop: the native build and 33 ctests pass. Byte identity in warn mode is GREEN for every family at llvm22 (85) and llvm23 (83); llvm20 was covered on 10-26 and the PR's code has not changed since. The representative corpus passes in both warn and error mode, and the arch-domain gate is GREEN (ran).
• rule_scan is clean (ran)
Cross-PR 🟡 Still conflicts with #14328 in domain.py. The merges with #13976 and #14211 are clean (ran)

No net-new findings. The worker's one item is dropped. The engine-only pytest still skips without the extension, but the C++ ctest rocke_arch_domain_refusal and the extension-free message test pin the same text in CI.

Utility: 🟢 Positive — one refusal rule and one message in both engines, with a per-cell waiver as the escape hatch. The corpus passes in error mode, which supports making that mode the default later.


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

Follow-up review: a local qwen3.6 model made the independent first pass. The orchestrator (Claude Opus 5.5) verified it. The checks were a range-diff against the head we reviewed and the author's conflict resolutions. On a merge with today's develop they covered the native build and ctests, byte identity for every family, the corpus in both modes, the lane tests with and without the engine, and the arch-domain gate. They also included cross-merges with #14328, #13976 and #14211, and the CI logs at head. The orchestrator owns this merge-worthiness call. It flags whether a closer look is needed; it is not an approval or a rejection.

@CongMa13
CongMa13 merged commit 99bf5e1 into develop Oct 27, 2026
100 of 107 checks passed
@CongMa13
CongMa13 deleted the users/congma/ck/rocke-arch-domain-binding branch October 27, 2026 03:49
@assistant-librarian

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

jakpiase added a commit that referenced this pull request Oct 28, 2026
…nd moe_fused_mega_fp8

rocke_differential_golden_ll and rocke_differential_golden_canonical fail
on develop, and so on this branch: #13723 changed the target_intrinsics
IR and the moe_fused_mega_fp8 LLVM IR after #13974 blessed the golden.

Re-record only those two families (llvm20, llvm22, llvm23 and canonical)
with run_diff.py --python-only --record-golden --only. The hashes match
the ones in #14501, which re-blesses the same entries on develop.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
BradPepersAMD pushed a commit that referenced this pull request Oct 28, 2026
The golden in llvm_gfx_all.json was recorded on a base without #13723
and #13976. Both PRs change the emitted IR. The checks
rocke_differential_golden_ll and rocke_differential_golden_canonical
fail on develop.

Re-record the golden on f37a705 for llvm20, llvm22, llvm23 and the
canonical IR mode. The golden also moves for
gfx942_attention_tiled_2d, which no earlier PR blessed.

Fixes #14474

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
adickin-amd pushed a commit that referenced this pull request Oct 28, 2026
Fixes #14474

# Summary: For human reviewers

The golden file `llvm_gfx_all.json` is stale. The checks
`rocke_differential_golden_ll` and `rocke_differential_golden_canonical`
fail on `develop`.

This PR records the golden again on `f37a7056c79`. The PR changes only
`llvm_gfx_all.json`.

## What changed

- **`target_intrinsics`**: #13723 changed the IR. The golden changes for
llvm20, llvm22, llvm23 and canonical.
- **`moe_fused_mega_fp8`**: #13976 changed the IR. The golden changes
for llvm20, llvm22 and llvm23.
- **`gfx942_attention_tiled_2d`**: The golden also changes for this
family. The issue does not list it. The cause is not confirmed.

## Review request

1. Confirm the authors of #13723 and #13976 meant the IR changes.
2. Confirm the author of #14349 meant the `gfx942_attention_tiled_2d`
change. The probable source is #14349.

## Verification

- Before the change, the golden check failed on `develop` for llvm20 and
llvm22.
- After the change, `run_diff.py --mode ll --python-only --check-golden
--flavor llvm20 --only target_intrinsics,moe_fused_mega_fp8` prints
`GOLDEN OK`.
- The full check for each flavor, the canonical check and the
byte-identity gate did not run. CI runs them.

# Summary: For AI reviewers

- The diff changes one JSON file. No emitter code changes.
- The command `run_diff.py --record-golden` wrote the file with
`--python-only`.
- Check the changed hashes only against the three families above.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
renjithravindrankannath pushed a commit that referenced this pull request Nov 3, 2026
…se MMA instruction (#14328)

## Motivation

rocKE should know every production target its MMA catalog names, and
should be able to issue every dense MMA instruction each of those
targets has, in both engines. Today some targets are unknown, several
MMA lowerings print a fixed operand type around any value, and integer
WMMA signedness is hard-coded to signed. This PR makes the ISA backends
complete for the dense instructions:
- every op is described as data;
- every target compiles for itself where its LLVM flavor accepts the
name;
- operand signedness comes from the operand dtype.

ISSUE ID : AICK-2571

## Technical Details

The 18 feature commits below, then the review fixes listed under
**Changes after review** (and merges of develop). Each builds and passes
its own tests on its own, and the Python and C++ engines emit
byte-identical LLVM IR at every commit.

1. **Delete the duplicate f16 32x32x16 MFMA handler.**
2. **Resolve each LLVM flavor's intrinsic declarations in one place**,
in both engines. Each declaration generation (LLVM 22, 23, 24) holds
only its own changes and inherits the older ones.
3. **Describe matrix intrinsic calls as data.** `MATRIX_CALLS` lists
each op's intrinsic and operand forms. An op lowered through it is
refused on a target whose catalog does not list it.
4. **Let a target row take its facts from another row.** Every member of
a generic target gets a pointer row.
5. **Add the missing gfx targets.**
   - gfx908 gets a full fact row; its occupancy is measured with `llc`.
- gfx1100-1103 and gfx1150-1154 point to gfx11-generic; gfx1200 points
to gfx12-generic.
- Every new target's arch-domain cells are measured on each flavor's
compiler.
6. **Compile each target for itself where the compiler that runs accepts
it.** Otherwise it compiles for the generic its row points at; if
neither works, the compile fails loudly. The choice follows the LLVM
major of the compiler that will run (see Changes after review).
7. **Add CDNA f32, i8, f64, bf16 and mixed fp8 MFMA ops.** gfx942's f64
ops keep the lowering #13794 added; gfx90a and gfx950 list the f64
16x16x4 atom.
8. **Add gfx12 fp8, bf8, i8 and 16-bit-output WMMA ops.**
9. **Add gfx12 int4 WMMA ops, and derive integer WMMA signedness from
the operand dtypes.** The unsigned rows are named `_u8_i8`, `_i8_u8` and
`_u8_u8` (and the u4 equivalents). The gfx11 integer handler moves into
`MATRIX_CALLS`.
10. **Add gfx11 16-bit-output WMMA ops.**
11. **Describe MMA instructions whose result type differs from the
accumulator.**
- A row may carry a result dtype `dst`, the name agreed on #11642.
`MmaOp.dst_dtype` defaults to the accumulator dtype, and catalog queries
take `dst_dtype`.
- The C++ op struct grows, so the binary ABI goes from 6 (develop) to 7
in `abi.h` and in Python's `BINARY_ABI`.
    - Adds the gfx1250 bf16-result op.
12. **Add gfx1250 16-bit-output, fp8-to-f16 and i8 WMMA ops, and wire
f32 16x16x4.**
13. **Add the gfx1250 f8f6f4 and f4 32x16x128 WMMA ops.**
14. **Add the gfx950 f8f6f4 MFMA ops with a 6-bit or 4-bit operand.**
Covers every format pair, at both shapes.
15. **Add the larger-K variants of existing MMA shapes.**
16. **Accept logical and packed operands on the gfx1250 scaled WMMA
ops.** A and B can be logical fp8 vectors or packed 6/4-bit words, as
well as the padded `<16 x i32>` form. The gfx950 scaled MFMA ops keep
the packed-word form: their only caller already loads those words.
17. **Run the fp64 GEMM numerics on every target that admits them.**
They built for gfx942 only; they now build for the GPU they run on and
run on gfx90a, gfx942 and gfx950 (the hardware-verified targets, marked
`arch_specific`). An admitted target that has not run them yet skips, or
fails when the lane requires it.
18. **Move the fp64 GEMM numerics to their own file and share the GPU
checks.** `tests/numeric/test_f64_gemm_numeric.py` holds them; the
gfx942 f16/bf16 swizzle regression keeps its own gfx942 file. A CPU-only
test checks that both engines refuse fp64 `direct_to_lds` on targets
without DRAM-to-LDS DMA. Shared test helpers replace copies in the
numeric tests: `current_device_arch_in` (the GPU's arch if it is in a
list, else skip, and fail under `ROCKE_REQUIRE_<ARCH>=1`),
`available_engines` / `all_engines_backend`, and `universal_gemm` (lower
and launch a universal GEMM spec with a required target arch).

**Changes after review.** These commits follow the 18 above and change
behavior:
- **Compile-target routing follows the compiler that runs.**
`compile_kernel` and `lower_kernel_for_comgr` route by the LLVM major of
the COMGR compiler they load, and the hipcc helpers by hipcc's clang
major, through `compiler_target_for_llvm_major` in `rocke.core.arch`.
`ROCKE_LLVM_FLAVOR` still picks the emitted IR's conventions only. A
major in a qualified lane uses that lane's measured processor answers; a
newer or unknown major keeps the processor's own name, and a COMGR build
refuses a compiler it cannot place. The hipcc helpers now run `hipcc
--version` and raise if it prints no `clang version N.` line.
- **Dispatch candidates list each target with its own facts once.**
`arches_with_own_facts()` in `rocke.core.arch` gives the known targets
whose rows hold their own facts; norm and the unified attention paths
declare it, and the fp16/bf16 RCR WMMA GEMM candidates share
`RDNA_WMMA_GEMM_ARCHES`. A processor is admitted through the generic its
row points at (`listed_arch_for`), so example-parity builds each
distinct target once.
- **Generic members resolve through their generic's backend row, in both
engines.** The backend registries hold rows only for targets with facts
of their own; a member gets its generic's backend under its own name.
Both engines refuse an unknown target with the same message, built from
the registry.
- **The C++ engine's compiler processor rejections are generated** from
the arch-domain columns by `tools/gen_arch_domain_cpp.py`, through the
Python reader, instead of a hand-kept list.
- **New refusals, in both engines where both lower:** an f8f6f4 operand
that is short of its packed words (for example one fp4 word where eight
are needed) instead of zero-padding it; an MMA result declared in
another number format than the instruction returns instead of bitcasting
it; an arch-domain processor answer that is not a JSON boolean.
- **Universal GEMM test launches** pack the arguments the builder
declares (strides for every batched kernel) and default C to NaN in the
kernel's storage dtype.
- **Tests and docs.** The test-helper and gfx908 occupancy tests moved
under `platform/tests/support` and `platform/tests/benchmark`, where
`rocke_pytest` collects them; the shared llc helpers live in
`rocke.tests.llc`. The unknown-target verdict test skips on a host that
cannot judge any module, such as the installed test tree. The
backend-registry and arch-data ctests are in the provider's quick tiers,
and a test fails when a C++ test is in no tier. The fp64 GEMM docs name
gfx90a, gfx942 and gfx950 everywhere, with a test that checks every such
sentence, and the dispatch docs say that a listed generic target covers
its members.

Every lane map comes from an arbiter: hardware, a public calculator,
public ISA tables, or CK where it is the named reference. None is
invented. The docs list every new op and target: `dtype_support.md`,
`mfma_atom_catalog.md` and `mma_metadata.md`.

**Built on develop with #13976 and #13723:**
- #13976 removed the two gfx950 ops that do not exist on the hardware,
which was this PR's first commit, so that commit is gone.
- The gfx950 f8f6f4 16x16x128 rows call the declaration #13976 made
shared (`mfma.scale.f32.16x16x128.f8f6f4.v8i32.v8i32`), so a kernel that
mixes them with #13976's builder ops emits one `declare`.
- Develop's binary ABI is 6 (#14706, #14616); this PR is 7.
- Develop's LLVM 24 lane (#14692): the named-barrier declarations are
LLVM 24's own overrides in the chain, and the LLVM 24 arch-domain column
and goldens cover this PR's targets and ops.
- The LLVM 24 arch-domain column is regenerated with the llvm24 compiler
CI runs (AMD clang 24.0.0git, llvm-project 2792f8ceeb5a), and its header
now records that compiler. It answers 38 of develop's cells differently
from the build develop's column recorded: on the five targets without
gfx12 barriers the s.barrier.init/join/signal/signal.var/wait and
s.cluster.barrier declarations crash the compiler (`toolchain_crash`)
instead of being refused, the s.barrier.leave refusal has new wording,
and gfx1250 cannot select the bf16 fmuladd, maxnum and minnum. That
compiler accepts gfx1154 and gfx1250 by name, so at llvm24 both compile
for themselves.
- #14156 (gfx1250 LDS 320 KiB) is independent of this PR.

## Test Plan

- One test file per op group (Python, both engines in strict mode) and
one ctest per group (C++ builders and catalog lookups):
- the IR for every new op at llvm20, llvm22, llvm23 and llvm24, and both
engines equal;
  - the refusals, with their exact texts;
  - selection over every target and dtype;
  - lane-map checks against each arbiter;
  - compile and link per target.
- Numeric tests on hardware where a GPU is available: the gfx90a MFMA
f64, f32, i8 and bf16 ops, and the fp64 GEMM numerics on gfx90a, gfx942
and gfx950.
- The representative-IR golden gains the 10 new gemm cases (`gemm.json`
shard) at all four LLVM flavors, in commit 5. The other entries are
unchanged.

## Test Result

- Byte identity green at llvm20, llvm22, llvm23 and llvm24.
- ctest 46 / 46 at the head (the arch-domain reproducibility test skips
without its compiler).
- Full platform suite at the head (Python engine): 6015 passed, 0
failed.
- The portable-IR tests against a freshly built shared engine pass: 196.
- The touched tests pass with both engines in strict mode, apart from
`test_the_cpp_fallback_does_not_warn_twice`, which fails in strict mode
on develop too.
- The other platform test files and the library smoke: no new failure
against develop.
- pre-commit passes.
- Per-commit check: every commit builds, passes ctest and passes its own
tests (strict mode; the same develop test as above is the only
exception).
- The arch-domain columns equal a full regeneration at the head on
llvm20, llvm22, llvm23 and llvm24, cell for cell; the llvm24 column at
every commit is that commit's full regeneration with CI's llvm24
compiler.
- At llvm24 with CI's llvm24 compiler: the compile-module and target
identity tests pass, and so do the gfx90a numeric MFMA tests on a
gfx90a.
- gfx90a numeric tests pass.
- fp64 GEMM numerics, both engines in strict mode, against an fp64
reference (exact on integer inputs, within the runbook tolerance on
random inputs): every admitted config passes on gfx90a, gfx942 and
gfx950, with partial tiles and a problem smaller than one tile; the
Python and C++ IR are identical. gfx90a refuses the four `direct_to_lds`
configs (no DRAM-to-LDS DMA), as the test expects.
- The other targets are compile-verified.

## Submission Checklist

- [x] 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>
Co-authored-by: John Shumway <jshumway@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants