Skip to content

[AMDGPU] Merge consecutive wait_alu instruction #128916

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 4 commits into from
Mar 12, 2025
Merged

[AMDGPU] Merge consecutive wait_alu instruction #128916

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5752242
merge consecutive wait_alu instructions
mihajlovicana Feb 26, 2025
8d02e56
skip debug instructions
mihajlovicana Mar 6, 2025
f093ec6
removed skipping hazard state update
mihajlovicana Mar 7, 2025
bba8686
style changes, small fix
mihajlovicana Mar 11, 2025
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
37 changes: 37 additions & 0 deletions llvm/lib/Target/AMDGPU/AMDGPUWaitSGPRHazards.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,32 @@ class AMDGPUWaitSGPRHazards {
BuildMI(MBB, MI, MI->getDebugLoc(), TII->get(AMDGPU::DS_NOP));
}

unsigned mergeMasks(unsigned Mask1, unsigned Mask2) {
unsigned Mask = 0xffff;
Mask = AMDGPU::DepCtr::encodeFieldSaSdst(
Mask, std::min(AMDGPU::DepCtr::decodeFieldSaSdst(Mask1),
AMDGPU::DepCtr::decodeFieldSaSdst(Mask2)));
Mask = AMDGPU::DepCtr::encodeFieldVaVcc(
Mask, std::min(AMDGPU::DepCtr::decodeFieldVaVcc(Mask1),
AMDGPU::DepCtr::decodeFieldVaVcc(Mask2)));
Mask = AMDGPU::DepCtr::encodeFieldVmVsrc(
Mask, std::min(AMDGPU::DepCtr::decodeFieldVmVsrc(Mask1),
AMDGPU::DepCtr::decodeFieldVmVsrc(Mask2)));
Mask = AMDGPU::DepCtr::encodeFieldVaSdst(
Mask, std::min(AMDGPU::DepCtr::decodeFieldVaSdst(Mask1),
AMDGPU::DepCtr::decodeFieldVaSdst(Mask2)));
Mask = AMDGPU::DepCtr::encodeFieldVaVdst(
Mask, std::min(AMDGPU::DepCtr::decodeFieldVaVdst(Mask1),
AMDGPU::DepCtr::decodeFieldVaVdst(Mask2)));
Mask = AMDGPU::DepCtr::encodeFieldHoldCnt(
Mask, std::min(AMDGPU::DepCtr::decodeFieldHoldCnt(Mask1),
AMDGPU::DepCtr::decodeFieldHoldCnt(Mask2)));
Mask = AMDGPU::DepCtr::encodeFieldVaSsrc(
Mask, std::min(AMDGPU::DepCtr::decodeFieldVaSsrc(Mask1),
AMDGPU::DepCtr::decodeFieldVaSsrc(Mask2)));
return Mask;
}

