From 554aea52d79ebd9353911ecf2ffe14aca132f452 Mon Sep 17 00:00:00 2001 From: Alexey Lapshin Date: Tue, 28 Jun 2022 19:52:12 +0300 Subject: [PATCH] [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 --- .../llvm/CodeGen/DwarfStringPoolEntry.h | 86 ++++++++++--- llvm/unittests/CodeGen/CMakeLists.txt | 1 + .../CodeGen/DwarfStringPoolEntryRefTest.cpp | 120 ++++++++++++++++++ 3 files changed, 187 insertions(+), 20 deletions(-) create mode 100644 llvm/unittests/CodeGen/DwarfStringPoolEntryRefTest.cpp 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. +/// 2. By pointer - StringMapEntry. +/// +/// 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 *MapEntry = nullptr; + /// Pointer type for "By value" string entry. + using ByValStringEntryPtr = const StringMapEntry *; - const StringMapEntry *getMapEntry() const { - return MapEntry; - } + /// Pointer type for "By pointer" string entry. + using ByPtrStringEntryPtr = const StringMapEntry *; + + /// Pointer to the dwarf string pool Entry. + PointerUnion MapEntry = nullptr; public: DwarfStringPoolEntryRef() = default; + + /// ASSUMPTION: DwarfStringPoolEntryRef keeps pointer to \p Entry, + /// thus specified entry mustn`t be reallocated. DwarfStringPoolEntryRef(const StringMapEntry &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 &Entry) + : MapEntry(&Entry) { + assert(MapEntry.get()->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()) + return MapEntry.get()->first(); + + return MapEntry.get()->first(); + } + + /// \returns the entire string pool entry for convenience. + const DwarfStringPoolEntry &getEntry() const { + if (MapEntry.is()) + return MapEntry.get()->second; + + return *MapEntry.get()->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 + +using namespace llvm; + +TEST(DwarfStringPoolEntryRefTest, TestFullEntry) { + BumpPtrAllocator Allocator; + StringMapEntry *StringEntry1 = + StringMapEntry::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 *StringEntry2 = + StringMapEntry::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 *StringEntry1 = + StringMapEntry::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 *StringEntry2 = + StringMapEntry::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 *StringEntry1 = + StringMapEntry::Create("Key1", Allocator, + &DwarfEntry1); + DwarfStringPoolEntryRef Ref1(*StringEntry1); + + StringMapEntry *StringEntry2 = + StringMapEntry::Create( + "Key1", Allocator, DwarfStringPoolEntry{nullptr, 0, 0}); + DwarfStringPoolEntryRef Ref2(*StringEntry2); + + EXPECT_FALSE(Ref1 == Ref2); +}