Skip to content

[AMDGPU] Combine or remove redundant waitcnts at the end of each MBB #87539

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Apr 4, 2024
Merged

[AMDGPU] Combine or remove redundant waitcnts at the end of each MBB #87539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5dbe01c
[AMDGPU] Combine or remove redundant waitcnts at the end of each MBB
jayfoad Apr 3, 2024
435d48f
Fix typo in comment
jayfoad Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 15 additions & 30 deletions llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -708,9 +708,6 @@ class SIInsertWaitcnts : public MachineFunctionPass {
WaitcntBrackets &ScoreBrackets,
MachineInstr *OldWaitcntInstr,
bool FlushVmCnt);
bool generateWaitcntBlockEnd(MachineBasicBlock &Block,
WaitcntBrackets &ScoreBrackets,
MachineInstr *OldWaitcntInstr);
bool generateWaitcnt(AMDGPU::Waitcnt Wait,
MachineBasicBlock::instr_iterator It,
MachineBasicBlock &Block, WaitcntBrackets &ScoreBrackets,
Expand Down Expand Up @@ -1902,31 +1899,6 @@ bool SIInsertWaitcnts::generateWaitcntInstBefore(MachineInstr &MI,
OldWaitcntInstr);
}

// Add a waitcnt to flush the LOADcnt, SAMPLEcnt and BVHcnt counters at the
// end of the given block if needed.
bool SIInsertWaitcnts::generateWaitcntBlockEnd(MachineBasicBlock &Block,
WaitcntBrackets &ScoreBrackets,
MachineInstr *OldWaitcntInstr) {
AMDGPU::Waitcnt Wait;

unsigned LoadCntPending = ScoreBrackets.hasPendingEvent(LOAD_CNT);
unsigned SampleCntPending = ScoreBrackets.hasPendingEvent(SAMPLE_CNT);
unsigned BvhCntPending = ScoreBrackets.hasPendingEvent(BVH_CNT);

if (LoadCntPending == 0 && SampleCntPending == 0 && BvhCntPending == 0)
return false;

if (LoadCntPending != 0)
Wait.LoadCnt = 0;
if (SampleCntPending != 0)
Wait.SampleCnt = 0;
if (BvhCntPending != 0)
Wait.BvhCnt = 0;

return generateWaitcnt(Wait, Block.instr_end(), Block, ScoreBrackets,
OldWaitcntInstr);
}

bool SIInsertWaitcnts::generateWaitcnt(AMDGPU::Waitcnt Wait,
MachineBasicBlock::instr_iterator It,
MachineBasicBlock &Block,
Expand Down Expand Up @@ -2355,9 +2327,22 @@ bool SIInsertWaitcnts::insertWaitcntInBlock(MachineFunction &MF,
++Iter;
}

// Flush the LOADcnt, SAMPLEcnt and BVHcnt counters at the end of the block if
// needed.
AMDGPU::Waitcnt Wait;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the variable "Wait" inside the if?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. It's used outside the if.

if (Block.getFirstTerminator() == Block.end() &&
isPreheaderToFlush(Block, ScoreBrackets))
Modified |= generateWaitcntBlockEnd(Block, ScoreBrackets, OldWaitcntInstr);
isPreheaderToFlush(Block, ScoreBrackets)) {
if (ScoreBrackets.hasPendingEvent(LOAD_CNT))
Wait.LoadCnt = 0;
if (ScoreBrackets.hasPendingEvent(SAMPLE_CNT))
Wait.SampleCnt = 0;
if (ScoreBrackets.hasPendingEvent(BVH_CNT))
Wait.BvhCnt = 0;
}

// Combine or remove any redundant waitcnts at the end of the block.
Modified |= generateWaitcnt(Wait, Block.instr_end(), Block, ScoreBrackets,
OldWaitcntInstr);

