Skip to content

Commit e5e55c0

Browse files
[GlobalMerge][NFC] Skip sorting by profitability when it is not needed (#124146)
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.
1 parent 970094d commit e5e55c0

File tree

1 file changed

+12
-14
lines changed

1 file changed

+12
-14
lines changed

llvm/lib/CodeGen/GlobalMerge.cpp

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -423,24 +423,12 @@ bool GlobalMergeImpl::doMerge(SmallVectorImpl<GlobalVariable *> &Globals,
423423
}
424424
}
425425

426-
// Now we found a bunch of sets of globals used together. We accumulated
427-
// the number of times we encountered the sets (i.e., the number of functions
428-
// that use that exact set of globals).
429-
//
430-
// Multiply that by the size of the set to give us a crude profitability
431-
// metric.
432-
llvm::stable_sort(UsedGlobalSets,
433-
[](const UsedGlobalSet &UGS1, const UsedGlobalSet &UGS2) {
434-
return UGS1.Globals.count() * UGS1.UsageCount <
435-
UGS2.Globals.count() * UGS2.UsageCount;
436-
});
437-
438426
// We can choose to merge all globals together, but ignore globals never used
439427
// with another global. This catches the obviously non-profitable cases of
440428
// having a single global, but is aggressive enough for any other case.
441429
if (GlobalMergeIgnoreSingleUse) {
442430
BitVector AllGlobals(Globals.size());
443-
for (const UsedGlobalSet &UGS : llvm::reverse(UsedGlobalSets)) {
431+
for (const UsedGlobalSet &UGS : UsedGlobalSets) {
444432
if (UGS.UsageCount == 0)
445433
continue;
446434
if (UGS.Globals.count() > 1)
@@ -449,6 +437,16 @@ bool GlobalMergeImpl::doMerge(SmallVectorImpl<GlobalVariable *> &Globals,
449437
return doMerge(Globals, AllGlobals, M, isConst, AddrSpace);
450438
}
451439

440+
// Now we found a bunch of sets of globals used together. We accumulated
441+
// the number of times we encountered the sets (i.e., the number of functions
442+
// that use that exact set of globals). Multiply that by the size of the set
443+
// to give us a crude profitability metric.
444+
llvm::stable_sort(UsedGlobalSets,
445+
[](const UsedGlobalSet &UGS1, const UsedGlobalSet &UGS2) {
446+
return UGS1.Globals.count() * UGS1.UsageCount >=
447+
UGS2.Globals.count() * UGS2.UsageCount;
448+
});
449+
452450
// Starting from the sets with the best (=biggest) profitability, find a
453451
// good combination.
454452
// The ideal (and expensive) solution can only be found by trying all
@@ -458,7 +456,7 @@ bool GlobalMergeImpl::doMerge(SmallVectorImpl<GlobalVariable *> &Globals,
458456
BitVector PickedGlobals(Globals.size());
459457
bool Changed = false;
460458

461-
for (const UsedGlobalSet &UGS : llvm::reverse(UsedGlobalSets)) {
459+
for (const UsedGlobalSet &UGS : UsedGlobalSets) {
462460
if (UGS.UsageCount == 0)
463461
continue;
464462
if (PickedGlobals.anyCommon(UGS.Globals))

0 commit comments

Comments
 (0)