Skip to content

[llvm-debuginfo-analyzer] Remove LVScope::Children container #144750

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 2 commits into
base: main
Choose a base branch
from
Open

[llvm-debuginfo-analyzer] Remove LVScope::Children container #144750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
24cad3a
[ADT] Fix llvm::concat_iterator for `ValueT == common_base_class *`
jalopezg-git Jun 24, 2025
c5cea09
[llvm-debuginfo-analyzer] Remove `LVScope::Children` container
jalopezg-git Jun 24, 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
29 changes: 21 additions & 8 deletions llvm/include/llvm/ADT/STLExtras.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1032,13 +1032,22 @@ class concat_iterator

static constexpr bool ReturnsByValue =
!(std::is_reference_v<decltype(*std::declval<IterTs>())> && ...);
static constexpr bool ReturnsConvertiblePointer =
std::is_pointer_v<ValueT> &&
(std::is_convertible_v<decltype(*std::declval<IterTs>()), ValueT> && ...);

using reference_type =
typename std::conditional_t<ReturnsByValue, ValueT, ValueT &>;

using handle_type =
typename std::conditional_t<ReturnsByValue, std::optional<ValueT>,
ValueT *>;
typename std::conditional_t<ReturnsByValue || ReturnsConvertiblePointer,
ValueT, ValueT &>;

using optional_value_type =
std::conditional_t<ReturnsByValue, std::optional<ValueT>, ValueT *>;
// handle_type is used to return an optional value from `getHelper()`. If
// the type resulting from dereferencing all IterTs is a pointer that can be
// converted to `ValueT`, use that pointer type instead to avoid implicit
// conversion issues.
using handle_type = typename std::conditional_t<ReturnsConvertiblePointer,
ValueT, optional_value_type>;

/// We store both the current and end iterators for each concatenated
/// sequence in a tuple of pairs.
Expand Down Expand Up @@ -1088,7 +1097,7 @@ class concat_iterator
if (Begin == End)
return {};

if constexpr (ReturnsByValue)
if constexpr (ReturnsByValue || ReturnsConvertiblePointer)
return *Begin;
else
return &*Begin;
Expand All @@ -1105,8 +1114,12 @@ class concat_iterator

// Loop over them, and return the first result we find.
for (auto &GetHelperFn : GetHelperFns)
if (auto P = (this->*GetHelperFn)())
return *P;
if (auto P = (this->*GetHelperFn)()) {
if constexpr (ReturnsConvertiblePointer)
return P;
else
return *P;
}

llvm_unreachable("Attempted to get a pointer from an end concat iterator!");
}
Expand Down
24 changes: 14 additions & 10 deletions llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#ifndef LLVM_DEBUGINFO_LOGICALVIEW_CORE_LVSCOPE_H
#define LLVM_DEBUGINFO_LOGICALVIEW_CORE_LVSCOPE_H

