[llvm] r222211 - IR: Move MDNode operands from the back to the front
Duncan P. N. Exon Smith
dexonsmith at apple.com
Mon Nov 17 17:56:14 PST 2014
Author: dexonsmith
Date: Mon Nov 17 19:56:14 2014
New Revision: 222211
URL: http://llvm.org/viewvc/llvm-project?rev=222211&view=rev
Log:
IR: Move MDNode operands from the back to the front
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.
Modified:
llvm/trunk/include/llvm/IR/Metadata.h
llvm/trunk/lib/IR/LLVMContextImpl.cpp
llvm/trunk/lib/IR/Metadata.cpp
Modified: llvm/trunk/include/llvm/IR/Metadata.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Metadata.h?rev=222211&r1=222210&r2=222211&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/Metadata.h (original)
+++ llvm/trunk/include/llvm/IR/Metadata.h Mon Nov 17 19:56:14 2014
@@ -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;
};
//===----------------------------------------------------------------------===//
Modified: llvm/trunk/lib/IR/LLVMContextImpl.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LLVMContextImpl.cpp?rev=222211&r1=222210&r2=222211&view=diff
==============================================================================
--- llvm/trunk/lib/IR/LLVMContextImpl.cpp (original)
+++ llvm/trunk/lib/IR/LLVMContextImpl.cpp Mon Nov 17 19:56:14 2014
@@ -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.");
Modified: llvm/trunk/lib/IR/Metadata.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Metadata.cpp?rev=222211&r1=222210&r2=222211&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Metadata.cpp (original)
+++ llvm/trunk/lib/IR/Metadata.cpp Mon Nov 17 19:56:14 2014
@@ -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(
/// \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
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
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() co
#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 &C
}
// 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
}
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(MDNodeOperan
auto I = Store.find_as(Key);
if (I != Store.end()) {
N->replaceAllUsesWith(*I);
- N->destroy();
+ delete N;
return;
}
More information about the llvm-commits
mailing list