bool runOnMachineBasicBlock(MachineBasicBlock &MBB, bool Emit) {
enum { WA_VALU = 0x1, WA_SALU = 0x2, WA_VCC = 0x4 };

Expand Down Expand Up @@ -362,6 +388,17 @@ class AMDGPUWaitSGPRHazards {
Mask = AMDGPU::DepCtr::encodeFieldVaSdst(Mask, 0);
}
if (Emit) {
if (MI != MBB.instr_begin()) {
MachineBasicBlock::instr_iterator It = std::prev(MI);
while (It != MBB.instr_begin() && It->isDebugInstr())
--It;
if (It->getOpcode() == AMDGPU::S_WAITCNT_DEPCTR) {
Mask = mergeMasks(Mask, It->getOperand(0).getImm());
It->getOperand(0).setImm(Mask);
continue;
Copy link
Contributor

@jayfoad jayfoad Mar 6, 2025

Choose a reason for hiding this comment

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

This will skip the "Update hazards based on defs" code below. Is that really safe?

Also I think you still need to set Emitted = true in this path, since it will be used to determine whether this pass changed anything at all in the MIR.
Copy link
Contributor

@perlfu perlfu Mar 7, 2025

Choose a reason for hiding this comment

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

Yes code below should not be skipped.
Copy link
Contributor Author

@mihajlovicana mihajlovicana Mar 7, 2025

Choose a reason for hiding this comment

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

Thanks for pointing this out !
}
}

auto NewMI = BuildMI(MBB, MI, MI->getDebugLoc(),
TII->get(AMDGPU::S_WAITCNT_DEPCTR))
.addImm(Mask);
Expand Down
36 changes: 36 additions & 0 deletions llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,18 @@ inline unsigned getSaSdstBitWidth() { return 1; }
/// \returns SaSdst bit shift
inline unsigned getSaSdstBitShift() { return 0; }

/// \returns VaSsrc width
inline unsigned getVaSsrcBitWidth() { return 1; }

/// \returns VaSsrc bit shift
inline unsigned getVaSsrcBitShift() { return 8; }

/// \returns HoldCnt bit shift
inline unsigned getHoldCntWidth() { return 1; }

/// \returns HoldCnt bit shift
inline unsigned getHoldCntBitShift() { return 7; }

} // end anonymous namespace

namespace llvm {
Expand Down Expand Up @@ -1740,6 +1752,14 @@ unsigned decodeFieldVaVcc(unsigned Encoded) {
return unpackBits(Encoded, getVaVccBitShift(), getVaVccBitWidth());
}

unsigned decodeFieldVaSsrc(unsigned Encoded) {
return unpackBits(Encoded, getVaSsrcBitShift(), getVaSsrcBitWidth());
}

unsigned decodeFieldHoldCnt(unsigned Encoded) {
return unpackBits(Encoded, getHoldCntBitShift(), getHoldCntWidth());
}

unsigned encodeFieldVmVsrc(unsigned Encoded, unsigned VmVsrc) {
return packBits(VmVsrc, Encoded, getVmVsrcBitShift(), getVmVsrcBitWidth());
}
Expand Down Expand Up @@ -1780,6 +1800,22 @@ unsigned encodeFieldVaVcc(unsigned VaVcc) {
return encodeFieldVaVcc(0xffff, VaVcc);
}

unsigned encodeFieldVaSsrc(unsigned Encoded, unsigned VaSsrc) {
return packBits(VaSsrc, Encoded, getVaSsrcBitShift(), getVaSsrcBitWidth());
}

unsigned encodeFieldVaSsrc(unsigned VaSsrc) {
return encodeFieldVaSsrc(0xfff, VaSsrc);
Copy link
Collaborator

@nhaehnle nhaehnle Mar 10, 2025

Choose a reason for hiding this comment

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

Should be 0xffff?
}

unsigned encodeFieldHoldCnt(unsigned Encoded, unsigned HoldCnt) {
return packBits(HoldCnt, Encoded, getHoldCntBitShift(), getHoldCntWidth());
}

unsigned encodeFieldHoldCnt(unsigned HoldCnt) {
return encodeFieldHoldCnt(0xfff, HoldCnt);
Copy link
Collaborator

@nhaehnle nhaehnle Mar 10, 2025

Choose a reason for hiding this comment

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

Same here: 0xffff?
Copy link
Contributor Author

@mihajlovicana mihajlovicana Mar 11, 2025

Choose a reason for hiding this comment

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

yes, that should be fixed
}

} // namespace DepCtr

//===----------------------------------------------------------------------===//
Expand Down
18 changes: 18 additions & 0 deletions llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1180,6 +1180,12 @@ unsigned decodeFieldVaSdst(unsigned Encoded);
/// \returns Decoded VaVcc from given immediate \p Encoded.
unsigned decodeFieldVaVcc(unsigned Encoded);

/// \returns Decoded SaSrc from given immediate \p Encoded.
unsigned decodeFieldVaSsrc(unsigned Encoded);

/// \returns Decoded HoldCnt from given immediate \p Encoded.
unsigned decodeFieldHoldCnt(unsigned Encoded);

/// \returns \p VmVsrc as an encoded Depctr immediate.
unsigned encodeFieldVmVsrc(unsigned VmVsrc);

Expand Down Expand Up @@ -1210,6 +1216,18 @@ unsigned encodeFieldVaVcc(unsigned VaVcc);
/// \returns \p Encoded combined with encoded \p VaVcc.
unsigned encodeFieldVaVcc(unsigned Encoded, unsigned VaVcc);

/// \returns \p HoldCnt as an encoded Depctr immediate.
unsigned encodeFieldHoldCnt(unsigned HoldCnt);

/// \returns \p Encoded combined with encoded \p HoldCnt.
unsigned encodeFieldHoldCnt(unsigned HoldCnt, unsigned Encoded);

/// \returns \p VaSsrc as an encoded Depctr immediate.
unsigned encodeFieldVaSsrc(unsigned VaSsrc);

/// \returns \p Encoded combined with encoded \p VaSsrc.
unsigned encodeFieldVaSsrc(unsigned Encoded, unsigned VaSsrc);

} // namespace DepCtr

