[PATCH] D103431: [AMDGPU] Fix missing lowering of LDS used in global scope.

Stanislav Mekhanoshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 8 11:59:53 PDT 2021


rampitec added a comment.

Can you add tests for GlobalAlias? It is not immediately clear if these are handled properly.



================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp:76
+
+    auto *C = cast<Constant>(U);
+    append_range(Stack, C->users());
----------------
You do not need to cast here, just append U->users().


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp:99
 
-    if (auto *G = dyn_cast<GlobalValue>(V->stripPointerCasts())) {
-      if (UsedList.contains(G)) {
-        continue;
+    if (auto *G = dyn_cast<GlobalVariable>(V)) {
+      StringRef GName = G->getName();
----------------
stripPointerCasts() still makes sense.


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp:125
+    // User V should be a constant, recursively visit users of V.
+    auto *E = cast<Constant>(V);
+    append_range(Stack, E->users());
----------------
No need to cast, you are adding all users anyway. You might want an assert(isa<Constant>(V)) though.


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp:135
+    for (auto *G : GlobalUsers)
+      Ret |= hasUserInstruction(G);
   }
----------------
How can you get here? You should have inspected all the users by this time and met Instruction uses if any.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103431



More information about the llvm-commits mailing list