[PATCH] D154970: [NFC][AMDGPULowerModuleLDSPass] Factorize repetead sort code

[Jon Chesterfield via Phabricator via llvm-commits]

JonChesterfield accepted this revision.
JonChesterfield added a comment.
This revision is now accepted and ready to land.

The move -> return approach seems reasonable. Could we go with GlobalVariable* instead of T before landing?



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:249
+template <typename T> std::vector<T> sortByName(std::vector<T> &&V) {
+  llvm::sort(V.begin(), V.end(), [](const auto *L, const auto *R) {
+    return L->getName() < R->getName();
----------------
arsenm wrote:
> The danger of anonymous values may require a stable_sort (or do we error on those here, I forget)
No anonymous variables here - these globals are all created by this pass, and all given non-empty names.

There's a hard error on anonymous functions at present, can drop that if we find a better way to bind structs to functions


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:1318
       Module &M, std::string VarName,
       DenseSet<GlobalVariable *> const &LDSVarsToTransform) {
     // Create a struct instance containing LDSVarsToTransform and map from those
----------------
This is a bit dubious in general, should probably be establishing the sorted invariant before calling into this. For another patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154970