[llvm] acacec3 - [LiveDebugValues][nfc] Reduce memory usage of InstrRef (#76051)
via llvm-commits
llvm-commits at lists.llvm.org
Sat Dec 23 08:44:50 PST 2023
Author: Felipe de Azevedo Piovezan
Date: 2023-12-23T13:44:45-03:00
New Revision: acacec3bbf4586ef9bc6c4f31707d3515d5215a1
URL: https://github.com/llvm/llvm-project/commit/acacec3bbf4586ef9bc6c4f31707d3515d5215a1
DIFF: https://github.com/llvm/llvm-project/commit/acacec3bbf4586ef9bc6c4f31707d3515d5215a1.diff
LOG: [LiveDebugValues][nfc] Reduce memory usage of InstrRef (#76051)
Commit 1b531d54f623 (#74203) removed the usage of unique_ptrs of arrays
in favour of using vectors, but inadvertently increased peak memory
usage by removing the ability to deallocate vector memory that was no
longer needed mid-LDV.
In that same review, it was pointed out that `FuncValueTable` typedef
could be removed, since it was "just a vector".
This commit addresses both issues by making `FuncValueTable` a real data
structure, capable of mapping BBs to ValueTables and able to free
ValueTables as needed.
This reduces peak memory usage in the compiler by 10% in the benchmarks
flagged by the original review.
As a consequence, we had to remove a handful of instances of the
"declare-then-initialize" antipattern in unittests, as the
FuncValueTable class is no longer default-constructible.
Added:
Modified:
llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
llvm/unittests/CodeGen/InstrRefLDVTest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
index aeb8a20e1f1221..9037f752dc4f36 100644
--- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
+++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
@@ -2413,7 +2413,7 @@ bool InstrRefBasedLDV::mlocJoin(
// Pick out the first predecessors live-out value for this location. It's
// guaranteed to not be a backedge, as we order by RPO.
- ValueIDNum FirstVal = OutLocs[BlockOrders[0]->getNumber()][Idx.asU64()];
+ ValueIDNum FirstVal = OutLocs[*BlockOrders[0]][Idx.asU64()];
// If we've already eliminated a PHI here, do no further checking, just
// propagate the first live-in value into this block.
@@ -2430,8 +2430,7 @@ bool InstrRefBasedLDV::mlocJoin(
bool Disagree = false;
for (unsigned int I = 1; I < BlockOrders.size(); ++I) {
const MachineBasicBlock *PredMBB = BlockOrders[I];
- const ValueIDNum &PredLiveOut =
- OutLocs[PredMBB->getNumber()][Idx.asU64()];
+ const ValueIDNum &PredLiveOut = OutLocs[*PredMBB][Idx.asU64()];
// Incoming values agree, continue trying to eliminate this PHI.
if (FirstVal == PredLiveOut)
@@ -2556,7 +2555,7 @@ void InstrRefBasedLDV::placeMLocPHIs(
auto InstallPHIsAtLoc = [&PHIBlocks, &MInLocs](LocIdx L) {
for (const MachineBasicBlock *MBB : PHIBlocks)
- MInLocs[MBB->getNumber()][L.asU64()] = ValueIDNum(MBB->getNumber(), 0, L);
+ MInLocs[*MBB][L.asU64()] = ValueIDNum(MBB->getNumber(), 0, L);
};
// For locations with no reg units, just place PHIs.
@@ -2635,7 +2634,8 @@ void InstrRefBasedLDV::buildMLocValueMap(
// Initialize entry block to PHIs. These represent arguments.
for (auto Location : MTracker->locations())
- MInLocs[0][Location.Idx.asU64()] = ValueIDNum(0, 0, Location.Idx);
+ MInLocs.tableForEntryMBB()[Location.Idx.asU64()] =
+ ValueIDNum(0, 0, Location.Idx);
MTracker->reset();
@@ -2664,7 +2664,7 @@ void InstrRefBasedLDV::buildMLocValueMap(
// Join the values in all predecessor blocks.
bool InLocsChanged;
- InLocsChanged = mlocJoin(*MBB, Visited, MOutLocs, MInLocs[CurBB]);
+ InLocsChanged = mlocJoin(*MBB, Visited, MOutLocs, MInLocs[*MBB]);
InLocsChanged |= Visited.insert(MBB).second;
// Don't examine transfer function if we've visited this loc at least
@@ -2673,7 +2673,7 @@ void InstrRefBasedLDV::buildMLocValueMap(
continue;
// Load the current set of live-ins into MLocTracker.
- MTracker->loadFromArray(MInLocs[CurBB], CurBB);
+ MTracker->loadFromArray(MInLocs[*MBB], CurBB);
// Each element of the transfer function can be a new def, or a read of
// a live-in value. Evaluate each element, and store to "ToRemap".
@@ -2700,8 +2700,8 @@ void InstrRefBasedLDV::buildMLocValueMap(
// the transfer function, and mlocJoin.
bool OLChanged = false;
for (auto Location : MTracker->locations()) {
- OLChanged |= MOutLocs[CurBB][Location.Idx.asU64()] != Location.Value;
- MOutLocs[CurBB][Location.Idx.asU64()] = Location.Value;
+ OLChanged |= MOutLocs[*MBB][Location.Idx.asU64()] != Location.Value;
+ MOutLocs[*MBB][Location.Idx.asU64()] = Location.Value;
}
MTracker->reset();
@@ -2844,7 +2844,6 @@ std::optional<ValueIDNum> InstrRefBasedLDV::pickOperandPHILoc(
unsigned NumLocs = MTracker->getNumLocs();
for (const auto p : BlockOrders) {
- unsigned ThisBBNum = p->getNumber();
auto OutValIt = LiveOuts.find(p);
assert(OutValIt != LiveOuts.end());
const DbgValue &OutVal = *OutValIt->second;
@@ -2863,7 +2862,7 @@ std::optional<ValueIDNum> InstrRefBasedLDV::pickOperandPHILoc(
ValueIDNum ValToLookFor = OutValOp.ID;
// Search the live-outs of the predecessor for the specified value.
for (unsigned int I = 0; I < NumLocs; ++I) {
- if (MOutLocs[ThisBBNum][I] == ValToLookFor)
+ if (MOutLocs[*p][I] == ValToLookFor)
Locs.back().push_back(LocIdx(I));
}
} else {
@@ -2876,7 +2875,7 @@ std::optional<ValueIDNum> InstrRefBasedLDV::pickOperandPHILoc(
// machine-value PHI locations.
for (unsigned int I = 0; I < NumLocs; ++I) {
ValueIDNum MPHI(MBB.getNumber(), 0, LocIdx(I));
- if (MOutLocs[ThisBBNum][I] == MPHI)
+ if (MOutLocs[*p][I] == MPHI)
Locs.back().push_back(LocIdx(I));
}
}
@@ -3498,19 +3497,15 @@ bool InstrRefBasedLDV::depthFirstVLocAndEmit(
// Helper lambda for ejecting a block -- if nothing is going to use the block,
// we can translate the variable location information into DBG_VALUEs and then
// free all of InstrRefBasedLDV's data structures.
- SmallPtrSet<const MachineBasicBlock *, 8> EjectedBBs;
auto EjectBlock = [&](MachineBasicBlock &MBB) -> void {
- if (EjectedBBs.insert(&MBB).second == false)
- return;
unsigned BBNum = MBB.getNumber();
AllTheVLocs[BBNum].clear();
// Prime the transfer-tracker, and then step through all the block
// instructions, installing transfers.
MTracker->reset();
- MTracker->loadFromArray(MInLocs[BBNum], BBNum);
- TTracker->loadInlocs(MBB, MInLocs[BBNum], DbgOpStore, Output[BBNum],
- NumLocs);
+ MTracker->loadFromArray(MInLocs[MBB], BBNum);
+ TTracker->loadInlocs(MBB, MInLocs[MBB], DbgOpStore, Output[BBNum], NumLocs);
CurBB = BBNum;
CurInst = 1;
@@ -3521,8 +3516,8 @@ bool InstrRefBasedLDV::depthFirstVLocAndEmit(
}
// Free machine-location tables for this block.
- MInLocs[BBNum] = ValueTable();
- MOutLocs[BBNum] = ValueTable();
+ MInLocs.ejectTableForBlock(MBB);
+ MOutLocs.ejectTableForBlock(MBB);
// We don't need live-in variable values for this block either.
Output[BBNum].clear();
AllTheVLocs[BBNum].clear();
@@ -3587,7 +3582,8 @@ bool InstrRefBasedLDV::depthFirstVLocAndEmit(
// anything for such out-of-scope blocks, but for the sake of being similar
// to VarLocBasedLDV, eject these too.
for (auto *MBB : ArtificialBlocks)
- EjectBlock(*MBB);
+ if (MInLocs.hasTableFor(*MBB))
+ EjectBlock(*MBB);
return emitTransfers(AllVarsNumbering);
}
@@ -3686,8 +3682,8 @@ bool InstrRefBasedLDV::ExtendRanges(MachineFunction &MF,
// machine values. The outer dimension is the block number; while the inner
// dimension is a LocIdx from MLocTracker.
unsigned NumLocs = MTracker->getNumLocs();
- FuncValueTable MOutLocs(MaxNumBlocks, ValueTable(NumLocs));
- FuncValueTable MInLocs(MaxNumBlocks, ValueTable(NumLocs));
+ FuncValueTable MOutLocs(MaxNumBlocks, NumLocs);
+ FuncValueTable MInLocs(MaxNumBlocks, NumLocs);
// Solve the machine value dataflow problem using the MLocTransfer function,
// storing the computed live-ins / live-outs into the array-of-arrays. We use
@@ -3725,7 +3721,7 @@ bool InstrRefBasedLDV::ExtendRanges(MachineFunction &MF,
CurBB = MBB.getNumber();
VTracker = &vlocs[CurBB];
VTracker->MBB = &MBB;
- MTracker->loadFromArray(MInLocs[CurBB], CurBB);
+ MTracker->loadFromArray(MInLocs[MBB], CurBB);
CurInst = 1;
for (auto &MI : MBB) {
process(MI, &MOutLocs, &MInLocs);
@@ -3939,7 +3935,7 @@ class LDVSSAUpdater {
/// Find the live-in value number for the given block. Looks up the value at
/// the PHI location on entry.
BlockValueNum getValue(LDVSSABlock *LDVBB) {
- return MLiveIns[LDVBB->BB.getNumber()][Loc.asU64()].asU64();
+ return MLiveIns[LDVBB->BB][Loc.asU64()].asU64();
}
};
@@ -4179,8 +4175,7 @@ std::optional<ValueIDNum> InstrRefBasedLDV::resolveDbgPHIsImpl(
});
for (auto &PHI : SortedPHIs) {
- ValueIDNum ThisBlockValueNum =
- MLiveIns[PHI->ParentBlock->BB.getNumber()][Loc.asU64()];
+ ValueIDNum ThisBlockValueNum = MLiveIns[PHI->ParentBlock->BB][Loc.asU64()];
// Are all these things actually defined?
for (auto &PHIIt : PHI->IncomingValues) {
@@ -4189,7 +4184,7 @@ std::optional<ValueIDNum> InstrRefBasedLDV::resolveDbgPHIsImpl(
return std::nullopt;
ValueIDNum ValueToCheck;
- const ValueTable &BlockLiveOuts = MLiveOuts[PHIIt.first->BB.getNumber()];
+ const ValueTable &BlockLiveOuts = MLiveOuts[PHIIt.first->BB];
auto VVal = ValidatedValues.find(PHIIt.first);
if (VVal == ValidatedValues.end()) {
diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
index d6dbb1feda3e8e..ccc284b6233105 100644
--- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
+++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
@@ -207,9 +207,48 @@ using namespace llvm;
/// Type for a table of values in a block.
using ValueTable = SmallVector<ValueIDNum, 0>;
-/// Type for a table-of-table-of-values, i.e., the collection of either
-/// live-in or live-out values for each block in the function.
-using FuncValueTable = SmallVector<ValueTable, 0>;
+/// A collection of ValueTables, one per BB in a function, with convenient
+/// accessor methods.
+struct FuncValueTable {
+ FuncValueTable(int NumBBs, int NumLocs) {
+ Storage.reserve(NumBBs);
+ for (int i = 0; i != NumBBs; ++i)
+ Storage.push_back(
+ std::make_unique<ValueTable>(NumLocs, ValueIDNum::EmptyValue));
+ }
+
+ /// Returns the ValueTable associated with MBB.
+ ValueTable &operator[](const MachineBasicBlock &MBB) const {
+ return (*this)[MBB.getNumber()];
+ }
+
+ /// Returns the ValueTable associated with the MachineBasicBlock whose number
+ /// is MBBNum.
+ ValueTable &operator[](int MBBNum) const {
+ auto &TablePtr = Storage[MBBNum];
+ assert(TablePtr && "Trying to access a deleted table");
+ return *TablePtr;
+ }
+
+ /// Returns the ValueTable associated with the entry MachineBasicBlock.
+ ValueTable &tableForEntryMBB() const { return (*this)[0]; }
+
+ /// Returns true if the ValueTable associated with MBB has not been freed.
+ bool hasTableFor(MachineBasicBlock &MBB) const {
+ return Storage[MBB.getNumber()] != nullptr;
+ }
+
+ /// Frees the memory of the ValueTable associated with MBB.
+ void ejectTableForBlock(const MachineBasicBlock &MBB) {
+ Storage[MBB.getNumber()].reset();
+ }
+
+private:
+ /// ValueTables are stored as unique_ptrs to allow for deallocation during
+ /// LDV; this was measured to have a significant impact on compiler memory
+ /// usage.
+ SmallVector<std::unique_ptr<ValueTable>, 0> Storage;
+};
/// Thin wrapper around an integer -- designed to give more type safety to
/// spill location numbers.
diff --git a/llvm/unittests/CodeGen/InstrRefLDVTest.cpp b/llvm/unittests/CodeGen/InstrRefLDVTest.cpp
index acbcd247fa9a01..7d4eaf388f7271 100644
--- a/llvm/unittests/CodeGen/InstrRefLDVTest.cpp
+++ b/llvm/unittests/CodeGen/InstrRefLDVTest.cpp
@@ -497,8 +497,7 @@ body: |
std::pair<FuncValueTable, FuncValueTable>
allocValueTables(unsigned Blocks, unsigned Locs) {
- return {FuncValueTable(Blocks, ValueTable(Locs)),
- FuncValueTable(Blocks, ValueTable(Locs))};
+ return {FuncValueTable(Blocks, Locs), FuncValueTable(Blocks, Locs)};
}
};
@@ -917,8 +916,7 @@ TEST_F(InstrRefLDVTest, MLocSingleBlock) {
// Set up live-in and live-out tables for this function: two locations (we
// add one later) in a single block.
- FuncValueTable MOutLocs, MInLocs;
- std::tie(MOutLocs, MInLocs) = allocValueTables(1, 2);
+ auto [MOutLocs, MInLocs] = allocValueTables(1, 2);
// Transfer function: nothing.
SmallVector<MLocTransferMap, 1> TransferFunc;
@@ -983,8 +981,7 @@ TEST_F(InstrRefLDVTest, MLocDiamondBlocks) {
Register RAX = getRegByName("RAX");
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(RAX);
- FuncValueTable MInLocs, MOutLocs;
- std::tie(MInLocs, MOutLocs) = allocValueTables(4, 2);
+ auto [MInLocs, MOutLocs] = allocValueTables(4, 2);
// Transfer function: start with nothing.
SmallVector<MLocTransferMap, 1> TransferFunc;
@@ -1137,8 +1134,7 @@ TEST_F(InstrRefLDVTest, MLocDiamondSpills) {
// There are other locations, for things like xmm0, which we're going to
// ignore here.
- FuncValueTable MInLocs, MOutLocs;
- std::tie(MInLocs, MOutLocs) = allocValueTables(4, 11);
+ auto [MInLocs, MOutLocs] = allocValueTables(4, 11);
// Transfer function: start with nothing.
SmallVector<MLocTransferMap, 1> TransferFunc;
@@ -1199,8 +1195,7 @@ TEST_F(InstrRefLDVTest, MLocSimpleLoop) {
Register RAX = getRegByName("RAX");
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(RAX);
- FuncValueTable MInLocs, MOutLocs;
- std::tie(MInLocs, MOutLocs) = allocValueTables(3, 2);
+ auto [MInLocs, MOutLocs] = allocValueTables(3, 2);
SmallVector<MLocTransferMap, 1> TransferFunc;
TransferFunc.resize(3);
@@ -1298,8 +1293,7 @@ TEST_F(InstrRefLDVTest, MLocNestedLoop) {
Register RAX = getRegByName("RAX");
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(RAX);
- FuncValueTable MInLocs, MOutLocs;
- std::tie(MInLocs, MOutLocs) = allocValueTables(5, 2);
+ auto [MInLocs, MOutLocs] = allocValueTables(5, 2);
SmallVector<MLocTransferMap, 1> TransferFunc;
TransferFunc.resize(5);
@@ -1500,8 +1494,7 @@ TEST_F(InstrRefLDVTest, MLocNoDominatingLoop) {
Register RAX = getRegByName("RAX");
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(RAX);
- FuncValueTable MInLocs, MOutLocs;
- std::tie(MInLocs, MOutLocs) = allocValueTables(5, 2);
+ auto [MInLocs, MOutLocs] = allocValueTables(5, 2);
SmallVector<MLocTransferMap, 1> TransferFunc;
TransferFunc.resize(5);
@@ -1656,8 +1649,7 @@ TEST_F(InstrRefLDVTest, MLocBadlyNestedLoops) {
Register RAX = getRegByName("RAX");
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(RAX);
- FuncValueTable MInLocs, MOutLocs;
- std::tie(MInLocs, MOutLocs) = allocValueTables(5, 2);
+ auto [MInLocs, MOutLocs] = allocValueTables(5, 2);
SmallVector<MLocTransferMap, 1> TransferFunc;
TransferFunc.resize(5);
@@ -1789,8 +1781,7 @@ TEST_F(InstrRefLDVTest, pickVPHILocDiamond) {
Register RAX = getRegByName("RAX");
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(RAX);
- FuncValueTable MInLocs, MOutLocs;
- std::tie(MInLocs, MOutLocs) = allocValueTables(4, 2);
+ auto [MInLocs, MOutLocs] = allocValueTables(4, 2);
initValueArray(MOutLocs, 4, 2);
@@ -1986,8 +1977,7 @@ TEST_F(InstrRefLDVTest, pickVPHILocLoops) {
Register RAX = getRegByName("RAX");
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(RAX);
- FuncValueTable MInLocs, MOutLocs;
- std::tie(MInLocs, MOutLocs) = allocValueTables(3, 2);
+ auto [MInLocs, MOutLocs] = allocValueTables(3, 2);
initValueArray(MOutLocs, 3, 2);
@@ -2117,8 +2107,7 @@ TEST_F(InstrRefLDVTest, pickVPHILocBadlyNestedLoops) {
Register RBX = getRegByName("RBX");
LocIdx RbxLoc = MTracker->lookupOrTrackRegister(RBX);
- FuncValueTable MInLocs, MOutLocs;
- std::tie(MInLocs, MOutLocs) = allocValueTables(5, 3);
+ auto [MInLocs, MOutLocs] = allocValueTables(5, 3);
initValueArray(MOutLocs, 5, 3);
@@ -2635,8 +2624,7 @@ TEST_F(InstrRefLDVTest, VLocSingleBlock) {
ASSERT_TRUE(MTracker->getNumLocs() == 1);
LocIdx RspLoc(0);
- FuncValueTable MInLocs, MOutLocs;
- std::tie(MInLocs, MOutLocs) = allocValueTables(1, 2);
+ auto [MInLocs, MOutLocs] = allocValueTables(1, 2);
ValueIDNum LiveInRsp = ValueIDNum(0, 0, RspLoc);
DbgOpID LiveInRspID = addValueDbgOp(LiveInRsp);
@@ -2699,8 +2687,7 @@ TEST_F(InstrRefLDVTest, VLocDiamondBlocks) {
DbgOpID LiveInRaxID = addValueDbgOp(LiveInRax);
DbgOpID RspPHIInBlk3ID = addValueDbgOp(RspPHIInBlk3);
- FuncValueTable MInLocs, MOutLocs;
- std::tie(MInLocs, MOutLocs) = allocValueTables(4, 2);
+ auto [MInLocs, MOutLocs] = allocValueTables(4, 2);
initValueArray(MInLocs, 4, 2);
initValueArray(MOutLocs, 4, 2);
@@ -2921,8 +2908,7 @@ TEST_F(InstrRefLDVTest, VLocSimpleLoop) {
DbgOpID RspDefInBlk1ID = addValueDbgOp(RspDefInBlk1);
DbgOpID RaxPHIInBlk1ID = addValueDbgOp(RaxPHIInBlk1);
- FuncValueTable MInLocs, MOutLocs;
- std::tie(MInLocs, MOutLocs) = allocValueTables(3, 2);
+ auto [MInLocs, MOutLocs] = allocValueTables(3, 2);
initValueArray(MInLocs, 3, 2);
initValueArray(MOutLocs, 3, 2);
@@ -3200,8 +3186,7 @@ TEST_F(InstrRefLDVTest, VLocNestedLoop) {
DbgOpID RspPHIInBlk2ID = addValueDbgOp(RspPHIInBlk2);
DbgOpID RspDefInBlk2ID = addValueDbgOp(RspDefInBlk2);
- FuncValueTable MInLocs, MOutLocs;
- std::tie(MInLocs, MOutLocs) = allocValueTables(5, 2);
+ auto [MInLocs, MOutLocs] = allocValueTables(5, 2);
initValueArray(MInLocs, 5, 2);
initValueArray(MOutLocs, 5, 2);
More information about the llvm-commits
mailing list