[newling]

Teach the race detector to report write-after-write hazards involving pending scalar-memory loads and SGPR (and TTMP) destinations.

Background

GPU memory operations execute asynchronously. A later instruction can execute while a load result is still pending, so software must use the appropriate wait-counter instruction before reading or overwriting the destination.

The race detector already applies this rule to VGPRs. For example, it reports when an instruction overwrites a VGPR that is still the destination of a pending global or LDS load. For scalar registers, however, the detector previously checked reads but not writes: it could report an instruction reading a pending scalar-load destination, but not an instruction overwriting that destination.

For example:

s_load_dword s4, s[0:1], 0
s_mov_b32 s4, 7

The s_mov_b32 writes s4 while the scalar load is still pending. If the load completes afterward, it overwrites the value written by s_mov_b32. This is a write-after-write hazard caused by the asynchronous load.

A related hazard occurs when two scalar loads target the same register:

s_load_dword s4, s[0:1], 0
s_load_dword s4, s[2:3], 0

Scalar-memory reads may complete out of issue order, so the first load may overwrite the result of the second. In both examples, an s_waitcnt lgkmcnt(0) before the second write establishes the required ordering.

Change

The race detector now checks instruction writes to scalar registers against pending scalar-memory destinations. It also performs the same check before registering a new scalar load, covering the load-after-load case.

Instruction writes use the existing scalar-register write callback. Scalar-memory completion and runtime initialization continue to update raw register storage without firing that callback, because completion is the asynchronous operation being tracked rather than a new instruction write.

Wide accesses report each conflicting memory event once, while separate pending loads remain separately identifiable.

Testing

Added focused core and plugin tests for scalar-register writes, wide destinations, multiple pending loads, and waits, together with gfx950 end-to-end safe and racy load-to-move and load-to-load cases.

Related: #7950

[therock-pr-bot]

✅ 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]

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

[kuhar]

A few comments

[newling]

