From 79e04c584474f81e3faef29e33b7195bc7e4d1f6 Mon Sep 17 00:00:00 2001 From: Pete Cooper Date: Tue, 9 Jun 2015 18:36:13 +0000 Subject: [PATCH] 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 --- include/llvm/MC/MCSymbol.h | 53 ++++++++++++++++++++++++++++++++------ lib/MC/MCContext.cpp | 11 ++++---- lib/MC/MCSymbol.cpp | 15 +++++++++++ 3 files changed, 66 insertions(+), 13 deletions(-) diff --git a/include/llvm/MC/MCSymbol.h b/include/llvm/MC/MCSymbol.h index 078f3d77e55..36ff8ba7270 100644 --- 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 *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 *Name, bool isTemporary) - : Name(Name), Value(nullptr), IsTemporary(isTemporary), + + typedef const StringMapEntry 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(this); + return *(Name - 1); + } + NameEntryTy *const &getNameEntryPtr() const { + assert(HasName && "Name is required"); + NameEntryTy *const *Name = reinterpret_cast(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 1e52eedaf18..785f9ac5456 100644 --- 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 *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 8d07b7605ce..7eba5e0317c 100644 --- 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(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() <= alignof(NameEntryTy *) && + "Bad alignment of MCSymbol"); + void *Storage = Ctx.allocate(Size, alignof(NameEntryTy *)); + NameEntryTy **Start = static_cast(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!"); -- 2.34.1