Skip to content

feat(rocke): Add missing target intrinsics and llvm23 support - #9583

Merged
tenpercent merged 11 commits into
developfrom
users/yraparti/rocke/llvm23_flavor_support
Jul 30, 2026
Merged

tenpercent merged 11 commits into
developfrom
users/yraparti/rocke/llvm23_flavor_support

Conversation

@yraparti

@yraparti yraparti commented Jul 18, 2026 •

Copy link
Copy Markdown
Contributor

ISSUE ID : AICK-1576

Motivation

Several missing LLVM intrinsics and bugs in lowering were found while exploring LLVM23 release.

Category New LLVM 23 AMDGPU Intrinsics / Features
Async LDS asyncmark, wait.asyncmark, raw.ptr.buffer.load.async.lds,
global.load.async.to.lds.b8
DPP / Swizzle mov.dpp8.{i32,f32}, ds_swizzle (raw-offset API)
Reductions wave.reduce.{fmax,fadd}.f32,
wave.reduce.{add,max,min}.i32
Lane Ops readlane.{i32,f32}, writelane.{i32,f32}
Relayout permlane16, permlane64, alignbyte, s.wqm.{i32,i64}
Vector Mem av.load.b128, av.store.b128
Scheduler s.alloc.vgpr, s.wait.event, s.prefetch.inst

Technical Details

New IRBuilder APIs
ds_swizzle, mov_dpp8, wave_reduce, readlane, writelane, permlane16, permlane64, alignbyte, s_wqm, av_load_b128, av_store_b128, s_alloc_vgpr, asyncmark, wait_asyncmark, s_wait_event, s_prefetch_inst, buffer_load_lds_async; global_load_async_to_lds now supports 1-byte width.

Added c++ mirroring for the new intrinsics and the IRBuilder

Test Plan

test_future_intrinsic_lowering — exercises the new ops and verifies LLVM emission + declares.
tests/core/future_intrinsic_lowering.cpp — C++ equivalent of Python TestHelpers::test_future_intrinsic_lowering; builds the same smoke kernel and checks for all 12 @llvm.amdgcn.* declares
tests/CMakeLists.txt — registers rocke_future_intrinsic_lowering as a CTest target

Test Result

C++ test: pass
ctest -R rocke_future_intrinsic_lowering: pass
Python test_future_intrinsic_lowering: pass
check_byte_identity.py (llvm23): GREEN (65 families)

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Jul 18, 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

Copy link
Copy Markdown

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

Copilot AI 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.

Pull request overview

This PR extends Rocke’s IR and lowering pipeline to support LLVM 23 (ROCm 7.13+) and adds a set of previously-missing AMDGPU intrinsics/APIs, with parity across the Python and C++ engines and updated tests/golden baselines.

Changes:

  • Add an llvm23 flavor across Python/C++ (datalayout selection, flavor resolution, intrinsic decl overrides, parity harness/golden updates).
  • Introduce IRBuilder + lowerer support for new cross-lane, reduction, async-marker, vector-mem, and scheduler intrinsics (and wire them through the C++ engine too).
  • Add/extend unit tests (Python + new C++ parity test) and update representative IR hash golden to include llvm23.

Reviewed changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
dnn-providers/hip-kernel-provider/rocke/platform/tests/test_rocke.py Adds Python smoke tests for new intrinsics and LLVM23 buffer/mfma flavoring.
dnn-providers/hip-kernel-provider/rocke/platform/tests/instances/test_rocke_multiarch.py Extends datalayout drift guard to account for llvm23.
dnn-providers/hip-kernel-provider/rocke/platform/tests/instances/rocke_ir_parity_harness.py Updates parity harness to include llvm23 in golden flavors.
dnn-providers/hip-kernel-provider/rocke/platform/tests/golden/rocke_representative_ir_sha256.json Adds llvm23 representative IR hashes.
dnn-providers/hip-kernel-provider/rocke/platform/tests/core/test_mma_frag_tables.py Updates MMA frag-length consistency checks to use SSOT-backed lookups.
dnn-providers/hip-kernel-provider/rocke/platform/tests/core/test_arch_mma_ssot.py New tests pinning the bare-op_id accumulator-dtype SSOT contract.
dnn-providers/hip-kernel-provider/rocke/platform/tests/core/future_intrinsic_lowering.cpp New C++ parity smoke test for the new intrinsics’ declares.
dnn-providers/hip-kernel-provider/rocke/platform/tests/CMakeLists.txt Registers the new C++ parity test as a CTest target and installable test.
dnn-providers/hip-kernel-provider/rocke/platform/python/rocke/runtime/comgr.py Improves IR/comgr flavor mismatch detection for llvm20 vs “modern” (llvm22/llvm23).
dnn-providers/hip-kernel-provider/rocke/platform/python/rocke/core/lower_llvm.py Adds llvm23 flavor, new intrinsic decls, and lowering for new ops + metadata emission.
dnn-providers/hip-kernel-provider/rocke/platform/python/rocke/core/ir.py Adds IRBuilder APIs for new intrinsics and moves MMA sizing to arch SSOT.
dnn-providers/hip-kernel-provider/rocke/platform/python/rocke/core/arch/target.py Adds _op_id_c_dtype() SSOT aggregation helper (cached) for bare-op_id queries.
dnn-providers/hip-kernel-provider/rocke/platform/cpp/include/rocke/lower_llvm.h Extends public C API to include ROCKE_LLVM_FLAVOR_LLVM23.
dnn-providers/hip-kernel-provider/rocke/platform/cpp/include/rocke/lower_llvm_internal.h Adds llvm23 datalayout symbol, modern-flavor helper, and llvm23 override plumbing.
dnn-providers/hip-kernel-provider/rocke/platform/cpp/include/rocke/ir.h Adds new IR opcodes and builder APIs for the new intrinsics.
dnn-providers/hip-kernel-provider/rocke/platform/cpp/include/rocke/arch_target.h Exposes bare-op_id SSOT lookups (frag len + accumulator dtype).
dnn-providers/hip-kernel-provider/rocke/platform/cpp/core/lower_llvm/mma.cpp Uses “modern flavor” predicate for mfma operand packing (llvm22/llvm23).
dnn-providers/hip-kernel-provider/rocke/platform/cpp/core/lower_llvm/mem.cpp Adds lowering for async LDS loads and global async-to-LDS intrinsic emission.
dnn-providers/hip-kernel-provider/rocke/platform/cpp/core/lower_llvm/data.cpp Adds llvm23 datalayout, new intrinsic decls, llvm23 overrides, and override resolver.
dnn-providers/hip-kernel-provider/rocke/platform/cpp/core/lower_llvm/crosslane.cpp Implements lowering for new cross-lane/reduction/scheduler/vector-mem intrinsics.
dnn-providers/hip-kernel-provider/rocke/platform/cpp/core/lower_llvm/core.cpp Adds llvm23 flavor resolution and generalized override selection + metadata emission.
dnn-providers/hip-kernel-provider/rocke/platform/cpp/core/lower_llvm/control.cpp Adds lowering for asyncmark/wait_asyncmark/s_wait_event/s_prefetch_inst (and gating for asynccnt).
dnn-providers/hip-kernel-provider/rocke/platform/cpp/core/isa/backend.cpp Clarifies datalayout behavior for llvm23 (currently matches llvm22).
dnn-providers/hip-kernel-provider/rocke/platform/cpp/core/ir/ir_tile.cpp Switches MMA sizing from private tables to arch SSOT bare-op_id lookups.
dnn-providers/hip-kernel-provider/rocke/platform/cpp/core/ir/ir_flow.cpp Adds builder-layer validation + construction for new IR ops.
dnn-providers/hip-kernel-provider/rocke/platform/cpp/core/ir/core_types.cpp Registers opcode names/purity for new ops.
dnn-providers/hip-kernel-provider/rocke/platform/cpp/core/arch/query.cpp Implements bare-op_id accumulator-dtype lookup across arch registry.
dnn-providers/hip-kernel-provider/rocke/platform/cpp/core/arch/data.cpp Adds SSOT mirror table for MMA frag lengths + exposes rocke_arch_mma_c_frag_len.
dnn-providers/hip-kernel-provider/rocke/platform/cpp/bindings/rocke_engine.cpp Updates Python bindings error messages/docs to include llvm23 flavor.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@yraparti
yraparti force-pushed the users/yraparti/rocke/llvm23_flavor_support branch from 5d5f5f9 to 56006f3 Compare July 28, 2026 16:49
@tenpercent

