Allocate space for MCSymbol::Name only if required.

Similarly to User which allocates a number of Use's prior to the this pointer,
allocate space for the Name* for MCSymbol only when we need a name.

Given that an MCSymbol is 48-bytes on 64-bit systems, this saves a decent % of space.

Given the verify_uselistorder test case with debug info and llc, 50k symbols have names
out of 700k so this optimises for the common case of temporary unnamed symbols.

Reviewed by David Blaikie.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@239423 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/MC/MCSymbol.h b/include/llvm/MC/MCSymbol.h
index 078f3d77e55cd04ca19074e164bf68610677dee3..36ff8ba727060f775d09d483334f214d022d9075 100644 (file)
--- a/include/llvm/MC/MCSymbol.h
+++ b/include/llvm/MC/MCSymbol.h
@@ -52,10 +52,6 @@ protected:
   // FIXME: Use a PointerInt wrapper for this?
   static MCSection *AbsolutePseudoSection;
 
-  /// Name - The name of the symbol.  The referred-to string data is actually
-  /// held by the StringMap that lives in MCContext.
-  const StringMapEntry<bool> *Name;
-
   /// If a symbol has a Fragment, the section is implied, so we only need
   /// one pointer.
   /// FIXME: We might be able to simplify this by having the asm streamer create
@@ -91,6 +87,11 @@ protected:
   /// This symbol is private extern.
   mutable unsigned IsPrivateExtern : 1;
 
+  /// True if this symbol is named.
+  /// A named symbol will have a pointer to the name allocated in the bytes
+  /// immediately prior to the MCSymbol.
+  unsigned HasName : 1;
+
   /// LLVM RTTI discriminator. This is actually a SymbolKind enumerator, but is
   /// unsigned to avoid sign extension and achieve better bitpacking with MSVC.
   unsigned Kind : 2;
@@ -118,15 +119,34 @@ protected:
 protected: // MCContext creates and uniques these.
   friend class MCExpr;
   friend class MCContext;
-  MCSymbol(SymbolKind Kind, const StringMapEntry<bool> *Name, bool isTemporary)
-      : Name(Name), Value(nullptr), IsTemporary(isTemporary),
+
+  typedef const StringMapEntry<bool> NameEntryTy;
+  MCSymbol(SymbolKind Kind, NameEntryTy *Name, bool isTemporary)
+      : Value(nullptr), IsTemporary(isTemporary),
         IsRedefinable(false), IsUsed(false), IsRegistered(false),
-        IsExternal(false), IsPrivateExtern(false),
+        IsExternal(false), IsPrivateExtern(false), HasName(!!Name),
         Kind(Kind) {
     Offset = 0;
+    if (Name)
+      getNameEntryPtr() = Name;
   }
 
+  // Provide custom new/delete as we will only allocate space for a name
+  // if we need one.
+  void *operator new(size_t s, NameEntryTy *Name, MCContext &Ctx);
+
 private:
+
+  void operator delete(void *);
+  /// \brief Placement delete - required by std, but never called.
+  void operator delete(void*, unsigned) {
+    llvm_unreachable("Constructor throws?");
+  }
+  /// \brief Placement delete - required by std, but never called.
+  void operator delete(void*, unsigned, bool) {
+    llvm_unreachable("Constructor throws?");
+  }
+
   MCSymbol(const MCSymbol &) = delete;
   void operator=(const MCSymbol &) = delete;
   MCSection *getSectionPtr() const {
@@ -139,9 +159,26 @@ private:
     return Section = Value->findAssociatedSection();
   }
 
+  /// \brief Get a reference to the name field.  Requires that we have a name
+  NameEntryTy *&getNameEntryPtr() {
+    assert(HasName && "Name is required");
+    NameEntryTy **Name = reinterpret_cast<NameEntryTy **>(this);
+    return *(Name - 1);
+  }
+  NameEntryTy *const &getNameEntryPtr() const {
+    assert(HasName && "Name is required");
+    NameEntryTy *const *Name = reinterpret_cast<NameEntryTy *const *>(this);
+    return *(Name - 1);
+  }
+
 public:
   /// getName - Get the symbol name.
