Repository navigation
[rocjitsu] Apply async IOFFSET to LDS addresses - #10199
Conversation
✅ 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. |
There was a problem hiding this comment.
Pull request overview
This PR updates rocJITsu’s CDNA5 async-to-LDS address semantics so the signed 24-bit IOFFSET is applied consistently to both global and LDS sides for async loads/stores (including cluster variants), while also hardening LDS bounds/overflow handling and ensuring invalid lanes are dropped from writeback and race detection.
Changes:
- Apply signed 24-bit
IOFFSETuniformly via a shared helper (signed_ioffset) and compute async LDS destinations with allocation-bounds validation (async_lds_lane_address). - Introduce and propagate an explicit per-workgroup LDS allocation size on
Wavefront(lds_size) so address validation can reject out-of-range destinations. - Widen/validate cluster LDS multicast address computations and drop invalid/out-of-range per-lane targets; extend unit/integration/plugin/codegen tests for offsets, wraparound, and lane dropping.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| emulation/rocjitsu/tests/execution_plugin_test.cpp | Adds plugin coverage ensuring dropped async LDS lanes do not create spurious low-address races. |
| emulation/rocjitsu/tests/cdna5_instruction_test.cpp | Ensures instruction tests initialize Wavefront LDS allocation size used by new address validation. |
| emulation/rocjitsu/tests/cdna5_dispatch_memory_test.cpp | Adds LDS wraparound-range rejection test exercising overflow-safe bounds checks. |
| emulation/rocjitsu/tests/cdna5_cluster_test.cpp | Expands decode/execute/simulation tests for signed IOFFSET, bounds rejection, and cluster multicast dropping/overflow handling. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/vm/plugins/race_detector/plugin.cpp | Filters invalid LDS lane addresses from race detector lane masks. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/vm/amdgpu/wavefront.h | Adds lds_size tracking on wavefronts for per-workgroup LDS allocation bounds. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/vm/amdgpu/spi.h | Initializes Wavefront::lds_size during dispatch. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/vm/amdgpu/memory_pipeline.cpp | Skips writeback and improves trace output for invalid per-lane LDS destinations. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/vm/amdgpu/lds.h | Introduces kInvalidLdsAddress sentinel and overflow-safe bounds checking helpers. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/vm/amdgpu/command_processor.cpp | Initializes Wavefront::lds_size for CP-dispatched workgroups. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/vm/amdgpu/cluster_lds_multicast.h | Widens remap/lane-address helpers to uint64_t to detect/handle overflow. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/vm/amdgpu/cluster_lds_multicast.cpp | Drops invalid/out-of-range per-lane multicast targets and validates widened addresses. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna5/vglobal_exec.cpp | Switches to signed_ioffset and uses async_lds_lane_address for async LDS operands. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/cdna5/addr_calc.h | Adds signed_ioffset and declares async_lds_lane_address. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/cdna5/addr_calc.cpp | Implements allocation-bounded async LDS address computation; reuses signed_ioffset broadly. |
| emulation/rocjitsu/lib/python/amdisa/tests/test_generator_profile_gates.py | Adds generator test assertions ensuring new helpers are emitted in generated bodies. |
| emulation/rocjitsu/lib/python/amdisa/codegen/_generator.py | Updates codegen to emit async_lds_lane_address and signed_ioffset usage and revised comments. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
4b77b8d to
0a9b79a
Compare
Apply the signed 24-bit IOFFSET to both the global and LDS addresses for ordinary and cluster async loads and async stores, matching CDNA5 section 10.8.1 and expressions 95-102 and 106-109. Reject LDS destinations outside the issuing workgroup allocation, keep cluster remapping widened through validation, and filter invalid lanes from writeback and race detection. Add generator, execution, multicast, and plugin coverage for signed offsets, allocation bounds, and wraparound addresses. Fixes #8829
atgutier
left a comment
There was a problem hiding this comment.
LGTM. Just one comment that could harden our bounds check.
0a9b79a to
becb955
Compare
Apply the signed 24-bit IOFFSET to both the global and LDS addresses for ordinary and cluster async loads and async stores, matching CDNA5 section 10.8.1 and expressions 95-102 and 106-109. Reject LDS destinations outside the issuing workgroup allocation, keep cluster remapping widened through validation, and filter invalid lanes from writeback and race detection. Add generator, execution, multicast, and plugin coverage for signed offsets, allocation bounds, and wraparound addresses. Fixes #8829 (cherry picked from commit be1f919)
…t calls (#10444) ## Motivation hipTensor and rocThrust gfx1250 objects are refused at code-object load time with "indirect branch or call target recovery is not implemented for relocated kernel text" on `s_swap_pc_i64`. A refused object leaves HIP holding an invalid kernel handle, so the failure surfaces as a segfault rather than a diagnostic. The fix is upstream as #10376; this backports it to `release/bkc/therock-10.1-20260811`. ## Commits Cherry-Picked | # | Upstream | PR | Backport | Subject | |---|----------|-----|----------|---------| | 1 | `b6d838028e` | #10197 | `846fb16d12` | [rocjitsu][docs] Refresh architecture support documentation | | 2 | `5c8e33e391` | #10200 | `db5c58b1be` | [rocjitsu] Define standalone decoder ownership | | 3 | `be1f91998f` | #10199 | `ed102baf1c` | [rocjitsu] Apply async IOFFSET to LDS addresses | | 4 | `fda635e5b4` | — | `ad32ff7c18` | [rocjitsu] Extract per-scope rollback and object-level checks from translate() | | 5 | `8f75eb781e` | — | `4c0cb05207` | [rocjitsu] Emit leading V_NOPs ahead of a gfx1250 MODE write | | 6 | `00aa1e260f` | #9941 | `ded43b1787` | fix(rocjitsu): disable zero-count TDM descriptors | | 7 | `c810647609` | #10192 | `50ca7f1732` | [rocjitsu] Apply MUBUF LDS-load instruction offset to the LDS address | | 8 | `1d7866269d` | #9560 | `2e6bf26943` | [rocjitsu] code rewrite condition validator | | 9 | `95b6c410bb` | #10175 | `8418901076` | [rocjitsu] Fix ds_read_b64_tr_b16 lane layout in gfx950 simulator | | 10 | `bfaebcfb5c` | #10240 | `c4eac1e1f7` | [rocjitsu][util] Add result and diagnostic utilities | | 11 | `dbb7221fa7` | #9775 | `cdd0993a5f` | fix(rocjitsu): route tensor DMA through wave VMID | | 12 | `2779247222` | #9779 | `7bfeba6f5d` | feat(rocjitsu): Lazy vgpr allocation system | | 13 | `a659172a31` | #9936 | `180cf00325` | fix(rocjitsu): honor TDM tensor bounds across iterations | | 14 | `60c29efb39` | #10204 | `370bfb9796` | [rocjitsu] Fix gfx950 decoder extension handling | | 15 | `c1dba564e7` | #10345 | `536110a083` | [rocjitsu] Model gfx1250 wave ABI state | | 16 | `7f71be9f32` | #10346 | `3f922ad5f2` | [rocjitsu] Model gfx1250 barrier synchronization | | 17 | `bea7ac8937` | #10368 | `7b5ecf6c7b` | [rocjitsu] Replace decoder exceptions with results | | 18 | `f2e878694e` | #10371 | `8cf750d23f` | [rocjitsu] Dispatch operand validation by type | | 19 | `dd619bed77` | #9735 | `1a612c47c0` | fix(rocjitsu): respect same-wave LDS ordering | | 20 | `c5fae8d2b2` | #10137 | `4360c4b978` | [rocjitsu] Rename gfx1250 to cdna5 for enums, configs, amdisa codegen helpers and generated symbols. (NFC) | | 21 | `b05a587f89` | #10274 | `bfb5f9fab0` | [rocjitsu] Align CDNA4 MI350X configs with captured topology | | 22 | `f81f9d40db` | #10362 | `8177b5725c` | feat(rocjitsu): cdna5 OOB behavior implementation | | 23 | `53a939bd00` | #10283 | `141b109432` | [rocjitsu] add per-XCD workgroup dispatch observability | | 24 | `a0fb11bc17` | #10414 | `6bb4c5a959` | [rocjitsu] Refactor test encoding handling in execution tests (NFC) | | 25 | `e5109e315e` | #9844 | `322ac26b44` | feat(emulation): debug emulated GPU kernels with real ROCgdb | | 26 | `efa24f23c9` | — | `c3c9641073` | rocjitsu: Point the XCD distribution test at the config that exists | | 27 | `8b7763ef5e` | #10376 | `c3f419555a` | [rocjitsu] Translate gfx1250 lane-banked and kernarg-supplied indirect calls | ## Issue Tracking [ROCM-29436] ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. [ROCM-29346]: https://amd-hub.atlassian.net/browse/ROCM-29346?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [ROCM-29436]: https://amd-hub.atlassian.net/browse/ROCM-29436?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Signed-off-by: Tony Gutierrez <anthony.gutierrez@amd.com> Signed-off-by: hanhanW <hanhan0912@gmail.com> Co-authored-by: Jakub Kuderski <jakub@nod-labs.com> Co-authored-by: James Newling <james.newling@gmail.com> Co-authored-by: Fei Kou <feikou@meta.com> Co-authored-by: Austin Lu <austin.lu@amd.com> Co-authored-by: Fei Kou <feikou@anthropic.com> Co-authored-by: Erick Ochoa Lopez <eochoalo@amd.com> Co-authored-by: Han-Chung Wang <hanhan0912@gmail.com> Co-authored-by: Rosa, Ada <arosa@amd.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Apply the signed 24-bit IOFFSET to both the global and LDS addresses for ordinary and cluster async loads and async stores, matching CDNA5 section 10.8.1 and expressions 95-102 and 106-109.
Reject LDS destinations outside the issuing workgroup allocation, keep cluster remapping widened through validation, and filter invalid lanes from writeback and race detection.
Add generator, execution, multicast, and plugin coverage for signed offsets, allocation bounds, and wraparound addresses.
Fixes #8829