[llvm] Revert "[GlobalMerge][NFC] Skip sorting by profitability when it is not needed" (PR #124411)

James Y Knight via llvm-commits llvm-commits at lists.llvm.org
Sat Jan 25 07:15:47 PST 2025


https://github.com/jyknight created https://github.com/llvm/llvm-project/pull/124411

Reverts llvm/llvm-project#124146 -- new comparator is not a strict-weak as required by stable_sort.

>From a41ded832d91141939c1b4aa2e955471a1047755 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Fri, 24 Jan 2025 23:42:18 -0500
Subject: [PATCH] =?UTF-8?q?Revert=20"[GlobalMerge][NFC]=20Skip=20sorting?=
 =?UTF-8?q?=20by=20profitability=20when=20it=20is=20not=20neede=E2=80=A6"?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This reverts commit e5e55c04d6af4ae32c99d574f59e632595abf607.
---
 llvm/lib/CodeGen/GlobalMerge.cpp | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/llvm/lib/CodeGen/GlobalMerge.cpp b/llvm/lib/CodeGen/GlobalMerge.cpp
index 41e01a1d3ccd52..7b76155b175d1d 100644
--- a/llvm/lib/CodeGen/GlobalMerge.cpp
+++ b/llvm/lib/CodeGen/GlobalMerge.cpp
@@ -423,12 +423,24 @@ bool GlobalMergeImpl::doMerge(SmallVectorImpl<GlobalVariable *> &Globals,
     }
   }
 
+  // Now we found a bunch of sets of globals used together.  We accumulated
+  // the number of times we encountered the sets (i.e., the number of functions
+  // that use that exact set of globals).
+  //
+  // Multiply that by the size of the set to give us a crude profitability
+  // metric.
+  llvm::stable_sort(UsedGlobalSets,
+                    [](const UsedGlobalSet &UGS1, const UsedGlobalSet &UGS2) {
+                      return UGS1.Globals.count() * UGS1.UsageCount <
+                             UGS2.Globals.count() * UGS2.UsageCount;
+                    });
+
   // We can choose to merge all globals together, but ignore globals never used
   // with another global.  This catches the obviously non-profitable cases of
   // having a single global, but is aggressive enough for any other case.
   if (GlobalMergeIgnoreSingleUse) {
     BitVector AllGlobals(Globals.size());
-    for (const UsedGlobalSet &UGS : UsedGlobalSets) {
+    for (const UsedGlobalSet &UGS : llvm::reverse(UsedGlobalSets)) {
       if (UGS.UsageCount == 0)
         continue;
       if (UGS.Globals.count() > 1)
@@ -437,16 +449,6 @@ bool GlobalMergeImpl::doMerge(SmallVectorImpl<GlobalVariable *> &Globals,
     return doMerge(Globals, AllGlobals, M, isConst, AddrSpace);
   }
 
-  // Now we found a bunch of sets of globals used together. We accumulated
-  // the number of times we encountered the sets (i.e., the number of functions
-  // that use that exact set of globals). Multiply that by the size of the set
-  // to give us a crude profitability metric.
-  llvm::stable_sort(UsedGlobalSets,
-                    [](const UsedGlobalSet &UGS1, const UsedGlobalSet &UGS2) {
-                      return UGS1.Globals.count() * UGS1.UsageCount >=
-                             UGS2.Globals.count() * UGS2.UsageCount;
-                    });
-
   // Starting from the sets with the best (=biggest) profitability, find a
   // good combination.
   // The ideal (and expensive) solution can only be found by trying all
@@ -456,7 +458,7 @@ bool GlobalMergeImpl::doMerge(SmallVectorImpl<GlobalVariable *> &Globals,
   BitVector PickedGlobals(Globals.size());
   bool Changed = false;
 
-  for (const UsedGlobalSet &UGS : UsedGlobalSets) {
+  for (const UsedGlobalSet &UGS : llvm::reverse(UsedGlobalSets)) {
     if (UGS.UsageCount == 0)
       continue;
     if (PickedGlobals.anyCommon(UGS.Globals))



More information about the llvm-commits mailing list