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

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp:55
+
+    if (isa<GlobalVariable>(U) || isa<Constant>(U)) {
+      append_range(Stack, U->users());
----------------
A GlobalVariable is a Constant, so the first isa<> check is redundant.


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp:71
+  // We are not interested in kernel LDS lowering for module LDS itself.
+  if (F && GV.getName().equals("llvm.amdgcn.module.lds")) {
+    return false;
----------------
Don't need braces around a single line.


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp:71
+  // We are not interested in kernel LDS lowering for module LDS itself.
+  if (F && GV.getName() == "llvm.amdgcn.module.lds") {
+    return false;
----------------
rampitec wrote:
> .equals(). I don't think == does what you want.
No, `==` should be fine. Both `==` and `equals` take two StringRefs and do a full string comparison on them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103431