Skip to content

fix(rocjitsu): route tensor DMA through wave VMID - #9775

Merged
kuhar merged 1 commit into
ROCm:developfrom
newling:fix-gfx1250-tensor-dma-vmid
Aug 18, 2026
Merged

kuhar merged 1 commit into
ROCm:developfrom
newling:fix-gfx1250-tensor-dma-vmid

Conversation

@newling

@newling newling commented Aug 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This change fixes the first rocJITsu tensor-DMA memory-translation failure reached by a gfx1250 TensileLite BF16 TDM GEMM.

The workload comes from:

Tensile/Tests/common/gemm/gfx12/tdm_gfx1250.yaml
problem identifier: Contraction_l_Alik_Bljk_Cijk_Dijk
generated group: Cijk_Alik_Bljk_BBS_BH_UserArgs_00
representative problem: (64, 64, 1, 512)

Failure before this fix

tensilelite-client segfaulted while executing a gfx1250 tensor load-to-LDS operation:

tensor_dma_detail::copy_bytes
  -> tensor_dma_detail::copy_dense_tensor
  -> execute_tensor_load_to_lds<TensorLoadToLdsVimage>

The fault was a direct host dereference of a GPU virtual address such as 0x7fff2cc00000.

Root cause

Tensor DMA accessed GpuMemory without binding the issuing wavefront's process ID. The access therefore used VMID zero. In local functional mode, VMID-zero passthrough treated the user-space-looking GPU VA as a host pointer instead of resolving it through the dispatched process page table.

Fix

  • Add wave-scoped block read and write methods that always bind Wavefront::process_id().
  • Route tensor-DMA global-memory accesses through those methods.
  • Remove the raw InstructionComputeUnitView::memory() accessor so instruction code cannot accidentally omit the wave VMID.
  • Use one GpuMemory::read_block or write_block operation per 1-, 2-, 4-, or 8-byte tensor element instead of acquiring translation locks once per byte.
  • Keep VMID zero as the intentional default for host, driver, and test callers.

An equivalent VMID propagation exists in the broader unmerged users/kuhar/dbt-sim work at commit 08a5c461a5; this PR isolates the tensor-DMA correction, strengthens the instruction-facing memory contract, and adds direct regression coverage.

Test coverage

The new unit test:

  • registers a nonzero process page table;
  • maps load and store GPU VAs to separate host arrays;
  • proves VMID zero resolves to different storage;
  • verifies tensor load-to-LDS uses the wave process page table; and
  • verifies tensor store-from-LDS uses the same page table.

Local validation includes the complete Gfx1250ExecutionTest.TensorDma* test set and changed-file pre-commit checks.

Workload status

With this fix, the generated TensileLite client advances past the original VMID-zero direct-VA fault. The complete stock workload still has separate tensor-DMA tail and descriptor-contract questions tracked by #9935. This PR removes the narrow translation failure and does not claim to resolve the full workload.

Issue Tracking

Closes #9774

This is the first independently reviewable rocJITsu correction tracked by #9935.

@therock-pr-bot

therock-pr-bot Bot commented Aug 6, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Aug 6, 2026

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@newling
newling marked this pull request as ready for review August 6, 2026 15:35
@newling
newling requested review from a team and atgutier as code owners August 6, 2026 15:35

@kuhar kuhar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I forgot to cherry pick this fix...

@kuhar kuhar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments

@kuhar
kuhar self-requested a review August 6, 2026 16:28
@newling
newling force-pushed the fix-gfx1250-tensor-dma-vmid branch from b45692c to f8d63a3 Compare August 9, 2026 18:03

@kuhar kuhar left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment.

@newling

newling commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor Author

Related earlier implementation: commit 08a5c461 contains the same tensor-DMA wave-VMID propagation within broader RDNA runtime work. The production overlap is effectively exact in tensor_dma.h—all three VMID-propagation edits—while this PR’s focused regression is distinct. Related but separate PR #9003 addresses safe VMID-miss/backing behavior; it shares no changed files with this PR and does not supply the wave VMID.

@kuhar kuhar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@newling
newling force-pushed the fix-gfx1250-tensor-dma-vmid branch 3 times, most recently from d70e029 to 9bd684d Compare August 13, 2026 15:01
@kuhar

kuhar commented Aug 13, 2026

Copy link
Copy Markdown
Member

@newling the newest revision doesn't build for me

@newling
newling force-pushed the fix-gfx1250-tensor-dma-vmid branch from 9bd684d to a5d9614 Compare August 13, 2026 15:54
@newling
newling force-pushed the fix-gfx1250-tensor-dma-vmid branch from a5d9614 to 8ef11c8 Compare August 18, 2026 15:51
@kuhar
kuhar merged commit dbb7221 into ROCm:develop Aug 18, 2026
24 of 26 checks passed
kuhar pushed a commit that referenced this pull request Aug 18, 2026
## Summary

