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

[Jon Chesterfield via Phabricator via llvm-commits]

JonChesterfield added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp:125
+    assert(isa<Constant>(V) && "Expected a constant.");
+    append_range(Stack, V->users());
+  }
----------------
JonChesterfield wrote:
> After this patch, all users are added. Before it, only those that were not already in the Visited set were added. On the face of it, this seems to discard the guard against cycles. This patch also leaves the Visited set dead (values are inserted into it, but never read).
> 
> I don't see any discussion in this patch about removing that guard and it's strange that the set was left in place if that was intentional. Is there a reason cycles cannot occur here?
We might be OK - uses can definitely be cyclic, e.g.
`uint64_t * __attribute__((address_space(3))) loc = (void*)&loc;`
in which case this logic does indeed loop forever, but I don't think it's possible to construct an expression that hits that which gets past the isa<UndefValue> check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103431