[llvm-commits] CVS: llvm/lib/Transforms/IPO/InlineSimple.cpp Inliner.cpp Inliner.h

Chris Lattner lattner at cs.uiuc.edu
Thu Apr 8 01:35:04 PDT 2004


Changes in directory llvm/lib/Transforms/IPO:

InlineSimple.cpp updated: 1.61 -> 1.62
Inliner.cpp updated: 1.6 -> 1.7
Inliner.h updated: 1.4 -> 1.5

---
Log message:

Remove the "really gross hacks" that are there to deal with recursive functions.
Now we collect all of the call sites we are interested in inlining, then inline
them.  This entirely avoids issues with trying to inline a call site we got by
inlining another call site.  This also eliminates iterator invalidation issues.


---
Diffs of the changes:  (+57 -86)

Index: llvm/lib/Transforms/IPO/InlineSimple.cpp
diff -u llvm/lib/Transforms/IPO/InlineSimple.cpp:1.61 llvm/lib/Transforms/IPO/InlineSimple.cpp:1.62
--- llvm/lib/Transforms/IPO/InlineSimple.cpp:1.61	Mon Mar 15 00:38:14 2004
+++ llvm/lib/Transforms/IPO/InlineSimple.cpp	Thu Apr  8 01:34:31 2004
@@ -217,13 +217,7 @@
   // Don't inline into something too big, which would make it bigger.  Here, we
   // count each basic block as a single unit.
   //
-  // FIXME: THIS IS A TOTAL HACK.  The problem is that we don't keep track of
-  // which call sites are the result of an inlining operation.  Because of this,
-  // if we inline a recursive function into a callee, we will see a new call to
-  // the recursive function.  Every time we inline we get a new callsite for the
-  // function, which only stops when the caller reaches its inlining limit.
-  // Until the real problem is fixed, we apply this gnasty hack.
-  InlineCost += Caller->size();
+  InlineCost += Caller->size()/20;
 
 
   // Look at the size of the callee.  Each basic block counts as 20 units, and


Index: llvm/lib/Transforms/IPO/Inliner.cpp
diff -u llvm/lib/Transforms/IPO/Inliner.cpp:1.6 llvm/lib/Transforms/IPO/Inliner.cpp:1.7
--- llvm/lib/Transforms/IPO/Inliner.cpp:1.6	Fri Nov 21 15:45:31 2003
+++ llvm/lib/Transforms/IPO/Inliner.cpp	Thu Apr  8 01:34:31 2004
@@ -58,84 +58,67 @@
     if (F == 0 || F->isExternal()) continue;
     DEBUG(std::cerr << "  Inspecting function: " << F->getName() << "\n");
 
+    // Scan through and identify all call sites ahead of time so that we only
+    // inline call sites in the original functions, not call sites that result
+    // in inlining other functions.
+    std::vector<CallSite> CallSites;
     for (Function::iterator BB = F->begin(), E = F->end(); BB != E; ++BB)
