diff --git a/include/llvm/IR/Metadata.h b/include/llvm/IR/Metadata.h index 6f38013b877..3acf6c216bb 100644 --- a/include/llvm/IR/Metadata.h +++ b/include/llvm/IR/Metadata.h @@ -138,12 +138,11 @@ class MDNodeOperand; //===----------------------------------------------------------------------===// /// \brief Generic tuple of metadata. -class MDNode : public Metadata, public FoldingSetNode { +class MDNode : public Metadata { MDNode(const MDNode &) LLVM_DELETED_FUNCTION; void operator=(const MDNode &) LLVM_DELETED_FUNCTION; friend class MDNodeOperand; friend class LLVMContextImpl; - friend struct FoldingSetTrait; /// \brief If the MDNode is uniqued cache the hash to speed up lookup. unsigned Hash; @@ -224,8 +223,8 @@ public: /// code because it recursively visits all the MDNode's operands. const Function *getFunction() const; - /// \brief Calculate a unique identifier for this MDNode. - void Profile(FoldingSetNodeID &ID) 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) { diff --git a/lib/IR/LLVMContextImpl.cpp b/lib/IR/LLVMContextImpl.cpp index df3449d12c8..cd9da0d0e08 100644 --- a/lib/IR/LLVMContextImpl.cpp +++ b/lib/IR/LLVMContextImpl.cpp @@ -124,9 +124,7 @@ LLVMContextImpl::~LLVMContextImpl() { // and the NonUniquedMDNodes sets, so copy the values out first. SmallVector MDNodes; MDNodes.reserve(MDNodeSet.size() + NonUniquedMDNodes.size()); - for (FoldingSetIterator I = MDNodeSet.begin(), E = MDNodeSet.end(); - I != E; ++I) - MDNodes.push_back(&*I); + MDNodes.append(MDNodeSet.begin(), MDNodeSet.end()); MDNodes.append(NonUniquedMDNodes.begin(), NonUniquedMDNodes.end()); for (SmallVectorImpl::iterator I = MDNodes.begin(), E = MDNodes.end(); I != E; ++I) diff --git a/lib/IR/LLVMContextImpl.h b/lib/IR/LLVMContextImpl.h index 3190c22fceb..48343b2989e 100644 --- a/lib/IR/LLVMContextImpl.h +++ b/lib/IR/LLVMContextImpl.h @@ -22,6 +22,7 @@ #include "llvm/ADT/APInt.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/DenseSet.h" #include "llvm/ADT/FoldingSet.h" #include "llvm/ADT/Hashing.h" #include "llvm/ADT/SmallPtrSet.h" @@ -190,23 +191,52 @@ struct FunctionTypeKeyInfo { } }; -// Provide a FoldingSetTrait::Equals specialization for MDNode that can use a -// shortcut to avoid comparing all operands. -template<> struct FoldingSetTrait : DefaultFoldingSetTrait { - static bool Equals(const MDNode &X, const FoldingSetNodeID &ID, - unsigned IDHash, FoldingSetNodeID &TempID) { - assert(!X.isNotUniqued() && "Non-uniqued MDNode in FoldingSet?"); - // First, check if the cached hashes match. If they don't we can skip the - // expensive operand walk. - if (X.Hash != IDHash) - return false; +/// \brief DenseMapInfo for MDNode. +/// +/// Note that we don't need the is-function-local bit, since that's implicit in +/// the operands. +struct GenericMDNodeInfo { + struct KeyTy { + ArrayRef Ops; + unsigned Hash; - // If they match we have to compare the operands. - X.Profile(TempID); - return TempID == ID; + KeyTy(ArrayRef Ops) + : Ops(Ops), Hash(hash_combine_range(Ops.begin(), Ops.end())) {} + + KeyTy(MDNode *N, SmallVectorImpl &Storage) { + Storage.resize(N->getNumOperands()); + for (unsigned I = 0, E = N->getNumOperands(); I != E; ++I) + Storage[I] = N->getOperand(I); + Ops = Storage; + Hash = hash_combine_range(Ops.begin(), Ops.end()); + } + + bool operator==(const MDNode *RHS) const { + if (RHS == getEmptyKey() || RHS == getTombstoneKey()) + return false; + if (Hash != RHS->getHash() || Ops.size() != RHS->getNumOperands()) + return false; + for (unsigned I = 0, E = Ops.size(); I != E; ++I) + if (Ops[I] != RHS->getOperand(I)) + return false; + return true; + } + }; + static inline MDNode *getEmptyKey() { + return DenseMapInfo::getEmptyKey(); } - static unsigned ComputeHash(const MDNode &X, FoldingSetNodeID &) { - return X.Hash; // Return cached hash. + static inline MDNode *getTombstoneKey() { + return DenseMapInfo::getTombstoneKey(); + } + static unsigned getHashValue(const KeyTy &Key) { return Key.Hash; } + static unsigned getHashValue(const MDNode *U) { + return U->getHash(); + } + static bool isEqual(const KeyTy &LHS, const MDNode *RHS) { + return LHS == RHS; + } + static bool isEqual(const MDNode *LHS, const MDNode *RHS) { + return LHS == RHS; } }; @@ -263,7 +293,7 @@ public: StringMap MDStringCache; - FoldingSet MDNodeSet; + DenseSet 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 diff --git a/lib/IR/Metadata.cpp b/lib/IR/Metadata.cpp index cdd106033f8..e15317445f9 100644 --- a/lib/IR/Metadata.cpp +++ b/lib/IR/Metadata.cpp @@ -119,7 +119,7 @@ void MDNode::replaceOperandWith(unsigned i, Value *Val) { } MDNode::MDNode(LLVMContext &C, ArrayRef Vals, bool isFunctionLocal) - : Metadata(C, Value::MDNodeVal) { + : Metadata(C, Value::MDNodeVal), Hash(0) { NumOperands = Vals.size(); if (isFunctionLocal) @@ -145,7 +145,7 @@ MDNode::~MDNode() { if (isNotUniqued()) { pImpl->NonUniquedMDNodes.erase(this); } else { - pImpl->MDNodeSet.RemoveNode(this); + pImpl->MDNodeSet.erase(this); } // Destroy the operands. @@ -224,21 +224,14 @@ static bool isFunctionLocalValue(Value *V) { MDNode *MDNode::getMDNode(LLVMContext &Context, ArrayRef Vals, FunctionLocalness FL, bool Insert) { - LLVMContextImpl *pImpl = Context.pImpl; + auto &Store = Context.pImpl->MDNodeSet; - // Add all the operand pointers. Note that we don't have to add the - // isFunctionLocal bit because that's implied by the operands. - // Note that if the operands are later nulled out, the node will be - // removed from the uniquing map. - FoldingSetNodeID ID; - for (Value *V : Vals) - ID.AddPointer(V); - - void *InsertPoint; - MDNode *N = pImpl->MDNodeSet.FindNodeOrInsertPos(ID, InsertPoint); - - if (N || !Insert) - return N; + GenericMDNodeInfo::KeyTy Key(Vals); + auto I = Store.find_as(Key); + if (I != Store.end()) + return *I; + if (!Insert) + return nullptr; bool isFunctionLocal = false; switch (FL) { @@ -261,14 +254,10 @@ MDNode *MDNode::getMDNode(LLVMContext &Context, ArrayRef Vals, // Coallocate space for the node and Operands together, then placement new. void *Ptr = malloc(sizeof(MDNode) + Vals.size() * sizeof(MDNodeOperand)); - N = new (Ptr) MDNode(Context, Vals, isFunctionLocal); - - // Cache the operand hash. - N->Hash = ID.ComputeHash(); - - // InsertPoint will have been set by the FindNodeOrInsertPos call. - pImpl->MDNodeSet.InsertNode(N, InsertPoint); + MDNode *N = new (Ptr) MDNode(Context, Vals, isFunctionLocal); + N->Hash = Key.Hash; + Store.insert(N); return N; } @@ -298,7 +287,7 @@ MDNode *MDNode::getTemporary(LLVMContext &Context, ArrayRef Vals) { void MDNode::deleteTemporary(MDNode *N) { assert(N->use_empty() && "Temporary MDNode has uses!"); - assert(!N->getContext().pImpl->MDNodeSet.RemoveNode(N) && + assert(!N->getContext().pImpl->MDNodeSet.erase(N) && "Deleting a non-temporary uniqued node!"); assert(!N->getContext().pImpl->NonUniquedMDNodes.erase(N) && "Deleting a non-temporary non-uniqued node!"); @@ -316,18 +305,10 @@ Value *MDNode::getOperand(unsigned i) const { return *getOperandPtr(const_cast(this), i); } -void MDNode::Profile(FoldingSetNodeID &ID) const { - // Add all the operand pointers. Note that we don't have to add the - // isFunctionLocal bit because that's implied by the operands. - // Note that if the operands are later nulled out, the node will be - // removed from the uniquing map. - for (unsigned i = 0, e = getNumOperands(); i != e; ++i) - ID.AddPointer(getOperand(i)); -} - void MDNode::setIsNotUniqued() { setValueSubclassData(getSubclassDataFromValue() | NotUniquedBit); LLVMContextImpl *pImpl = getType()->getContext().pImpl; + Hash = 0; pImpl->NonUniquedMDNodes.insert(this); } @@ -356,44 +337,44 @@ void MDNode::replaceOperand(MDNodeOperand *Op, Value *To) { if (From == To) return; - // Update the operand. - Op->set(To); - // If this node is already not being uniqued (because one of the operands // already went to null), then there is nothing else to do here. - if (isNotUniqued()) return; + if (isNotUniqued()) { + Op->set(To); + return; + } - LLVMContextImpl *pImpl = getType()->getContext().pImpl; + auto &Store = getContext().pImpl->MDNodeSet; - // Remove "this" from the context map. FoldingSet doesn't have to reprofile - // this node to remove it, so we don't care what state the operands are in. - pImpl->MDNodeSet.RemoveNode(this); + // Remove "this" from the context map. + Store.erase(this); + + // Update the operand. + Op->set(To); // If we are dropping an argument to null, we choose to not unique the MDNode // anymore. This commonly occurs during destruction, and uniquing these // brings little reuse. Also, this means we don't need to include - // isFunctionLocal bits in FoldingSetNodeIDs for MDNodes. + // isFunctionLocal bits in the hash for MDNodes. if (!To) { setIsNotUniqued(); return; } - // Now that the node is out of the folding set, get ready to reinsert it. - // First, check to see if another node with the same operands already exists - // in the set. If so, then this node is redundant. - FoldingSetNodeID ID; - Profile(ID); - void *InsertPoint; - if (MDNode *N = pImpl->MDNodeSet.FindNodeOrInsertPos(ID, InsertPoint)) { - replaceAllUsesWith(N); + // Now that the node is out of the table, get ready to reinsert it. First, + // check to see if another node with the same operands already exists in the + // set. If so, then this node is redundant. + SmallVector Vals; + GenericMDNodeInfo::KeyTy Key(this, Vals); + auto I = Store.find_as(Key); + if (I != Store.end()) { + replaceAllUsesWith(*I); destroy(); return; } - // Cache the operand hash. - Hash = ID.ComputeHash(); - // InsertPoint will have been set by the FindNodeOrInsertPos call. - pImpl->MDNodeSet.InsertNode(this, InsertPoint); + this->Hash = Key.Hash; + Store.insert(this); // If this MDValue was previously function-local but no longer is, clear // its function-local flag. diff --git a/test/Linker/Inputs/unique-fwd-decl-b.ll b/test/Linker/Inputs/unique-fwd-decl-b.ll new file mode 100644 index 00000000000..240fbeea34b --- /dev/null +++ b/test/Linker/Inputs/unique-fwd-decl-b.ll @@ -0,0 +1,3 @@ +!b = !{!0} +!0 = metadata !{metadata !1} +!1 = metadata !{} diff --git a/test/Linker/unique-fwd-decl-a.ll b/test/Linker/unique-fwd-decl-a.ll new file mode 100644 index 00000000000..b9c7b2faac5 --- /dev/null +++ b/test/Linker/unique-fwd-decl-a.ll @@ -0,0 +1,9 @@ +; RUN: llvm-link %s %S/Inputs/unique-fwd-decl-b.ll -S -o - | FileCheck %s + +; Test that the arguments of !a and !b get uniqued. +; CHECK: !a = !{!0} +; CHECK: !b = !{!0} + +!a = !{!0} +!0 = metadata !{metadata !1} +!1 = metadata !{}