[PATCH] D139433: [amdgpu] Reimplement LDS lowering

[Jon Chesterfield via Phabricator via llvm-commits]

JonChesterfield marked an inline comment as done.
JonChesterfield added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:176-185
+bool isKernelLDS(const Function *F) {
+  // Some weirdness here. AMDGPU::isKernelCC does not call into
+  // AMDGPU::isKernel with the calling conv, it instead calls into
+  // isModuleEntryFunction which returns true for more calling conventions
+  // than AMDGPU::isKernel does. There's a FIXME on AMDGPU::isKernel.
+  // There's also a test that checks that the LDS lowering does not hit on
+  // a graphics shader, denoted amdgpu_ps, so stay with the limited case.
----------------
arsenm wrote:
> Can you fix this situation in a later commit?
Unclear. It effectively means using this pass for graphics, as well as compute.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:398
+                                /* IgnoreCallbackUses */ false,
+                                /* IgnoreAssumeLikeCalls */ false,
+                                /* IgnoreLLVMUsed */ true,
----------------
arsenm wrote:
> Not sure why this is false, change in a separate commit?
it's the defaults except for ignoring llvm used iirc, will have to look up what qualifies for the other cases to see if I can get test coverage for them


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139433