[AMDGPU] Warn if 'amdgpu-waves-per-eu' target occupancy was not met

[Pierre-vh]

This should make it a bit harder to miss this type of issue. The warning only shows if amdgpu-waves-per-eu is used.

See SWDEV-434482

[llvmbot]

@llvm/pr-subscribers-backend-amdgpu

Author: Pierre van Houtryve (Pierre-vh)

Changes

This should make it a bit harder to miss this type of issue. The warning only shows if amdgpu-waves-per-eu is used.

See SWDEV-434482

---

Full diff: https://github.com/llvm/llvm-project/pull/74055.diff

2 Files Affected:

• (modified) llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp (+11)
• (added) llvm/test/CodeGen/AMDGPU/min-waves-per-eu-not-respected.ll (+16)

diff --git a/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp b/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
index eb30f31af6d6b68..4bf1f1357b694ee 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
@@ -939,6 +939,17 @@ void AMDGPUAsmPrinter::getSIProgramInfo(SIProgramInfo &ProgInfo,
   ProgInfo.Occupancy = STM.computeOccupancy(MF.getFunction(), ProgInfo.LDSSize,
                                             ProgInfo.NumSGPRsForWavesPerEU,
                                             ProgInfo.NumVGPRsForWavesPerEU);
+  const auto [MinWEU, MaxWEU] =
+      AMDGPU::getIntegerPairAttribute(F, "amdgpu-waves-per-eu", {0, 0}, true);
+  if (ProgInfo.Occupancy < MinWEU) {
+    DiagnosticInfoOptimizationFailure Diag(
+        F, F.getSubprogram(),
+        "failed to meet occupancy target given by 'amdgpu-waves-per-eu' in "
+        "'" +
+            F.getName() + "': desired occupancy was " + Twine(MinWEU) +
+            ", final occupancy is " + Twine(ProgInfo.Occupancy));
+    F.getContext().diagnose(Diag);
+  }
 }
 
 static unsigned getRsrcReg(CallingConv::ID CallConv) {
diff --git a/llvm/test/CodeGen/AMDGPU/min-waves-per-eu-not-respected.ll b/llvm/test/CodeGen/AMDGPU/min-waves-per-eu-not-respected.ll
new file mode 100644
index 000000000000000..50de3541e05b4d6
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/min-waves-per-eu-not-respected.ll
@@ -0,0 +1,16 @@
+; RUN: llc -march=amdgcn -mcpu=gfx900 -verify-machineinstrs < %s 2>&1 | FileCheck -check-prefix=WARN %s
+
+; 1024 flat work group size across 2560 possible threads -> occupancy should be 8 max.
+; WARN: warning: <unknown>:0:0: failed to meet occupancy target given by 'amdgpu-waves-per-eu' in 'occupancy_8_target_9': desired occupancy was 9, final occupancy is 8
+define amdgpu_kernel void @occupancy_8_target_9() #0 {
+  ret void
+}
+
+; Impossible occupancy target
+; WARN: warning: <unknown>:0:0: failed to meet occupancy target given by 'amdgpu-waves-per-eu' in 'impossible_occupancy': desired occupancy was 11, final occupancy is 10
+define amdgpu_kernel void @impossible_occupancy() #1 {
+  ret void
+}
+
+attributes #0 = { "amdgpu-flat-work-group-size"="1,1024" "amdgpu-waves-per-eu"="9" }
+attributes #1 = { "amdgpu-flat-work-group-size"="1,256" "amdgpu-waves-per-eu"="11" }

[nhaehnle]

Seems fine to me from a graphics perspective.

[arsenm]

Can you check how many times this fires in the test suite? I know we routinely clamp the lower bound (but also don't actually do anything with it)

Also, isn't there already a wrapped query for the attribute?

[Pierre-vh]

It triggers 10 times in the test suite

[build] Failed Tests (10):
[build]   LLVM :: CodeGen/AMDGPU/attr-amdgpu-waves-per-eu.ll
[build]   LLVM :: CodeGen/AMDGPU/copy-vgpr-clobber-spill-vgpr.mir
[build]   LLVM :: CodeGen/AMDGPU/inline-asm-reserved-regs.ll
[build]   LLVM :: CodeGen/AMDGPU/mfma-cd-select.ll
[build]   LLVM :: CodeGen/AMDGPU/min-waves-per-eu-not-respected.ll
[build]   LLVM :: CodeGen/AMDGPU/occupancy-levels.ll
[build]   LLVM :: CodeGen/AMDGPU/scc-clobbered-sgpr-to-vmem-spill.ll
[build]   LLVM :: CodeGen/AMDGPU/sgpr-spill-incorrect-fi-bookkeeping-bug.ll
[build]   LLVM :: CodeGen/AMDGPU/sgpr-spill-no-vgprs.ll
[build]   LLVM :: CodeGen/AMDGPU/spill-scavenge-offset.ll
[build] 

The wrapped query does a bit of extra work other than getting the attribute. It'll return a default if the attribute isn't present, but here we just want to trigger the warning if the attribute is there so I think it's better to get it manually