Repository navigation
[rocjitsu] Apply MUBUF LDS-load instruction offset to the LDS address - #10192
Merged
Merged
Conversation
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
atgutier
reviewed
Aug 15, 2026
Contributor
There was a problem hiding this comment.
Thanks for the fix!
Everything with the fix to the generator and resulting C++ is in working order.
Can you remove the tests for this PR? We prefer not to grow individual HIP C++ tests per instruction for testing semantics and are currently working on systematically adding more tests for the ISAs.
1 task done
Hardware forms LDS_ADDR = LDSbase + M0 + inst_offset + TIDinWave * 4 for buffer loads with the LDS bit (CDNA3/CDNA4 ISA §9.1.9); the simulator applied inst_offset to the buffer address only, so `offset:N lds` loads sharing one M0 overwrote the same LDS tile. Add inst_.offset to the LDS base in the amdisa MUBUF LDS-load template and regenerate cdna1-4/ rdna1-2 mubuf_exec.cpp. Add an in-process MubufLdsTest.
feikou
force-pushed
the
fix-mubuf-lds-inst-offset
branch
from
August 17, 2026 18:22
5072fea to
f7b703a
Compare
Contributor
Author
|
Done — dropped the HIP test and its CMake registration; kept the in-process |
kuhar
approved these changes
Aug 17, 2026
atgutier
pushed a commit
that referenced
this pull request
Aug 19, 2026
…#10192) ## Motivation `buffer_load_* ... lds` ignores the instruction's immediate `offset` when computing the LDS destination address in the simulator. Per the CDNA3/CDNA4 ISA reference guides, §9.1.9 "Memory Buffer Load to LDS": ``` LDS_ADDR = LDSbase + LDS_offset (M0) + inst_offset + (TIDinWave * 4) MEM_ADDR = Base + mem_offset (soffset) + inst_offset + off_vgpr + stride * (idx_vgpr + TIDinWave) ``` `inst_offset` feeds both addresses (LLVM `IntrinsicsAMDGPU.td`, `int_amdgcn_load_to_lds`, which also covers buffer fat pointers: "imm offset (applied to both input and LDS address)"). Kernels that set M0 once and issue several `buffer_load_dword ... offen offset:0/256/512 lds` to fill consecutive LDS tiles therefore kept overwriting the first tile under the simulator. ## Technical Details - `lib/python/amdisa/codegen/_generator.py`: the MUBUF LDS-load template now sets `d->lds_base = wf.m0() + inst_.offset + wf.lds_base()`. `soffset` and the VGPR offset stay memory-side only, matching the formula above. - Regenerated `generated/{cdna1,cdna2,cdna3,cdna4,rdna1,rdna2}/mubuf_exec.cpp` (documented `python -m amdisa --multi ... --gen-isas` flow + clang-format). With the in-tree ISA XMLs the unmodified generator reproduces every checked-in `mubuf*` file byte-for-byte; the only resulting change is that one line in the 18 `buffer_load_*` LDS branches per file. gfx1250's `*_load_async_to_lds_*` (per-lane VGPR LDS address, separate template) and rdna3_5's `buffer_load_lds_*` stubs (`UnimplementedInst`) are untouched; rdna3/rdna4 generate no MUBUF LDS-load path. - `tests/amdgpu_vm_test.cpp`: `MubufLdsTest.LoadDwordLdsAppliesInstOffset` (CPU-only, cdna1-4): M0 = 0x200, two encoded `buffer_load_dword v1, s[4:7], 0 offen [offset:256] lds` from two distinct source rows, all 64 lanes of both LDS rows checked. ## Issue Tracking No existing issue; related: #5747 (workgroup `lds_base` term on the same path). ## Test Plan - `rocjitsu_tests --gtest_filter='MubufLdsTest.*'` before and after the generator change. - `python -m pytest lib/python/amdisa/tests` and full `ctest` for regressions. ## Test Result - `MubufLdsTest.LoadDwordLdsAppliesInstOffset`: on unmodified `develop` it fails for all four CDNA targets (lane 0: LDS+0x200 reads 0xB0000000 instead of 0xA0000000 because the `offset:256` load overwrote row 0; LDS+0x300 still holds the 0xDEADBEEF fill); passes with this change. - Cross-checked on gfx950 hardware (ROCm 7.2.0) with an equivalent inline-asm kernel: the two rows land at LDS+0 and LDS+256 on hardware; the simulator matches only with this change. - Full `ctest`: no status changes versus `develop` other than the added `MubufLdsTest.LoadDwordLdsAppliesInstOffset`. - `python -m pytest lib/python/amdisa/tests`: 1319 passed, 46 skipped. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. (cherry picked from commit c810647)
1 task done
atgutier
added a commit
that referenced
this pull request
Aug 20, 2026
…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>
1 task done
atgutier
pushed a commit
that referenced
this pull request
Sep 3, 2026
…10928) ## Motivation CDNA buffer addressing and range checking didn't match gfx950 hardware. This fixes, for CDNA1-4: - SOFFSET and the instruction offset count toward the bounds check (only VOFFSET + inst_offset wraps at 32 bits). - An access must end within NUM_RECORDS, checked per dword for `dwordx{2,3,4}` loads and stores; format ops, atomics and MTBUF stay all-or-nothing. - Only IDXEN uses the index check on a strided buffer; OFFEN-only is checked by byte offset. - ADD_TID_ENABLE buffers add `thread_id * stride` to the address (the DFMT-extended stride for non-format ops) and, with SWIZZLE_ENABLE, use the swizzled element layout; neither is range-checked. - The RDNA OOB_SELECT bit is no longer consulted on CDNA descriptors. - Out-of-range lanes of `buffer_load ... lds` write zeros to LDS instead of being skipped. Previously the simulator ignored SOFFSET, dropped whole lanes, index-checked any strided buffer, gave every lane the same address on ADD_TID_ENABLE buffers, and left LDS untouched. ## Technical Details - `shared/addr_calc_buffer.h`: CDNA1-4 compute the per-lane offset in one helper shared by the MUBUF and MTBUF paths and apply the GFX9 range-check rules; per-dword results go through `element_lane_masks` (#10362) and swizzled multi-dword accesses reuse the scratch per-element stride. RDNA and gfx1250 are unchanged. - `vm/amdgpu/memory_pipeline.cpp`: on CDNA1-4, out-of-range lanes of LDS loads write zeros. The ISA docs and LLVM describe SOFFSET as excluded from the check; gfx950 hardware includes it, so this follows the hardware. DATA_FORMAT = 0 (non-format loads return zeros on hardware) isn't modelled. ## Issue Tracking No existing issue. Related: #10362, #10192. ## Test Plan An encoded-kernel test on cdna1-4 for the range-check cases and one for ADD_TID/swizzled loads and stores (`tests/amdgpu_vm_test.cpp`), direct address-calculation tests on cdna3/cdna4 for each rule (`CdnaAddrCalcTest.Mubuf*`), and one RDNA3 case to confirm that path is unchanged. ## Test Result The new tests fail on `develop` and pass with this change. Expected values were measured on gfx950 (ROCm 7.2). The rest of `ctest` is unchanged and no generated files change. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.
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
buffer_load_* ... ldsignores the instruction's immediateoffsetwhen computing the LDS destination address in the simulator. Per the CDNA3/CDNA4 ISA reference guides, §9.1.9 "Memory Buffer Load to LDS":inst_offsetfeeds both addresses (LLVMIntrinsicsAMDGPU.td,int_amdgcn_load_to_lds, which also covers buffer fat pointers: "imm offset (applied to both input and LDS address)"). Kernels that set M0 once and issue severalbuffer_load_dword ... offen offset:0/256/512 ldsto fill consecutive LDS tiles therefore kept overwriting the first tile under the simulator.Technical Details
lib/python/amdisa/codegen/_generator.py: the MUBUF LDS-load template now setsd->lds_base = wf.m0() + inst_.offset + wf.lds_base().soffsetand the VGPR offset stay memory-side only, matching the formula above.generated/{cdna1,cdna2,cdna3,cdna4,rdna1,rdna2}/mubuf_exec.cpp(documentedpython -m amdisa --multi ... --gen-isasflow + clang-format). With the in-tree ISA XMLs the unmodified generator reproduces every checked-inmubuf*file byte-for-byte; the only resulting change is that one line in the 18buffer_load_*LDS branches per file. gfx1250's*_load_async_to_lds_*(per-lane VGPR LDS address, separate template) and rdna3_5'sbuffer_load_lds_*stubs (UnimplementedInst) are untouched; rdna3/rdna4 generate no MUBUF LDS-load path.tests/amdgpu_vm_test.cpp:MubufLdsTest.LoadDwordLdsAppliesInstOffset(CPU-only, cdna1-4): M0 = 0x200, two encodedbuffer_load_dword v1, s[4:7], 0 offen [offset:256] ldsfrom two distinct source rows, all 64 lanes of both LDS rows checked.Issue Tracking
No existing issue; related: #5747 (workgroup
lds_baseterm on the same path).Test Plan
rocjitsu_tests --gtest_filter='MubufLdsTest.*'before and after the generator change.python -m pytest lib/python/amdisa/testsand fullctestfor regressions.Test Result
MubufLdsTest.LoadDwordLdsAppliesInstOffset: on unmodifieddevelopit fails for all four CDNA targets (lane 0: LDS+0x200 reads 0xB0000000 instead of 0xA0000000 because theoffset:256load overwrote row 0; LDS+0x300 still holds the 0xDEADBEEF fill); passes with this change.ctest: no status changes versusdevelopother than the addedMubufLdsTest.LoadDwordLdsAppliesInstOffset.python -m pytest lib/python/amdisa/tests: 1319 passed, 46 skipped.Submission Checklist