return Modified;
}
Expand Down
4 changes: 0 additions & 4 deletions llvm/test/CodeGen/AMDGPU/GlobalISel/divergent-control-flow.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ define i32 @divergent_if_swap_brtarget_order0(i32 %value) {
; CHECK-NEXT: s_waitcnt vmcnt(0)
; CHECK-NEXT: .LBB0_2: ; %endif
; CHECK-NEXT: s_or_b64 exec, exec, s[4:5]
; CHECK-NEXT: s_waitcnt vmcnt(0)
; CHECK-NEXT: s_setpc_b64 s[30:31]
entry:
%c = icmp ne i32 %value, 0
Expand Down Expand Up @@ -44,7 +43,6 @@ define i32 @divergent_if_swap_brtarget_order1(i32 %value) {
; CHECK-NEXT: s_waitcnt vmcnt(0)
; CHECK-NEXT: .LBB1_2: ; %endif
; CHECK-NEXT: s_or_b64 exec, exec, s[4:5]
; CHECK-NEXT: s_waitcnt vmcnt(0)
; CHECK-NEXT: s_setpc_b64 s[30:31]
entry:
%c = icmp ne i32 %value, 0
Expand Down Expand Up @@ -74,7 +72,6 @@ define i32 @divergent_if_nonboolean_condition0(i32 %value) {
; CHECK-NEXT: s_waitcnt vmcnt(0)
; CHECK-NEXT: .LBB2_2: ; %endif
; CHECK-NEXT: s_or_b64 exec, exec, s[4:5]
; CHECK-NEXT: s_waitcnt vmcnt(0)
; CHECK-NEXT: s_setpc_b64 s[30:31]
entry:
%c = trunc i32 %value to i1
Expand Down Expand Up @@ -106,7 +103,6 @@ define i32 @divergent_if_nonboolean_condition1(ptr addrspace(1) %ptr) {
; CHECK-NEXT: s_waitcnt vmcnt(0)
; CHECK-NEXT: .LBB3_2: ; %endif
; CHECK-NEXT: s_or_b64 exec, exec, s[4:5]
; CHECK-NEXT: s_waitcnt vmcnt(0)
; CHECK-NEXT: s_setpc_b64 s[30:31]
entry:
%value = load i32, ptr addrspace(1) %ptr
Expand Down
2 changes: 0 additions & 2 deletions llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.is.private.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,6 @@ define amdgpu_kernel void @is_private_sgpr(ptr %ptr) {
; GFX11-NEXT: global_store_b32 v[0:1], v0, off dlc
; GFX11-NEXT: s_waitcnt_vscnt null, 0x0
; GFX11-NEXT: .LBB1_2: ; %bb1
; GFX11-NEXT: s_nop 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dont' understand this change, no waits changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SIInsertWaitcnts only inserts that s_nop/s_sendmsg sequence if it thinks that vscnt might be non-zero at the s_endpgm. I think what happened here is that the pass now takes notice of the "s_waitcnt_vscnt 0" at the end of the previous MBB. You're right that it didn't get changed, but the pass's bookkeeping got updated with the knowledge that vscnt must be zero after that.

; GFX11-NEXT: s_sendmsg sendmsg(MSG_DEALLOC_VGPRS)
; GFX11-NEXT: s_endpgm
%val = call i1 @llvm.amdgcn.is.private(ptr %ptr)
br i1 %val, label %bb0, label %bb1
Expand Down
2 changes: 0 additions & 2 deletions llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.is.shared.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,6 @@ define amdgpu_kernel void @is_local_sgpr(ptr %ptr) {
; GFX11-NEXT: global_store_b32 v[0:1], v0, off dlc
; GFX11-NEXT: s_waitcnt_vscnt null, 0x0
; GFX11-NEXT: .LBB1_2: ; %bb1
; GFX11-NEXT: s_nop 0
; GFX11-NEXT: s_sendmsg sendmsg(MSG_DEALLOC_VGPRS)
; GFX11-NEXT: s_endpgm
%val = call i1 @llvm.amdgcn.is.shared(ptr %ptr)
br i1 %val, label %bb0, label %bb1
Expand Down
Loading