[llvm-branch-commits] [llvm] [AMDGPU][SIInsertWaitCnts] Remove VMemTypes (PR #206440)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Jun 29 02:30:26 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-amdgpu
Author: Pierre van Houtryve (Pierre-vh)
<details>
<summary>Changes</summary>
This can be considered a RFC. I'd personally like to get rid of VMEMTypes
but I don't know if anyone feels strongly that they should be kept.
My motivation for removing VMemTypes is simple: They are just a repeat
of VMEM events, just under a different name, and messier (defined as a
basic enum but actually stored as a bitmask later). It's just confusing.
This patch eliminates the need for them by:
- Adding a new entrypoint in AMDGPUHWEvents to get the basic set of
VMEM events issued by a VMEM Instruction.
- Set BVH/SAMPLER events irrespective of whether the HW can track them.
These events exist anyway, it should be up to InsertWaitCnt to deal with them
properly (which is easy, only `counterOutOfOrder` needed work).
- Tracking an additional set of per-VGPR "PendingEvents" which is
set using the "basic set of VMEM events" and cleared as needed.
I believe this is worth it because it streamlines the implementation
of `SIInsertWaitCnts` and eliminates a bit of code duplication
along the way.
---
Full diff: https://github.com/llvm/llvm-project/pull/206440.diff
3 Files Affected:
- (modified) llvm/lib/Target/AMDGPU/AMDGPUHWEvents.cpp (+7-6)
- (modified) llvm/lib/Target/AMDGPU/AMDGPUHWEvents.h (+11)
- (modified) llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp (+37-54)
``````````diff
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUHWEvents.cpp b/llvm/lib/Target/AMDGPU/AMDGPUHWEvents.cpp
index 1087b08ac9d91..acfc54f9db04e 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUHWEvents.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUHWEvents.cpp
@@ -56,8 +56,8 @@ static HWEvents getExpertSchedulingEventType(const MachineInstr &Inst,
return HWEvents::NONE;
}
-static HWEvents getVmemHWEvent(const MachineInstr &Inst, const GCNSubtarget &ST,
- const SIInstrInfo &TII) {
+HWEvents getSimplifiedVMEMEventsFor(const MachineInstr &Inst,
+ const SIInstrInfo &TII) {
switch (Inst.getOpcode()) {
// FIXME: GLOBAL_INV needs to be tracked with xcnt too.
case AMDGPU::GLOBAL_INV:
@@ -82,7 +82,8 @@ static HWEvents getVmemHWEvent(const MachineInstr &Inst, const GCNSubtarget &ST,
return HWEvents::SCRATCH_WRITE_ACCESS;
return HWEvents::VMEM_WRITE_ACCESS;
}
- if (!ST.hasExtendedWaitCounts() || SIInstrInfo::isFLAT(Inst))
+
+ if (SIInstrInfo::isFLAT(Inst))
return HWEvents::VMEM_READ_ACCESS;
if (SIInstrInfo::isImage(Inst)) {
@@ -116,14 +117,14 @@ static HWEvents getEventsForImpl(const MachineInstr &Inst,
if (TII.isFLAT(Inst)) {
if (SIInstrInfo::isGFX12CacheInvOrWBInst(Inst.getOpcode()))
- return getVmemHWEvent(Inst, ST, TII);
+ return getSimplifiedVMEMEventsFor(Inst, TII);
assert(Inst.mayLoadOrStore());
HWEvents E = HWEvents::NONE;
if (TII.mayAccessVMEMThroughFlat(Inst)) {
if (ST.hasWaitXcnt())
E |= HWEvents::VMEM_GROUP;
- E |= getVmemHWEvent(Inst, ST, TII);
+ E |= getSimplifiedVMEMEventsFor(Inst, TII);
}
if (TII.mayAccessLDSThroughFlat(Inst))
@@ -144,7 +145,7 @@ static HWEvents getEventsForImpl(const MachineInstr &Inst,
// BUFFER_WBL2 is included here because unlike invalidates, has to be
// followed "S_WAITCNT vmcnt(0)" is needed after to ensure the writeback has
// completed.
- HWEvents E = getVmemHWEvent(Inst, ST, TII);
+ HWEvents E = getSimplifiedVMEMEventsFor(Inst, TII);
if (ST.hasWaitXcnt())
E |= HWEvents::VMEM_GROUP;
if (ST.vmemWriteNeedsExpWaitcnt() &&
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUHWEvents.h b/llvm/lib/Target/AMDGPU/AMDGPUHWEvents.h
index 5901ee40177b3..bc5e10fae1c42 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUHWEvents.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPUHWEvents.h
@@ -21,6 +21,7 @@ namespace llvm {
class GCNSubtarget;
class MachineInstr;
class raw_ostream;
+class SIInstrInfo;
namespace AMDGPU {
@@ -164,6 +165,16 @@ class HWEvents {
value_type Data = NONE;
};
+/// \param Inst A VMEM instruction (as per `SIInstrInfo::isVMEM`).
+/// \returns the simplified set of events triggered by the VMEM instruction \p
+/// Inst. The returned mask is not exhaustive, but is guaranteed to be a subset
+/// of the mask that'd be returned by \ref getEventsFor.
+///
+/// Useful to quickly categorize VMEM instructions without having to fetch all
+/// events.
+HWEvents getSimplifiedVMEMEventsFor(const MachineInstr &Inst,
+ const SIInstrInfo &TII);
+
/// \returns A bitmask of HWEvent triggered by \p Inst
HWEvents getEventsFor(const MachineInstr &Inst, const GCNSubtarget &ST,
bool IsExpertMode);
diff --git a/llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp b/llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp
index c261ae9c4b6c6..c496666601967 100644
--- a/llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp
@@ -114,21 +114,6 @@ static constexpr VMEMID toVMEMID(MCRegUnit RU) {
namespace {
-// Enumerate different types of result-returning VMEM operations. Although
-// s_waitcnt orders them all with a single vmcnt counter, in the absence of
-// s_waitcnt only instructions of the same VmemType are guaranteed to write
-// their results in order -- so there is no need to insert an s_waitcnt between
-// two instructions of the same type that write the same vgpr.
-enum VmemType {
- // BUF instructions and MIMG instructions without a sampler.
- VMEM_NOSAMPLER,
- // MIMG instructions with a sampler.
- VMEM_SAMPLER,
- // BVH instructions
- VMEM_BVH,
- NUM_VMEM_TYPES
-};
-
// Maps values of InstCounterType to the instruction that waits on that
// counter. Only used if GCNSubtarget::hasExtendedWaitCounts()
// returns true, and does not cover VA_VDST or VM_VSRC.
@@ -163,26 +148,6 @@ static bool isNormalMode(AMDGPU::InstCounterType MaxCounter) {
}
#endif // NDEBUG
-VmemType getVmemType(const MachineInstr &Inst) {
- assert(updateVMCntOnly(Inst));
- if (!SIInstrInfo::isImage(Inst))
- return VMEM_NOSAMPLER;
- const AMDGPU::MIMGInfo *Info = AMDGPU::getMIMGInfo(Inst.getOpcode());
- const AMDGPU::MIMGBaseOpcodeInfo *BaseInfo =
- AMDGPU::getMIMGBaseOpcodeInfo(Info->BaseOpcode);
-
- if (BaseInfo->BVH)
- return VMEM_BVH;
-
- // We have to make an additional check for isVSAMPLE here since some
- // instructions don't have a sampler, but are still classified as sampler
- // instructions for the purposes of e.g. waitcnt.
- if (BaseInfo->Sampler || BaseInfo->MSAA || SIInstrInfo::isVSAMPLE(Inst))
- return VMEM_SAMPLER;
-
- return VMEM_NOSAMPLER;
-}
-
class WaitcntBrackets;
// This abstracts the logic for generating and updating S_WAIT* instructions
@@ -636,20 +601,20 @@ class WaitcntBrackets {
void setPendingGDS() { LastGDS = ScoreUBs[AMDGPU::DS_CNT]; }
// Return true if there might be pending writes to the vgpr-interval by VMEM
- // instructions with types different from V.
- bool hasOtherPendingVmemTypes(MCPhysReg Reg, VmemType V) const {
+ // instructions where the HWEvents in VGPRContext are not contained in E.
+ bool hasDifferentVGPRPendingEvents(MCPhysReg Reg, HWEvents E) const {
for (MCRegUnit RU : regunits(Reg)) {
auto It = VMem.find(toVMEMID(RU));
- if (It != VMem.end() && (It->second.VMEMTypes & ~(1 << V)))
+ if (It != VMem.end() && (It->second.VGPRPendingEvents & ~E).any())
return true;
}
return false;
}
- void clearVgprVmemTypes(MCPhysReg Reg) {
+ void clearVGPRPendingEvents(MCPhysReg Reg) {
for (MCRegUnit RU : regunits(Reg)) {
if (auto It = VMem.find(toVMEMID(RU)); It != VMem.end()) {
- It->second.VMEMTypes = 0;
+ It->second.VGPRPendingEvents = HWEvents::NONE;
if (It->second.empty())
VMem.erase(It);
}
@@ -772,10 +737,13 @@ class WaitcntBrackets {
struct VMEMInfo {
// Scores for all instruction counters. Zero-initialized.
CounterValueArray Scores{};
- // Bitmask of the VmemTypes of VMEM instructions for this VGPR.
- unsigned VMEMTypes = 0;
+ // For VGPRs, we need to track an additional fine-grained set of pending
+ // events.
+ HWEvents VGPRPendingEvents;
- bool empty() const { return all_of(Scores, equal_to(0)) && !VMEMTypes; }
+ bool empty() const {
+ return all_of(Scores, equal_to(0)) && !VGPRPendingEvents;
+ }
};
/// Wait cnt scores for every sgpr, the DS_CNT (corresponding to LGKMcnt
@@ -884,7 +852,7 @@ bool WaitcntBrackets::hasPointSamplePendingVmemTypes(const MachineInstr &MI,
if (!hasPointSampleAccel(MI))
return false;
- return hasOtherPendingVmemTypes(Reg, VMEM_NOSAMPLER);
+ return hasDifferentVGPRPendingEvents(Reg, HWEvents::VMEM_READ_ACCESS);
}
void WaitcntBrackets::updateByEvent(HWEvents E, MachineInstr &Inst) {
@@ -1025,16 +993,16 @@ void WaitcntBrackets::updateByEvent(HWEvents E, MachineInstr &Inst) {
if (updateVMCntOnly(Inst)) {
// updateVMCntOnly should only leave us with VGPRs
// MUBUF, MTBUF, MIMG, FlatGlobal, and FlatScratch only have VGPR/AGPR
- // defs. That's required for a sane index into `VgprMemTypes` below
+ // defs.
assert(TRI.isVectorRegister(MRI, Op.getReg()));
- VmemType V = getVmemType(Inst);
- unsigned char TypesMask = 1 << V;
+ HWEvents VGPRContext =
+ AMDGPU::getSimplifiedVMEMEventsFor(Inst, Context->TII);
// If instruction can have Point Sample Accel applied, we have to flag
// this with another potential dependency
if (hasPointSampleAccel(Inst))
- TypesMask |= 1 << VMEM_NOSAMPLER;
+ VGPRContext |= HWEvents::VMEM_READ_ACCESS;
for (MCRegUnit RU : regunits(Op.getReg().asMCReg()))
- VMem[toVMEMID(RU)].VMEMTypes |= TypesMask;
+ VMem[toVMEMID(RU)].VGPRPendingEvents |= VGPRContext;
}
}
setScoreByOperand(Op, T, CurrScore);
@@ -1552,6 +1520,19 @@ bool WaitcntBrackets::counterOutOfOrder(AMDGPU::InstCounterType T) const {
HWEvents Events = PendingEvents & Context->getWaitEvents(T);
+ // If the target does not have extended counters, VMEM_BVH/SAMPLE_READ
+ // events are equivalent to VMEM_READ_ACCESS. We do not go out of order in
+ // such cases.
+ static constexpr HWEvents ExtendedImageEvents =
+ HWEvents::VMEM_SAMPLER_READ_ACCESS | HWEvents::VMEM_BVH_READ_ACCESS;
+ if (!Context->ST.hasExtendedWaitCounts() &&
+ (Events & ExtendedImageEvents).any()) {
+ Events -= ExtendedImageEvents; // TODO: Tests pass even if I only use
+ // VMEM_SAMPLER_READ_ACCESS which isn't
+ // normal; indicates weak testing coverage
+ Events |= HWEvents::VMEM_READ_ACCESS;
+ }
+
// GLOBAL_INV completes in-order with other LOAD_CNT events,
// so having GLOBAL_INV_ACCESS mixed with other LOAD_CNT
// events doesn't cause out-of-order completion.
@@ -2451,7 +2432,8 @@ bool SIInsertWaitcnts::generateWaitcntInstBefore(
// Additionally check instructions where Point Sample Acceleration
// might be applied.
if (Op.isUse() || !updateVMCntOnly(MI) ||
- ScoreBrackets.hasOtherPendingVmemTypes(Reg, getVmemType(MI)) ||
+ ScoreBrackets.hasDifferentVGPRPendingEvents(
+ Reg, AMDGPU::getSimplifiedVMEMEventsFor(MI, TII)) ||
ScoreBrackets.hasPointSamplePendingVmemTypes(MI, Reg) ||
!ST.hasVmemWriteVgprInOrder()) {
ScoreBrackets.determineWaitForPhysReg(AMDGPU::LOAD_CNT, Reg, Wait,
@@ -2460,7 +2442,7 @@ bool SIInsertWaitcnts::generateWaitcntInstBefore(
MI);
ScoreBrackets.determineWaitForPhysReg(AMDGPU::BVH_CNT, Reg, Wait,
MI);
- ScoreBrackets.clearVgprVmemTypes(Reg);
+ ScoreBrackets.clearVGPRPendingEvents(Reg);
}
if (Op.isDef() ||
@@ -2848,9 +2830,10 @@ bool WaitcntBrackets::merge(const WaitcntBrackets &Other) {
for (auto &[TID, Info] : VMem) {
if (auto It = Other.VMem.find(TID); It != Other.VMem.end()) {
- unsigned char NewVmemTypes = Info.VMEMTypes | It->second.VMEMTypes;
- StrictDom |= NewVmemTypes != Info.VMEMTypes;
- Info.VMEMTypes = NewVmemTypes;
+ HWEvents NewVGPRContext =
+ Info.VGPRPendingEvents | It->second.VGPRPendingEvents;
+ StrictDom |= NewVGPRContext != Info.VGPRPendingEvents;
+ Info.VGPRPendingEvents = NewVGPRContext;
}
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/206440
More information about the llvm-branch-commits
mailing list