replica
/
hanchenye-llvm-project
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix some nasty callgraph dangling pointer problems in 

argpromotion and structretpromote.  Basically, when replacing
a function, they used the 'changeFunction' api which changes
the entry in the function map (and steals/reuses the callgraph
node).

This has some interesting effects: first, the problem is that it doesn't
update the "callee" edges in any callees of the function in the call graph.
Second, this covers for a major problem in all the CGSCC pass stuff, which 
is that it is completely broken when functions are deleted if they *don't*
reuse a CGN.  (there is a cute little fixme about this though :).

This patch changes the protocol that CGSCC passes must obey: now the CGSCC 
pass manager copies the SCC and preincrements its iterator to avoid passes
invalidating it.  This allows CGSCC passes to mutate the current SCC.  However
multiple passes may be run on that SCC, so if passes do this, they are now
required to *update* the SCC to be current when they return.

Other less interesting parts of this patch are that it makes passes update
the CG more directly, eliminates changeFunction, and requires clients of
replaceCallSite to specify the new callee CGN if they are changing it.

llvm-svn: 80527
This commit is contained in:
Chris Lattner 2009-08-31 00:19:58 +00:00
parent a64b1c1d4c
commit 305b115a87
11 changed files with 112 additions and 88 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
llvm/include/llvm/Analysis/CallGraph.h
View File

@ -130,12 +130,6 @@ public:
return removeFunctionFromModule((*this)[F]);
}
/// changeFunction - This method changes the function associated with this
/// CallGraphNode, for use by transformations that need to change the
/// prototype of a Function (thus they must create a new Function and move the
/// old code over).
void changeFunction(Function *OldF, Function *NewF);
/// getOrInsertFunction - This method is identical to calling operator[], but
/// it will insert a new CallGraphNode for the specified function if one does
/// not already exist.
@ -212,6 +206,15 @@ public:
void removeAllCalledFunctions() {
CalledFunctions.clear();
}
/// stealCalledFunctionsFrom - Move all the callee information from N to this
/// node.
void stealCalledFunctionsFrom(CallGraphNode *N) {
assert(CalledFunctions.empty() &&
"Cannot steal callsite information if I already have some");
std::swap(CalledFunctions, N->CalledFunctions);
}
/// addCalledFunction - Add a function to the list of functions called by this
/// one.
@ -236,7 +239,7 @@ public:
/// replaceCallSite - Make the edge in the node for Old CallSite be for
/// New CallSite instead. Note that this method takes linear time, so it
/// should be used sparingly.
void replaceCallSite(CallSite Old, CallSite New);
void replaceCallSite(CallSite Old, CallSite New, CallGraphNode *NewCallee);
friend class CallGraph;

5
llvm/include/llvm/CallGraphSCCPass.h
View File

