[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
Mon Jun 7 12:59:29 PDT 2021


rampitec added a comment.

This probably will need rebase on top of D103655 <https://reviews.llvm.org/D103655> which presumably shall simplify shouldLowerLDSToStruct() logic?



================
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;
----------------
hsmhsm wrote:
> rampitec wrote:
> > foad wrote:
> > > 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.
> > I do not see StringRef::operator==() in the documentation?
> @rampitec Here is the StringRef::operator==() - https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/ADT/StringRef.h#L897
> 
> It infact internally calls **.equals()** only.
> 
> Anyway, I think, it is fine either way except that **.equals()** is bit verbose, but, I kept **.equals()** only for now.
Thanks!


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