#include "llvm/ADT/STLExtras.h"
#include "llvm/DebugInfo/LogicalView/Core/LVElement.h"
#include "llvm/DebugInfo/LogicalView/Core/LVLocation.h"
#include "llvm/DebugInfo/LogicalView/Core/LVSort.h"
Expand Down Expand Up @@ -94,6 +95,11 @@ class LLVM_ABI LVScope : public LVElement {
LVProperties<LVScopeKind> Kinds;
LVProperties<Property> Properties;
static LVScopeDispatch Dispatch;
// Empty containers used in `getChildren()` in case there is no Types,
// Symbols, or Scopes.
static const LVTypes EmptyTypes;
static const LVSymbols EmptySymbols;
static const LVScopes EmptyScopes;

// Size in bits if this scope represents also a compound type.
uint32_t BitSize = 0;
Expand Down Expand Up @@ -128,14 +134,6 @@ class LLVM_ABI LVScope : public LVElement {
std::unique_ptr<LVLines> Lines;
std::unique_ptr<LVLocations> Ranges;

// Vector of elements (types, scopes and symbols).
// It is the union of (*Types, *Symbols and *Scopes) to be used for
// the following reasons:
// - Preserve the order the logical elements are read in.
// - To have a single container with all the logical elements, when
// the traversal does not require any specific element kind.
std::unique_ptr<LVElements> Children;

// Resolve the template parameters/arguments relationship.
void resolveTemplate();
void printEncodedArgs(raw_ostream &OS, bool Full) const;
Expand Down Expand Up @@ -213,7 +211,14 @@ class LLVM_ABI LVScope : public LVElement {
const LVScopes *getScopes() const { return Scopes.get(); }
const LVSymbols *getSymbols() const { return Symbols.get(); }
const LVTypes *getTypes() const { return Types.get(); }
const LVElements *getChildren() const { return Children.get(); }
// Return view over union of child Types, Symbols, and Scopes.
auto getUnsortedChildren() const {
return llvm::concat<LVElement *const>(Scopes ? *Scopes : EmptyScopes,
Types ? *Types : EmptyTypes,
Symbols ? *Symbols : EmptySymbols);
}
// Return sorted children (see `LVOptions::setSortMode()`).
LVElements getChildren() const;

void addElement(LVElement *Element);
void addElement(LVLine *Line);
Expand All @@ -222,7 +227,6 @@ class LLVM_ABI LVScope : public LVElement {
void addElement(LVType *Type);
void addObject(LVLocation *Location);
void addObject(LVAddress LowerAddress, LVAddress UpperAddress);
void addToChildren(LVElement *Element);

// Add the missing elements from the given 'Reference', which is the
// scope associated with any DW_AT_specification, DW_AT_abstract_origin.
Expand Down
1 change: 1 addition & 0 deletions llvm/include/llvm/DebugInfo/LogicalView/Core/LVSort.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ LLVM_ABI LVSortValue compareRange(const LVObject *LHS, const LVObject *RHS);
LLVM_ABI LVSortValue sortByKind(const LVObject *LHS, const LVObject *RHS);
LLVM_ABI LVSortValue sortByLine(const LVObject *LHS, const LVObject *RHS);
LLVM_ABI LVSortValue sortByName(const LVObject *LHS, const LVObject *RHS);
LLVM_ABI LVSortValue sortByOffset(const LVObject *LHS, const LVObject *RHS);

} // end namespace logicalview
} // end namespace llvm
Expand Down
83 changes: 42 additions & 41 deletions llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,23 @@ LVScopeDispatch LVScope::Dispatch = {
{LVScopeKind::IsTryBlock, &LVScope::getIsTryBlock},
{LVScopeKind::IsUnion, &LVScope::getIsUnion}};

void LVScope::addToChildren(LVElement *Element) {
if (!Children)
Children = std::make_unique<LVElements>();
Children->push_back(Element);
const LVTypes LVScope::EmptyTypes{};
const LVSymbols LVScope::EmptySymbols{};
const LVScopes LVScope::EmptyScopes{};

LVElements LVScope::getChildren() const {
const auto UnsortedChildren = getUnsortedChildren();
LVElements Elements{UnsortedChildren.begin(), UnsortedChildren.end()};
if (LVSortFunction SortFunction = getSortFunction()) {
llvm::stable_sort(Elements, SortFunction);
} else {
// No specific sort function, sort by `LVObject::ID` which replicates the
// order in which the elements were created.
llvm::stable_sort(Elements, [](const LVObject *LHS, const LVObject *RHS) {
return LHS->getID() < RHS->getID();
});
}
return Elements;
}

void LVScope::addElement(LVElement *Element) {
Expand Down Expand Up @@ -175,7 +188,6 @@ void LVScope::addElement(LVScope *Scope) {

// Add it to parent.
Scopes->push_back(Scope);
addToChildren(Scope);
Scope->setParent(this);

// Notify the reader about the new element being added.
Expand All @@ -202,7 +214,6 @@ void LVScope::addElement(LVSymbol *Symbol) {

// Add it to parent.
Symbols->push_back(Symbol);
addToChildren(Symbol);
Symbol->setParent(this);

// Notify the reader about the new element being added.
Expand All @@ -229,7 +240,6 @@ void LVScope::addElement(LVType *Type) {

// Add it to parent.
Types->push_back(Type);
addToChildren(Type);
Type->setParent(this);

// Notify the reader about the new element being added.
Expand Down Expand Up @@ -277,15 +287,12 @@ bool LVScope::removeElement(LVElement *Element) {
if (Element->getIsLine())
return RemoveElement(Lines);

if (RemoveElement(Children)) {
if (Element->getIsSymbol())
return RemoveElement(Symbols);
if (Element->getIsType())
return RemoveElement(Types);
if (Element->getIsScope())
return RemoveElement(Scopes);
llvm_unreachable("Invalid element.");
}
if (Element->getIsSymbol())
return RemoveElement(Symbols);
if (Element->getIsType())
return RemoveElement(Types);
if (Element->getIsScope())
return RemoveElement(Scopes);

return false;
}
Expand Down Expand Up @@ -356,9 +363,8 @@ void LVScope::updateLevel(LVScope *Parent, bool Moved) {
setLevel(Parent->getLevel() + 1);

// Update the children.
if (Children)
for (LVElement *Element : *Children)
Element->updateLevel(this, Moved);
for (LVElement *Element : getUnsortedChildren())
Element->updateLevel(this, Moved);

// Update any lines.
if (Lines)
Expand All @@ -374,13 +380,12 @@ void LVScope::resolve() {
LVElement::resolve();

// Resolve the children.
if (Children)
for (LVElement *Element : *Children) {
if (getIsGlobalReference())
// If the scope is a global reference, mark all its children as well.
Element->setIsGlobalReference();
Element->resolve();
}
for (LVElement *Element : getUnsortedChildren()) {
if (getIsGlobalReference())
// If the scope is a global reference, mark all its children as well.
Element->setIsGlobalReference();
Element->resolve();
}
}

void LVScope::resolveName() {
Expand Down Expand Up @@ -633,14 +638,13 @@ Error LVScope::doPrint(bool Split, bool Match, bool Print, raw_ostream &OS,
options().getPrintFormatting() &&
getLevel() < options().getOutputLevel()) {
// Print the children.
if (Children)
for (const LVElement *Element : *Children) {
if (Match && !Element->getHasPattern())
continue;
if (Error Err =
Element->doPrint(Split, Match, Print, *StreamSplit, Full))
return Err;
}
for (const LVElement *Element : getChildren()) {
if (Match && !Element->getHasPattern())
continue;
if (Error Err =
Element->doPrint(Split, Match, Print, *StreamSplit, Full))
return Err;
}

// Print the line records.
if (Lines)
Expand Down Expand Up @@ -692,7 +696,6 @@ void LVScope::sort() {
Traverse(Parent->Symbols, SortFunction);
Traverse(Parent->Scopes, SortFunction);
Traverse(Parent->Ranges, compareRange);
Traverse(Parent->Children, SortFunction);

if (Parent->Scopes)
for (LVScope *Scope : *Parent->Scopes)
Expand Down Expand Up @@ -978,9 +981,8 @@ bool LVScope::equals(const LVScopes *References, const LVScopes *Targets) {
void LVScope::report(LVComparePass Pass) {
getComparator().printItem(this, Pass);
getComparator().push(this);
if (Children)
for (LVElement *Element : *Children)
Element->report(Pass);
for (LVElement *Element : getChildren())
Element->report(Pass);

if (Lines)
for (LVLine *Line : *Lines)
Expand Down Expand Up @@ -1656,9 +1658,8 @@ void LVScopeCompileUnit::printMatchedElements(raw_ostream &OS,
// Print the view for the matched scopes.
for (const LVScope *Scope : MatchedScopes) {
Scope->print(OS);
if (const LVElements *Elements = Scope->getChildren())
for (LVElement *Element : *Elements)
Element->print(OS);
for (LVElement *Element : Scope->getChildren())
Element->print(OS);
}
}

Expand Down
52 changes: 34 additions & 18 deletions llvm/lib/DebugInfo/LogicalView/Core/LVSort.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,44 +64,60 @@ LVSortValue llvm::logicalview::compareRange(const LVObject *LHS,
LVSortValue llvm::logicalview::sortByKind(const LVObject *LHS,
const LVObject *RHS) {
// Order in which the object attributes are used for comparison:
// kind, name, line number, offset.
std::tuple<std::string, StringRef, uint32_t, LVOffset> Left(
LHS->kind(), LHS->getName(), LHS->getLineNumber(), LHS->getOffset());
std::tuple<std::string, StringRef, uint32_t, LVOffset> Right(
RHS->kind(), RHS->getName(), RHS->getLineNumber(), RHS->getOffset());
// kind, name, line number, offset, ID.
std::tuple<std::string, StringRef, uint32_t, LVOffset, uint32_t> Left(
LHS->kind(), LHS->getName(), LHS->getLineNumber(), LHS->getOffset(),
LHS->getID());
std::tuple<std::string, StringRef, uint32_t, LVOffset, uint32_t> Right(
RHS->kind(), RHS->getName(), RHS->getLineNumber(), RHS->getOffset(),
RHS->getID());
return Left < Right;
}

// Callback comparator based on multiple keys (First: Line).
LVSortValue llvm::logicalview::sortByLine(const LVObject *LHS,
const LVObject *RHS) {
// Order in which the object attributes are used for comparison:
// line number, name, kind, offset.
std::tuple<uint32_t, StringRef, std::string, LVOffset> Left(
LHS->getLineNumber(), LHS->getName(), LHS->kind(), LHS->getOffset());
std::tuple<uint32_t, StringRef, std::string, LVOffset> Right(
RHS->getLineNumber(), RHS->getName(), RHS->kind(), RHS->getOffset());
// line number, name, kind, offset, ID.
std::tuple<uint32_t, StringRef, std::string, LVOffset, uint32_t> Left(
LHS->getLineNumber(), LHS->getName(), LHS->kind(), LHS->getOffset(),
LHS->getID());
std::tuple<uint32_t, StringRef, std::string, LVOffset, uint32_t> Right(
RHS->getLineNumber(), RHS->getName(), RHS->kind(), RHS->getOffset(),
RHS->getID());
return Left < Right;
}

// Callback comparator based on multiple keys (First: Name).
LVSortValue llvm::logicalview::sortByName(const LVObject *LHS,
const LVObject *RHS) {
// Order in which the object attributes are used for comparison:
// name, line number, kind, offset.
std::tuple<StringRef, uint32_t, std::string, LVOffset> Left(
LHS->getName(), LHS->getLineNumber(), LHS->kind(), LHS->getOffset());
std::tuple<StringRef, uint32_t, std::string, LVOffset> Right(
RHS->getName(), RHS->getLineNumber(), RHS->kind(), RHS->getOffset());
// name, line number, kind, offset, ID.
std::tuple<StringRef, uint32_t, std::string, LVOffset, uint32_t> Left(
LHS->getName(), LHS->getLineNumber(), LHS->kind(), LHS->getOffset(),
LHS->getID());
std::tuple<StringRef, uint32_t, std::string, LVOffset, uint32_t> Right(
RHS->getName(), RHS->getLineNumber(), RHS->kind(), RHS->getOffset(),
RHS->getID());
return Left < Right;
}

// Callback comparator based on multiple keys (First: Offset).
LVSortValue llvm::logicalview::sortByOffset(const LVObject *LHS,
const LVObject *RHS) {
// Order in which the object attributes are used for comparison:
// offset, ID.
std::tuple<LVOffset, uint32_t> Left(LHS->getOffset(), LHS->getID());
std::tuple<LVOffset, uint32_t> Right(RHS->getOffset(), RHS->getID());
return Left < Right;
}

LVSortFunction llvm::logicalview::getSortFunction() {
using LVSortInfo = std::map<LVSortMode, LVSortFunction>;
static LVSortInfo SortInfo = {
{LVSortMode::None, nullptr}, {LVSortMode::Kind, sortByKind},
{LVSortMode::Line, sortByLine}, {LVSortMode::Name, sortByName},
{LVSortMode::Offset, compareOffset},
{LVSortMode::None, nullptr}, {LVSortMode::Kind, sortByKind},
{LVSortMode::Line, sortByLine}, {LVSortMode::Name, sortByName},
{LVSortMode::Offset, sortByOffset},
};

LVSortFunction SortFunction = nullptr;
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/tools/llvm-debuginfo-analyzer/DWARF/01-dwarf-compare-logical-elements.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@
; ONE-NEXT: [002] 1 {TypeAlias} 'INTPTR' -> '* const int'
; ONE-NEXT: [002] 2 {Function} extern not_inlined 'foo' -> 'int'
; ONE-NEXT: [003] {Block}
; ONE-NEXT: [004] 5 {Variable} 'CONSTANT' -> 'const INTEGER'
; ONE-NEXT: +[004] 4 {TypeAlias} 'INTEGER' -> 'int'
Copy link
Contributor Author

@jalopezg-git jalopezg-git Jun 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Hmmm... that is weird, given that for comparison, sort mode is line (see LVOptions.cpp:219), unless I am mistaken, INTEGER should have come before CONSTANT, but it was not the case. Was this (partially) broken before?

FYI, @CarlosAlbertoEnciso.

; ONE-NEXT:  [004]     5             {Variable} 'CONSTANT' -> 'const INTEGER'
; ONE-NEXT: +[004]     4             {TypeAlias} 'INTEGER' -> 'int'

; ONE-NEXT: [004] 5 {Variable} 'CONSTANT' -> 'const INTEGER'
; ONE-NEXT: [003] 2 {Parameter} 'ParamBool' -> 'bool'
; ONE-NEXT: [003] 2 {Parameter} 'ParamPtr' -> 'INTPTR'
; ONE-NEXT: [003] 2 {Parameter} 'ParamUnsigned' -> 'unsigned int'
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/tools/llvm-debuginfo-analyzer/DWARF/pr-57040-incorrect-function-compare.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@
; ONE-NEXT: [002] 1 {TypeAlias} 'INTPTR' -> '* const int'
; ONE-NEXT: [002] 2 {Function} extern not_inlined 'foo' -> 'int'
; ONE-NEXT: [003] {Block}
; ONE-NEXT: [004] 5 {Variable} 'CONSTANT' -> 'const INTEGER'
Copy link
Contributor Author

@jalopezg-git jalopezg-git Jun 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Same as before (see my other comment) -- also seen in llvm/test/tools/llvm-debuginfo-analyzer/WebAssembly/01-wasm-compare-logical-elements.test.

; ONE-NEXT: +[004] 4 {TypeAlias} 'INTEGER' -> 'int'
; ONE-NEXT: [004] 5 {Variable} 'CONSTANT' -> 'const INTEGER'
; ONE-NEXT: [003] 2 {Parameter} 'ParamBool' -> 'bool'
; ONE-NEXT: [003] 2 {Parameter} 'ParamPtr' -> 'INTPTR'
; ONE-NEXT: [003] 2 {Parameter} 'ParamUnsigned' -> 'unsigned int'
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/tools/llvm-debuginfo-analyzer/WebAssembly/01-wasm-compare-logical-elements.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@
; ONE-NEXT: [002] 1 {TypeAlias} 'INTPTR' -> '* const int'
; ONE-NEXT: [002] 2 {Function} extern not_inlined 'foo' -> 'int'
; ONE-NEXT: [003] {Block}
; ONE-NEXT: [004] 5 {Variable} 'CONSTANT' -> 'const INTEGER'
; ONE-NEXT: +[004] 4 {TypeAlias} 'INTEGER' -> 'int'
; ONE-NEXT: [004] 5 {Variable} 'CONSTANT' -> 'const INTEGER'
; ONE-NEXT: [003] 2 {Parameter} 'ParamBool' -> 'bool'
; ONE-NEXT: [003] 2 {Parameter} 'ParamPtr' -> 'INTPTR'
; ONE-NEXT: [003] 2 {Parameter} 'ParamUnsigned' -> 'unsigned int'
Expand Down
Loading
Loading