[cfe-commits] r65160 - /cfe/trunk/lib/Analysis/ExplodedGraph.cpp
Zhongxing Xu
xuzhongxing at gmail.com
Fri Feb 20 22:19:45 PST 2009
Thanks Ted. The old code is a mystery for me.
On Sat, Feb 21, 2009 at 5:10 AM, Ted Kremenek <kremenek at apple.com> wrote:
> Author: kremenek
> Date: Fri Feb 20 15:10:26 2009
> New Revision: 65160
>
> URL: http://llvm.org/viewvc/llvm-project?rev=65160&view=rev
> Log:
> Greatly simplify the logic in ExplodedGraphImpl::TrimGraph. Now we just do
> a
> vanilla reverse-BFS followed by a forward-DFS instead of resulting to
> strange
> histrionics (whose purpose I can no longer remember) in the reverse-BFS
> stage.
> This fixes an assertion failure in BugReporter due to edge cases where no
> root
> was being hit in the reverse-BFS phase.
>
> Modified:
> cfe/trunk/lib/Analysis/ExplodedGraph.cpp
>
> Modified: cfe/trunk/lib/Analysis/ExplodedGraph.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ExplodedGraph.cpp?rev=65160&r1=65159&r2=65160&view=diff
>
>
> ==============================================================================
> --- cfe/trunk/lib/Analysis/ExplodedGraph.cpp (original)
> +++ cfe/trunk/lib/Analysis/ExplodedGraph.cpp Fri Feb 20 15:10:26 2009
> @@ -127,142 +127,80 @@
> llvm::DenseMap<const void*, const void*>
> *InverseMap)
> const {
>
> - typedef llvm::DenseMap<const ExplodedNodeImpl*,
> - const ExplodedNodeImpl*> Pass1Ty;
> + typedef llvm::DenseSet<const ExplodedNodeImpl*> Pass1Ty;
> Pass1Ty Pass1;
>
> - typedef llvm::DenseMap<const ExplodedNodeImpl*,
> - ExplodedNodeImpl*> Pass2Ty;
> -
> + typedef llvm::DenseMap<const ExplodedNodeImpl*, ExplodedNodeImpl*>
> Pass2Ty;
> Pass2Ty& Pass2 = M->M;
>
> + std::list<const ExplodedNodeImpl*> WL1;
> llvm::SmallVector<const ExplodedNodeImpl*, 10> WL2;
>
> - { // ===- Pass 1 (reverse BFS) -===
> -
> - // Enqueue the source nodes to the first worklist.
> + // ===- Pass 1 (reverse BFS) -===
> + for (const ExplodedNodeImpl* const* I = BeginSources; I != EndSources;
> ++I) {
> + assert(*I);
> + WL1.push_back(*I);
> + }
>
> - std::list<std::pair<const ExplodedNodeImpl*,
> - const ExplodedNodeImpl*> > WL1, WL1_Loops;
> -
> - for (const ExplodedNodeImpl* const* I = BeginSources; I != EndSources;
> ++I){
> - assert(*I);
> - WL1.push_back(std::make_pair(*I, *I));
> - }
> + // Process the first worklist until it is empty. Because it is a
> std::list
> + // it acts like a FIFO queue.
> + while (!WL1.empty()) {
> + const ExplodedNodeImpl *N = WL1.back();
> + WL1.pop_back();
>
> - // Process the worklist.
> + // Have we already visited this node? If so, continue to the next
> one.
> + if (Pass1.count(N))
> + continue;
>
> - while (!WL1.empty() || !WL1_Loops.empty()) {
> - // Only dequeue from the "loops" worklist if WL1 has no items.
> - // Thus we prioritize for paths that don't span loop boundaries.
> - const ExplodedNodeImpl *N = 0, *Src = 0;
> -
> - if (WL1.empty()) {
> - assert(!WL1_Loops.empty());
> - N = WL1_Loops.back().first;
> - assert(N);
> - Src = WL1_Loops.back().second;
> - WL1_Loops.pop_back();
> - }
> - else {
> - N = WL1.back().first;
> - assert(N);
> - Src = WL1.back().second;
> - WL1.pop_back();
> - }
> -
> - if (Pass1.find(N) != Pass1.end())
> - continue;
> -
> - bool PredHasSameSource = false;
> - bool VisitPreds = true;
> -
> - for (ExplodedNodeImpl** I=N->Preds.begin(), **E=N->Preds.end();
> - I!=E; ++I) {
> -
> - Pass1Ty::iterator pi = Pass1.find(*I);
> -
> - if (pi == Pass1.end())
> - continue;
> -
> - VisitPreds = false;
> -
> - if (pi->second == Src) {
> - PredHasSameSource = true;
> - break;
> - }
> - }
> -
> - if (VisitPreds || !PredHasSameSource) {
> -
> - Pass1[N] = Src;
> -
> - if (N->Preds.empty()) {
> - WL2.push_back(N);
> - continue;
> - }
> - }
> - else
> - Pass1[N] = NULL;
> -
> - if (VisitPreds)
> - for (ExplodedNodeImpl** I=N->Preds.begin(), **E=N->Preds.end();
> - I!=E; ++I) {
> -
> - ProgramPoint P = Src->getLocation();
> -
> - if (const BlockEdge *BE = dyn_cast<BlockEdge>(&P))
> - if (Stmt* T = BE->getSrc()->getTerminator())
> - switch (T->getStmtClass()) {
> - default: break;
> - case Stmt::ForStmtClass:
> - case Stmt::WhileStmtClass:
> - case Stmt::DoStmtClass:
> - assert(*I);
> - WL1_Loops.push_front(std::make_pair(*I, Src));
> - continue;
> -
> - }
> -
> - assert(*I);
> - WL1.push_front(std::make_pair(*I, Src));
> - }
> + // Otherwise, mark this node as visited.
> + Pass1.insert(N);
> +
> + // If this is a root enqueue it to the second worklist.
> + if (N->Preds.empty()) {
> + WL2.push_back(N);
> + continue;
> }
> +
> + // Visit our predecessors and enqueue them.
> + for (ExplodedNodeImpl** I=N->Preds.begin(), **E=N->Preds.end(); I!=E;
> ++I)
> + WL1.push_front(*I);
> }
>
> + // We didn't hit a root? Return with a null pointer for the new graph.
> if (WL2.empty())
> return 0;
>
> + // Create an empty graph.
> ExplodedGraphImpl* G = MakeEmptyGraph();
>
> - // ===- Pass 2 (forward DFS to construct the new graph) -===
> -
> + // ===- Pass 2 (forward DFS to construct the new graph) -===
> while (!WL2.empty()) {
> -
> const ExplodedNodeImpl* N = WL2.back();
> WL2.pop_back();
>
> // Skip this node if we have already processed it.
> -
> if (Pass2.find(N) != Pass2.end())
> continue;
>
> - // Create the corresponding node in the new graph.
> -
> + // Create the corresponding node in the new graph and record the
> mapping
> + // from the old node to the new node.
> ExplodedNodeImpl* NewN = G->getNodeImpl(N->getLocation(), N->State,
> NULL);
> Pass2[N] = NewN;
> +
> + // Also record the reverse mapping from the new node to the old node.
> if (InverseMap) (*InverseMap)[NewN] = N;
>
> + // If this node is a root, designate it as such in the graph.
> if (N->Preds.empty())
> G->addRoot(NewN);
>
> // In the case that some of the intended predecessors of NewN have
> already
> // been created, we should hook them up as predecessors.
> -
> +
> + // Walk through the predecessors of 'N' and hook up their
> corresponding
> + // nodes in the new graph (if any) to the freshly created node.
> for (ExplodedNodeImpl **I=N->Preds.begin(), **E=N->Preds.end(); I!=E;
> ++I) {
> -
> Pass2Ty::iterator PI = Pass2.find(*I);
> -
> if (PI == Pass2.end())
> continue;
>
> @@ -273,26 +211,19 @@
> // been created, we should hook them up as successors. Otherwise,
> enqueue
> // the new nodes from the original graph that should have nodes created
> // in the new graph.
> -
> for (ExplodedNodeImpl **I=N->Succs.begin(), **E=N->Succs.end(); I!=E;
> ++I) {
> -
> - Pass2Ty::iterator PI = Pass2.find(*I);
> -
> + Pass2Ty::iterator PI = Pass2.find(*I);
> if (PI != Pass2.end()) {
> PI->second->addPredecessor(NewN);
> continue;
> }
>
> // Enqueue nodes to the worklist that were marked during pass 1.
> -
> - Pass1Ty::iterator pi = Pass1.find(*I);
> -
> - if (pi == Pass1.end() || pi->second == NULL)
> - continue;
> -
> - WL2.push_back(*I);
> + if (Pass1.count(*I))
> + WL2.push_back(*I);
> }
>
> + // Finally, explictly mark all nodes without any successors as sinks.
> if (N->isSink())
> NewN->markAsSink();
> }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20090221/38173f90/attachment.html>
More information about the cfe-commits
mailing list