Skip to content

Commit a2eccfe

Browse files
committed
[NFC] Cleanup + comments
1 parent 3487c85 commit a2eccfe

File tree

1 file changed

+34
-55
lines changed

1 file changed

+34
-55
lines changed

llvm/lib/CodeGen/StackColoring.cpp

Lines changed: 34 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,7 @@
1010
// lifetime markers machine instructions (LIFETIME_START and LIFETIME_END),
1111
// which represent the possible lifetime of stack slots. It attempts to
1212
// merge disjoint stack slots and reduce the used stack space.
13-
// NOTE: This pass is not StackSlotColoring, which optimizes spill slots.
14-
//
15-
// TODO: In the future we plan to improve stack coloring in the following ways:
16-
// 1. Allow merging multiple small slots into a single larger slot at different
17-
// offsets.
18-
// 2. Merge this pass with StackSlotColoring and allow merging of allocas with
19-
// spill slots.
13+
// NOTE: This pass is not StackSlotColoring, which optimizes only spill slots.
2014
//
2115
//===----------------------------------------------------------------------===//
2216

@@ -25,7 +19,6 @@
2519
#include "llvm/ADT/DenseMap.h"
2620
#include "llvm/ADT/DepthFirstIterator.h"
2721
#include "llvm/ADT/SmallPtrSet.h"
28-
#include "llvm/ADT/SmallSet.h"
2922
#include "llvm/ADT/SmallVector.h"
3023
#include "llvm/ADT/Statistic.h"
3124
#include "llvm/Analysis/ValueTracking.h"
@@ -101,9 +94,8 @@ static cl::opt<bool> UseNewStackColoring(
10194
"new-stack-coloring", cl::init(false), cl::Hidden,
10295
cl::desc("Use a better logic to try to reduce stack usage"));
10396

