-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[AMDGPU] Omit image waits in function prologue on gfx1250 #145097
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
[AMDGPU] Omit image waits in function prologue on gfx1250 #145097
Conversation
@llvm/pr-subscribers-backend-amdgpu Author: Stanislav Mekhanoshin (rampitec) ChangesFull diff: https://github.com/llvm/llvm-project/pull/145097.diff 1 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp b/llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp
index f7b88bf2d5ebc..a60e2102d4e8c 100644
--- a/llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp
@@ -2681,6 +2681,10 @@ bool SIInsertWaitcnts::run(MachineFunction &MF) {
if (CT == LOAD_CNT || CT == DS_CNT || CT == STORE_CNT)
continue;
+ if (!ST->hasImageInsts() &&
+ (CT == EXP_CNT || CT == SAMPLE_CNT || CT == BVH_CNT))
+ continue;
+
BuildMI(EntryBB, I, DebugLoc(),
TII->get(instrsForExtendedCounterTypes[CT]))
.addImm(0);
|
@@ -2681,6 +2681,10 @@ bool SIInsertWaitcnts::run(MachineFunction &MF) { | |||
if (CT == LOAD_CNT || CT == DS_CNT || CT == STORE_CNT) | |||
continue; | |||
|
|||
if (!ST->hasImageInsts() && | |||
(CT == EXP_CNT || CT == SAMPLE_CNT || CT == BVH_CNT)) |
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.
Using hasImageInsts to control EXP_CNT looks weird - will this be changed again 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 may add a feature, but the problem with it we do not like negative features. So a FeatureEXPInsts will need to be added almost everywhere. Sound very invasive.
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.
Well I guess it's OK for now
No description provided.