[llvm] 72cd6b6 - Revert "[Debuginfo][DWARF][NFC] Refactor DwarfStringPoolEntryRef."

Vitaly Buka via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 29 17:53:47 PDT 2022


Author: Vitaly Buka
Date: 2022-06-29T17:53:42-07:00
New Revision: 72cd6b6c8356757c3f38bf91fb7a22f28bfb891e

URL: https://github.com/llvm/llvm-project/commit/72cd6b6c8356757c3f38bf91fb7a22f28bfb891e
DIFF: https://github.com/llvm/llvm-project/commit/72cd6b6c8356757c3f38bf91fb7a22f28bfb891e.diff

LOG: Revert "[Debuginfo][DWARF][NFC] Refactor DwarfStringPoolEntryRef."

Breaks msan bot, see D126883

This reverts commit 77df3be0dee415713cf5c79543f00532674f428b.

Added: 
    

Modified: 
    llvm/include/llvm/CodeGen/DwarfStringPoolEntry.h
    llvm/unittests/CodeGen/CMakeLists.txt

Removed: 
    llvm/unittests/CodeGen/DwarfStringPoolEntryRefTest.cpp


################################################################################
diff  --git a/llvm/include/llvm/CodeGen/DwarfStringPoolEntry.h b/llvm/include/llvm/CodeGen/DwarfStringPoolEntry.h
index f19d321793e9c..79c4c07a720a7 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/PointerUnion.h"
+#include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/StringMap.h"
 
 namespace llvm {
@@ -20,91 +20,45 @@ class MCSymbol;
 struct DwarfStringPoolEntry {
   static constexpr unsigned NotIndexed = -1;
 
-  MCSymbol *Symbol = nullptr;
-  uint64_t Offset = 0;
-  unsigned Index = 0;
+  MCSymbol *Symbol;
+  uint64_t Offset;
+  unsigned Index;
 
   bool isIndexed() const { return Index != NotIndexed; }
 };
 
-/// 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.
-
+/// String pool entry reference.
 class DwarfStringPoolEntryRef {
-  /// Pointer type for "By value" string entry.
-  using ByValStringEntryPtr = const StringMapEntry<DwarfStringPoolEntry> *;
-
-  /// Pointer type for "By pointer" string entry.
-  using ByPtrStringEntryPtr = const StringMapEntry<DwarfStringPoolEntry *> *;
+  const StringMapEntry<DwarfStringPoolEntry> *MapEntry = nullptr;
 
-  /// Pointer to the dwarf string pool Entry.
-  PointerUnion<ByValStringEntryPtr, ByPtrStringEntryPtr> MapEntry = nullptr;
+  const StringMapEntry<DwarfStringPoolEntry> *getMapEntry() const {
+    return MapEntry;
+  }
 
 public:
   DwarfStringPoolEntryRef() = default;
-
-  /// ASSUMPTION: DwarfStringPoolEntryRef keeps pointer to \p Entry,
-  /// thus specified entry mustn`t be reallocated.
   DwarfStringPoolEntryRef(const StringMapEntry<DwarfStringPoolEntry> &Entry)
       : MapEntry(&Entry) {}
 
-  /// 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.
+  explicit operator bool() const { return getMapEntry(); }
   MCSymbol *getSymbol() const {
-    assert(getEntry().Symbol && "No symbol available!");
-    return getEntry().Symbol;
+    assert(getMapEntry()->second.Symbol && "No symbol available!");
+    return getMapEntry()->second.Symbol;
   }
-
-  /// \returns offset for the dwarf string.
-  uint64_t getOffset() const { return getEntry().Offset; }
-
-  /// \returns index for the dwarf string.
+  uint64_t getOffset() const { return getMapEntry()->second.Offset; }
   unsigned getIndex() const {
-    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;
+    assert(getMapEntry()->getValue().isIndexed());
+    return getMapEntry()->second.Index;
   }
+  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 MapEntry.getOpaqueValue() == X.MapEntry.getOpaqueValue();
+    return getMapEntry() == X.getMapEntry();
   }
-
   bool operator!=(const DwarfStringPoolEntryRef &X) const {
-    return MapEntry.getOpaqueValue() != X.MapEntry.getOpaqueValue();
+    return getMapEntry() != X.getMapEntry();
   }
 };
 

diff  --git a/llvm/unittests/CodeGen/CMakeLists.txt b/llvm/unittests/CodeGen/CMakeLists.txt
index def69d1d0e1e8..a85e3c0d9921b 100644
--- a/llvm/unittests/CodeGen/CMakeLists.txt
+++ b/llvm/unittests/CodeGen/CMakeLists.txt
@@ -21,7 +21,6 @@ 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
deleted file mode 100644
index 2869d875f4551..0000000000000
--- a/llvm/unittests/CodeGen/DwarfStringPoolEntryRefTest.cpp
+++ /dev/null
@@ -1,117 +0,0 @@
-//===- 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);
-}
-
-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(memcmp(&Ref1.getEntry(), &DwarfEntry1,
-                     sizeof(DwarfStringPoolEntry)) == 0);
-
-  DwarfStringPoolEntryRef Ref2(*StringEntry1);
-  EXPECT_TRUE(Ref2.getString() == "Key1");
-  EXPECT_TRUE(Ref2.getOffset() == 0);
-  EXPECT_TRUE(memcmp(&Ref2.getEntry(), &DwarfEntry1,
-                     sizeof(DwarfStringPoolEntry)) == 0);
-  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(memcmp(&Ref3.getEntry(), &DwarfEntry2,
-                     sizeof(DwarfStringPoolEntry)) == 0);
-  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