Repository navigation
refactor(rocke): read gfx1250-only checks from target facts in both engines - #14040
Merged
cgmillette merged 3 commits intoOct 21, 2026
Merged
Conversation
…facts Both engines refused the tensor data mover, workgroup-cluster, multicast, data-prefetch, split and named barrier, s_setreg, transposed global load and asynchronous LDS-to-global store operations unless the target's name was gfx1250. They now read the fact the target's row records instead: - memory.has_tdm: tensor_load_to_lds, tensor_store_from_lds, s_wait_tensorcnt - memory.has_cluster_launch: cluster_dims, the cluster id reads, cluster_barrier - memory.has_multicast_load: cluster_load, cluster_load_async_to_lds - memory.has_scalar_data_prefetch: s_prefetch_data, s_buffer_prefetch_data - memory.has_global_prefetch: global_prefetch, flat_prefetch - target_family gfx12_cdna: the split and named barriers, s_setreg, global_load_tr16_b128, global_store_async_from_lds (no flag records these) The Python engine names the facts in _OpTargetFact and the C++ engine in rocke_ll_target_fact_t; one shared C++ helper replaces the three copies of the name check. The refusal now reads "<op> requires a target with <fact>, got <target>", identically in both engines. The llvm23 flavor requirement is unchanged. Every target that lowered these operations still does, so no emitted IR changes. A new test lowers each operation on every known target with both engines: targets that record the fact must produce identical IR, all others must be refused by both with the same message. Co-Authored-By: Claude <noreply@anthropic.com>
… a target name The scaled WMMA (SCALE/SCALE16) support read gfx1250's row by name in four places. Each now reads the catalog data instead: - The MMA row loader admits the scaled-family declaration by its content on any arch's row; it only accepted it on gfx1250's. - Scaled fragment shapes are registered for the family's atoms in every arch's catalog, not only gfx1250's. - An op_id resolves to its scaled operand contract through a target-free lookup over all catalogs (_mma_op_by_id in Python, rocke_arch_mma_op_by_id in C++), first hit in known-arch order, as the existing op_id lookups do. - LLVM lowering admits a scaled WMMA op on a target whose MMA catalog lists its atom. The C++ engine checked the target name; the Python engine only reached the emitter through the gfx1250 backend class. The emitter moves to the ISA backend base class and both engines refuse with the same message, "<op> is not available on <target>: its MMA catalog does not list it". Only gfx1250 lists these atoms today, so no emitted IR changes. Tests cover the declaration on another arch's row, the target-free lookup in both engines, and scaled WMMA lowering on every known target with both engines. Co-Authored-By: Claude <noreply@anthropic.com>
The C++ HIP lowerer refused a scaled WMMA op unless the target's name was gfx1250, while the Python HIP lowerer already asked whether the target's MMA catalog lists the atom. The C++ lowerer now applies the catalog rule too and refuses with the Python message, so both engines admit and refuse the same targets with the same text. Only gfx1250 lists these atoms, so the emitted HIP source does not change. A test lowers the scaled atoms to HIP on every known target with both engines. Co-Authored-By: Claude <noreply@anthropic.com>
github-actions
Bot
added
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. |
cgmillette
enabled auto-merge (squash)
October 21, 2026 00:10
CongMa13
approved these changes
Oct 21, 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.
cgmillette
deleted the
users/cgmillette/rocke/gfx1250-checks-from-facts
branch
October 21, 2026 02:35
1 task
Contributor
TheRock Submodule Bump ActivityNewest first
|
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.
Motivation
Both engines decided whether a target may use the tensor data mover, workgroup clusters, multicast loads, data prefetch, the split and named barriers and the scaled WMMA atoms by comparing the target's name with "gfx1250". The target's row in
arch_specs.jsonalready records these facts (memory.has_tdm,memory.has_cluster_launch, ...; the MMA catalog lists the scaled atoms), so a second target with the same features would be refused by code that never looks at them. Some of the checks were also spelled differently in the two engines: scaled WMMA was refused by name in C++ and by backend class in Python, with different messages.Each check now reads one recorded fact, with the same rule and the same message in both engines.
ISSUE ID : AICK-2661
Technical Details
Three commits, grouped by area.
memory.has_tdm(tensor load/store,s_wait_tensorcnt),memory.has_cluster_launch(cluster_dims, cluster id reads,cluster_barrier),memory.has_multicast_load(cluster_load,cluster_load_async_to_lds),memory.has_scalar_data_prefetch,memory.has_global_prefetch. Operations no flag records (split and named barriers,s_setreg,global_load_tr16_b128,global_store_async_from_lds) usetarget_family gfx12_cdna._OpTargetFactand C++rocke_ll_target_fact_tname the facts; one shared C++ helper replaces three copies of the name check. The refusal reads"<op> requires a target with <fact>, got <target>"in both engines. The llvm23 requirement is unchanged._mma_op_by_id/rocke_arch_mma_op_by_id), first hit in known-arch order like the existing op_id lookups."<op> is not available on <target>: its MMA catalog does not list it".Only gfx1250 records these facts and lists these atoms, so every target accepts and refuses exactly what it did before and no emitted IR or HIP source changes.
Not changed: the inline-asm scalar controls (
s_delay_alu,s_wait_alu,s_clause,s_wait_xcnt) recordrequired_arch: "gfx1250"in the IR itself; expressing that as a fact would change the IR. The backend registries still map each target name to its backend, as for every target.Test Plan
test_target_fact_gates.py: every gated operation on every known target with both engines: identical IR where the target records the fact, the same refusal from both engines elsewhere. Also checks that each fact admits on some target and refuses on another, and lowers the scaled atoms to LLVM IR and to HIP on every known target with both engines.future_intrinsic_loweringcases.test_arch_mma_ssot.pyand C++test_mma_catalog_lookup.cpp: the scaled-family declaration is accepted on another arch's row; the target-free lookup finds every catalog atom.Test Result
Submission Checklist
🤖 Generated with Claude Code