[PATCH] D154977: Reland "[NFC][AMDGPULowerModuleLDSPass] Factorize repetead sort code"Fixed compilation error and reudndant copy warning

Juan Manuel Martinez CaamaƱo via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 12 00:38:27 PDT 2023


This revision was not accepted when it landed; it landed in state "Needs Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3a75551e857b: Reland "[NFC][AMDGPULowerModuleLDSPass] Factorize repetead sort code" (authored by jmmartinez).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154977

Files:
  llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp


Index: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp
===================================================================
--- llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp
@@ -245,6 +245,13 @@
   return AMDGPU::isKernel(F->getCallingConv());
 }
 
+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();
+  });
+  return {std::move(V)};
+}
+
 class AMDGPULowerModuleLDS : public ModulePass {
 
   static void
@@ -733,10 +740,7 @@
       }
 
       // Put them in an arbitrary but reproducible order
-      llvm::sort(OrderedKernels.begin(), OrderedKernels.end(),
-                 [](const Function *lhs, const Function *rhs) -> bool {
-                   return lhs->getName() < rhs->getName();
-                 });
+      OrderedKernels = sortByName(std::move(OrderedKernels));
 
       // Annotate the kernels with their order in this vector
       LLVMContext &Ctx = M->getContext();
@@ -1185,13 +1189,8 @@
 
       // The order must be consistent between lookup table and accesses to
       // lookup table
-      std::vector<GlobalVariable *> TableLookupVariablesOrdered(
-          TableLookupVariables.begin(), TableLookupVariables.end());
-      llvm::sort(TableLookupVariablesOrdered.begin(),
-                 TableLookupVariablesOrdered.end(),
-                 [](const GlobalVariable *lhs, const GlobalVariable *rhs) {
-                   return lhs->getName() < rhs->getName();
-                 });
+      auto TableLookupVariablesOrdered = sortByName(std::vector(
+          TableLookupVariables.begin(), TableLookupVariables.end()));
 
       GlobalVariable *LookupTable = buildLookupTable(
           M, TableLookupVariablesOrdered, OrderedKernels, KernelToReplacement);
@@ -1338,12 +1337,9 @@
       // The order of fields in this struct depends on the order of
       // varables in the argument which varies when changing how they
       // are identified, leading to spurious test breakage.
-      std::vector<GlobalVariable *> Sorted(LDSVarsToTransform.begin(),
-                                           LDSVarsToTransform.end());
-      llvm::sort(Sorted.begin(), Sorted.end(),
-                 [](const GlobalVariable *lhs, const GlobalVariable *rhs) {
-                   return lhs->getName() < rhs->getName();
-                 });
+      auto Sorted = sortByName(
+          std::vector(LDSVarsToTransform.begin(), LDSVarsToTransform.end()));
+
       for (GlobalVariable *GV : Sorted) {
         OptimizedStructLayoutField F(GV,
                                      DL.getTypeAllocSize(GV->getValueType()),
@@ -1424,19 +1420,15 @@
   template <typename PredicateTy>
   static void replaceLDSVariablesWithStruct(
       Module &M, DenseSet<GlobalVariable *> const &LDSVarsToTransformArg,
-      LDSVariableReplacement Replacement, PredicateTy Predicate) {
+      const LDSVariableReplacement &Replacement, PredicateTy Predicate) {
     LLVMContext &Ctx = M.getContext();
     const DataLayout &DL = M.getDataLayout();
 
     // A hack... we need to insert the aliasing info in a predictable order for
     // lit tests. Would like to have them in a stable order already, ideally the
     // same order they get allocated, which might mean an ordered set container
-    std::vector<GlobalVariable *> LDSVarsToTransform(
-        LDSVarsToTransformArg.begin(), LDSVarsToTransformArg.end());
-    llvm::sort(LDSVarsToTransform.begin(), LDSVarsToTransform.end(),
-               [](const GlobalVariable *lhs, const GlobalVariable *rhs) {
-                 return lhs->getName() < rhs->getName();
-               });
+    std::vector<GlobalVariable *> LDSVarsToTransform = sortByName(std::vector(
+        LDSVarsToTransformArg.begin(), LDSVarsToTransformArg.end()));
 
     // Create alias.scope and their lists. Each field in the new structure
     // does not alias with all other fields.
@@ -1458,7 +1450,7 @@
     // field of the instance that will be allocated by AMDGPUMachineFunction
     for (size_t I = 0; I < NumberVars; I++) {
       GlobalVariable *GV = LDSVarsToTransform[I];
-      Constant *GEP = Replacement.LDSVarsToConstantGEP[GV];
+      Constant *GEP = Replacement.LDSVarsToConstantGEP.at(GV);
 
       GV->replaceUsesWithIf(GEP, Predicate);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D154977.539416.patch
Type: text/x-patch
Size: 4386 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230712/eda38acf/attachment.bin>


More information about the llvm-commits mailing list