[PATCH] D94648: [amdgpu] Implement lower function LDS pass

[Matt Arsenault via Phabricator via llvm-commits]

arsenm accepted this revision.
arsenm added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:141
+
+  static void removeFromUsedList(Module &M, StringRef Name,
+                                 SmallPtrSetImpl<Constant *> &ToRemove) {
----------------
JonChesterfield wrote:
> arsenm wrote:
> > JonChesterfield wrote:
> > > arsenm wrote:
> > > > Needs a comment
> > > A comment saying what? The function and helper does what the name and parameter types claim it'll do in almost as boring a fashion as possible.
> > On first read it sounds very generic, and not related to the special intrinsic global variables. I had to read the function to see what it actually did
> It is generic. If this lands, I'm hoping to move it under ModuleUtils to go alongside appendToUsed which it closely resembles. Probably as one entry point to remove a set/sequence of constants from llvm.used and a different entry point to remove them from llvm.compiler.used.
> 
> As it's far from certain whether this will land, I don't want to propose a function for ModuleUtils with no users, as it'll be rightly rejected as dead code.
Committing utlities before uses is not unheard of


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94648