Copy link
Copy Markdown
Contributor

Review: AMDGPU lowering correctness

Reviewed with a focus on AMDGPU correctness, grounding every intrinsic signature against ROCm/llvm-project@amd-staging (9eb9105703b7) rather than recall. Findings below were reproduced end-to-end, not just read off the diff: I built rocke_core + rocke_engine from this PR's head (56006f38) inside a LLVM 23.0.0git container (rocm-devtools-therock-7.15) and ran both engines.

Repro scripts: https://gist.github.com/tenpercent/7d8a007be464f82d38898663e94b18da

Most of the new intrinsics check out — details at the end. Three issues worth addressing, one of them a hard IR bug.


1. global_load_async_to_lds (C++) uses the raw smem name as the GEP base — emits invalid IR

cpp/core/lower_llvm/mem.cpp:1348,1358 feeds gname straight into the destination GEP:

gname = rocke_ll_smem_global_name(L, lds_smem, &stype);
...
rocke_strbuf_appendf(&gep, "  %s = getelementptr inbounds %s, ptr addrspace(3) %s, i32 0",
                     gep_l, agg_ty, gname);

rocke_ll_smem_global_name only resolves the allocation's key; rocke_ll_emit_smem_base_ptr (core.cpp:1319) is the required second step that maps it into the unified pool. Every sibling handler calls both (mem.cpp:406-407, 432-433, 461-462, 490-491, 530-531, 604-605, 637-638, 687-688), as does the Python reference this mirrors (python/rocke/core/lower_llvm.py:3386-3387).

Only @smem_pool.<kernel> is ever emitted as an addrspace(3) global, so @<alloc>.<kernel> never exists. Real output from the engine built from this branch (two LDS allocs, arch=gfx950, flavor=llvm23):

byte-identical?  False
CPP:  %async_dst.14 = getelementptr ... ptr addrspace(3) @stageB3.f2_repro, i32 0, i32 0, i32 0
PY :  %async_dst.15 = getelementptr ... ptr addrspace(3) %smem_base.14,     i32 0, i32 0, i32 0

globals defined in C++ output: @smem_pool.f2_repro      (@stageB3 definitions: 0)
opt -passes=verify  python .ll -> ACCEPTED
opt -passes=verify  c++    .ll -> REJECTED: use of undefined value '@stageB3.f2_repro'

This hits every kernel using the op, not only multi-allocation ones: with a single alloc at pool offset 0 the correct base is @smem_pool.<kernel>, which still isn't gname.

Fix: use rocke_ll_emit_smem_base_ptr(L, gname, stype) as the GEP base, as the sibling handlers do.


2. backend.py flavor gate wasn't extended to llvm23 — the cpp engine is silently bypassed

