[llvm] 554aea5 - [reland][Debuginfo][DWARF][NFC] Refactor DwarfStringPoolEntryRef.
Alexey Lapshin via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 1 10:09:15 PDT 2022
Author: Alexey Lapshin
Date: 2022-07-01T20:08:09+03:00
New Revision: 554aea52d79ebd9353911ecf2ffe14aca132f452
URL: https://github.com/llvm/llvm-project/commit/554aea52d79ebd9353911ecf2ffe14aca132f452
DIFF: https://github.com/llvm/llvm-project/commit/554aea52d79ebd9353911ecf2ffe14aca132f452.diff
LOG: [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
Added:
llvm/unittests/CodeGen/DwarfStringPoolEntryRefTest.cpp
Modified:
llvm/include/llvm/CodeGen/DwarfStringPoolEntry.h
llvm/unittests/CodeGen/CMakeLists.txt
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/DwarfStringPoolEntry.h b/llvm/include/llvm/CodeGen/DwarfStringPoolEntry.h
index 79c4c07a720a7..f19d321793e9c 100644
--- a/llvm/include/llvm/CodeGen/DwarfStringPoolEntry.h
+++ b/llvm/include/llvm/CodeGen/DwarfStringPoolEntry.h
@@ -9,7 +9,7 @@
#ifndef LLVM_CODEGEN_DWARFSTRINGPOOLENTRY_H
#define LLVM_CODEGEN_DWARFSTRINGPOOLENTRY_H
-#include "llvm/ADT/PointerIntPair.h"
+#include "llvm/ADT/PointerUnion.h"
#include "llvm/ADT/StringMap.h"
namespace llvm {
@@ -20,45 +20,91 @@ class MCSymbol;
struct DwarfStringPoolEntry {
static constexpr unsigned NotIndexed = -1;
- MCSymbol *Symbol;
- uint64_t Offset;
- unsigned Index;
+ MCSymbol *Symbol = nullptr;
+ uint64_t Offset = 0;
+ unsigned Index = 0;
bool isIndexed() const { return Index != NotIndexed; }
};
-/// String pool entry reference.
+/// DwarfStringPoolEntryRef: Dwarf string pool entry reference.
+///
+/// Dwarf string pool entry keeps string value and its data.
+/// There are two variants how data are represented:
+///
+/// 1. By value - StringMapEntry<DwarfStringPoolEntry>.
+/// 2. By pointer - StringMapEntry<DwarfStringPoolEntry *>.
+///
+/// The "By pointer" variant allows for reducing memory usage for the case
+/// when string pool entry does not have data: it keeps the null pointer
+/// and so no need to waste space for the full DwarfStringPoolEntry.
+/// It is recommended to use "By pointer" variant if not all entries
+/// of dwarf string pool have corresponding DwarfStringPoolEntry.
+
class DwarfStringPoolEntryRef {
- const StringMapEntry<DwarfStringPoolEntry> *MapEntry = nullptr;
+ /// Pointer type for "By value" string entry.
+ using ByValStringEntryPtr = const StringMapEntry<DwarfStringPoolEntry> *;
- const StringMapEntry<DwarfStringPoolEntry> *getMapEntry() const {
- return MapEntry;
- }
+ /// Pointer type for "By pointer" string entry.
+ using ByPtrStringEntryPtr = const StringMapEntry<DwarfStringPoolEntry *> *;
+
+ /// Pointer to the dwarf string pool Entry.
+ PointerUnion<ByValStringEntryPtr, ByPtrStringEntryPtr> MapEntry = nullptr;
public:
DwarfStringPoolEntryRef() = default;
+
+ /// ASSUMPTION: DwarfStringPoolEntryRef keeps pointer to \p Entry,
+ /// thus specified entry mustn`t be reallocated.
DwarfStringPoolEntryRef(const StringMapEntry<DwarfStringPoolEntry> &Entry)
: MapEntry(&Entry) {}
- explicit operator bool() const { return getMapEntry(); }
+ /// ASSUMPTION: DwarfStringPoolEntryRef keeps pointer to \p Entry,
+ /// thus specified entry mustn`t be reallocated.
+ DwarfStringPoolEntryRef(const StringMapEntry<DwarfStringPoolEntry *> &Entry)
+ : MapEntry(&Entry) {
+ assert(MapEntry.get<ByPtrStringEntryPtr>()->second != nullptr);
+ }
+
+ explicit operator bool() const { return !MapEntry.isNull(); }
+
+ /// \returns symbol for the dwarf string.
MCSymbol *getSymbol() const {
- assert(getMapEntry()->second.Symbol && "No symbol available!");
- return getMapEntry()->second.Symbol;
+ assert(getEntry().Symbol && "No symbol available!");
+ return getEntry().Symbol;
}
- uint64_t getOffset() const { return getMapEntry()->second.Offset; }
+
+ /// \returns offset for the dwarf string.
+ uint64_t getOffset() const { return getEntry().Offset; }
+
+ /// \returns index for the dwarf string.
unsigned getIndex() const {
- assert(getMapEntry()->getValue().isIndexed());
- return getMapEntry()->second.Index;
+ assert(getEntry().isIndexed() && "Index is not set!");
+ return getEntry().Index;
+ }
+
+ /// \returns string.
+ StringRef getString() const {
+ if (MapEntry.is<ByValStringEntryPtr>())
+ return MapEntry.get<ByValStringEntryPtr>()->first();
+
+ return MapEntry.get<ByPtrStringEntryPtr>()->first();
+ }
+
+ /// \returns the entire string pool entry for convenience.
+ const DwarfStringPoolEntry &getEntry() const {
+ if (MapEntry.is<ByValStringEntryPtr>())
+ return MapEntry.get<ByValStringEntryPtr>()->second;
+
+ return *MapEntry.get<ByPtrStringEntryPtr>()->second;
}
- StringRef getString() const { return getMapEntry()->first(); }
- /// Return the entire string pool entry for convenience.
- DwarfStringPoolEntry getEntry() const { return getMapEntry()->getValue(); }
bool operator==(const DwarfStringPoolEntryRef &X) const {
- return getMapEntry() == X.getMapEntry();
+ return MapEntry.getOpaqueValue() == X.MapEntry.getOpaqueValue();
}
+
bool operator!=(const DwarfStringPoolEntryRef &X) const {
- return getMapEntry() != X.getMapEntry();
+ return MapEntry.getOpaqueValue() != X.MapEntry.getOpaqueValue();
}
};
diff --git a/llvm/unittests/CodeGen/CMakeLists.txt b/llvm/unittests/CodeGen/CMakeLists.txt
index a85e3c0d9921b..def69d1d0e1e8 100644
--- a/llvm/unittests/CodeGen/CMakeLists.txt
+++ b/llvm/unittests/CodeGen/CMakeLists.txt
@@ -21,6 +21,7 @@ add_llvm_unittest(CodeGenTests
AsmPrinterDwarfTest.cpp
DIEHashTest.cpp
DIETest.cpp
+ DwarfStringPoolEntryRefTest.cpp
InstrRefLDVTest.cpp
LowLevelTypeTest.cpp
LexicalScopesTest.cpp
diff --git a/llvm/unittests/CodeGen/DwarfStringPoolEntryRefTest.cpp b/llvm/unittests/CodeGen/DwarfStringPoolEntryRefTest.cpp
new file mode 100644
index 0000000000000..623ac4e1a878e
--- /dev/null
+++ b/llvm/unittests/CodeGen/DwarfStringPoolEntryRefTest.cpp
@@ -0,0 +1,120 @@
+//===- llvm/unittest/CodeGen/DwarfStringPoolEntryRefTest.cpp --------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/CodeGen/DwarfStringPoolEntry.h"
+#include "llvm/Support/Allocator.h"
+#include "llvm/Testing/Support/Error.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include <string>
+
+using namespace llvm;
+
+TEST(DwarfStringPoolEntryRefTest, TestFullEntry) {
+ BumpPtrAllocator Allocator;
+ StringMapEntry<DwarfStringPoolEntry> *StringEntry1 =
+ StringMapEntry<DwarfStringPoolEntry>::Create(
+ "Key1", Allocator, DwarfStringPoolEntry{nullptr, 0, 0});
+
+ EXPECT_TRUE(StringEntry1->getKey() == "Key1");
+ EXPECT_TRUE(StringEntry1->second.Symbol == nullptr);
+ EXPECT_TRUE(StringEntry1->second.Offset == 0);
+ EXPECT_TRUE(StringEntry1->second.Index == 0);
+
+ DwarfStringPoolEntryRef Ref1(*StringEntry1);
+ EXPECT_TRUE(Ref1.getString() == "Key1");
+ EXPECT_TRUE(Ref1.getOffset() == 0);
+ EXPECT_TRUE(Ref1.getIndex() == 0);
+
+ DwarfStringPoolEntryRef Ref2(*StringEntry1);
+ EXPECT_TRUE(Ref2.getString() == "Key1");
+ EXPECT_TRUE(Ref2.getOffset() == 0);
+ EXPECT_TRUE(Ref2.getIndex() == 0);
+ EXPECT_TRUE(Ref1 == Ref2);
+ EXPECT_FALSE(Ref1 != Ref2);
+
+ StringMapEntry<DwarfStringPoolEntry> *StringEntry2 =
+ StringMapEntry<DwarfStringPoolEntry>::Create(
+ "Key2", Allocator, DwarfStringPoolEntry{nullptr, 0x1000, 1});
+ EXPECT_TRUE(StringEntry2->getKey() == "Key2");
+ EXPECT_TRUE(StringEntry2->second.Symbol == nullptr);
+ EXPECT_TRUE(StringEntry2->second.Offset == 0x1000);
+ EXPECT_TRUE(StringEntry2->second.Index == 1);
+
+ DwarfStringPoolEntryRef Ref3(*StringEntry2);
+ EXPECT_TRUE(Ref3.getString() == "Key2");
+ EXPECT_TRUE(Ref3.getOffset() == 0x1000);
+ EXPECT_TRUE(Ref3.getIndex() == 1);
+ EXPECT_TRUE(Ref1 != Ref3);
+}
+
+bool isEntryEqual(const DwarfStringPoolEntry &LHS,
+ const DwarfStringPoolEntry &RHS) {
+ return LHS.Symbol == RHS.Symbol && LHS.Offset == RHS.Offset &&
+ LHS.Index == RHS.Index;
+}
+
+TEST(DwarfStringPoolEntryRefTest, TestShortEntry) {
+ BumpPtrAllocator Allocator;
+ DwarfStringPoolEntry DwarfEntry1 = {nullptr, 0, 0};
+ StringMapEntry<DwarfStringPoolEntry *> *StringEntry1 =
+ StringMapEntry<DwarfStringPoolEntry *>::Create("Key1", Allocator,
+ &DwarfEntry1);
+
+ EXPECT_TRUE(StringEntry1->getKey() == "Key1");
+ EXPECT_TRUE(StringEntry1->second->Symbol == nullptr);
+ EXPECT_TRUE(StringEntry1->second->Offset == 0);
+ EXPECT_TRUE(StringEntry1->second->Index == 0);
+
+ DwarfStringPoolEntryRef Ref1(*StringEntry1);
+ EXPECT_TRUE(Ref1.getString() == "Key1");
+ EXPECT_TRUE(Ref1.getOffset() == 0);
+ EXPECT_TRUE(Ref1.getIndex() == 0);
+ EXPECT_TRUE(isEntryEqual(Ref1.getEntry(), DwarfEntry1));
+
+ DwarfStringPoolEntryRef Ref2(*StringEntry1);
+ EXPECT_TRUE(Ref2.getString() == "Key1");
+ EXPECT_TRUE(Ref2.getOffset() == 0);
+ EXPECT_TRUE(isEntryEqual(Ref2.getEntry(), DwarfEntry1));
+ EXPECT_TRUE(Ref1 == Ref2);
+ EXPECT_FALSE(Ref1 != Ref2);
+
+ DwarfStringPoolEntry DwarfEntry2 = {nullptr, 0x1000, 1};
+ StringMapEntry<DwarfStringPoolEntry *> *StringEntry2 =
+ StringMapEntry<DwarfStringPoolEntry *>::Create("Key2", Allocator,
+ &DwarfEntry2);
+ EXPECT_TRUE(StringEntry2->getKey() == "Key2");
+ EXPECT_TRUE(StringEntry2->second->Symbol == nullptr);
+ EXPECT_TRUE(StringEntry2->second->Offset == 0x1000);
+ EXPECT_TRUE(StringEntry2->second->Index == 1);
+
+ DwarfStringPoolEntryRef Ref3(*StringEntry2);
+ EXPECT_TRUE(Ref3.getString() == "Key2");
+ EXPECT_TRUE(Ref3.getOffset() == 0x1000);
+ EXPECT_TRUE(Ref3.getIndex() == 1);
+ EXPECT_TRUE(isEntryEqual(Ref3.getEntry(), DwarfEntry2));
+ EXPECT_TRUE(Ref1 != Ref3);
+}
+
+TEST(DwarfStringPoolEntryRefTest, CompareFullAndShort) {
+ BumpPtrAllocator Allocator;
+
+ DwarfStringPoolEntry DwarfEntry1 = {nullptr, 0, 0};
+ StringMapEntry<DwarfStringPoolEntry *> *StringEntry1 =
+ StringMapEntry<DwarfStringPoolEntry *>::Create("Key1", Allocator,
+ &DwarfEntry1);
+ DwarfStringPoolEntryRef Ref1(*StringEntry1);
+
+ StringMapEntry<DwarfStringPoolEntry> *StringEntry2 =
+ StringMapEntry<DwarfStringPoolEntry>::Create(
+ "Key1", Allocator, DwarfStringPoolEntry{nullptr, 0, 0});
+ DwarfStringPoolEntryRef Ref2(*StringEntry2);
+
+ EXPECT_FALSE(Ref1 == Ref2);
+}
More information about the llvm-commits
mailing list