[llvm] 2740c18 - [NFC][Metadata] Refactor allocation, initalization and deletion of MDNodes.
Wolfgang Pieb via llvm-commits
llvm-commits at lists.llvm.org
Fri May 13 16:09:12 PDT 2022
Author: Wolfgang Pieb
Date: 2022-05-13T16:05:29-07:00
New Revision: 2740c1875d1c6d6b78baf4f8215e36362ca0e98c
URL: https://github.com/llvm/llvm-project/commit/2740c1875d1c6d6b78baf4f8215e36362ca0e98c
DIFF: https://github.com/llvm/llvm-project/commit/2740c1875d1c6d6b78baf4f8215e36362ca0e98c.diff
LOG: [NFC][Metadata] Refactor allocation, initalization and deletion of MDNodes.
This patch is refactoring the allocation, initialization and deletion
of MDNodes. It is intended as a preparatory patch for the upcoming
addition of dynamic resizability of MDNodes. It is fundamentally NFC,
but removes the necessity for suppressing the memory sanitizer for
MDNode's operator delete.
Reviewers: dexonsmith
Differential Revision: https://reviews.llvm.org/D125489
Added:
Modified:
llvm/include/llvm/IR/Metadata.h
llvm/lib/IR/DebugInfoMetadata.cpp
llvm/lib/IR/Metadata.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/Metadata.h b/llvm/include/llvm/IR/Metadata.h
index 31583ac1eb53..af3e5e967db6 100644
--- a/llvm/include/llvm/IR/Metadata.h
+++ b/llvm/include/llvm/IR/Metadata.h
@@ -928,8 +928,41 @@ class MDNode : public Metadata {
friend class LLVMContextImpl;
friend class DIArgList;
- unsigned NumOperands;
- unsigned NumUnresolved;
+ /// The header that is coallocated with an MDNode, along with the operands.
+ /// It is located immediately before the main body of the node. The operands
+ /// are in turn located immediately before the header.
+ struct Header {
+ unsigned NumOperands;
+ unsigned NumUnresolved = 0;
+
+ static constexpr size_t getOpSize(unsigned NumOps) {
+ return sizeof(MDOperand) * NumOps;
+ }
+ static constexpr size_t getAllocSize(unsigned NumOps) {
+ return getOpSize(NumOps) + sizeof(Header);
+ }
+ void *getAllocation() {
+ return reinterpret_cast<char *>(this + 1) -
+ alignTo(getAllocSize(NumOperands), alignof(uint64_t));
+ }
+
+ explicit Header(unsigned NumOperands);
+ ~Header();
+ MutableArrayRef<MDOperand> operands() {
+ return makeMutableArrayRef(
+ reinterpret_cast<MDOperand *>(this) - NumOperands, NumOperands);
+ }
+ ArrayRef<MDOperand> operands() const {
+ return makeArrayRef(
+ reinterpret_cast<const MDOperand *>(this) - NumOperands, NumOperands);
+ }
+ };
+
+ Header &getHeader() { return *(reinterpret_cast<Header *>(this) - 1); }
+
+ const Header &getHeader() const {
+ return *(reinterpret_cast<const Header *>(this) - 1);
+ }
ContextAndReplaceableUses Context;
@@ -938,7 +971,7 @@ class MDNode : public Metadata {
ArrayRef<Metadata *> Ops1, ArrayRef<Metadata *> Ops2 = None);
~MDNode() = default;
- void *operator new(size_t Size, unsigned NumOps);
+ void *operator new(size_t Size, unsigned NumOps, StorageType Storage);
void operator delete(void *Mem);
/// Required by std, but never called.
@@ -953,8 +986,8 @@ class MDNode : public Metadata {
void dropAllReferences();
- MDOperand *mutable_begin() { return mutable_end() - NumOperands; }
- MDOperand *mutable_end() { return reinterpret_cast<MDOperand *>(this); }
+ MDOperand *mutable_begin() { return getHeader().operands().begin(); }
+ MDOperand *mutable_end() { return getHeader().operands().end(); }
using mutable_op_range = iterator_range<MDOperand *>;
@@ -1000,7 +1033,7 @@ class MDNode : public Metadata {
/// As forward declarations are resolved, their containers should get
/// resolved automatically. However, if this (or one of its operands) is
/// involved in a cycle, \a resolveCycles() needs to be called explicitly.
- bool isResolved() const { return !isTemporary() && !NumUnresolved; }
+ bool isResolved() const { return !isTemporary() && !getNumUnresolved(); }
bool isUniqued() const { return Storage == Uniqued; }
bool isDistinct() const { return Storage == Distinct; }
@@ -1094,6 +1127,9 @@ class MDNode : public Metadata {
/// Sets the operand directly, without worrying about uniquing.
void setOperand(unsigned I, Metadata *New);
+ unsigned getNumUnresolved() const { return getHeader().NumUnresolved; }
+
+ void setNumUnresolved(unsigned N) { getHeader().NumUnresolved = N; }
void storeDistinctInContext();
template <class T, class StoreT>
static T *storeImpl(T *N, StorageType Storage, StoreT &Store);
@@ -1155,12 +1191,12 @@ class MDNode : public Metadata {
op_range operands() const { return op_range(op_begin(), op_end()); }
const MDOperand &getOperand(unsigned I) const {
- assert(I < NumOperands && "Out of range");
- return op_begin()[I];
+ assert(I < getNumOperands() && "Out of range");
+ return getHeader().operands()[I];
}
/// Return number of MDNode operands.
- unsigned getNumOperands() const { return NumOperands; }
+ unsigned getNumOperands() const { return getHeader().NumOperands; }
/// Methods for support type inquiry through isa, cast, and dyn_cast:
static bool classof(const Metadata *MD) {
diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp
index b412fb8de47c..2a1dc2e75b6f 100644
--- a/llvm/lib/IR/DebugInfoMetadata.cpp
+++ b/llvm/lib/IR/DebugInfoMetadata.cpp
@@ -78,8 +78,8 @@ DILocation *DILocation::getImpl(LLVMContext &Context, unsigned Line,
Ops.push_back(Scope);
if (InlinedAt)
Ops.push_back(InlinedAt);
- return storeImpl(new (Ops.size()) DILocation(Context, Storage, Line, Column,
- Ops, ImplicitCode),
+ return storeImpl(new (Ops.size(), Storage) DILocation(
+ Context, Storage, Line, Column, Ops, ImplicitCode),
Storage, Context.pImpl->DILocations);
}
@@ -304,7 +304,7 @@ GenericDINode *GenericDINode::getImpl(LLVMContext &Context, unsigned Tag,
// Use a nullptr for empty headers.
assert(isCanonical(Header) && "Expected canonical MDString");
Metadata *PreOps[] = {Header};
- return storeImpl(new (DwarfOps.size() + 1) GenericDINode(
+ return storeImpl(new (DwarfOps.size() + 1, Storage) GenericDINode(
Context, Storage, Hash, Tag, PreOps, DwarfOps),
Storage, Context.pImpl->GenericDINodes);
}
@@ -329,17 +329,19 @@ void GenericDINode::recalculateHash() {
} \
} while (false)
#define DEFINE_GETIMPL_STORE(CLASS, ARGS, OPS) \
- return storeImpl(new (array_lengthof(OPS)) \
+ return storeImpl(new (array_lengthof(OPS), Storage) \
CLASS(Context, Storage, UNWRAP_ARGS(ARGS), OPS), \
Storage, Context.pImpl->CLASS##s)
#define DEFINE_GETIMPL_STORE_NO_OPS(CLASS, ARGS) \
- return storeImpl(new (0u) CLASS(Context, Storage, UNWRAP_ARGS(ARGS)), \
+ return storeImpl(new (0u, Storage) \
+ CLASS(Context, Storage, UNWRAP_ARGS(ARGS)), \
Storage, Context.pImpl->CLASS##s)
#define DEFINE_GETIMPL_STORE_NO_CONSTRUCTOR_ARGS(CLASS, OPS) \
- return storeImpl(new (array_lengthof(OPS)) CLASS(Context, Storage, OPS), \
+ return storeImpl(new (array_lengthof(OPS), Storage) \
+ CLASS(Context, Storage, OPS), \
Storage, Context.pImpl->CLASS##s)
#define DEFINE_GETIMPL_STORE_N(CLASS, ARGS, OPS, NUM_OPS) \
- return storeImpl(new (NUM_OPS) \
+ return storeImpl(new (NUM_OPS, Storage) \
CLASS(Context, Storage, UNWRAP_ARGS(ARGS), OPS), \
Storage, Context.pImpl->CLASS##s)
@@ -848,7 +850,7 @@ DICompileUnit *DICompileUnit::getImpl(
Macros,
SysRoot,
SDK};
- return storeImpl(new (array_lengthof(Ops)) DICompileUnit(
+ return storeImpl(new (array_lengthof(Ops), Storage) DICompileUnit(
Context, Storage, SourceLanguage, IsOptimized,
RuntimeVersion, EmissionKind, DWOId, SplitDebugInlining,
DebugInfoForProfiling, NameTableKind, RangesBaseAddress,
diff --git a/llvm/lib/IR/Metadata.cpp b/llvm/lib/IR/Metadata.cpp
index cb003885d712..b5ea17f77914 100644
--- a/llvm/lib/IR/Metadata.cpp
+++ b/llvm/lib/IR/Metadata.cpp
@@ -523,35 +523,26 @@ StringRef MDString::getString() const {
"Alignment is insufficient after objects prepended to " #CLASS);
#include "llvm/IR/Metadata.def"
-void *MDNode::operator new(size_t Size, unsigned NumOps) {
- size_t OpSize = NumOps * sizeof(MDOperand);
+void *MDNode::operator new(size_t Size, unsigned NumOps,
+ StorageType /* Storage */) {
// uint64_t is the most aligned type we need support (ensured by static_assert
// above)
- OpSize = alignTo(OpSize, alignof(uint64_t));
- void *Ptr = reinterpret_cast<char *>(::operator new(OpSize + Size)) + OpSize;
- MDOperand *O = static_cast<MDOperand *>(Ptr);
- for (MDOperand *E = O - NumOps; O != E; --O)
- (void)new (O - 1) MDOperand;
- return Ptr;
+ size_t AllocSize = alignTo(Header::getAllocSize(NumOps), alignof(uint64_t));
+ char *Mem = reinterpret_cast<char *>(::operator new(AllocSize + Size));
+ Header *H = new (Mem + AllocSize - sizeof(Header)) Header(NumOps);
+ return reinterpret_cast<void *>(H + 1);
}
-// Repress memory sanitization, due to use-after-destroy by operator
-// delete. Bug report 24578 identifies this issue.
-LLVM_NO_SANITIZE_MEMORY_ATTRIBUTE void MDNode::operator delete(void *Mem) {
- MDNode *N = static_cast<MDNode *>(Mem);
- size_t OpSize = N->NumOperands * sizeof(MDOperand);
- OpSize = alignTo(OpSize, alignof(uint64_t));
-
- MDOperand *O = static_cast<MDOperand *>(Mem);
- for (MDOperand *E = O - N->NumOperands; O != E; --O)
- (O - 1)->~MDOperand();
- ::operator delete(reinterpret_cast<char *>(Mem) - OpSize);
+void MDNode::operator delete(void *N) {
+ Header *H = reinterpret_cast<Header *>(N) - 1;
+ void *Mem = H->getAllocation();
+ H->~Header();
+ ::operator delete(Mem);
}
MDNode::MDNode(LLVMContext &Context, unsigned ID, StorageType Storage,
ArrayRef<Metadata *> Ops1, ArrayRef<Metadata *> Ops2)
- : Metadata(ID, Storage), NumOperands(Ops1.size() + Ops2.size()),
- NumUnresolved(0), Context(Context) {
+ : Metadata(ID, Storage), Context(Context) {
unsigned Op = 0;
for (Metadata *MD : Ops1)
setOperand(Op++, MD);
@@ -577,6 +568,19 @@ TempMDNode MDNode::clone() const {
}
}
+MDNode::Header::Header(unsigned NumOps) {
+ NumOperands = NumOps;
+ MDOperand *O = reinterpret_cast<MDOperand *>(this);
+ for (MDOperand *E = O - NumOps; O != E; --O)
+ (void)new (O - 1) MDOperand();
+}
+
+MDNode::Header::~Header() {
+ MDOperand *O = reinterpret_cast<MDOperand *>(this) - NumOperands;
+ for (MDOperand *E = O + NumOperands; O != E; ++O)
+ (void)O->~MDOperand();
+}
+
static bool isOperandUnresolved(Metadata *Op) {
if (auto *N = dyn_cast_or_null<MDNode>(Op))
return !N->isResolved();
@@ -584,9 +588,9 @@ static bool isOperandUnresolved(Metadata *Op) {
}
void MDNode::countUnresolvedOperands() {
- assert(NumUnresolved == 0 && "Expected unresolved ops to be uncounted");
+ assert(getNumUnresolved() == 0 && "Expected unresolved ops to be uncounted");
assert(isUniqued() && "Expected this to be uniqued");
- NumUnresolved = count_if(operands(), isOperandUnresolved);
+ setNumUnresolved(count_if(operands(), isOperandUnresolved));
}
void MDNode::makeUniqued() {
@@ -600,7 +604,7 @@ void MDNode::makeUniqued() {
// Make this 'uniqued'.
Storage = Uniqued;
countUnresolvedOperands();
- if (!NumUnresolved) {
+ if (!getNumUnresolved()) {
dropReplaceableUses();
assert(isResolved() && "Expected this to be resolved");
}
@@ -624,14 +628,14 @@ void MDNode::resolve() {
assert(isUniqued() && "Expected this to be uniqued");
assert(!isResolved() && "Expected this to be unresolved");
- NumUnresolved = 0;
+ setNumUnresolved(0);
dropReplaceableUses();
assert(isResolved() && "Expected this to be resolved");
}
void MDNode::dropReplaceableUses() {
- assert(!NumUnresolved && "Unexpected unresolved operand");
+ assert(!getNumUnresolved() && "Unexpected unresolved operand");
// Drop any RAUW support.
if (Context.hasReplaceableUses())
@@ -640,13 +644,13 @@ void MDNode::dropReplaceableUses() {
void MDNode::resolveAfterOperandChange(Metadata *Old, Metadata *New) {
assert(isUniqued() && "Expected this to be uniqued");
- assert(NumUnresolved != 0 && "Expected unresolved operands");
+ assert(getNumUnresolved() != 0 && "Expected unresolved operands");
// Check if an operand was resolved.
if (!isOperandUnresolved(Old)) {
if (isOperandUnresolved(New))
// An operand was un-resolved!
- ++NumUnresolved;
+ setNumUnresolved(getNumUnresolved() + 1);
} else if (!isOperandUnresolved(New))
decrementUnresolvedOperandCount();
}
@@ -657,7 +661,8 @@ void MDNode::decrementUnresolvedOperandCount() {
return;
assert(isUniqued() && "Expected this to be uniqued");
- if (--NumUnresolved)
+ setNumUnresolved(getNumUnresolved() - 1);
+ if (getNumUnresolved())
return;
// Last unresolved operand has just been resolved.
@@ -732,7 +737,7 @@ void MDTuple::recalculateHash() {
}
void MDNode::dropAllReferences() {
- for (unsigned I = 0, E = NumOperands; I != E; ++I)
+ for (unsigned I = 0, E = getNumOperands(); I != E; ++I)
setOperand(I, nullptr);
if (Context.hasReplaceableUses()) {
Context.getReplaceableUses()->resolveAllUses(/* ResolveUsers */ false);
@@ -868,7 +873,8 @@ MDTuple *MDTuple::getImpl(LLVMContext &Context, ArrayRef<Metadata *> MDs,
assert(ShouldCreate && "Expected non-uniqued nodes to always be created");
}
- return storeImpl(new (MDs.size()) MDTuple(Context, Storage, Hash, MDs),
+ return storeImpl(new (MDs.size(), Storage)
+ MDTuple(Context, Storage, Hash, MDs),
Storage, Context.pImpl->MDTuples);
}
@@ -880,7 +886,7 @@ void MDNode::deleteTemporary(MDNode *N) {
void MDNode::storeDistinctInContext() {
assert(!Context.hasReplaceableUses() && "Unexpected replaceable uses");
- assert(!NumUnresolved && "Unexpected unresolved nodes");
+ assert(!getNumUnresolved() && "Unexpected unresolved nodes");
Storage = Distinct;
assert(isResolved() && "Expected this to be resolved");
@@ -913,7 +919,7 @@ void MDNode::replaceOperandWith(unsigned I, Metadata *New) {
}
void MDNode::setOperand(unsigned I, Metadata *New) {
- assert(I < NumOperands);
+ assert(I < getNumOperands());
mutable_begin()[I].reset(New, isUniqued() ? this : nullptr);
}
More information about the llvm-commits
mailing list