Skip to content

[llvm] Use llvm::less_first and llvm::less_second (NFC) #136272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

kazutakahirata
Copy link
Contributor

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Apr 18, 2025

@llvm/pr-subscribers-vectorizers
@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-backend-nvptx

Author: Kazu Hirata (kazutakahirata)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/136272.diff

3 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp (+2-6)
  • (modified) llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp (+2-7)
  • (modified) llvm/tools/llvm-gpu-loader/nvptx.cpp (+2-2)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp b/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp
index 153b14ce60507..815715604bc96 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp
@@ -588,12 +588,8 @@ void PipelineSolver::populateReadyList(
       ReadyList.push_back(std::pair(*I, -1));
   }
 
-  if (UseCostHeur) {
-    std::sort(ReadyList.begin(), ReadyList.end(),
-              [](std::pair<int, int> A, std::pair<int, int> B) {
-                return A.second < B.second;
-              });
-  }
+  if (UseCostHeur)
+    llvm::sort(ReadyList, llvm::less_second());
 
   assert(ReadyList.size() == CurrSU.second.size());
 }
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 13c1eb572ef92..4301bc7d0d68b 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -6466,10 +6466,7 @@ static bool clusterSortPtrAccesses(ArrayRef<Value *> VL,
   for (auto &Base : Bases) {
     for (auto &Vec : Base.second) {
       if (Vec.size() > 1) {
-        stable_sort(Vec, [](const std::tuple<Value *, int, unsigned> &X,
-                            const std::tuple<Value *, int, unsigned> &Y) {
-          return std::get<1>(X) < std::get<1>(Y);
-        });
+        stable_sort(Vec, llvm::less_second());
         int InitialOffset = std::get<1>(Vec[0]);
         bool AnyConsecutive =
             all_of(enumerate(Vec), [InitialOffset](const auto &P) {
@@ -7986,9 +7983,7 @@ bool BoUpSLP::canFormVector(ArrayRef<StoreInst *> StoresVec,
   // Check if the stores are consecutive by checking if their difference is 1.
   if (StoreOffsetVec.size() != StoresVec.size())
     return false;
-  sort(StoreOffsetVec,
-       [](const std::pair<int, unsigned> &L,
-          const std::pair<int, unsigned> &R) { return L.first < R.first; });
+  sort(StoreOffsetVec, llvm::less_first());
   unsigned Idx = 0;
   int PrevDist = 0;
   for (const auto &P : StoreOffsetVec) {
diff --git a/llvm/tools/llvm-gpu-loader/nvptx.cpp b/llvm/tools/llvm-gpu-loader/nvptx.cpp
index 13c62d50e6077..781a045e71a94 100644
--- a/llvm/tools/llvm-gpu-loader/nvptx.cpp
+++ b/llvm/tools/llvm-gpu-loader/nvptx.cpp
@@ -84,8 +84,8 @@ Expected<void *> get_ctor_dtor_array(const void *image, const size_t size,
   }
   // Lower priority constructors are run before higher ones. The reverse is true
   // for destructors.
-  llvm::sort(ctors, [](auto x, auto y) { return x.second < y.second; });
-  llvm::sort(dtors, [](auto x, auto y) { return x.second < y.second; });
+  llvm::sort(ctors, llvm::less_second());
+  llvm::sort(dtors, llvm::less_second());
 
   // Allocate host pinned memory to make these arrays visible to the GPU.
   CUdeviceptr *dev_memory = reinterpret_cast<CUdeviceptr *>(allocator(

@llvmbot
Copy link
Member

llvmbot commented Apr 18, 2025

@llvm/pr-subscribers-backend-amdgpu

Author: Kazu Hirata (kazutakahirata)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/136272.diff

3 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp (+2-6)
  • (modified) llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp (+2-7)
  • (modified) llvm/tools/llvm-gpu-loader/nvptx.cpp (+2-2)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp b/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp
index 153b14ce60507..815715604bc96 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp
@@ -588,12 +588,8 @@ void PipelineSolver::populateReadyList(
       ReadyList.push_back(std::pair(*I, -1));
   }
 
-  if (UseCostHeur) {
-    std::sort(ReadyList.begin(), ReadyList.end(),
-              [](std::pair<int, int> A, std::pair<int, int> B) {
-                return A.second < B.second;
-              });
-  }
+  if (UseCostHeur)
+    llvm::sort(ReadyList, llvm::less_second());
 
   assert(ReadyList.size() == CurrSU.second.size());
 }
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 13c1eb572ef92..4301bc7d0d68b 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -6466,10 +6466,7 @@ static bool clusterSortPtrAccesses(ArrayRef<Value *> VL,
   for (auto &Base : Bases) {
     for (auto &Vec : Base.second) {
       if (Vec.size() > 1) {
-        stable_sort(Vec, [](const std::tuple<Value *, int, unsigned> &X,
-                            const std::tuple<Value *, int, unsigned> &Y) {
-          return std::get<1>(X) < std::get<1>(Y);
-        });
+        stable_sort(Vec, llvm::less_second());
         int InitialOffset = std::get<1>(Vec[0]);
         bool AnyConsecutive =
             all_of(enumerate(Vec), [InitialOffset](const auto &P) {
@@ -7986,9 +7983,7 @@ bool BoUpSLP::canFormVector(ArrayRef<StoreInst *> StoresVec,
   // Check if the stores are consecutive by checking if their difference is 1.
   if (StoreOffsetVec.size() != StoresVec.size())
     return false;
-  sort(StoreOffsetVec,
-       [](const std::pair<int, unsigned> &L,
-          const std::pair<int, unsigned> &R) { return L.first < R.first; });
+  sort(StoreOffsetVec, llvm::less_first());
   unsigned Idx = 0;
   int PrevDist = 0;
   for (const auto &P : StoreOffsetVec) {
diff --git a/llvm/tools/llvm-gpu-loader/nvptx.cpp b/llvm/tools/llvm-gpu-loader/nvptx.cpp
index 13c62d50e6077..781a045e71a94 100644
--- a/llvm/tools/llvm-gpu-loader/nvptx.cpp
+++ b/llvm/tools/llvm-gpu-loader/nvptx.cpp
@@ -84,8 +84,8 @@ Expected<void *> get_ctor_dtor_array(const void *image, const size_t size,
   }
   // Lower priority constructors are run before higher ones. The reverse is true
   // for destructors.
-  llvm::sort(ctors, [](auto x, auto y) { return x.second < y.second; });
-  llvm::sort(dtors, [](auto x, auto y) { return x.second < y.second; });
+  llvm::sort(ctors, llvm::less_second());
+  llvm::sort(dtors, llvm::less_second());
 
   // Allocate host pinned memory to make these arrays visible to the GPU.
   CUdeviceptr *dev_memory = reinterpret_cast<CUdeviceptr *>(allocator(

@kazutakahirata kazutakahirata merged commit 5e1b0f9 into llvm:main Apr 18, 2025
14 of 16 checks passed
@kazutakahirata kazutakahirata deleted the cleanup_001_less_second_llvm branch April 18, 2025 17:05
@llvm-ci
Copy link
Collaborator

llvm-ci commented Apr 18, 2025

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-expensive-checks-debian running on gribozavr4 while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/16/builds/17613

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM :: CodeGen/AMDGPU/sched-group-barrier-pre-RA.mir' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
/b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/llc -mtriple=amdgcn -mcpu=gfx908 -misched-cluster=false -run-pass=machine-scheduler -verify-misched -o - /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/CodeGen/AMDGPU/sched-group-barrier-pre-RA.mir | /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/FileCheck /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/CodeGen/AMDGPU/sched-group-barrier-pre-RA.mir # RUN: at line 2
+ /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/llc -mtriple=amdgcn -mcpu=gfx908 -misched-cluster=false -run-pass=machine-scheduler -verify-misched -o - /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/CodeGen/AMDGPU/sched-group-barrier-pre-RA.mir
+ /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/FileCheck /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/CodeGen/AMDGPU/sched-group-barrier-pre-RA.mir
/b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/llc -mtriple=amdgcn -mcpu=gfx908 -misched-cluster=false -run-pass=machine-scheduler -amdgpu-igrouplp-exact-solver -verify-misched -o - /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/CodeGen/AMDGPU/sched-group-barrier-pre-RA.mir | /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/FileCheck -check-prefix=EXACT /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/CodeGen/AMDGPU/sched-group-barrier-pre-RA.mir # RUN: at line 3
+ /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/FileCheck -check-prefix=EXACT /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/CodeGen/AMDGPU/sched-group-barrier-pre-RA.mir
+ /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/llc -mtriple=amdgcn -mcpu=gfx908 -misched-cluster=false -run-pass=machine-scheduler -amdgpu-igrouplp-exact-solver -verify-misched -o - /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/CodeGen/AMDGPU/sched-group-barrier-pre-RA.mir
/b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/CodeGen/AMDGPU/sched-group-barrier-pre-RA.mir:201:16: error: EXACT-NEXT: is not on the line after the previous match
 ; EXACT-NEXT: [[DEF2:%[0-9]+]]:areg_128 = IMPLICIT_DEF
               ^
<stdin>:394:2: note: 'next' match was here
 %2:areg_128 = IMPLICIT_DEF
 ^
<stdin>:390:58: note: previous match ended here
 %4:vgpr_32 = nsw V_MUL_LO_U32_e64 %3, %3, implicit $exec
                                                         ^
<stdin>:391:1: note: non-matching line after previous match is here
 GLOBAL_STORE_DWORD_SADDR %1, %4, %0, 0, 0, implicit $exec :: (store (s32) into %ir.out, !noalias !0, addrspace 1)
^
/b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/CodeGen/AMDGPU/sched-group-barrier-pre-RA.mir:293:16: error: EXACT-NEXT: is not on the line after the previous match
 ; EXACT-NEXT: [[V_MUL_LO_U32_e64_1:%[0-9]+]]:vgpr_32 = nsw V_MUL_LO_U32_e64 [[GLOBAL_LOAD_DWORD_SADDR]], [[DEF1]], implicit $exec
               ^
<stdin>:527:2: note: 'next' match was here
 %5:vgpr_32 = nsw V_MUL_LO_U32_e64 %3, %1, implicit $exec
 ^
<stdin>:523:91: note: previous match ended here
 %8:areg_128 = V_MFMA_F32_4X4X1F32_e64 %1, %3, %7, 0, 0, 0, implicit $mode, implicit $exec
                                                                                          ^
<stdin>:524:1: note: non-matching line after previous match is here
 S_NOP 0
^

Input file: <stdin>
Check file: /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/CodeGen/AMDGPU/sched-group-barrier-pre-RA.mir

-dump-input=help explains the following input dump.

Input was:
<<<<<<
          .
          .
          .
        389:  %3:vgpr_32 = GLOBAL_LOAD_DWORD_SADDR %0, %1, 0, 0, implicit $exec :: (load (s32) from %ir.in, !alias.scope !0, addrspace 1) 
        390:  %4:vgpr_32 = nsw V_MUL_LO_U32_e64 %3, %3, implicit $exec 
        391:  GLOBAL_STORE_DWORD_SADDR %1, %4, %0, 0, 0, implicit $exec :: (store (s32) into %ir.out, !noalias !0, addrspace 1) 
        392:  S_NOP 0 
...

@vvereschaka
Copy link
Contributor

@kazutakahirata ,

llvm-clang-x86_64-expensive-checks-ubuntu builder also has broken with the following failed tests:

  • LLVM::sched-group-barrier-pre-RA.mir
  • LLVM::sched-group-barrier-pipeline-solver.mir

see more details here: https://lab.llvm.org/buildbot/#/builders/16/builds/17613/steps/6/logs/stdio

@kazutakahirata
Copy link
Contributor Author

@kazutakahirata ,

llvm-clang-x86_64-expensive-checks-ubuntu builder also has broken with the following failed tests:

  • LLVM::sched-group-barrier-pre-RA.mir
  • LLVM::sched-group-barrier-pipeline-solver.mir

see more details here: https://lab.llvm.org/buildbot/#/builders/16/builds/17613/steps/6/logs/stdio

Thank you for letting me know! Reverting to std::sort in llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp seems to fix the problems. I'll post a PR shortly.

@kazutakahirata
Copy link
Contributor Author

@vvereschaka

Reverting to std::sort in llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp seems to fix the problems. I'll post a PR shortly.

I just posted #136615.

IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants