The MDString class stored a StringRef to the string which was already in a

StringMap. This was redundant and unnecessarily bloated the MDString class.

Because the MDString class is a "Value" and will never have a "name", and
because the Name field in the Value class is a pointer to a StringMap entry, we
repurpose the Name field for an MDString. It stores the StringMap entry in the
Name field, and uses the normal methods to get the string (name) back.

PR12474

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

diff --git a/include/llvm/Metadata.h b/include/llvm/Metadata.h
index 29458ef34613aa6f63176b8500f6d993c13ec0a9..0d438522ab1f659c15cfb5e68b18aa14cf4e79ab 100644 (file)
--- a/include/llvm/Metadata.h
+++ b/include/llvm/Metadata.h
@@ -39,28 +39,24 @@ class MDString : public Value {
   virtual void anchor();
   MDString(const MDString &);            // DO NOT IMPLEMENT
 
-  StringRef Str;
-  explicit MDString(LLVMContext &C, StringRef S);
-
+  explicit MDString(LLVMContext &C);
 public:
   static MDString *get(LLVMContext &Context, StringRef Str);
   static MDString *get(LLVMContext &Context, const char *Str) {
     return get(Context, Str ? StringRef(Str) : StringRef());
   }
 
-  StringRef getString() const { return Str; }
+  StringRef getString() const { return getName(); }
 
-  unsigned getLength() const { return (unsigned)Str.size(); }
+  unsigned getLength() const { return (unsigned)getName().size(); }
 
   typedef StringRef::iterator iterator;
   
   /// begin() - Pointer to the first byte of the string.
-  ///
-  iterator begin() const { return Str.begin(); }
+  iterator begin() const { return getName().begin(); }
 
   /// end() - Pointer to one byte past the end of the string.
-  ///
-  iterator end() const { return Str.end(); }
+  iterator end() const { return getName().end(); }
 
   /// Methods for support type inquiry through isa, cast, and dyn_cast:
   static inline bool classof(const MDString *) { return true; }

diff --git a/include/llvm/Value.h b/include/llvm/Value.h
index 84ed037ae77eccadeb2fdea54afdc6f4e1e085dc..a82ac45c49edf3341017ba51340c908b19b10561 100644 (file)
--- a/include/llvm/Value.h
+++ b/include/llvm/Value.h
@@ -107,9 +107,10 @@ public:
   /// All values hold a context through their type.
   LLVMContext &getContext() const;
 
-  // All values can potentially be named...
-  bool hasName() const { return Name != 0; }
+  // All values can potentially be named.
+  bool hasName() const { return Name != 0 && SubclassID != MDStringVal; }
   ValueName *getValueName() const { return Name; }
+  void setValueName(ValueName *VN) { Name = VN; }
   
   /// getName() - Return a constant reference to the value's name. This is cheap
   /// and guaranteed to return the same reference as long as the value is not

diff --git a/lib/VMCore/LLVMContextImpl.h b/lib/VMCore/LLVMContextImpl.h
index f98526de650e9d06f6c4449bbd8cc39bcda0d918..1a4bf6d7b44b645ca22b8336855c64e6227234e3 100644 (file)
--- a/lib/VMCore/LLVMContextImpl.h
+++ b/lib/VMCore/LLVMContextImpl.h
@@ -234,7 +234,7 @@ public:
                          DenseMapAPFloatKeyInfo> FPMapTy;
   FPMapTy FPConstants;
   
-  StringMap<MDString*> MDStringCache;
+  StringMap<Value*> MDStringCache;
   
   FoldingSet<MDNode> MDNodeSet;
   // MDNodes may be uniqued or not uniqued.  When they're not uniqued, they

diff --git a/lib/VMCore/Metadata.cpp b/lib/VMCore/Metadata.cpp
index e6fd64c0afaf83f5cbbad5c71a9ba90cb0a3c3a1..55de0dc6515de9958cacd0ce2f74e4aac9fb0220 100644 (file)
--- a/lib/VMCore/Metadata.cpp
+++ b/lib/VMCore/Metadata.cpp
@@ -31,16 +31,17 @@ using namespace llvm;
 
 void MDString::anchor() { }
 
-MDString::MDString(LLVMContext &C, StringRef S)
-  : Value(Type::getMetadataTy(C), Value::MDStringVal), Str(S) {}
+MDString::MDString(LLVMContext &C)
+  : Value(Type::getMetadataTy(C), Value::MDStringVal) {}
 
 MDString *MDString::get(LLVMContext &Context, StringRef Str) {
   LLVMContextImpl *pImpl = Context.pImpl;
-  StringMapEntry<MDString *> &Entry =
+  StringMapEntry<Value*> &Entry =
     pImpl->MDStringCache.GetOrCreateValue(Str);
-  MDString *&S = Entry.getValue();
-  if (!S) S = new MDString(Context, Entry.getKey());
-  return S;
+  Value *&S = Entry.getValue();
+  if (!S) S = new MDString(Context);
+  S->setValueName(&Entry);
+  return cast<MDString>(S);
 }
 
 //===----------------------------------------------------------------------===//

diff --git a/lib/VMCore/Value.cpp b/lib/VMCore/Value.cpp
index 788f4191f5b6faafd8d0307cc7220b6038f423f6..4006b2c554188d57e70a81a082413cbbb10f86a8 100644 (file)
--- a/lib/VMCore/Value.cpp
+++ b/lib/VMCore/Value.cpp
@@ -76,7 +76,7 @@ Value::~Value() {
 
   // If this value is named, destroy the name.  This should not be in a symtab
   // at this point.
-  if (Name)
+  if (Name && SubclassID != MDStringVal)
     Name->Destroy();
 
   // There should be no uses of this object anymore, remove it.
@@ -170,6 +170,9 @@ StringRef Value::getName() const {
 }
 
 void Value::setName(const Twine &NewName) {
+  assert(SubclassID != MDStringVal &&
+         "Cannot set the name of MDString with this method!");
+
   // Fast path for common IRBuilder case of setName("") when there is no name.
   if (NewName.isTriviallyEmpty() && !hasName())
     return;
@@ -228,6 +231,8 @@ void Value::setName(const Twine &NewName) {
 /// takeName - transfer the name from V to this value, setting V's name to
 /// empty.  It is an error to call V->takeName(V).
 void Value::takeName(Value *V) {
+  assert(SubclassID != MDStringVal && "Cannot take the name of an MDString!");
+
   ValueSymbolTable *ST = 0;
   // If this value has a name, drop it.
   if (hasName()) {