-  StringRef getName() const { return Name ? Name->first() : ""; }
+  StringRef getName() const {
+    if (!HasName)
+      return StringRef();
+
+    return getNameEntryPtr()->first();
+  }
 
   bool isRegistered() const { return IsRegistered; }
   void setIsRegistered(bool Value) const { IsRegistered = Value; }

diff --git a/lib/MC/MCContext.cpp b/lib/MC/MCContext.cpp
index 1e52eedaf188dde34e12ee7bf5ffaa66e712dbc6..785f9ac545691e551138ac522de459eb3e8dd417 100644 (file)
--- a/lib/MC/MCContext.cpp
+++ b/lib/MC/MCContext.cpp
@@ -135,7 +135,7 @@ MCSymbolELF *MCContext::getOrCreateSectionSymbol(const MCSectionELF &Section) {
   }
 
   auto NameIter = UsedNames.insert(std::make_pair(Name, true)).first;
-  Sym = new (*this) MCSymbolELF(&*NameIter, /*isTemporary*/ false);
+  Sym = new (&*NameIter, *this) MCSymbolELF(&*NameIter, /*isTemporary*/ false);
 
   if (!OldSym)
     OldSym = Sym;
@@ -164,14 +164,15 @@ MCSymbol *MCContext::createSymbolImpl(const StringMapEntry<bool> *Name,
   if (MOFI) {
     switch (MOFI->getObjectFileType()) {
     case MCObjectFileInfo::IsCOFF:
-      return new (*this) MCSymbolCOFF(Name, IsTemporary);
+      return new (Name, *this) MCSymbolCOFF(Name, IsTemporary);
     case MCObjectFileInfo::IsELF:
-      return new (*this) MCSymbolELF(Name, IsTemporary);
+      return new (Name, *this) MCSymbolELF(Name, IsTemporary);
     case MCObjectFileInfo::IsMachO:
-      return new (*this) MCSymbolMachO(Name, IsTemporary);
+      return new (Name, *this) MCSymbolMachO(Name, IsTemporary);
     }
   }
-  return new (*this) MCSymbol(MCSymbol::SymbolKindUnset, Name, IsTemporary);
+  return new (Name, *this) MCSymbol(MCSymbol::SymbolKindUnset, Name,
+                                    IsTemporary);
 }
 
 MCSymbol *MCContext::createSymbol(StringRef Name, bool AlwaysAddSuffix,

diff --git a/lib/MC/MCSymbol.cpp b/lib/MC/MCSymbol.cpp
index 8d07b7605ceaa9a6b00ac39e7d5e9ca197ae9296..7eba5e0317c6285bb9b77492742267f35c7e297e 100644 (file)
--- a/lib/MC/MCSymbol.cpp
+++ b/lib/MC/MCSymbol.cpp
@@ -9,6 +9,7 @@
 
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/MCAsmInfo.h"
+#include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
@@ -18,6 +19,20 @@ using namespace llvm;
 // Sentinel value for the absolute pseudo section.
 MCSection *MCSymbol::AbsolutePseudoSection = reinterpret_cast<MCSection *>(1);
 
+void *MCSymbol::operator new(size_t s, NameEntryTy *Name, MCContext &Ctx) {
+  size_t Size = s + (Name ? sizeof(Name) : 0);
+
+  // For safety, ensure that the alignment of a pointer is enough for an
+  // MCSymbol.  This also ensures we don't need padding between the name and
+  // symbol.
+  assert(alignOf<MCSymbol>() <= alignof(NameEntryTy *) &&
+         "Bad alignment of MCSymbol");
+  void *Storage = Ctx.allocate(Size, alignof(NameEntryTy *));
+  NameEntryTy **Start = static_cast<NameEntryTy**>(Storage);
+  NameEntryTy **End = Start + (Name ? 1 : 0);
+  return End;
+}
+
 void MCSymbol::setVariableValue(const MCExpr *Value) {
   assert(!IsUsed && "Cannot set a variable that has already been used.");
   assert(Value && "Invalid variable value!");