[llvm-commits] [llvm] r80533 - in /llvm/trunk: include/llvm/Analysis/CallGraph.h lib/Analysis/IPA/CallGraph.cpp lib/Transforms/IPO/Inliner.cpp test/Transforms/Inline/indirect_resolve.ll
Chris Lattner
sabre at nondot.org
Sun Aug 30 20:15:50 PDT 2009
Author: lattner
Date: Sun Aug 30 22:15:49 2009
New Revision: 80533
URL: http://llvm.org/viewvc/llvm-project?rev=80533&view=rev
Log:
Fix PR4834, a tricky case where the inliner would resolve an
indirect function pointer, inline it, then go to delete the body.
The problem is that the callgraph had other references to the function,
though the inliner had no way to know it, so we got a dangling pointer
and an invalid iterator out of the deal.
The fix to this is pretty simple: stop the inliner from deleting the
function by knowing that there are references to it. Do this by making
CallGraphNodes contain a refcount. This requires moving deletion of
available_externally functions to the module-level cleanup sweep where
it belongs.
Added:
llvm/trunk/test/Transforms/Inline/indirect_resolve.ll
Modified:
llvm/trunk/include/llvm/Analysis/CallGraph.h
llvm/trunk/lib/Analysis/IPA/CallGraph.cpp
llvm/trunk/lib/Transforms/IPO/Inliner.cpp
Modified: llvm/trunk/include/llvm/Analysis/CallGraph.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/CallGraph.h?rev=80533&r1=80532&r2=80533&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/CallGraph.h (original)
+++ llvm/trunk/include/llvm/Analysis/CallGraph.h Sun Aug 30 22:15:49 2009
@@ -77,7 +77,7 @@
public:
static char ID; // Class identification, replacement for typeinfo
//===---------------------------------------------------------------------
- // Accessors...
+ // Accessors.
//
typedef FunctionMapTy::iterator iterator;
typedef FunctionMapTy::const_iterator const_iterator;
@@ -136,7 +136,7 @@
CallGraphNode *getOrInsertFunction(const Function *F);
//===---------------------------------------------------------------------
- // Pass infrastructure interface glue code...
+ // Pass infrastructure interface glue code.
//
protected:
CallGraph() {}
@@ -163,19 +163,31 @@
Function *F;
typedef std::pair<CallSite,CallGraphNode*> CallRecord;
std::vector<CallRecord> CalledFunctions;
+
+ /// NumReferences - This is the number of times that this CallGraphNode occurs
+ /// in the CalledFunctions array of this or other CallGraphNodes.
+ unsigned NumReferences;
- CallGraphNode(const CallGraphNode &); // Do not implement
+ CallGraphNode(const CallGraphNode &); // DO NOT IMPLEMENT
+ void operator=(const CallGraphNode &); // DO NOT IMPLEMENT
+
+ void DropRef() { --NumReferences; }
+ void AddRef() { ++NumReferences; }
public:
typedef std::vector<CallRecord> CalledFunctionsVector;
+
+ // CallGraphNode ctor - Create a node for the specified function.
+ inline CallGraphNode(Function *f) : F(f), NumReferences(0) {}
+
//===---------------------------------------------------------------------
- // Accessor methods...
+ // Accessor methods.
//
typedef std::vector<CallRecord>::iterator iterator;
typedef std::vector<CallRecord>::const_iterator const_iterator;
- // getFunction - Return the function that this call graph node represents...
+ // getFunction - Return the function that this call graph node represents.
Function *getFunction() const { return F; }
inline iterator begin() { return CalledFunctions.begin(); }
@@ -185,9 +197,14 @@
inline bool empty() const { return CalledFunctions.empty(); }
inline unsigned size() const { return (unsigned)CalledFunctions.size(); }
- // Subscripting operator - Return the i'th called function...
+ /// getNumReferences - Return the number of other CallGraphNodes in this
+ /// CallGraph that reference this node in their callee list.
+ unsigned getNumReferences() const { return NumReferences; }
+
+ // Subscripting operator - Return the i'th called function.
//
CallGraphNode *operator[](unsigned i) const {
+ assert(i < CalledFunctions.size() && "Invalid index");
return CalledFunctions[i].second;
}
@@ -204,7 +221,10 @@
/// removeAllCalledFunctions - As the name implies, this removes all edges
/// from this CallGraphNode to any functions it calls.
void removeAllCalledFunctions() {
- CalledFunctions.clear();
+ while (!CalledFunctions.empty()) {
+ CalledFunctions.back().second->DropRef();
+ CalledFunctions.pop_back();
+ }
}
/// stealCalledFunctionsFrom - Move all the callee information from N to this
@@ -220,6 +240,7 @@
/// one.
void addCalledFunction(CallSite CS, CallGraphNode *M) {
CalledFunctions.push_back(std::make_pair(CS, M));
+ M->AddRef();
}
/// removeCallEdgeFor - This method removes the edge in the node for the
@@ -240,11 +261,6 @@
/// New CallSite instead. Note that this method takes linear time, so it
/// should be used sparingly.
void replaceCallSite(CallSite Old, CallSite New, CallGraphNode *NewCallee);
-
- friend class CallGraph;
-
- // CallGraphNode ctor - Create a node for the specified function.
- inline CallGraphNode(Function *f) : F(f) {}
};
//===----------------------------------------------------------------------===//
Modified: llvm/trunk/lib/Analysis/IPA/CallGraph.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/IPA/CallGraph.cpp?rev=80533&r1=80532&r2=80533&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/IPA/CallGraph.cpp (original)
+++ llvm/trunk/lib/Analysis/IPA/CallGraph.cpp Sun Aug 30 22:15:49 2009
@@ -53,7 +53,7 @@
CallsExternalNode = new CallGraphNode(0);
Root = 0;
- // Add every function to the call graph...
+ // Add every function to the call graph.
for (Module::iterator I = M.begin(), E = M.end(); I != E; ++I)
addToCallGraph(I);
@@ -169,12 +169,12 @@
}
void CallGraph::destroy() {
- if (!FunctionMap.empty()) {
- for (FunctionMapTy::iterator I = FunctionMap.begin(), E = FunctionMap.end();
- I != E; ++I)
- delete I->second;
- FunctionMap.clear();
- }
+ if (FunctionMap.empty()) return;
+
+ for (FunctionMapTy::iterator I = FunctionMap.begin(), E = FunctionMap.end();
+ I != E; ++I)
+ delete I->second;
+ FunctionMap.clear();
}
void CallGraph::print(raw_ostream &OS, Module*) const {
@@ -219,10 +219,11 @@
void CallGraphNode::print(raw_ostream &OS) const {
if (Function *F = getFunction())
- OS << "Call graph node for function: '" << F->getName()
- << "'<<0x" << this << ">>\n";
+ OS << "Call graph node for function: '" << F->getName() << "'";
else
- OS << "Call graph node <<null function: 0x" << this << ">>:\n";
+ OS << "Call graph node <<null function>>";
+
+ OS << "<<0x" << this << ">> #uses=" << getNumReferences() << '\n';
for (const_iterator I = begin(), E = end(); I != E; ++I)
if (Function *FI = I->second->getFunction())
@@ -241,7 +242,9 @@
for (CalledFunctionsVector::iterator I = CalledFunctions.begin(); ; ++I) {
assert(I != CalledFunctions.end() && "Cannot find callsite to remove!");
if (I->first == CS) {
- CalledFunctions.erase(I);
+ I->second->DropRef();
+ *I = CalledFunctions.back();
+ CalledFunctions.pop_back();
return;
}
}
@@ -254,6 +257,7 @@
void CallGraphNode::removeAnyCallEdgeTo(CallGraphNode *Callee) {
for (unsigned i = 0, e = CalledFunctions.size(); i != e; ++i)
if (CalledFunctions[i].second == Callee) {
+ Callee->DropRef();
CalledFunctions[i] = CalledFunctions.back();
CalledFunctions.pop_back();
--i; --e;
@@ -266,8 +270,10 @@
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()) {
- CalledFunctions.erase(I);
+ if (CR.second == Callee && CR.first.getInstruction() == 0) {
+ Callee->DropRef();
+ *I = CalledFunctions.back();
+ CalledFunctions.pop_back();
return;
}
}
@@ -285,8 +291,11 @@
// If the callee is changing, not just the callsite, then update it as
// well.
- if (NewCallee)
+ if (NewCallee) {
+ I->second->DropRef();
I->second = NewCallee;
+ I->second->AddRef();
+ }
return;
}
}
Modified: llvm/trunk/lib/Transforms/IPO/Inliner.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/Inliner.cpp?rev=80533&r1=80532&r2=80533&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/Inliner.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/Inliner.cpp Sun Aug 30 22:15:49 2009
@@ -293,10 +293,12 @@
// If we inlined the last possible call site to the function, delete the
// function body now.
- if (Callee->use_empty() &&
- (Callee->hasLocalLinkage() ||
- Callee->hasAvailableExternallyLinkage()) &&
- !SCCFunctions.count(Callee)) {
+ if (Callee->use_empty() && Callee->hasLocalLinkage() &&
+ !SCCFunctions.count(Callee) &&
+ // The function may be apparently dead, but if there are indirect
+ // callgraph references to the node, we cannot delete it yet, this
+ // could invalidate the CGSCC iterator.
+ CG[Callee]->getNumReferences() == 0) {
DEBUG(errs() << " -> Deleting dead function: "
<< Callee->getName() << "\n");
CallGraphNode *CalleeNode = CG[Callee];
@@ -353,7 +355,7 @@
// from the program. Insert the dead ones in the FunctionsToRemove set.
for (CallGraph::iterator I = CG.begin(), E = CG.end(); I != E; ++I) {
CallGraphNode *CGN = I->second;
- if (CGN == 0 || CGN->getFunction() == 0)
+ if (CGN->getFunction() == 0)
continue;
Function *F = CGN->getFunction();
@@ -364,7 +366,8 @@
if (DNR && DNR->count(F))
continue;
- if (!F->hasLinkOnceLinkage() && !F->hasLocalLinkage())
+ if (!F->hasLinkOnceLinkage() && !F->hasLocalLinkage() &&
+ !F->hasAvailableExternallyLinkage())
continue;
if (!F->use_empty())
continue;
Added: llvm/trunk/test/Transforms/Inline/indirect_resolve.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/indirect_resolve.ll?rev=80533&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/Inline/indirect_resolve.ll (added)
+++ llvm/trunk/test/Transforms/Inline/indirect_resolve.ll Sun Aug 30 22:15:49 2009
@@ -0,0 +1,16 @@
+; RUN: llvm-as < %s | opt -inline | llvm-dis
+; PR4834
+
+define i32 @main() {
+ %funcall1_ = call fastcc i32 ()* ()* @f1()
+ %executecommandptr1_ = call i32 %funcall1_()
+ ret i32 %executecommandptr1_
+}
+
+define internal fastcc i32 ()* @f1() nounwind readnone {
+ ret i32 ()* @f2
+}
+
+define internal i32 @f2() nounwind readnone {
+ ret i32 1
+}
More information about the llvm-commits
mailing list