python/rocke/core/backend.py:254-256 (not in this PR's changed files) still reads:

from .lower_llvm import LLVM_FLAVOR_LLVM20, LLVM_FLAVOR_LLVM22
if flavor not in (LLVM_FLAVOR_LLVM20, LLVM_FLAVOR_LLVM22):
    raise ValueError(f"unknown LLVM flavor {flavor!r}")

lower_llvm.py:961-966 was updated to accept LLVM_FLAVOR_LLVM23, but this parallel gate on the cpp dispatch path was not. Since _DEFAULT_BACKEND = BACKEND_CPP, on a ROCm >= 7.13 host:

ROCm-detected flavor : llvm23     default backend : cpp
WARNING: backend='cpp' fell back to the Python lowerer: ValueError: unknown LLVM flavor 'llvm23'

So on the platform this PR targets, the C++ engine is bypassed for every kernel and everything quietly lowers via Python — no hard failure unless ROCKE_CPP_STRICT=1. That also means the C++ half of this PR doesn't execute through the normal API.

To be clear about scope: this does not invalidate the check_byte_identity result, since run_diff.py compiles the native *_emit.c against the engine directly and the golden harness calls the Python lowerer explicitly. It's the runtime path that's affected.

Fix: add LLVM_FLAVOR_LLVM23 to that tuple.


3. s_prefetch_inst hardcodes addrspace(0)

cpp/core/lower_llvm/control.cpp:1033 and python/rocke/core/lower_llvm.py:3287 (decl at :817) emit a bare ptr:

call void @llvm.amdgcn.s.prefetch.inst(ptr %s, i32 %s)

.td:3264 types this operand llvm_anyptr_ty. Verified on LLVM 23:

actual operand type result
ptr (addrspace 0) ACCEPTED
ptr addrspace(4) — instruction memory REJECTED: '%p' defined with type 'ptr addrspace(4)' but expected 'ptr'
ptr addrspace(1) — global REJECTED (same)

LLVM's own test (llvm.amdgcn.s.prefetch.inst.ll) uses addrspace(4), so this breaks as soon as a realistic pointer is passed.

Worth noting so nobody over-fixes: the missing .pN mangling suffix is not a problem — LLVM auto-upgrades @llvm.amdgcn.s.prefetch.inst to .p0 and it codegens to the same global_prefetch_b8. Only the address space needs fixing.


Test coverage (medium)

9 of the 19 new opcodes have no direct test: permlane16, buffer_load_lds_async, global_load_async_to_lds, av_load_b128, av_store_b128, s_alloc_vgpr, s_wait_asynccnt, s_wait_event, s_prefetch_inst. All three issues above sit in that set. The smoke tests assert substrings against each engine's own output rather than cross-comparing, and no parity emitter exercises the new ops — a single cross-engine case for global_load_async_to_lds would have caught #1 immediately.

Smaller items

  • wait_asyncmark / s_wait_asynccnt don't mask their i16 immediates, unlike s_wait_event which masks in both builder and lowering. LLVM silently truncates (i16 70000 -> i16 4464), so an out-of-range wait count becomes a wrong wait count with no diagnostic. Both engines behave the same, so parity is intact.
  • The C++ engine registers no gfx1250 backend (gfx908/90a/942/950/1151/1201), so ll_has_async_lds_counter()'s strncmp(gfx,"gfx125",6) can never be true and s_wait_asynccnt is an unconditional no-op there. Python gates on a backend capability flag instead; consider matching that rather than a string prefix.
  • s.wait.asynccnt's declare omits immarg though .td:3808 has ImmArg<ArgIndex<0>> (siblings include it). Likely cosmetic — I did not verify whether it matters.
  • New opcodes are spliced mid-enum rather than appended. Safe here (serialization is by-name, the three tables stay in sync, no numeric opcode crosses pybind), but appending is more defensive.

Verified correct

ds_swizzle, mov_dpp8 (24-bit sel mask is right), wave_reduce (the 5 permitted ops are all valid members of LLVM's 13), readlane/writelane (correctly runtime, not immarg), permlane16 (all 6 args), permlane64, alignbyte, s_wqm (return type tracks operand width), s_alloc_vgpr (the i1 success flag is captured and zext-ed, not dropped), s_wait_event (doesn't conflate with the arg-less s_wait_event_export_ready), av_load/store_b128 (metadata scope arg present, !3 = !{!"agent"} emitted), and buffer_load_lds_async — 7 args in the right order, dwordsx4 -> bytes {4,12,16}, a valid subset of LLVM's {1,2,4,12,16}, with all three ImmArg positions literal.

The llvm23 flavor plumbing is otherwise complete, and the llvm23 == llvm22 datalayout alias has a live drift-guard test rather than being silently permanent.

…ic cases, clamped LLVM_FLAVORS version ladder, fixes llvm23 rejection bugs in c++ mirror
@tenpercent

Copy link
Copy Markdown
Contributor

Re-review of f19a5b09

Re-ran the review against the new head. Thanks for the fast turnaround on the tests — coverage went from 10/19 to 18/19 opcodes, and TestLlvmFlavorEnumeration is genuinely good work (the AST lint that forbids re-listing flavors by hand is exactly the right shape; I mutation-tested it against a reverted backend.py and it catches the regression).

Everything below was built and run: engine compiled from this branch, IR validated on LLVM 23.0.0git (ROCm/llvm-project@9f1cce5a, ROCm 7.15). Repros + the suggested patch: https://gist.github.com/tenpercent/83ee6deeb261f934c62a61bd88654ebd

Status of the three earlier findings

backend.py llvm23 gate fixed — flavor not in LLVM_FLAVORS is correct and complete; I swept for other hardcoded pairs and found none
global_load_async_to_lds LDS base not fixed — and see below, it is now live
s_prefetch_inst address space not fixed — and the scope is larger than I first reported

One note on the commit message: the core.cpp change is comment-only. The llvm23 rejection fix is entirely in backend.py.

1. Fixing the flavor gate made the LDS bug live (blocking)

These two interact. Previously backend="cpp" rejected llvm23 and silently fell back to Python, which emits correct IR — that accidentally shielded the mem.cpp defect. With the gate fixed, the default path now reaches the C++ engine:

resolved flavor: llvm23 | default backend: cpp
default-path emits:  %async_dst.14 = getelementptr ... ptr addrspace(3) @stageB3.esc, ...
opt -passes=verify:  error: use of undefined value '@stageB3.esc'

So on an llvm23 host with no overrides, lower_kernel_to_llvm now produces invalid IR where it previously produced valid IR. mem.cpp:1348 is the only one of 14 rocke_ll_smem_global_name call sites in the C++ tree not paired with rocke_ll_emit_smem_base_ptr — adding that hop is the fix.

2. Four cases emit IR that LLVM 23 rejects, in both engines

av_load_b128 and av_store_b128 have the same address-space defect as s_prefetch_inst, and buffer_load_lds_async has a separate one:

case engines agree? LLVM 23
av_load_b128 identical '%p' defined with type 'ptr addrspace(1)' but expected 'ptr'
av_store_b128 identical same
s_prefetch_inst identical same
buffer_load_lds_async identical '%lds_addr3' defined with type 'i64' but expected 'ptr addrspace(3)'
global_load_async_to_lds differ C++ use of undefined value '@stageB3...'; Python OK

buffer_load_lds_async is new since my last pass: the handler feeds smem_addr_of() (an i64) straight in, where the sibling _op_tile_async_buffer_load_lds_addr does an inttoptr first.

To save anyone re-deriving it: the missing .pN mangling suffix is not a problem — LLVM auto-upgrades it. Only the address space needs fixing.

3. The new tests pin the broken IR

This is the part worth acting on first. Running the PR's own tests against exactly the code above:

$ pytest -k "av_load_b128_emits or av_store_b128_emits or s_prefetch_inst_emits or buffer_load_lds_async_converts"
4 passed

The asserted strings are the rejected text, e.g. future_intrinsic_lowering.cpp:388:

EXPECT_IR(ir, "call void @llvm.amdgcn.s.prefetch.inst(ptr %code, i32 64)");

with the test passing a "global" (addrspace 1) pointer. A correct fix makes these tests fail, so whoever implements it will be nudged to "repair" the expectation back to the broken form. Worth updating the assertions in the same change.

Two changes that would shorten this loop, with measured effect

Both verified on this branch.

a. Run the suite once with ROCKE_BACKEND=both. No code change. backend="both" already raises BackendMismatch on divergence:

ROCKE_BACKEND unset   32 passed
ROCKE_BACKEND=python  32 passed
ROCKE_BACKEND=cpp     32 passed     <- emits the undefined global, assertions can't see it
ROCKE_BACKEND=both     1 failed, 31 passed
                       -> test_global_load_async_to_lds_supports_b8_width

That single env var turns the C++/Python divergence from invisible into a named failing test.

b. Assemble the emitted IR in the shared _lower helper (08_suggested_fix_validate_ir.patch in the gist, ~10 lines in one function):

before: 32 passed
after:   5 failed, 27 passed
        av_load_b128 / av_store_b128 / buffer_load_lds_async
        global_load_async_to_lds_b8 / s_prefetch_inst

Exactly the five defective cases, no false positives. It skips silently when llvm-as is absent. One caveat: it introduces subprocess, which test_rocke.py:15 currently disclaims — gating it behind an env var would preserve that contract.

The two are complementary: (b) catches malformed IR from whichever engine runs, (a) catches the engines disagreeing even when both are individually valid. Together they would have caught all five defects before review.

On the "c++ parity for 24 new intrinsic cases" framing — the two suites are independently hand-written string literals, and neither invokes the other engine. future_intrinsic_lowering.cpp:14-15 states the byte-identical contract, but nothing enforces it, which is why both a shared mistake and a genuine divergence pass today.

Smaller items

  • s_wait_asynccnt is the one opcode still untested in both suites, and it is also the one whose declare omits immarg while its .td-identical siblings wait.asyncmark / s.wait.event carry it.
  • The new llvm23 golden sub-document is never exercised: check_golden compares only the host's autodetected flavor, which is llvm23 only on ROCm >= 7.13. Looping over GOLDEN_FLAVORS is nearly free since lowering takes the flavor explicitly.
  • permlane16 / permlane64 / s_wqm pin non-canonical mangling. LLVM 23 canonicalises these to permlane64.i32 and s.wqm.i32.i32 (two overloaded types, two suffixes). Auto-upgraded, so cosmetic — but the tests currently make the canonical form a failure.
  • _lower doesn't pin its engine: _DEFAULT_BACKEND = BACKEND_CPP with silent fallback, so this class tests C++ where the engine is built and Python where it isn't. Your own harness warns about this trap at rocke_ir_parity_harness.py:33-36.
  • "Adding llvm24 is a one-line edit" holds for the Python ladder; the C++ mirror still needs five coordinated edits plus the pybind error string, and it keeps the hand-rolled membership list the new Python SSOT lint forbids.
  • backend_details.md still documents a two-rung ladder and "both constants"; core.cpp:74 cites _LLVM23_MIN_ROCM, which this commit renamed to _ROCM_FLAVOR_LADDER; lower_llvm.py:122 cites test_llvm_flavors_enumeration_is_ssot, which doesn't exist (the test is test_no_hand_rolled_flavor_membership_lists).

FWIW the (7,13) threshold checks out — I measured ROCm 7.12 shipping clang 22.0.0git and 7.13/7.14/7.15 shipping 23.0.0git, so the "confirm on an LLVM 23 host" caveat in that comment can be dropped.

Verified correct

The LLVM_FLAVORS refactor is faithful: I extracted ll_flavor_for_rocm and diffed it against _flavor_for_rocm over 716 points including negatives and clamps — identical everywhere. Explicit-flavor validation was not weakened ('llvm2', 'LLVM23', '', 'llvm24' all still raise); the only input that newly succeeds is llvm23 on backend="cpp". "Never throw" isn't a behavior change — the old mapping was already total. The comgr.py refactor is clean, cycle-free, and the commit changed no emitted IR (45 golden cases x 3 flavors reproduce exactly).

…arity

The AMDGPU review on #9583 found the C++ global_load_async_to_lds GEP based on
the raw smem allocation name instead of the unified pool base pointer, which
references a global that is never defined, and s_prefetch_inst naming a bare
ptr for an llvm_anyptr_ty operand, which LLVM rejects for any non-flat pointer.
Both are fixed, and two more instances of the same class turned up while
covering them: av_load/store_b128 had the identical anyptr defect (the existing
tests pinned the buggy text), and buffer_load_lds_async passed the i64 that
smem_addr_of returns where its own declare says ptr addrspace(3), so it now
converts instead of relabelling.

The anyptr overloads are mangled per address space rather than left unmangled,
because a kernel touching two spaces would otherwise declare one symbol twice
with two signatures -- the same reason mov_dpp8 is mangled by type. Each engine
resolves the space through one shared helper so the mangled name, the declare,
and the call site cannot disagree.

The smoke tests asserted substrings against each engine's own output, so they
passed on IR that named *a* pointer regardless of which one; none of these bugs
were visible to them. The new target_intrinsics parity family byte-compares the
two engines over the previously untested ops, with config 0 reproducing the GEP
bug directly.

Also from the review: i16 wait immediates now raise instead of truncating
silently (in the builder and again in the lowerer, which serialized IR reaches
directly), s.wait.asynccnt declares its immarg, and the gfx1250 async-counter
gate reads a backend capability field instead of matching a gfx string prefix.
The new opcodes stay in their family groups in rocke_opcode_t; a test pins each
one's dotted name, since the real splice hazard is the two opcode-indexed
tables silently shifting rather than the enum values themselves.

Gates: 530 pytest, ctest 4/4, byte-identity GREEN across 67 families, all 7
new parity configs pass opt -passes=verify.

Co-authored-by: Cursor <cursoragent@cursor.com>
@tenpercent

Copy link
Copy Markdown
Contributor

Re-review of 5bb9126a

First, a correction to my own review. I wrote that the .pN mangling suffix was unnecessary and only the address space needed fixing. That is true for a single address space, where LLVM auto-upgrades — but it is wrong the moment one module uses the intrinsic at two spaces, because the declare table is keyed by name and unmangled declares collide. On LLVM 23:

two UNMANGLED declares, two address spaces   REJECTED: invalid redefinition of function 'llvm.amdgcn.s.prefetch.inst'
two MANGLED declares (.p1 + .p4)             ACCEPTED

So the mangling machinery is load-bearing, not gold-plating, and test_s_prefetch_inst_spaces_coexist_in_one_module is a case my review missed. Sorry for the misdirection.

Verification below ran on LLVM 23.0.0git (ROCm/llvm-project@9f1cce5a, ROCm 7.15) with both engines built from this branch. Scripts: https://gist.github.com/tenpercent/6c93e66ca548d8320d1afc8bfb58b2ff

What works

All five defects from the previous round are fixed, and the fix is surgical.

check result
the 5 previously-invalid cases all emit IR that LLVM 23 accepts
C++ vs Python byte-identical on all five, including the two-address-space case
kernels changed vs f19a5b09 exactly 5 of 30 — the five defective ones; the other 25 byte-identical
golden IR, 45 cases x 3 flavors no drift
TestNewTargetIntrinsics 35 passed under ROCKE_BACKEND=python, cpp, and both
same suite + llvm-as on every emitted module 35 passed, 0 failed

Specifics worth calling out, because several were easy to get wrong:

  • Suffix counts are right. s_prefetch_inst and av_load/store_b128 each have exactly one overloaded type, so one suffix; AMDGPUAVLoad's return is concrete llvm_v4i32_ty, so it correctly does not add a second. raw.ptr.buffer.load.async.lds and global.load.async.to.lds.* have zero overloaded types and are correctly left unmangled. LLVM 23 leaves every emitted name unchanged — the mangling is canonical, not merely tolerated.
  • Declare ordering is safe. Both engines emit declares by iterating the base table filtered by need, not in need-registration order, so on-demand .pN rows cannot reorder between engines. I went looking for a byte-identity hazard here and there isn't one.
  • The declare tables are genuinely mirrored — 148 keys, identical order, byte-identical text on both sides.
  • Address space comes from the operand's module-visible type, so the function header, the call operand, and the .pN suffix always agree; I could not construct a wrong-but-parseable mismatch.
  • inttoptr i64 ... to ptr addrspace(3) is the right conversion for buffer_load_lds_async, applied symmetrically in both engines.
  • Parity config 0 is a two-allocation kernel, which is exactly the shape that forces a non-zero pool offset and would have caught the original mem.cpp bug. Well-targeted.
  • s.wait.asynccnt gaining immarg matches AMDGPUWaitAsyncIntrinsic and brings it in line with its siblings.

New findings

1. The whitelist -> declare invariant is asserted only in a comment (medium).
Both engines document it — lower_llvm.py:940-944 and lower_llvm_internal.h:537-540 — and nothing tests it. Adding a space to a whitelist without the matching declare row emits a call with no declare at all:

call void @llvm.amdgcn.s.prefetch.inst.p3(ptr addrspace(3) %p, i32 64)
  -> error: use of undefined value '@llvm.amdgcn.s.prefetch.inst.p3'

It fails at assembly rather than miscompiling, which caps the severity, but the lowerer reports success. Two lines per engine:

for prefix, tbl in (("s.prefetch.inst", _S_PREFETCH_INST_PTR_TYPES),
                    ("av.load.b128", _AV_B128_PTR_TYPES),
                    ("av.store.b128", _AV_B128_PTR_TYPES)):
    for space in tbl:
        self.assertIn(f"{prefix}.p{space}", _INTRINSIC_DECLS)

A table-parity assertion (Python whitelist == C++ whitelist) is worth adding alongside it — the two are hand-duplicated and nothing would catch them drifting.

2. s_wait_event changed from masking to raising (medium).
ir.py:2423 now routes through _check_u16, so:

input f19a5b09 5bb9126a
-1 s.wait.event(i16 65535) ValueError
65536 s.wait.event(i16 0) ValueError

-1 is the natural spelling for "all events" and used to produce the correct 65535. Rejecting silent truncation is defensible — the .td does mark this ImmArg — but it is a source-breaking change to a public builder that arrived unrequested, with no boundary test and no note in the PR description. Either accept -1 as an alias for 0xFFFF, or keep the raise and pin the boundaries with a test plus a line in the description.

3. s_wait_asynccnt validation is arch-dependent (low).
In both engines the _check_u16 call sits after the has_async_lds_counter early return (control.cpp:1004 then :1008; lower_llvm.py:3837 then :3838). The same serialized IR with n = 70000 validates clean on gfx950 and raises on gfx1250. Nothing wrong reaches LLVM, since the early return emits no call — but it makes CI's verdict target-dependent. Hoisting the check above the early return is one line each.

4. C++ coverage of the new mangling is .p1-only (low).
future_intrinsic_lowering.cpp contains no .p4, no .p0, and no rejection-path case, and both parity emitters use only global_ptr_param. I confirmed the C++ path is correct at .p4 and for two spaces in one module — byte-identical to Python, valid IR — so this is a coverage gap rather than a defect. An 8th parity config prefetching a global and a constant pointer would cover both engines and give you cross-engine byte-parity on multi-space declare ordering for free.

5. Arena OOM in the mangled need-key silently drops the declare (low).
control.cpp:1055, crosslane.cpp:490,509 pass rocke_arena_printf(...) straight into rocke_ll_need, which returns early on a NULL key. That reproduces finding 1's failure mode from a different direction. The peer helper rocke_ll_fresh checks and calls rocke_ll_fail(ROCKE_ERR_OOM, ...); doing the same here, or failing on a NULL key inside rocke_ll_need, closes it.

6. Smaller robustness nits (low).
An unrecognized PtrType.space (e.g. a typo, or "private") silently falls back to addrspace 0, which now picks the intrinsic overload as well as the pointer text; validating space against the known set would catch it. Unrecognized addr_space override values are silently ignored rather than rejected. rocke_ll_param_llvm_type and rocke_ll_value_ptr_type are exported from the internal header but used only in core.cpp and could be static.

On the size of the change

The PR went from +2363 to +4434. Breaking that down: 72% of the added lines are tests, and the 630 lines of production code implement address-space resolution across five intrinsics in two mirrored engines, which is proportionate. The parity emitters follow house conventions, are auto-discovered by run_diff.py, and their C and Python halves are config-for-config equivalent.

Three chunks look separable, if you want the diff smaller for review:

  • The LlvmDatalayoutKind refactor (~230 lines including tests) does not fix a bug. The guard it replaces was already generation-based — _flavor_for_rocm only ever returns {llvm20, llvm22, llvm23}, so lib_modern = lib_flavor != "llvm20" already matched p8_indexed IR. The net change is a better diagnostic string. Worth keeping, but as its own PR.
  • _check_u16 is the same ~10-line body in four places (builder and lowerer, times two engines) and carries the behaviour change in finding 2.
  • test_no_hand_rolled_flavor_membership_lists (51 lines of AST walking, with an allowlist to maintain) guards the same bug as test_cpp_backend_path_accepts_every_known_flavor (32 lines, behavioural, same commit). The behavioural one is sufficient; a static guard fits better in lint config than in a unittest.

None of these is a correctness problem — the fixes themselves are clean and verified.

@tenpercent

Copy link
Copy Markdown
Contributor

Re-review of 6efd67eb

Verified on LLVM 23.0.0git (ROCm/llvm-project@9f1cce5a, ROCm 7.15), engine built from this head.

check result
full suite, ROCKE_BACKEND=python / cpp / both 277 passed, 14 skipped each
same, with ROCKE_TEST_VERIFY_IR=1 277 passed — the emitted IR genuinely assembles
golden IR, 45 cases x 3 flavors no drift
the five previously-invalid cases valid IR, byte-identical across engines

No correctness defect found at this head. Everything from the previous rounds is fixed and stays fixed.

Done well

Worth being specific, because several of these are better than what was asked for.

  • _lower now pins the native Python lowerer and cross-checks the C++ engine. This is the fix for "the same suite means two different things depending on whether the extension is built", and the docstring states the reasoning better than my review comment did.
  • _assert_ir_assembles treats a missing or too-old llvm-as as a failure, not a skip. That is the more careful choice and it is the exact trap I fell into myself earlier in this review: my first assembler check "passed" only because that LLVM did not know the intrinsics and accepted a bogus @llvm.amdgcn.* name outright. The comment naming s.prefetch.inst, av.load.b128 and raw.ptr.buffer.load.async.lds as absent from LLVM 22 is the right guard.
  • The gfx1250 parity family closes the s_wait_asynccnt gap properly — exercising it on both arches so the "emits on one, nothing on the other" contract is pinned in both directions, rather than just adding a call that silently lowers to nothing on gfx950.
  • The per-address-space mangling was the right call and I was wrong to question it. Unmangled declares collide once one module uses two spaces (invalid redefinition of function on LLVM 23), so the machinery is load-bearing.
  • Suffix counts, whitelist/declare agreement (148 mirrored keys), and declare ordering all check out; the earlier fix was surgical — exactly 5 of 30 kernels changed, the other 25 byte-identical.

One new finding

The IR-verification lane is opt-in and nothing opts in. ROCKE_TEST_VERIFY_IR is implemented and documented in AGENTS.md, but no CMake target, conftest.py, run_all.py, or CI workflow sets it — so the check never runs outside a manual invocation. I ran it with the variable set on LLVM 23 and it passes cleanly, so the lane is ready; it just is not wired to anything.

The gating itself is correct — enabling it unconditionally would false-positive on an older llvm-as. What is missing is one lane that sets it where a new-enough toolchain is known to exist, otherwise this decays into dead code the next time someone touches the emitters.

Still open from the previous round, all Low and all reasonable to defer: the whitelist -> declare invariant has no test, _check_u16 for s_wait_asynccnt sits behind the arch early-return so validation is target-dependent, and an arena OOM in the mangled need-key would silently drop a declare.

On the last commit's message

6efd67eb is titled "black format", but it is not a formatting change:

with whitespace : 31 files, 2651 insertions
ignoring ws (-w): 31 files, 2649 insertions

Two of the 2651 lines are whitespace. The rest is substantive, and 1165 lines of it are production code — a gfx1250 backend row in isa/backend.cpp, six gfx1250 WMMA lowerings in mma.cpp, ~500 lines in core.cpp, plus two new parity emitters and a new C++ test. I checked the WMMA names against IntrinsicsAMDGPU.td at amd-staging@9eb9105 and all six exist upstream, so the work looks sound — it is the label that is wrong.

This matters because of the timing: the approval landed at 14:20Z on 5bb9126a, and this commit is 21:11Z. The approval still shows as current, so as it stands 2649 lines including a new architecture's support can merge under a review that never saw them, behind a message that tells a reader there is nothing to see.

Since the PR is DIRTY and needs a rebase anyway, rewording is close to free at that point. Two concrete asks:

  1. Reword the commit to describe what it does, and add the gfx1250 WMMA work to the PR description — the description still frames this PR as LLVM 23 intrinsic lowering plus fixes.
  2. Have someone look at the 1165 production lines before merge, or split the gfx1250 enablement into its own PR. This PR is now +6971/-473 across 39 files; the gfx1250 backend and WMMA lowerings are separable from the intrinsic work and would get better scrutiny on their own.

None of that is a correctness objection — I could not break this head. It is about the reviewed surface matching the merged surface.

@tenpercent tenpercent 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

@tenpercent

Copy link
Copy Markdown
Contributor
  • the merge commit is simple python import list consolidation
  • work is protected by a disabled-by-default feature flag

@tenpercent tenpercent 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.

wait, windows build is broken

@tenpercent

Copy link
Copy Markdown
Contributor

HipDNN Windows Superbuild failure — cause and patch

core.cpp:284:12: error: unused function 'll_rocm_name_newer' [-Werror,-Wunused-function]

Repro, patch and a scan script: https://gist.github.com/tenpercent/7075fbecc7e9c2cfa2b2c55c1a1ec7ee

Cause

ll_rocm_name_newer is defined at core.cpp:284, outside any conditional. Both of its call sites are inside #ifndef _WIN32 blocks, next to the <dirent.h> / opendir code they exist to serve:

 29  #ifndef _WIN32
 30  #include <dirent.h>
...
284  static int ll_rocm_name_newer(const char* a, const char* b)     <- unguarded
...
348  #ifndef _WIN32
367      if(found && ll_rocm_name_newer(ent->d_name, best) <= 0)
382  #endif
391  #ifndef _WIN32
405      if(ll_rocm_name_newer(ent->d_name, out[i]) > 0)
417  #endif

On Windows both callers compile out, the definition survives, and -Werror,-Wunused-function fails the build. Reduced to seven lines, no ROCm or LLVM needed:

current structure, -D_WIN32   -> error: 'helper' defined but not used [-Werror=unused-function]
definition inside the guard   -> compiles clean
same, on POSIX                -> compiles clean

Patch

Wrapping the definition in its own guard is enough; the POSIX path is untouched. git apply -p1 verified against a clean checkout of 6efd67eb:

--- a/dnn-providers/hip-kernel-provider/rocke/platform/cpp/core/lower_llvm/core.cpp
+++ b/dnn-providers/hip-kernel-provider/rocke/platform/cpp/core/lower_llvm/core.cpp
@@ -277,6 +277,7 @@
     return found;
 }
 
