[PATCH] D94648: [amdgpu] Implement lower function LDS pass

[Sebastian Neubauer via Phabricator via llvm-commits]

sebastian-ne added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerFunctionLDSPass.cpp:54
+    return AMDGPU::isModuleEntryFunctionCC(Func->getCallingConv());
+  }
+
----------------
hsmhsm wrote:
> JonChesterfield wrote:
> > hsmhsm wrote:
> > > The function AMDGPU::isModuleEntryFunctionCC() rerturns true for graphics, shaders, SPIR (OpeCL?), etc. Is it what we expect here? Is not it that we are concerned here only with the CC - CallingConv::AMDGPU_KERNEL?
> > LLC (e.g. AMDGPULegalizerInfo) uses isModuleEntryFunction to detect non-kernel use of LDS, which resolves to a function in Utils/AMDGPUBaseInfo.cpp that returns true for AMDGPU_KERNEL, SPIR_KERNEL and various calling conventions I don't recognise. AMDGPU_VS etc.
> > 
> > Some opencl I compiled as a sanity check used SPIR_KERNEL as the calling convention.
> > 
> > I don't know whether that's right, only that this pass should use exactly the same predicate as the one guarding allocateLDSGlobal.
> IMHO, LLC is a generic back-end for OpenCL, OpenMP, HIP, Graphic Shader language, etc. So, AMDGPULegalizerInfo might be generally using it. but, this pass is concerned with only CallingConv::AMDGPU_KERNEL. That said, probably @arsenm  or others in the review list could better clarify it.
If I understand the purpose of `isKernelCC` correctly, it should return true if the given function is able to allocate LDS (in the sense of “an entry point is allowed to allocate LDS”).
That is exactly what `isModuleEntryFunctionCC` returns, so this looks right to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94648