From c3661decc3e6b38173a958c2518dafbce7d6585a Mon Sep 17 00:00:00 2001 From: Ted Kremenek Date: Wed, 7 Oct 2009 00:42:52 +0000 Subject: [PATCH] Change ExplodedNode to have its NodeGroups all BumpPtrAllocated, avoiding malloc() traffic when adding successors/predecessors to a node. This was done by introducing BumpVector, which is essentially SmallVector with all memory being BumpPtrAllocated (this can certainly be cleaned up or moved into llvm/ADT). This change yields a 1.8% speed increase when running the analyzer (with -analyzer-store=region) on a small benchmark file. llvm-svn: 83439 --- .../Analysis/PathSensitive/ExplodedGraph.h | 31 +-- .../clang/Analysis/Support/BumpVector.h | 200 ++++++++++++++++++ clang/lib/Analysis/BugReporter.cpp | 2 +- clang/lib/Analysis/ExplodedGraph.cpp | 61 +++--- clang/lib/Analysis/GRCoreEngine.cpp | 16 +- 5 files changed, 253 insertions(+), 57 deletions(-) create mode 100644 clang/include/clang/Analysis/Support/BumpVector.h diff --git a/clang/include/clang/Analysis/PathSensitive/ExplodedGraph.h b/clang/include/clang/Analysis/PathSensitive/ExplodedGraph.h index d8659c230202..a7bbdf939f87 100644 --- a/clang/include/clang/Analysis/PathSensitive/ExplodedGraph.h +++ b/clang/include/clang/Analysis/PathSensitive/ExplodedGraph.h @@ -26,12 +26,14 @@ #include "llvm/ADT/GraphTraits.h" #include "llvm/ADT/DepthFirstIterator.h" #include "llvm/Support/Casting.h" +#include "clang/Analysis/Support/BumpVector.h" namespace clang { class GRState; class CFG; class ASTContext; +class ExplodedGraph; //===----------------------------------------------------------------------===// // ExplodedGraph "implementation" classes. These classes are not typed to @@ -68,20 +70,18 @@ class ExplodedNode : public llvm::FoldingSetNode { public: NodeGroup() : P(0) {} - ~NodeGroup(); + ExplodedNode **begin() const; - ExplodedNode** begin() const; - - ExplodedNode** end() const; + ExplodedNode **end() const; unsigned size() const; - bool empty() const { return size() == 0; } + bool empty() const { return (P & ~Mask) == 0; } - void addNode(ExplodedNode* N); + void addNode(ExplodedNode* N, ExplodedGraph &G); void setFlag() { - assert (P == 0); + assert(P == 0); P = AuxFlag; } @@ -131,7 +131,10 @@ public: const T* getLocationAs() const { return llvm::dyn_cast(&Location); } static void Profile(llvm::FoldingSetNodeID &ID, - const ProgramPoint& Loc, const GRState* state); + const ProgramPoint& Loc, const GRState* state) { + ID.Add(Loc); + ID.AddPointer(state); + } void Profile(llvm::FoldingSetNodeID& ID) const { Profile(ID, getLocation(), getState()); @@ -139,7 +142,7 @@ public: /// addPredeccessor - Adds a predecessor to the current node, and /// in tandem add this node as a successor of the other node. - void addPredecessor(ExplodedNode* V); + void addPredecessor(ExplodedNode* V, ExplodedGraph &G); unsigned succ_size() const { return Succs.size(); } unsigned pred_size() const { return Preds.size(); } @@ -229,8 +232,9 @@ protected: /// Nodes - The nodes in the graph. llvm::FoldingSet Nodes; - /// Allocator - BumpPtrAllocator to create nodes. - llvm::BumpPtrAllocator Allocator; + /// BVC - Allocator and context for allocating nodes and their predecessor + /// and successor groups. + BumpVectorContext BVC; /// Ctx - The ASTContext used to "interpret" CodeDecl. ASTContext& Ctx; @@ -265,7 +269,7 @@ public: ExplodedGraph(ASTContext& ctx) : Ctx(ctx), NumNodes(0) {} - virtual ~ExplodedGraph() {} + ~ExplodedGraph() {} unsigned num_roots() const { return Roots.size(); } unsigned num_eops() const { return EndNodes.size(); } @@ -307,7 +311,8 @@ public: const_eop_iterator eop_end() const { return EndNodes.end(); } - llvm::BumpPtrAllocator& getAllocator() { return Allocator; } + llvm::BumpPtrAllocator & getAllocator() { return BVC.getAllocator(); } + BumpVectorContext &getNodeAllocator() { return BVC; } ASTContext& getContext() { return Ctx; } diff --git a/clang/include/clang/Analysis/Support/BumpVector.h b/clang/include/clang/Analysis/Support/BumpVector.h new file mode 100644 index 000000000000..d1c6d4ba4a08 --- /dev/null +++ b/clang/include/clang/Analysis/Support/BumpVector.h @@ -0,0 +1,200 @@ +//===-- BumpVector.h - Vector-like ADT that uses bump allocation --*- C++ -*-=// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This file provides BumpVector, a vector-like ADT whose contents are +// allocated from a BumpPtrAllocator. +// +//===----------------------------------------------------------------------===// + +// FIXME: Most of this is copy-and-paste from SmallVector.h. We can +// refactor this core logic into something common that is shared between +// the two. The main thing that is different is the allocation strategy. + +#ifndef LLVM_CLANG_BUMP_VECTOR +#define LLVM_CLANG_BUMP_VECTOR + +#include "llvm/Support/type_traits.h" +#include "llvm/Support/Allocator.h" +#include + +namespace clang { + +class BumpVectorContext { + llvm::BumpPtrAllocator Alloc; +public: + llvm::BumpPtrAllocator &getAllocator() { return Alloc; } +}; + +template +class BumpVector { + T *Begin, *End, *Capacity; +public: + // Default ctor - Initialize to empty. + explicit BumpVector(BumpVectorContext &C, unsigned N) + : Begin(NULL), End(NULL), Capacity(NULL) { + reserve(C, N); + } + + ~BumpVector() { + if (llvm::is_class::value) { + // Destroy the constructed elements in the vector. + destroy_range(Begin, End); + } + } + + typedef size_t size_type; + typedef ptrdiff_t difference_type; + typedef T value_type; + typedef T* iterator; + typedef const T* const_iterator; + + typedef std::reverse_iterator const_reverse_iterator; + typedef std::reverse_iterator reverse_iterator; + + typedef T& reference; + typedef const T& const_reference; + typedef T* pointer; + typedef const T* const_pointer; + + // forward iterator creation methods. + iterator begin() { return Begin; } + const_iterator begin() const { return Begin; } + iterator end() { return End; } + const_iterator end() const { return End; } + + // reverse iterator creation methods. + reverse_iterator rbegin() { return reverse_iterator(end()); } + const_reverse_iterator rbegin() const{ return const_reverse_iterator(end()); } + reverse_iterator rend() { return reverse_iterator(begin()); } + const_reverse_iterator rend() const { return const_reverse_iterator(begin());} + + bool empty() const { return Begin == End; } + size_type size() const { return End-Begin; } + + reference operator[](unsigned idx) { + assert(Begin + idx < End); + return Begin[idx]; + } + const_reference operator[](unsigned idx) const { + assert(Begin + idx < End); + return Begin[idx]; + } + + reference front() { + return begin()[0]; + } + const_reference front() const { + return begin()[0]; + } + + reference back() { + return end()[-1]; + } + const_reference back() const { + return end()[-1]; + } + + void pop_back() { + --End; + End->~T(); + } + + T pop_back_val() { + T Result = back(); + pop_back(); + return Result; + } + + void clear() { + if (llvm::is_class::value) { + destroy_range(Begin, End); + } + End = Begin; + } + + /// data - Return a pointer to the vector's buffer, even if empty(). + pointer data() { + return pointer(Begin); + } + + /// data - Return a pointer to the vector's buffer, even if empty(). + const_pointer data() const { + return const_pointer(Begin); + } + + void push_back(const_reference Elt, BumpVectorContext &C) { + if (End < Capacity) { + Retry: + new (End) T(Elt); + ++End; + return; + } + grow(C); + goto Retry; + } + + void reserve(BumpVectorContext &C, unsigned N) { + if (unsigned(Capacity-Begin) < N) + grow(C, N); + } + + /// capacity - Return the total number of elements in the currently allocated + /// buffer. + size_t capacity() const { return Capacity - Begin; } + +private: + /// grow - double the size of the allocated memory, guaranteeing space for at + /// least one more element or MinSize if specified. + void grow(BumpVectorContext &C, size_type MinSize = 0); + + void construct_range(T *S, T *E, const T &Elt) { + for (; S != E; ++S) + new (S) T(Elt); + } + + void destroy_range(T *S, T *E) { + while (S != E) { + --E; + E->~T(); + } + } +}; + +// Define this out-of-line to dissuade the C++ compiler from inlining it. +template +void BumpVector::grow(BumpVectorContext &C, size_t MinSize) { + size_t CurCapacity = Capacity-Begin; + size_t CurSize = size(); + size_t NewCapacity = 2*CurCapacity; + if (NewCapacity < MinSize) + NewCapacity = MinSize; + + // Allocate the memory from the BumpPtrAllocator. + T *NewElts = C.getAllocator().Allocate(NewCapacity); + + // Copy the elements over. + if (llvm::is_class::value) { + std::uninitialized_copy(Begin, End, NewElts); + // Destroy the original elements. + destroy_range(Begin, End); + } + else { + // Use memcpy for PODs (std::uninitialized_copy optimizes to memmove). + memcpy(NewElts, Begin, CurSize * sizeof(T)); + } + + // For now, leak 'Begin'. We can add it back to a freelist in + // BumpVectorContext. + Begin = NewElts; + End = NewElts+CurSize; + Capacity = Begin+NewCapacity; +} + +} // end: clang namespace +#endif // end: LLVM_CLANG_BUMP_VECTOR diff --git a/clang/lib/Analysis/BugReporter.cpp b/clang/lib/Analysis/BugReporter.cpp index 064fff47f4ac..8235f4acb179 100644 --- a/clang/lib/Analysis/BugReporter.cpp +++ b/clang/lib/Analysis/BugReporter.cpp @@ -1432,7 +1432,7 @@ MakeReportGraph(const ExplodedGraph* G, // Link up the new node with the previous node. if (Last) - NewN->addPredecessor(Last); + NewN->addPredecessor(Last, *GNew); Last = NewN; diff --git a/clang/lib/Analysis/ExplodedGraph.cpp b/clang/lib/Analysis/ExplodedGraph.cpp index 463b171249f3..0dc81a4225a8 100644 --- a/clang/lib/Analysis/ExplodedGraph.cpp +++ b/clang/lib/Analysis/ExplodedGraph.cpp @@ -43,53 +43,48 @@ void ExplodedNode::SetAuditor(ExplodedNode::Auditor* A) { // ExplodedNode. //===----------------------------------------------------------------------===// -static inline std::vector& getVector(void* P) { - return *reinterpret_cast*>(P); +static inline BumpVector& getVector(void* P) { + return *reinterpret_cast*>(P); } -void ExplodedNode::Profile(llvm::FoldingSetNodeID& ID, - const ProgramPoint& Loc, - const GRState* state) { - ID.Add(Loc); - ID.AddPointer(state); -} - -void ExplodedNode::addPredecessor(ExplodedNode* V) { +void ExplodedNode::addPredecessor(ExplodedNode* V, ExplodedGraph &G) { assert (!V->isSink()); - Preds.addNode(V); - V->Succs.addNode(this); + Preds.addNode(V, G); + V->Succs.addNode(this, G); #ifndef NDEBUG if (NodeAuditor) NodeAuditor->AddEdge(V, this); #endif } -void ExplodedNode::NodeGroup::addNode(ExplodedNode* N) { - - assert ((reinterpret_cast(N) & Mask) == 0x0); - assert (!getFlag()); +void ExplodedNode::NodeGroup::addNode(ExplodedNode* N, ExplodedGraph &G) { + assert((reinterpret_cast(N) & Mask) == 0x0); + assert(!getFlag()); if (getKind() == Size1) { if (ExplodedNode* NOld = getNode()) { - std::vector* V = new std::vector(); - assert ((reinterpret_cast(V) & Mask) == 0x0); - V->push_back(NOld); - V->push_back(N); + BumpVectorContext &Ctx = G.getNodeAllocator(); + BumpVector *V = + G.getAllocator().Allocate >(); + new (V) BumpVector(Ctx, 4); + + assert((reinterpret_cast(V) & Mask) == 0x0); + V->push_back(NOld, Ctx); + V->push_back(N, Ctx); P = reinterpret_cast(V) | SizeOther; - assert (getPtr() == (void*) V); - assert (getKind() == SizeOther); + assert(getPtr() == (void*) V); + assert(getKind() == SizeOther); } else { P = reinterpret_cast(N); - assert (getKind() == Size1); + assert(getKind() == Size1); } } else { - assert (getKind() == SizeOther); - getVector(getPtr()).push_back(N); + assert(getKind() == SizeOther); + getVector(getPtr()).push_back(N, G.getNodeAllocator()); } } - unsigned ExplodedNode::NodeGroup::size() const { if (getFlag()) return 0; @@ -100,7 +95,7 @@ unsigned ExplodedNode::NodeGroup::size() const { return getVector(getPtr()).size(); } -ExplodedNode** ExplodedNode::NodeGroup::begin() const { +ExplodedNode **ExplodedNode::NodeGroup::begin() const { if (getFlag()) return NULL; @@ -119,14 +114,10 @@ ExplodedNode** ExplodedNode::NodeGroup::end() const { else { // Dereferencing end() is undefined behaviour. The vector is not empty, so // we can dereference the last elem and then add 1 to the result. - return const_cast(&getVector(getPtr()).back()) + 1; + return const_cast(getVector(getPtr()).end()); } } -ExplodedNode::NodeGroup::~NodeGroup() { - if (getKind() == SizeOther) delete &getVector(getPtr()); -} - ExplodedNode *ExplodedGraph::getNode(const ProgramPoint& L, const GRState* State, bool* IsNew) { // Profile 'State' to determine if we already have an existing node. @@ -138,7 +129,7 @@ ExplodedNode *ExplodedGraph::getNode(const ProgramPoint& L, if (!V) { // Allocate a new node. - V = (NodeTy*) Allocator.Allocate(); + V = (NodeTy*) getAllocator().Allocate(); new (V) NodeTy(L, State); // Insert the node into the node set and return it. @@ -253,7 +244,7 @@ ExplodedGraph::TrimInternal(const ExplodedNode* const* BeginSources, if (PI == Pass2.end()) continue; - NewN->addPredecessor(PI->second); + NewN->addPredecessor(PI->second, *G); } // In the case that some of the intended successors of NewN have already @@ -263,7 +254,7 @@ ExplodedGraph::TrimInternal(const ExplodedNode* const* BeginSources, for (ExplodedNode **I=N->Succs.begin(), **E=N->Succs.end(); I!=E; ++I) { Pass2Ty::iterator PI = Pass2.find(*I); if (PI != Pass2.end()) { - PI->second->addPredecessor(NewN); + PI->second->addPredecessor(NewN, *G); continue; } diff --git a/clang/lib/Analysis/GRCoreEngine.cpp b/clang/lib/Analysis/GRCoreEngine.cpp index 909f6196d6f3..87472472fdee 100644 --- a/clang/lib/Analysis/GRCoreEngine.cpp +++ b/clang/lib/Analysis/GRCoreEngine.cpp @@ -372,7 +372,7 @@ void GRCoreEngine::GenerateNode(const ProgramPoint& Loc, ExplodedNode* Node = G->getNode(Loc, State, &IsNew); if (Pred) - Node->addPredecessor(Pred); // Link 'Node' with its predecessor. + Node->addPredecessor(Pred, *G); // Link 'Node' with its predecessor. else { assert (IsNew); G->addRoot(Node); // 'Node' has no predecessor. Make it a root. @@ -412,7 +412,7 @@ void GRStmtNodeBuilder::GenerateAutoTransition(ExplodedNode* N) { bool IsNew; ExplodedNode* Succ = Eng.G->getNode(Loc, N->State, &IsNew); - Succ->addPredecessor(N); + Succ->addPredecessor(N, *Eng.G); if (IsNew) Eng.WList->Enqueue(Succ, B, Idx+1); @@ -471,7 +471,7 @@ GRStmtNodeBuilder::generateNodeInternal(const ProgramPoint &Loc, ExplodedNode* Pred) { bool IsNew; ExplodedNode* N = Eng.G->getNode(Loc, State, &IsNew); - N->addPredecessor(Pred); + N->addPredecessor(Pred, *Eng.G); Deferred.erase(Pred); if (IsNew) { @@ -497,7 +497,7 @@ ExplodedNode* GRBranchNodeBuilder::generateNode(const GRState* State, Eng.G->getNode(BlockEdge(Src,branch ? DstT:DstF,Pred->getLocationContext()), State, &IsNew); - Succ->addPredecessor(Pred); + Succ->addPredecessor(Pred, *Eng.G); if (branch) GeneratedTrue = true; @@ -529,7 +529,7 @@ GRIndirectGotoNodeBuilder::generateNode(const iterator& I, const GRState* St, ExplodedNode* Succ = Eng.G->getNode(BlockEdge(Src, I.getBlock(), Pred->getLocationContext()), St, &IsNew); - Succ->addPredecessor(Pred); + Succ->addPredecessor(Pred, *Eng.G); if (IsNew) { @@ -552,7 +552,7 @@ GRSwitchNodeBuilder::generateCaseStmtNode(const iterator& I, const GRState* St){ ExplodedNode* Succ = Eng.G->getNode(BlockEdge(Src, I.getBlock(), Pred->getLocationContext()), St, &IsNew); - Succ->addPredecessor(Pred); + Succ->addPredecessor(Pred, *Eng.G); if (IsNew) { Eng.WList->Enqueue(Succ); @@ -574,7 +574,7 @@ GRSwitchNodeBuilder::generateDefaultCaseNode(const GRState* St, bool isSink) { ExplodedNode* Succ = Eng.G->getNode(BlockEdge(Src, DefaultBlock, Pred->getLocationContext()), St, &IsNew); - Succ->addPredecessor(Pred); + Succ->addPredecessor(Pred, *Eng.G); if (IsNew) { if (isSink) @@ -602,7 +602,7 @@ GREndPathNodeBuilder::generateNode(const GRState* State, const void *tag, ExplodedNode* Node = Eng.G->getNode(BlockEntrance(&B, Pred->getLocationContext(), tag), State, &IsNew); - Node->addPredecessor(P ? P : Pred); + Node->addPredecessor(P ? P : Pred, *Eng.G); if (IsNew) { Eng.G->addEndOfPath(Node);