+#ifndef _WIN32 /* POSIX-only: used solely by the dirent scans below */
 /* Python runtime_coexistence._version_key, as a comparison: compare the runs of
  * digits in each name as an integer sequence so "rocm-7.10" sorts NEWER than
  * "rocm-7.2" (a plain strcmp gets this backwards, because '1' < '2'). Returns
@@ -310,6 +311,7 @@
             return (va > vb) ? 1 : -1;
     }
 }
+#endif /* !_WIN32 */

I would avoid [[maybe_unused]] here — the function is genuinely POSIX-only, since it exists to order /opt/rocm* directory names, so the guard states that rather than suppressing the symptom.

No second one behind it

I scanned every static in core.cpp for the same shape — defined outside a _WIN32 guard, used only inside one. ll_rocm_name_newer is the only instance, and the scan reports zero after the patch. The script is in the gist if it is worth keeping around.

Provenance

56006f38  0 refs
f19a5b09  0 refs
5bb9126a  0 refs
6efd67eb  3 refs

This arrived in 6efd67eb, the commit titled "black format". That is the same commit carrying 2649 substantive lines and the gfx1250 enablement, so the Windows failure is new rather than pre-existing — and it is a concrete instance of the point in my previous comment: the commit message gives a reader no reason to look at it, and it is where the break is.

