Repository navigation
fix(rocjitsu): respect same-wave LDS ordering - #9735
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. |
6577097 to
fba9cac
Compare
fba9cac to
394bc92
Compare
394bc92 to
dc44f02
Compare
|
Related earlier implementation: commit |
kuhar
left a comment
There was a problem hiding this comment.
It would be good to remove the unused exec parameters from race_test_builder.h:78 and race_test_builder.h:93. Both ldsWrite and ldsRead replace zero with defaultExec_, but always pass 1ULL << lane to registerLdsEvent; no current caller supplies exec after this PR replaced the old Exec_* cases. Leaving the parameters in place makes tests appear to configure a mask that is silently ignored. globalToLds is the helper that accepts an exec mask.
dc44f02 to
ac30e3b
Compare
ac30e3b to
88cd03b
Compare
88cd03b to
d0cc3a4
Compare
d0cc3a4 to
bbc5ac2
Compare
## Summary This PR fixes a general misunderstanding I had about same-wave LDS synchronization on our GPUs.. I previously assumed that sequences such as: ```asm ds_write_b8 v2, v0 # write to LDS at address in v2 ds_read_u8 v14, v2 ``` and the reverse: ```asm ds_read_u8 v14, v2 ds_write_b8 v2, v0 ``` were racy unless an `s_waitcnt lgkmcnt(0)` appeared between the two instructions. That is true for consuming the result of an LDS read. For example, this is racy: ```asm ds_read_u8 v14, v2 v_and_b32_e32 v5, 1, v14 ``` The read may not have returned its result to `v14` yet. This requires a wait: ```asm ds_read_u8 v14, v2 s_waitcnt lgkmcnt(0) v_and_b32_e32 v5, 1, v14 ``` However, ordinary DS operations issued by the same wave are ordered with respect to each other. They may be in flight at the same time, but a later DS instruction **does not overtake an earlier one** in the LDS pipeline. Consequently: - A same-wave DS write followed by a DS read observes the write. - A same-wave DS read followed by a DS write observes the value from before the write. - The destination VGPR of an LDS read still requires the appropriate wait before it can be consumed or overwritten. An agent confirmed this experimentally on a gfx1151 GPU using both same-lane and cross-lane byte accesses. Repeated no-wait write/read and read/write sequences completed without a single value mismatch. The issue originally surfaced as an LDS race report in hipBLASLt's HPL initialization kernel. LLVM had promoted a private Boolean temporary to per-workitem LDS storage and generated: ```asm ds_write_b8 ... ds_read_u8 ... s_waitcnt lgkmcnt(0) ``` The plugin incorrectly reported the read as racing with the earlier same-wave write. ## Changes The race detector now: - Treats ordinary same-wave DS write-to-read and read-to-write accesses as ordered. - Continues tracking an LDS read's destination VGPR until `lgkmcnt` permits its use. - Continues reporting overlapping cross-wave LDS accesses without the required barrier. - Continues requiring `vmcnt` before the owning wave consumes data written through a direct-to-LDS VMEM operation. The tests now include focused same-lane and cross-lane RAW/WAR unit coverage, plus one gfx950 end-to-end ordering test. ## Testing New ones added, incorrect ones fixed. ## Issue Tracking Related: #9577 (cherry picked from commit dd619be)
…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>
## Summary This PR fixes a general misunderstanding I had about same-wave LDS synchronization on our GPUs.. I previously assumed that sequences such as: ```asm ds_write_b8 v2, v0 # write to LDS at address in v2 ds_read_u8 v14, v2 ``` and the reverse: ```asm ds_read_u8 v14, v2 ds_write_b8 v2, v0 ``` were racy unless an `s_waitcnt lgkmcnt(0)` appeared between the two instructions. That is true for consuming the result of an LDS read. For example, this is racy: ```asm ds_read_u8 v14, v2 v_and_b32_e32 v5, 1, v14 ``` The read may not have returned its result to `v14` yet. This requires a wait: ```asm ds_read_u8 v14, v2 s_waitcnt lgkmcnt(0) v_and_b32_e32 v5, 1, v14 ``` However, ordinary DS operations issued by the same wave are ordered with respect to each other. They may be in flight at the same time, but a later DS instruction **does not overtake an earlier one** in the LDS pipeline. Consequently: - A same-wave DS write followed by a DS read observes the write. - A same-wave DS read followed by a DS write observes the value from before the write. - The destination VGPR of an LDS read still requires the appropriate wait before it can be consumed or overwritten. An agent confirmed this experimentally on a gfx1151 GPU using both same-lane and cross-lane byte accesses. Repeated no-wait write/read and read/write sequences completed without a single value mismatch. The issue originally surfaced as an LDS race report in hipBLASLt's HPL initialization kernel. LLVM had promoted a private Boolean temporary to per-workitem LDS storage and generated: ```asm ds_write_b8 ... ds_read_u8 ... s_waitcnt lgkmcnt(0) ``` The plugin incorrectly reported the read as racing with the earlier same-wave write. ## Changes The race detector now: - Treats ordinary same-wave DS write-to-read and read-to-write accesses as ordered. - Continues tracking an LDS read's destination VGPR until `lgkmcnt` permits its use. - Continues reporting overlapping cross-wave LDS accesses without the required barrier. - Continues requiring `vmcnt` before the owning wave consumes data written through a direct-to-LDS VMEM operation. The tests now include focused same-lane and cross-lane RAW/WAR unit coverage, plus one gfx950 end-to-end ordering test. ## Testing New ones added, incorrect ones fixed. ## Issue Tracking Related: #9577
Summary
This PR fixes a general misunderstanding I had about same-wave LDS synchronization on our GPUs..
I previously assumed that sequences such as:
and the reverse:
were racy unless an
s_waitcnt lgkmcnt(0)appeared between the two instructions. That is true for consuming the result of an LDS read. For example, this is racy:The read may not have returned its result to
v14yet. This requires a wait:However, ordinary DS operations issued by the same wave are ordered with respect to each other. They may be in flight at the same time, but a later DS instruction does not overtake an earlier one in the LDS pipeline. Consequently:
An agent confirmed this experimentally on a gfx1151 GPU using both same-lane and cross-lane byte accesses. Repeated no-wait write/read and read/write sequences completed without a single value mismatch.
The issue originally surfaced as an LDS race report in hipBLASLt's HPL initialization kernel. LLVM had promoted a private Boolean temporary to per-workitem LDS storage and generated:
The plugin incorrectly reported the read as racing with the earlier same-wave write.
Changes
The race detector now:
lgkmcntpermits its use.vmcntbefore the owning wave consumes data written through a direct-to-LDS VMEM operation.The tests now include focused same-lane and cross-lane RAW/WAR unit coverage, plus one gfx950 end-to-end ordering test.
Testing
New ones added, incorrect ones fixed.
Issue Tracking
Related: #9577