For PR950:
[oota-llvm.git] / lib / Transforms / Utils / CodeExtractor.cpp
index 530d2f66b731ddad41850a01f29ed75ed0d573a9..ba403450a76fbda59f836c1189d6a4605ed65d6e 100644 (file)
@@ -1,10 +1,10 @@
 //===- CodeExtractor.cpp - Pull code region into a new function -----------===//
-// 
+//
 //                     The LLVM Compiler Infrastructure
 //
 // This file was developed by the LLVM research group and is distributed under
 // the University of Illinois Open Source License. See LICENSE.TXT for details.
-// 
+//
 //===----------------------------------------------------------------------===//
 //
 // This file implements the interface to tear out a code region, such as an
 //
 //===----------------------------------------------------------------------===//
 
-#include "llvm/BasicBlock.h"
+#include "llvm/Transforms/Utils/FunctionUtils.h"
 #include "llvm/Constants.h"
 #include "llvm/DerivedTypes.h"
 #include "llvm/Instructions.h"
+#include "llvm/Intrinsics.h"
 #include "llvm/Module.h"
 #include "llvm/Pass.h"
+#include "llvm/Analysis/Dominators.h"
 #include "llvm/Analysis/LoopInfo.h"
+#include "llvm/Analysis/Verifier.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
-#include "llvm/Transforms/Utils/FunctionUtils.h"
-#include "Support/Debug.h"
-#include "Support/StringExtras.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/ADT/StringExtras.h"
 #include <algorithm>
-#include <map>
-#include <vector>
+#include <set>
+#include <iostream>
 using namespace llvm;
 
