OSDN Git Service

IR: Move MDNode operands from the back to the front
authorDuncan P. N. Exon Smith <dexonsmith@apple.com>
Tue, 18 Nov 2014 01:56:14 +0000 (01:56 +0000)
committerDuncan P. N. Exon Smith <dexonsmith@apple.com>
Tue, 18 Nov 2014 01:56:14 +0000 (01:56 +0000)
Having the operands at the back prevents subclasses from safely adding
fields.  Move them to the front.

Instead of replicating the custom `malloc()`, `free()` and `DestroyFlag`
logic that was there before, overload `new` and `delete`.

I added calls to a new `GenericMDNode::dropAllReferences()` in
`LLVMContextImpl::~LLVMContextImpl()`.  There's a maze of callbacks
happening during teardown, and this resolves them before we enter
the destructors.

Part of PR21532.

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

include/llvm/IR/Metadata.h
lib/IR/LLVMContextImpl.cpp
lib/IR/Metadata.cpp

index b35165a..6da5b21 100644 (file)
@@ -22,6 +22,7 @@
 #include "llvm/ADT/ilist_node.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/IR/Value.h"
+#include "llvm/Support/ErrorHandling.h"
 
 namespace llvm {
 class LLVMContext;
@@ -145,8 +146,24 @@ class MDNode : public Metadata {
   void operator=(const MDNode &) LLVM_DELETED_FUNCTION;
   friend class MDNodeOperand;
   friend class LLVMContextImpl;
+  void *operator new(size_t) LLVM_DELETED_FUNCTION;
 
 protected:
+  void *operator new(size_t Size, unsigned NumOps);
+
+  /// \brief Required by std, but never called.
+  void operator delete(void *Mem);
+
+  /// \brief Required by std, but never called.
+  void operator delete(void *, unsigned) {
+    llvm_unreachable("Constructor throws?");
+  }
+
+  /// \brief Required by std, but never called.
+  void operator delete(void *, unsigned, bool) {
+    llvm_unreachable("Constructor throws?");
+  }
+
   // TODO: Sink this into GenericMDNode.  Can't do this until operands are
   // allocated at the front (currently they're at the back).
   unsigned Hash;
@@ -160,11 +177,7 @@ protected:
 
     /// NotUniquedBit - This is set on MDNodes that are not uniqued because they
     /// have a null operand.
-    NotUniquedBit    = 1 << 1,
-
-    /// DestroyFlag - This bit is set by destroy() so the destructor can assert
-    /// that the node isn't being destroyed with a plain 'delete'.
-    DestroyFlag      = 1 << 2
+    NotUniquedBit    = 1 << 1
   };
 
   /// \brief FunctionLocal enums.
@@ -176,10 +189,10 @@ protected:
 
   /// \brief Replace each instance of the given operand with a new value.
   void replaceOperand(MDNodeOperand *Op, Value *NewVal);
-  ~MDNode();
 
   MDNode(LLVMContext &C, unsigned ID, ArrayRef<Value *> Vals,
          bool isFunctionLocal);
+  ~MDNode() {}
 
   static MDNode *getMDNode(LLVMContext &C, ArrayRef<Value*> Vals,
                            FunctionLocalness FL, bool Insert = true);
@@ -270,11 +283,14 @@ protected:
 /// TODO: Drop support for RAUW.
 class GenericMDNode : public MDNode {
   friend class MDNode;
+  friend class LLVMContextImpl;
 
   GenericMDNode(LLVMContext &C, ArrayRef<Value *> Vals, bool isFunctionLocal)
       : MDNode(C, GenericMDNodeVal, Vals, isFunctionLocal) {}
   ~GenericMDNode();
 
+  void dropAllReferences();
+
 public:
   /// \brief Get the hash, if any.
   unsigned getHash() const { return Hash; }
@@ -282,12 +298,6 @@ public:
   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.
@@ -306,11 +316,6 @@ 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;
 };
 
 //===----------------------------------------------------------------------===//
index d4239e5..3fd0bb3 100644 (file)
@@ -126,8 +126,10 @@ LLVMContextImpl::~LLVMContextImpl() {
   MDNodes.reserve(MDNodeSet.size() + NonUniquedMDNodes.size());
   MDNodes.append(MDNodeSet.begin(), MDNodeSet.end());
   MDNodes.append(NonUniquedMDNodes.begin(), NonUniquedMDNodes.end());
-  for (auto &I : MDNodes)
-    I->destroy();
+  for (GenericMDNode *I : MDNodes)
+    I->dropAllReferences();
+  for (GenericMDNode *I : MDNodes)
+    delete I;
   assert(MDNodeSet.empty() && NonUniquedMDNodes.empty() &&
          "Destroying all MDNodes didn't empty the Context's sets.");
 
index 3c6c846..b4da2e0 100644 (file)
@@ -66,25 +66,25 @@ class MDNodeOperand : public CallbackVH {
     MDNodeOperand *Cur = this;
 
     while (Cur->getValPtrInt() != 1)
-      --Cur;
+      ++Cur;
 
     assert(Cur->getValPtrInt() == 1 &&
-           "Couldn't find the beginning of the operand list!");
-    return reinterpret_cast<MDNode*>(Cur) - 1;
+           "Couldn't find the end of the operand list!");
+    return reinterpret_cast<MDNode *>(Cur + 1);
   }
 
 public:
-  MDNodeOperand(Value *V) : CallbackVH(V) {}
+  MDNodeOperand() {}
   virtual ~MDNodeOperand();
 
   void set(Value *V) {
-    unsigned IsFirst = this->getValPtrInt();
+    unsigned IsLast = this->getValPtrInt();
     this->setValPtr(V);
-    this->setAsFirstOperand(IsFirst);
+    this->setAsLastOperand(IsLast);
   }
 
   /// \brief Accessor method to mark the operand as the first in the list.
-  void setAsFirstOperand(unsigned V) { this->setValPtrInt(V); }
+  void setAsLastOperand(unsigned I) { this->setValPtrInt(I); }
 
   void deleted() override;
   void allUsesReplacedWith(Value *NV) override;
@@ -108,14 +108,9 @@ 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;
+  return reinterpret_cast<MDNodeOperand *>(N) - N->getNumOperands() + Op;
 }
 
 void MDNode::replaceOperandWith(unsigned i, Value *Val) {
@@ -123,6 +118,26 @@ void MDNode::replaceOperandWith(unsigned i, Value *Val) {
   replaceOperand(Op, Val);
 }
 
+void *MDNode::operator new(size_t Size, unsigned NumOps) {
+  void *Ptr = ::operator new(Size + NumOps * sizeof(MDNodeOperand));
+  MDNodeOperand *Op = static_cast<MDNodeOperand *>(Ptr);
+  if (NumOps) {
+    MDNodeOperand *Last = Op + NumOps;
+    for (; Op != Last; ++Op)
+      new (Op) MDNodeOperand();
+    (Op - 1)->setAsLastOperand(1);
+  }
+  return Op;
+}
+
+void MDNode::operator delete(void *Mem) {
+  MDNode *N = static_cast<MDNode *>(Mem);
+  MDNodeOperand *Op = static_cast<MDNodeOperand *>(Mem);
+  for (unsigned I = 0, E = N->NumOperands; I != E; ++I)
+    (--Op)->~MDNodeOperand();
+  ::operator delete(Op);
+}
+
 MDNode::MDNode(LLVMContext &C, unsigned ID, ArrayRef<Value *> Vals,
                bool isFunctionLocal)
     : Metadata(C, ID), Hash(0) {
@@ -131,16 +146,11 @@ MDNode::MDNode(LLVMContext &C, unsigned ID, ArrayRef<Value *> Vals,
   if (isFunctionLocal)
     setValueSubclassData(getSubclassDataFromValue() | FunctionLocalBit);
 
-  // Initialize the operand list, which is co-allocated on the end of the node.
+  // Initialize the operand list.
   unsigned i = 0;
-  for (MDNodeOperand *Op = getOperandPtr(this, 0), *E = Op+NumOperands;
-       Op != E; ++Op, ++i) {
-    new (Op) MDNodeOperand(Vals[i]);
-
-    // Mark the first MDNodeOperand as being the first in the list of operands.
-    if (i == 0)
-      Op->setAsFirstOperand(1);
-  }
+  for (MDNodeOperand *Op = getOperandPtr(this, 0), *E = Op + NumOperands;
+       Op != E; ++Op, ++i)
+    Op->set(Vals[i]);
 }
 
 GenericMDNode::~GenericMDNode() {
@@ -152,14 +162,10 @@ GenericMDNode::~GenericMDNode() {
   }
 }
 
-MDNode::~MDNode() {
-  assert((getSubclassDataFromValue() & DestroyFlag) != 0 &&
-         "Not being destroyed through destroy()?");
-
-  // Destroy the operands.
-  for (MDNodeOperand *Op = getOperandPtr(this, 0), *E = Op+NumOperands;
+void GenericMDNode::dropAllReferences() {
+  for (MDNodeOperand *Op = getOperandPtr(this, 0), *E = Op + NumOperands;
        Op != E; ++Op)
-    Op->~MDNodeOperand();
+    Op->set(nullptr);
 }
 
 static const Function *getFunctionForValue(Value *V) {
@@ -216,21 +222,6 @@ const Function *MDNode::getFunction() const {
 #endif
 }
 
-// destroy - Delete this node.  Only when there are no uses.
-void GenericMDNode::destroy() {
-  setValueSubclassData(getSubclassDataFromValue() | DestroyFlag);
-  // Placement delete, then free the memory.
-  this->~GenericMDNode();
-  free(this);
-}
-
-void MDNodeFwdDecl::destroy() {
-  setValueSubclassData(getSubclassDataFromValue() | DestroyFlag);
-  // Placement delete, then free the memory.
-  this->~MDNodeFwdDecl();
-  free(this);
-}
-
 /// \brief Check if the Value  would require a function-local MDNode.
 static bool isFunctionLocalValue(Value *V) {
   return isa<Instruction>(V) || isa<Argument>(V) || isa<BasicBlock>(V) ||
@@ -268,9 +259,8 @@ MDNode *MDNode::getMDNode(LLVMContext &Context, ArrayRef<Value*> Vals,
   }
 
   // Coallocate space for the node and Operands together, then placement new.
-  void *Ptr =
-      malloc(sizeof(GenericMDNode) + Vals.size() * sizeof(MDNodeOperand));
-  GenericMDNode *N = new (Ptr) GenericMDNode(Context, Vals, isFunctionLocal);
+  GenericMDNode *N =
+      new (Vals.size()) GenericMDNode(Context, Vals, isFunctionLocal);
 
   N->Hash = Key.Hash;
   Store.insert(N);
@@ -292,11 +282,8 @@ MDNode *MDNode::getIfExists(LLVMContext &Context, ArrayRef<Value*> Vals) {
 }
 
 MDNode *MDNode::getTemporary(LLVMContext &Context, ArrayRef<Value*> Vals) {
-  MDNode *N = (MDNode *)malloc(sizeof(MDNodeFwdDecl) +
-                               Vals.size() * sizeof(MDNodeOperand));
-  N = new (N) MDNodeFwdDecl(Context, Vals, FL_No);
-  N->setValueSubclassData(N->getSubclassDataFromValue() |
-                          NotUniquedBit);
+  MDNode *N = new (Vals.size()) MDNodeFwdDecl(Context, Vals, FL_No);
+  N->setValueSubclassData(N->getSubclassDataFromValue() | NotUniquedBit);
   LeakDetector::addGarbageObject(N);
   return N;
 }
@@ -306,10 +293,8 @@ void MDNode::deleteTemporary(MDNode *N) {
   assert(isa<MDNodeFwdDecl>(N) && "Expected forward declaration");
   assert((N->getSubclassDataFromValue() & NotUniquedBit) &&
          "Temporary MDNode does not have NotUniquedBit set!");
-  assert((N->getSubclassDataFromValue() & DestroyFlag) == 0 &&
-         "Temporary MDNode has DestroyFlag set!");
   LeakDetector::removeGarbageObject(N);
-  cast<MDNodeFwdDecl>(N)->destroy();
+  delete cast<MDNodeFwdDecl>(N);
 }
 
 /// \brief Return specified operand.
@@ -384,7 +369,7 @@ void MDNode::replaceOperand(MDNodeOperand *Op, Value *To) {
   auto I = Store.find_as(Key);
   if (I != Store.end()) {
     N->replaceAllUsesWith(*I);
-    N->destroy();
+    delete N;
     return;
   }