Repository navigation
Conversation
Use the decoded scalar operand for indirect-PC prechecks so TTMP targets do not fall back to physical SGPR aliases. Keep scalar loads into TTMP out of the race detector's ordinary-SGPR event table until TTMP load hazards are modeled. Add regressions for both paths. Related: ROCm#9577
✅ 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. |
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.
rocJITsu must distinguish ordinary scalar registers from trap-temporary registers (TTMP0–15). Although both are named by scalar operand selectors, TTMP selectors 108–123 refer to storage owned directly by each wave; they must never be interpreted as offsets into that wave’s ordinary SGPR allocation.
Most of this distinction is already implemented on
develop. #9844 introduced wave-private TTMP storage and routed scalar operand, address-calculation, scalar-memory completion, checkpoint, and debugger paths through it. #10345 completed the GFX12 launch-state initialization. Those changes superseded most of the broader implementation proposed in #9578, but two paths still treated a TTMP selector as an ordinary SGPR number.Before executing
s_setpc_b64ors_swappc_b64, the compute unit checks whether the target is zero. That check decoded the instruction’s raw selector and readsgpr_base + selectordirectly. For a target held in a TTMP pair, it therefore read unrelated physical SGPR storage and could halt the wave even though the actual target was nonzero. This change reads the already-decoded source operand through the normal register-access interface, which resolves ordinary SGPRs, TTMPs, and other scalar operand kinds according to their architectural meaning.The race detector had a related bookkeeping problem. It registered every scalar-memory load destination as an ordinary
GLOBAL_TO_SGPRevent. A load into TTMP does not produce the ordinary-SGPR callbacks needed to consume that event, and selector 108 can lie beyond the event table for a wave with 104 ordinary SGPRs. This change keeps TTMP destinations out of the ordinary-SGPR event table. Modeling pending load hazards for TTMP consumers can be added separately rather than representing them incorrectly as SGPR hazards.Both regressions were reproduced on unmodified
develop: the indirect-PC test followed the wrong zero-target path, while the race-detector test terminated with a segmentation fault. With this change, both pass. Ten related TTMP storage, checkpoint, translation, and GFX12 launch-state tests pass, as do all 3,289 enabled non-benchmark C++ tests; two tests are skipped. The race-detector shared library builds successfully, and formatting checks pass.This is a standalone follow-up to #9844 and #10345, and it carries forward the remaining useful corrections identified in #9578. The wave-owned observation work in #10030 is related but is not a prerequisite. #10467 currently touches the same indirect-PC pre-check and will need the equivalent selector-aware read if it lands first.
Related: #9577