Ready for another review.
There is one thing I've noticed that I'm not happy with here, and that is the special need that SGPR writes have (friendship to Wavefront). This wasn't needed for SGPR reads or VGPR reads/writes. I'd prefer to clean this up in a follow-up (I've left a TODO)

[kuhar]

We should avoid turning out-of-range register observations into uncaught exceptions. At emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/vm/amdgpu/compute_unit.h:458 and :470, the new VGPR allocation checks run before the zero-mask early-outs, so even an inert notification can throw; GPR indexing can also form an index outside the wave block from guest-controlled M0. At emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/vm/plugins/race_detector/plugin.cpp:333, the previous below-base skip is now requirePhysicalRegisterRange, and the same throwing helper guards the other routing and callback paths. issue_instruction only catches InvalidInst around decode, so these exceptions escape step(). Please use a nonfatal policy, such as skipping with a rate-limited warning or halting the wave, while retaining the bounds validation.

[newling]

I believe I've addressed the comments related to warning/exception/failure modes. However note that for the comment

Please use a nonfatal policy, such as skipping with a rate-limited warning or halting the wave, while retaining the bounds validation.

I've not implemented any rate-limited warning as this is IMO always a bad compromise between a simple failure and success.

[newling]

Related similar work: PR #8180 implements analogous VGPR write-after-load detection, while the users/kuhar/waitcheck branch statically diagnoses the same scalar-result overwrite hazards. #8180 shares 16 of this PR’s 22 changed paths but targets VGPR rather than SGPR hazards; waitcheck has almost no implementation overlap because it is a static object-code analyzer rather than a runtime plugin.

[kuhar]

A few comments

[kuhar]

One comment, plus an inline reply

[atgutier]

[BLOCKER] /tmp/pr9470-review/emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/vm/amdgpu/
compute_unit.h:401 — SGPR ownership is incorrectly restricted to the descriptor-requested count

Reasoning: owns_sgpr_range() uses wf.sgpr_alloc().count, which is the descriptor-reported, rounded
SGPR usage—not the size of the physical block reserved for the wave. Valid compiler/ABI temporaries
above that count are consequently read as zero and their writes silently discarded by the new
helpers at line 421. Instruction accesses reach those helpers through /tmp/pr9470-review/emulation/
rocjitsu/lib/rocjitsu/src/rocjitsu/vm/amdgpu/register_access.h:999.

Evidence:

• The reproduced gfx1201 k_readlane kernel reports 36 SGPRs, producing a 40-register dispatch
  request.

• Called code legitimately executes:

  s_or_saveexec_b32 s40, -1
  s_mov_b32 exec_lo, s40

• Register s40 is immediately outside [base, base + 40). The new code drops the first write and
  returns zero for the second read, clearing EXEC. The remaining vector work produces zero output.

• Exact PR head a488688 fails all 36 executed fpsan_xlane_test cases with zero results. The exact
  base 95b56f2 passes the identical isolated workload.

• A diagnostic head build that bypasses this rejection makes the previously failing
  Xlane.ReadlaneFloat0 pass.

• Gfx1250’s fpsan_cvt_f16_fp8_gfx1250_test also fails all 10 cases with the same zero-result
  signature.

• /tmp/pr9470-review/emulation/rocjitsu/lib/simdojo/include/simdojo/components/register_file.h:44
  confirms that allocate(count) reserves and clears the entire fixed regs_per_block_ block. /tmp/
  pr9470-review/emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/vm/amdgpu/compute_unit.cpp:47
  configures that block as config.sgprs_per_wf.

Suggested fix: Base ownership on the reserved physical block:

return RegAllocation{wf.sgpr_alloc().base, config_.sgprs_per_wf}
.contains(physical_base, physical_count);

Keep the descriptor-requested count separately for resource metadata. If race detection should cover
compiler/ABI temporaries, consistently size its SGPR state and physical-to-logical mapping using the
physical block size as well. Add a regression where the requested count is smaller than sgprs_per_wf
and called code accesses a valid register in the remainder of that block.

Confidence: high.

MAJOR

[MAJOR] /tmp/pr9470-review/emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/vm/plugins/race_detector/
plugin.cpp:419 — SGPR-event retirement uses stale cumulative wait state instead of the current wait
instruction

Reasoning: After every instruction whose mnemonic starts with s_waitcnt, the plugin rereads the
persistent Wavefront::wait_target() and dispatches only its vmcnt and lgkmcnt values. This is not
equivalent to processing the counter fields changed by the instruction that just executed.

It fails in both directions:

• False positive on RDNA4/gfx1250:

  s_load_b32 s0, ...
  s_wait_kmcnt 0
  s_mov_b32 s0, ...

  Scalar loads use KMCNT at /tmp/pr9470-review/emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/
  arch/amdgpu/generated/rdna4/smem.cpp:48, and /tmp/pr9470-review/emulation/rocjitsu/lib/rocjitsu/
  src/rocjitsu/isa/arch/amdgpu/generated/rdna4/sopp.cpp:611 updates that counter. Its mnemonic does
  not start with s_waitcnt, so the pending SGPR event is never retired and the safe s_mov is
  reported as a race.

• False negative on RDNA2:

  s_waitcnt lgkmcnt(0)
  s_load_b32 s0, ...
  s_waitcnt_vscnt 0
  s_mov_b32 s0, ...

  /tmp/pr9470-review/emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna2/
  sopk.cpp:421 changes only vscnt, but its name matches the prefix. The plugin rereads the stale
  cumulative lgkmcnt=0, prematurely retires the new scalar-load event, and misses the WAW.

Suggested fix: Dispatch a typed, per-instruction wait update containing only the counters encoded by
the current instruction. Extend PendingWaitCount beyond its current vmcnt/lgkmcnt pair to represent
kmcnt, loadcnt, storecnt, dscnt, and the other modeled split counters. Do not infer semantics from
mnemonic prefixes or persistent wave state.

Required regression tests:

• s_load; s_wait_kmcnt 0; overwrite same SGPR → no race.
• s_load; s_waitcnt_vscnt 0; overwrite same SGPR → race remains.
• Combined split-counter waits should retire only the counters they encode.

Confidence: high.

[newling]

Thanks for the reviews. This PR has accumulated foundational register-access changes well beyond its original SGPR race-detection feature. Continuing to patch those concerns in place is making the change increasingly difficult to reason about and review.

I’m going to close this PR for now and split the work into smaller pieces:

1. A focused foundational change that unifies explicit-wave SGPR/VGPR observation and defines ownership using the reserved physical register block.
2. The gfx950 SGPR WAW detector as a small follow-up built on that foundation.

Regarding the latest review @atgutier: the SGPR ownership blocker is valid and helped expose the important distinction between the descriptor-requested SGPR count and the reserved physical block. The foundational work will address that.

The split wait-counter finding concerns RDNA2 and gfx1250. The race detector currently has no support for either architecture; this PR’s SGPR WAW behavior is scoped and tested on gfx950’s combined lgkmcnt model. Typed handling for kmcnt, vscnt, and the other split counters would be part of adding future race-detector support for those architectures, rather than part of this change. I’ll make that support boundary more explicit in the follow-up.

[newling]

This PR is back and ready for another review! It is significantly simpler now.

[atgutier]

LGTM now, thanks!

[systems-assistant]

TheRock Submodule Bump Activity

Newest first

• 2026-09-08 — Included in TheRock via ROCm/TheRock#7974.