This change fixes tensor-DMA bounds contracts exposed while reducing the
stock gfx1250 TensileLite TDM tail failure tracked by #9935.

It is independent of the VMID correction in #9775 at the source level.
Both changes are needed to advance the original workload through the
currently known rocJITsu model gaps.

## Related producer-side evidence

- ROCm/rocm-libraries#7976 introduced TDM iterate mode and the
`tdm_gfx1250.yaml` configuration lineage that exposed this gap.
- ROCm/rocm-libraries#8406 contains the same affected configuration
after iterate mode was enabled and reports passing FFM and B0
validation. This is file-level evidence rather than a published
per-solution trace.
- ROCm/rocm-libraries#9410 added iterate-mode edge cases with waves that
have zero valid rows specifically to exercise descriptor bounds
clamping, and reports passing FFM validation.

## Failure before this fix

Iterate mode advanced the starting global address for each repeated
tile, but rocJITsu continued to apply bounds using tile-local
coordinates. A final row outside the tensor was therefore copied into
LDS instead of being zero-filled.

An active tensor extent of zero was treated as unbounded. The complete
dense or gather tile could therefore be copied instead of being masked.

## Fixes

### Include iterate-mode address advancement in tensor bounds

The model decomposes each advancing iteration's linear global-address
increment into an absolute tensor-coordinate origin, then applies one
bounds predicate to that origin plus the tile-local coordinate.

This handles sub-row advances, row advances, and rank-3 plane advances
through the same path. Overlapping-stride layouts are rejected only when
multiple iterations actually advance the global origin; repeated-origin
descriptors do not require inverse coordinate decomposition.

### Treat zero active tensor extents as empty

For an active dense or gather tile dimension, tensor extent zero masks
the complete dimension. Loads zero-fill the affected LDS elements and
stores suppress the corresponding global writes.

Optional null descriptor groups retain their architectural zero-filled
value. A rank-3 tile with null D2 therefore has a zero third tensor
extent: it masks the transfer without aborting execution, still
completes an enabled barrier, and releases TENSORCNT normally.

### Avoid unnecessary origin decomposition

Non-iterating descriptors use an all-zero origin directly. This avoids
inverse stride decomposition, including accepted rank-5 layouts whose
large default strides can wrap.

## Test coverage

Focused decoded-instruction regressions verify:

- partial final-row load zero-fill and corresponding store suppression;
- active zero-length dense and gather extents;
- null-D2 rank-3 load masking and store suppression;
- completion-barrier arrival and TENSORCNT release for null D2;
- sub-row, row, and rank-3 plane iteration origins;
- rejection of advancing overlapping-stride layouts;
- acceptance of repeated-origin overlapping-stride layouts; and
- safe non-iterating rank-5 execution.

The complete `Gfx1250ExecutionTest.TensorDma*` test set and changed-file
pre-commit checks pass locally.

## Workload status

This PR intentionally does not claim to fix the complete TensileLite
reproducer. It removes independently testable descriptor-model
discrepancies and gets the workload closer to the unresolved tail-loop
semantic question.

## Issue tracking

Progress toward #9935.
atgutier pushed a commit that referenced this pull request Aug 19, 2026
## Summary

This change fixes the first rocJITsu tensor-DMA memory-translation
failure reached by a gfx1250 TensileLite BF16 TDM GEMM.

The workload comes from:

```text
Tensile/Tests/common/gemm/gfx12/tdm_gfx1250.yaml
problem identifier: Contraction_l_Alik_Bljk_Cijk_Dijk
generated group: Cijk_Alik_Bljk_BBS_BH_UserArgs_00
representative problem: (64, 64, 1, 512)
```

## Failure before this fix

`tensilelite-client` segfaulted while executing a gfx1250 tensor
load-to-LDS operation:

```text
tensor_dma_detail::copy_bytes
  -> tensor_dma_detail::copy_dense_tensor
  -> execute_tensor_load_to_lds<TensorLoadToLdsVimage>
```

The fault was a direct host dereference of a GPU virtual address such as
`0x7fff2cc00000`.

## Root cause

Tensor DMA accessed `GpuMemory` without binding the issuing wavefront's
process ID. The access therefore used VMID zero. In local functional
mode, VMID-zero passthrough treated the user-space-looking GPU VA as a
host pointer instead of resolving it through the dispatched process page
table.

## Fix

- Add wave-scoped block read and write methods that always bind
`Wavefront::process_id()`.
- Route tensor-DMA global-memory accesses through those methods.
- Remove the raw `InstructionComputeUnitView::memory()` accessor so
instruction code cannot accidentally omit the wave VMID.
- Use one `GpuMemory::read_block` or `write_block` operation per 1-, 2-,
4-, or 8-byte tensor element instead of acquiring translation locks once
per byte.
- Keep VMID zero as the intentional default for host, driver, and test
callers.