@ -46,7 +46,10 @@ struct CallGraphSCCPass : public Pass {
/// non-recursive (or only self-recursive) functions will have an SCC size of
/// 1, where recursive portions of the call graph will have SCC size > 1.
///
virtual bool runOnSCC(const std::vector<CallGraphNode *> &SCC) = 0;
/// SCC passes that add or delete functions to the SCC are required to update
/// the SCC list, otherwise stale pointers may be dereferenced.
///
virtual bool runOnSCC(std::vector<CallGraphNode *> &SCC) = 0;
/// doFinalization - This method is called after the SCC's of the program has
/// been processed, allowing the pass to do final cleanup as necessary.

2
llvm/include/llvm/Transforms/IPO/InlinerPass.h
View File

@ -40,7 +40,7 @@ struct Inliner : public CallGraphSCCPass {
// Main run interface method, this implements the interface required by the
// Pass class.
virtual bool runOnSCC(const std::vector<CallGraphNode *> &SCC);
virtual bool runOnSCC(std::vector<CallGraphNode *> &SCC);
// doFinalization - Remove now-dead linkonce functions at the end of
// processing to avoid breaking the SCC traversal.

25
llvm/lib/Analysis/IPA/CallGraph.cpp
View File

@ -206,20 +206,6 @@ Function *CallGraph::removeFunctionFromModule(CallGraphNode *CGN) {
return F;
}
// changeFunction - This method changes the function associated with this
// CallGraphNode, for use by transformations that need to change the prototype
// of a Function (thus they must create a new Function and move the old code
// over).
void CallGraph::changeFunction(Function *OldF, Function *NewF) {
iterator I = FunctionMap.find(OldF);
CallGraphNode *&New = FunctionMap[NewF];
assert(I != FunctionMap.end() && I->second && !New &&
"OldF didn't exist in CG or NewF already does!");
New = I->second;
New->F = NewF;
FunctionMap.erase(I);
}
// getOrInsertFunction - This method is identical to calling operator[], but
// it will insert a new CallGraphNode for the specified function if one does
// not already exist.
@ -233,7 +219,8 @@ CallGraphNode *CallGraph::getOrInsertFunction(const Function *F) {
void CallGraphNode::print(raw_ostream &OS) const {
if (Function *F = getFunction())
OS << "Call graph node for function: '" << F->getName() <<"'\n";
OS << "Call graph node for function: '" << F->getName()
<< "'<<0x" << this << ">>\n";
else
OS << "Call graph node <<null function: 0x" << this << ">>:\n";
@ -289,11 +276,17 @@ void CallGraphNode::removeOneAbstractEdgeTo(CallGraphNode *Callee) {
/// replaceCallSite - Make the edge in the node for Old CallSite be for
/// New CallSite instead. Note that this method takes linear time, so it
/// should be used sparingly.
void CallGraphNode::replaceCallSite(CallSite Old, CallSite New) {
void CallGraphNode::replaceCallSite(CallSite Old, CallSite New,
CallGraphNode *NewCallee) {
for (CalledFunctionsVector::iterator I = CalledFunctions.begin(); ; ++I) {
assert(I != CalledFunctions.end() && "Cannot find callsite to replace!");
if (I->first == Old) {
I->first = New;
// If the callee is changing, not just the callsite, then update it as
// well.
if (NewCallee)
I->second = NewCallee;
return;
}
}

20
llvm/lib/Analysis/IPA/CallGraphSCCPass.cpp
View File

@ -84,10 +84,16 @@ bool CGPassManager::runOnModule(Module &M) {
CallGraph &CG = getAnalysis<CallGraph>();
bool Changed = doInitialization(CG);
std::vector<CallGraphNode*> CurSCC;
// Walk SCC
for (scc_iterator<CallGraph*> I = scc_begin(&CG), E = scc_end(&CG);
I != E; ++I) {
for (scc_iterator<CallGraph*> CGI = scc_begin(&CG), E = scc_end(&CG);
CGI != E;) {
// Copy the current SCC and increment past it so that the pass can hack
// on the SCC if it wants to without invalidating our iterator.
CurSCC = *CGI;
++CGI;
// Run all passes on current SCC
for (unsigned Index = 0; Index < getNumContainedPasses(); ++Index) {
Pass *P = getContainedPass(Index);
@ -99,16 +105,14 @@ bool CGPassManager::runOnModule(Module &M) {
StartPassTimer(P);
if (CallGraphSCCPass *CGSP = dynamic_cast<CallGraphSCCPass *>(P))
Changed |= CGSP->runOnSCC(*I); // TODO : What if CG is changed ?
Changed |= CGSP->runOnSCC(CurSCC);
else {
FPPassManager *FPP = dynamic_cast<FPPassManager *>(P);
assert (FPP && "Invalid CGPassManager member");
// Run pass P on all functions current SCC
std::vector<CallGraphNode*> &SCC = *I;
for (unsigned i = 0, e = SCC.size(); i != e; ++i) {
Function *F = SCC[i]->getFunction();
if (F) {
for (unsigned i = 0, e = CurSCC.size(); i != e; ++i) {
if (Function *F = CurSCC[i]->getFunction()) {
dumpPassInfo(P, EXECUTION_MSG, ON_FUNCTION_MSG, F->getName());
Changed |= FPP->runOnFunction(*F);
}

52
llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
View File

@ -65,7 +65,7 @@ namespace {
CallGraphSCCPass::getAnalysisUsage(AU);
}
virtual bool runOnSCC(const std::vector<CallGraphNode *> &SCC);
virtual bool runOnSCC(std::vector<CallGraphNode *> &SCC);
static char ID; // Pass identification, replacement for typeid
explicit ArgPromotion(unsigned maxElements = 3)
: CallGraphSCCPass(&ID), maxElements(maxElements) {}
@ -74,11 +74,11 @@ namespace {
typedef std::vector<uint64_t> IndicesVector;
private:
bool PromoteArguments(CallGraphNode *CGN);
CallGraphNode *PromoteArguments(CallGraphNode *CGN);
bool isSafeToPromoteArgument(Argument *Arg, bool isByVal) const;
Function *DoPromotion(Function *F,
SmallPtrSet<Argument*, 8> &ArgsToPromote,
SmallPtrSet<Argument*, 8> &ByValArgsToTransform);
CallGraphNode *DoPromotion(Function *F,
SmallPtrSet<Argument*, 8> &ArgsToPromote,
SmallPtrSet<Argument*, 8> &ByValArgsToTransform);
/// The maximum number of elements to expand, or 0 for unlimited.
unsigned maxElements;
};
@ -92,14 +92,17 @@ Pass *llvm::createArgumentPromotionPass(unsigned maxElements) {
return new ArgPromotion(maxElements);
}
bool ArgPromotion::runOnSCC(const std::vector<CallGraphNode *> &SCC) {
bool ArgPromotion::runOnSCC(std::vector<CallGraphNode *> &SCC) {
bool Changed = false, LocalChange;
do { // Iterate until we stop promoting from this SCC.
LocalChange = false;
// Attempt to promote arguments from all functions in this SCC.
for (unsigned i = 0, e = SCC.size(); i != e; ++i)
LocalChange |= PromoteArguments(SCC[i]);
if (CallGraphNode *CGN = PromoteArguments(SCC[i])) {
LocalChange = true;
SCC[i] = CGN;
}
Changed |= LocalChange; // Remember that we changed something.
} while (LocalChange);
@ -111,11 +114,11 @@ bool ArgPromotion::runOnSCC(const std::vector<CallGraphNode *> &SCC) {
/// example, all callers are direct). If safe to promote some arguments, it
/// calls the DoPromotion method.
///
bool ArgPromotion::PromoteArguments(CallGraphNode *CGN) {
CallGraphNode *ArgPromotion::PromoteArguments(CallGraphNode *CGN) {
Function *F = CGN->getFunction();
// Make sure that it is local to this module.
if (!F || !F->hasLocalLinkage()) return false;
if (!F || !F->hasLocalLinkage()) return 0;
// First check: see if there are any pointer arguments! If not, quick exit.
SmallVector<std::pair<Argument*, unsigned>, 16> PointerArgs;
@ -124,12 +127,12 @@ bool ArgPromotion::PromoteArguments(CallGraphNode *CGN) {
I != E; ++I, ++ArgNo)
if (isa<PointerType>(I->getType()))
PointerArgs.push_back(std::pair<Argument*, unsigned>(I, ArgNo));
if (PointerArgs.empty()) return false;
if (PointerArgs.empty()) return 0;
// Second check: make sure that all callers are direct callers. We can't
// transform functions that have indirect callers.
if (F->hasAddressTaken())
return false;
return 0;
// Check to see which arguments are promotable. If an argument is promotable,
// add it to ArgsToPromote.
@ -174,13 +177,10 @@ bool ArgPromotion::PromoteArguments(CallGraphNode *CGN) {
}
// No promotable pointer arguments.
if (ArgsToPromote.empty() && ByValArgsToTransform.empty()) return false;
if (ArgsToPromote.empty() && ByValArgsToTransform.empty())
return 0;
Function *NewF = DoPromotion(F, ArgsToPromote, ByValArgsToTransform);
// Update the call graph to know that the function has been transformed.
getAnalysis<CallGraph>().changeFunction(F, NewF);
return true;
return DoPromotion(F, ArgsToPromote, ByValArgsToTransform);
}
/// IsAlwaysValidPointer - Return true if the specified pointer is always legal
@ -469,8 +469,8 @@ bool ArgPromotion::isSafeToPromoteArgument(Argument *Arg, bool isByVal) const {
/// DoPromotion - This method actually performs the promotion of the specified
/// arguments, and returns the new function. At this point, we know that it's
/// safe to do so.
Function *ArgPromotion::DoPromotion(Function *F,
SmallPtrSet<Argument*, 8> &ArgsToPromote,
CallGraphNode *ArgPromotion::DoPromotion(Function *F,
SmallPtrSet<Argument*, 8> &ArgsToPromote,
SmallPtrSet<Argument*, 8> &ByValArgsToTransform) {
// Start by computing a new prototype for the function, which is the same as
@ -608,6 +608,10 @@ Function *ArgPromotion::DoPromotion(Function *F,
// Get the callgraph information that we need to update to reflect our
// changes.
CallGraph &CG = getAnalysis<CallGraph>();
// Get a new callgraph node for NF.
CallGraphNode *NF_CGN = CG.getOrInsertFunction(NF);
// Loop over all of the callers of the function, transforming the call sites
// to pass in the loaded pointers.
@ -720,7 +724,7 @@ Function *ArgPromotion::DoPromotion(Function *F,
AA.replaceWithNewValue(Call, New);
// Update the callgraph to know that the callsite has been transformed.
CG[Call->getParent()->getParent()]->replaceCallSite(Call, New);
CG[Call->getParent()->getParent()]->replaceCallSite(Call, New, NF_CGN);
if (!Call->use_empty()) {
Call->replaceAllUsesWith(New);
@ -856,7 +860,11 @@ Function *ArgPromotion::DoPromotion(Function *F,
// Tell the alias analysis that the old function is about to disappear.
AA.replaceWithNewValue(F, NF);
NF_CGN->stealCalledFunctionsFrom(CG[F]);
// Now that the old function is dead, delete it.
F->eraseFromParent();
return NF;
delete CG.removeFunctionFromModule(F);
return NF_CGN;
}

4
llvm/lib/Transforms/IPO/FunctionAttrs.cpp
View File

@ -44,7 +44,7 @@ namespace {
FunctionAttrs() : CallGraphSCCPass(&ID) {}
// runOnSCC - Analyze the SCC, performing the transformation if possible.
bool runOnSCC(const std::vector<CallGraphNode *> &SCC);
bool runOnSCC(std::vector<CallGraphNode *> &SCC);
// AddReadAttrs - Deduce readonly/readnone attributes for the SCC.
bool AddReadAttrs(const std::vector<CallGraphNode *> &SCC);
@ -339,7 +339,7 @@ bool FunctionAttrs::AddNoAliasAttrs(const std::vector<CallGraphNode *> &SCC) {
return MadeChange;
}
bool FunctionAttrs::runOnSCC(const std::vector<CallGraphNode *> &SCC) {
bool FunctionAttrs::runOnSCC(std::vector<CallGraphNode *> &SCC) {
bool Changed = AddReadAttrs(SCC);
Changed |= AddNoCaptureAttrs(SCC);
Changed |= AddNoAliasAttrs(SCC);

2
llvm/lib/Transforms/IPO/Inliner.cpp
View File

@ -207,7 +207,7 @@ bool Inliner::shouldInline(CallSite CS) {
return true;
}
bool Inliner::runOnSCC(const std::vector<CallGraphNode*> &SCC) {
bool Inliner::runOnSCC(std::vector<CallGraphNode*> &SCC) {
CallGraph &CG = getAnalysis<CallGraph>();
const TargetData *TD = getAnalysisIfAvailable<TargetData>();

6
llvm/lib/Transforms/IPO/PruneEH.cpp
View File

@ -41,7 +41,7 @@ namespace {
PruneEH() : CallGraphSCCPass(&ID) {}
// runOnSCC - Analyze the SCC, performing the transformation if possible.
bool runOnSCC(const std::vector<CallGraphNode *> &SCC);
bool runOnSCC(std::vector<CallGraphNode *> &SCC);
bool SimplifyFunction(Function *F);
void DeleteBasicBlock(BasicBlock *BB);
@ -55,7 +55,7 @@ X("prune-eh", "Remove unused exception handling info");
Pass *llvm::createPruneEHPass() { return new PruneEH(); }
bool PruneEH::runOnSCC(const std::vector<CallGraphNode *> &SCC) {
bool PruneEH::runOnSCC(std::vector<CallGraphNode *> &SCC) {
SmallPtrSet<CallGraphNode *, 8> SCCNodes;
CallGraph &CG = getAnalysis<CallGraph>();
bool MadeChange = false;
@ -187,7 +187,7 @@ bool PruneEH::SimplifyFunction(Function *F) {
UnwindBlock->removePredecessor(II->getParent());
// Fix up the call graph.
CGN->replaceCallSite(II, Call);
CGN->replaceCallSite(II, Call, 0/*keep callee*/);
// Insert a branch to the normal destination right before the
// invoke.

65
llvm/lib/Transforms/IPO/StructRetPromotion.cpp
View File

@ -49,15 +49,15 @@ namespace {
CallGraphSCCPass::getAnalysisUsage(AU);
}
virtual bool runOnSCC(const std::vector<CallGraphNode *> &SCC);
virtual bool runOnSCC(std::vector<CallGraphNode *> &SCC);
static char ID; // Pass identification, replacement for typeid
SRETPromotion() : CallGraphSCCPass(&ID) {}
private:
bool PromoteReturn(CallGraphNode *CGN);
CallGraphNode *PromoteReturn(CallGraphNode *CGN);
bool isSafeToUpdateAllCallers(Function *F);
Function *cloneFunctionBody(Function *F, const StructType *STy);
void updateCallSites(Function *F, Function *NF);
CallGraphNode *updateCallSites(Function *F, Function *NF);
bool nestedStructType(const StructType *STy);
};
}
@ -70,44 +70,47 @@ Pass *llvm::createStructRetPromotionPass() {
return new SRETPromotion();
}
bool SRETPromotion::runOnSCC(const std::vector<CallGraphNode *> &SCC) {
bool SRETPromotion::runOnSCC(std::vector<CallGraphNode *> &SCC) {
bool Changed = false;
for (unsigned i = 0, e = SCC.size(); i != e; ++i)
Changed |= PromoteReturn(SCC[i]);
if (CallGraphNode *NewNode = PromoteReturn(SCC[i])) {
SCC[i] = NewNode;
Changed = true;
}
return Changed;
}
/// PromoteReturn - This method promotes function that uses StructRet paramater
/// into a function that uses mulitple return value.
bool SRETPromotion::PromoteReturn(CallGraphNode *CGN) {
/// into a function that uses multiple return values.
CallGraphNode *SRETPromotion::PromoteReturn(CallGraphNode *CGN) {
Function *F = CGN->getFunction();
if (!F || F->isDeclaration() || !F->hasLocalLinkage())
return false;
return 0;
// Make sure that function returns struct.
if (F->arg_size() == 0 || !F->hasStructRetAttr() || F->doesNotReturn())
return false;
return 0;
DEBUG(errs() << "SretPromotion: Looking at sret function "
<< F->getName() << "\n");
assert (F->getReturnType() == Type::getVoidTy(F->getContext()) &&
"Invalid function return type");
assert(F->getReturnType() == Type::getVoidTy(F->getContext()) &&
"Invalid function return type");
Function::arg_iterator AI = F->arg_begin();
const llvm::PointerType *FArgType = dyn_cast<PointerType>(AI->getType());
assert (FArgType && "Invalid sret parameter type");
assert(FArgType && "Invalid sret parameter type");
const llvm::StructType *STy =
dyn_cast<StructType>(FArgType->getElementType());
assert (STy && "Invalid sret parameter element type");
assert(STy && "Invalid sret parameter element type");
// Check if it is ok to perform this promotion.
if (isSafeToUpdateAllCallers(F) == false) {
DEBUG(errs() << "SretPromotion: Not all callers can be updated\n");
NumRejectedSRETUses++;
return false;
return 0;
}
DEBUG(errs() << "SretPromotion: sret argument will be promoted\n");
@ -135,11 +138,13 @@ bool SRETPromotion::PromoteReturn(CallGraphNode *CGN) {
Function *NF = cloneFunctionBody(F, STy);
// [4] Update all call sites to use new function
updateCallSites(F, NF);
CallGraphNode *NF_CFN = updateCallSites(F, NF);
F->eraseFromParent();
getAnalysis<CallGraph>().changeFunction(F, NF);
return true;
CallGraph &CG = getAnalysis<CallGraph>();
NF_CFN->stealCalledFunctionsFrom(CG[F]);
delete CG.removeFunctionFromModule(F);
return NF_CFN;
}
// Check if it is ok to perform this promotion.
@ -247,23 +252,26 @@ Function *SRETPromotion::cloneFunctionBody(Function *F,
Function::arg_iterator NI = NF->arg_begin();
++I;
while (I != E) {
I->replaceAllUsesWith(NI);
NI->takeName(I);
++I;
++NI;
I->replaceAllUsesWith(NI);
NI->takeName(I);
++I;
++NI;
}
return NF;
}
/// updateCallSites - Update all sites that call F to use NF.
void SRETPromotion::updateCallSites(Function *F, Function *NF) {
CallGraphNode *SRETPromotion::updateCallSites(Function *F, Function *NF) {
CallGraph &CG = getAnalysis<CallGraph>();
SmallVector<Value*, 16> Args;
// Attributes - Keep track of the parameter attributes for the arguments.
SmallVector<AttributeWithIndex, 8> ArgAttrsVec;
// Get a new callgraph node for NF.
CallGraphNode *NF_CGN = CG.getOrInsertFunction(NF);
while (!F->use_empty()) {
CallSite CS = CallSite::get(*F->use_begin());
Instruction *Call = CS.getInstruction();
@ -313,7 +321,7 @@ void SRETPromotion::updateCallSites(Function *F, Function *NF) {
New->takeName(Call);
// Update the callgraph to know that the callsite has been transformed.
CG[Call->getParent()->getParent()]->replaceCallSite(Call, New);
CG[Call->getParent()->getParent()]->replaceCallSite(Call, New, NF_CGN);
// Update all users of sret parameter to extract value using extractvalue.
for (Value::use_iterator UI = FirstCArg->use_begin(),
@ -322,7 +330,8 @@ void SRETPromotion::updateCallSites(Function *F, Function *NF) {
CallInst *C2 = dyn_cast<CallInst>(U2);
if (C2 && (C2 == Call))
continue;
else if (GetElementPtrInst *UGEP = dyn_cast<GetElementPtrInst>(U2)) {
if (GetElementPtrInst *UGEP = dyn_cast<GetElementPtrInst>(U2)) {
ConstantInt *Idx = dyn_cast<ConstantInt>(UGEP->getOperand(2));
assert (Idx && "Unexpected getelementptr index!");
Value *GR = ExtractValueInst::Create(New, Idx->getZExtValue(),
@ -335,11 +344,15 @@ void SRETPromotion::updateCallSites(Function *F, Function *NF) {
L->eraseFromParent();
}
UGEP->eraseFromParent();
continue;
}
else assert( 0 && "Unexpected sret parameter use");
assert(0 && "Unexpected sret parameter use");
}
Call->eraseFromParent();
}
return NF_CGN;
}
/// nestedStructType - Return true if STy includes any

2
llvm/tools/opt/opt.cpp
View File

@ -132,7 +132,7 @@ struct CallGraphSCCPassPrinter : public CallGraphSCCPass {
CallGraphSCCPassPrinter(const PassInfo *PI) :
CallGraphSCCPass(&ID), PassToPrint(PI) {}
virtual bool runOnSCC(const std::vector<CallGraphNode *>&SCC) {
virtual bool runOnSCC(std::vector<CallGraphNode *>&SCC) {
if (!Quiet) {
outs() << "Printing analysis '" << PassToPrint->getPassName() << "':\n";