[llvm] [GlobalMerge][NFC] Skip sorting by profitability when it is not needed (PR #124146)
Michael Maitland via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 23 09:24:40 PST 2025
https://github.com/michaelmaitland updated https://github.com/llvm/llvm-project/pull/124146
>From 0bbe4bad5c364c1d7fbf3bdb9ebb7fcb88cb470b Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Thu, 23 Jan 2025 08:51:14 -0800
Subject: [PATCH 1/2] [GlobalMerge][NFC] Skip sorting by profitability when it
is not needed
We were previously sorting by profitability even if we were choosing to merge
all globals together, which is not impacted by UsedGlobalSet order.
We can also remove iteration of UsedGlobalSets in reverse order in both cases.
In the first csae, the order does not matter. In the second case, we just sort
by the order we need instead of sorting in the opposite direction and calling
reverse.
This change should only be an improvement on compile time. I have not measured
it, but I think it would never make things worse.
---
llvm/lib/CodeGen/GlobalMerge.cpp | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/llvm/lib/CodeGen/GlobalMerge.cpp b/llvm/lib/CodeGen/GlobalMerge.cpp
index 9d4547df046d46..40e56a66c2f7fd 100644
--- a/llvm/lib/CodeGen/GlobalMerge.cpp
+++ b/llvm/lib/CodeGen/GlobalMerge.cpp
@@ -423,24 +423,12 @@ 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 : llvm::reverse(UsedGlobalSets)) {
+ for (const UsedGlobalSet &UGS : UsedGlobalSets) {
if (UGS.UsageCount == 0)
continue;
if (UGS.Globals.count() > 1)
@@ -449,6 +437,16 @@ 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
@@ -458,7 +456,7 @@ bool GlobalMergeImpl::doMerge(SmallVectorImpl<GlobalVariable *> &Globals,
BitVector PickedGlobals(Globals.size());
bool Changed = false;
- for (const UsedGlobalSet &UGS : llvm::reverse(UsedGlobalSets)) {
+ for (const UsedGlobalSet &UGS : UsedGlobalSets) {
if (UGS.UsageCount == 0)
continue;
if (PickedGlobals.anyCommon(UGS.Globals))
>From f71bd490c13036de6d96421d4c42b37510380366 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Thu, 23 Jan 2025 09:23:33 -0800
Subject: [PATCH 2/2] fixup! clang-format
---
llvm/lib/CodeGen/GlobalMerge.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/CodeGen/GlobalMerge.cpp b/llvm/lib/CodeGen/GlobalMerge.cpp
index 40e56a66c2f7fd..e1373224592df1 100644
--- a/llvm/lib/CodeGen/GlobalMerge.cpp
+++ b/llvm/lib/CodeGen/GlobalMerge.cpp
@@ -438,7 +438,7 @@ 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
+ // 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,
More information about the llvm-commits
mailing list