[PATCH] D105116: [AMDGPU] Set optional PAL metadata

Tue Jul 6 02:47:03 PDT 2021

sebastian-ne added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp:1406
+  MD->setFunctionLdsSize(MF, CurrentProgramInfo.LDSSize);
+  MD->setFunctionNumUsedVgprs(MF, CurrentProgramInfo.NumVGPRsForWavesPerEU);
+  MD->setFunctionNumUsedSgprs(MF, CurrentProgramInfo.NumSGPRsForWavesPerEU);
----------------
sebastian-ne wrote:
> foad wrote:
> > Is this the number of vgprs that are actually used by the generated ISA? From the name "NumVGPRsForWavesPerEU" it sounds like it might be the maximum number that //could// be used while still achieving some specified occupancy.
> I’m not sure either by looking at the name.
> But it seems to be the number of used registers, at least in the test case.
> Also, this variable is used to set the normal `.vgpr_count` too (with normal, I mean the `.vgpr_count` that we already set, in contrast to the one in `.shader_functions`, which is added here).
I looked a bit more, `NumVGPRsForWavesPerEU` is the amount of VGPRs used by the ISA, except if the function attribute `amdgpu-waves-per-eu` sets a *maximum* of waves per eu. Then `NumVGPRsForWavesPerEU` will be raised artificially, so that no more waves than specified fit in.
I guess this is a fair thing to do.
(For the record, if it is not obvious from the comments somewhere, `.vgpr_count` is only used by tools like RGP and does not affect actual execution.)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105116