@tenpercent tenpercent 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.

Thanks!

@tenpercent

Copy link
Copy Markdown
Contributor

drifted from develop, merged changes keeping both sides of the diff

@therock-pr-bot

Copy link
Copy Markdown

Pre-commit check failed

⛔ pre-commit failed

Please run locally:

  • python -m pip install pre-commit
  • pre-commit install
  • pre-commit run --all-files --show-diff-on-failure

This repo uses .pre-commit-config.yaml.

The develop merge (45974c8) left three problems in
dnn-providers/hip-kernel-provider/rocke/platform/tests/:

1. conftest.py lost `import pytest`. The branch had it; the resolution kept
   the new `@pytest.hookimpl` coverage-gap hook but dropped the import, so
   every module under platform/tests/ failed collection with a NameError.

2. conftest.py had two whitespace-only lines between that hook and develop's
   `_lib_root` probe, which is what turned the pre-commit check red.

3. The golden's llvm23 sub-document still held 45 cases while llvm20/llvm22
   were updated to develop's 79. Since the merge also made check_golden()
   verify every flavor rather than just the host's, the 34 attention cases
   develop added were reported as drift by test_rocke_ci_static.py and the
   rocke_golden_static ctest.

Re-blessed the golden with the harness. Beyond adding the 34 missing llvm23
entries, this rehashes 14 attention/gfx942 and attention/gfx950 cases under
llvm20 and llvm22. That is expected and reviewable, not a merge artifact:
this branch adds a `declare i32 @llvm.amdgcn.ds.swizzle(i32, i32 immarg)`
that develop omitted while emitting ten calls to it (+53 bytes per affected
kernel, the declare line). LLVM's parser auto-materializes intrinsic
declarations, so develop's output still assembled and the canonical
`convergent` attribute is applied either way -- the change makes the emitted
text self-describing and consistent with every other intrinsic the emitter
uses, with no behavioral difference.

