[AMDGPU] Occupancy w.r.t. workgroup size range is also a range #123748
Conversation
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
All unit tests updated.
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-llvm-globalisel @llvm/pr-subscribers-backend-amdgpu Author: Lucas Ramirez (lucas-rami) ChangesOccupancy (i.e., the number of waves per EU) depends, in addition to register usage, on per-workgroup LDS usage as well as on the range of possible workgroup sizes. Mirroring the latter, occupancy should therefore be expressed as a range since different group sizes generally yield different achievable occupancies.
Computing the achievable occupancy range is surprisingly involved. Minimum/maximum workgroup sizes do not necessarily yield maximum/minimum occupancies i.e., sometimes workgroup sizes inside the range yield the occupancy bounds. The implementation finds these sizes in constant time; heavy documentation explains the rationale behind the sometimes relatively obscure calculations. As a justifying example, consider a target with 10 waves / EU, 4 EUs/CU, 64-wide waves. Also consider a function with no LDS usage and a flat workgroup size range of [513,1024].
Therefore the achievable occupancy range for this function is not [8,9] as the group size bounds directly yield, but [7,10]. Naturally this change causes a lot of test churn as instruction scheduling is driven by achievable occupancy estimates. In most unit tests the flat workgroup size range is the default [1,1024] which, ignoring potential LDS limitations, would previously produce a scalar occupancy of 8 (derived from 1024) on a lot of targets, whereas we now consider the maximum occupancy to be 10 in such cases. Most tests are updated automatically and checked manually for sanity. I also manually changed some non-automatically generated assertions when necessary. In particular, a previously broken test ( Fixes #118220. Patch is 2.06 MiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/123748.diff 93 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp b/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
index 737b2f740d6f77..bdf12ccb302cbc 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
@@ -455,7 +455,7 @@ void AMDGPUAsmPrinter::validateMCResourceInfo(Function &F) {
uint64_t NumSGPRsForWavesPerEU = std::max(
{NumSgpr, (uint64_t)1, (uint64_t)STM.getMinNumSGPRs(MaxWaves)});
const MCExpr *OccupancyExpr = AMDGPUMCExpr::createOccupancy(
- STM.computeOccupancy(F, MFI.getLDSSize()),
+ STM.getOccupancyWithWorkGroupSizes(*MF).second,
MCConstantExpr::create(NumSGPRsForWavesPerEU, OutContext),
MCConstantExpr::create(NumVGPRsForWavesPerEU, OutContext), STM,
OutContext);
@@ -1262,7 +1262,7 @@ void AMDGPUAsmPrinter::getSIProgramInfo(SIProgramInfo &ProgInfo,
}
ProgInfo.Occupancy = AMDGPUMCExpr::createOccupancy(
- STM.computeOccupancy(F, ProgInfo.LDSSize), ProgInfo.NumSGPRsForWavesPerEU,
+ STM.computeOccupancy(F, ProgInfo.LDSSize).second, ProgInfo.NumSGPRsForWavesPerEU,
ProgInfo.NumVGPRsForWavesPerEU, STM, Ctx);
const auto [MinWEU, MaxWEU] =
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp b/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
index e27ef71c1c0883..907f82ed7fc528 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
@@ -1344,7 +1344,7 @@ bool AMDGPUPromoteAllocaImpl::hasSufficientLocalMem(const Function &F) {
}
unsigned MaxOccupancy =
- ST.getOccupancyWithLocalMemSize(CurrentLocalMemUsage, F);
+ ST.getOccupancyWithWorkGroupSizes(CurrentLocalMemUsage, F).second;
// Restrict local memory usage so that we don't drastically reduce occupancy,
// unless it is already significantly reduced.
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp b/llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp
index ae563df2a7a128..da729d4dc7e089 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp
@@ -55,55 +55,92 @@ AMDGPUSubtarget::getMaxLocalMemSizeWithWaveCount(unsigned NWaves,
return getLocalMemorySize() / WorkGroupsPerCU;
}
-// FIXME: Should return min,max range.
-//
-// Returns the maximum occupancy, in number of waves per SIMD / EU, that can
-// be achieved when only the given function is running on the machine; and
-// taking into account the overall number of wave slots, the (maximum) workgroup
-// size, and the per-workgroup LDS allocation size.
-unsigned AMDGPUSubtarget::getOccupancyWithLocalMemSize(uint32_t Bytes,
- const Function &F) const {
- const unsigned MaxWorkGroupSize = getFlatWorkGroupSizes(F).second;
- const unsigned MaxWorkGroupsPerCu = getMaxWorkGroupsPerCU(MaxWorkGroupSize);
- if (!MaxWorkGroupsPerCu)
- return 0;
-
- const unsigned WaveSize = getWavefrontSize();
-
- // FIXME: Do we need to account for alignment requirement of LDS rounding the
- // size up?
- // Compute restriction based on LDS usage
- unsigned NumGroups = getLocalMemorySize() / (Bytes ? Bytes : 1u);
-
- // This can be queried with more LDS than is possible, so just assume the
- // worst.
- if (NumGroups == 0)
- return 1;
-
- NumGroups = std::min(MaxWorkGroupsPerCu, NumGroups);
-
- // Round to the number of waves per CU.
- const unsigned MaxGroupNumWaves = divideCeil(MaxWorkGroupSize, WaveSize);
- unsigned MaxWaves = NumGroups * MaxGroupNumWaves;
-
- // Number of waves per EU (SIMD).
- MaxWaves = divideCeil(MaxWaves, getEUsPerCU());
-
- // Clamp to the maximum possible number of waves.
- MaxWaves = std::min(MaxWaves, getMaxWavesPerEU());
+std::pair<unsigned, unsigned>
+AMDGPUSubtarget::getOccupancyWithWorkGroupSizes(uint32_t LDSBytes,
+ const Function &F) const {
+ // FIXME: Is there an allocation granularity for the LDS? If so we would need
+ // to make sure the amount of bytes is aligned on that granularity.
+
+ // Compute occupancy restriction based on LDS usage.
+ const unsigned MaxWGsLDS = getLocalMemorySize() / std::max(LDSBytes, 1u);
+
+ // Queried LDS size may be larger than available on a CU, in which case we
+ // consider the only achievable occupancy to be 1, in line with what we
+ // consider the occupancy to be when the number of requested registers in a
+ // particular bank is higher than the number of available ones in that bank.
+ if (!MaxWGsLDS)
+ return {1, 1};
+
+ const unsigned WaveSize = getWavefrontSize(), WavesPerEU = getMaxWavesPerEU();
+ const unsigned WaveSlotsPerCU = WavesPerEU * getEUsPerCU();
+
+ auto PropsFromWGSize = [&](unsigned WGSize)
+ -> std::tuple<const unsigned, const unsigned, unsigned> {
+ unsigned WavesPerWG = divideCeil(WGSize, WaveSize);
+ unsigned WGsPerCU = std::min(getMaxWorkGroupsPerCU(WGSize), MaxWGsLDS);
+ return {WavesPerWG, WGsPerCU, WavesPerWG * WGsPerCU};
+ };
+
+ // The maximum group size will generally yield the minimum number of
+ // workgroups, maximum number of waves, and minimum occupancy. The opposite is
+ // generally true for the minimum group size. LDS or barrier ressource
+ // limitations can flip those minimums/maximums.
+ const auto [MinWGSize, MaxWGSize] = getFlatWorkGroupSizes(F);
+ auto [MinWavesPerWG, MaxWGsPerCU, MaxWavesPerCU] = PropsFromWGSize(MinWGSize);
+ auto [MaxWavesPerWG, MinWGsPerCU, MinWavesPerCU] = PropsFromWGSize(MaxWGSize);
+
+ // It is possible that we end up with flipped minimum and maximum number of
+ // waves per CU when the number of minimum/maximum concurrent groups on the CU
+ // is limited by LDS usage or barrier ressources.
+ if (MinWavesPerCU >= MaxWavesPerCU) {
+ std::swap(MinWavesPerCU, MaxWavesPerCU);
+ } else {
+ // Look for a potential smaller group size than the maximum which decreases
+ // the concurrent number of waves on the CU for the same number of
+ // concurrent workgroups on the CU.
+ unsigned MinWavesPerCUForWGSize =
+ divideCeil(WaveSlotsPerCU, MinWGsPerCU + 1) * MinWGsPerCU;
+ if (MinWavesPerCU > MinWavesPerCUForWGSize) {
+ unsigned ExcessSlots = MinWavesPerCU - MinWavesPerCUForWGSize;
+ if (unsigned ExcessSlotsPerWG = ExcessSlots / MinWGsPerCU) {
+ // There may exist a smaller group size than the maximum that achieves
+ // the minimum number of waves per CU. This group size is the largest
+ // possible size that requires MaxWavesPerWG - E waves where E is
+ // maximized under the following constraints.
+ // 1. 0 <= E <= ExcessSlotsPerWG
+ // 2. (MaxWavesPerWG - E) * WaveSize >= MinWGSize
+ MinWavesPerCU -= MinWGsPerCU * std::min(ExcessSlotsPerWG,
+ MaxWavesPerWG - MinWavesPerWG);
+ }
+ }
- // FIXME: Needs to be a multiple of the group size?
- //MaxWaves = MaxGroupNumWaves * (MaxWaves / MaxGroupNumWaves);
+ // Look for a potential larger group size than the minimum which increases
+ // the concurrent number of waves on the CU for the same number of
+ // concurrent workgroups on the CU.
+ unsigned LeftoverSlots = WaveSlotsPerCU - MaxWGsPerCU * MinWavesPerWG;
+ if (unsigned LeftoverSlotsPerWG = LeftoverSlots / MaxWGsPerCU) {
+ // There may exist a larger group size than the minimum that achieves the
+ // maximum number of waves per CU. This group size is the smallest
+ // possible size that requires MinWavesPerWG + L waves where L is
+ // maximized under the following constraints.
+ // 1. 0 <= L <= LeftoverSlotsPerWG
+ // 2. (MinWavesPerWG + L - 1) * WaveSize <= MaxWGSize
+ MaxWavesPerCU += MaxWGsPerCU * std::min(LeftoverSlotsPerWG,
+ ((MaxWGSize - 1) / WaveSize) + 1 -
+ MinWavesPerWG);
+ }
+ }
- assert(MaxWaves > 0 && MaxWaves <= getMaxWavesPerEU() &&
- "computed invalid occupancy");
- return MaxWaves;
+ // Return the minimum/maximum number of waves on any EU, assuming that all
+ // wavefronts are spread across all EUs as evenly as possible.
+ return {std::clamp(MinWavesPerCU / getEUsPerCU(), 1U, WavesPerEU),
+ std::clamp(divideCeil(MaxWavesPerCU, getEUsPerCU()), 1U, WavesPerEU)};
}
-unsigned
-AMDGPUSubtarget::getOccupancyWithLocalMemSize(const MachineFunction &MF) const {
+std::pair<unsigned, unsigned> AMDGPUSubtarget::getOccupancyWithWorkGroupSizes(
+ const MachineFunction &MF) const {
const auto *MFI = MF.getInfo<SIMachineFunctionInfo>();
- return getOccupancyWithLocalMemSize(MFI->getLDSSize(), MF.getFunction());
+ return getOccupancyWithWorkGroupSizes(MFI->getLDSSize(), MF.getFunction());
}
std::pair<unsigned, unsigned>
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h b/llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h
index 7701fef5365841..5944b69ce64162 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h
@@ -127,11 +127,21 @@ class AMDGPUSubtarget {
unsigned getMaxLocalMemSizeWithWaveCount(unsigned WaveCount,
const Function &) const;
- /// Inverse of getMaxLocalMemWithWaveCount. Return the maximum wavecount if
- /// the given LDS memory size is the only constraint.
- unsigned getOccupancyWithLocalMemSize(uint32_t Bytes, const Function &) const;
+ /// Subtarget's minimum/maximum occupancy, in number of waves per EU, that can
+ /// be achieved when the only function running on a CU is \p F and each
+ /// workgroup running the function requires \p LDSBytes bytes of LDS space.
+ /// This notably depends on the range of allowed flat group sizes for the
+ /// function and hardware characteristics.
+ std::pair<unsigned, unsigned>
+ getOccupancyWithWorkGroupSizes(uint32_t LDSBytes, const Function &F) const;
- unsigned getOccupancyWithLocalMemSize(const MachineFunction &MF) const;
+ /// Subtarget's minimum/maximum occupancy, in number of waves per EU, that can
+ /// be achieved when the only function running on a CU is \p MF. This notably
+ /// depends on the range of allowed flat group sizes for the function, the
+ /// amount of per-workgroup LDS space required by the function, and hardware
+ /// characteristics.
+ std::pair<unsigned, unsigned>
+ getOccupancyWithWorkGroupSizes(const MachineFunction &MF) const;
bool isAmdHsaOS() const {
return TargetTriple.getOS() == Triple::AMDHSA;
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
index f8b60630bb7f6c..05acd418a1cd0d 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
@@ -1717,7 +1717,7 @@ bool GCNTargetMachine::parseMachineFunctionInfo(
if (MFI->Occupancy == 0) {
// Fixup the subtarget dependent default value.
- MFI->Occupancy = ST.computeOccupancy(MF.getFunction(), MFI->getLDSSize());
+ MFI->Occupancy = ST.getOccupancyWithWorkGroupSizes(MF).second;
}
auto parseRegister = [&](const yaml::StringValue &RegName, Register &RegVal) {
diff --git a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
index f5bbc5482d347c..b00105ae9bd528 100644
--- a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
@@ -1089,9 +1089,8 @@ bool PreRARematStage::initGCNSchedStage() {
return false;
const TargetInstrInfo *TII = MF.getSubtarget().getInstrInfo();
- // Check maximum occupancy
- if (ST.computeOccupancy(MF.getFunction(), MFI.getLDSSize()) ==
- DAG.MinOccupancy)
+ // Rematerialization will not help if occupancy is not limited by reg usage.
+ if (ST.getOccupancyWithWorkGroupSizes(MF).second == DAG.MinOccupancy)
return false;
// FIXME: This pass will invalidate cached MBBLiveIns for regions
@@ -1272,8 +1271,8 @@ void GCNSchedStage::checkScheduling() {
return;
}
- unsigned TargetOccupancy =
- std::min(S.getTargetOccupancy(), ST.getOccupancyWithLocalMemSize(MF));
+ unsigned TargetOccupancy = std::min(
+ S.getTargetOccupancy(), ST.getOccupancyWithWorkGroupSizes(MF).second);
unsigned WavesAfter =
std::min(TargetOccupancy, PressureAfter.getOccupancy(ST));
unsigned WavesBefore =
diff --git a/llvm/lib/Target/AMDGPU/GCNSubtarget.cpp b/llvm/lib/Target/AMDGPU/GCNSubtarget.cpp
index 117afc4a8e8c60..22a550450dc2eb 100644
--- a/llvm/lib/Target/AMDGPU/GCNSubtarget.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNSubtarget.cpp
@@ -405,16 +405,16 @@ unsigned GCNSubtarget::getReservedNumSGPRs(const Function &F) const {
return getBaseReservedNumSGPRs(KernelUsesFlatScratch);
}
-unsigned GCNSubtarget::computeOccupancy(const Function &F, unsigned LDSSize,
- unsigned NumSGPRs,
- unsigned NumVGPRs) const {
- unsigned Occupancy =
- std::min(getMaxWavesPerEU(), getOccupancyWithLocalMemSize(LDSSize, F));
- if (NumSGPRs)
- Occupancy = std::min(Occupancy, getOccupancyWithNumSGPRs(NumSGPRs));
- if (NumVGPRs)
- Occupancy = std::min(Occupancy, getOccupancyWithNumVGPRs(NumVGPRs));
- return Occupancy;
+std::pair<unsigned, unsigned>
+GCNSubtarget::computeOccupancy(const Function &F, unsigned LDSSize,
+ unsigned NumSGPRs, unsigned NumVGPRs) const {
+ auto [MinOcc, MaxOcc] = getOccupancyWithWorkGroupSizes(LDSSize, F);
+ unsigned SGPROcc = getOccupancyWithNumSGPRs(NumSGPRs);
+ unsigned VGPROcc = getOccupancyWithNumVGPRs(NumVGPRs);
+
+ // Maximum occupancy may be further limited by high SGPR/VGPR usage.
+ MaxOcc = std::min(MaxOcc, std::min(SGPROcc, VGPROcc));
+ return {std::min(MinOcc, MaxOcc), MaxOcc};
}
unsigned GCNSubtarget::getBaseMaxNumSGPRs(
diff --git a/llvm/lib/Target/AMDGPU/GCNSubtarget.h b/llvm/lib/Target/AMDGPU/GCNSubtarget.h
index 3388bc3c5a8de1..a22e413508021d 100644
--- a/llvm/lib/Target/AMDGPU/GCNSubtarget.h
+++ b/llvm/lib/Target/AMDGPU/GCNSubtarget.h
@@ -1368,12 +1368,18 @@ class GCNSubtarget final : public AMDGPUGenSubtargetInfo,
/// VGPRs
unsigned getOccupancyWithNumVGPRs(unsigned VGPRs) const;
- /// Return occupancy for the given function. Used LDS and a number of
- /// registers if provided.
- /// Note, occupancy can be affected by the scratch allocation as well, but
+ /// Subtarget's minimum/maximum occupancy, in number of waves per EU, that can
+ /// be achieved when the only function running on a CU is \p F, each workgroup
+ /// uses \p LDSSize bytes of LDS, and each wave uses \p NumSGPRs SGPRs and \p
+ /// NumVGPRs VGPRs. The flat workgroup sizes associated to the function are a
+ /// range, so this returns a range as well.
+ ///
+ /// Note that occupancy can be affected by the scratch allocation as well, but
/// we do not have enough information to compute it.
- unsigned computeOccupancy(const Function &F, unsigned LDSSize = 0,
- unsigned NumSGPRs = 0, unsigned NumVGPRs = 0) const;
+ std::pair<unsigned, unsigned> computeOccupancy(const Function &F,
+ unsigned LDSSize = 0,
+ unsigned NumSGPRs = 0,
+ unsigned NumVGPRs = 0) const;
/// \returns true if the flat_scratch register should be initialized with the
/// pointer to the wave's scratch memory rather than a size and offset.
diff --git a/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp b/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp
index 169f1369fb5433..b73af929409064 100644
--- a/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp
@@ -48,7 +48,7 @@ SIMachineFunctionInfo::SIMachineFunctionInfo(const Function &F,
MaxNumWorkGroups = ST.getMaxNumWorkGroups(F);
assert(MaxNumWorkGroups.size() == 3);
- Occupancy = ST.computeOccupancy(F, getLDSSize());
+ Occupancy = ST.computeOccupancy(F, getLDSSize()).second;
CallingConv::ID CC = F.getCallingConv();
VRegFlags.reserve(1024);
@@ -185,8 +185,7 @@ MachineFunctionInfo *SIMachineFunctionInfo::clone(
void SIMachineFunctionInfo::limitOccupancy(const MachineFunction &MF) {
limitOccupancy(getMaxWavesPerEU());
const GCNSubtarget& ST = MF.getSubtarget<GCNSubtarget>();
- limitOccupancy(ST.getOccupancyWithLocalMemSize(getLDSSize(),
- MF.getFunction()));
+ limitOccupancy(ST.getOccupancyWithWorkGroupSizes(MF).second);
}
Register SIMachineFunctionInfo::addPrivateSegmentBuffer(
diff --git a/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp b/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
index 704435dad65d7b..11121e6058770f 100644
--- a/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
@@ -3642,18 +3642,15 @@ bool SIRegisterInfo::shouldCoalesce(MachineInstr *MI,
unsigned SIRegisterInfo::getRegPressureLimit(const TargetRegisterClass *RC,
MachineFunction &MF) const {
- const SIMachineFunctionInfo *MFI = MF.getInfo<SIMachineFunctionInfo>();
-
- unsigned Occupancy = ST.getOccupancyWithLocalMemSize(MFI->getLDSSize(),
- MF.getFunction());
+ unsigned MinOcc = ST.getOccupancyWithWorkGroupSizes(MF).first;
switch (RC->getID()) {
default:
return AMDGPUGenRegisterInfo::getRegPressureLimit(RC, MF);
case AMDGPU::VGPR_32RegClassID:
- return std::min(ST.getMaxNumVGPRs(Occupancy), ST.getMaxNumVGPRs(MF));
+ return std::min(ST.getMaxNumVGPRs(MinOcc), ST.getMaxNumVGPRs(MF));
case AMDGPU::SGPR_32RegClassID:
case AMDGPU::SGPR_LO16RegClassID:
- return std::min(ST.getMaxNumSGPRs(Occupancy, true), ST.getMaxNumSGPRs(MF));
+ return std::min(ST.getMaxNumSGPRs(MinOcc, true), ST.getMaxNumSGPRs(MF));
}
}
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/add.vni16.ll b/llvm/test/CodeGen/AMDGPU/GlobalISel/add.vni16.ll
index ab95c226b08b02..27b93872b9f1df 100644
--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/add.vni16.ll
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/add.vni16.ll
@@ -513,29 +513,29 @@ define void @add_v9i16(ptr addrspace(1) %ptra, ptr addrspace(1) %ptrb, ptr addrs
; GFX8-NEXT: flat_load_dwordx4 v[10:13], v[2:3]
; GFX8-NEXT: v_add_u32_e32 v0, vcc, 16, v0
; GFX8-NEXT: v_addc_u32_e32 v1, vcc, 0, v1, vcc
-; GFX8-NEXT: flat_load_ushort v16, v[0:1]
+; GFX8-NEXT: flat_load_ushort v14, v[0:1]
; GFX8-NEXT: v_add_u32_e32 v0, vcc, 16, v2
; GFX8-NEXT: v_addc_u32_e32 v1, vcc, 0, v3, vcc
; GFX8-NEXT: flat_load_ushort v0, v[0:1]
-; GFX8-NEXT: v_add_u32_e32 v14, vcc, 16, v4
-; GFX8-NEXT: v_addc_u32_e32 v15, vcc, 0, v5, vcc
; GFX8-NEXT: s_waitcnt vmcnt(2)
; GFX8-NEXT: v_add_u16_e32 v1, v6, v10
; GFX8-NEXT: v_add_u16_sdwa v2, v6, v10 dst_sel:WORD_1 dst_unused:UNUSED_PAD src0_sel:WORD_1 src1_sel:WORD_1
; GFX8-NEXT: v_add_u16_e32 v3, v7, v11
-; GFX8-NEXT: v_add_u16_sdwa v6, v7, v11 dst_sel:WORD_1 dst_unused:UNUSED_PAD src0_sel:WORD_1 src1_sel:WORD_1
-; GFX8-NEXT: v_add_u16_e32 v7, v8, v12
+; GFX8-NEXT: v_add_u16_sdwa v10, v7, v11 dst_sel:WORD_1 dst_unused:UNUSED_PAD src0_sel:WORD_1 src1_sel:WORD_1
+; GFX8-NEXT: v_add_u16_e32 v11, v8, v12
; GFX8-NEXT: v_add_u16_sdwa v8, v8, v12 dst_sel:WORD_1 dst_unused:UNUSED_PAD src0_sel:WORD_1 src1_sel:WORD_1
-; GFX8-NEXT: v_add_u16_e32 v10, v9, v13
+; GFX8-NEXT: v_add_u16_e32 v12, v9, v13
; GFX8-NEXT: v_add_u16_sdwa v9, v9, v13 dst_sel:WORD_1 dst_unused:UNUSED_PAD src0_sel:WORD_1 src1_sel:WORD_1
+; GFX8-NEXT: v_add_u32_e32 v6, vcc, 16, v4
; GFX8-NEXT: s_waitcnt vmcnt(0)
-; GFX8-NEXT: v_add_u16_e32 v11, v16, v0
+; GFX8-NEXT: v_add_u16_e32 v13, v14, v0
; GFX8-NEXT: v_or_b32_e32 v0, v1, v2
-; GFX8-NEXT: v_or_b32_e32 v1, v3, v6
-; GFX8-NEXT: v_or_b32_e32 v2, v7, v8
-; GFX8-NEXT: v_or_b32_e32 v3, v10, v9
+; GFX8-NEXT: v_or_b32_e32 v1, v3, v10
+; GFX8-NEXT: v_or_b32_e32 v2, v11, v8
+; GFX8-NEXT: v_or_b32_e32 v3, v12, v9
+; GFX8-NEXT: v_addc_u32_e32 v7, vcc, 0, v5, vcc
; GFX8-NEXT: flat_store_dwordx4 v[4:5], v[0:3]
-; GFX8-NEXT: flat_store_short v[14:15], v11
+; GFX8-NEXT: flat_store_short v[6:7], v13
; GFX8-NEXT: s_waitcnt vmcnt(0)
; GFX8-NEXT: s_setpc_b64 s[30:31]
;
@@ -661,55 +661,55 @@ define void @add_v11i16(ptr addrspace(1) %ptra, ptr addrspace(1) %ptrb, ...
[truncated]
|
Fix conflicting tests and new ones.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
shiltian
reviewed
Jan 21, 2025
There was a problem hiding this comment.
One potential issue I see is, the computation with workgroup size completely ignores amdgpu-waves-per-eu attribute, while it is considered with SGPRs and VGPRs. I understand the current implementation doesn't factor it in as well, but I don't know if we should.
As it should. The group size is a hard ABI requirement, and explicit. The amdgpu-waves-per-eu is an optimization hint and should not constrain the group size. |
|
I mean, when we compute occupancy (along) with workgroup size, such as the function |
arsenm
reviewed
Jan 22, 2025
| // to make sure the amount of bytes is aligned on that granularity. | ||
|
|
||
| // Compute occupancy restriction based on LDS usage. | ||
| const unsigned MaxWGsLDS = getLocalMemorySize() / std::max(LDSBytes, 1u); |
There was a problem hiding this comment.
We probably should try to account for the "amdgpu-lds-size" on the function, but that's beyond the scope of this patch
arsenm
reviewed
Jan 23, 2025
| // Compute occupancy restriction based on LDS usage. | ||
| if (TM.getTargetTriple().getArch() == Triple::amdgcn) { |
There was a problem hiding this comment.
I would leave the allocation granularity for a follow up. There always was an allocation granularity for r600, but that's not handled here.
Also just move this whole thing into GCNSubtarget, don't do the triple check and downcast
There was a problem hiding this comment.
I reverted all changes related to allocation granularity, and will properly address this for all subtargets in a different PR (probably through a new protected member variable on AMDGPUSubtarget). I reintroduced the FIXME in the function as well.
arsenm
approved these changes
Jan 23, 2025
|
@lucas-rami Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Occupancy (i.e., the number of waves per EU) depends, in addition to register usage, on per-workgroup LDS usage as well as on the range of possible workgroup sizes. Mirroring the latter, occupancy should therefore be expressed as a range since different group sizes generally yield different achievable occupancies.
getOccupancyWithLocalMemSizecurrently returns a scalar occupancy based on the maximum workgroup size and LDS usage. With respect to the workgroup size range, this scalar can be the minimum, the maximum, or neither of the two of the range of achievable occupancies. This commit fixes the function by making it compute and return the range of achievable occupancies w.r.t. workgroup size and LDS usage; it also renames it togetOccupancyWithWorkGroupSizessince it is the range of workgroup sizes that produces the range of achievable occupancies.Computing the achievable occupancy range is surprisingly involved. Minimum/maximum workgroup sizes do not necessarily yield maximum/minimum occupancies i.e., sometimes workgroup sizes inside the range yield the occupancy bounds. The implementation finds these sizes in constant time; heavy documentation explains the rationale behind the sometimes relatively obscure calculations.
As a justifying example, consider a target with 10 waves / EU, 4 EUs/CU, 64-wide waves. Also consider a function with no LDS usage and a flat workgroup size range of [513,1024].
Therefore the achievable occupancy range for this function is not [8,9] as the group size bounds directly yield, but [7,10].
Naturally this change causes a lot of test churn as instruction scheduling is driven by achievable occupancy estimates. In most unit tests the flat workgroup size range is the default [1,1024] which, ignoring potential LDS limitations, would previously produce a scalar occupancy of 8 (derived from 1024) on a lot of targets, whereas we now consider the maximum occupancy to be 10 in such cases. Most tests are updated automatically and checked manually for sanity. I also manually changed some non-automatically generated assertions when necessary. In particular, a previously broken test (
v32_asm_def_useinagpr-copy-no-free-registers.ll) seems to be fixed by the change, but this may be due to sheer luck.Fixes #118220.