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

[Mahesha S via Phabricator via llvm-commits]

hsmhsm marked 4 inline comments as done.
hsmhsm added a comment.

In D103431#2806206 <https://reviews.llvm.org/D103431#2806206>, @rampitec wrote:

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

I have not yet verified the GlobalAlias. Let me look into it.



================
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();
----------------
rampitec wrote:
> stripPointerCasts() still makes sense.
No, as I understand it, we should be very carefull when using stripPointerCasts(), otherwise, it will lead to unexpected and surprising behavior. 

For exmaple, in this case, let's try below simple example. Here, only user of "@lds.1" is bitcast inst within kernel. Now, when we are analysing this user, we apply stripPointerCasts(), which will return back "@lds.1" itself, and since it is a global variable, the IF condition satisfies, and we incorrectly land within IF block, where we did not want, which leads to incorrect transormation.

```
@lds.1 = internal unnamed_addr addrspace(3) global [1 x i8] undef, align 1

define amdgpu_kernel void @k0() {
  %bc = bitcast [1 x i8] addrspace(3)* @lds.1 to i8 addrspace(3)*
  store i8 1, i8 addrspace(3)* %bc, align 1
  ret void
}
```


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp:135
+    for (auto *G : GlobalUsers)
+      Ret |= hasUserInstruction(G);
   }
----------------
rampitec wrote:
> How can you get here? You should have inspected all the users by this time and met Instruction uses if any.
Please note that the global(s) that we are trying to inspect here are not lds (GV) itself, but, they are the ones which use lds (GV) as an initializer in the global scope or they are special llvm.used. We collect such globals which use lds (GV), and decide whether to lower or not based on the use of those globals.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103431