[llvm-debuginfo-analyzer] Remove LVScope::Children container
#144750
+60
−50
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix llvm::concat_iterator for the case of `ValueT` being a pointer to a common base class to which the result of dereferencing any iterator in `ItersT` can be casted to.
Remove the `LVScope::Children` container and use `llvm::concat()` instead to return a view over the types, symbols, and sub-scopes contained in a given `LVScope`.
|
@llvm/pr-subscribers-llvm-adt @llvm/pr-subscribers-debuginfo Author: Javier Lopez-Gomez (jalopezg-git) ChangesRemove the Fixes #69160. Full diff: https://github.com/llvm/llvm-project/pull/144750.diff 5 Files Affected:
diff --git a/llvm/include/llvm/ADT/STLExtras.h b/llvm/include/llvm/ADT/STLExtras.h
index eea06cfb99ba2..6dd9b5a2430d2 100644
--- a/llvm/include/llvm/ADT/STLExtras.h
+++ b/llvm/include/llvm/ADT/STLExtras.h
@@ -1030,14 +1030,15 @@ class concat_iterator
std::forward_iterator_tag, ValueT> {
using BaseT = typename concat_iterator::iterator_facade_base;
- static constexpr bool ReturnsByValue =
- !(std::is_reference_v<decltype(*std::declval<IterTs>())> && ...);
+ static constexpr bool ReturnsValueOrPointer =
+ !(std::is_reference_v<decltype(*std::declval<IterTs>())> && ...) ||
+ (std::is_pointer_v<IterTs> && ...);
using reference_type =
- typename std::conditional_t<ReturnsByValue, ValueT, ValueT &>;
+ typename std::conditional_t<ReturnsValueOrPointer, ValueT, ValueT &>;
using handle_type =
- typename std::conditional_t<ReturnsByValue, std::optional<ValueT>,
+ typename std::conditional_t<ReturnsValueOrPointer, std::optional<ValueT>,
ValueT *>;
/// We store both the current and end iterators for each concatenated
@@ -1088,7 +1089,7 @@ class concat_iterator
if (Begin == End)
return {};
- if constexpr (ReturnsByValue)
+ if constexpr (ReturnsValueOrPointer)
return *Begin;
else
return &*Begin;
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h
index 5715a37185b2b..afd30a24d0f8d 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h
@@ -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"
@@ -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;
@@ -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;
@@ -213,7 +211,11 @@ 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(); }
+ auto getChildren() const {
+ return llvm::concat<LVElement *const>(Types ? *Types : EmptyTypes,
+ Symbols ? *Symbols : EmptySymbols,
+ Scopes ? *Scopes : EmptyScopes);
+ }
void addElement(LVElement *Element);
void addElement(LVLine *Line);
@@ -222,7 +224,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.
diff --git a/llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp b/llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp
index 55880fab5e88e..70422555ea27e 100644
--- a/llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp
+++ b/llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp
@@ -107,11 +107,9 @@ 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{};
void LVScope::addElement(LVElement *Element) {
assert(Element && "Invalid element.");
@@ -175,7 +173,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.
@@ -202,7 +199,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.
@@ -229,7 +225,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.
@@ -277,15 +272,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;
}
@@ -356,8 +348,8 @@ void LVScope::updateLevel(LVScope *Parent, bool Moved) {
setLevel(Parent->getLevel() + 1);
// Update the children.
- if (Children)
- for (LVElement *Element : *Children)
+ if (auto Elements = getChildren(); Elements.begin() != Elements.end())
+ for (LVElement *Element : Elements)
Element->updateLevel(this, Moved);
// Update any lines.
@@ -374,8 +366,8 @@ void LVScope::resolve() {
LVElement::resolve();
// Resolve the children.
- if (Children)
- for (LVElement *Element : *Children) {
+ if (auto Elements = getChildren(); Elements.begin() != Elements.end())
+ for (LVElement *Element : Elements) {
if (getIsGlobalReference())
// If the scope is a global reference, mark all its children as well.
Element->setIsGlobalReference();
@@ -633,8 +625,9 @@ 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 (const auto Elements = getChildren();
+ Elements.begin() != Elements.end())
+ for (const LVElement *Element : Elements) {
if (Match && !Element->getHasPattern())
continue;
if (Error Err =
@@ -692,7 +685,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)
@@ -978,8 +970,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)
+ if (auto Elements = getChildren(); Elements.begin() != Elements.end())
+ for (LVElement *Element : Elements)
Element->report(Pass);
if (Lines)
@@ -1656,8 +1648,9 @@ 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)
+ if (const auto Elements = Scope->getChildren();
+ Elements.begin() != Elements.end())
+ for (LVElement *Element : Elements)
Element->print(OS);
}
}
diff --git a/llvm/unittests/ADT/STLExtrasTest.cpp b/llvm/unittests/ADT/STLExtrasTest.cpp
index 286cfa745fd14..98c13a33d35eb 100644
--- a/llvm/unittests/ADT/STLExtrasTest.cpp
+++ b/llvm/unittests/ADT/STLExtrasTest.cpp
@@ -398,6 +398,8 @@ struct some_struct {
std::string swap_val;
};
+struct derives_from_some_struct : some_struct {};
+
std::vector<int>::const_iterator begin(const some_struct &s) {
return s.data.begin();
}
@@ -532,6 +534,21 @@ TEST(STLExtrasTest, ConcatRangeADL) {
EXPECT_THAT(concat<const int>(S0, S1), ElementsAre(1, 2, 3, 4));
}
+TEST(STLExtrasTest, ConcatRangePtrToDerivedClass) {
+ auto S0 = std::make_unique<some_namespace::some_struct>();
+ auto S1 = std::make_unique<some_namespace::derives_from_some_struct>();
+ SmallVector<some_namespace::some_struct *> V0{S0.get()};
+ SmallVector<some_namespace::derives_from_some_struct *> V1{S1.get(),
+ S1.get()};
+
+ // Use concat over ranges of pointers to different (but related) types.
+ EXPECT_THAT(
+ concat<some_namespace::some_struct *>(V0, V1),
+ ElementsAre(S0.get(),
+ static_cast<some_namespace::some_struct *>(S1.get()),
+ static_cast<some_namespace::some_struct *>(S1.get())));
+}
+
TEST(STLExtrasTest, MakeFirstSecondRangeADL) {
// Make sure that we use the `begin`/`end` functions from `some_namespace`,
// using ADL.
diff --git a/llvm/unittests/DebugInfo/LogicalView/DWARFReaderTest.cpp b/llvm/unittests/DebugInfo/LogicalView/DWARFReaderTest.cpp
index 544c39a3c7b2e..ba6df7489b750 100644
--- a/llvm/unittests/DebugInfo/LogicalView/DWARFReaderTest.cpp
+++ b/llvm/unittests/DebugInfo/LogicalView/DWARFReaderTest.cpp
@@ -163,13 +163,12 @@ void checkUnspecifiedParameters(LVReader *Reader) {
LVPublicNames::const_iterator IterNames = PublicNames.cbegin();
LVScope *Function = (*IterNames).first;
EXPECT_EQ(Function->getName(), "foo_printf");
- const LVElements *Elements = Function->getChildren();
- ASSERT_NE(Elements, nullptr);
+ const auto Elements = Function->getChildren();
// foo_printf is a variadic function whose prototype is
// `int foo_printf(const char *, ...)`, where the '...' is represented by a
// DW_TAG_unspecified_parameters, i.e. we expect to find at least one child
// for which getIsUnspecified() returns true.
- EXPECT_TRUE(llvm::any_of(*Elements, [](const LVElement *elt) {
+ EXPECT_TRUE(llvm::any_of(Elements, [](const LVElement *elt) {
return elt->getIsSymbol() &&
static_cast<const LVSymbol *>(elt)->getIsUnspecified();
}));
@@ -183,10 +182,9 @@ void checkScopeModule(LVReader *Reader) {
EXPECT_EQ(Root->getFileFormatName(), "Mach-O 64-bit x86-64");
EXPECT_EQ(Root->getName(), DwarfClangModule);
- ASSERT_NE(CompileUnit->getChildren(), nullptr);
- LVElement *FirstChild = *(CompileUnit->getChildren()->begin());
- EXPECT_EQ(FirstChild->getIsScope(), 1);
- LVScopeModule *Module = static_cast<LVScopeModule *>(FirstChild);
+ ASSERT_NE(CompileUnit->getScopes(), nullptr);
+ LVElement *FirstScope = *(CompileUnit->getScopes()->begin());
+ LVScopeModule *Module = static_cast<LVScopeModule *>(FirstScope);
EXPECT_EQ(Module->getIsModule(), 1);
EXPECT_EQ(Module->getName(), "DebugModule");
}
|
kuhar
requested changes
Jun 18, 2025
|
This PR should yield further memory footprint improvements after #144399 is merged. As suggested in #69160, removing |
|
This required a minor change to |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove the
LVScope::Childrencontainer and usellvm::concat()instead to return a view over the types, symbols, and sub-scopes
contained in a given
LVScope.Fixes #69160.