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

[Jon Chesterfield via Phabricator via llvm-commits]

JonChesterfield added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerFunctionLDSPass.cpp:54
+    return AMDGPU::isModuleEntryFunctionCC(Func->getCallingConv());
+  }
+
----------------
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.


================
Comment at: llvm/test/CodeGen/AMDGPU/lower-function-lds.ll:51
+; CHECK: call void asm sideeffect "// Alloc function lds block", "r"(%__function_lds_t addrspace(3)* @__function_lds)
+define spir_kernel void @kern_empty() {
+  ret void
----------------
arsenm wrote:
> We're not really handling spir_kernel anymore, should use amdgpu_kernel
An OpenCL compilation emitted spir_kernel (somewhat to my surprise) so I thought I'd go for one of each. See also a comment above about wanting to be consistent with the guards around allocateLDSGlobal


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94648