Don't access the first element of a potentially empty
[oota-llvm.git] / lib / Transforms / IPO / DeadArgumentElimination.cpp
index c40d2640d7a916f44fbbb993eb09021441c7eb69..ad99eb4bf220adc61c6de0b5cf5e2af0837797d5 100644 (file)
 #include "llvm/DerivedTypes.h"
 #include "llvm/Instructions.h"
 #include "llvm/IntrinsicInst.h"
+#include "llvm/LLVMContext.h"
 #include "llvm/Module.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/CallSite.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/StringExtras.h"
@@ -70,9 +72,9 @@ namespace {
         return F == O.F && Idx == O.Idx && IsArg == O.IsArg;
       }
 
-      std::string getDescription() {
+      std::string getDescription() const {
         return std::string((IsArg ? "Argument #" : "Return value #")) 
-               + utostr(Idx) + " of function " + F->getName();
+               + utostr(Idx) + " of function " + F->getNameStr();
       }
     };
 
@@ -110,15 +112,18 @@ namespace {
     UseMap Uses;
 
     typedef std::set<RetOrArg> LiveSet;
+    typedef std::set<const Function*> LiveFuncSet;
 
     /// This set contains all values that have been determined to be live.
     LiveSet LiveValues;
+    /// This set contains all values that are cannot be changed in any way.
+    LiveFuncSet LiveFunctions;
 
     typedef SmallVector<RetOrArg, 5> UseVector;
 
   public:
     static char ID; // Pass identification, replacement for typeid
-    DAE() : ModulePass((intptr_t)&ID) {}
+    DAE() : ModulePass(&ID) {}
     bool runOnModule(Module &M);
 
     virtual bool ShouldHackArguments() const { return false; }
@@ -132,8 +137,9 @@ namespace {
     void SurveyFunction(Function &F);
     void MarkValue(const RetOrArg &RA, Liveness L,
                    const UseVector &MaybeLiveUses);
-    void MarkLive(RetOrArg RA);
+    void MarkLive(const RetOrArg &RA);
     void MarkLive(const Function &F);
+    void PropagateLiveness(const RetOrArg &RA);
     bool RemoveDeadStuffFromFunction(Function *F);
     bool DeleteDeadVarargs(Function &Fn);
   };
@@ -168,18 +174,11 @@ ModulePass *llvm::createDeadArgHackingPass() { return new DAH(); }
 /// llvm.vastart is never called, the varargs list is dead for the function.
 bool DAE::DeleteDeadVarargs(Function &Fn) {
   assert(Fn.getFunctionType()->isVarArg() && "Function isn't varargs!");
-  if (Fn.isDeclaration() || !Fn.hasInternalLinkage()) return false;
+  if (Fn.isDeclaration() || !Fn.hasLocalLinkage()) return false;
 
   // Ensure that the function is only directly called.
-  for (Value::use_iterator I = Fn.use_begin(), E = Fn.use_end(); I != E; ++I) {
-    // If this use is anything other than a call site, give up.
-    CallSite CS = CallSite::get(*I);
-    Instruction *TheCall = CS.getInstruction();
-    if (!TheCall) return false;   // Not a direct call site?
-
-    // The addr of this function is passed to the call.
-    if (I.getOperandNo() != 0) return false;
-  }
+  if (Fn.hasAddressTaken())
+    return false;
 
   // Okay, we know we can transform this function if safe.  Scan its body
   // looking for calls to llvm.vastart.
@@ -198,8 +197,10 @@ bool DAE::DeleteDeadVarargs(Function &Fn) {
   // Start by computing a new prototype for the function, which is the same as
   // the old function, but doesn't have isVarArg set.
   const FunctionType *FTy = Fn.getFunctionType();
+  
   std::vector<const Type*> Params(FTy->param_begin(), FTy->param_end());
-  FunctionType *NFTy = FunctionType::get(FTy->getReturnType(), Params, false);
+  FunctionType *NFTy = FunctionType::get(FTy->getReturnType(),
+                                                Params, false);
   unsigned NumArgs = Params.size();
 
   // Create the new function body and insert it into the module...
@@ -220,12 +221,14 @@ bool DAE::DeleteDeadVarargs(Function &Fn) {
     Args.assign(CS.arg_begin(), CS.arg_begin()+NumArgs);
 
     // Drop any attributes that were on the vararg arguments.
-    PAListPtr PAL = CS.getParamAttrs();
+    AttrListPtr PAL = CS.getAttributes();
     if (!PAL.isEmpty() && PAL.getSlot(PAL.getNumSlots() - 1).Index > NumArgs) {
-      SmallVector<ParamAttrsWithIndex, 8> ParamAttrsVec;
+      SmallVector<AttributeWithIndex, 8> AttributesVec;
       for (unsigned i = 0; PAL.getSlot(i).Index <= NumArgs; ++i)
-        ParamAttrsVec.push_back(PAL.getSlot(i));
-      PAL = PAListPtr::get(ParamAttrsVec.begin(), ParamAttrsVec.end());
+        AttributesVec.push_back(PAL.getSlot(i));
+      if (Attributes FnAttrs = PAL.getFnAttributes()) 
+        AttributesVec.push_back(AttributeWithIndex::get(~0, FnAttrs));
+      PAL = AttrListPtr::get(AttributesVec.begin(), AttributesVec.end());
     }
 
     Instruction *New;
@@ -233,11 +236,11 @@ bool DAE::DeleteDeadVarargs(Function &Fn) {
       New = InvokeInst::Create(NF, II->getNormalDest(), II->getUnwindDest(),
                                Args.begin(), Args.end(), "", Call);
       cast<InvokeInst>(New)->setCallingConv(CS.getCallingConv());
-      cast<InvokeInst>(New)->setParamAttrs(PAL);
+      cast<InvokeInst>(New)->setAttributes(PAL);
     } else {
       New = CallInst::Create(NF, Args.begin(), Args.end(), "", Call);
       cast<CallInst>(New)->setCallingConv(CS.getCallingConv());
-      cast<CallInst>(New)->setParamAttrs(PAL);
+      cast<CallInst>(New)->setAttributes(PAL);
       if (cast<CallInst>(Call)->isTailCall())
         cast<CallInst>(New)->setTailCall();
     }
@@ -278,7 +281,7 @@ bool DAE::DeleteDeadVarargs(Function &Fn) {
 /// for void functions and 1 for functions not returning a struct. It returns
 /// the number of struct elements for functions returning a struct.
 static unsigned NumRetVals(const Function *F) {
-  if (F->getReturnType() == Type::VoidTy)
+  if (F->getReturnType() == Type::getVoidTy(F->getContext()))
     return 0;
   else if (const StructType *STy = dyn_cast<StructType>(F->getReturnType()))
     return STy->getNumElements();
@@ -290,8 +293,8 @@ static unsigned NumRetVals(const Function *F) {
 /// live, it adds Use to the MaybeLiveUses argument. Returns the determined
 /// liveness of Use.
 DAE::Liveness DAE::MarkIfNotLive(RetOrArg Use, UseVector &MaybeLiveUses) {
-  // We're live if our use is already marked as live.
-  if (LiveValues.count(Use))
+  // We're live if our use or its Function is already marked as live.
+  if (LiveFunctions.count(Use.F) || LiveValues.count(Use))
     return Live;
 
   // We're maybe live otherwise, but remember that we must become live if
@@ -418,12 +421,12 @@ void DAE::SurveyFunction(Function &F) {
         return;
       }
 
-  if (!F.hasInternalLinkage() && (!ShouldHackArguments() || F.isIntrinsic())) {
+  if (!F.hasLocalLinkage() && (!ShouldHackArguments() || F.isIntrinsic())) {
     MarkLive(F);
     return;
   }
 
-  DOUT << "DAE - Inspecting callers for fn: " << F.getName() << "\n";
+  DEBUG(errs() << "DAE - Inspecting callers for fn: " << F.getName() << "\n");
   // Keep track of the number of live retvals, so we can skip checks once all
   // of them turn out to be live.
   unsigned NumLiveRetVals = 0;
@@ -432,13 +435,13 @@ void DAE::SurveyFunction(Function &F) {
   for (Value::use_iterator I = F.use_begin(), E = F.use_end(); I != E; ++I) {
     // If the function is PASSED IN as an argument, its address has been
     // taken.
-    if (I.getOperandNo() != 0) {
+    CallSite CS = CallSite::get(*I);
+    if (!CS.getInstruction() || !CS.isCallee(I)) {
       MarkLive(F);
       return;
     }
 
     // If this use is anything other than a call site, the function is alive.
-    CallSite CS = CallSite::get(*I);
     Instruction *TheCall = CS.getInstruction();
     if (!TheCall) {   // Not a direct call site?
       MarkLive(F);
@@ -486,7 +489,7 @@ void DAE::SurveyFunction(Function &F) {
   for (unsigned i = 0; i != RetCount; ++i)
     MarkValue(CreateRet(&F, i), RetValLiveness[i], MaybeLiveRetUses[i]);
 
-  DOUT << "DAE - Inspecting args for fn: " << F.getName() << "\n";
+  DEBUG(errs() << "DAE - Inspecting args for fn: " << F.getName() << "\n");
 
   // Now, check all of our arguments.
   unsigned i = 0;
@@ -528,29 +531,34 @@ void DAE::MarkValue(const RetOrArg &RA, Liveness L,
 /// mark any values that are used as this function's parameters or by its return
 /// values (according to Uses) live as well.
 void DAE::MarkLive(const Function &F) {
-    DOUT << "DAE - Intrinsically live fn: " << F.getName() << "\n";
+  DEBUG(errs() << "DAE - Intrinsically live fn: " << F.getName() << "\n");
+    // Mark the function as live.
+    LiveFunctions.insert(&F);
     // Mark all arguments as live.
     for (unsigned i = 0, e = F.arg_size(); i != e; ++i)
-      MarkLive(CreateArg(&F, i));
+      PropagateLiveness(CreateArg(&F, i));
     // Mark all return values as live.
     for (unsigned i = 0, e = NumRetVals(&F); i != e; ++i)
-      MarkLive(CreateRet(&F, i));
+      PropagateLiveness(CreateRet(&F, i));
 }
 
 /// MarkLive - Mark the given return value or argument as live. Additionally,
 /// mark any values that are used by this value (according to Uses) live as
 /// well.
-void DAE::MarkLive(RetOrArg RA) {
+void DAE::MarkLive(const RetOrArg &RA) {
+  if (LiveFunctions.count(RA.F))
+    return; // Function was already marked Live.
+
   if (!LiveValues.insert(RA).second)
     return; // We were already marked Live.
 
-  if (RA.IsArg)
-    DOUT << "DAE - Marking argument " << RA.Idx << " to function "
-         << RA.F->getNameStart() << " live\n";
-  else
-    DOUT << "DAE - Marking return value " << RA.Idx << " of function "
-         << RA.F->getNameStart() << " live\n";
+  DOUT << "DAE - Marking " << RA.getDescription() << " live\n";
+  PropagateLiveness(RA);
+}
 
+/// PropagateLiveness - Given that RA is a live value, propagate it's liveness
+/// to any other values it uses (according to Uses).
+void DAE::PropagateLiveness(const RetOrArg &RA) {
   // We don't use upper_bound (or equal_range) here, because our recursive call
   // to ourselves is likely to cause the upper_bound (which is the first value
   // not belonging to RA) to become erased and the iterator invalidated.
@@ -570,8 +578,8 @@ void DAE::MarkLive(RetOrArg RA) {
 // the function to not have these arguments and return values.
 //
 bool DAE::RemoveDeadStuffFromFunction(Function *F) {
-  // Quick exit path for external functions
-  if (!F->hasInternalLinkage() && (!ShouldHackArguments() || F->isIntrinsic()))
+  // Don't modify fully live functions
+  if (LiveFunctions.count(F))
     return false;
 
   // Start by computing a new prototype for the function, which is the same as
@@ -580,25 +588,24 @@ bool DAE::RemoveDeadStuffFromFunction(Function *F) {
   std::vector<const Type*> Params;
 
   // Set up to build a new list of parameter attributes.
-  SmallVector<ParamAttrsWithIndex, 8> ParamAttrsVec;
-  const PAListPtr &PAL = F->getParamAttrs();
+  SmallVector<AttributeWithIndex, 8> AttributesVec;
+  const AttrListPtr &PAL = F->getAttributes();
 
   // The existing function return attributes.
-  ParameterAttributes RAttrs = PAL.getParamAttrs(0);
-
+  Attributes RAttrs = PAL.getRetAttributes();
+  Attributes FnAttrs = PAL.getFnAttributes();
 
   // Find out the new return value.
 
   const Type *RetTy = FTy->getReturnType();
   const Type *NRetTy = NULL;
   unsigned RetCount = NumRetVals(F);
-  // Explicitly track if anything changed, for debugging.
-  bool Changed = false;
+  
   // -1 means unused, other numbers are the new index
   SmallVector<int, 5> NewRetIdxs(RetCount, -1);
   std::vector<const Type*> RetTypes;
-  if (RetTy == Type::VoidTy) {
-    NRetTy = Type::VoidTy;
+  if (RetTy == Type::getVoidTy(F->getContext())) {
+    NRetTy = Type::getVoidTy(F->getContext());
   } else {
     const StructType *STy = dyn_cast<StructType>(RetTy);
     if (STy)
@@ -610,9 +617,8 @@ bool DAE::RemoveDeadStuffFromFunction(Function *F) {
           NewRetIdxs[i] = RetTypes.size() - 1;
         } else {
           ++NumRetValsEliminated;
-          DOUT << "DAE - Removing return value " << i << " from "
-               << F->getNameStart() << "\n";
-          Changed = true;
+          DEBUG(errs() << "DAE - Removing return value " << i << " from "
+                << F->getName() << "\n");
         }
       }
     else
@@ -621,26 +627,25 @@ bool DAE::RemoveDeadStuffFromFunction(Function *F) {
         RetTypes.push_back(RetTy);
         NewRetIdxs[0] = 0;
       } else {
-        DOUT << "DAE - Removing return value from " << F->getNameStart()
-             << "\n";
+        DEBUG(errs() << "DAE - Removing return value from " << F->getName()
+              << "\n");
         ++NumRetValsEliminated;
-        Changed = true;
       }
-    if (RetTypes.size() > 1 || (STy && STy->getNumElements()==RetTypes.size()))
+    if (RetTypes.size() > 1)
       // More than one return type? Return a struct with them. Also, if we used
       // to return a struct and didn't change the number of return values,
       // return a struct again. This prevents changing {something} into
       // something and {} into void.
       // Make the new struct packed if we used to return a packed struct
       // already.
-      NRetTy = StructType::get(RetTypes, STy->isPacked());
+      NRetTy = StructType::get(STy->getContext(), RetTypes, STy->isPacked());
     else if (RetTypes.size() == 1)
       // One return type? Just a simple value then, but only if we didn't use to
       // return a struct with that simple value before.
       NRetTy = RetTypes.front();
     else if (RetTypes.size() == 0)
       // No return types? Make it void, but only if we didn't use to return {}.
-      NRetTy = Type::VoidTy;
+      NRetTy = Type::getVoidTy(F->getContext());
   }
 
   assert(NRetTy && "No new return type found?");
@@ -649,14 +654,14 @@ bool DAE::RemoveDeadStuffFromFunction(Function *F) {
   // values. Otherwise, ensure that we don't have any conflicting attributes
   // here. Currently, this should not be possible, but special handling might be
   // required when new return value attributes are added.
-  if (NRetTy == Type::VoidTy)
-    RAttrs &= ~ParamAttr::typeIncompatible(NRetTy);
+  if (NRetTy == Type::getVoidTy(F->getContext()))
+    RAttrs &= ~Attribute::typeIncompatible(NRetTy);
   else
-    assert((RAttrs & ParamAttr::typeIncompatible(NRetTy)) == 0 
+    assert((RAttrs & Attribute::typeIncompatible(NRetTy)) == 0 
            && "Return attributes no longer compatible?");
 
   if (RAttrs)
-    ParamAttrsVec.push_back(ParamAttrsWithIndex::get(0, RAttrs));
+    AttributesVec.push_back(AttributeWithIndex::get(0, RAttrs));
 
   // Remember which arguments are still alive.
   SmallVector<bool, 10> ArgAlive(FTy->getNumParams(), false);
@@ -673,18 +678,20 @@ bool DAE::RemoveDeadStuffFromFunction(Function *F) {
 
       // Get the original parameter attributes (skipping the first one, that is
       // for the return value.
-      if (ParameterAttributes Attrs = PAL.getParamAttrs(i + 1))
-        ParamAttrsVec.push_back(ParamAttrsWithIndex::get(Params.size(), Attrs));
+      if (Attributes Attrs = PAL.getParamAttributes(i + 1))
+        AttributesVec.push_back(AttributeWithIndex::get(Params.size(), Attrs));
     } else {
       ++NumArgumentsEliminated;
-      DOUT << "DAE - Removing argument " << i << " (" << I->getNameStart()
-           << ") from " << F->getNameStart() << "\n";
-      Changed = true;
+      DEBUG(errs() << "DAE - Removing argument " << i << " (" << I->getName()
+            << ") from " << F->getName() << "\n");
     }
   }
 
-  // Reconstruct the ParamAttrsList based on the vector we constructed.
-  PAListPtr NewPAL = PAListPtr::get(ParamAttrsVec.begin(), ParamAttrsVec.end());
+  if (FnAttrs != Attribute::None) 
+    AttributesVec.push_back(AttributeWithIndex::get(~0, FnAttrs));
+
+  // Reconstruct the AttributesList based on the vector we constructed.
+  AttrListPtr NewPAL = AttrListPtr::get(AttributesVec.begin(), AttributesVec.end());
 
   // Work around LLVM bug PR56: the CWriter cannot emit varargs functions which
   // have zero fixed arguments.
@@ -695,25 +702,21 @@ bool DAE::RemoveDeadStuffFromFunction(Function *F) {
   bool ExtraArgHack = false;
   if (Params.empty() && FTy->isVarArg() && FTy->getNumParams() != 0) {
     ExtraArgHack = true;
-    Params.push_back(Type::Int32Ty);
+    Params.push_back(Type::getInt32Ty(F->getContext()));
   }
 
   // Create the new function type based on the recomputed parameters.
-  FunctionType *NFTy = FunctionType::get(NRetTy, Params, FTy->isVarArg());
+  FunctionType *NFTy = FunctionType::get(NRetTy, Params,
+                                                FTy->isVarArg());
 
   // No change?
   if (NFTy == FTy)
     return false;
 
-  // The function type is only allowed to be different if we actually left out
-  // an argument or return value.
-  assert(Changed && "Function type changed while no arguments or return values"
-                    "were removed!");
-
   // Create the new function body and insert it into the module...
   Function *NF = Function::Create(NFTy, F->getLinkage());
   NF->copyAttributesFrom(F);
-  NF->setParamAttrs(NewPAL);
+  NF->setAttributes(NewPAL);
   // Insert the new function before the old function, so we won't be processing
   // it again.
   F->getParent()->getFunctionList().insert(F, NF);
@@ -727,15 +730,16 @@ bool DAE::RemoveDeadStuffFromFunction(Function *F) {
     CallSite CS = CallSite::get(F->use_back());
     Instruction *Call = CS.getInstruction();
 
-    ParamAttrsVec.clear();
-    const PAListPtr &CallPAL = CS.getParamAttrs();
+    AttributesVec.clear();
+    const AttrListPtr &CallPAL = CS.getAttributes();
 
     // The call return attributes.
-    ParameterAttributes RAttrs = CallPAL.getParamAttrs(0);
+    Attributes RAttrs = CallPAL.getRetAttributes();
+    Attributes FnAttrs = CallPAL.getFnAttributes();
     // Adjust in case the function was changed to return void.
-    RAttrs &= ~ParamAttr::typeIncompatible(NF->getReturnType());
+    RAttrs &= ~Attribute::typeIncompatible(NF->getReturnType());
     if (RAttrs)
-      ParamAttrsVec.push_back(ParamAttrsWithIndex::get(0, RAttrs));
+      AttributesVec.push_back(AttributeWithIndex::get(0, RAttrs));
 
     // Declare these outside of the loops, so we can reuse them for the second
     // loop, which loops the varargs.
@@ -747,34 +751,37 @@ bool DAE::RemoveDeadStuffFromFunction(Function *F) {
       if (ArgAlive[i]) {
         Args.push_back(*I);
         // Get original parameter attributes, but skip return attributes.
-        if (ParameterAttributes Attrs = CallPAL.getParamAttrs(i + 1))
-          ParamAttrsVec.push_back(ParamAttrsWithIndex::get(Args.size(), Attrs));
+        if (Attributes Attrs = CallPAL.getParamAttributes(i + 1))
+          AttributesVec.push_back(AttributeWithIndex::get(Args.size(), Attrs));
       }
 
     if (ExtraArgHack)
-      Args.push_back(UndefValue::get(Type::Int32Ty));
+      Args.push_back(UndefValue::get(Type::getInt32Ty(F->getContext())));
 
     // Push any varargs arguments on the list. Don't forget their attributes.
     for (CallSite::arg_iterator E = CS.arg_end(); I != E; ++I, ++i) {
       Args.push_back(*I);
-      if (ParameterAttributes Attrs = CallPAL.getParamAttrs(i + 1))
-        ParamAttrsVec.push_back(ParamAttrsWithIndex::get(Args.size(), Attrs));
+      if (Attributes Attrs = CallPAL.getParamAttributes(i + 1))
+        AttributesVec.push_back(AttributeWithIndex::get(Args.size(), Attrs));
     }
 
-    // Reconstruct the ParamAttrsList based on the vector we constructed.
-    PAListPtr NewCallPAL = PAListPtr::get(ParamAttrsVec.begin(),
-                                          ParamAttrsVec.end());
+    if (FnAttrs != Attribute::None)
+      AttributesVec.push_back(AttributeWithIndex::get(~0, FnAttrs));
+
+    // Reconstruct the AttributesList based on the vector we constructed.
+    AttrListPtr NewCallPAL = AttrListPtr::get(AttributesVec.begin(),
+                                              AttributesVec.end());
 
     Instruction *New;
     if (InvokeInst *II = dyn_cast<InvokeInst>(Call)) {
       New = InvokeInst::Create(NF, II->getNormalDest(), II->getUnwindDest(),
                                Args.begin(), Args.end(), "", Call);
       cast<InvokeInst>(New)->setCallingConv(CS.getCallingConv());
-      cast<InvokeInst>(New)->setParamAttrs(NewCallPAL);
+      cast<InvokeInst>(New)->setAttributes(NewCallPAL);
     } else {
       New = CallInst::Create(NF, Args.begin(), Args.end(), "", Call);
       cast<CallInst>(New)->setCallingConv(CS.getCallingConv());
-      cast<CallInst>(New)->setParamAttrs(NewCallPAL);
+      cast<CallInst>(New)->setAttributes(NewCallPAL);
       if (cast<CallInst>(Call)->isTailCall())
         cast<CallInst>(New)->setTailCall();
     }
@@ -785,50 +792,44 @@ bool DAE::RemoveDeadStuffFromFunction(Function *F) {
         // Return type not changed? Just replace users then.
         Call->replaceAllUsesWith(New);
         New->takeName(Call);
-      } else if (New->getType() == Type::VoidTy) {
+      } else if (New->getType() == Type::getVoidTy(F->getContext())) {
         // Our return value has uses, but they will get removed later on.
         // Replace by null for now.
         Call->replaceAllUsesWith(Constant::getNullValue(Call->getType()));
       } else {
-        assert(isa<StructType>(RetTy) && "Return type changed, but not into a"
-                                         "void. The old return type must have"
-                                         "been a struct!");
-        // The original return value was a struct, update all uses (which are
-        // all extractvalue instructions).
-        for (Value::use_iterator I = Call->use_begin(), E = Call->use_end();
-             I != E;) {
-          assert(isa<ExtractValueInst>(*I) && "Return value not only used by"
-                                              "extractvalue?");
-          ExtractValueInst *EV = cast<ExtractValueInst>(*I);
-          // Increment now, since we're about to throw away this use.
-          ++I;
-          assert(EV->hasIndices() && "Return value used by extractvalue without"
-                                     "indices?");
-          unsigned Idx = *EV->idx_begin();
-          if (NewRetIdxs[Idx] != -1) {
-            if (RetTypes.size() > 1) {
-              // We're still returning a struct, create a new extractvalue
-              // instruction with the first index updated
-              std::vector<unsigned> NewIdxs(EV->idx_begin(), EV->idx_end());
-              NewIdxs[0] = NewRetIdxs[Idx];
-              Value *NEV = ExtractValueInst::Create(New, NewIdxs.begin(),
-                                                    NewIdxs.end(), "retval",
-                                                    EV);
-              EV->replaceAllUsesWith(NEV);
-              EV->eraseFromParent();
-            } else {
-              // We are now only returning a simple value, remove the
-              // extractvalue.
-              EV->replaceAllUsesWith(New);
-              EV->eraseFromParent();
-            }
-          } else {
-            // Value unused, replace uses by null for now, they will get removed
-            // later on.
-            EV->replaceAllUsesWith(Constant::getNullValue(EV->getType()));
-            EV->eraseFromParent();
-          }
+        assert(isa<StructType>(RetTy) &&
+               "Return type changed, but not into a void. The old return type"
+               " must have been a struct!");
+        Instruction *InsertPt = Call;
+        if (InvokeInst *II = dyn_cast<InvokeInst>(Call)) {
+          BasicBlock::iterator IP = II->getNormalDest()->begin();
+          while (isa<PHINode>(IP)) ++IP;
+          InsertPt = IP;
         }
+          
+        // We used to return a struct. Instead of doing smart stuff with all the
+        // uses of this struct, we will just rebuild it using
+        // extract/insertvalue chaining and let instcombine clean that up.
+        //
+        // Start out building up our return value from undef
+        Value *RetVal = UndefValue::get(RetTy);
+        for (unsigned i = 0; i != RetCount; ++i)
+          if (NewRetIdxs[i] != -1) {
+            Value *V;
+            if (RetTypes.size() > 1)
+              // We are still returning a struct, so extract the value from our
+              // return value
+              V = ExtractValueInst::Create(New, NewRetIdxs[i], "newret",
+                                           InsertPt);
+            else
+              // We are now returning a single element, so just insert that
+              V = New;
+            // Insert the value at the old position
+            RetVal = InsertValueInst::Create(RetVal, V, i, "oldret", InsertPt);
+          }
+        // Now, replace all uses of the old call instruction with the return
+        // struct we built
+        Call->replaceAllUsesWith(RetVal);
         New->takeName(Call);
       }
     }
@@ -867,7 +868,7 @@ bool DAE::RemoveDeadStuffFromFunction(Function *F) {
       if (ReturnInst *RI = dyn_cast<ReturnInst>(BB->getTerminator())) {
         Value *RetVal;
 
-        if (NFTy->getReturnType() == Type::VoidTy) {
+        if (NFTy->getReturnType() == Type::getVoidTy(F->getContext())) {
           RetVal = 0;
         } else {
           assert (isa<StructType>(RetTy));
@@ -878,7 +879,7 @@ bool DAE::RemoveDeadStuffFromFunction(Function *F) {
           // clean that up.
           Value *OldRet = RI->getOperand(0);
           // Start out building up our return value from undef
-          RetVal = llvm::UndefValue::get(NRetTy);
+          RetVal = UndefValue::get(NRetTy);
           for (unsigned i = 0; i != RetCount; ++i)
             if (NewRetIdxs[i] != -1) {
               ExtractValueInst *EV = ExtractValueInst::Create(OldRet, i,
@@ -898,7 +899,7 @@ bool DAE::RemoveDeadStuffFromFunction(Function *F) {
         }
         // Replace the return instruction with one returning the new return
         // value (possibly 0 if we became void).
-        ReturnInst::Create(RetVal, RI);
+        ReturnInst::Create(F->getContext(), RetVal, RI);
         BB->getInstList().erase(RI);
       }