Repository navigation
test(rocke): qualify rocKE LLVM 24 lane - #14692
Merged
Merged
Conversation
github-actions
Bot
added
documentation
project: hip-kernel-provider
rocke: platform
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
shumway
force-pushed
the
users/shumway/aick-2742-llvm24-artifacts
branch
from
October 29, 2026 21:47
80a6329 to
94cfd94
Compare
Contributor
|
Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max PR #14692 — qualify rocKE LLVM 24 lane (shumway, AICK-2742) — 🔴 First-round: blocking — CI is red from this PR's own artifacts: seven library goldens and six llvm24 differential hashes were not re-blessed, and the LLVM 24 arch-domain column was measured with a different compiler than CI's (N1, N2); the two engines match at llvm24
Utility: 🟢 Positive — real LLVM 24 lane, engines byte-identical; goldens and column need refreshing. Legend: 🔴 blocking · 🟠 should-fix · 🟡 minor nit · 🟢 info/good · ⚪ retracted First-round review: a local qwen3.6 model made the independent first pass, and the orchestrator (Claude Opus 5.5) verified it. On a clean merge with develop, it ran the native build and ctest, byte identity (adding C++ vs Python at llvm24 for three families), both golden gates, a targeted llvm24 |
iq136boy
approved these changes
Oct 30, 2026
shumway
force-pushed
the
users/shumway/aick-2742-llvm24-artifacts
branch
from
October 30, 2026 15:19
94cfd94 to
f1795d5
Compare
github-actions
Bot
added
the
rocke-lib: attention
Contributor
Author
|
Addressed the blocking review findings and separated the work into three commits:
The reproducibility CTest no longer assumes that all development builds with the same LLVM major have identical intrinsic availability. It performs the exhaustive cell-for-cell comparison when compiler provenance matches. A different compiler build now reports Validation:
Broader cross-revision LLVM qualification, runtime ABI probing, and declaration-table composition remain deferred and are not included in this PR. |
Contributor
|
Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max PR #14692 — qualify rocKE LLVM 24 lane (shumway, AICK-2742) — 🟠 Follow-up: blockers fixed, rebase needs re-bless
Utility: 🟢 Positive — qualifies LLVM 24 with engine parity; rebase must re-record llvm24 artifacts. Legend: 🔴 blocking · 🟠 should-fix · 🟡 minor nit · 🟢 info/good · ⚪ retracted Follow-up review: first pass by a local qwen3.6 model, verified by the orchestrator (Claude Opus 5.5). Checks: range-diff vs |
Contributor
|
I reviewed this PR again. The following review threads are still unresolved, or comments have no author reply: If you have already fixed these, please reply on each comment to say so and resolve the threads. |
Contributor
Author
|
Follow-up review findings N1 and N2 are addressed by merge commit
Validation after merging
N3 remains intentionally deferred: this PR qualifies the recorded LLVM 24 development lane and does not claim compatibility with every LLVM 24 development snapshot. Runtime ABI probing remains outside this PR’s scope. There are currently no unresolved inline review threads. A fresh approval is needed because the previous approvals predate the updated commits. |
cgmillette
enabled auto-merge (squash)
October 30, 2026 17:15
CongMa13
approved these changes
Oct 30, 2026
CongMa13
left a comment
CongMa13
left a comment
Contributor
There was a problem hiding this comment.
No new P0 or P1 issues were identified during this review. The changes are approved.
Contributor
Author
|
Addressed the blocking review findings and separated the work into three commits:
The reproducibility CTest no longer assumes that all development builds with the same LLVM major have identical intrinsic availability. It performs the exhaustive cell-for-cell comparison when compiler provenance matches. A different compiler build now reports Validation:
Broader cross-revision LLVM qualification, runtime ABI probing, and declaration-table composition remain deferred and are not included in this PR. |
Contributor
|
Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max PR #14692 — qualify rocKE LLVM 24 lane (shumway, AICK-2742) — 🟡 Follow-up: N1–N2 fixed, CI pending
Net-new: none. Utility: 🟢 Positive — complete llvm24 artifacts on the merge; every golden gate passes locally. Legend: 🔴 blocking · 🟠 should-fix · 🟡 minor nit · 🟢 info/good · ⚪ retracted Follow-up review: first pass by a local qwen3.6 model, verified by the orchestrator (Claude Opus 5.5). Checks: merge-tree vs develop, representative and differential goldens at four flavors, arch-domain gate, targeted pytest, rules scan, CI at head. Flags whether a closer look is needed; not an approval or rejection. |
shumway
changed the title
AnaghaRaoAMD
approved these changes
Oct 30, 2026
cgmillette
previously requested changes
Oct 30, 2026
cgmillette
left a comment
cgmillette
left a comment
Contributor
There was a problem hiding this comment.
Automated merge-queue hold (rocKE merge monitor)
Hi @shumway, this PR is held in the rocKE merge queue: its LLVM 24 representative goldens are incomplete, so the installed golden gate fails at its head and on a merge with today's develop (ca118033). Develop itself passes, because it still aliases llvm24 to llvm23.
What fails: platform/tests/rocke_installed_golden_test.py reports FAIL for the llvm24 flavor.
- At the head
50dc1852alone: 5 cases have no llvm24 entry. They are the gfx1250 depthwise dgrad cases that #14386 added at this branch's merge base (43d958ba):conv_direct/gfx1250/dw_dgrad_stream_k3_w32,dw_dgrad_stream_k3_cpl2_w32anddw_dgrad_stream_k5_s2_bf16_cpl8_w32, andconv_direct_depthwise_tiled/gfx1250/dgrad_bf16_k31_w32anddgrad_fp16_k51x5_w32. For example,conv_direct.jsonhas 30 cases for llvm20, llvm22 and llvm23 but 27 for llvm24. - On the merge with develop: 29 more llvm24 cases in
attention_d256andattention_dense(5 new, 24 changed). #14367 (landed 10-30 20:44Z) changed those kernels and re-recorded the two shards for llvm20, llvm22 and llvm23 only.
Fix: rebase onto develop, re-record the llvm24 entries of every shard on the rebased tree (the same re-record this PR already does), and check that rocke_installed_golden_test.py passes.
Once it's fixed, push and re-request a review from @cgmillette. A held PR needs that explicit re-approval before it can merge.
cgmillette
disabled auto-merge
October 30, 2026 21:51
Contributor
|
Automated merge-queue status (rocKE merge monitor) Hi @shumway, I turned auto-merge off on this PR for now. Before a rocKE PR lands, the rocKE merge monitor checks it on top of today's develop branch, and this version would not land cleanly:
What to do: re-record (re-bless) the affected golden files on top of the current develop branch. After that, turn auto-merge back on; the monitor re-checks every push. Nothing else changed: the monitor does not approve, reject or merge PRs. It only turned auto-merge off to keep develop green. This comment is updated in place. |
shumway
force-pushed
the
users/shumway/aick-2742-llvm24-artifacts
branch
from
October 31, 2026 02:31
50dc185 to
d1e6aee
Compare
Contributor
Author
|
Rebased onto current Addressed the merge-monitor and follow-up review findings:
Validation on the rebased tree:
Exact arch-domain reproduction remains provenance-bound. This host has a different LLVM 24 build, so it correctly reports the lane as unvalidated rather than claiming reproduction. |
shumway
force-pushed
the
users/shumway/aick-2742-llvm24-artifacts
branch
from
October 31, 2026 03:02
d1e6aee to
4bfa24e
Compare
shumway
dismissed
cgmillette’s stale review
October 31, 2026 03:09
This requested change has already been included in my follow up commmits.
Contributor
|
Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max PR #14692 — qualify rocKE LLVM 24 lane (shumway, AICK-2742) — 🔴 Follow-up: hold fixed, new llvm24 gap
Utility: 🟢 Positive — measured LLVM 24 lane with engine parity; one more re-record needed. Legend: 🔴 blocking · 🟠 should-fix · 🟡 minor nit · 🟢 info/good · ⚪ retracted Follow-up review: first pass by a local qwen3.6 model, verified by the orchestrator (Claude Opus 5.5). Checks: history map vs |
2 of 3 tasks
Contributor
|
Automated develop-health notice (rocKE merge monitor) A follow-up for this PR, merged as
From python tests/instances/rocke_ir_parity_harness.py --write tests/golden/rocke_representative_ir_sha256Please fix forward with a small PR that re-records these entries and references #14815. |
shumway
added a commit
that referenced
this pull request
Oct 31, 2026
## Summary Fix forward the rocKE installed golden failure introduced when the LLVM 24 representative goldens landed without the attention LSE cases subsequently merged into `develop`. This re-records only the LLVM 24 attention entries. Reverting the LLVM 24 qualification would remove intended test coverage, so updating the stale golden is the safer fix-forward. ## Risk Assessment Risk level 1. This changes generated regression-test metadata only; it does not change kernel emission or product behavior. LLVM 20, LLVM 22, and LLVM 23 entries are unchanged. ## Related - Work tracking: #14815 - Related PRs: #14692, #14616 Fixes #14815 ## Device / Architecture Coverage No GPU validation is required because this is a host-only representative-IR golden update. The affected cases cover `gfx942` and `gfx950`, and the installed golden gate evaluates every recorded LLVM flavor without requiring local hardware. ## Testing Summary - Reproduced by the failure recorded in #14815. - Confirmed the refreshed installed golden passes for LLVM 20, LLVM 22, LLVM 23, and LLVM 24. - Confirmed the diff contains only six new LLVM 24 LSE cases and two refreshed LLVM 24 `reduce_lse` entries. ## Testing Checklist - [x] Installed representative-IR golden gate — `cd platform && PYTHONPATH="$PWD/python:$PWD/../library" python tests/rocke_installed_golden_test.py` — Status: Passed - [x] Golden flavor-diff audit — LLVM 20–23 unchanged; LLVM 24 has six additions and two expected updates - [ ] PR CI — GitHub PR checks — Status: Pending ## Flags / Guardrails None. This does not alter runtime behavior. ## Adjacent Tests Considered The byte-identity and GPU numeric lanes are not required because emitted IR and kernel behavior are unchanged. The installed representative-IR golden gate is the lowest-level test that directly reproduces and validates this regression. ## Technical Changes - Add the six missing LLVM 24 attention LSE representative cases for `gfx942` and `gfx950`. - Refresh the two stale LLVM 24 `reduce_lse` hashes for `gfx942` and `gfx950`. - Preserve all LLVM 20, LLVM 22, and LLVM 23 golden entries unchanged.
1 task done
AviralGoelAMD
added a commit
to Parthiv911/rocm-libraries
that referenced
this pull request
Nov 1, 2026
Resolve conflict in attention_dense_gfx942_ir_sha256.json: develop (ROCm#14692) qualified the LLVM 24 lane (aliases {} + explicit llvm24 hashes). Re-blessed with test_attention_dense_gfx942_golden.py --write: 0 existing hashes changed; adds llvm24 entries for the 12 KDB/VPF cases this PR introduced.
Merged
1 task done
10 tasks done
Contributor
TheRock Submodule Bump ActivityNewest first
|
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>
7 of 8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
addrspace(15)declarations and callsQualification found that LLVM 24 retains LLVM 23’s datalayout and target-operation surface, but changes the four named-barrier pointer declarations from
addrspace(3)toaddrspace(15). The Python and C++ engines insert matching address-space casts and remain byte-identical.Existing LLVM 20, LLVM 22, and LLVM 23 golden sections are unchanged.
ISSUE ID : AICK-2742
Test plan
python -m pytest platform/tests -q— 4638 passed, 1800 skippedFollow-up
The C++ declaration tables remain flattened to preserve the existing resolver structure. Composing declaration deltas instead of duplicating inherited entries is tracked separately in AICK-2857.