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).

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

diff --git a/include/llvm/IR/Value.h b/include/llvm/IR/Value.h
index 3bf2943a8980a612b9bc8a8bbc9fb74e666ea769..19a1d6cd91daafb2eb1016c919fb07709f3dde24 100644 (file)
--- a/include/llvm/IR/Value.h
+++ b/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<ValueName *, 1> 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 <typename UseT> // 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/lib/IR/LLVMContextImpl.h b/lib/IR/LLVMContextImpl.h
index f81db6077704dc4506535fbbb2d81b7177cd77a9..3a573362b4114f5dd379a3b1bb18d2198a8cde2f 100644 (file)
--- a/lib/IR/LLVMContextImpl.h
+++ b/lib/IR/LLVMContextImpl.h
@@ -922,6 +922,8 @@ public:
   DenseMap<Value *, ValueAsMetadata *> ValuesAsMetadata;
   DenseMap<Metadata *, MetadataAsValue *> MetadataAsValues;
 
+  DenseMap<const Value*, ValueName*> ValueNames;
+
 #define HANDLE_MDNODE_LEAF(CLASS) DenseSet<CLASS *, CLASS##Info> CLASS##s;
 #include "llvm/IR/Metadata.def"
 

diff --git a/lib/IR/Metadata.cpp b/lib/IR/Metadata.cpp
index 23a17a52b2b8ca2df581174464641ca234af6176..75b4046ef4429f2fbec92e0ad36e5b92d0cca1c6 100644 (file)
--- a/lib/IR/Metadata.cpp
+++ b/lib/IR/Metadata.cpp
@@ -256,9 +256,9 @@ ValueAsMetadata *ValueAsMetadata::get(Value *V) {
   if (!Entry) {
     assert((isa<Constant>(V) || isa<Argument>(V) || isa<Instruction>(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<Constant>(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/lib/IR/Value.cpp b/lib/IR/Value.cpp
index fd0ed31ccc623482fd41325b87949711ccf18fec..dcf0ad50190f63c10dde574d7841c48e3be02de4 100644 (file)
--- a/lib/IR/Value.cpp
+++ b/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<CallInst> 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();
 }