Repository navigation
[rocjtisu] Fix isa semantics for rdna3 and rdna4 - #7646
Merged
Merged
Conversation
kuhar
force-pushed
the
users/kuhar/rdna3-rdna4-isa-split
branch
from
June 23, 2026 01:43
9b189f8 to
7756d4a
Compare
atgutier
approved these changes
Jun 23, 2026
atgutier
left a comment
atgutier
left a comment
Contributor
There was a problem hiding this comment.
This LGTM, but should probably fix the dot2_true16 dispatch entry problem.
Here are some other agent comments:
- Systemic copy-paste of per-arch operand logic (MINOR, the main maintainability risk)
packed_16bit_vgpr_source/dst, the Packed16VgprSource struct, resolve_src_scalar16, the half-merge
read_lane/write_lane, and the VCCZ/EXECZ active-mask fix are byte-for-byte identical across
rdna3/rdna3_5/rdna4/gfx1250 (and resolve_src_scalar16 across cdna1–4 too — 7+ files). Confirmed via
diff. This violates the project's stated style (factor reuse into templates+using), and a future fix
to the true16 half-mapping would have to land in N places — real drift risk.
→ Hoist into a shared CRTP/base in shared/ (e.g. extend AmdgpuIsaOperand), parameterized by the
per-ISA inline-constant table.
Lower priority
- packed.py:561-571 (+628, +700) (MINOR) — read_mix_src f16 path only treats selectors 240–248 as
float inline constants; integer inline constants fed to an f16 mix get reinterpreted as raw f16
bits. Nonsensical-but-legal encoding, low impact. Assert or document. - Wait-counter update inconsistency (MINOR) — _generator.py:2682-2691 emits lgkmcnt via a fragile
read-modify-write of the combined set_wait_target(...) while the other three counters use
single-field setters. Verified functionally correct, but a dedicated set_wait_target_lgkmcnt() would
be cleaner. Mirror in rdna4/sopk.cpp etc. where expcnt uses the string-keyed
set_wait_counter("wait_expcnt", …) vs typed siblings. - addr_calc_buffer.h:129 (MINOR, pre-existing) — static uint64_t pl_count in the MUBUF log block is
non-thread_local; benign race across SIMD threads. Not a regression (this PR only added constexpr
helpers nearby). Make thread_local while in the area, like mma_exec.h:895. - read_lane/write_lane half-select asymmetry (NIT) — write_lane calls packed_16bit_vgpr_dst
unconditionally while read_lane gates on packed_16bit_source_. Currently unreachable (generated
true16 ops always pass the flag or merge manually), but a foot-gun. Gate symmetrically or assert. - rdna4/operand.cpp:561-566 OPR_WAITCNT decode (NIT) — the combined S_WAITCNT is ignored on gfx12
(separated into s_wait_loadcnt/dscnt/expcnt). The new vmcnt/expcnt/lgkmcnt bit-decode is disassembly
cosmetics only, not a semantic fix. Optionally render as s_wait_idle/"(ignored)".
kuhar
changed the title
Update the AMDISA generator, generated RDNA/CDNA execution sources, shared executor helpers, and ISA-focused tests for the RDNA3/RDNA4 kernels exercised by IREE and hip-fpsan. Add generated RDNA3/RDNA4 execution coverage for packed operations, DPP/true16 handling, VOP3P FP8 paths, matrix dispatch, and address wrapping. Refresh the generated gfx1250 ISA split and DBT legalization tables from the same generator state so the checked-in generated outputs remain reproducible.
kuhar
force-pushed
the
users/kuhar/rdna3-rdna4-isa-split
branch
from
June 23, 2026 17:43
7756d4a to
6852fe2
Compare
yashjha123
added a commit
that referenced
this pull request
Oct 26, 2026
RDNA scratch addressed each lane's private space contiguously (scratch_base + lane * lane_size + offset). The RDNA3 and RDNA3.5 ISA guides (11.2) define scratch as dword-swizzled with index_stride equal to the wave size: Addr = SCRATCH_BASE + (offset / 4) * 4 * index_stride + offset % 4 + TID * 4 The RDNA4 guide uses the same SWIZZLE(offset, ThreadID) wording, and rocm-dbgapi reads gfx10/11/12 private memory with this 4-byte interleave. CDNA already uses this layout. RDNA's per-lane layout came from #7646, which copied the CDNA convention of the time, and #13019 named it FlatPrivateLayout::Linear to preserve that behavior. Map SCRATCH instructions and private-aperture FLAT lanes through scratch_lane_address(), and mark them as interleaved private lanes so multi-dword accesses split per dword. FLAT wait planning uses the same layout. A spill where every lane uses the same offset now touches one contiguous line instead of one line per lane. FlatPrivateLayout::Linear has no remaining users. Remove it together with FlatAddress::swizzled, which every remaining translation set with private_address; the wait scoreboard now reads private_address. Interleaving moves a lane's high dword a wave stride away, so 8-byte FLAT atomics on private lanes would modify the neighbouring lane's dword. RDNA3/3.5/4 section 11.1 specifies MEMVIOL for them; extend the existing CDNA5 rejection to these targets. Run the layout test on RDNA3 and RDNA3.5 for wave32 and wave64, check mixed private/global FLAT lanes on RDNA3, RDNA3.5 and RDNA4, and update the expectations that encoded RDNA4's per-lane backing. Run the decoded FLAT wait-plan cross-check with b128 loads and returning 32-bit atomics too, so private lanes that split per dword or take the atomic handlers keep the routing execution uses.
yashjha123
added a commit
that referenced
this pull request
Oct 28, 2026
RDNA scratch addressed each lane's private space contiguously (scratch_base + lane * lane_size + offset). The RDNA3 and RDNA3.5 ISA guides (11.2) define scratch as dword-swizzled with index_stride equal to the wave size: Addr = SCRATCH_BASE + (offset / 4) * 4 * index_stride + offset % 4 + TID * 4 The RDNA4 guide uses the same SWIZZLE(offset, ThreadID) wording, and rocm-dbgapi reads gfx10/11/12 private memory with this 4-byte interleave. CDNA already uses this layout. RDNA's per-lane layout came from #7646, which copied the CDNA convention of the time, and #13019 named it FlatPrivateLayout::Linear to preserve that behavior. Map SCRATCH instructions and private-aperture FLAT lanes through scratch_lane_address(), and mark them as interleaved private lanes so multi-dword accesses split per dword. FLAT wait planning uses the same layout. A spill where every lane uses the same offset now touches one contiguous line instead of one line per lane. FlatPrivateLayout::Linear has no remaining users. Remove it together with FlatAddress::swizzled, which every remaining translation set with private_address; the wait scoreboard now reads private_address. Interleaving moves a lane's high dword a wave stride away, so 8-byte FLAT atomics on private lanes would modify the neighbouring lane's dword. RDNA3/3.5/4 section 11.1 specifies MEMVIOL for them; extend the existing CDNA5 rejection to these targets. Run the layout test on RDNA3 and RDNA3.5 for wave32 and wave64, check mixed private/global FLAT lanes on RDNA3, RDNA3.5 and RDNA4, and update the expectations that encoded RDNA4's per-lane backing. Run the decoded FLAT wait-plan cross-check with b128 loads and returning 32-bit atomics too, so private lanes that split per dword or take the atomic handlers keep the routing execution uses.
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
This extends RocJITsu’s generated AMDGPU ISA support so RDNA3/RDNA3.5/RDNA4 simulation can execute the instruction patterns needed by IREE e2e tests and the fpsan numeric suite.
The bulk of the diff is regenerated ISA output. The source changes are concentrated in the Python generator/profile logic, shared execution helpers, and regression tests.
This needs #7625 to work e2e.
Technical Details
Add generator-backed execution coverage for RDNA packed/vector paths:
op_selhandlingAdd RDNA WMMA/SWMMAC dispatch support:
Fix RDNA4 compatibility behavior:
s_waitcntcompatibility form where the XML only exposes split wait instructionsRefresh generated outputs from the same generator state:
Expand regression coverage:
JIRA ID
N/A
Test Plan
Unit tests, IREE e2e tests, fpsan tests across rdan3, rdna4, and gfx1250.
Rerun the generator across all ISAs.
Test Result
All good with the runtime patch.
Submission Checklist