[PATCH] D101633: [AMDGPU] Set number vgprs used in PS shaders based on input registers actually used

[David Stuttard via Phabricator via llvm-commits]

dstuttard marked 4 inline comments as done.
dstuttard added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp:770
+          }
+          PSVGPRCount++;
+        } else {
----------------
sebastian-ne wrote:
> Should this be an increment or a `+= NumRegs`?
It should be += NumRegs - I've renamed PSVGPRCount to PSArgCount instead


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp:1154-1155
   if (isShader(F.getCallingConv())) {
+    bool ProcessingPS =
+        F.getCallingConv() == CallingConv::AMDGPU_PS && !STM.isAmdHsaOS();
+
----------------
sebastian-ne wrote:
> dstuttard wrote:
> > sebastian-ne wrote:
> > > Checking for AMDGPU_PS is enough, having this calling convention in AmdHsaOS would throw an error in SITargetLowering::LowerFormalArguments.
> > > Also, maybe the name `IsPixelShader` is clearer? (It’s called that in AMDGPUAtomicOptimizer.cpp.)
> > Actually, removing the AmdHsaOS check causes one of the lit tests to fail: no-hsa-graphics-shaders.ll
> The lit test fails because the assertion `assert((InputEna || InputAddr))` fails and the error message that amdgpu_ps is unsupported in HSA is not printed anymore. So amdgpu_ps in HSA is illegal. Not sure if there’s a better way to handle this though.
Let's leave it as-is for now then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101633