[llvm-commits] [llvm] r80527 - in /llvm/trunk: include/llvm/Analysis/CallGraph.h include/llvm/CallGraphSCCPass.h include/llvm/Transforms/IPO/InlinerPass.h lib/Analysis/IPA/CallGraph.cpp lib/Analysis/IPA/CallGraphSCCPass.cpp lib/Transforms/IPO/ArgumentPromotion.cpp lib/Transforms/IPO/FunctionAttrs.cpp lib/Transforms/IPO/Inliner.cpp lib/Transforms/IPO/PruneEH.cpp lib/Transforms/IPO/StructRetPromotion.cpp tools/opt/opt.cpp

Chris Lattner sabre at nondot.org
Sun Aug 30 17:19:59 PDT 2009


Author: lattner
Date: Sun Aug 30 19:19:58 2009
New Revision: 80527

URL: http://llvm.org/viewvc/llvm-project?rev=80527&view=rev
Log:
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.


Modified:
    llvm/trunk/include/llvm/Analysis/CallGraph.h
    llvm/trunk/include/llvm/CallGraphSCCPass.h
    llvm/trunk/include/llvm/Transforms/IPO/InlinerPass.h
    llvm/trunk/lib/Analysis/IPA/CallGraph.cpp
    llvm/trunk/lib/Analysis/IPA/CallGraphSCCPass.cpp
    llvm/trunk/lib/Transforms/IPO/ArgumentPromotion.cpp
    llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp
    llvm/trunk/lib/Transforms/IPO/Inliner.cpp
    llvm/trunk/lib/Transforms/IPO/PruneEH.cpp
    llvm/trunk/lib/Transforms/IPO/StructRetPromotion.cpp
    llvm/trunk/tools/opt/opt.cpp

Modified: llvm/trunk/include/llvm/Analysis/CallGraph.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/CallGraph.h?rev=80527&r1=80526&r2=80527&view=diff

==============================================================================
--- llvm/trunk/include/llvm/Analysis/CallGraph.h (original)
+++ llvm/trunk/include/llvm/Analysis/CallGraph.h Sun Aug 30 19:19:58 2009
@@ -130,12 +130,6 @@
     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 @@
   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 @@
   /// 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;
 

Modified: llvm/trunk/include/llvm/CallGraphSCCPass.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CallGraphSCCPass.h?rev=80527&r1=80526&r2=80527&view=diff

==============================================================================
--- llvm/trunk/include/llvm/CallGraphSCCPass.h (original)
+++ llvm/trunk/include/llvm/CallGraphSCCPass.h Sun Aug 30 19:19:58 2009
@@ -46,7 +46,10 @@
   /// 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.

Modified: llvm/trunk/include/llvm/Transforms/IPO/InlinerPass.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/IPO/InlinerPass.h?rev=80527&r1=80526&r2=80527&view=diff

==============================================================================
--- llvm/trunk/include/llvm/Transforms/IPO/InlinerPass.h (original)
+++ llvm/trunk/include/llvm/Transforms/IPO/InlinerPass.h Sun Aug 30 19:19:58 2009
@@ -40,7 +40,7 @@
 
   // 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.

Modified: llvm/trunk/lib/Analysis/IPA/CallGraph.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/IPA/CallGraph.cpp?rev=80527&r1=80526&r2=80527&view=diff

==============================================================================
--- llvm/trunk/lib/Analysis/IPA/CallGraph.cpp (original)
+++ llvm/trunk/lib/Analysis/IPA/CallGraph.cpp Sun Aug 30 19:19:58 2009
@@ -206,20 +206,6 @@
   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 @@
 
 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 @@
 /// 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;
     }
   }

Modified: llvm/trunk/lib/Analysis/IPA/CallGraphSCCPass.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/IPA/CallGraphSCCPass.cpp?rev=80527&r1=80526&r2=80527&view=diff

==============================================================================
--- llvm/trunk/lib/Analysis/IPA/CallGraphSCCPass.cpp (original)
+++ llvm/trunk/lib/Analysis/IPA/CallGraphSCCPass.cpp Sun Aug 30 19:19:58 2009
@@ -84,10 +84,16 @@
   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 @@
 
       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);
           }

Modified: llvm/trunk/lib/Transforms/IPO/ArgumentPromotion.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/ArgumentPromotion.cpp?rev=80527&r1=80526&r2=80527&view=diff

==============================================================================
--- llvm/trunk/lib/Transforms/IPO/ArgumentPromotion.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/ArgumentPromotion.cpp Sun Aug 30 19:19:58 2009
@@ -65,7 +65,7 @@
       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 @@
     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 @@
   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 @@
 /// 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 @@
        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 @@
   }
 
   // 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 @@
 /// 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 @@
   // 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 @@
     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 @@
   // 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;
 }

Modified: llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp?rev=80527&r1=80526&r2=80527&view=diff

==============================================================================
--- llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp Sun Aug 30 19:19:58 2009
@@ -44,7 +44,7 @@
     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 @@
   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);

Modified: llvm/trunk/lib/Transforms/IPO/Inliner.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/Inliner.cpp?rev=80527&r1=80526&r2=80527&view=diff

==============================================================================
--- llvm/trunk/lib/Transforms/IPO/Inliner.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/Inliner.cpp Sun Aug 30 19:19:58 2009
@@ -207,7 +207,7 @@
   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>();
 

Modified: llvm/trunk/lib/Transforms/IPO/PruneEH.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/PruneEH.cpp?rev=80527&r1=80526&r2=80527&view=diff

==============================================================================
--- llvm/trunk/lib/Transforms/IPO/PruneEH.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/PruneEH.cpp Sun Aug 30 19:19:58 2009
@@ -41,7 +41,7 @@
     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 @@
 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 @@
         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.

Modified: llvm/trunk/lib/Transforms/IPO/StructRetPromotion.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/StructRetPromotion.cpp?rev=80527&r1=80526&r2=80527&view=diff

==============================================================================
--- llvm/trunk/lib/Transforms/IPO/StructRetPromotion.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/StructRetPromotion.cpp Sun Aug 30 19:19:58 2009
@@ -49,15 +49,15 @@
       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 @@
   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 @@
   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::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 @@
     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 @@
       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 @@
           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

Modified: llvm/trunk/tools/opt/opt.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/opt/opt.cpp?rev=80527&r1=80526&r2=80527&view=diff

==============================================================================
--- llvm/trunk/tools/opt/opt.cpp (original)
+++ llvm/trunk/tools/opt/opt.cpp Sun Aug 30 19:19:58 2009
@@ -132,7 +132,7 @@
   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";
 





More information about the llvm-commits mailing list