Skip to content

[llvm-objdump] Optimize live element tracking #158763

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+240 −71
Open

[llvm-objdump] Optimize live element tracking #158763

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
52720f9
[llvm-objdump] Optimize live element tracking
gulfemsavrun Sep 16, 2025
71d057a
[llvm-objdump] Remove redundant code
gulfemsavrun Sep 23, 2025
c4fb64e
[llvm-objdump] Replace std::unordered_multimap with llvm::MapVector
gulfemsavrun Oct 2, 2025
768700e
[llvm-objdump] Address further feedback
gulfemsavrun Nov 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
278 changes: 223 additions & 55 deletions llvm/tools/llvm-objdump/SourcePrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,6 @@ void InlinedFunction::dump(raw_ostream &OS) const {
void InlinedFunction::printElementLine(raw_ostream &OS,
object::SectionedAddress Addr,
bool IsEnd) const {
bool LiveIn = !IsEnd && Range.LowPC == Addr.Address;
bool LiveOut = IsEnd && Range.HighPC == Addr.Address;
if (!(LiveIn || LiveOut))
return;

uint32_t CallFile, CallLine, CallColumn, CallDiscriminator;
InlinedFuncDie.getCallerFrame(CallFile, CallLine, CallColumn,
CallDiscriminator);
Expand Down Expand Up @@ -126,8 +121,18 @@ void LiveElementPrinter::addInlinedFunction(DWARFDie FuncDie,
DWARFUnit *U = InlinedFuncDie.getDwarfUnit();
const char *InlinedFuncName = InlinedFuncDie.getName(DINameKind::LinkageName);
DWARFAddressRange Range{FuncLowPC, FuncHighPC, SectionIndex};
// Add the new element to the main vector.
LiveElements.emplace_back(std::make_unique<InlinedFunction>(
InlinedFuncName, U, FuncDie, InlinedFuncDie, Range));
// Map the element's low address (LowPC) to its pointer for fast range start
// lookup.
LiveElementsByAddress[FuncLowPC].push_back(LiveElements.back().get());
// Map the element's high address (HighPC) to its pointer for fast range end
// lookup.
LiveElementsByEndAddress[FuncHighPC].push_back(LiveElements.back().get());
// Map the pointer to its DWARF discovery index (ElementIdx) for deterministic
Copy link
Collaborator

@jh7370 jh7370 Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as elsewhere "ElementIdx" in this context is not really a useful term. Better would be to say what is meant by "discovery index".

Copy link
Contributor Author

@gulfemsavrun gulfemsavrun Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// ordering.
ElementPtrToIndex[LiveElements.back().get()] = LiveElements.size() - 1;
}

void LiveElementPrinter::addVariable(DWARFDie FuncDie, DWARFDie VarDie) {
Expand All @@ -147,18 +152,34 @@ void LiveElementPrinter::addVariable(DWARFDie FuncDie, DWARFDie VarDie) {
}

for (const DWARFLocationExpression &LocExpr : *Locs) {
std::unique_ptr<LiveVariable> NewVar;
if (LocExpr.Range) {
LiveElements.emplace_back(
std::make_unique<LiveVariable>(LocExpr, VarName, U, FuncDie));
NewVar = std::make_unique<LiveVariable>(LocExpr, VarName, U, FuncDie);
Copy link
Collaborator

@jh7370 jh7370 Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than creating a local variable here and in the else clause, then immediately moving it after the if/else, perhaps you could have a new function along the lines of createNewLiveVariable that creates the entry in LiveElements directly, like before, and adds it to the various data structures too? I think it would help reduce the size of this function, which is getting a bit big.

Copy link
Contributor Author

@gulfemsavrun gulfemsavrun Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I introduced registerNewVariable function.

} else {
// If the LocExpr does not have an associated range, it is valid for
// the whole of the function.
// TODO: technically it is not valid for any range covered by another
// LocExpr, does that happen in reality?
DWARFLocationExpression WholeFuncExpr{
DWARFAddressRange(FuncLowPC, FuncHighPC, SectionIndex), LocExpr.Expr};
LiveElements.emplace_back(
std::make_unique<LiveVariable>(WholeFuncExpr, VarName, U, FuncDie));
NewVar =
std::make_unique<LiveVariable>(WholeFuncExpr, VarName, U, FuncDie);
}

// Add the new variable to all the data structures.
if (NewVar) {
LiveElements.emplace_back(std::move(NewVar));
LiveVariable *CurrentVar =
static_cast<LiveVariable *>(LiveElements.back().get());
// Map from a LiveElement pointer to its index in the LiveElements vector.
ElementPtrToIndex.emplace(CurrentVar, LiveElements.size() - 1);
if (CurrentVar->getLocExpr().Range) {
// Add the variable to address-based maps.
LiveElementsByAddress[CurrentVar->getLocExpr().Range->LowPC].push_back(
CurrentVar);
LiveElementsByEndAddress[CurrentVar->getLocExpr().Range->HighPC]
.push_back(CurrentVar);
}
}
}
}
Expand Down Expand Up @@ -205,14 +226,56 @@ unsigned LiveElementPrinter::moveToFirstVarColumn(formatted_raw_ostream &OS) {
return FirstUnprintedLogicalColumn;
}

unsigned LiveElementPrinter::findFreeColumn() {
for (unsigned ColIdx = 0; ColIdx < ActiveCols.size(); ++ColIdx)
if (!ActiveCols[ColIdx].isActive())
return ColIdx;
unsigned LiveElementPrinter::getOrCreateColumn(unsigned ElementIdx) {
// Check if the element already has an assigned column.
auto it = ElementToColumn.find(ElementIdx);
if (it != ElementToColumn.end()) {
Copy link
Collaborator

@jh7370 jh7370 Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: coding standard for single line if says no braces.

Copy link
Contributor Author

@gulfemsavrun gulfemsavrun Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

return it->second;
}

size_t OldSize = ActiveCols.size();
ActiveCols.grow(std::max<size_t>(OldSize * 2, 1));
return OldSize;
unsigned ColIdx;
if (!FreeCols.empty()) {
// Get the smallest available index from the set.
ColIdx = *FreeCols.begin();
// Remove the index from the set.
FreeCols.erase(FreeCols.begin());
} else {
// No free columns, so create a new one.
ColIdx = ActiveCols.size();
ActiveCols.emplace_back();
}

// Assign the element to the column and update the map.
ElementToColumn[ElementIdx] = ColIdx;
ActiveCols[ColIdx].ElementIdx = ElementIdx;
return ColIdx;
}

void LiveElementPrinter::freeColumn(unsigned ColIdx) {
unsigned ElementIdx = ActiveCols[ColIdx].ElementIdx;

// Clear the column's data and add it to the free list.
ActiveCols[ColIdx].ElementIdx = Column::NullElementIdx;
ActiveCols[ColIdx].LiveIn = false;
ActiveCols[ColIdx].LiveOut = false;
ActiveCols[ColIdx].MustDrawLabel = false;

// Remove the element's entry from the map and add the column to the free
// list.
ElementToColumn.erase(ElementIdx);
FreeCols.insert(ColIdx);
}

std::vector<unsigned>
LiveElementPrinter::getSortedActiveElementIndices() const {
// Get all ElementIdx values that currently have an assigned column.
std::vector<unsigned> Indices;
for (const auto &Pair : ElementToColumn)
Indices.push_back(Pair.first);

// Sort by ElementIdx, which is the DWARF discovery order.
llvm::stable_sort(Indices);
return Indices;
}

void LiveElementPrinter::dump() const {
Expand All @@ -239,57 +302,116 @@ void LiveElementPrinter::addCompileUnit(DWARFDie D) {
void LiveElementPrinter::update(object::SectionedAddress ThisAddr,
object::SectionedAddress NextAddr,
bool IncludeDefinedVars) {
// Do not create live ranges when debug-inlined-funcs option is provided with
// line format option.
// Exit early if only printing function limits.
if (DbgInlinedFunctions == DFLimitsOnly)
return;

// First, check variables which have already been assigned a column, so
// that we don't change their order.
SmallSet<unsigned, 8> CheckedElementIdxs;
// Free columns identified in the previous cycle.
for (unsigned ColIdx : ColumnsToFreeNextCycle)
freeColumn(ColIdx);
ColumnsToFreeNextCycle.clear();

// Update status of active columns and collect those to free next cycle.
for (unsigned ColIdx = 0, End = ActiveCols.size(); ColIdx < End; ++ColIdx) {
if (!ActiveCols[ColIdx].isActive())
continue;

CheckedElementIdxs.insert(ActiveCols[ColIdx].ElementIdx);
const std::unique_ptr<LiveElement> &LE =
LiveElements[ActiveCols[ColIdx].ElementIdx];
ActiveCols[ColIdx].LiveIn = LE->liveAtAddress(ThisAddr);
ActiveCols[ColIdx].LiveOut = LE->liveAtAddress(NextAddr);
std::string Name = Demangle ? demangle(LE->getName()) : LE->getName();
LLVM_DEBUG(dbgs() << "pass 1, " << ThisAddr.Address << "-"
<< NextAddr.Address << ", " << Name << ", Col " << ColIdx
<< ": LiveIn=" << ActiveCols[ColIdx].LiveIn
<< ", LiveOut=" << ActiveCols[ColIdx].LiveOut << "\n");

if (!ActiveCols[ColIdx].LiveIn && !ActiveCols[ColIdx].LiveOut)
LLVM_DEBUG({
std::string Name = Demangle ? demangle(LE->getName()) : LE->getName();
dbgs() << "pass 1, " << ThisAddr.Address << "-" << NextAddr.Address
<< ", " << Name << ", Col " << ColIdx
<< ": LiveIn=" << ActiveCols[ColIdx].LiveIn
<< ", LiveOut=" << ActiveCols[ColIdx].LiveOut << "\n";
});

// If element is fully dead, deactivate column immediately.
if (!ActiveCols[ColIdx].LiveIn && !ActiveCols[ColIdx].LiveOut) {
ActiveCols[ColIdx].ElementIdx = Column::NullElementIdx;
continue;
}

// Mark for cleanup in the next cycle if range ends here.
if (ActiveCols[ColIdx].LiveIn && !ActiveCols[ColIdx].LiveOut)
ColumnsToFreeNextCycle.push_back(ColIdx);
}

// Next, look for variables which don't already have a column, but which
// are now live.
// are now live (those starting at ThisAddr or NextAddr).
if (IncludeDefinedVars) {
for (unsigned ElementIdx = 0, End = LiveElements.size(); ElementIdx < End;
++ElementIdx) {
if (CheckedElementIdxs.count(ElementIdx))
// Collect all elements starting at ThisAddr and NextAddr.
std::vector<std::pair<unsigned, LiveElement *>> NewLiveElements;
auto CollectNewElements = [&](const auto &It) {
if (It == LiveElementsByAddress.end())
return;

const std::vector<LiveElement *> &ElementList = It->second;
// Get the ElementIdx for sorting and column management.
Copy link
Collaborator

@jh7370 jh7370 Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than refer to the variable name (which you don't even use until later), just use the real term, i.e. something like "element index".

Copy link
Contributor Author

@gulfemsavrun gulfemsavrun Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

for (LiveElement *LE : ElementList) {
auto IndexIt = ElementPtrToIndex.find(LE);
if (IndexIt == ElementPtrToIndex.end()) {
LLVM_DEBUG(
dbgs()
<< "Error: LiveElement in map but not in ElementPtrToIndex!\n");
Copy link
Collaborator

@jh7370 jh7370 Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this not be an assertion?

Copy link
Contributor Author

@gulfemsavrun gulfemsavrun Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I converted the check to an assertion.

continue;
}

unsigned ElementIdx = IndexIt->second;
// Skip elements that already have a column.
if (ElementToColumn.count(ElementIdx))
continue;

bool LiveIn = LE->liveAtAddress(ThisAddr);
bool LiveOut = LE->liveAtAddress(NextAddr);
if (!LiveIn && !LiveOut)
continue;

NewLiveElements.emplace_back(ElementIdx, LE);
}
};

// Collect elements starting at ThisAddr.
CollectNewElements(LiveElementsByAddress.find(ThisAddr.Address));
// Collect elements starting at NextAddr (the address immediately
// following the instruction).
CollectNewElements(LiveElementsByAddress.find(NextAddr.Address));
// Sort elements by DWARF discovery order (ElementIdx) for deterministic
// column assignment.
llvm::stable_sort(NewLiveElements, [](const auto &A, const auto &B) {
return A.first < B.first;
});

// Assign columns in deterministic order.
for (const auto &ElementPair : NewLiveElements) {
unsigned ElementIdx = ElementPair.first;
// Skip if element was already added from the first range.
if (ElementToColumn.count(ElementIdx))
continue;

const std::unique_ptr<LiveElement> &LE = LiveElements[ElementIdx];
LiveElement *LE = ElementPair.second;
bool LiveIn = LE->liveAtAddress(ThisAddr);
bool LiveOut = LE->liveAtAddress(NextAddr);
if (!LiveIn && !LiveOut)
continue;

unsigned ColIdx = findFreeColumn();
std::string Name = Demangle ? demangle(LE->getName()) : LE->getName();
LLVM_DEBUG(dbgs() << "pass 2, " << ThisAddr.Address << "-"
<< NextAddr.Address << ", " << Name << ", Col "
<< ColIdx << ": LiveIn=" << LiveIn
<< ", LiveOut=" << LiveOut << "\n");
ActiveCols[ColIdx].ElementIdx = ElementIdx;
// Assign or create a column.
unsigned ColIdx = getOrCreateColumn(ElementIdx);
LLVM_DEBUG({
std::string Name = Demangle ? demangle(LE->getName()) : LE->getName();
dbgs() << "pass 2, " << ThisAddr.Address << "-" << NextAddr.Address
<< ", " << Name << ", Col " << ColIdx << ": LiveIn=" << LiveIn
<< ", LiveOut=" << LiveOut << "\n";
});

ActiveCols[ColIdx].LiveIn = LiveIn;
ActiveCols[ColIdx].LiveOut = LiveOut;
ActiveCols[ColIdx].MustDrawLabel = true;

// Mark for cleanup next cycle if range ends here.
if (ActiveCols[ColIdx].LiveIn && !ActiveCols[ColIdx].LiveOut)
ColumnsToFreeNextCycle.push_back(ColIdx);
}
}
}
Expand Down Expand Up @@ -360,7 +482,14 @@ void LiveElementPrinter::printAfterOtherLine(formatted_raw_ostream &OS,
void LiveElementPrinter::printBetweenInsts(formatted_raw_ostream &OS,
bool MustPrint) {
bool PrintedSomething = false;
for (unsigned ColIdx = 0, End = ActiveCols.size(); ColIdx < End; ++ColIdx) {
// Get all active elements, sorted by discovery order (ElementIdx).
std::vector<unsigned> SortedElementIndices = getSortedActiveElementIndices();
// The outer loop iterates over the deterministic DWARF discovery order
// (ElementIdx).
for (unsigned ElementIdx : SortedElementIndices) {
// Look up the physical column index (ColIdx) assigned to this
// element. We use .at() because we are certain the element is active.
unsigned ColIdx = ElementToColumn.at(ElementIdx);
if (ActiveCols[ColIdx].isActive() && ActiveCols[ColIdx].MustDrawLabel) {
// First we need to print the live range markers for any active
// columns to the left of this one.
Expand All @@ -375,8 +504,7 @@ void LiveElementPrinter::printBetweenInsts(formatted_raw_ostream &OS,
OS << " ";
}

const std::unique_ptr<LiveElement> &LE =
LiveElements[ActiveCols[ColIdx].ElementIdx];
const std::unique_ptr<LiveElement> &LE = LiveElements[ElementIdx];
// Then print the variable name and location of the new live range,
// with box drawing characters joining it to the live range line.
OS << getLineChar(ActiveCols[ColIdx].LiveIn ? LineChar::LabelCornerActive
Expand Down Expand Up @@ -440,20 +568,60 @@ void LiveElementPrinter::printAfterInst(formatted_raw_ostream &OS) {

void LiveElementPrinter::printStartLine(formatted_raw_ostream &OS,
object::SectionedAddress Addr) {
// Print a line to idenfity the start of an inlined function if line format
// is specified.
if (DbgInlinedFunctions == DFLimitsOnly)
for (const std::unique_ptr<LiveElement> &LE : LiveElements)
LE->printElementLine(OS, Addr, false);
// Only print the start line for inlined functions if DFLimitsOnly is
// enabled.
if (DbgInlinedFunctions != DFLimitsOnly)
return;

// Use the map to find all elements that start at the given address.
std::vector<unsigned> ElementIndices;
auto It = LiveElementsByAddress.find(Addr.Address);
if (It != LiveElementsByAddress.end()) {
for (LiveElement *LE : It->second) {
// Look up the ElementIdx from the pointer.
auto IndexIt = ElementPtrToIndex.find(LE);
if (IndexIt != ElementPtrToIndex.end())
Copy link
Collaborator

@jh7370 jh7370 Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Under what circumstances can IndexIt be the end iterator?

Copy link
Contributor Author

@gulfemsavrun gulfemsavrun Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is more reasonable to convert this into an assertion. As long as live elements are correctly populated across all data structures, IndexIt will never equal the end iterator.

ElementIndices.push_back(IndexIt->second);
}
}

// Sort the indices to ensure deterministic output order (by DWARF discovery
// order).
llvm::stable_sort(ElementIndices);

for (unsigned ElementIdx : ElementIndices) {
LiveElement *LE = LiveElements[ElementIdx].get();
LE->printElementLine(OS, Addr, false);
}
}

void LiveElementPrinter::printEndLine(formatted_raw_ostream &OS,
object::SectionedAddress Addr) {
// Print a line to idenfity the end of an inlined function if line format is
// specified.
if (DbgInlinedFunctions == DFLimitsOnly)
for (const std::unique_ptr<LiveElement> &LE : LiveElements)
LE->printElementLine(OS, Addr, true);
// Only print the end line for inlined functions if DFLimitsOnly is
Copy link
Collaborator

@jh7370 jh7370 Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is nearly identical to printStartLine, just with a different address map to look up in and a true instead of a false. Could you fold the business logic together and just pass these in?

Copy link
Contributor Author

@gulfemsavrun gulfemsavrun Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// enabled.
if (DbgInlinedFunctions != DFLimitsOnly)
return;

// Use the map to find elements that end at the given address.
std::vector<unsigned> ElementIndices;
auto It = LiveElementsByEndAddress.find(Addr.Address);
if (It != LiveElementsByEndAddress.end()) {
for (LiveElement *LE : It->second) {
// Look up the ElementIdx from the pointer.
auto IndexIt = ElementPtrToIndex.find(LE);
if (IndexIt != ElementPtrToIndex.end())
ElementIndices.push_back(IndexIt->second);
}
}

// Sort the indices to ensure deterministic output order (by DWARF discovery
// order).
llvm::stable_sort(ElementIndices);

for (unsigned ElementIdx : ElementIndices) {
LiveElement *LE = LiveElements[ElementIdx].get();
LE->printElementLine(OS, Addr, true);
}
}

bool SourcePrinter::cacheSource(const DILineInfo &LineInfo) {
Expand Down
Loading