-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[Draft][AMDGPU] Rematerialize VGPR candidates when SGPR spills results in VGPR Excess #168079
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
jmmartinez
wants to merge
21
commits into
main
Choose a base branch
from
users/jmmartinez/fix/remat_on_excess_sgpr_to_vgpr
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+2,264
−623
Draft
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
10a0962
Scoring system for rematerializations
lucas-rami 48d1231
Format
lucas-rami 3f55867
Address feedback + fix failing test
lucas-rami c459495
Remove REMAT_DEBUG and break ties in score
lucas-rami 64e077e
Fix failing tests and rollback mask change
lucas-rami 15faedc
Address more feedback
lucas-rami 2793aa8
Rebase for new test + improve comment
lucas-rami 029a2c6
Refactor scoring system + Remove always benef/latency calc
lucas-rami cdba9b8
Fix tests
lucas-rami 5552bf4
Improve code for frequency-based calculations
lucas-rami f57f3bd
Add empty region test
lucas-rami fde49a3
Correctly derive (sub)reg size and frequency fix
lucas-rami b805a71
Walk over all regions to compute AchievedOcc
lucas-rami b0f4cd6
Use std::max for freq calc.
lucas-rami 43c740c
Simplify score calculation and improve debug
lucas-rami b5f0950
Early exit on maxWavesPerEU and rebase
lucas-rami 5f07ff3
Fix tests and remove arg from TII.rematerialize
lucas-rami bbc341d
[NFC][AMDGPU] Refactor common code computing excess register preassur…
jmmartinez e331b29
Unacceptably large test
jmmartinez cd41eb1
[AMDGPU] Rematerialize VGPR candidates when SGPR spills to VGPR over …
jmmartinez f013974
Remove undef from test (it still preserves the test behavour before a…
jmmartinez File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't
MaxVGPRsbeMaxUnifiedVGPRs? I don't think we want the function's behavior to change when we don't have SGPR spills, and it looks like this will cause the function to report new beneficial saves in cases where the target has already been reached (e.g. gfx942,MaxUnifiedVGPRs=512,MaxVGPRs=256).Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're right. In that case it should be:
RegExcess Excess(MF, RP, MaxSGPRs, UnifiedRF ? MaxUnifiedVGPRs : MaxVGPRs);PS: I have to test this because all these register classes are quite confusing