Verified: `uvx pre-commit --from-ref origin/develop` all hooks pass; harness
`--check` reports `IR parity OK [llvm20, llvm22, llvm23]`; pytest over
platform/tests/ is 516 passed, 78 skipped, 84 subtests passed. Confirmed the
14 rehashes come from this branch and not from the local environment by
regenerating on the develop-side parent, which reproduces develop's committed
golden exactly for both llvm20 and llvm22.

Generated-by: Claude Code (claude-opus-5)
@tenpercent

Copy link
Copy Markdown
Contributor

Pushed ce0328bb to get the branch green after the develop merge — flagging what's in it, since some of it goes past plain conflict resolution.

Merge fallout, straightforward: my resolution dropped import pytest from platform/tests/conftest.py while keeping the new pytest_runtest_makereport hook, so the whole test tree failed collection. Restored. Also cleaned up two whitespace-only lines, which is what the pre-commit check was catching.

Golden re-bless, worth a look: develop's #9392 added 34 attention* cases to llvm20/llvm22, but llvm23 was still at 45. Since the merge also made check_golden() verify every flavor rather than only the host's — matching what dsl_docs/ir_lowering/backend_details.md already described — those 34 showed up as drift. Re-blessed with the harness.

That re-bless also rehashes 14 attention/gfx942+gfx950 cases under llvm20 and llvm22, which I want to call out rather than bury in a hash diff. Each affected kernel grows by exactly 53 bytes — one line:

