Repository navigation
fix(rocjitsu): Add InstDefUse Checks to Handle Partial Defs - #7979
Merged
Merged
Conversation
❌ PR Check — Action Required
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
accauble
approved these changes
Jul 1, 2026
accauble
left a comment
accauble
left a comment
Contributor
There was a problem hiding this comment.
Looks good but consider addressing a couple nits.
newling
reviewed
Jul 1, 2026
Groverkss
requested changes
Jul 3, 2026
accauble
reviewed
Jul 6, 2026
1 task
… analysis, analysis gets the proper info with no changes
theSK2005
force-pushed
the
users/skushwah/liveness-partial-def
branch
from
July 10, 2026 23:05
8550bf4 to
227b377
Compare
Groverkss
reviewed
Jul 14, 2026
Groverkss
approved these changes
Jul 14, 2026
theSK2005
added a commit
that referenced
this pull request
Aug 8, 2026
## Motivation D16 memory loads are 16 bit writes into VGPRs. These operations currently are seen as full defs by liveness analysis since VDST is only a dst and not a src and has a 32 bit operand size so PR #7979 won't cover it. ## Technical Details Added check in _generator.py's _dst_is_also_source to cover d16 load operations and have codegen write VDST as both a dst and a src. Fixed existing semantics.py bug that didn't use _BUFFER_FORMAT_MAP for derive_mubuf which left many instructions unimplemented and not properly flagged. ## Test Plan - Verify InstDefUse on decoded D16 instructions sees VDST as a src and a dst. - Verify all other tests pass. ## Test Result All tests pass ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Motivation
Some instructions have dst operands that are under 32 bits wide. These will be a partial write for 32 bit registers and InstDefUse/liveness need to account for this and not have incorrect kills.
Technical Details
Added a check for dst operand size to add as uses if their bit size is less than 32. Remaining logic is handled by current liveness implementation
JIRA ID
Test Plan
Write tests to make sure partial defs don't cause incorrect kills and that full defs continue to cause correct kills. All prior tests pass.
Test Result
New tests pass, prior tests all pass.
Submission Checklist