From ec39f095f5abaf1ec90d7c6c46454032cda36e1c Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Tue, 30 Mar 2010 23:03:27 +0000 Subject: [PATCH] Fix a major source of compile-time slowness at -O0 -g by optimizing the storage of !dbg metadata kinds in the instruction themselves. The on-the-side hash table works great for metadata that not-all instructions get, or for metadata that only exists when optimizing. But when compile-time is everything, it isn't great. I'm not super thrilled with the fact that this plops a TrackingVH in Instruction, because it grows it by 3 words. I'm investigating alternatives, but this should be a step in the right direction in any case. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@99957 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Instruction.h | 12 ++++++-- include/llvm/LLVMContext.h | 6 ++++ include/llvm/Support/ValueHandle.h | 2 +- lib/VMCore/Instruction.cpp | 4 +-- lib/VMCore/LLVMContext.cpp | 6 +++- lib/VMCore/Metadata.cpp | 47 +++++++++++++++++++++++------- 6 files changed, 60 insertions(+), 17 deletions(-) diff --git a/include/llvm/Instruction.h b/include/llvm/Instruction.h index cf9dc4456fa..41841126b31 100644 --- a/include/llvm/Instruction.h +++ b/include/llvm/Instruction.h @@ -17,6 +17,7 @@ #include "llvm/User.h" #include "llvm/ADT/ilist_node.h" +#include "llvm/Support/ValueHandle.h" namespace llvm { @@ -31,6 +32,7 @@ class Instruction : public User, public ilist_node { Instruction(const Instruction &); // Do not implement BasicBlock *Parent; + TrackingVH DbgInfo; // 'dbg' Metadata cache. enum { /// HasMetadataBit - This is a bit stored in the SubClassData field which @@ -123,7 +125,7 @@ public: /// hasMetadata() - Return true if this instruction has any metadata attached /// to it. bool hasMetadata() const { - return (getSubclassDataFromValue() & HasMetadataBit) != 0; + return DbgInfo != 0 || hasMetadataHashEntry(); } /// getMetadata - Get the metadata of given kind attached to this Instruction. @@ -155,6 +157,12 @@ public: void setMetadata(const char *Kind, MDNode *Node); private: + /// hasMetadataHashEntry - Return true if we have an entry in the on-the-side + /// metadata hash. + bool hasMetadataHashEntry() const { + return (getSubclassDataFromValue() & HasMetadataBit) != 0; + } + // These are all implemented in Metadata.cpp. MDNode *getMetadataImpl(unsigned KindID) const; MDNode *getMetadataImpl(const char *Kind) const; @@ -315,7 +323,7 @@ private: return Value::getSubclassDataFromValue(); } - void setHasMetadata(bool V) { + void setHasMetadataHashEntry(bool V) { setValueSubclassData((getSubclassDataFromValue() & ~HasMetadataBit) | (V ? HasMetadataBit : 0)); } diff --git a/include/llvm/LLVMContext.h b/include/llvm/LLVMContext.h index 6d36d5eb1e3..ea7f4a2d8e9 100644 --- a/include/llvm/LLVMContext.h +++ b/include/llvm/LLVMContext.h @@ -36,6 +36,12 @@ public: LLVMContext(); ~LLVMContext(); + // Pinned metadata names, which always have the same value. This is a + // compile-time performance optimization, not a correctness optimization. + enum { + MD_dbg = 1 // "dbg" -> 1. + }; + /// getMDKindID - Return a unique non-zero ID for the specified metadata kind. /// This ID is uniqued across modules in the current LLVMContext. unsigned getMDKindID(StringRef Name) const; diff --git a/include/llvm/Support/ValueHandle.h b/include/llvm/Support/ValueHandle.h index 82c3caeb3ad..0e61d093b9d 100644 --- a/include/llvm/Support/ValueHandle.h +++ b/include/llvm/Support/ValueHandle.h @@ -302,7 +302,7 @@ class TrackingVH : public ValueHandleBase { ValueTy *getValPtr() const { CheckValidity(); - return static_cast(ValueHandleBase::getValPtr()); + return (ValueTy*)ValueHandleBase::getValPtr(); } void setValPtr(ValueTy *P) { CheckValidity(); diff --git a/lib/VMCore/Instruction.cpp b/lib/VMCore/Instruction.cpp index a37fe070bda..ee75a9783ee 100644 --- a/lib/VMCore/Instruction.cpp +++ b/lib/VMCore/Instruction.cpp @@ -22,7 +22,7 @@ using namespace llvm; Instruction::Instruction(const Type *ty, unsigned it, Use *Ops, unsigned NumOps, Instruction *InsertBefore) - : User(ty, Value::InstructionVal + it, Ops, NumOps), Parent(0) { + : User(ty, Value::InstructionVal + it, Ops, NumOps), Parent(0), DbgInfo(0) { // Make sure that we get added to a basicblock LeakDetector::addGarbageObject(this); @@ -36,7 +36,7 @@ Instruction::Instruction(const Type *ty, unsigned it, Use *Ops, unsigned NumOps, Instruction::Instruction(const Type *ty, unsigned it, Use *Ops, unsigned NumOps, BasicBlock *InsertAtEnd) - : User(ty, Value::InstructionVal + it, Ops, NumOps), Parent(0) { + : User(ty, Value::InstructionVal + it, Ops, NumOps), Parent(0), DbgInfo(0) { // Make sure that we get added to a basicblock LeakDetector::addGarbageObject(this); diff --git a/lib/VMCore/LLVMContext.cpp b/lib/VMCore/LLVMContext.cpp index f9d3fe0021d..2a870ec6cfd 100644 --- a/lib/VMCore/LLVMContext.cpp +++ b/lib/VMCore/LLVMContext.cpp @@ -26,7 +26,11 @@ LLVMContext& llvm::getGlobalContext() { return *GlobalContext; } -LLVMContext::LLVMContext() : pImpl(new LLVMContextImpl(*this)) { } +LLVMContext::LLVMContext() : pImpl(new LLVMContextImpl(*this)) { + // Create the first metadata kind, which is always 'dbg'. + unsigned DbgID = getMDKindID("dbg"); + assert(DbgID == MD_dbg && "dbg kind id drifted"); (void)DbgID; +} LLVMContext::~LLVMContext() { delete pImpl; } diff --git a/lib/VMCore/Metadata.cpp b/lib/VMCore/Metadata.cpp index 793c8744860..77926f9af34 100644 --- a/lib/VMCore/Metadata.cpp +++ b/lib/VMCore/Metadata.cpp @@ -430,12 +430,19 @@ MDNode *Instruction::getMetadataImpl(const char *Kind) const { void Instruction::setMetadata(unsigned KindID, MDNode *Node) { if (Node == 0 && !hasMetadata()) return; + // Handle 'dbg' as a special case since it is not stored in the hash table. + if (KindID == LLVMContext::MD_dbg) { + DbgInfo = Node; + return; + } + // Handle the case when we're adding/updating metadata on an instruction. if (Node) { LLVMContextImpl::MDMapTy &Info = getContext().pImpl->MetadataStore[this]; - assert(!Info.empty() == hasMetadata() && "HasMetadata bit is wonked"); + assert(!Info.empty() == hasMetadataHashEntry() && + "HasMetadata bit is wonked"); if (Info.empty()) { - setHasMetadata(true); + setHasMetadataHashEntry(true); } else { // Handle replacement of an existing value. for (unsigned i = 0, e = Info.size(); i != e; ++i) @@ -451,18 +458,19 @@ void Instruction::setMetadata(unsigned KindID, MDNode *Node) { } // Otherwise, we're removing metadata from an instruction. - assert(hasMetadata() && getContext().pImpl->MetadataStore.count(this) && + assert(hasMetadataHashEntry() && + getContext().pImpl->MetadataStore.count(this) && "HasMetadata bit out of date!"); LLVMContextImpl::MDMapTy &Info = getContext().pImpl->MetadataStore[this]; // Common case is removing the only entry. if (Info.size() == 1 && Info[0].first == KindID) { getContext().pImpl->MetadataStore.erase(this); - setHasMetadata(false); + setHasMetadataHashEntry(false); return; } - // Handle replacement of an existing value. + // Handle removal of an existing value. for (unsigned i = 0, e = Info.size(); i != e; ++i) if (Info[i].first == KindID) { Info[i] = Info.back(); @@ -474,8 +482,14 @@ void Instruction::setMetadata(unsigned KindID, MDNode *Node) { } MDNode *Instruction::getMetadataImpl(unsigned KindID) const { + // Handle 'dbg' as a special case since it is not stored in the hash table. + if (KindID == LLVMContext::MD_dbg) + return DbgInfo; + + if (!hasMetadataHashEntry()) return 0; + LLVMContextImpl::MDMapTy &Info = getContext().pImpl->MetadataStore[this]; - assert(hasMetadata() && !Info.empty() && "Shouldn't have called this"); + assert(!Info.empty() && "bit out of sync with hash table"); for (LLVMContextImpl::MDMapTy::iterator I = Info.begin(), E = Info.end(); I != E; ++I) @@ -485,14 +499,22 @@ MDNode *Instruction::getMetadataImpl(unsigned KindID) const { } void Instruction::getAllMetadataImpl(SmallVectorImpl > &Result)const { - assert(hasMetadata() && getContext().pImpl->MetadataStore.count(this) && + MDNode*> > &Result) const { + Result.clear(); + + // Handle 'dbg' as a special case since it is not stored in the hash table. + if (DbgInfo) { + Result.push_back(std::make_pair((unsigned)LLVMContext::MD_dbg, DbgInfo)); + if (!hasMetadataHashEntry()) return; + } + + assert(hasMetadataHashEntry() && + getContext().pImpl->MetadataStore.count(this) && "Shouldn't have called this"); const LLVMContextImpl::MDMapTy &Info = getContext().pImpl->MetadataStore.find(this)->second; assert(!Info.empty() && "Shouldn't have called this"); - Result.clear(); Result.append(Info.begin(), Info.end()); // Sort the resulting array so it is stable. @@ -503,7 +525,10 @@ void Instruction::getAllMetadataImpl(SmallVectorImplMetadataStore.erase(this); - setHasMetadata(false); + DbgInfo = 0; + if (hasMetadataHashEntry()) { + getContext().pImpl->MetadataStore.erase(this); + setHasMetadataHashEntry(false); + } }