+declare i32 @llvm.amdgcn.ds.swizzle(i32, i32 immarg)

Develop emits ten calls to ds.swizzle without declaring it; this branch adds the declare. I checked the consequences: LLVM's parser auto-materializes intrinsic declarations, and the canonical convergent attribute is applied whether or not the declare is explicit — so there's no behavioral change, and no risk of the cross-lane op losing convergent. It's a genuine cleanup, it just happens to move hashes on flavors other than llvm23.

I confirmed those 14 are caused by this branch and not by my environment by regenerating the golden on the develop-side parent of the merge, which reproduces develop's committed golden exactly for both llvm20 and llvm22.

Verified: pre-commit green, IR parity OK [llvm20, llvm22, llvm23], and pytest over platform/tests/ at 516 passed / 78 skipped (GPU lanes) / 84 subtests passed.

@tenpercent
tenpercent merged commit 692ef3f into develop Jul 30, 2026
38 of 40 checks passed
@tenpercent
tenpercent deleted the users/yraparti/rocke/llvm23_flavor_support branch July 30, 2026 21:34
shumway added a commit that referenced this pull request Aug 3, 2026
… its support (#10260)

ISSUE ID : AICK-1517

## Summary

llvm23 support (#9583) shipped `_DATALAYOUT_LLVM23` as an alias of the
llvm22 layout, with a comment to re-derive it on a real LLVM-23 host and
split it out if drift was found. On a real LLVM-23 host (ROCm 7.13, AMD
clang 23.0.0git) the AMDGPU datalayout includes the ELF symbol-mangling
spec `m:e` (`e-` → `e-m:e-`) that LLVM 20/22 omit, so the aliased stub
disagrees with what the toolchain emits and the drift guard fails on
every wired arch. The stub was harmless at runtime only because the
comgr SOURCE path overrides the module datalayout; bitcode input or a
stricter verifier would reject it.

## Changes

- Split `_DATALAYOUT_LLVM23` out from the llvm22 alias with the `m:e`
spec, in lockstep across all three engine mirror sites
(`_DATALAYOUT_LLVM23`, `ROCKE_LL_DATALAYOUT_LLVM23`,
`ROCKE_DATALAYOUT_LLVM23`) plus the header, and regenerate the llvm23
golden IR hashes (only llvm23's 79 hashes change; llvm20/llvm22
untouched).
- **Add an always-run host-only test** asserting `llvm23 == llvm22 +
m:e`, so an accidental re-alias fails on the bare CI gate rather than
only on a ROCm-7.13 box with hipcc. This is the gap that let the stub
land green: byte-identity is self-consistent with the stub, and the
toolchain drift guard skips without a real LLVM-23 toolchain.
- **Add an `m:e` per-flavor pin** to the C++ flavor-datalayout SSOT
test, mirroring the existing p8 pin, so the new drift axis has the same
structural guard.
- Flip the drift-guard's pre-drift pin to assert the proven `llvm22 +
m:e` relationship (derived from the llvm22 constant, not a second copy).
- Correct datalayout comments: `p8` is the buffer-resource address space
(128-bit buffer descriptor), not the buffer fat pointer (that is `p7`);
the `LlvmDatalayoutKind` docstring now reflects that two fields drift
(p8 shape and `m:e`), with the kind grouping flavors by p8 shape /
intrinsic-declare compatibility rather than full datalayout identity.

## Local validation (real gfx1151, ROCm 7.13 / clang 23)

- Full C++ build: clean, 0 errors/warnings, all targets incl. the
`rocke_engine` pybind extension.
- ctest: 5/5 passed, including the new `rocke_flavor_datalayout_ssot`
`m:e` pin.
- Byte-identity gate: GREEN — 68 families, 0 divergences (C++ ↔ Python
`.ll` byte-identical, incl. the llvm23 `m:e` path).
- pytest (single-engine and `ROCKE_BACKEND=both` differential): 546
passed, 32 skipped (all torch-gated, expected in the torch-free venv).
- The llvm23 drift guard (`test_datalayout_matches_hipcc_emitted_ir`)
now passes against the installed toolchain on every wired arch.

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

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cgmillette added a commit that referenced this pull request Oct 20, 2026
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>
cgmillette added a commit that referenced this pull request Oct 26, 2026
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>
cgmillette added a commit that referenced this pull request Oct 26, 2026
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>
cgmillette added a commit that referenced this pull request Oct 27, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants