Skip to content

Commit 554aea5

Browse files
avl-llvmavl-llvm
committed
[reland][Debuginfo][DWARF][NFC] Refactor DwarfStringPoolEntryRef.
This review is extracted from D96035. This patch adds possibility to keep not only DwarfStringPoolEntry, but also pointer to it. The DwarfStringPoolEntryRef keeps reference to the string map entry. String map keeps string data and corresponding DwarfStringPoolEntry info. Not all string map entries may be included into the result, and then not all string entries should have DwarfStringPoolEntry info. Currently StringMap keeps DwarfStringPoolEntry for all entries. It leads to extra memory usage. This patch allows to keep DwarfStringPoolEntry info only for entries which really need it. [reland] : make msan happy. Reviewed By: JDevlieghere Differential Revision: https://reviews.llvm.org/D126883
1 parent b5e49cd commit 554aea5

File tree

3 files changed

+187
-20
lines changed

3 files changed

+187
-20
lines changed

‎llvm/include/llvm/CodeGen/DwarfStringPoolEntry.h

Lines changed: 66 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
#ifndef LLVM_CODEGEN_DWARFSTRINGPOOLENTRY_H
1010
#define LLVM_CODEGEN_DWARFSTRINGPOOLENTRY_H
1111

12-
#include "llvm/ADT/PointerIntPair.h"
12+
#include "llvm/ADT/PointerUnion.h"
1313
#include "llvm/ADT/StringMap.h"
1414

1515
namespace llvm {
@@ -20,45 +20,91 @@ class MCSymbol;
2020
struct DwarfStringPoolEntry {
2121
static constexpr unsigned NotIndexed = -1;
2222

23-
MCSymbol *Symbol;
24-
uint64_t Offset;
25-
unsigned Index;
23+
MCSymbol *Symbol = nullptr;
24+
uint64_t Offset = 0;
25+
unsigned Index = 0;
2626

2727
bool isIndexed() const { return Index != NotIndexed; }
2828
};
2929

30-
/// String pool entry reference.
30+
/// DwarfStringPoolEntryRef: Dwarf string pool entry reference.
31+
///
32+
/// Dwarf string pool entry keeps string value and its data.
33+
/// There are two variants how data are represented:
34+
///
35+
/// 1. By value - StringMapEntry<DwarfStringPoolEntry>.
36+
/// 2. By pointer - StringMapEntry<DwarfStringPoolEntry *>.
37+
///
38+
/// The "By pointer" variant allows for reducing memory usage for the case
39+
/// when string pool entry does not have data: it keeps the null pointer
40+
/// and so no need to waste space for the full DwarfStringPoolEntry.
41+
/// It is recommended to use "By pointer" variant if not all entries
42+
/// of dwarf string pool have corresponding DwarfStringPoolEntry.
43+
3144
class DwarfStringPoolEntryRef {
32-
const StringMapEntry<DwarfStringPoolEntry> *MapEntry = nullptr;
45+
/// Pointer type for "By value" string entry.
46+
using ByValStringEntryPtr = const StringMapEntry<DwarfStringPoolEntry> *;
3347

34-
const StringMapEntry<DwarfStringPoolEntry> *getMapEntry() const {
35-
return MapEntry;
36-
}
48+
/// Pointer type for "By pointer" string entry.
49+
using ByPtrStringEntryPtr = const StringMapEntry<DwarfStringPoolEntry *> *;
50+
51+
/// Pointer to the dwarf string pool Entry.
52+
PointerUnion<ByValStringEntryPtr, ByPtrStringEntryPtr> MapEntry = nullptr;
3753

3854
public:
3955
DwarfStringPoolEntryRef() = default;
56+
57+
/// ASSUMPTION: DwarfStringPoolEntryRef keeps pointer to \p Entry,
58+
/// thus specified entry mustn`t be reallocated.
4059
DwarfStringPoolEntryRef(const StringMapEntry<DwarfStringPoolEntry> &Entry)
4160
: MapEntry(&Entry) {}
4261

43-
explicit operator bool() const { return getMapEntry(); }
62+
/// ASSUMPTION: DwarfStringPoolEntryRef keeps pointer to \p Entry,
63+
/// thus specified entry mustn`t be reallocated.
64+
DwarfStringPoolEntryRef(const StringMapEntry<DwarfStringPoolEntry *> &Entry)
65+
: MapEntry(&Entry) {
66+
assert(MapEntry.get<ByPtrStringEntryPtr>()->second != nullptr);
67+
}
68+
69+
explicit operator bool() const { return !MapEntry.isNull(); }
70+
71+
/// \returns symbol for the dwarf string.
4472
MCSymbol *getSymbol() const {
45-
assert(getMapEntry()->second.Symbol && "No symbol available!");
46-
return getMapEntry()->second.Symbol;
73+
assert(getEntry().Symbol && "No symbol available!");
74+
return getEntry().Symbol;
4775
}
48-
uint64_t getOffset() const { return getMapEntry()->second.Offset; }
76+
77+
/// \returns offset for the dwarf string.
78+
uint64_t getOffset() const { return getEntry().Offset; }
79+
80+
/// \returns index for the dwarf string.
4981
unsigned getIndex() const {
50-
assert(getMapEntry()->getValue().isIndexed());
51-
return getMapEntry()->second.Index;
82+
assert(getEntry().isIndexed() && "Index is not set!");
83+
return getEntry().Index;
84+
}
85+
86+
/// \returns string.
87+
StringRef getString() const {
88+
if (MapEntry.is<ByValStringEntryPtr>())
89+
return MapEntry.get<ByValStringEntryPtr>()->first();
90+
91+
return MapEntry.get<ByPtrStringEntryPtr>()->first();
92+
}
93+
94+
/// \returns the entire string pool entry for convenience.
95+
const DwarfStringPoolEntry &getEntry() const {
96+
if (MapEntry.is<ByValStringEntryPtr>())
97+
return MapEntry.get<ByValStringEntryPtr>()->second;
98+
99+
return *MapEntry.get<ByPtrStringEntryPtr>()->second;
52100
}
53-
StringRef getString() const { return getMapEntry()->first(); }
54-
/// Return the entire string pool entry for convenience.
55-
DwarfStringPoolEntry getEntry() const { return getMapEntry()->getValue(); }
56101

57102
bool operator==(const DwarfStringPoolEntryRef &X) const {
58-
return getMapEntry() == X.getMapEntry();
103+
return MapEntry.getOpaqueValue() == X.MapEntry.getOpaqueValue();
59104
}
105+
60106
bool operator!=(const DwarfStringPoolEntryRef &X) const {
61-
return getMapEntry() != X.getMapEntry();
107+
return MapEntry.getOpaqueValue() != X.MapEntry.getOpaqueValue();
62108
}
63109
};
64110

‎llvm/unittests/CodeGen/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ add_llvm_unittest(CodeGenTests
2121
AsmPrinterDwarfTest.cpp
2222
DIEHashTest.cpp
2323
DIETest.cpp
24+
DwarfStringPoolEntryRefTest.cpp
2425
InstrRefLDVTest.cpp
2526
LowLevelTypeTest.cpp
2627
LexicalScopesTest.cpp

‎llvm/unittests/CodeGen/DwarfStringPoolEntryRefTest.cpp

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
//===- llvm/unittest/CodeGen/DwarfStringPoolEntryRefTest.cpp --------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "llvm/CodeGen/DwarfStringPoolEntry.h"
10+
#include "llvm/Support/Allocator.h"
11+
#include "llvm/Testing/Support/Error.h"
12+
13+
#include "gmock/gmock.h"
14+
#include "gtest/gtest.h"
15+
#include <string>
16+
17+
using namespace llvm;
18+
19+
TEST(DwarfStringPoolEntryRefTest, TestFullEntry) {
20+
BumpPtrAllocator Allocator;
21+
StringMapEntry<DwarfStringPoolEntry> *StringEntry1 =
22+
StringMapEntry<DwarfStringPoolEntry>::Create(
23+
"Key1", Allocator, DwarfStringPoolEntry{nullptr, 0, 0});
24+
25+
EXPECT_TRUE(StringEntry1->getKey() == "Key1");
26+
EXPECT_TRUE(StringEntry1->second.Symbol == nullptr);
27+
EXPECT_TRUE(StringEntry1->second.Offset == 0);
28+
EXPECT_TRUE(StringEntry1->second.Index == 0);
29+
30+
DwarfStringPoolEntryRef Ref1(*StringEntry1);
31+
EXPECT_TRUE(Ref1.getString() == "Key1");
32+
EXPECT_TRUE(Ref1.getOffset() == 0);
33+
EXPECT_TRUE(Ref1.getIndex() == 0);
34+
35+
DwarfStringPoolEntryRef Ref2(*StringEntry1);
36+
EXPECT_TRUE(Ref2.getString() == "Key1");
37+
EXPECT_TRUE(Ref2.getOffset() == 0);
38+
EXPECT_TRUE(Ref2.getIndex() == 0);
39+
EXPECT_TRUE(Ref1 == Ref2);
40+
EXPECT_FALSE(Ref1 != Ref2);
41+
42+
StringMapEntry<DwarfStringPoolEntry> *StringEntry2 =
43+
StringMapEntry<DwarfStringPoolEntry>::Create(
44+
"Key2", Allocator, DwarfStringPoolEntry{nullptr, 0x1000, 1});
45+
EXPECT_TRUE(StringEntry2->getKey() == "Key2");
46+
EXPECT_TRUE(StringEntry2->second.Symbol == nullptr);
47+
EXPECT_TRUE(StringEntry2->second.Offset == 0x1000);
48+
EXPECT_TRUE(StringEntry2->second.Index == 1);
49+
50+
DwarfStringPoolEntryRef Ref3(*StringEntry2);
51+
EXPECT_TRUE(Ref3.getString() == "Key2");
52+
EXPECT_TRUE(Ref3.getOffset() == 0x1000);
53+
EXPECT_TRUE(Ref3.getIndex() == 1);
54+
EXPECT_TRUE(Ref1 != Ref3);
55+
}
56+
57+
bool isEntryEqual(const DwarfStringPoolEntry &LHS,
58+
const DwarfStringPoolEntry &RHS) {
59+
return LHS.Symbol == RHS.Symbol && LHS.Offset == RHS.Offset &&
60+
LHS.Index == RHS.Index;
61+
}
62+
63+
TEST(DwarfStringPoolEntryRefTest, TestShortEntry) {
64+
BumpPtrAllocator Allocator;
65+
DwarfStringPoolEntry DwarfEntry1 = {nullptr, 0, 0};
66+
StringMapEntry<DwarfStringPoolEntry *> *StringEntry1 =
67+
StringMapEntry<DwarfStringPoolEntry *>::Create("Key1", Allocator,
68+
&DwarfEntry1);
69+
70+
EXPECT_TRUE(StringEntry1->getKey() == "Key1");
71+
EXPECT_TRUE(StringEntry1->second->Symbol == nullptr);
72+
EXPECT_TRUE(StringEntry1->second->Offset == 0);
73+
EXPECT_TRUE(StringEntry1->second->Index == 0);
74+
75+
DwarfStringPoolEntryRef Ref1(*StringEntry1);
76+
EXPECT_TRUE(Ref1.getString() == "Key1");
77+
EXPECT_TRUE(Ref1.getOffset() == 0);
78+
EXPECT_TRUE(Ref1.getIndex() == 0);
79+
EXPECT_TRUE(isEntryEqual(Ref1.getEntry(), DwarfEntry1));
80+
81+
DwarfStringPoolEntryRef Ref2(*StringEntry1);
82+
EXPECT_TRUE(Ref2.getString() == "Key1");
83+
EXPECT_TRUE(Ref2.getOffset() == 0);
84+
EXPECT_TRUE(isEntryEqual(Ref2.getEntry(), DwarfEntry1));
85+
EXPECT_TRUE(Ref1 == Ref2);
86+
EXPECT_FALSE(Ref1 != Ref2);
87+
88+
DwarfStringPoolEntry DwarfEntry2 = {nullptr, 0x1000, 1};
89+
StringMapEntry<DwarfStringPoolEntry *> *StringEntry2 =
90+
StringMapEntry<DwarfStringPoolEntry *>::Create("Key2", Allocator,
91+
&DwarfEntry2);
92+
EXPECT_TRUE(StringEntry2->getKey() == "Key2");
93+
EXPECT_TRUE(StringEntry2->second->Symbol == nullptr);
94+
EXPECT_TRUE(StringEntry2->second->Offset == 0x1000);
95+
EXPECT_TRUE(StringEntry2->second->Index == 1);
96+
97+
DwarfStringPoolEntryRef Ref3(*StringEntry2);
98+
EXPECT_TRUE(Ref3.getString() == "Key2");
99+
EXPECT_TRUE(Ref3.getOffset() == 0x1000);
100+
EXPECT_TRUE(Ref3.getIndex() == 1);
101+
EXPECT_TRUE(isEntryEqual(Ref3.getEntry(), DwarfEntry2));
102+
EXPECT_TRUE(Ref1 != Ref3);
103+
}
104+
105+
TEST(DwarfStringPoolEntryRefTest, CompareFullAndShort) {
106+
BumpPtrAllocator Allocator;
107+
108+
DwarfStringPoolEntry DwarfEntry1 = {nullptr, 0, 0};
109+
StringMapEntry<DwarfStringPoolEntry *> *StringEntry1 =
110+
StringMapEntry<DwarfStringPoolEntry *>::Create("Key1", Allocator,
111+
&DwarfEntry1);
112+
DwarfStringPoolEntryRef Ref1(*StringEntry1);
113+
114+
StringMapEntry<DwarfStringPoolEntry> *StringEntry2 =
115+
StringMapEntry<DwarfStringPoolEntry>::Create(
116+
"Key1", Allocator, DwarfStringPoolEntry{nullptr, 0, 0});
117+
DwarfStringPoolEntryRef Ref2(*StringEntry2);
118+
119+
EXPECT_FALSE(Ref1 == Ref2);
120+
}

0 commit comments

Comments
 (0)