[PATCH] D138141: [amdgpu] Reimplement LDS lowering

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:502
+
+    std::vector<Constant *> Elements;
+    for (size_t i = 0; i < Variables.size(); i++) {
----------------
SmallVector


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:534
+    std::vector<Constant *> overallConstantExprElts(kernels.size());
+    for (size_t i = 0; i < kernels.size(); i++) {
+
----------------
Use NumberKernels from above?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:535
+    for (size_t i = 0; i < kernels.size(); i++) {
+
+      LDSVariableReplacement Replacement = KernelToReplacement[kernels[i]];
----------------
Extra line


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:572
+        BasicBlock::iterator it =
+            F->getEntryBlock().getFirstInsertionPt();
+        Instruction &i = *it;
----------------
getFirstNonPHIOrDbgOrAlloca


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:586
+      for (Use &U : make_early_inc_range(GV->uses())) {
+
+        auto *I = dyn_cast<Instruction>(U.getUser());
----------------
Extra line


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:589
+        if (!I) {
+          continue;
+        }
----------------
The constant expression case matters? 

In particular instruction uses of addrspacecasted globals come up a lot. The constant initializer case is harder 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138141