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

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
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;
----------------
.equals(). I don't think == does what you want.


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp:102
+        StringRef GName = G->getName();
+        if (GName != "llvm.used" && GName != "llvm.compiler.used") {
+          return false;
----------------
.equals().


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp:109
 
-    if (auto *E = dyn_cast<ConstantExpr>(V)) {
-      if (F) {
-        // Any use which does not end up an instruction disqualifies a
-        // variable to be put into a kernel's LDS structure because later
-        // we will need to replace only this kernel's uses for which we
-        // need to identify a using function.
-        if (!isUsedOnlyFromFunction(E, F))
-          return false;
+    if (isa<Constant>(U)) {
+      // Recursively traverse through constant expressions.
----------------
It seems to miss the case when a same constant used by multiple kernels if I am not missing something. We could fix it by converting constant to an instruction but not doing it yet. You even had a utility function for that in some other patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103431