From 7349ab9385070b80b282232485724b8e3d759654 Mon Sep 17 00:00:00 2001 From: Owen Anderson Date: Mon, 1 Jun 2015 22:24:01 +0000 Subject: [PATCH] Move the name pointer out of Value into a map that lives on the LLVMContext. Production builds of clang do not set names on most Value's, so this is wasted space on almost all subclasses of Value. This reduces the size of all Value subclasses by 8 bytes on 64 bit hosts. The one tricky part of this change is averting compile time regression by keeping Value::hasName() fast. This required stealing bits out of NumOperands. With this change, peak memory usage on verify-uselistorder-nodbg.lto.bc is decreased by approximately 2.3% (~3MB absolute on my machine). llvm-svn: 238791 --- llvm/include/llvm/IR/Value.h | 16 +++++++++------- llvm/lib/IR/LLVMContextImpl.h | 2 ++ llvm/lib/IR/Metadata.cpp | 14 +++++++------- llvm/lib/IR/Value.cpp | 35 ++++++++++++++++++++++++++++++++--- 4 files changed, 50 insertions(+), 17 deletions(-) diff --git a/llvm/include/llvm/IR/Value.h b/llvm/include/llvm/IR/Value.h index 3bf2943..19a1d6c 100644 --- a/llvm/include/llvm/IR/Value.h +++ b/llvm/include/llvm/IR/Value.h @@ -69,9 +69,8 @@ class Value { Type *VTy; Use *UseList; - friend class ValueAsMetadata; // Allow access to NameAndIsUsedByMD. + friend class ValueAsMetadata; // Allow access to IsUsedByMD. friend class ValueHandleBase; - PointerIntPair NameAndIsUsedByMD; const unsigned char SubclassID; // Subclass identifier (for isa/dyn_cast) unsigned char HasValueHandle : 1; // Has a ValueHandle pointing to this? @@ -101,7 +100,10 @@ protected: /// This is stored here to save space in User on 64-bit hosts. Since most /// instances of Value have operands, 32-bit hosts aren't significantly /// affected. - unsigned NumOperands; + unsigned NumOperands : 30; + + bool IsUsedByMD : 1; + bool HasName : 1; private: template // UseT == 'Use' or 'const Use' @@ -210,9 +212,9 @@ public: LLVMContext &getContext() const; // \brief All values can potentially be named. - bool hasName() const { return getValueName() != nullptr; } - ValueName *getValueName() const { return NameAndIsUsedByMD.getPointer(); } - void setValueName(ValueName *VN) { NameAndIsUsedByMD.setPointer(VN); } + bool hasName() const { return HasName; } + ValueName *getValueName() const; + void setValueName(ValueName *VN); private: void destroyValueName(); @@ -394,7 +396,7 @@ public: bool hasValueHandle() const { return HasValueHandle; } /// \brief Return true if there is metadata referencing this value. - bool isUsedByMetadata() const { return NameAndIsUsedByMD.getInt(); } + bool isUsedByMetadata() const { return IsUsedByMD; } /// \brief Strip off pointer casts, all-zero GEPs, and aliases. /// diff --git a/llvm/lib/IR/LLVMContextImpl.h b/llvm/lib/IR/LLVMContextImpl.h index f81db60..3a57336 100644 --- a/llvm/lib/IR/LLVMContextImpl.h +++ b/llvm/lib/IR/LLVMContextImpl.h @@ -922,6 +922,8 @@ public: DenseMap ValuesAsMetadata; DenseMap MetadataAsValues; + DenseMap ValueNames; + #define HANDLE_MDNODE_LEAF(CLASS) DenseSet CLASS##s; #include "llvm/IR/Metadata.def" diff --git a/llvm/lib/IR/Metadata.cpp b/llvm/lib/IR/Metadata.cpp index 23a17a52..75b4046 100644 --- a/llvm/lib/IR/Metadata.cpp +++ b/llvm/lib/IR/Metadata.cpp @@ -256,9 +256,9 @@ ValueAsMetadata *ValueAsMetadata::get(Value *V) { if (!Entry) { assert((isa(V) || isa(V) || isa(V)) && "Expected constant or function-local value"); - assert(!V->NameAndIsUsedByMD.getInt() && + assert(!V->IsUsedByMD && "Expected this to be the only metadata use"); - V->NameAndIsUsedByMD.setInt(true); + V->IsUsedByMD = true; if (auto *C = dyn_cast(V)) Entry = new ConstantAsMetadata(C); else @@ -302,15 +302,15 @@ void ValueAsMetadata::handleRAUW(Value *From, Value *To) { auto &Store = Context.pImpl->ValuesAsMetadata; auto I = Store.find(From); if (I == Store.end()) { - assert(!From->NameAndIsUsedByMD.getInt() && + assert(!From->IsUsedByMD && "Expected From not to be used by metadata"); return; } // Remove old entry from the map. - assert(From->NameAndIsUsedByMD.getInt() && + assert(From->IsUsedByMD && "Expected From to be used by metadata"); - From->NameAndIsUsedByMD.setInt(false); + From->IsUsedByMD = false; ValueAsMetadata *MD = I->second; assert(MD && "Expected valid metadata"); assert(MD->getValue() == From && "Expected valid mapping"); @@ -346,9 +346,9 @@ void ValueAsMetadata::handleRAUW(Value *From, Value *To) { } // Update MD in place (and update the map entry). - assert(!To->NameAndIsUsedByMD.getInt() && + assert(!To->IsUsedByMD && "Expected this to be the only metadata use"); - To->NameAndIsUsedByMD.setInt(true); + To->IsUsedByMD = true; MD->V = To; Entry = MD; } diff --git a/llvm/lib/IR/Value.cpp b/llvm/lib/IR/Value.cpp index fd0ed31..dcf0ad5 100644 --- a/llvm/lib/IR/Value.cpp +++ b/llvm/lib/IR/Value.cpp @@ -46,8 +46,9 @@ static inline Type *checkType(Type *Ty) { } Value::Value(Type *ty, unsigned scid) - : VTy(checkType(ty)), UseList(nullptr), SubclassID(scid), HasValueHandle(0), - SubclassOptionalData(0), SubclassData(0), NumOperands(0) { + : VTy(checkType(ty)), UseList(nullptr), SubclassID(scid), + HasValueHandle(0), SubclassOptionalData(0), SubclassData(0), + NumOperands(0), IsUsedByMD(false), HasName(false) { // FIXME: Why isn't this in the subclass gunk?? // Note, we cannot call isa before the CallInst has been // constructed. @@ -157,11 +158,39 @@ static bool getSymTab(Value *V, ValueSymbolTable *&ST) { return false; } +ValueName *Value::getValueName() const { + if (!HasName) return nullptr; + + LLVMContext &Ctx = getContext(); + auto I = Ctx.pImpl->ValueNames.find(this); + assert(I != Ctx.pImpl->ValueNames.end() && + "No name entry found!"); + + return I->second; +} + +void Value::setValueName(ValueName *VN) { + LLVMContext &Ctx = getContext(); + + assert(HasName == Ctx.pImpl->ValueNames.count(this) && + "HasName bit out of sync!"); + + if (!VN) { + if (HasName) + Ctx.pImpl->ValueNames.erase(this); + HasName = false; + return; + } + + HasName = true; + Ctx.pImpl->ValueNames[this] = VN; +} + StringRef Value::getName() const { // Make sure the empty string is still a C string. For historical reasons, // some clients want to call .data() on the result and expect it to be null // terminated. - if (!getValueName()) + if (!hasName()) return StringRef("", 0); return getValueName()->getKey(); } -- 2.7.4