[PATCH] D138141: [amdgpu] Reimplement LDS lowering

[Jon Chesterfield via Phabricator via llvm-commits]

JonChesterfield added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:573
+        M, AllKernelsOffsetsType, true, GlobalValue::InternalLinkage, init,
+        "llvm.amdgcn.lds_offset_table", nullptr, GlobalValue::NotThreadLocal,
+        AMDGPUAS::CONSTANT_ADDRESS);
----------------
arsenm wrote:
> Not sure if this is just copying something existing, but I think it would be preferably to consistently use . as the word separator instead of mixing in _
OK, so the rest of the similar things are dot delimited, but I_write_in_snake_case by default so that's how this happens. Will rename it to `llvm.amdgcn.lds.offset.table`


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:655
+    DenseSet<Function *> KernelSet;
+    if (VariableSet.empty()) return KernelSet;
+
----------------
arsenm wrote:
> New line
Ambiguous comment, let me know if you meant something other than a newline between the declaration and the early return


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138141