-      for (BasicBlock::iterator I = BB->begin(); I != BB->end(); ) {
-        bool ShouldInc = true;
-        // Found a call or invoke instruction?
-        if (isa<CallInst>(I) || isa<InvokeInst>(I)) {
-          CallSite CS = CallSite::get(I);
-          if (Function *Callee = CS.getCalledFunction())
-            if (!Callee->isExternal() && !IsRecursiveFunction.count(Callee)) {
-              // Determine whether this is a function IN the SCC...
-              bool inSCC = SCCFunctions.count(Callee);
-
-              // Keep track of whether this is a directly recursive function.
-              if (Callee == F) IsRecursiveFunction.insert(F);
-
-              // If the policy determines that we should inline this function,
-              // try to do so...
-              int InlineCost = inSCC ? getRecursiveInlineCost(CS) :
-                                       getInlineCost(CS);
-              if (InlineCost >= (int)InlineThreshold) {
-                DEBUG(std::cerr << "    NOT Inlining: cost=" << InlineCost
-                                << ", Call: " << *CS.getInstruction());
-              } else {
-                DEBUG(std::cerr << "    Inlining: cost=" << InlineCost
-                                << ", Call: " << *CS.getInstruction());
+      for (BasicBlock::iterator I = BB->begin(); I != BB->end(); ++I) {
+        CallSite CS = CallSite::get(I);
+        if (CS.getInstruction() && CS.getCalledFunction() &&
+            !CS.getCalledFunction()->isExternal())
+          CallSites.push_back(CS);
+      }
 
-                // Save an iterator to the instruction before the call if it
-                // exists, otherwise get an iterator at the end of the
-                // block... because the call will be destroyed.
-                //
-                BasicBlock::iterator SI;
-                if (I != BB->begin()) {
-                  SI = I; --SI;           // Instruction before the call...
-                } else {
-                  SI = BB->end();
-                }
-                
-                if (performInlining(CS, SCCFunctions)) {
-                  // Move to instruction before the call...
-                  I = (SI == BB->end()) ? BB->begin() : SI;
-                  ShouldInc = false; // Don't increment iterator until next time
-                  Changed = true;
-                }
-              }
-            }
+    // Now that we have all of the call sites, loop over them and inline them if
+    // it looks profitable to do so.
+    for (unsigned i = 0, e = CallSites.size(); i != e; ++i) {
+      CallSite CS = CallSites[i];
+      Function *Callee = CS.getCalledFunction();
+      // Determine whether this is a function IN the SCC...
+      bool inSCC = SCCFunctions.count(Callee);
+    
+      // If the policy determines that we should inline this function,
+      // try to do so...
+      int InlineCost = inSCC ? getRecursiveInlineCost(CS) : getInlineCost(CS);
+      if (InlineCost >= (int)InlineThreshold) {
+        DEBUG(std::cerr << "    NOT Inlining: cost=" << InlineCost
+              << ", Call: " << *CS.getInstruction());
+      } else {
+        DEBUG(std::cerr << "    Inlining: cost=" << InlineCost
+              << ", Call: " << *CS.getInstruction());
+      
+        Function *Caller = CS.getInstruction()->getParent()->getParent();
+
+        // Attempt to inline the function...
+        if (InlineFunction(CS)) {
+          ++NumInlined;
+  
+          if (Callee->hasOneUse())
+            if (ConstantPointerRef *CPR =
+                dyn_cast<ConstantPointerRef>(Callee->use_back()))
+              if (CPR->use_empty())
+                CPR->destroyConstant();
+        
+          // If we inlined the last possible call site to the function,
+          // delete the function body now.
+          if (Callee->use_empty() && Callee != Caller &&
+              (Callee->hasInternalLinkage() || Callee->hasLinkOnceLinkage())) {
+            DEBUG(std::cerr << "    -> Deleting dead function: "
+                  << (void*)Callee << Callee->getName() << "\n");
+            std::set<Function*>::iterator I = SCCFunctions.find(Callee);
+            if (I != SCCFunctions.end())    // Remove function from this SCC.
+              SCCFunctions.erase(I);
+
+            Callee->getParent()->getFunctionList().erase(Callee);
+            ++NumDeleted;
+          }
+          Changed = true;
         }
-        if (ShouldInc) ++I;
       }
+    }
   }
-  return Changed;
-}
-
-bool Inliner::performInlining(CallSite CS, std::set<Function*> &SCC) {
-  Function *Callee = CS.getCalledFunction();
-  Function *Caller = CS.getInstruction()->getParent()->getParent();
 
-  // Attempt to inline the function...
-  if (!InlineFunction(CS)) return false;
-  ++NumInlined;
-  
-  if (Callee->hasOneUse())
-    if (ConstantPointerRef *CPR =
-        dyn_cast<ConstantPointerRef>(Callee->use_back()))
-      if (CPR->use_empty())
-        CPR->destroyConstant();
-  
-  // If we inlined the last possible call site to the function,
-  // delete the function body now.
-  if (Callee->use_empty() && Callee != Caller &&
-      (Callee->hasInternalLinkage() || Callee->hasLinkOnceLinkage())) {
-    DEBUG(std::cerr << "    -> Deleting dead function: " << (void*)Callee
-                    << Callee->getName() << "\n");
-    std::set<Function*>::iterator I = SCC.find(Callee);
-    if (I != SCC.end())       // Remove function from this SCC...
-      SCC.erase(I);
-
-    Callee->getParent()->getFunctionList().erase(Callee);
-    ++NumDeleted;
-  }
-  return true; 
+  return Changed;
 }
 


Index: llvm/lib/Transforms/IPO/Inliner.h
diff -u llvm/lib/Transforms/IPO/Inliner.h:1.4 llvm/lib/Transforms/IPO/Inliner.h:1.5
--- llvm/lib/Transforms/IPO/Inliner.h:1.4	Tue Nov 11 16:41:33 2003
+++ llvm/lib/Transforms/IPO/Inliner.h	Thu Apr  8 01:34:31 2004
@@ -56,12 +56,6 @@
 private:
   // InlineThreshold - Cache the value here for easy access.
   unsigned InlineThreshold;
-
-  // IsRecursiveFunction - This contains all functions which are directly
-  // recursive, which we do NOT want to inline into other functions.
-  std::set<Function*> IsRecursiveFunction;
-
-  bool performInlining(CallSite CS, std::set<Function*> &SCC);
 };
 
 } // End llvm namespace





More information about the llvm-commits mailing list