[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