[llvm] r222191 - IR: Simplify uniquing for MDNode
Duncan P. N. Exon Smith
dexonsmith at apple.com
Mon Nov 17 15:28:21 PST 2014
Author: dexonsmith
Date: Mon Nov 17 17:28:21 2014
New Revision: 222191
URL: http://llvm.org/viewvc/llvm-project?rev=222191&view=rev
Log:
IR: Simplify uniquing for MDNode
Change uniquing from a `FoldingSet` to a `DenseSet` with custom
`DenseMapInfo`. Unfortunately, this doesn't save any memory, since
`DenseSet<T>` is a simple wrapper for `DenseMap<T, char>`, but I'll come
back to fix that later.
I used the name `GenericDenseMapInfo` to the custom `DenseMapInfo` since
I'll be splitting `MDNode` into two classes soon: `MDNodeFwdDecl` for
temporaries, and `GenericMDNode` for everything else.
I also added a non-debug-info reduced version of a type-uniquing test
that started failing on an earlier draft of this patch.
Part of PR21532.
Added:
llvm/trunk/test/Linker/Inputs/unique-fwd-decl-b.ll
llvm/trunk/test/Linker/unique-fwd-decl-a.ll
Modified:
llvm/trunk/include/llvm/IR/Metadata.h
llvm/trunk/lib/IR/LLVMContextImpl.cpp
llvm/trunk/lib/IR/LLVMContextImpl.h
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=222191&r1=222190&r2=222191&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/Metadata.h (original)
+++ llvm/trunk/include/llvm/IR/Metadata.h Mon Nov 17 17:28:21 2014
@@ -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<MDNode>;
/// \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) {
Modified: llvm/trunk/lib/IR/LLVMContextImpl.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LLVMContextImpl.cpp?rev=222191&r1=222190&r2=222191&view=diff
==============================================================================
--- llvm/trunk/lib/IR/LLVMContextImpl.cpp (original)
+++ llvm/trunk/lib/IR/LLVMContextImpl.cpp Mon Nov 17 17:28:21 2014
@@ -124,9 +124,7 @@ LLVMContextImpl::~LLVMContextImpl() {
// and the NonUniquedMDNodes sets, so copy the values out first.
SmallVector<MDNode*, 8> MDNodes;
MDNodes.reserve(MDNodeSet.size() + NonUniquedMDNodes.size());
- for (FoldingSetIterator<MDNode> 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<MDNode *>::iterator I = MDNodes.begin(),
E = MDNodes.end(); I != E; ++I)
Modified: llvm/trunk/lib/IR/LLVMContextImpl.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LLVMContextImpl.h?rev=222191&r1=222190&r2=222191&view=diff
==============================================================================
--- llvm/trunk/lib/IR/LLVMContextImpl.h (original)
+++ llvm/trunk/lib/IR/LLVMContextImpl.h Mon Nov 17 17:28:21 2014
@@ -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<MDNode> : DefaultFoldingSetTrait<MDNode> {
- 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<Value *> Ops;
+ unsigned Hash;
+
+ KeyTy(ArrayRef<Value *> Ops)
+ : Ops(Ops), Hash(hash_combine_range(Ops.begin(), Ops.end())) {}
+
+ KeyTy(MDNode *N, SmallVectorImpl<Value *> &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());
+ }
- // If they match we have to compare the operands.
- X.Profile(TempID);
- return TempID == ID;
+ 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<MDNode *>::getEmptyKey();
+ }
+ static inline MDNode *getTombstoneKey() {
+ return DenseMapInfo<MDNode *>::getTombstoneKey();
+ }
+ static unsigned getHashValue(const KeyTy &Key) { return Key.Hash; }
+ static unsigned getHashValue(const MDNode *U) {
+ return U->getHash();
}
- static unsigned ComputeHash(const MDNode &X, FoldingSetNodeID &) {
- return X.Hash; // Return cached hash.
+ 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<MDString> MDStringCache;
- FoldingSet<MDNode> MDNodeSet;
+ DenseSet<MDNode *, 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
Modified: llvm/trunk/lib/IR/Metadata.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Metadata.cpp?rev=222191&r1=222190&r2=222191&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Metadata.cpp (original)
+++ llvm/trunk/lib/IR/Metadata.cpp Mon Nov 17 17:28:21 2014
@@ -119,7 +119,7 @@ void MDNode::replaceOperandWith(unsigned
}
MDNode::MDNode(LLVMContext &C, ArrayRef<Value *> 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 *
MDNode *MDNode::getMDNode(LLVMContext &Context, ArrayRef<Value*> 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 &C
// 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
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) co
return *getOperandPtr(const_cast<MDNode*>(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(MDNodeOperan
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.
+ Store.erase(this);
- // 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);
+ // 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<Value *, 8> 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.
Added: llvm/trunk/test/Linker/Inputs/unique-fwd-decl-b.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/Inputs/unique-fwd-decl-b.ll?rev=222191&view=auto
==============================================================================
--- llvm/trunk/test/Linker/Inputs/unique-fwd-decl-b.ll (added)
+++ llvm/trunk/test/Linker/Inputs/unique-fwd-decl-b.ll Mon Nov 17 17:28:21 2014
@@ -0,0 +1,3 @@
+!b = !{!0}
+!0 = metadata !{metadata !1}
+!1 = metadata !{}
Added: llvm/trunk/test/Linker/unique-fwd-decl-a.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/unique-fwd-decl-a.ll?rev=222191&view=auto
==============================================================================
--- llvm/trunk/test/Linker/unique-fwd-decl-a.ll (added)
+++ llvm/trunk/test/Linker/unique-fwd-decl-a.ll Mon Nov 17 17:28:21 2014
@@ -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 !{}
More information about the llvm-commits
mailing list