[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

diff --git a/llvm/include/llvm/CodeGen/DwarfStringPoolEntry.h b/llvm/include/llvm/CodeGen/DwarfStringPoolEntry.h
index 79c4c07..f19d321 100644 (file)
--- 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 a85e3c0..def69d1 100644 (file)
--- 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 (file)
index 0000000..623ac4e
--- /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);
+}