104-
static constexpr unsigned MaxCandidatesToConsiderDefault = 5;
105-
static cl::opt<unsigned> MaxCandidatesToConsider(
106-
"stackcoloring-max-candidates", cl::init(MaxCandidatesToConsiderDefault),
97+
static cl::opt<unsigned> MaxCandidatesOpt(
98+
"stackcoloring-max-candidates", cl::init(0),
10799
cl::Hidden,
108100
cl::desc(
109101
"Max number of candidates that will be evaluated, 0 means no limit"));
@@ -658,7 +650,7 @@ LLVM_DUMP_METHOD void StackColoring::SlotInfo::dump(const StackColoring* State)
658650
Slot = this - State->Slot2Info.data();
659651
dbgs() << "fi#" << Slot;
660652
} else
661-
dbgs() << "SlotInfo";
653+
dbgs() << "SlotInfo";
662654
dbgs() << ":";
663655
if (Offset != InvalidIdx)
664656
dbgs() << " offset=" << Offset;
@@ -667,10 +659,8 @@ LLVM_DUMP_METHOD void StackColoring::SlotInfo::dump(const StackColoring* State)
667659
dbgs() << " \"" << State->MFI->getObjectAllocation(Slot)->getName() << "\"";
668660
if (State->MFI->isSpillSlotObjectIndex(Slot))
669661
dbgs() << " spill";
670-
}
662+
}
671663
dbgs() << " size=" << Size << " align=" << Align.value() << '\n';
672-
if (IndexBasedLiveRange)
673-
dbgs() << "Index: " << *IndexBasedLiveRange << "\n";
674664
dumpBV("LIVENESS ", Liveness);
675665
BitVector Start;
676666
Start.resize(Liveness.size());
@@ -975,6 +965,13 @@ void StackColoring::calculateLiveIntervals(unsigned NumSlots) {
975965
unsigned SpillChangeCounter = 0;
976966

977967
if (LS && LS->getNumIntervals()) {
968+
// Here we prepare Spill slots lifetime informations
969+
// Live ranges in the LiveStacks seem to be slightly outdated in many small
970+
// ways. this is not an issue for stack-slot-coloring, because its only
971+
// operating on LiveRange form LiveStack, but it is an issue here,
972+
// So we only rely on LiveStack, to give us live edges, and conservatively
973+
// re-construct in-block liveness changes
974+
978975
for (const MachineBasicBlock &MBB : *MF) {
979976
BlockLifetimeInfo &MBBLiveness = BlockLiveness[MBB.getNumber()];
980977
MBBLiveness.LiveIn.resize(NumSlots);
@@ -1017,14 +1014,6 @@ void StackColoring::calculateLiveIntervals(unsigned NumSlots) {
10171014
unsigned Slot = PSV->getFrameIndex();
10181015
if (!LS->hasInterval(Slot))
10191016
continue;
1020-
// if (Slot == 17) {
1021-
// dbgs() << "MI: " << MI;
1022-
// dbgs() << "MBB: " << MBB.getName() << "\n";
1023-
// dbgs() << "MBB range:" << Indexes->getMBBRange(&MBB).first << "-"
1024-
// << Indexes->getMBBRange(&MBB).second << "\n";
1025-
// dbgs() << "slot range: " << LS->getInterval(Slot) << "\n";
1026-
// dbgs() << "\n";
1027-
// }
10281017
assert(MMO->isStore() != MMO->isLoad());
10291018
if (MMO->isStore()) {
10301019
if (!IsStoredTo[Slot]) {
@@ -1050,6 +1039,8 @@ void StackColoring::calculateLiveIntervals(unsigned NumSlots) {
10501039
/*IsStart=*/false, Slot});
10511040
}
10521041

1042+
// Ensure that the changes are in the same order they will be found and
1043+
// need to be processed in
10531044
std::stable_sort(MidBlockSpillChanges.begin() + SizeOnStart,
10541045
MidBlockSpillChanges.end(),
10551046
[&](SplitSlotChanges Lhs, SplitSlotChanges Rhs) -> bool {
@@ -1083,6 +1074,8 @@ void StackColoring::calculateLiveIntervals(unsigned NumSlots) {
10831074

10841075
bool StartedSinceInc = false;
10851076
auto EndRangeFor = [&](int Slot) {
1077+
// The less index the better, so we only increase if the ranges would not
1078+
// be accurate without
10861079
if (StartIdx[Slot] == CurrIdx || StartedSinceInc) {
10871080
CurrIdx++;
10881081
StartedSinceInc = false;
@@ -1103,9 +1096,6 @@ void StackColoring::calculateLiveIntervals(unsigned NumSlots) {
11031096
continue;
11041097
SlotIndex ThisIndex = Indexes->getInstructionIndex(MI);
11051098
auto OnChange = [&](unsigned Slot, bool IsStart) {
1106-
// if (Slot == 3) {
1107-
// outs() << "HERE\n";
1108-
// }
11091099
if (IsStart) {
11101100
StartedSinceInc = true;
11111101
// If a slot is already definitely in use, we don't have to emit
@@ -1211,7 +1201,6 @@ void StackColoring::remapInstructions(DenseMap<int, int>& SlotRemap, int MergedS
12111201
continue;
12121202
int Slot = VI.getStackSlot();
12131203
if (Slot >= 0 && Slot2Info[Slot].Offset != InvalidIdx) {
1214-
// FIXME: properly update the offset into MergedSlot debug
12151204
VI.updateStackSlot(MergedSlot);
12161205
}
12171206
if (auto It = SlotRemap.find(Slot); It != SlotRemap.end()) {
@@ -1587,7 +1576,7 @@ unsigned StackColoring::doMerging(unsigned NumSlots) {
15871576
SlotInfo* LastQueryLhs = nullptr;
15881577
SlotInfo* LastQueryRhs = nullptr;
15891578
bool LastQueryRes = false;
1590-
// TODO: Real caching ?
1579+
// Maybe there should be real caching here
15911580
auto HasOverlapCached = [&](SlotInfo &Lhs, SlotInfo &Rhs) {
15921581
if (&Lhs == LastQueryLhs && LastQueryRhs == &Rhs)
15931582
return LastQueryRes;
@@ -1618,8 +1607,10 @@ unsigned StackColoring::doMerging(unsigned NumSlots) {
16181607

16191608
// The slots in the linked-list are always kept in ascending order, so the
16201609
// earliest slot has the lowest offset
1621-
// This loop handles cases where the latest slot doesn't cannot be both live
1622-
// because of the CFG, so even if there lifetime overlap, they can overlap
1610+
// This loop handles cases where this slot and the latest slot doesn't
1611+
// cannot be both live because of the CFG, so even if there lifetime
1612+
// overlap, they can overlap
1613+
// See comment about implementation higher in the file
16231614
while (LLVM_UNLIKELY(Last->Slot != InvalidIdx &&
16241615
!HasOverlapCached(Info, Slot2Info[Last->Slot])))
16251616
Last = &OlderStatus[Last->Prev];
@@ -1640,7 +1631,7 @@ unsigned StackColoring::doMerging(unsigned NumSlots) {
16401631
return;
16411632
}
16421633

1643-
// Insure ordering of slots
1634+
// Ensure ordering of slots
16441635
Status* Inserted = &OlderStatus.back();
16451636
Inserted->Offset = Offset;
16461637
Inserted->Slot = &Info - Slot2Info.data();
@@ -1653,9 +1644,10 @@ unsigned StackColoring::doMerging(unsigned NumSlots) {
16531644
Curr->Prev = Idx;
16541645
};
16551646

1656-
SmallVector<unsigned, MaxCandidatesToConsiderDefault> Candidates;
1657-
unsigned MaxCandidates =
1658-
MaxCandidatesToConsider == 0 ? ~0u : MaxCandidatesToConsider;
1647+
// This is a vector but element ordering is not relevant
1648+
SmallVector<unsigned> Candidates;
1649+
1650+
unsigned MaxCandidates = MaxCandidatesOpt == 0 ? ~0u : MaxCandidatesOpt;
16591651
for (unsigned I = 0; I < MaxCandidates; I++) {
16601652
if (SlotStack.empty())
16611653
break;
@@ -1668,7 +1660,7 @@ unsigned StackColoring::doMerging(unsigned NumSlots) {
16681660
unsigned BestIdx = InvalidIdx;
16691661
unsigned BestOffset = InvalidIdx;
16701662

1671-
LLVM_DEBUG(dbgs() << "top=" << WorseCaseOffset << " choosing: ");
1663+
LLVM_DEBUG(dbgs() << "Worse is at " << WorseCaseOffset << ", choosing: ");
16721664
for (unsigned K = 0; K < Candidates.size(); K++) {
16731665
SlotInfo &Info = Slot2Info[Candidates[K]];
16741666
unsigned Offset = 0;
@@ -1707,7 +1699,8 @@ unsigned StackColoring::doMerging(unsigned NumSlots) {
17071699
if (Other.SlotPriority != Info.SlotPriority)
17081700
return Other.SlotPriority < Info.SlotPriority;
17091701

1710-
// Both are always stored in Slot2Info, so this is deterministic
1702+
// Both are always stored in Slot2Info, so this is equivalent to
1703+
// FrameIndex comparaison
17111704
return &Other < &Info;
17121705
}();
17131706

@@ -1785,10 +1778,10 @@ bool StackColoring::run(MachineFunction &Func) {
17851778
VNInfoAllocator.Reset();
17861779
Slot2Info.clear();
17871780

1788-
unsigned NumSlots = MFI->getObjectIndexEnd();
1781+
if (!UseNewStackColoring)
1782+
LS = nullptr;
17891783

1790-
// if (MF->getName() == "_ZL9transformPjS_Rm")
1791-
// outs() << "HERE\n";
1784+
unsigned NumSlots = MFI->getObjectIndexEnd();
17921785

17931786
// If there are no stack slots then there are no markers to remove.
17941787
if (NumSlots < 2 || DisableColoring)
@@ -1900,22 +1893,10 @@ bool StackColoring::run(MachineFunction &Func) {
19001893
auto &SecondS = LiveStarts[SecondSlot];
19011894
assert(!First->empty() && !Second->empty() && "Found an empty range");
19021895

1903-
bool OldNoOverlap = !First->isLiveAtIndexes(SecondS) &&
1904-
!Second->isLiveAtIndexes(FirstS);
1905-
1906-
SlotInfo &FSlot = Slot2Info[FirstSlot];
1907-
SlotInfo &SSlot = Slot2Info[SecondSlot];
1908-
bool NewNoOverlap = !FSlot.hasOverlap(SSlot);
1909-
1910-
// if (NewNoOverlap != OldNoOverlap) {
1911-
// LLVM_DEBUG(dbgs() << "OldNoOverlap=" << OldNoOverlap
1912-
// << " NewNoOverlap=" << NewNoOverlap << "\n");
1913-
// }
1914-
// assert(OldNoOverlap == NewNoOverlap);
1915-
19161896
// Merge disjoint slots. This is a little bit tricky - see the
19171897
// Implementation Notes section for an explanation.
1918-
if (OldNoOverlap) {
1898+
if (!First->isLiveAtIndexes(SecondS) &&
1899+
!Second->isLiveAtIndexes(FirstS)) {
19191900
Changed = true;
19201901
First->MergeSegmentsInAsValue(*Second, First->getValNumInfo(0));
19211902

@@ -1924,8 +1905,6 @@ bool StackColoring::run(MachineFunction &Func) {
19241905
auto Mid = FirstS.begin() + OldSize;
19251906
std::inplace_merge(FirstS.begin(), Mid, FirstS.end());
19261907

1927-
// FSlot.Liveness |= SSlot.Liveness;
1928-
19291908
SlotRemap[SecondSlot] = FirstSlot;
19301909
SortedSlots[J] = -1;
19311910
LLVM_DEBUG(dbgs() << "Merging #" << FirstSlot << " and slots #"

0 commit comments

Comments
 (0)