[llvm-commits] [llvm] r80663 - in /llvm/trunk: include/llvm/Analysis/CallGraph.h lib/Analysis/IPA/CallGraph.cpp lib/Analysis/IPA/CallGraphSCCPass.cpp lib/Transforms/IPO/PruneEH.cpp lib/Transforms/Utils/InlineFunction.cpp test/Transforms/ArgumentPromotion/callgraph-update.ll
Chris Lattner
sabre at nondot.org
Mon Aug 31 23:31:32 PDT 2009
Author: lattner
Date: Tue Sep 1 01:31:31 2009
New Revision: 80663
URL: http://llvm.org/viewvc/llvm-project?rev=80663&view=rev
Log:
Change CallGraphNode to maintain it's Function as an AssertingVH
for sanity. This didn't turn up any bugs.
Change CallGraphNode to maintain its "callsite" information in the
call edges list as a WeakVH instead of as an instruction*. This fixes
a broad class of dangling pointer bugs, and makes CallGraph have a number
of useful invariants again. This fixes the class of problem indicated
by PR4029 and PR3601.
Added:
llvm/trunk/test/Transforms/ArgumentPromotion/callgraph-update.ll
Modified:
llvm/trunk/include/llvm/Analysis/CallGraph.h
llvm/trunk/lib/Analysis/IPA/CallGraph.cpp
llvm/trunk/lib/Analysis/IPA/CallGraphSCCPass.cpp
llvm/trunk/lib/Transforms/IPO/PruneEH.cpp
llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp
Modified: llvm/trunk/include/llvm/Analysis/CallGraph.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/CallGraph.h?rev=80663&r1=80662&r2=80663&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/CallGraph.h (original)
+++ llvm/trunk/include/llvm/Analysis/CallGraph.h Tue Sep 1 01:31:31 2009
@@ -55,6 +55,7 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/Pass.h"
#include "llvm/Support/CallSite.h"
+#include "llvm/Support/ValueHandle.h"
#include "llvm/System/IncludeFile.h"
#include <map>
@@ -158,11 +159,16 @@
};
//===----------------------------------------------------------------------===//
-// CallGraphNode class definition
+// CallGraphNode class definition.
//
class CallGraphNode {
- Function *F;
- typedef std::pair<CallSite,CallGraphNode*> CallRecord;
+ AssertingVH<Function> F;
+
+ // CallRecord - This is a pair of the calling instruction (a call or invoke)
+ // and the callgraph node being called.
+public:
+ typedef std::pair<WeakVH, CallGraphNode*> CallRecord;
+private:
std::vector<CallRecord> CalledFunctions;
/// NumReferences - This is the number of times that this CallGraphNode occurs
@@ -240,19 +246,22 @@
/// addCalledFunction - Add a function to the list of functions called by this
/// one.
void addCalledFunction(CallSite CS, CallGraphNode *M) {
- CalledFunctions.push_back(std::make_pair(CS, M));
+ CalledFunctions.push_back(std::make_pair(CS.getInstruction(), M));
M->AddRef();
}
+ void removeCallEdge(iterator I) {
+ I->second->DropRef();
+ *I = CalledFunctions.back();
+ CalledFunctions.pop_back();
+ }
+
+
/// removeCallEdgeFor - This method removes the edge in the node for the
/// specified call site. Note that this method takes linear time, so it
/// should be used sparingly.
void removeCallEdgeFor(CallSite CS);
- // FIXME: REMOVE THIS WHEN HACK IS REMOVED FROM CGSCCPASSMGR.
- void removeCallEdgeFor(Instruction *CS);
-
-
/// removeAnyCallEdgeTo - This method removes all call edges from this node
/// to the specified callee function. This takes more time to execute than
/// removeCallEdgeTo, so it should not be used unless necessary.
@@ -278,7 +287,7 @@
template <> struct GraphTraits<CallGraphNode*> {
typedef CallGraphNode NodeType;
- typedef std::pair<CallSite, CallGraphNode*> CGNPairTy;
+ typedef CallGraphNode::CallRecord CGNPairTy;
typedef std::pointer_to_unary_function<CGNPairTy, CallGraphNode*> CGNDerefFun;
static NodeType *getEntryNode(CallGraphNode *CGN) { return CGN; }
Modified: llvm/trunk/lib/Analysis/IPA/CallGraph.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/IPA/CallGraph.cpp?rev=80663&r1=80662&r2=80663&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/IPA/CallGraph.cpp (original)
+++ llvm/trunk/lib/Analysis/IPA/CallGraph.cpp Tue Sep 1 01:31:31 2009
@@ -241,7 +241,7 @@
void CallGraphNode::removeCallEdgeFor(CallSite CS) {
for (CalledFunctionsVector::iterator I = CalledFunctions.begin(); ; ++I) {
assert(I != CalledFunctions.end() && "Cannot find callsite to remove!");
- if (I->first == CS) {
+ if (I->first == CS.getInstruction()) {
I->second->DropRef();
*I = CalledFunctions.back();
CalledFunctions.pop_back();
@@ -250,21 +250,6 @@
}
}
-// FIXME: REMOVE THIS WHEN HACK IS REMOVED FROM CGSCCPASSMGR.
-void CallGraphNode::removeCallEdgeFor(Instruction *CS) {
- for (CalledFunctionsVector::iterator I = CalledFunctions.begin(); ; ++I) {
- assert(I != CalledFunctions.end() && "Cannot find callsite to remove!");
- if (I->first.getInstruction() == CS) {
- I->second->DropRef();
- *I = CalledFunctions.back();
- CalledFunctions.pop_back();
- return;
- }
- }
-
-}
-
-
// removeAnyCallEdgeTo - This method removes any call edges from this node to
// the specified callee function. This takes more time to execute than
@@ -285,7 +270,7 @@
for (CalledFunctionsVector::iterator I = CalledFunctions.begin(); ; ++I) {
assert(I != CalledFunctions.end() && "Cannot find callee to remove!");
CallRecord &CR = *I;
- if (CR.second == Callee && CR.first.getInstruction() == 0) {
+ if (CR.second == Callee && CR.first == 0) {
Callee->DropRef();
*I = CalledFunctions.back();
CalledFunctions.pop_back();
@@ -301,9 +286,9 @@
CallGraphNode *NewCallee) {
for (CalledFunctionsVector::iterator I = CalledFunctions.begin(); ; ++I) {
assert(I != CalledFunctions.end() && "Cannot find callsite to replace!");
- if (I->first != Old) continue;
+ if (I->first != Old.getInstruction()) continue;
- I->first = New;
+ I->first = New.getInstruction();
// If the callee is changing, not just the callsite, then update it as
// well.
Modified: llvm/trunk/lib/Analysis/IPA/CallGraphSCCPass.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/IPA/CallGraphSCCPass.cpp?rev=80663&r1=80662&r2=80663&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/IPA/CallGraphSCCPass.cpp (original)
+++ llvm/trunk/lib/Analysis/IPA/CallGraphSCCPass.cpp Tue Sep 1 01:31:31 2009
@@ -125,7 +125,7 @@
void CGPassManager::RefreshCallGraph(std::vector<CallGraphNode*> &CurSCC,
CallGraph &CG) {
- DenseMap<Instruction*, CallGraphNode*> CallSites;
+ DenseMap<Value*, CallGraphNode*> CallSites;
DEBUG(errs() << "CGSCCPASSMGR: Refreshing SCC with " << CurSCC.size()
<< " nodes:\n";
@@ -146,12 +146,28 @@
// Get the set of call sites currently in the function.
for (CallGraphNode::iterator I = CGN->begin(), E = CGN->end(); I != E; ++I){
- assert(I->first.getInstruction() &&
- "Call site record in function should not be abstract");
- assert(!CallSites.count(I->first.getInstruction()) &&
+ // If this call site is null, then the function pass deleted the call
+ // entirely and the WeakVH nulled it out.
+ if (I->first == 0 ||
+ // If we've already seen this call site, then the FunctionPass RAUW'd
+ // one call with another, which resulted in two "uses" in the edge
+ // list of the same call.
+ CallSites.count(I->first) ||
+
+ // If the call edge is not from a call or invoke, then the function
+ // pass RAUW'd a call with another value. This can happen when
+ // constant folding happens of well known functions etc.
+ CallSite::get(I->first).getInstruction() == 0) {
+ // Just remove the edge from the set of callees.
+ CGN->removeCallEdge(I);
+ E = CGN->end();
+ --I;
+ continue;
+ }
+
+ assert(!CallSites.count(I->first) &&
"Call site occurs in node multiple times");
- CallSites.insert(std::make_pair(I->first.getInstruction(),
- I->second));
+ CallSites.insert(std::make_pair(I->first, I->second));
}
// Loop over all of the instructions in the function, getting the callsites.
@@ -162,7 +178,7 @@
// If this call site already existed in the callgraph, just verify it
// matches up to expectations and remove it from CallSites.
- DenseMap<Instruction*, CallGraphNode*>::iterator ExistingIt =
+ DenseMap<Value*, CallGraphNode*>::iterator ExistingIt =
CallSites.find(CS.getInstruction());
if (ExistingIt != CallSites.end()) {
CallGraphNode *ExistingNode = ExistingIt->second;
@@ -201,18 +217,14 @@
}
// After scanning this function, if we still have entries in callsites, then
- // they are dangling pointers. Crap. Well, until we change CallGraph to
- // use CallbackVH, we'll just zap them here. When we have that, this should
- // turn into an assertion.
- if (CallSites.empty()) continue;
-
- for (DenseMap<Instruction*, CallGraphNode*>::iterator I = CallSites.begin(),
- E = CallSites.end(); I != E; ++I)
- // FIXME: I had to add a special horrible form of removeCallEdgeFor to
- // support this. Remove the Instruction* version of it when we can.
- CGN->removeCallEdgeFor(I->first);
- MadeChange = true;
- CallSites.clear();
+ // they are dangling pointers. WeakVH should save us for this, so abort if
+ // this happens.
+ assert(CallSites.empty() && "Dangling pointers found in call sites map");
+
+ // Periodically do an explicit clear to remove tombstones when processing
+ // large scc's.
+ if ((sccidx & 15) == 0)
+ CallSites.clear();
}
DEBUG(if (MadeChange) {
Modified: llvm/trunk/lib/Transforms/IPO/PruneEH.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/PruneEH.cpp?rev=80663&r1=80662&r2=80663&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/PruneEH.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/PruneEH.cpp Tue Sep 1 01:31:31 2009
@@ -165,9 +165,6 @@
// function if we have invokes to non-unwinding functions or code after calls to
// no-return functions.
bool PruneEH::SimplifyFunction(Function *F) {
- CallGraph &CG = getAnalysis<CallGraph>();
- CallGraphNode *CGN = CG[F];
-
bool MadeChange = false;
for (Function::iterator BB = F->begin(), E = F->end(); BB != E; ++BB) {
if (InvokeInst *II = dyn_cast<InvokeInst>(BB->getTerminator()))
@@ -181,14 +178,13 @@
Call->setAttributes(II->getAttributes());
// Anything that used the value produced by the invoke instruction
- // now uses the value produced by the call instruction.
+ // now uses the value produced by the call instruction. Note that we
+ // do this even for void functions and calls with no uses so that the
+ // callgraph edge is updated.
II->replaceAllUsesWith(Call);
BasicBlock *UnwindBlock = II->getUnwindDest();
UnwindBlock->removePredecessor(II->getParent());
- // Fix up the call graph.
- CGN->replaceCallSite(II, Call, 0/*keep callee*/);
-
// Insert a branch to the normal destination right before the
// invoke.
BranchInst::Create(II->getNormalDest(), II);
Modified: llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp?rev=80663&r1=80662&r2=80663&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp Tue Sep 1 01:31:31 2009
@@ -212,7 +212,7 @@
}
for (; I != E; ++I) {
- const Instruction *OrigCall = I->first.getInstruction();
+ const Value *OrigCall = I->first;
DenseMap<const Value*, Value*>::iterator VMI = ValueMap.find(OrigCall);
// Only copy the edge if the call was inlined!
Added: llvm/trunk/test/Transforms/ArgumentPromotion/callgraph-update.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ArgumentPromotion/callgraph-update.ll?rev=80663&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/ArgumentPromotion/callgraph-update.ll (added)
+++ llvm/trunk/test/Transforms/ArgumentPromotion/callgraph-update.ll Tue Sep 1 01:31:31 2009
@@ -0,0 +1,23 @@
+; RUN: llvm-as < %s | opt -argpromotion -simplifycfg -constmerge | llvm-dis
+target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:128:128"
+target triple = "i386-apple-darwin10.0"
+
+%struct.VEC2 = type { double, double, double }
+%struct.VERTEX = type { %struct.VEC2, %struct.VERTEX*, %struct.VERTEX* }
+%struct.edge_rec = type { %struct.VERTEX*, %struct.edge_rec*, i32, i8* }
+
+declare %struct.edge_rec* @alloc_edge() nounwind ssp
+
+define i64 @build_delaunay(%struct.VERTEX* %tree, %struct.VERTEX* %extra) nounwind ssp {
+entry:
+ br i1 undef, label %bb11, label %bb12
+
+bb11: ; preds = %bb10
+ %a = call %struct.edge_rec* @alloc_edge() nounwind ; <%struct.edge_rec*> [#uses=0]
+ ret i64 123
+
+bb12: ; preds = %bb10
+ %b = call %struct.edge_rec* @alloc_edge() nounwind ; <%struct.edge_rec*> [#uses=1]
+ %c = ptrtoint %struct.edge_rec* %b to i64
+ ret i64 %c
+}
More information about the llvm-commits
mailing list