[PATCH] D159452: [AMDGPU] Machine scheduler should take care of the 'amdgpu-num-vgp' function attribute

Tue Sep 5 12:25:48 PDT 2023

rampitec added a comment.

In D159452#4638703 <https://reviews.llvm.org/D159452#4638703>, @kerbowa wrote:

> I thought this already was used in the excess VGPRs pressure and wave-per-eu is that just not true?

I thought it is buried inside the logic of waves per eu calculation/getEffectiveWavesPerEU/getWavesPerEU, but apparently it is not.

================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp:677

+unsigned llvm::GCNSubtarget::getOccupancyWithNumVGPRsForFunction(
+    const MachineFunction &MF, unsigned VGPRs) const {
----------------
'llvm::' is not needed.

================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp:682
+  unsigned MaxWaves = WavesPerEU.second;
+  unsigned Granule = getVGPRAllocGranule();
+  if (VGPRs < Granule)
----------------
Why not just call `getOccupancyWithNumVGPRs` here? This logic is already in the `getNumWavesPerEUWithNumVGPRs`.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159452/new/

https://reviews.llvm.org/D159452