[llvm-commits] [llvm] r80868 - in /llvm/trunk: include/llvm/Constants.h include/llvm/Metadata.h lib/VMCore/Constants.cpp lib/VMCore/ConstantsContext.h lib/VMCore/LLVMContext.cpp lib/VMCore/LLVMContextImpl.h lib/VMCore/Metadata.cpp unittests/VMCore/MetadataTest.cpp
Devang Patel
dpatel at apple.com
Wed Sep 2 18:39:20 PDT 2009
Author: dpatel
Date: Wed Sep 2 20:39:20 2009
New Revision: 80868
URL: http://llvm.org/viewvc/llvm-project?rev=80868&view=rev
Log:
Now Bitcode reader bug is fixed. Reapply 80839.
Use CallbackVH, instead of WeakVH, to hold MDNode elements.
Use FoldingSetNode to unique MDNodes in a context.
Use CallbackVH hooks to update context's MDNodeSet appropriately.
Modified:
llvm/trunk/include/llvm/Constants.h
llvm/trunk/include/llvm/Metadata.h
llvm/trunk/lib/VMCore/Constants.cpp
llvm/trunk/lib/VMCore/ConstantsContext.h
llvm/trunk/lib/VMCore/LLVMContext.cpp
llvm/trunk/lib/VMCore/LLVMContextImpl.h
llvm/trunk/lib/VMCore/Metadata.cpp
llvm/trunk/unittests/VMCore/MetadataTest.cpp
Modified: llvm/trunk/include/llvm/Constants.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Constants.h?rev=80868&r1=80867&r2=80868&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Constants.h (original)
+++ llvm/trunk/include/llvm/Constants.h Wed Sep 2 20:39:20 2009
@@ -38,7 +38,7 @@
template<class ConstantClass, class TypeClass, class ValType>
struct ConstantCreator;
template<class ConstantClass, class TypeClass>
-struct ConvertConstant;
+struct ConvertConstantType;
//===----------------------------------------------------------------------===//
/// This is the shared class of boolean and integer constants. This class
@@ -559,7 +559,7 @@
class ConstantExpr : public Constant {
friend struct ConstantCreator<ConstantExpr,Type,
std::pair<unsigned, std::vector<Constant*> > >;
- friend struct ConvertConstant<ConstantExpr, Type>;
+ friend struct ConvertConstantType<ConstantExpr, Type>;
protected:
ConstantExpr(const Type *ty, unsigned Opcode, Use *Ops, unsigned NumOps)
Modified: llvm/trunk/include/llvm/Metadata.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Metadata.h?rev=80868&r1=80867&r2=80868&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Metadata.h (original)
+++ llvm/trunk/include/llvm/Metadata.h Wed Sep 2 20:39:20 2009
@@ -19,7 +19,9 @@
#include "llvm/User.h"
#include "llvm/Type.h"
#include "llvm/OperandTraits.h"
+#include "llvm/ADT/FoldingSet.h"
#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/ilist_node.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/ValueHandle.h"
@@ -27,8 +29,6 @@
namespace llvm {
class Constant;
class LLVMContext;
-template<class ConstantClass, class TypeClass, class ValType>
-struct ConstantCreator;
//===----------------------------------------------------------------------===//
// MetadataBase - A base class for MDNode, MDString and NamedMDNode.
@@ -103,14 +103,32 @@
/// MDNode - a tuple of other values.
/// These contain a list of the values that represent the metadata.
/// MDNode is always unnamed.
-class MDNode : public MetadataBase {
+class MDNode : public MetadataBase, public FoldingSetNode {
MDNode(const MDNode &); // DO NOT IMPLEMENT
void *operator new(size_t, unsigned); // DO NOT IMPLEMENT
// getNumOperands - Make this only available for private uses.
unsigned getNumOperands() { return User::getNumOperands(); }
- SmallVector<WeakVH, 4> Node;
- friend struct ConstantCreator<MDNode, Type, std::vector<Value*> >;
+ friend class ElementVH;
+ // Use CallbackVH to hold MDNOde elements.
+ struct ElementVH : public CallbackVH {
+ MDNode *Parent;
+ ElementVH(Value *V, MDNode *P) : CallbackVH(V), Parent(P) {}
+ ~ElementVH() {}
+
+ virtual void deleted() {
+ Parent->replaceElement(this->operator Value*(), 0);
+ }
+
+ virtual void allUsesReplacedWith(Value *NV) {
+ Parent->replaceElement(this->operator Value*(), NV);
+ }
+ };
+ // Replace each instance of F from the element list of this node with T.
+ void replaceElement(Value *F, Value *T);
+
+ SmallVector<ElementVH, 4> Node;
+
protected:
explicit MDNode(LLVMContext &C, Value*const* Vals, unsigned NumVals);
public:
@@ -140,8 +158,8 @@
}
// Element access
- typedef SmallVectorImpl<WeakVH>::const_iterator const_elem_iterator;
- typedef SmallVectorImpl<WeakVH>::iterator elem_iterator;
+ typedef SmallVectorImpl<ElementVH>::const_iterator const_elem_iterator;
+ typedef SmallVectorImpl<ElementVH>::iterator elem_iterator;
/// elem_empty - Return true if MDNode is empty.
bool elem_empty() const { return Node.empty(); }
const_elem_iterator elem_begin() const { return Node.begin(); }
@@ -156,6 +174,10 @@
return false;
}
+ /// Profile - calculate a unique identifier for this MDNode to collapse
+ /// duplicates
+ void Profile(FoldingSetNodeID &ID) const;
+
virtual void replaceUsesOfWithOnConstant(Value *From, Value *To, Use *U) {
llvm_unreachable("This should never be called because MDNodes have no ops");
}
Modified: llvm/trunk/lib/VMCore/Constants.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Constants.cpp?rev=80868&r1=80867&r2=80868&view=diff
==============================================================================
--- llvm/trunk/lib/VMCore/Constants.cpp (original)
+++ llvm/trunk/lib/VMCore/Constants.cpp Wed Sep 2 20:39:20 2009
@@ -1880,7 +1880,7 @@
pImpl->ArrayConstants.InsertOrGetItem(Lookup, Exists);
if (Exists) {
- Replacement = cast<Constant>(I->second);
+ Replacement = I->second;
} else {
// Okay, the new shape doesn't exist in the system yet. Instead of
// creating a new constant array, inserting it, replaceallusesof'ing the
@@ -1967,7 +1967,7 @@
pImpl->StructConstants.InsertOrGetItem(Lookup, Exists);
if (Exists) {
- Replacement = cast<Constant>(I->second);
+ Replacement = I->second;
} else {
// Okay, the new shape doesn't exist in the system yet. Instead of
// creating a new constant struct, inserting it, replaceallusesof'ing the
Modified: llvm/trunk/lib/VMCore/ConstantsContext.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/ConstantsContext.h?rev=80868&r1=80867&r2=80868&view=diff
==============================================================================
--- llvm/trunk/lib/VMCore/ConstantsContext.h (original)
+++ llvm/trunk/lib/VMCore/ConstantsContext.h Wed Sep 2 20:39:20 2009
@@ -16,7 +16,6 @@
#define LLVM_CONSTANTSCONTEXT_H
#include "llvm/Instructions.h"
-#include "llvm/Metadata.h"
#include "llvm/Operator.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/ErrorHandling.h"
@@ -341,7 +340,7 @@
};
template<class ConstantClass, class TypeClass>
-struct ConvertConstant {
+struct ConvertConstantType {
static void convert(ConstantClass *OldC, const TypeClass *NewTy) {
llvm_unreachable("This type cannot be converted!");
}
@@ -392,7 +391,7 @@
};
template<>
-struct ConvertConstant<ConstantExpr, Type> {
+struct ConvertConstantType<ConstantExpr, Type> {
static void convert(ConstantExpr *OldC, const Type *NewTy) {
Constant *New;
switch (OldC->getOpcode()) {
@@ -445,14 +444,7 @@
};
template<>
-struct ConstantCreator<MDNode, Type, std::vector<Value*> > {
- static MDNode *create(const Type* Ty, const std::vector<Value*> &V) {
- return new MDNode(Ty->getContext(), &V[0], V.size());
- }
-};
-
-template<>
-struct ConvertConstant<ConstantVector, VectorType> {
+struct ConvertConstantType<ConstantVector, VectorType> {
static void convert(ConstantVector *OldC, const VectorType *NewTy) {
// Make everyone now use a constant of the new type...
std::vector<Constant*> C;
@@ -466,7 +458,7 @@
};
template<>
-struct ConvertConstant<ConstantAggregateZero, Type> {
+struct ConvertConstantType<ConstantAggregateZero, Type> {
static void convert(ConstantAggregateZero *OldC, const Type *NewTy) {
// Make everyone now use a constant of the new type...
Constant *New = ConstantAggregateZero::get(NewTy);
@@ -477,7 +469,7 @@
};
template<>
-struct ConvertConstant<ConstantArray, ArrayType> {
+struct ConvertConstantType<ConstantArray, ArrayType> {
static void convert(ConstantArray *OldC, const ArrayType *NewTy) {
// Make everyone now use a constant of the new type...
std::vector<Constant*> C;
@@ -491,7 +483,7 @@
};
template<>
-struct ConvertConstant<ConstantStruct, StructType> {
+struct ConvertConstantType<ConstantStruct, StructType> {
static void convert(ConstantStruct *OldC, const StructType *NewTy) {
// Make everyone now use a constant of the new type...
std::vector<Constant*> C;
@@ -514,7 +506,7 @@
};
template<>
-struct ConvertConstant<ConstantPointerNull, PointerType> {
+struct ConvertConstantType<ConstantPointerNull, PointerType> {
static void convert(ConstantPointerNull *OldC, const PointerType *NewTy) {
// Make everyone now use a constant of the new type...
Constant *New = ConstantPointerNull::get(NewTy);
@@ -533,7 +525,7 @@
};
template<>
-struct ConvertConstant<UndefValue, Type> {
+struct ConvertConstantType<UndefValue, Type> {
static void convert(UndefValue *OldC, const Type *NewTy) {
// Make everyone now use a constant of the new type.
Constant *New = UndefValue::get(NewTy);
@@ -548,8 +540,8 @@
class ValueMap : public AbstractTypeUser {
public:
typedef std::pair<const Type*, ValType> MapKey;
- typedef std::map<MapKey, Value *> MapTy;
- typedef std::map<Value*, typename MapTy::iterator> InverseMapTy;
+ typedef std::map<MapKey, Constant *> MapTy;
+ typedef std::map<Constant*, typename MapTy::iterator> InverseMapTy;
typedef std::map<const Type*, typename MapTy::iterator> AbstractTypeMapTy;
private:
/// Map - This is the main map from the element descriptor to the Constants.
@@ -767,7 +759,8 @@
// leaving will remove() itself, causing the AbstractTypeMapEntry to be
// eliminated eventually.
do {
- ConvertConstant<ConstantClass, TypeClass>::convert(
+ ConvertConstantType<ConstantClass,
+ TypeClass>::convert(
static_cast<ConstantClass *>(I->second->second),
cast<TypeClass>(NewTy));
Modified: llvm/trunk/lib/VMCore/LLVMContext.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/LLVMContext.cpp?rev=80868&r1=80867&r2=80868&view=diff
==============================================================================
--- llvm/trunk/lib/VMCore/LLVMContext.cpp (original)
+++ llvm/trunk/lib/VMCore/LLVMContext.cpp Wed Sep 2 20:39:20 2009
@@ -13,6 +13,7 @@
//===----------------------------------------------------------------------===//
#include "llvm/LLVMContext.h"
+#include "llvm/Metadata.h"
#include "llvm/Constants.h"
#include "llvm/Instruction.h"
#include "llvm/Support/ManagedStatic.h"
@@ -48,10 +49,10 @@
bool Changed = false;
while (1) {
- for (SmallPtrSet<const MDNode *, 8>::iterator
- I = pImpl->MDNodes.begin(),
- E = pImpl->MDNodes.end(); I != E; ++I) {
- const MDNode *N = cast<MDNode>(*I);
+ for (FoldingSet<MDNode>::iterator
+ I = pImpl->MDNodeSet.begin(),
+ E = pImpl->MDNodeSet.end(); I != E; ++I) {
+ const MDNode *N = &(*I);
if (N->use_empty())
DeadMDNodes.push_back(N);
}
Modified: llvm/trunk/lib/VMCore/LLVMContextImpl.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/LLVMContextImpl.h?rev=80868&r1=80867&r2=80868&view=diff
==============================================================================
--- llvm/trunk/lib/VMCore/LLVMContextImpl.h (original)
+++ llvm/trunk/lib/VMCore/LLVMContextImpl.h Wed Sep 2 20:39:20 2009
@@ -19,6 +19,7 @@
#include "LeaksContext.h"
#include "TypesContext.h"
#include "llvm/LLVMContext.h"
+#include "llvm/Metadata.h"
#include "llvm/Constants.h"
#include "llvm/DerivedTypes.h"
#include "llvm/System/Mutex.h"
@@ -106,10 +107,12 @@
StringMap<MDString*> MDStringCache;
+ FoldingSet<MDNode> MDNodeSet;
+
ValueMap<char, Type, ConstantAggregateZero> AggZeroConstants;
SmallPtrSet<const MDNode *, 8> MDNodes;
-
+
typedef ValueMap<std::vector<Constant*>, ArrayType,
ConstantArray, true /*largekey*/> ArrayConstantsTy;
ArrayConstantsTy ArrayConstants;
@@ -199,7 +202,6 @@
ArrayConstants.freeConstants();
StructConstants.freeConstants();
VectorConstants.freeConstants();
-
AggZeroConstants.freeConstants();
NullPtrConstants.freeConstants();
UndefValueConstants.freeConstants();
Modified: llvm/trunk/lib/VMCore/Metadata.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Metadata.cpp?rev=80868&r1=80867&r2=80868&view=diff
==============================================================================
--- llvm/trunk/lib/VMCore/Metadata.cpp (original)
+++ llvm/trunk/lib/VMCore/Metadata.cpp Wed Sep 2 20:39:20 2009
@@ -72,18 +72,37 @@
// Only record metadata uses.
if (MetadataBase *MB = dyn_cast_or_null<MetadataBase>(Vals[i]))
OperandList[NumOperands++] = MB;
- Node.push_back(WeakVH(Vals[i]));
+ Node.push_back(ElementVH(Vals[i], this));
}
}
+void MDNode::Profile(FoldingSetNodeID &ID) const {
+ for (const_elem_iterator I = elem_begin(), E = elem_end(); I != E; ++I)
+ ID.AddPointer(*I);
+}
+
MDNode *MDNode::get(LLVMContext &Context, Value*const* Vals, unsigned NumVals) {
- std::vector<Value*> V;
- V.reserve(NumVals);
- for (unsigned i = 0; i < NumVals; ++i)
- V.push_back(Vals[i]);
+ LLVMContextImpl *pImpl = Context.pImpl;
+ FoldingSetNodeID ID;
+ for (unsigned i = 0; i != NumVals; ++i)
+ ID.AddPointer(Vals[i]);
+
+ pImpl->ConstantsLock.reader_acquire();
+ void *InsertPoint;
+ MDNode *N = pImpl->MDNodeSet.FindNodeOrInsertPos(ID, InsertPoint);
+ pImpl->ConstantsLock.reader_release();
- // FIXME : Avoid creating duplicate node.
- return new MDNode(Context, &V[0], V.size());
+ if (!N) {
+ sys::SmartScopedWriter<true> Writer(pImpl->ConstantsLock);
+ N = pImpl->MDNodeSet.FindNodeOrInsertPos(ID, InsertPoint);
+ if (!N) {
+ // InsertPoint will have been set by the FindNodeOrInsertPos call.
+ N = new MDNode(Context, Vals, NumVals);
+ pImpl->MDNodeSet.InsertNode(N, InsertPoint);
+ }
+ }
+
+ return N;
}
/// dropAllReferences - Remove all uses and clear node vector.
@@ -93,10 +112,73 @@
}
MDNode::~MDNode() {
- getType()->getContext().pImpl->MDNodes.erase(this);
+ getType()->getContext().pImpl->MDNodeSet.RemoveNode(this);
dropAllReferences();
}
+// Replace value from this node's element list.
+void MDNode::replaceElement(Value *From, Value *To) {
+ if (From == To || !getType())
+ return;
+ LLVMContext &Context = getType()->getContext();
+ LLVMContextImpl *pImpl = Context.pImpl;
+
+ // Find value. This is a linear search, do something if it consumes
+ // lot of time. It is possible that to have multiple instances of
+ // From in this MDNode's element list.
+ SmallVector<unsigned, 4> Indexes;
+ unsigned Index = 0;
+ for (SmallVector<ElementVH, 4>::iterator I = Node.begin(),
+ E = Node.end(); I != E; ++I, ++Index) {
+ Value *V = *I;
+ if (V && V == From)
+ Indexes.push_back(Index);
+ }
+
+ if (Indexes.empty())
+ return;
+
+ // Remove "this" from the context map.
+ {
+ sys::SmartScopedWriter<true> Writer(pImpl->ConstantsLock);
+ pImpl->MDNodeSet.RemoveNode(this);
+ }
+
+ // Replace From element(s) in place.
+ for (SmallVector<unsigned, 4>::iterator I = Indexes.begin(), E = Indexes.end();
+ I != E; ++I) {
+ unsigned Index = *I;
+ Node[Index] = ElementVH(To, this);
+ }
+
+ // Insert updated "this" into the context's folding node set.
+ // If a node with same element list already exist then before inserting
+ // updated "this" into the folding node set, replace all uses of existing
+ // node with updated "this" node.
+ FoldingSetNodeID ID;
+ Profile(ID);
+ pImpl->ConstantsLock.reader_acquire();
+ void *InsertPoint;
+ MDNode *N = pImpl->MDNodeSet.FindNodeOrInsertPos(ID, InsertPoint);
+ pImpl->ConstantsLock.reader_release();
+
+ if (N) {
+ N->replaceAllUsesWith(this);
+ delete N;
+ N = 0;
+ }
+
+ {
+ sys::SmartScopedWriter<true> Writer(pImpl->ConstantsLock);
+ N = pImpl->MDNodeSet.FindNodeOrInsertPos(ID, InsertPoint);
+ if (!N) {
+ // InsertPoint will have been set by the FindNodeOrInsertPos call.
+ N = this;
+ pImpl->MDNodeSet.InsertNode(N, InsertPoint);
+ }
+ }
+}
+
//===----------------------------------------------------------------------===//
//NamedMDNode implementation
//
Modified: llvm/trunk/unittests/VMCore/MetadataTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/VMCore/MetadataTest.cpp?rev=80868&r1=80867&r2=80868&view=diff
==============================================================================
--- llvm/trunk/unittests/VMCore/MetadataTest.cpp (original)
+++ llvm/trunk/unittests/VMCore/MetadataTest.cpp Wed Sep 2 20:39:20 2009
@@ -85,7 +85,7 @@
MDNode *n2 = MDNode::get(Context, &c1, 1);
MDNode *n3 = MDNode::get(Context, &V[0], 3);
EXPECT_NE(n1, n2);
- // FIXME: Enable uniqueness test. EXPECT_EQ(n1, n3);
+ EXPECT_EQ(n1, n3);
EXPECT_EQ(3u, n1->getNumElements());
EXPECT_EQ(s1, n1->getElement(0));
More information about the llvm-commits
mailing list