-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[AMDGPU][InstCombine] Fold ballot intrinsic based on llvm.assume hints #160670
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
Open
TejaX-Alaghari
wants to merge
16
commits into
llvm:main
Choose a base branch
from
TejaX-Alaghari:DA_Assume
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.
+979
−0
Open
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
0e4dd51
[InstCombine] Optimize AMDGPU ballot + assume uniformity patterns
TejaX-Alaghari 258ecfe
[InstCombine] Add constant folding for AMDGPU ballot intrinsics
TejaX-Alaghari 408bc09
[InstCombine] Implement generic assume-based uniformity optimization
TejaX-Alaghari f0ed709
[InstCombine] Add focused assume-based optimizations
TejaX-Alaghari 2f5587e
Address @ssahasra's review feedback
TejaX-Alaghari b7a013c
Address feedback on the location of the opt
TejaX-Alaghari 1fbc805
Refactored the ballot optimization condition into propagateEquality m…
TejaX-Alaghari c7a1f6a
Implement reviewer's suggestions to -
TejaX-Alaghari 08b6db2
Moved the assume based ballot folding logic to AMDGPUInstCombineIntri…
TejaX-Alaghari 20ad9d2
Simplified the logic to check for CompareValue and return nullptr whe…
TejaX-Alaghari 5e3d47e
Skip the optimization when ballot size < wave size
TejaX-Alaghari 9b6e186
Only proceed with the optiization if ballot arg is an instruction
TejaX-Alaghari 0312bf5
Add additional check to demonstrate E2E impact of this optimization
TejaX-Alaghari bc961db
Address feedback:1. Add the condition to make sure ballot width match…
TejaX-Alaghari f4f1277
Removed unnecessary temp varaible for holding wave front size
TejaX-Alaghari b19b765
Skip only constant args
TejaX-Alaghari 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
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.
We allow ballot.i32 in wave64 mode, which would break these optimizations.
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 was not aware of this! thanks for pointing out.
If you're referring to this condition in Instruction selection, doesn't it mean that "emitting i64 ballots in wave32 mode is supported" and not the other way around.
Please let me know if I'm missing something in understanding this logic.
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.
Hmm. I thought there was also a plan to support "emitting i32 ballots in wave64 mode". Maybe that part was never implemented. I guess your patch is OK then, but personally I would add a check that the ballot size matches the wave size just for safety.
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 added a check in a recent commit for proceeding with this optimization only when
ballotWidth >= waveSize.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.
If you are going to all that trouble, then the tests should check all four combinations: i32 ballot in wave32, i32 ballot in wave64, etc.