[PATCH] D138141: [amdgpu] Reimplement LDS lowering

[Matt Arsenault via Phabricator via llvm-commits]

arsenm accepted this revision.
arsenm added a comment.
This revision is now accepted and ready to land.

LGTM with some nits



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:893
+
+      // remove preserves existing codegen
+      removeFromUsedLists(M, KernelUsedVariables);
----------------
Capitalize


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:1177
       GV->replaceUsesWithIf(GEP, Predicate);
-      if (GV->use_empty()) {
-        GV->eraseFromParent();
-      }
+      // trunk does a eraseFromParent here
 
----------------
Junk comment?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUMachineFunction.cpp:139
+  if (GVM && f && !canElideModuleLDS(*f)) {
+    // allocator aligns this to var align, but it's zero to begin with
+    Offset += DL.getTypeAllocSize(GVM->getValueType());
----------------
Capitalize


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:6012
+  auto const AS = GA->getAddressSpace();
+  if (AS == AMDGPUAS::GLOBAL_ADDRESS) return true;
+  if (AS == AMDGPUAS::CONSTANT_ADDRESS) return true;
----------------
Newlines


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:6018
+  // Some LDS variables have compile time known addresses
+  if (AS == AMDGPUAS::LOCAL_ADDRESS) {
+    if (const GlobalVariable *GV =
----------------
More early return?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138141