[WIP] fix(rocjitsu): model CDNA wait-counter capacity in race detector - #10925
Draft
newling wants to merge 1 commit into
Conversation
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.
Summary
lgkmcnt(15)andvmcnt(63)do not complete pending events.counters from overflowing.
partial waits and write-after-write checks use only ordering that hardware
guarantees.
s_waitcnt,s_waitcnt_vmcnt, ands_waitcnt_lgkmcnt.gfx1151 HIP coverage for the standalone legacy wait forms.
Problem
CDNA's LGKMCNT is four bits and VMCNT is six bits:
The 15-entry LGKMCNT capacity applies to the complete counter, not just LDS.
On pre-GFX12 hardware LGKMCNT accounts for LDS, GDS, scalar-memory, and message
events. Those event classes share capacity but are not all ordered relative to
one another. Fifteen later LDS operations are sufficient to prove that an
older LDS operation completed; fifteen arbitrary LGKM-counted operations are
not sufficient to identify which operation completed.
The race detector previously kept an unbounded event queue and passed every
decoded wait value through
total - limitretirement logic. That produced twoopposite errors:
queue exceeded the hardware counter's capacity, even though that operand is
architecturally “do not wait.”
must have completed them to make room for another issue.
The detector also used
MemoryEventTypeas a proxy for completion order. Thatcould suppress a real WAW race between ordinary VMEM and generic FLAT loads,
or choose an arbitrary scalar-memory result after a partial LGKM wait.
This became visible in production-generated gfx950 prefixes. In one case an
LDS result had 123 younger local-DS operations before its consumer; in another
the result had 17 younger local-DS operations. Both must have completed before
their consumers, even though the intervening
lgkmcnt(15)itself performs nowait.
Architecture and compiler cross-checks
S_WAITCNTinstruction. Its operand is four LGKM bits, six VM bits, and three export
bits; all ones means “do not wait” for each field.
S_WAITCNT_VMCNTandS_WAITCNT_LGKMCNTinstructions occurin the RDNA1--RDNA3.5 specifications. LLVM's MC tests explicitly reject them
on gfx9 and gfx12.
S_WAIT_LOADCNT,S_WAIT_DSCNT, andS_WAIT_KMCNT.to LGKMCNT. Its ordering logic treats scalar memory and mixed event types as
potentially out of order, while pre-VSCNT VMEM loads and stores share one
ordered counter stream.
waitcnt-overflow.mirregression test andSIInsertWaitcnts.cpp::determineWaitForScorecap overflow-age dependenciesat
limit - 1:lgkmcnt(14)andvmcnt(62)on gfx9.Relevant upstream references:
Implementation
Each event now carries an explicit completion-order class:
VMEMfor CDNA non-FLAT VMEM operations, including stores and direct-to-LDS;LDSfor native local-DS operations; andUNORDEREDfor scalar memory, generic FLAT, GDS, and cases without a usableordering guarantee.
Before an ordered memory instruction executes, the detector checks whether
its class already occupies the counter's capacity. If so, it marks the oldest
event in that class wave-complete before the current instruction's operand and
memory checks. This matches the hardware stall that makes room for the new
issue.
For a nonzero partial wait, only the oldest prefix that is provably complete
within an ordered class is retired. A zero wait still drains every event on
the selected counter. All-ones operands retire nothing. Mixed-class pressure
is deliberately conservative: it may leave an event pending when hardware has
made some progress, but it never guesses which event completed.
gfx950 encodes these fields in the combined
s_waitcntinstruction. Thestandalone
s_waitcnt_vmcntands_waitcnt_lgkmcntforms are retained for thealready-supported gfx1151 path; they are GFX10/GFX11 instructions, not gfx1250
instructions. gfx1250 uses split instructions such as
s_wait_loadcntands_wait_dscnt, which remain outside this PR's race-detector scope.Evidence
Native MI355X probes distinguish an explicit no-wait from capacity-forced
progress:
lgkmcnt(15)vmcnt(63)vmcnt(63)The new end-to-end tests demonstrate that the all-ones operands leave a
below-capacity dependency pending (runtime addresses omitted):
With the complete capacity model, four bounded production-prefix replays are
clean:
The former bug-2 and bug-3 race reports were therefore detector-model
artifacts, not evidence that hipBLASLt needed the proposed extra waits.
The HIP boundary tests exercise the exact issue counts:
Four gfx1151 HIP tests independently verify that each standalone legacy wait
drains its own counter and does not drain the other counter.
Testing
110/110RaceDetector.*andRaceDetectorPlugin.*tests passed.11,000test executions).
52/52gfx950 and gfx1151 race-plugin end-to-end tests passed, includingten new no-wait, capacity-boundary, and standalone-wait cases.
the one failure was an unrelated DBT offset expectation that reproduces on
the pre-change build.
git diff --checkpassed for every changed file.Remaining work before marking ready
sanitizer/Waitcheck prerequisite branch.
basis for grouping all native local-DS operations and all non-FLAT VMEM
operations. LLVM uses the same distinction, and native MI355X probes cover
LDS reads plus VMEM loads/stores, but this is the central architectural
premise.
this implementation intentionally keeps such events pending rather than
guessing which one hardware completed.
Issue Tracking
Related: ROCM-28430
Related: #9577