namespace Exp {
Expand Down
78 changes: 78 additions & 0 deletions llvm/test/CodeGen/AMDGPU/merge-consecutive-wait-alus.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
# RUN: llc -mtriple=amdgcn -mcpu=gfx1200 -verify-machineinstrs -run-pass amdgpu-wait-sgpr-hazards -o - %s | FileCheck %s


---
name: merge_consecutive_wait_alus
body: |
bb.0:
liveins: $vgpr0

; CHECK-LABEL: name: merge_consecutive_wait_alus
; CHECK: liveins: $vgpr0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: renamable $sgpr0 = V_CMP_NE_U32_e64 0, $vgpr0, implicit $exec, implicit-def $vcc_lo, implicit-def $vcc_lo
; CHECK-NEXT: S_WAITCNT_DEPCTR 61946
; CHECK-NEXT: renamable $vgpr0 = V_CNDMASK_B32_e64 0, -1, 0, killed $vgpr0, killed $sgpr0, implicit $exec, implicit-def $vcc_lo
renamable $sgpr0 = V_CMP_NE_U32_e64 0, $vgpr0, implicit $exec, implicit-def $vcc_lo, implicit-def $vcc
S_WAITCNT_DEPCTR 65530
renamable $vgpr0 = V_CNDMASK_B32_e64 0, -1, 0, killed $vgpr0, killed $sgpr0, implicit $exec, implicit-def $vcc
...
---
name: merge_consecutive_wait_alus_two_bb
body: |
; CHECK-LABEL: name: merge_consecutive_wait_alus_two_bb
; CHECK: bb.0:
; CHECK-NEXT: successors: %bb.1(0x80000000)
; CHECK-NEXT: liveins: $vgpr0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: renamable $sgpr0 = V_CMP_NE_U32_e64 0, $vgpr0, implicit $exec, implicit-def $vcc_lo, implicit-def $vcc_lo
; CHECK-NEXT: S_WAITCNT_DEPCTR 65530
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.1:
; CHECK-NEXT: liveins: $sgpr0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: S_WAITCNT_DEPCTR 61951
; CHECK-NEXT: renamable $vgpr0 = V_CNDMASK_B32_e64 0, -1, 0, killed $vgpr0, killed $sgpr0, implicit $exec, implicit-def $vcc_lo
bb.0:
liveins: $vgpr0

renamable $sgpr0 = V_CMP_NE_U32_e64 0, $vgpr0, implicit $exec, implicit-def $vcc_lo, implicit-def $vcc
S_WAITCNT_DEPCTR 65530

Copy link
Contributor

@arsenm arsenm Feb 27, 2025

Choose a reason for hiding this comment

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

Test where it's the start of the block and end of the block
Copy link
Contributor

@arsenm arsenm Feb 27, 2025

Choose a reason for hiding this comment

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

Also add a test that skips meta instructions
Copy link
Contributor Author

@mihajlovicana mihajlovicana Feb 28, 2025

Choose a reason for hiding this comment

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

Test where it's the start of the block and end of the block

Do you mean same basic block or successor and predecessor ?
bb.1:
liveins: $sgpr0

renamable $vgpr0 = V_CNDMASK_B32_e64 0, -1, 0, killed $vgpr0, killed $sgpr0, implicit $exec, implicit-def $vcc
...
---
name: meta_instructions
machineFunctionInfo:
body: |
bb.0:
; CHECK-LABEL: name: meta_instructions
; CHECK: renamable $sgpr0 = V_CMP_NE_U32_e64 0, $vgpr0, implicit $exec, implicit-def $vcc_lo, implicit-def $vcc_lo
; CHECK-NEXT: S_WAITCNT_DEPCTR 65530
; CHECK-NEXT: SCHED_BARRIER 0
; CHECK-NEXT: S_WAITCNT_DEPCTR 61951
; CHECK-NEXT: renamable $vgpr0 = V_CNDMASK_B32_e64 0, -1, 0, killed $vgpr0, killed $sgpr0, implicit $exec, implicit-def $vcc_lo
renamable $sgpr0 = V_CMP_NE_U32_e64 0, $vgpr0, implicit $exec, implicit-def $vcc_lo, implicit-def $vcc
S_WAITCNT_DEPCTR 65530
SCHED_BARRIER 0
renamable $vgpr0 = V_CNDMASK_B32_e64 0, -1, 0, killed $vgpr0, killed $sgpr0, implicit $exec, implicit-def $vcc
...
---
name: debug_instruction
machineFunctionInfo:
body: |
bb.0:
; CHECK-LABEL: name: debug_instruction
; CHECK: renamable $sgpr0 = V_CMP_NE_U32_e64 0, $vgpr0, implicit $exec, implicit-def $vcc_lo, implicit-def $vcc_lo
; CHECK-NEXT: S_WAITCNT_DEPCTR 61946
; CHECK-NEXT: DBG_VALUE $sgpr0
; CHECK-NEXT: renamable $vgpr0 = V_CNDMASK_B32_e64 0, -1, 0, killed $vgpr0, killed $sgpr0, implicit $exec, implicit-def $vcc_lo
renamable $sgpr0 = V_CMP_NE_U32_e64 0, $vgpr0, implicit $exec, implicit-def $vcc_lo, implicit-def $vcc
S_WAITCNT_DEPCTR 65530
DBG_VALUE $sgpr0
renamable $vgpr0 = V_CNDMASK_B32_e64 0, -1, 0, killed $vgpr0, killed $sgpr0, implicit $exec, implicit-def $vcc
...