IR: Split MDNode into GenericMDNode and MDNodeFwdDecl

Split `MDNode` into two classes:

  - `GenericMDNode`, which is uniquable (and for now, always starts
    uniqued).  Once `Metadata` is split from the `Value` hierarchy, this
    class will lose the ability to RAUW itself.

  - `MDNodeFwdDecl`, which is used for the "temporary" interface, is
    never uniqued, and isn't managed by `LLVMContext` at all.

I've left most of the guts in `MDNode` for now, but I'll incrementally
move things to the right places (or delete the functionality, as
appropriate).

Part of PR21532.

llvm-svn: 222205

llvm/include/llvm/IR/Metadata.h

@@ -45,7 +45,9 @@ protected:
 
 public:
   static bool classof(const Value *V) {
-    return V->getValueID() == MDNodeVal || V->getValueID() == MDStringVal;
+    return V->getValueID() == GenericMDNodeVal ||
+           V->getValueID() == MDNodeFwdDeclVal ||
+           V->getValueID() == MDStringVal;
   }
 };
 
@@ -137,14 +139,16 @@ struct DenseMapInfo<AAMDNodes> {
 class MDNodeOperand;
 
 //===----------------------------------------------------------------------===//
-/// \brief Generic tuple of metadata.
+/// \brief Tuple of metadata.
 class MDNode : public Metadata {
   MDNode(const MDNode &) LLVM_DELETED_FUNCTION;
   void operator=(const MDNode &) LLVM_DELETED_FUNCTION;
   friend class MDNodeOperand;
   friend class LLVMContextImpl;
 
-  /// \brief If the MDNode is uniqued cache the hash to speed up lookup.
+protected:
+  // TODO: Sink this into GenericMDNode.  Can't do this until operands are
+  // allocated at the front (currently they're at the back).
   unsigned Hash;
 
   /// \brief Subclass data enums.
@@ -174,7 +178,8 @@ class MDNode : public Metadata {
   void replaceOperand(MDNodeOperand *Op, Value *NewVal);
   ~MDNode();
 
-  MDNode(LLVMContext &C, ArrayRef<Value*> Vals, bool isFunctionLocal);
+  MDNode(LLVMContext &C, unsigned ID, ArrayRef<Value *> Vals,
+         bool isFunctionLocal);
 
   static MDNode *getMDNode(LLVMContext &C, ArrayRef<Value*> Vals,
                            FunctionLocalness FL, bool Insert = true);
@@ -223,12 +228,10 @@ public:
   /// code because it recursively visits all the MDNode's operands.
   const Function *getFunction() const;
 
-  /// \brief Get the hash, if any.
-  unsigned getHash() const { return Hash; }
-
   /// \brief Methods for support type inquiry through isa, cast, and dyn_cast:
   static bool classof(const Value *V) {
-    return V->getValueID() == MDNodeVal;
+    return V->getValueID() == GenericMDNodeVal ||
+           V->getValueID() == MDNodeFwdDeclVal;
   }
 
   /// \brief Check whether MDNode is a vtable access.
@@ -241,10 +244,8 @@ public:
   static AAMDNodes getMostGenericAA(const AAMDNodes &A, const AAMDNodes &B);
   static MDNode *getMostGenericFPMath(MDNode *A, MDNode *B);
   static MDNode *getMostGenericRange(MDNode *A, MDNode *B);
-private:
-  /// \brief Delete this node.  Only when there are no uses.
-  void destroy();
 
+protected:
   bool isNotUniqued() const {
     return (getSubclassDataFromValue() & NotUniquedBit) != 0;
   }
@@ -257,6 +258,61 @@ private:
   }
 };
 
+/// \brief Generic metadata node.
+///
+/// Generic metadata nodes, with opt-out support for uniquing.
+///
+/// Although nodes are uniqued by default, \a GenericMDNode has no support for
+/// RAUW.  If an operand change (due to RAUW or otherwise) causes a uniquing
+/// collision, the uniquing bit is dropped.
+///
+/// TODO: Make uniquing opt-out (status: mandatory, sometimes dropped).
+/// TODO: Drop support for RAUW.
+class GenericMDNode : public MDNode {
+  friend class MDNode;
+
+  GenericMDNode(LLVMContext &C, ArrayRef<Value *> Vals, bool isFunctionLocal)
+      : MDNode(C, GenericMDNodeVal, Vals, isFunctionLocal) {}
+  ~GenericMDNode();
+
+public:
+  /// \brief Get the hash, if any.
+  unsigned getHash() const { return Hash; }
+
+  static bool classof(const Value *V) {
+    return V->getValueID() == GenericMDNodeVal;
+  }
+
+private:
+  /// \brief Delete this node.  Only when there are no uses.
+  void destroy();
+  friend class MDNode;
+  friend class LLVMContextImpl;
+};
+
+/// \brief Forward declaration of metadata.
+///
+/// Forward declaration of metadata, in the form of a metadata node.  Unlike \a
+/// GenericMDNode, this class has support for RAUW and is suitable for forward
+/// references.
+class MDNodeFwdDecl : public MDNode {
+  friend class MDNode;
+
+  MDNodeFwdDecl(LLVMContext &C, ArrayRef<Value *> Vals, bool isFunctionLocal)
+      : MDNode(C, MDNodeFwdDeclVal, Vals, isFunctionLocal) {}
+  ~MDNodeFwdDecl() {}
+
+public:
+  static bool classof(const Value *V) {
+    return V->getValueID() == MDNodeFwdDeclVal;
+  }
+
+private:
+  /// \brief Delete this node.  Only when there are no uses.
+  void destroy();
+  friend class MDNode;
+};
+
 //===----------------------------------------------------------------------===//
 /// \brief A tuple of MDNodes.
 ///

llvm/include/llvm/IR/Value.h

@@ -345,7 +345,8 @@ public:
     ConstantStructVal,        // This is an instance of ConstantStruct
     ConstantVectorVal,        // This is an instance of ConstantVector
     ConstantPointerNullVal,   // This is an instance of ConstantPointerNull
-    MDNodeVal,                // This is an instance of MDNode
+    GenericMDNodeVal,         // This is an instance of GenericMDNode
+    MDNodeFwdDeclVal,         // This is an instance of MDNodeFwdDecl
     MDStringVal,              // This is an instance of MDString
     InlineAsmVal,             // This is an instance of InlineAsm
     InstructionVal,           // This is an instance of Instruction
@@ -681,7 +682,8 @@ template <> struct isa_impl<GlobalObject, Value> {
 
 template <> struct isa_impl<MDNode, Value> {
   static inline bool doit(const Value &Val) {
-    return Val.getValueID() == Value::MDNodeVal;
+    return Val.getValueID() == Value::GenericMDNodeVal ||
+           Val.getValueID() == Value::MDNodeFwdDeclVal;
   }
 };
 

llvm/lib/IR/LLVMContextImpl.cpp

@@ -122,13 +122,12 @@ LLVMContextImpl::~LLVMContextImpl() {
 
   // Destroy MDNodes.  ~MDNode can move and remove nodes between the MDNodeSet
   // and the NonUniquedMDNodes sets, so copy the values out first.
-  SmallVector<MDNode*, 8> MDNodes;
+  SmallVector<GenericMDNode *, 8> MDNodes;
   MDNodes.reserve(MDNodeSet.size() + NonUniquedMDNodes.size());
   MDNodes.append(MDNodeSet.begin(), MDNodeSet.end());
   MDNodes.append(NonUniquedMDNodes.begin(), NonUniquedMDNodes.end());
-  for (SmallVectorImpl<MDNode *>::iterator I = MDNodes.begin(),
+  for (auto &I : MDNodes)
-         E = MDNodes.end(); I != E; ++I)
+    I->destroy();
-    (*I)->destroy();
   assert(MDNodeSet.empty() && NonUniquedMDNodes.empty() &&
          "Destroying all MDNodes didn't empty the Context's sets.");
 

llvm/lib/IR/LLVMContextImpl.h

@@ -191,7 +191,7 @@ struct FunctionTypeKeyInfo {
   }
 };
 
-/// \brief DenseMapInfo for MDNode.
+/// \brief DenseMapInfo for GenericMDNode.
 ///
 /// Note that we don't need the is-function-local bit, since that's implicit in
 /// the operands.
@@ -203,7 +203,7 @@ struct GenericMDNodeInfo {
     KeyTy(ArrayRef<Value *> Ops)
         : Ops(Ops), Hash(hash_combine_range(Ops.begin(), Ops.end())) {}
 
-    KeyTy(MDNode *N, SmallVectorImpl<Value *> &Storage) {
+    KeyTy(GenericMDNode *N, SmallVectorImpl<Value *> &Storage) {
       Storage.resize(N->getNumOperands());
       for (unsigned I = 0, E = N->getNumOperands(); I != E; ++I)
         Storage[I] = N->getOperand(I);
@@ -211,7 +211,7 @@ struct GenericMDNodeInfo {
       Hash = hash_combine_range(Ops.begin(), Ops.end());
     }
 
-    bool operator==(const MDNode *RHS) const {
+    bool operator==(const GenericMDNode *RHS) const {
       if (RHS == getEmptyKey() || RHS == getTombstoneKey())
         return false;
       if (Hash != RHS->getHash() || Ops.size() != RHS->getNumOperands())
@@ -222,20 +222,20 @@ struct GenericMDNodeInfo {
       return true;
     }
   };
-  static inline MDNode *getEmptyKey() {
+  static inline GenericMDNode *getEmptyKey() {
-    return DenseMapInfo<MDNode *>::getEmptyKey();
+    return DenseMapInfo<GenericMDNode *>::getEmptyKey();
   }
-  static inline MDNode *getTombstoneKey() {
+  static inline GenericMDNode *getTombstoneKey() {
-    return DenseMapInfo<MDNode *>::getTombstoneKey();
+    return DenseMapInfo<GenericMDNode *>::getTombstoneKey();
   }
   static unsigned getHashValue(const KeyTy &Key) { return Key.Hash; }
-  static unsigned getHashValue(const MDNode *U) {
+  static unsigned getHashValue(const GenericMDNode *U) {
     return U->getHash();
   }
-  static bool isEqual(const KeyTy &LHS, const MDNode *RHS) {
+  static bool isEqual(const KeyTy &LHS, const GenericMDNode *RHS) {
     return LHS == RHS;
   }
-  static bool isEqual(const MDNode *LHS, const MDNode *RHS) {
+  static bool isEqual(const GenericMDNode *LHS, const GenericMDNode *RHS) {
     return LHS == RHS;
   }
 };
@@ -293,14 +293,14 @@ public:
 
   StringMap<MDString> MDStringCache;
 
-  DenseSet<MDNode *, GenericMDNodeInfo> MDNodeSet;
+  DenseSet<GenericMDNode *, GenericMDNodeInfo> MDNodeSet;
 
   // MDNodes may be uniqued or not uniqued.  When they're not uniqued, they
   // aren't in the MDNodeSet, but they're still shared between objects, so no
   // one object can destroy them.  This set allows us to at least destroy them
   // on Context destruction.
-  SmallPtrSet<MDNode*, 1> NonUniquedMDNodes;
+  SmallPtrSet<GenericMDNode *, 1> NonUniquedMDNodes;
-  
+
   DenseMap<Type*, ConstantAggregateZero*> CAZConstants;
 
   typedef ConstantUniqueMap<ConstantArray> ArrayConstantsTy;

llvm/lib/IR/Metadata.cpp

@@ -108,6 +108,11 @@ void MDNodeOperand::allUsesReplacedWith(Value *NV) {
 
 /// \brief Get the MDNodeOperand's coallocated on the end of the MDNode.
 static MDNodeOperand *getOperandPtr(MDNode *N, unsigned Op) {
+  static_assert(sizeof(GenericMDNode) == sizeof(MDNode),
+                "Expected subclasses to have no size overhead");
+  static_assert(sizeof(MDNodeFwdDecl) == sizeof(MDNode),
+                "Expected subclasses to have no size overhead");
+
   // Use <= instead of < to permit a one-past-the-end address.
   assert(Op <= N->getNumOperands() && "Invalid operand number");
   return reinterpret_cast<MDNodeOperand*>(N + 1) + Op;
@@ -118,8 +123,9 @@ void MDNode::replaceOperandWith(unsigned i, Value *Val) {
   replaceOperand(Op, Val);
 }
 
-MDNode::MDNode(LLVMContext &C, ArrayRef<Value *> Vals, bool isFunctionLocal)
+MDNode::MDNode(LLVMContext &C, unsigned ID, ArrayRef<Value *> Vals,
-    : Metadata(C, Value::MDNodeVal), Hash(0) {
+               bool isFunctionLocal)
+    : Metadata(C, ID), Hash(0) {
   NumOperands = Vals.size();
 
   if (isFunctionLocal)
@@ -137,16 +143,18 @@ MDNode::MDNode(LLVMContext &C, ArrayRef<Value *> Vals, bool isFunctionLocal)
   }
 }
 
-/// ~MDNode - Destroy MDNode.
+GenericMDNode::~GenericMDNode() {
-MDNode::~MDNode() {
-  assert((getSubclassDataFromValue() & DestroyFlag) != 0 &&
-         "Not being destroyed through destroy()?");
   LLVMContextImpl *pImpl = getType()->getContext().pImpl;
   if (isNotUniqued()) {
     pImpl->NonUniquedMDNodes.erase(this);
   } else {
     pImpl->MDNodeSet.erase(this);
   }
+}
+
+MDNode::~MDNode() {
+  assert((getSubclassDataFromValue() & DestroyFlag) != 0 &&
+         "Not being destroyed through destroy()?");
 
   // Destroy the operands.
   for (MDNodeOperand *Op = getOperandPtr(this, 0), *E = Op+NumOperands;
@@ -209,10 +217,17 @@ const Function *MDNode::getFunction() const {
 }
 
 // destroy - Delete this node.  Only when there are no uses.
-void MDNode::destroy() {
+void GenericMDNode::destroy() {
   setValueSubclassData(getSubclassDataFromValue() | DestroyFlag);
   // Placement delete, then free the memory.
-  this->~MDNode();
+  this->~GenericMDNode();
+  free(this);
+}
+
+void MDNodeFwdDecl::destroy() {
+  setValueSubclassData(getSubclassDataFromValue() | DestroyFlag);
+  // Placement delete, then free the memory.
+  this->~MDNodeFwdDecl();
   free(this);
 }
 
@@ -253,8 +268,9 @@ MDNode *MDNode::getMDNode(LLVMContext &Context, ArrayRef<Value*> Vals,
   }
 
   // Coallocate space for the node and Operands together, then placement new.
-  void *Ptr = malloc(sizeof(MDNode) + Vals.size() * sizeof(MDNodeOperand));
+  void *Ptr =
-  MDNode *N = new (Ptr) MDNode(Context, Vals, isFunctionLocal);
+      malloc(sizeof(GenericMDNode) + Vals.size() * sizeof(MDNodeOperand));
+  GenericMDNode *N = new (Ptr) GenericMDNode(Context, Vals, isFunctionLocal);
 
   N->Hash = Key.Hash;
   Store.insert(N);
@@ -276,9 +292,9 @@ MDNode *MDNode::getIfExists(LLVMContext &Context, ArrayRef<Value*> Vals) {
 }
 
 MDNode *MDNode::getTemporary(LLVMContext &Context, ArrayRef<Value*> Vals) {
-  MDNode *N =
+  MDNode *N = (MDNode *)malloc(sizeof(MDNodeFwdDecl) +
-    (MDNode *)malloc(sizeof(MDNode) + Vals.size() * sizeof(MDNodeOperand));
+                               Vals.size() * sizeof(MDNodeOperand));
-  N = new (N) MDNode(Context, Vals, FL_No);
+  N = new (N) MDNodeFwdDecl(Context, Vals, FL_No);
   N->setValueSubclassData(N->getSubclassDataFromValue() |
                           NotUniquedBit);
   LeakDetector::addGarbageObject(N);
@@ -287,16 +303,13 @@ MDNode *MDNode::getTemporary(LLVMContext &Context, ArrayRef<Value*> Vals) {
 
 void MDNode::deleteTemporary(MDNode *N) {
   assert(N->use_empty() && "Temporary MDNode has uses!");
-  assert(!N->getContext().pImpl->MDNodeSet.erase(N) &&
+  assert(isa<MDNodeFwdDecl>(N) && "Expected forward declaration");
-         "Deleting a non-temporary uniqued node!");
-  assert(!N->getContext().pImpl->NonUniquedMDNodes.erase(N) &&
-         "Deleting a non-temporary non-uniqued node!");
   assert((N->getSubclassDataFromValue() & NotUniquedBit) &&
          "Temporary MDNode does not have NotUniquedBit set!");
   assert((N->getSubclassDataFromValue() & DestroyFlag) == 0 &&
          "Temporary MDNode has DestroyFlag set!");
   LeakDetector::removeGarbageObject(N);
-  N->destroy();
+  cast<MDNodeFwdDecl>(N)->destroy();
 }
 
 /// \brief Return specified operand.
@@ -308,8 +321,9 @@ Value *MDNode::getOperand(unsigned i) const {
 void MDNode::setIsNotUniqued() {
   setValueSubclassData(getSubclassDataFromValue() | NotUniquedBit);
   LLVMContextImpl *pImpl = getType()->getContext().pImpl;
-  Hash = 0;
+  auto *G = cast<GenericMDNode>(this);
-  pImpl->NonUniquedMDNodes.insert(this);
+  G->Hash = 0;
+  pImpl->NonUniquedMDNodes.insert(G);
 }
 
 // Replace value from this node's operand list.
@@ -345,9 +359,10 @@ void MDNode::replaceOperand(MDNodeOperand *Op, Value *To) {
   }
 
   auto &Store = getContext().pImpl->MDNodeSet;
+  auto *N = cast<GenericMDNode>(this);
 
   // Remove "this" from the context map.
-  Store.erase(this);
+  Store.erase(N);
 
   // Update the operand.
   Op->set(To);
@@ -365,16 +380,16 @@ void MDNode::replaceOperand(MDNodeOperand *Op, Value *To) {
   // check to see if another node with the same operands already exists in the
   // set.  If so, then this node is redundant.
   SmallVector<Value *, 8> Vals;
-  GenericMDNodeInfo::KeyTy Key(this, Vals);
+  GenericMDNodeInfo::KeyTy Key(N, Vals);
   auto I = Store.find_as(Key);
   if (I != Store.end()) {
-    replaceAllUsesWith(*I);
+    N->replaceAllUsesWith(*I);
-    destroy();
+    N->destroy();
     return;
   }
 
-  this->Hash = Key.Hash;
+  N->Hash = Key.Hash;
-  Store.insert(this);
+  Store.insert(N);
 
   // If this MDValue was previously function-local but no longer is, clear
   // its function-local flag.