-namespace {
-
-  inline bool contains(const std::vector<BasicBlock*> &V, const BasicBlock *BB){
-    return std::find(V.begin(), V.end(), BB) != V.end();
-  }
-
-  /// getFunctionArg - Return a pointer to F's ARGNOth argument.
-  ///
-  Argument *getFunctionArg(Function *F, unsigned argno) {
-    Function::aiterator ai = F->abegin();
-    while (argno) { ++ai; --argno; }
-    return &*ai;
-  }
+// Provide a command-line option to aggregate function arguments into a struct
+// for functions produced by the code extrator. This is useful when converting
+// extracted functions to pthread-based code, as only one argument (void*) can
+// be passed in to pthread_create().
+static cl::opt<bool>
+AggregateArgsOpt("aggregate-extracted-args", cl::Hidden,
+                 cl::desc("Aggregate arguments to code-extracted functions"));
 
-  struct CodeExtractor {
+namespace {
+  class CodeExtractor {
     typedef std::vector<Value*> Values;
-    typedef std::vector<std::pair<unsigned, unsigned> > PhiValChangesTy;
-    typedef std::map<PHINode*, PhiValChangesTy> PhiVal2ArgTy;
-    PhiVal2ArgTy PhiVal2Arg;
-
+    std::set<BasicBlock*> BlocksToExtract;
+    DominatorSet *DS;
+    bool AggregateArgs;
+    unsigned NumExitBlocks;
+    const Type *RetTy;
   public:
+    CodeExtractor(DominatorSet *ds = 0, bool AggArgs = false)
+      : DS(ds), AggregateArgs(AggArgs||AggregateArgsOpt), NumExitBlocks(~0U) {}
+
     Function *ExtractCodeRegion(const std::vector<BasicBlock*> &code);
 
+    bool isEligible(const std::vector<BasicBlock*> &code);
+
   private:
-    void findInputsOutputs(const std::vector<BasicBlock*> &code,
-                           Values &inputs,
-                           Values &outputs,
-                           BasicBlock *newHeader,
-                           BasicBlock *newRootNode);
+    /// definedInRegion - Return true if the specified value is defined in the
+    /// extracted region.
+    bool definedInRegion(Value *V) const {
+      if (Instruction *I = dyn_cast<Instruction>(V))
+        if (BlocksToExtract.count(I->getParent()))
+          return true;
+      return false;
+    }
 
-    void processPhiNodeInputs(PHINode *Phi,
-                              const std::vector<BasicBlock*> &code,
-                              Values &inputs,
-                              BasicBlock *newHeader,
-                              BasicBlock *newRootNode);
+    /// definedInCaller - Return true if the specified value is defined in the
+    /// function being code extracted, but not in the region being extracted.
+    /// These values must be passed in as live-ins to the function.
+    bool definedInCaller(Value *V) const {
+      if (isa<Argument>(V)) return true;
+      if (Instruction *I = dyn_cast<Instruction>(V))
+        if (!BlocksToExtract.count(I->getParent()))
+          return true;
+      return false;
+    }
 
-    void rewritePhiNodes(Function *F, BasicBlock *newFuncRoot);
+    void severSplitPHINodes(BasicBlock *&Header);
+    void splitReturnBlocks();
+    void findInputsOutputs(Values &inputs, Values &outputs);
 
     Function *constructFunction(const Values &inputs,
                                 const Values &outputs,
+                                BasicBlock *header,
                                 BasicBlock *newRootNode, BasicBlock *newHeader,
-                                const std::vector<BasicBlock*> &code,
                                 Function *oldFunction, Module *M);
 
-    void moveCodeToFunction(const std::vector<BasicBlock*> &code,
-                            Function *newFunction);
+    void moveCodeToFunction(Function *newFunction);
 
     void emitCallAndSwitchStatement(Function *newFunction,
                                     BasicBlock *newHeader,
-                                    const std::vector<BasicBlock*> &code,
                                     Values &inputs,
                                     Values &outputs);
 
   };
 }
 
-void CodeExtractor::processPhiNodeInputs(PHINode *Phi,
-                                         const std::vector<BasicBlock*> &code,
-                                         Values &inputs,
-                                         BasicBlock *codeReplacer,
-                                         BasicBlock *newFuncRoot)
-{
-  // Separate incoming values and BasicBlocks as internal/external. We ignore
-  // the case where both the value and BasicBlock are internal, because we don't
-  // need to do a thing.
-  std::vector<unsigned> EValEBB;
-  std::vector<unsigned> EValIBB;
-  std::vector<unsigned> IValEBB;
-
-  for (unsigned i = 0, e = Phi->getNumIncomingValues(); i != e; ++i) {
-    Value *phiVal = Phi->getIncomingValue(i);
-    if (Instruction *Inst = dyn_cast<Instruction>(phiVal)) {
-      if (contains(code, Inst->getParent())) {
-        if (!contains(code, Phi->getIncomingBlock(i)))
-          IValEBB.push_back(i);
-      } else {
-        if (contains(code, Phi->getIncomingBlock(i)))
-          EValIBB.push_back(i);
-        else
-          EValEBB.push_back(i);
-      }
-    } else if (Constant *Const = dyn_cast<Constant>(phiVal)) {
-      // Constants are internal, but considered `external' if they are coming
-      // from an external block.
-      if (!contains(code, Phi->getIncomingBlock(i)))
-        EValEBB.push_back(i);
-    } else if (Argument *Arg = dyn_cast<Argument>(phiVal)) {
-      // arguments are external
-      if (contains(code, Phi->getIncomingBlock(i)))
-        EValIBB.push_back(i);
+/// severSplitPHINodes - If a PHI node has multiple inputs from outside of the
+/// region, we need to split the entry block of the region so that the PHI node
+/// is easier to deal with.
+void CodeExtractor::severSplitPHINodes(BasicBlock *&Header) {
+  bool HasPredsFromRegion = false;
+  unsigned NumPredsOutsideRegion = 0;
+
+  if (Header != &Header->getParent()->front()) {
+    PHINode *PN = dyn_cast<PHINode>(Header->begin());
+    if (!PN) return;  // No PHI nodes.
+
+    // If the header node contains any PHI nodes, check to see if there is more
+    // than one entry from outside the region.  If so, we need to sever the
+    // header block into two.
+    for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i)
+      if (BlocksToExtract.count(PN->getIncomingBlock(i)))
+        HasPredsFromRegion = true;
       else
-        EValEBB.push_back(i);
-    } else {
-      phiVal->dump();
-      assert(0 && "Unhandled input in a Phi node");
-    }
-  }
+        ++NumPredsOutsideRegion;
 
-  // Both value and block are external. Need to group all of
-  // these, have an external phi, pass the result as an
-  // argument, and have THIS phi use that result.
-  if (EValEBB.size() > 0) {
-    if (EValEBB.size() == 1) {
-      // Now if it's coming from the newFuncRoot, it's that funky input
-      unsigned phiIdx = EValEBB[0];
-      if (!dyn_cast<Constant>(Phi->getIncomingValue(phiIdx)))
-      {
-        PhiVal2Arg[Phi].push_back(std::make_pair(phiIdx, inputs.size()));
-        // We can just pass this value in as argument
-        inputs.push_back(Phi->getIncomingValue(phiIdx));
-      }
-      Phi->setIncomingBlock(phiIdx, newFuncRoot);
-    } else {
-      PHINode *externalPhi = new PHINode(Phi->getType(), "extPhi");
-      codeReplacer->getInstList().insert(codeReplacer->begin(), externalPhi);
-      for (std::vector<unsigned>::iterator i = EValEBB.begin(),
-             e = EValEBB.end(); i != e; ++i)
-      {
-        externalPhi->addIncoming(Phi->getIncomingValue(*i),
-                                 Phi->getIncomingBlock(*i));
-
-        // We make these values invalid instead of deleting them because that
-        // would shift the indices of other values... The fixPhiNodes should
-        // clean these phi nodes up later.
-        Phi->setIncomingValue(*i, 0);
-        Phi->setIncomingBlock(*i, 0);
-      }
-      PhiVal2Arg[Phi].push_back(std::make_pair(Phi->getNumIncomingValues(),
-                                               inputs.size()));
-      // We can just pass this value in as argument
-      inputs.push_back(externalPhi);
-    }
+    // If there is one (or fewer) predecessor from outside the region, we don't
+    // need to do anything special.
+    if (NumPredsOutsideRegion <= 1) return;
   }
 
-  // When the value is external, but block internal...
-  // just pass it in as argument, no change to phi node
-  for (std::vector<unsigned>::iterator i = EValIBB.begin(),
-         e = EValIBB.end(); i != e; ++i)
-  {
-    // rewrite the phi input node to be an argument
-    PhiVal2Arg[Phi].push_back(std::make_pair(*i, inputs.size()));
-    inputs.push_back(Phi->getIncomingValue(*i));
+  // Otherwise, we need to split the header block into two pieces: one
+  // containing PHI nodes merging values from outside of the region, and a
+  // second that contains all of the code for the block and merges back any
+  // incoming values from inside of the region.
+  BasicBlock::iterator AfterPHIs = Header->begin();
+  while (isa<PHINode>(AfterPHIs)) ++AfterPHIs;
+  BasicBlock *NewBB = Header->splitBasicBlock(AfterPHIs,
+                                              Header->getName()+".ce");
+
+  // We only want to code extract the second block now, and it becomes the new
+  // header of the region.
+  BasicBlock *OldPred = Header;
+  BlocksToExtract.erase(OldPred);
+  BlocksToExtract.insert(NewBB);
+  Header = NewBB;
+
+  // Okay, update dominator sets. The blocks that dominate the new one are the
+  // blocks that dominate TIBB plus the new block itself.
+  if (DS) {
+    DominatorSet::DomSetType DomSet = DS->getDominators(OldPred);
+    DomSet.insert(NewBB);  // A block always dominates itself.
+    DS->addBasicBlock(NewBB, DomSet);
+
+    // Additionally, NewBB dominates all blocks in the function that are
+    // dominated by OldPred.
+    Function *F = Header->getParent();
+    for (Function::iterator I = F->begin(), E = F->end(); I != E; ++I)
+      if (DS->properlyDominates(OldPred, I))
+        DS->addDominator(I, NewBB);
   }
 
-  // Value internal, block external
-  // this can happen if we are extracting a part of a loop
-  for (std::vector<unsigned>::iterator i = IValEBB.begin(),
-         e = IValEBB.end(); i != e; ++i)
-  {
-    assert(0 && "Cannot (YET) handle internal values via external blocks");
-  }
-}
-
-
-void CodeExtractor::findInputsOutputs(const std::vector<BasicBlock*> &code,
-                                      Values &inputs,
-                                      Values &outputs,
-                                      BasicBlock *newHeader,
-                                      BasicBlock *newRootNode)
-{
-  for (std::vector<BasicBlock*>::const_iterator ci = code.begin(), 
-       ce = code.end(); ci != ce; ++ci) {
-    BasicBlock *BB = *ci;
-    for (BasicBlock::iterator BBi = BB->begin(), BBe = BB->end();
-         BBi != BBe; ++BBi) {
-      // If a use is defined outside the region, it's an input.
-      // If a def is used outside the region, it's an output.
-      if (Instruction *I = dyn_cast<Instruction>(&*BBi)) {
-        // If it's a phi node
-        if (PHINode *Phi = dyn_cast<PHINode>(I)) {
-          processPhiNodeInputs(Phi, code, inputs, newHeader, newRootNode);
-        } else {
-          // All other instructions go through the generic input finder
-          // Loop over the operands of each instruction (inputs)
-          for (User::op_iterator op = I->op_begin(), opE = I->op_end();
-               op != opE; ++op) {
-            if (Instruction *opI = dyn_cast<Instruction>(op->get())) {
-              // Check if definition of this operand is within the loop
-              if (!contains(code, opI->getParent())) {
-                // add this operand to the inputs
-                inputs.push_back(opI);
-              }
-            }
-          }
-        }
+  // Okay, now we need to adjust the PHI nodes and any branches from within the
+  // region to go to the new header block instead of the old header block.
+  if (HasPredsFromRegion) {
+    PHINode *PN = cast<PHINode>(OldPred->begin());
+    // Loop over all of the predecessors of OldPred that are in the region,
+    // changing them to branch to NewBB instead.
+    for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i)
+      if (BlocksToExtract.count(PN->getIncomingBlock(i))) {
+        TerminatorInst *TI = PN->getIncomingBlock(i)->getTerminator();
+        TI->replaceUsesOfWith(OldPred, NewBB);
+      }
 
-        // Consider uses of this instruction (outputs)
-        for (Value::use_iterator use = I->use_begin(), useE = I->use_end();
-             use != useE; ++use) {
-          if (Instruction* inst = dyn_cast<Instruction>(*use)) {
-            if (!contains(code, inst->getParent())) {
-              // add this op to the outputs
-              outputs.push_back(I);
-            }
-          }
+    // Okay, everthing within the region is now branching to the right block, we
+    // just have to update the PHI nodes now, inserting PHI nodes into NewBB.
+    for (AfterPHIs = OldPred->begin(); isa<PHINode>(AfterPHIs); ++AfterPHIs) {
+      PHINode *PN = cast<PHINode>(AfterPHIs);
+      // Create a new PHI node in the new region, which has an incoming value
+      // from OldPred of PN.
+      PHINode *NewPN = new PHINode(PN->getType(), PN->getName()+".ce",
+                                   NewBB->begin());
+      NewPN->addIncoming(PN, OldPred);
+
+      // Loop over all of the incoming value in PN, moving them to NewPN if they
+      // are from the extracted region.
+      for (unsigned i = 0; i != PN->getNumIncomingValues(); ++i) {
+        if (BlocksToExtract.count(PN->getIncomingBlock(i))) {
+          NewPN->addIncoming(PN->getIncomingValue(i), PN->getIncomingBlock(i));
+          PN->removeIncomingValue(i);
+          --i;
         }
-      } /* if */
-    } /* for: insts */
-  } /* for: basic blocks */
-}
-
-void CodeExtractor::rewritePhiNodes(Function *F,
-                                    BasicBlock *newFuncRoot) {
-  // Write any changes that were saved before: use function arguments as inputs
-  for (PhiVal2ArgTy::iterator i = PhiVal2Arg.begin(), e = PhiVal2Arg.end();
-       i != e; ++i)
-  {
-    PHINode *phi = (*i).first;
-    PhiValChangesTy &values = (*i).second;
-    for (unsigned cIdx = 0, ce = values.size(); cIdx != ce; ++cIdx)
-    {
-      unsigned phiValueIdx = values[cIdx].first, argNum = values[cIdx].second;
-      if (phiValueIdx < phi->getNumIncomingValues())
-        phi->setIncomingValue(phiValueIdx, getFunctionArg(F, argNum));
-      else
-        phi->addIncoming(getFunctionArg(F, argNum), newFuncRoot);
-    }
-  }
-
-  // Delete any invalid Phi node inputs that were marked as NULL previously
-  for (PhiVal2ArgTy::iterator i = PhiVal2Arg.begin(), e = PhiVal2Arg.end();
-       i != e; ++i)
-  {
-    PHINode *phi = (*i).first;
-    for (unsigned idx = 0, end = phi->getNumIncomingValues(); idx != end; ++idx)
-    {
-      if (phi->getIncomingValue(idx) == 0 && phi->getIncomingBlock(idx) == 0) {
-        phi->removeIncomingValue(idx);
-        --idx;
-        --end;
       }
     }
   }
+}
 
-  // We are done with the saved values
-  PhiVal2Arg.clear();
+void CodeExtractor::splitReturnBlocks() {
+  for (std::set<BasicBlock*>::iterator I = BlocksToExtract.begin(),
+         E = BlocksToExtract.end(); I != E; ++I)
+    if (ReturnInst *RI = dyn_cast<ReturnInst>((*I)->getTerminator()))
+      (*I)->splitBasicBlock(RI, (*I)->getName()+".ret");
 }
 
+// findInputsOutputs - Find inputs to, outputs from the code region.
+//
+void CodeExtractor::findInputsOutputs(Values &inputs, Values &outputs) {
+  std::set<BasicBlock*> ExitBlocks;
+  for (std::set<BasicBlock*>::const_iterator ci = BlocksToExtract.begin(),
+       ce = BlocksToExtract.end(); ci != ce; ++ci) {
+    BasicBlock *BB = *ci;
+
+    for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I != E; ++I) {
+      // If a used value is defined outside the region, it's an input.  If an
+      // instruction is used outside the region, it's an output.
+      for (User::op_iterator O = I->op_begin(), E = I->op_end(); O != E; ++O)
+        if (definedInCaller(*O))
+          inputs.push_back(*O);
+
+      // Consider uses of this instruction (outputs).
+      for (Value::use_iterator UI = I->use_begin(), E = I->use_end();
+           UI != E; ++UI)
+        if (!definedInRegion(*UI)) {
+          outputs.push_back(I);
+          break;
+        }
+    } // for: insts
+
+    // Keep track of the exit blocks from the region.
+    TerminatorInst *TI = BB->getTerminator();
+    for (unsigned i = 0, e = TI->getNumSuccessors(); i != e; ++i)
+      if (!BlocksToExtract.count(TI->getSuccessor(i)))
+        ExitBlocks.insert(TI->getSuccessor(i));
+  } // for: basic blocks
+
+  NumExitBlocks = ExitBlocks.size();
+
+  // Eliminate duplicates.
+  std::sort(inputs.begin(), inputs.end());
+  inputs.erase(std::unique(inputs.begin(), inputs.end()), inputs.end());
+  std::sort(outputs.begin(), outputs.end());
+  outputs.erase(std::unique(outputs.begin(), outputs.end()), outputs.end());
+}
 
 /// constructFunction - make a function based on inputs and outputs, as follows:
 /// f(in0, ..., inN, out0, ..., outN)
 ///
 Function *CodeExtractor::constructFunction(const Values &inputs,
                                            const Values &outputs,
+                                           BasicBlock *header,
                                            BasicBlock *newRootNode,
                                            BasicBlock *newHeader,
-                                           const std::vector<BasicBlock*> &code,
-                                           Function *oldFunction, Module *M) {
+                                           Function *oldFunction,
+                                           Module *M) {
   DEBUG(std::cerr << "inputs: " << inputs.size() << "\n");
   DEBUG(std::cerr << "outputs: " << outputs.size() << "\n");
-  BasicBlock *header = code[0];
 
   // This function returns unsigned, outputs will go back by reference.
-  Type *retTy = Type::UShortTy;
+  switch (NumExitBlocks) {
+  case 0:
+  case 1: RetTy = Type::VoidTy; break;
+  case 2: RetTy = Type::BoolTy; break;
+  default: RetTy = Type::UShortTy; break;
+  }
+
   std::vector<const Type*> paramTy;
 
   // Add the types of the input values to the function's argument list
   for (Values::const_iterator i = inputs.begin(),
          e = inputs.end(); i != e; ++i) {
     const Value *value = *i;
-    DEBUG(std::cerr << "value used in func: " << value << "\n");
+    DEBUG(std::cerr << "value used in func: " << *value << "\n");
     paramTy.push_back(value->getType());
   }
 
-  // Add the types of the output values to the function's argument list, but
-  // make them pointer types for scalars
-  for (Values::const_iterator i = outputs.begin(),
-         e = outputs.end(); i != e; ++i) {
-    const Value *value = *i;
-    DEBUG(std::cerr << "instr used in func: " << value << "\n");
-    const Type *valueType = value->getType();
-    // Convert scalar types into a pointer of that type
-    if (valueType->isPrimitiveType()) {
-      valueType = PointerType::get(valueType);
-    }
-    paramTy.push_back(valueType);
+  // Add the types of the output values to the function's argument list.
+  for (Values::const_iterator I = outputs.begin(), E = outputs.end();
+       I != E; ++I) {
+    DEBUG(std::cerr << "instr used in func: " << **I << "\n");
+    if (AggregateArgs)
+      paramTy.push_back((*I)->getType());
+    else
+      paramTy.push_back(PointerType::get((*I)->getType()));
   }
 
-  DEBUG(std::cerr << "Function type: " << retTy << " f(");
-  for (std::vector<const Type*>::iterator i = paramTy.begin(),
-         e = paramTy.end(); i != e; ++i)
-    DEBUG(std::cerr << (*i) << ", ");
+  DEBUG(std::cerr << "Function type: " << *RetTy << " f(");
+  DEBUG(for (std::vector<const Type*>::iterator i = paramTy.begin(),
+               e = paramTy.end(); i != e; ++i) std::cerr << **i << ", ");
   DEBUG(std::cerr << ")\n");
 
-  const FunctionType *funcType = FunctionType::get(retTy, paramTy, false);
+  if (AggregateArgs && (inputs.size() + outputs.size() > 0)) {
+    PointerType *StructPtr = PointerType::get(StructType::get(paramTy));
+    paramTy.clear();
+    paramTy.push_back(StructPtr);
+  }
+  const FunctionType *funcType = FunctionType::get(RetTy, paramTy, false);
 
   // Create the new function
   Function *newFunction = new Function(funcType,
                                        GlobalValue::InternalLinkage,
-                                       oldFunction->getName() + "_code", M);
+                                       oldFunction->getName() + "_" +
+                                       header->getName(), M);
   newFunction->getBasicBlockList().push_back(newRootNode);
 
+  // Create an iterator to name all of the arguments we inserted.
+  Function::arg_iterator AI = newFunction->arg_begin();
+
+  // Rewrite all users of the inputs in the extracted region to use the
+  // arguments (or appropriate addressing into struct) instead.
   for (unsigned i = 0, e = inputs.size(); i != e; ++i) {
+    Value *RewriteVal;
+    if (AggregateArgs) {
+      std::vector<Value*> Indices;
+      Indices.push_back(Constant::getNullValue(Type::UIntTy));
+      Indices.push_back(ConstantInt::get(Type::UIntTy, i));
+      std::string GEPname = "gep_" + inputs[i]->getName();
+      TerminatorInst *TI = newFunction->begin()->getTerminator();
+      GetElementPtrInst *GEP = new GetElementPtrInst(AI, Indices, GEPname, TI);
+      RewriteVal = new LoadInst(GEP, "load" + GEPname, TI);
+    } else
+      RewriteVal = AI++;
+
     std::vector<User*> Users(inputs[i]->use_begin(), inputs[i]->use_end());
     for (std::vector<User*>::iterator use = Users.begin(), useE = Users.end();
-         use != useE; ++use) {
-      if (Instruction* inst = dyn_cast<Instruction>(*use)) {
-        if (contains(code, inst->getParent())) {
-          inst->replaceUsesOfWith(inputs[i], getFunctionArg(newFunction, i));
-        }
-      }
-    }
+         use != useE; ++use)
+      if (Instruction* inst = dyn_cast<Instruction>(*use))
+        if (BlocksToExtract.count(inst->getParent()))
+          inst->replaceUsesOfWith(inputs[i], RewriteVal);
+  }
+
+  // Set names for input and output arguments.
+  if (!AggregateArgs) {
+    AI = newFunction->arg_begin();
+    for (unsigned i = 0, e = inputs.size(); i != e; ++i, ++AI)
+      AI->setName(inputs[i]->getName());
+    for (unsigned i = 0, e = outputs.size(); i != e; ++i, ++AI)
+      AI->setName(outputs[i]->getName()+".out");
   }
 
   // Rewrite branches to basic blocks outside of the loop to new dummy blocks
   // within the new function. This must be done before we lose track of which
   // blocks were originally in the code region.
   std::vector<User*> Users(header->use_begin(), header->use_end());
-  for (std::vector<User*>::iterator i = Users.begin(), e = Users.end();
-       i != e; ++i) {
-    if (BranchInst *inst = dyn_cast<BranchInst>(*i)) {
-      BasicBlock *BB = inst->getParent();
-      if (!contains(code, BB) && BB->getParent() == oldFunction) {
-        // The BasicBlock which contains the branch is not in the region
-        // modify the branch target to a new block
-        inst->replaceUsesOfWith(header, newHeader);
-      }
-    }
-  }
+  for (unsigned i = 0, e = Users.size(); i != e; ++i)
+    // The BasicBlock which contains the branch is not in the region
+    // modify the branch target to a new block
+    if (TerminatorInst *TI = dyn_cast<TerminatorInst>(Users[i]))
+      if (!BlocksToExtract.count(TI->getParent()) &&
+          TI->getParent()->getParent() == oldFunction)
+        TI->replaceUsesOfWith(header, newHeader);
 
   return newFunction;
 }
 
-void CodeExtractor::moveCodeToFunction(const std::vector<BasicBlock*> &code,
-                                       Function *newFunction)
-{
-  for (std::vector<BasicBlock*>::const_iterator i = code.begin(), e =code.end();
-       i != e; ++i) {
-    BasicBlock *BB = *i;
-    Function *oldFunc = BB->getParent();
-    Function::BasicBlockListType &oldBlocks = oldFunc->getBasicBlockList();
+/// emitCallAndSwitchStatement - This method sets up the caller side by adding
+/// the call instruction, splitting any PHI nodes in the header block as
+/// necessary.
+void CodeExtractor::
+emitCallAndSwitchStatement(Function *newFunction, BasicBlock *codeReplacer,
+                           Values &inputs, Values &outputs) {
+  // Emit a call to the new function, passing in: *pointer to struct (if
+  // aggregating parameters), or plan inputs and allocated memory for outputs
+  std::vector<Value*> params, StructValues, ReloadOutputs;
+
+  // Add inputs as params, or to be filled into the struct
+  for (Values::iterator i = inputs.begin(), e = inputs.end(); i != e; ++i)
+    if (AggregateArgs)
+      StructValues.push_back(*i);
+    else
+      params.push_back(*i);
 
-    // Delete the basic block from the old function, and the list of blocks
-    oldBlocks.remove(BB);
+  // Create allocas for the outputs
+  for (Values::iterator i = outputs.begin(), e = outputs.end(); i != e; ++i) {
+    if (AggregateArgs) {
+      StructValues.push_back(*i);
+    } else {
+      AllocaInst *alloca =
+        new AllocaInst((*i)->getType(), 0, (*i)->getName()+".loc",
+                       codeReplacer->getParent()->begin()->begin());
+      ReloadOutputs.push_back(alloca);
+      params.push_back(alloca);
+    }
+  }
 
-    // Insert this basic block into the new function
-    Function::BasicBlockListType &newBlocks = newFunction->getBasicBlockList();
-    newBlocks.push_back(BB);
+  AllocaInst *Struct = 0;
+  if (AggregateArgs && (inputs.size() + outputs.size() > 0)) {
+    std::vector<const Type*> ArgTypes;
+    for (Values::iterator v = StructValues.begin(),
+           ve = StructValues.end(); v != ve; ++v)
+      ArgTypes.push_back((*v)->getType());
+
+    // Allocate a struct at the beginning of this function
+    Type *StructArgTy = StructType::get(ArgTypes);
+    Struct =
+      new AllocaInst(StructArgTy, 0, "structArg",
+                     codeReplacer->getParent()->begin()->begin());
+    params.push_back(Struct);
+
+    for (unsigned i = 0, e = inputs.size(); i != e; ++i) {
+      std::vector<Value*> Indices;
+      Indices.push_back(Constant::getNullValue(Type::UIntTy));
+      Indices.push_back(ConstantInt::get(Type::UIntTy, i));
+      GetElementPtrInst *GEP =
+        new GetElementPtrInst(Struct, Indices,
+                              "gep_" + StructValues[i]->getName());
+      codeReplacer->getInstList().push_back(GEP);
+      StoreInst *SI = new StoreInst(StructValues[i], GEP);
+      codeReplacer->getInstList().push_back(SI);
+    }
   }
-}
 
-void
-CodeExtractor::emitCallAndSwitchStatement(Function *newFunction,
-                                          BasicBlock *codeReplacer,
-                                          const std::vector<BasicBlock*> &code,
-                                          Values &inputs,
-                                          Values &outputs)
-{
-  // Emit a call to the new function, passing allocated memory for outputs and
-  // just plain inputs for non-scalars
-  std::vector<Value*> params;
-  BasicBlock *codeReplacerTail = new BasicBlock("codeReplTail",
-                                                codeReplacer->getParent());
-  for (Values::const_iterator i = inputs.begin(),
-         e = inputs.end(); i != e; ++i)
-    params.push_back(*i);
-  for (Values::const_iterator i = outputs.begin(), 
-         e = outputs.end(); i != e; ++i) {
-    // Create allocas for scalar outputs
-    if ((*i)->getType()->isPrimitiveType()) {
-      Constant *one = ConstantUInt::get(Type::UIntTy, 1);
-      AllocaInst *alloca = new AllocaInst((*i)->getType(), one);
-      codeReplacer->getInstList().push_back(alloca);
-      params.push_back(alloca);
+  // Emit the call to the function
+  CallInst *call = new CallInst(newFunction, params,
+                                NumExitBlocks > 1 ? "targetBlock" : "");
+  codeReplacer->getInstList().push_back(call);
 
-      LoadInst *load = new LoadInst(alloca, "alloca");
-      codeReplacerTail->getInstList().push_back(load);
-      std::vector<User*> Users((*i)->use_begin(), (*i)->use_end());
-      for (std::vector<User*>::iterator use = Users.begin(), useE =Users.end();
-           use != useE; ++use) {
-        if (Instruction* inst = dyn_cast<Instruction>(*use)) {
-          if (!contains(code, inst->getParent())) {
-            inst->replaceUsesOfWith(*i, load);
-          }
-        }
-      }
+  Function::arg_iterator OutputArgBegin = newFunction->arg_begin();
+  unsigned FirstOut = inputs.size();
+  if (!AggregateArgs)
+    std::advance(OutputArgBegin, inputs.size());
+
+  // Reload the outputs passed in by reference
+  for (unsigned i = 0, e = outputs.size(); i != e; ++i) {
+    Value *Output = 0;
+    if (AggregateArgs) {
+      std::vector<Value*> Indices;
+      Indices.push_back(Constant::getNullValue(Type::UIntTy));
+      Indices.push_back(ConstantInt::get(Type::UIntTy, FirstOut + i));
+      GetElementPtrInst *GEP
+        = new GetElementPtrInst(Struct, Indices,
+                                "gep_reload_" + outputs[i]->getName());
+      codeReplacer->getInstList().push_back(GEP);
+      Output = GEP;
     } else {
-      params.push_back(*i);
+      Output = ReloadOutputs[i];
+    }
+    LoadInst *load = new LoadInst(Output, outputs[i]->getName()+".reload");
+    codeReplacer->getInstList().push_back(load);
+    std::vector<User*> Users(outputs[i]->use_begin(), outputs[i]->use_end());
+    for (unsigned u = 0, e = Users.size(); u != e; ++u) {
+      Instruction *inst = cast<Instruction>(Users[u]);
+      if (!BlocksToExtract.count(inst->getParent()))
+        inst->replaceUsesOfWith(outputs[i], load);
     }
   }
-  CallInst *call = new CallInst(newFunction, params, "targetBlock");
-  codeReplacer->getInstList().push_back(call);
-  codeReplacer->getInstList().push_back(new BranchInst(codeReplacerTail));
 
   // Now we can emit a switch statement using the call as a value.
-  // FIXME: perhaps instead of default being self BB, it should be a second
-  // dummy block which asserts that the value is not within the range...?
-  //BasicBlock *defaultBlock = new BasicBlock("defaultBlock", oldF);
-  //insert abort() ?
-  //defaultBlock->getInstList().push_back(new BranchInst(codeReplacer));
-
-  SwitchInst *switchInst = new SwitchInst(call, codeReplacerTail,
-                                          codeReplacerTail);
+  SwitchInst *TheSwitch =
+    new SwitchInst(ConstantInt::getNullValue(Type::UShortTy),
+                   codeReplacer, 0, codeReplacer);
 
   // Since there may be multiple exits from the original region, make the new
-  // function return an unsigned, switch on that number
+  // function return an unsigned, switch on that number.  This loop iterates
+  // over all of the blocks in the extracted region, updating any terminator
+  // instructions in the to-be-extracted region that branch to blocks that are
+  // not in the region to be extracted.
+  std::map<BasicBlock*, BasicBlock*> ExitBlockMap;
+
   unsigned switchVal = 0;
-  for (std::vector<BasicBlock*>::const_iterator i =code.begin(), e = code.end();
-       i != e; ++i) {
-    BasicBlock *BB = *i;
-
-    // rewrite the terminator of the original BasicBlock
-    Instruction *term = BB->getTerminator();
-    if (BranchInst *brInst = dyn_cast<BranchInst>(term)) {
-
-      // Restore values just before we exit
-      // FIXME: Use a GetElementPtr to bunch the outputs in a struct
-      for (unsigned outIdx = 0, outE = outputs.size(); outIdx != outE; ++outIdx)
-      {
-        new StoreInst(outputs[outIdx],
-                      getFunctionArg(newFunction, outIdx),
-                      brInst);
-      }
+  for (std::set<BasicBlock*>::const_iterator i = BlocksToExtract.begin(),
+         e = BlocksToExtract.end(); i != e; ++i) {
+    TerminatorInst *TI = (*i)->getTerminator();
+    for (unsigned i = 0, e = TI->getNumSuccessors(); i != e; ++i)
+      if (!BlocksToExtract.count(TI->getSuccessor(i))) {
+        BasicBlock *OldTarget = TI->getSuccessor(i);
+        // add a new basic block which returns the appropriate value
+        BasicBlock *&NewTarget = ExitBlockMap[OldTarget];
+        if (!NewTarget) {
+          // If we don't already have an exit stub for this non-extracted
+          // destination, create one now!
+          NewTarget = new BasicBlock(OldTarget->getName() + ".exitStub",
+                                     newFunction);
+          unsigned SuccNum = switchVal++;
+
+          Value *brVal = 0;
+          switch (NumExitBlocks) {
+          case 0:
+          case 1: break;  // No value needed.
+          case 2:         // Conditional branch, return a bool
+            brVal = ConstantBool::get(!SuccNum);
+            break;
+          default:
+            brVal = ConstantInt::get(Type::UShortTy, SuccNum);
+            break;
+          }
 
-      // Rewrite branches into exists which return a value based on which
-      // exit we take from this function
-      if (brInst->isUnconditional()) {
-        if (!contains(code, brInst->getSuccessor(0))) {
-          ConstantUInt *brVal = ConstantUInt::get(Type::UShortTy, switchVal);
-          ReturnInst *newRet = new ReturnInst(brVal);
-          // add a new target to the switch
-          switchInst->addCase(brVal, brInst->getSuccessor(0));
-          ++switchVal;
-          // rewrite the branch with a return
-          BasicBlock::iterator ii(brInst);
-          ReplaceInstWithInst(BB->getInstList(), ii, newRet);
-          delete brInst;
-        }
-      } else {
-        // Replace the conditional branch to branch
-        // to two new blocks, each of which returns a different code.
-        for (unsigned idx = 0; idx < 2; ++idx) {
-          BasicBlock *oldTarget = brInst->getSuccessor(idx);
-          if (!contains(code, oldTarget)) {
-            // add a new basic block which returns the appropriate value
-            BasicBlock *newTarget = new BasicBlock("newTarget", newFunction);
-            ConstantUInt *brVal = ConstantUInt::get(Type::UShortTy, switchVal);
-            ReturnInst *newRet = new ReturnInst(brVal);
-            newTarget->getInstList().push_back(newRet);
-            // rewrite the original branch instruction with this new target
-            brInst->setSuccessor(idx, newTarget);
-            // the switch statement knows what to do with this value
-            switchInst->addCase(brVal, oldTarget);
-            ++switchVal;
+          ReturnInst *NTRet = new ReturnInst(brVal, NewTarget);
+
+          // Update the switch instruction.
+          TheSwitch->addCase(ConstantInt::get(Type::UShortTy, SuccNum),
+                             OldTarget);
+
+          // Restore values just before we exit
+          Function::arg_iterator OAI = OutputArgBegin;
+          for (unsigned out = 0, e = outputs.size(); out != e; ++out) {
+            // For an invoke, the normal destination is the only one that is
+            // dominated by the result of the invocation
+            BasicBlock *DefBlock = cast<Instruction>(outputs[out])->getParent();
+
+            bool DominatesDef = true;
+
+            if (InvokeInst *Invoke = dyn_cast<InvokeInst>(outputs[out])) {
+              DefBlock = Invoke->getNormalDest();
+
+              // Make sure we are looking at the original successor block, not
+              // at a newly inserted exit block, which won't be in the dominator
+              // info.
+              for (std::map<BasicBlock*, BasicBlock*>::iterator I =
+                     ExitBlockMap.begin(), E = ExitBlockMap.end(); I != E; ++I)
+                if (DefBlock == I->second) {
+                  DefBlock = I->first;
+                  break;
+                }
+
+              // In the extract block case, if the block we are extracting ends
+              // with an invoke instruction, make sure that we don't emit a
+              // store of the invoke value for the unwind block.
+              if (!DS && DefBlock != OldTarget)
+                DominatesDef = false;
+            }
+
+            if (DS)
+              DominatesDef = DS->dominates(DefBlock, OldTarget);
+
+            if (DominatesDef) {
+              if (AggregateArgs) {
+                std::vector<Value*> Indices;
+                Indices.push_back(Constant::getNullValue(Type::UIntTy));
+                Indices.push_back(ConstantInt::get(Type::UIntTy,FirstOut+out));
+                GetElementPtrInst *GEP =
+                  new GetElementPtrInst(OAI, Indices,
+                                        "gep_" + outputs[out]->getName(),
+                                        NTRet);
+                new StoreInst(outputs[out], GEP, NTRet);
+              } else {
+                new StoreInst(outputs[out], OAI, NTRet);
+              }
+            }
+            // Advance output iterator even if we don't emit a store
+            if (!AggregateArgs) ++OAI;
           }
         }
+
+        // rewrite the original branch instruction with this new target
+        TI->setSuccessor(i, NewTarget);
       }
-    } else if (ReturnInst *retTerm = dyn_cast<ReturnInst>(term)) {
-      assert(0 && "Cannot handle return instructions just yet.");
-      // FIXME: what if the terminator is a return!??!
-      // Need to rewrite: add new basic block, move the return there
-      // treat the original as an unconditional branch to that basicblock
-    } else if (SwitchInst *swTerm = dyn_cast<SwitchInst>(term)) {
-      assert(0 && "Cannot handle switch instructions just yet.");
-    } else if (InvokeInst *invInst = dyn_cast<InvokeInst>(term)) {
-      assert(0 && "Cannot handle invoke instructions just yet.");
+  }
+
+  // Now that we've done the deed, simplify the switch instruction.
+  const Type *OldFnRetTy = TheSwitch->getParent()->getParent()->getReturnType();
+  switch (NumExitBlocks) {
+  case 0:
+    // There are no successors (the block containing the switch itself), which
+    // means that previously this was the last part of the function, and hence
+    // this should be rewritten as a `ret'
+
+    // Check if the function should return a value
+    if (OldFnRetTy == Type::VoidTy) {
+      new ReturnInst(0, TheSwitch);  // Return void
+    } else if (OldFnRetTy == TheSwitch->getCondition()->getType()) {
+      // return what we have
+      new ReturnInst(TheSwitch->getCondition(), TheSwitch);
     } else {
-      assert(0 && "Unrecognized terminator, or badly-formed BasicBlock.");
+      // Otherwise we must have code extracted an unwind or something, just
+      // return whatever we want.
+      new ReturnInst(Constant::getNullValue(OldFnRetTy), TheSwitch);
     }
+
+    TheSwitch->getParent()->getInstList().erase(TheSwitch);
+    break;
+  case 1:
+    // Only a single destination, change the switch into an unconditional
+    // branch.
+    new BranchInst(TheSwitch->getSuccessor(1), TheSwitch);
+    TheSwitch->getParent()->getInstList().erase(TheSwitch);
+    break;
+  case 2:
+    new BranchInst(TheSwitch->getSuccessor(1), TheSwitch->getSuccessor(2),
+                   call, TheSwitch);
+    TheSwitch->getParent()->getInstList().erase(TheSwitch);
+    break;
+  default:
+    // Otherwise, make the default destination of the switch instruction be one
+    // of the other successors.
+    TheSwitch->setOperand(0, call);
+    TheSwitch->setSuccessor(0, TheSwitch->getSuccessor(NumExitBlocks));
+    TheSwitch->removeCase(NumExitBlocks);  // Remove redundant case
+    break;
   }
 }
 
+void CodeExtractor::moveCodeToFunction(Function *newFunction) {
+  Function *oldFunc = (*BlocksToExtract.begin())->getParent();
+  Function::BasicBlockListType &oldBlocks = oldFunc->getBasicBlockList();
+  Function::BasicBlockListType &newBlocks = newFunction->getBasicBlockList();
+
+  for (std::set<BasicBlock*>::const_iterator i = BlocksToExtract.begin(),
+         e = BlocksToExtract.end(); i != e; ++i) {
+    // Delete the basic block from the old function, and the list of blocks
+    oldBlocks.remove(*i);
+
+    // Insert this basic block into the new function
+    newBlocks.push_back(*i);
+  }
+}
 
 /// ExtractRegion - Removes a loop from a function, replaces it with a call to
 /// new function. Returns pointer to the new function.
@@ -499,85 +604,146 @@ CodeExtractor::emitCallAndSwitchStatement(Function *newFunction,
 ///
 /// find inputs and outputs for the region
 ///
-/// for inputs: add to function as args, map input instr* to arg# 
-/// for outputs: add allocas for scalars, 
+/// for inputs: add to function as args, map input instr* to arg#
+/// for outputs: add allocas for scalars,
 ///             add to func as args, map output instr* to arg#
 ///
 /// rewrite func to use argument #s instead of instr*
 ///
-/// for each scalar output in the function: at every exit, store intermediate 
+/// for each scalar output in the function: at every exit, store intermediate
 /// computed result back into memory.
 ///
-Function *CodeExtractor::ExtractCodeRegion(const std::vector<BasicBlock*> &code)
-{
+Function *CodeExtractor::
+ExtractCodeRegion(const std::vector<BasicBlock*> &code) {
+  if (!isEligible(code))
+    return 0;
+
   // 1) Find inputs, outputs
   // 2) Construct new function
   //  * Add allocas for defs, pass as args by reference
   //  * Pass in uses as args
   // 3) Move code region, add call instr to func
-  // 
+  //
+  BlocksToExtract.insert(code.begin(), code.end());
 
   Values inputs, outputs;
 
   // Assumption: this is a single-entry code region, and the header is the first
-  // block in the region. FIXME: is this true for a list of blocks from a
-  // natural function?
+  // block in the region.
   BasicBlock *header = code[0];
+
+  for (unsigned i = 1, e = code.size(); i != e; ++i)
+    for (pred_iterator PI = pred_begin(code[i]), E = pred_end(code[i]);
+         PI != E; ++PI)
+      assert(BlocksToExtract.count(*PI) &&
+             "No blocks in this region may have entries from outside the region"
+             " except for the first block!");
+
+  // If we have to split PHI nodes or the entry block, do so now.
+  severSplitPHINodes(header);
+
+  // If we have any return instructions in the region, split those blocks so
+  // that the return is not in the region.
+  splitReturnBlocks();
+
   Function *oldFunction = header->getParent();
-  Module *module = oldFunction->getParent();
 
   // This takes place of the original loop
-  BasicBlock *codeReplacer = new BasicBlock("codeRepl", oldFunction);
+  BasicBlock *codeReplacer = new BasicBlock("codeRepl", oldFunction, header);
 
   // The new function needs a root node because other nodes can branch to the
-  // head of the loop, and the root cannot have predecessors
+  // head of the region, but the entry node of a function cannot have preds.
   BasicBlock *newFuncRoot = new BasicBlock("newFuncRoot");
   newFuncRoot->getInstList().push_back(new BranchInst(header));
 
-  // Find inputs to, outputs from the code region
-  //
-  // If one of the inputs is coming from a different basic block and it's in a
-  // phi node, we need to rewrite the phi node:
-  //
-  // * All the inputs which involve basic blocks OUTSIDE of this region go into
-  //   a NEW phi node that takes care of finding which value really came in.
-  //   The result of this phi is passed to the function as an argument. 
-  //
-  // * All the other phi values stay.
-  //
-  // FIXME: PHI nodes' incoming blocks aren't being rewritten to accomodate for
-  // blocks moving to a new function.
-  // SOLUTION: move Phi nodes out of the loop header into the codeReplacer, pass
-  // the values as parameters to the function
-  findInputsOutputs(code, inputs, outputs, codeReplacer, newFuncRoot);
+  // Find inputs to, outputs from the code region.
+  findInputsOutputs(inputs, outputs);
+
+  // Construct new function based on inputs/outputs & add allocas for all defs.
+  Function *newFunction = constructFunction(inputs, outputs, header,
+                                            newFuncRoot,
+                                            codeReplacer, oldFunction,
+                                            oldFunction->getParent());
 
-  // Step 2: Construct new function based on inputs/outputs,
-  // Add allocas for all defs
-  Function *newFunction = constructFunction(inputs, outputs, newFuncRoot, 
-                                            codeReplacer, code, 
-                                            oldFunction, module);
+  emitCallAndSwitchStatement(newFunction, codeReplacer, inputs, outputs);
 
-  rewritePhiNodes(newFunction, newFuncRoot);
+  moveCodeToFunction(newFunction);
 
-  emitCallAndSwitchStatement(newFunction, codeReplacer, code, inputs, outputs);
+  // Loop over all of the PHI nodes in the header block, and change any
+  // references to the old incoming edge to be the new incoming edge.
+  for (BasicBlock::iterator I = header->begin(); isa<PHINode>(I); ++I) {
+    PHINode *PN = cast<PHINode>(I);
+    for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i)
+      if (!BlocksToExtract.count(PN->getIncomingBlock(i)))
+        PN->setIncomingBlock(i, newFuncRoot);
+  }
+
+  // Look at all successors of the codeReplacer block.  If any of these blocks
+  // had PHI nodes in them, we need to update the "from" block to be the code
+  // replacer, not the original block in the extracted region.
+  std::vector<BasicBlock*> Succs(succ_begin(codeReplacer),
+                                 succ_end(codeReplacer));
+  for (unsigned i = 0, e = Succs.size(); i != e; ++i)
+    for (BasicBlock::iterator I = Succs[i]->begin(); isa<PHINode>(I); ++I) {
+      PHINode *PN = cast<PHINode>(I);
+      std::set<BasicBlock*> ProcessedPreds;
+      for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i)
+        if (BlocksToExtract.count(PN->getIncomingBlock(i)))
+          if (ProcessedPreds.insert(PN->getIncomingBlock(i)).second)
+            PN->setIncomingBlock(i, codeReplacer);
+          else {
+            // There were multiple entries in the PHI for this block, now there
+            // is only one, so remove the duplicated entries.
+            PN->removeIncomingValue(i, false);
+            --i; --e;
+          }
+    }
 
-  moveCodeToFunction(code, newFunction);
+  //std::cerr << "NEW FUNCTION: " << *newFunction;
+  //  verifyFunction(*newFunction);
 
+  //  std::cerr << "OLD FUNCTION: " << *oldFunction;
+  //  verifyFunction(*oldFunction);
+
+  DEBUG(if (verifyFunction(*newFunction)) abort());
   return newFunction;
 }
 
+bool CodeExtractor::isEligible(const std::vector<BasicBlock*> &code) {
+  // Deny code region if it contains allocas or vastarts.
+  for (std::vector<BasicBlock*>::const_iterator BB = code.begin(), e=code.end();
+       BB != e; ++BB)
+    for (BasicBlock::const_iterator I = (*BB)->begin(), Ie = (*BB)->end();
+         I != Ie; ++I)
+      if (isa<AllocaInst>(*I))
+        return false;
+      else if (const CallInst *CI = dyn_cast<CallInst>(I))
+        if (const Function *F = CI->getCalledFunction())
+          if (F->getIntrinsicID() == Intrinsic::vastart)
+            return false;
+  return true;
+}
+
+
+/// ExtractCodeRegion - slurp a sequence of basic blocks into a brand new
+/// function
+///
+Function* llvm::ExtractCodeRegion(DominatorSet &DS,
+                                  const std::vector<BasicBlock*> &code,
+                                  bool AggregateArgs) {
+  return CodeExtractor(&DS, AggregateArgs).ExtractCodeRegion(code);
+}
+
 /// ExtractBasicBlock - slurp a natural loop into a brand new function
 ///
-Function* llvm::ExtractLoop(Loop *L) {
-  CodeExtractor CE;
-  return CE.ExtractCodeRegion(L->getBlocks());
+Function* llvm::ExtractLoop(DominatorSet &DS, Loop *L, bool AggregateArgs) {
+  return CodeExtractor(&DS, AggregateArgs).ExtractCodeRegion(L->getBlocks());
 }
 
 /// ExtractBasicBlock - slurp a basic block into a brand new function
 ///
-Function* llvm::ExtractBasicBlock(BasicBlock *BB) {
-  CodeExtractor CE;
+Function* llvm::ExtractBasicBlock(BasicBlock *BB, bool AggregateArgs) {
   std::vector<BasicBlock*> Blocks;
   Blocks.push_back(BB);
-  return CE.ExtractCodeRegion(Blocks);  
+  return CodeExtractor(0, AggregateArgs).ExtractCodeRegion(Blocks);
 }