An equivalent VMID propagation exists in the broader unmerged
[`users/kuhar/dbt-sim`](https://github.com/ROCm/rocm-systems/tree/users/kuhar/dbt-sim)
work at commit
[`08a5c461a5`](08a5c46);
this PR isolates the tensor-DMA correction, strengthens the
instruction-facing memory contract, and adds direct regression coverage.

## Test coverage

The new unit test:

- registers a nonzero process page table;
- maps load and store GPU VAs to separate host arrays;
- proves VMID zero resolves to different storage;
- verifies tensor load-to-LDS uses the wave process page table; and
- verifies tensor store-from-LDS uses the same page table.

Local validation includes the complete `Gfx1250ExecutionTest.TensorDma*`
test set and changed-file pre-commit checks.

## Workload status

With this fix, the generated TensileLite client advances past the
original VMID-zero direct-VA fault. The complete stock workload still
has separate tensor-DMA tail and descriptor-contract questions tracked
by #9935. This PR removes the narrow translation failure and does not
claim to resolve the full workload.

## Issue Tracking

Closes #9774

This is the first independently reviewable rocJITsu correction tracked
by #9935.

(cherry picked from commit dbb7221)
atgutier pushed a commit that referenced this pull request Aug 19, 2026
## Summary

This change fixes tensor-DMA bounds contracts exposed while reducing the
stock gfx1250 TensileLite TDM tail failure tracked by #9935.

It is independent of the VMID correction in #9775 at the source level.
Both changes are needed to advance the original workload through the
currently known rocJITsu model gaps.

## Related producer-side evidence

- ROCm/rocm-libraries#7976 introduced TDM iterate mode and the
`tdm_gfx1250.yaml` configuration lineage that exposed this gap.
- ROCm/rocm-libraries#8406 contains the same affected configuration
after iterate mode was enabled and reports passing FFM and B0
validation. This is file-level evidence rather than a published
per-solution trace.
- ROCm/rocm-libraries#9410 added iterate-mode edge cases with waves that
have zero valid rows specifically to exercise descriptor bounds
clamping, and reports passing FFM validation.

## Failure before this fix

Iterate mode advanced the starting global address for each repeated
tile, but rocJITsu continued to apply bounds using tile-local
coordinates. A final row outside the tensor was therefore copied into
LDS instead of being zero-filled.

An active tensor extent of zero was treated as unbounded. The complete
dense or gather tile could therefore be copied instead of being masked.

## Fixes

### Include iterate-mode address advancement in tensor bounds

The model decomposes each advancing iteration's linear global-address
increment into an absolute tensor-coordinate origin, then applies one
bounds predicate to that origin plus the tile-local coordinate.

This handles sub-row advances, row advances, and rank-3 plane advances
through the same path. Overlapping-stride layouts are rejected only when
multiple iterations actually advance the global origin; repeated-origin
descriptors do not require inverse coordinate decomposition.

### Treat zero active tensor extents as empty

For an active dense or gather tile dimension, tensor extent zero masks
the complete dimension. Loads zero-fill the affected LDS elements and
stores suppress the corresponding global writes.

Optional null descriptor groups retain their architectural zero-filled
value. A rank-3 tile with null D2 therefore has a zero third tensor
extent: it masks the transfer without aborting execution, still
completes an enabled barrier, and releases TENSORCNT normally.

### Avoid unnecessary origin decomposition

Non-iterating descriptors use an all-zero origin directly. This avoids
inverse stride decomposition, including accepted rank-5 layouts whose
large default strides can wrap.

## Test coverage

Focused decoded-instruction regressions verify:

- partial final-row load zero-fill and corresponding store suppression;
- active zero-length dense and gather extents;
- null-D2 rank-3 load masking and store suppression;
- completion-barrier arrival and TENSORCNT release for null D2;
- sub-row, row, and rank-3 plane iteration origins;
- rejection of advancing overlapping-stride layouts;
- acceptance of repeated-origin overlapping-stride layouts; and
- safe non-iterating rank-5 execution.

The complete `Gfx1250ExecutionTest.TensorDma*` test set and changed-file
pre-commit checks pass locally.

## Workload status

This PR intentionally does not claim to fix the complete TensileLite
reproducer. It removes independently testable descriptor-model
discrepancies and gets the workload closer to the unresolved tail-loop
semantic question.

## Issue tracking

Progress toward #9935.

(cherry picked from commit a659172)
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[rocjitsu][gfx1250] Tensor DMA ignores the dispatch VMID

2 participants