[llvm][NFC][CallSite] Remove CallSite from DeadArgumentElimination

[mtrofin]

Summary

Also capitalized some induction variables, to match coding style.

[craig.topper]

llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp

565–566

I don't understand why this block exists. We already returned if it wasn't a callsite.

869

This should be a cast I think.

[mtrofin]

feedback

llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp

565–566

Nice catch! Indeed, 557 took care of it.

[craig.topper]

LGTM

[foad]

llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp

444

Needs to be CB->getArgOperand. This is making my DEBUG build fail.

[mtrofin]

llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp

444

Noted, doing a rebuild locally to validate there's no other failures and sending a fix asap. Sorry about this.

[lebedev.ri]

llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp

444

(It might be a really good idea to do all local development with LLVM_ENABLE_ASSERTIONS enabled..)

[MaskRay]

llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp

444

Just saw the discussions here. I fixed it in cca545ce462b61bdd4bd1b8d81d9eb921e13aebc

For a CMAKE_BUILD_TYPE=Release build, -DLLVM_ENABLE_ASSERTIONS defaults to OFF. It may be good setting it to ON.

[mtrofin]

llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp

444

+1 - good suggestion, you mean locally, I assume.

llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp

 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/IPO/DeadArgumentElimination.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/IR/Argument.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
-#include "llvm/IR/CallSite.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
bool DeadArgumentEliminationPass::DeleteDeadVarargs(Function &Fn) {
   Fn.getParent()->getFunctionList().insert(Fn.getIterator(), NF);
   NF->takeName(&Fn);
 
   // Loop over all of the callers of the function, transforming the call sites
   // to pass in a smaller number of arguments into the new function.
   //
   std::vector<Value *> Args;
   for (Value::user_iterator I = Fn.user_begin(), E = Fn.user_end(); I != E; ) {
-    CallSite CS(*I++);
-    if (!CS)
+    CallBase *CB = dyn_cast<CallBase>(*I++);
+    if (!CB)
       continue;
-    Instruction *Call = CS.getInstruction();
 
     // Pass all the same arguments.
-    Args.assign(CS.arg_begin(), CS.arg_begin() + NumArgs);
+    Args.assign(CB->arg_begin(), CB->arg_begin() + NumArgs);
 
     // Drop any attributes that were on the vararg arguments.
-    AttributeList PAL = CS.getAttributes();
+    AttributeList PAL = CB->getAttributes();
     if (!PAL.isEmpty()) {
       SmallVector<AttributeSet, 8> ArgAttrs;
       for (unsigned ArgNo = 0; ArgNo < NumArgs; ++ArgNo)
         ArgAttrs.push_back(PAL.getParamAttributes(ArgNo));
       PAL = AttributeList::get(Fn.getContext(), PAL.getFnAttributes(),
                                PAL.getRetAttributes(), ArgAttrs);
     }
 
     SmallVector<OperandBundleDef, 1> OpBundles;
-    CS.getOperandBundlesAsDefs(OpBundles);
+    CB->getOperandBundlesAsDefs(OpBundles);
 
-    CallSite NewCS;
-    if (InvokeInst *II = dyn_cast<InvokeInst>(Call)) {
-      NewCS = InvokeInst::Create(NF, II->getNormalDest(), II->getUnwindDest(),
-                                 Args, OpBundles, "", Call);
+    CallBase *NewCB = nullptr;
+    if (InvokeInst *II = dyn_cast<InvokeInst>(CB)) {
+      NewCB = InvokeInst::Create(NF, II->getNormalDest(), II->getUnwindDest(),
+                                 Args, OpBundles, "", CB);
     } else {
-      NewCS = CallInst::Create(NF, Args, OpBundles, "", Call);
-      cast<CallInst>(NewCS.getInstruction())
-          ->setTailCallKind(cast<CallInst>(Call)->getTailCallKind());
-    }
-    NewCS.setCallingConv(CS.getCallingConv());
-    NewCS.setAttributes(PAL);
-    NewCS->setDebugLoc(Call->getDebugLoc());
+      NewCB = CallInst::Create(NF, Args, OpBundles, "", CB);
+      cast<CallInst>(NewCB)->setTailCallKind(
+          cast<CallInst>(CB)->getTailCallKind());
+    }
+    NewCB->setCallingConv(CB->getCallingConv());
+    NewCB->setAttributes(PAL);
+    NewCB->setDebugLoc(CB->getDebugLoc());
     uint64_t W;
-    if (Call->extractProfTotalWeight(W))
-      NewCS->setProfWeight(W);
+    if (CB->extractProfTotalWeight(W))
+      NewCB->setProfWeight(W);
 
     Args.clear();
 
-    if (!Call->use_empty())
-      Call->replaceAllUsesWith(NewCS.getInstruction());
+    if (!CB->use_empty())
+      CB->replaceAllUsesWith(NewCB);
 
-    NewCS->takeName(Call);
+    NewCB->takeName(CB);
 
     // Finally, remove the old call from the program, reducing the use-count of
     // F.
-    Call->eraseFromParent();
+    CB->eraseFromParent();
   }
 
   // Since we have now created the new function, splice the body of the old
   // function right into the new function, leaving the old rotting hulk of the
   // function empty.
   NF->getBasicBlockList().splice(NF->begin(), Fn.getBasicBlockList());
 
   // Loop over the argument list, transferring uses of the old arguments over to
if (!Arg.hasSwiftErrorAttr() && Arg.use_empty() && !Arg.hasByValOrInAllocaAttr()) {
       UnusedArgs.push_back(Arg.getArgNo());
     }
   }
 
   if (UnusedArgs.empty())
     return false;
 
   for (Use &U : Fn.uses()) {
-    CallSite CS(U.getUser());
-    if (!CS || !CS.isCallee(&U))
+    CallBase *CB = dyn_cast<CallBase>(U.getUser());
+    if (!CB || !CB->isCallee(&U))
       continue;
 
     // Now go through all unused args and replace them with "undef".
     for (unsigned I = 0, E = UnusedArgs.size(); I != E; ++I) {
       unsigned ArgNo = UnusedArgs[I];
 
-      Value *Arg = CS.getArgument(ArgNo);
-      CS.setArgument(ArgNo, UndefValue::get(Arg->getType()));
+      Value *Arg = CB->getArgOperand(ArgNo);
+      CB->setArgOperand(ArgNo, UndefValue::get(Arg->getType()));
       ++NumArgumentsReplacedWithUndef;
       Changed = true;
     }
   }
 
   return Changed;
 }
 
if (const ReturnInst *RI = dyn_cast<ReturnInst>(V)) {
       // original Use.
       const Function *F = RI->getParent()->getParent();
       if (RetValNum != -1U) {
         RetOrArg Use = CreateRet(F, RetValNum);
         // We might be live, depending on the liveness of Use.
         return MarkIfNotLive(Use, MaybeLiveUses);
       } else {
         DeadArgumentEliminationPass::Liveness Result = MaybeLive;
-        for (unsigned i = 0; i < NumRetVals(F); ++i) {
-          RetOrArg Use = CreateRet(F, i);
+        for (unsigned Ri = 0; Ri < NumRetVals(F); ++Ri) {
+          RetOrArg Use = CreateRet(F, Ri);
           // We might be live, depending on the liveness of Use. If any
           // sub-value is live, then the entire value is considered live. This
           // is a conservative choice, and better tracking is possible.
           DeadArgumentEliminationPass::Liveness SubResult =
               MarkIfNotLive(Use, MaybeLiveUses);
           if (Result != Live)
             Result = SubResult;
         }
if (const InsertValueInst *IV = dyn_cast<InsertValueInst>(V)) {
       for (const Use &UU : IV->uses()) {
         Result = SurveyUse(&UU, MaybeLiveUses, RetValNum);
         if (Result == Live)
           break;
       }
       return Result;
     }
 
-    if (auto CS = ImmutableCallSite(V)) {
-      const Function *F = CS.getCalledFunction();
+    if (const auto *CB = dyn_cast<CallBase>(V)) {
+      const Function *F = CB->getCalledFunction();
       if (F) {
         // Used in a direct call.
 
         // The function argument is live if it is used as a bundle operand.
-        if (CS.isBundleOperand(U))
+        if (CB->isBundleOperand(U))
           return Live;
 
         // Find the argument number. We know for sure that this use is an
         // argument, since if it was the function argument this would be an
         // indirect call and the we know can't be looking at a value of the
         // label type (for the invoke instruction).
-        unsigned ArgNo = CS.getArgumentNo(U);
+        unsigned ArgNo = CB->getArgOperandNo(U);
 
         if (ArgNo >= F->getFunctionType()->getNumParams())
           // The value is passed in through a vararg! Must be live.
           return Live;
 
-        assert(CS.getArgument(ArgNo)
-               == CS->getOperand(U->getOperandNo())
-               && "Argument is not where we expected it");
+        assert(CB->getArgument(ArgNo) == CB->getOperand(U->getOperandNo()) &&
+               "Argument is not where we expected it");
 
         // Value passed to a normal call. It's only live when the corresponding
         // argument to the called function turns out live.
         RetOrArg Use = CreateArg(F, ArgNo);
         return MarkIfNotLive(Use, MaybeLiveUses);
       }
     }
     // Used in any other way? Value must be live.
void DeadArgumentEliminationPass::SurveyFunction(const Function &F) {
   unsigned NumLiveRetVals = 0;
 
   bool HasMustTailCallers = false;
 
   // Loop all uses of the function.
   for (const Use &U : F.uses()) {
     // If the function is PASSED IN as an argument, its address has been
     // taken.
-    ImmutableCallSite CS(U.getUser());
-    if (!CS || !CS.isCallee(&U)) {
+    const auto *CB = dyn_cast<CallBase>(U.getUser());
+    if (!CB || !CB->isCallee(&U)) {
       MarkLive(F);
       return;
     }
 
     // The number of arguments for `musttail` call must match the number of
     // arguments of the caller
-    if (CS.isMustTailCall())
+    if (CB->isMustTailCall())
       HasMustTailCallers = true;
 
-    // If this use is anything other than a call site, the function is alive.
-    const Instruction *TheCall = CS.getInstruction();
-    if (!TheCall) {   // Not a direct call site?
-      MarkLive(F);
-      return;
-    }
-
     // If we end up here, we are looking at a direct call to our function.
 
     // Now, check how our return value(s) is/are used in this caller. Don't
     // bother checking return values if all of them are live already.
     if (NumLiveRetVals == RetCount)
       continue;
 
     // Check all uses of the return value.
-    for (const Use &U : TheCall->uses()) {
+    for (const Use &U : CB->uses()) {
       if (ExtractValueInst *Ext = dyn_cast<ExtractValueInst>(U.getUser())) {
         // This use uses a part of our return value, survey the uses of
         // that part and store the results for this index only.
         unsigned Idx = *Ext->idx_begin();
         if (RetValLiveness[Idx] != Live) {
           RetValLiveness[Idx] = SurveyUses(Ext, MaybeLiveRetUses[Idx]);
           if (RetValLiveness[Idx] == Live)
             NumLiveRetVals++;
         }
       } else {
         // Used by something else than extractvalue. Survey, but assume that the
         // result applies to all sub-values.
         UseVector MaybeLiveAggregateUses;
         if (SurveyUse(&U, MaybeLiveAggregateUses) == Live) {
           NumLiveRetVals = RetCount;
           RetValLiveness.assign(RetCount, Live);
           break;
         } else {
-          for (unsigned i = 0; i != RetCount; ++i) {
-            if (RetValLiveness[i] != Live)
-              MaybeLiveRetUses[i].append(MaybeLiveAggregateUses.begin(),
+          for (unsigned Ri = 0; Ri != RetCount; ++Ri) {
+            if (RetValLiveness[Ri] != Live)
+              MaybeLiveRetUses[Ri].append(MaybeLiveAggregateUses.begin(),
                                           MaybeLiveAggregateUses.end());
           }
         }
       }
     }
   }
 
   if (HasMustTailCallers) {
     LLVM_DEBUG(dbgs() << "DeadArgumentEliminationPass - " << F.getName()
                       << " has musttail callers\n");
   }
 
   // Now we've inspected all callers, record the liveness of our return values.
-  for (unsigned i = 0; i != RetCount; ++i)
-    MarkValue(CreateRet(&F, i), RetValLiveness[i], MaybeLiveRetUses[i]);
+  for (unsigned Ri = 0; Ri != RetCount; ++Ri)
+    MarkValue(CreateRet(&F, Ri), RetValLiveness[Ri], MaybeLiveRetUses[Ri]);
 
   LLVM_DEBUG(dbgs() << "DeadArgumentEliminationPass - Inspecting args for fn: "
                     << F.getName() << "\n");
 
   // Now, check all of our arguments.
-  unsigned i = 0;
+  unsigned ArgI = 0;
   UseVector MaybeLiveArgUses;
-  for (Function::const_arg_iterator AI = F.arg_begin(),
-       E = F.arg_end(); AI != E; ++AI, ++i) {
+  for (Function::const_arg_iterator AI = F.arg_begin(), E = F.arg_end();
+       AI != E; ++AI, ++ArgI) {
     Liveness Result;
     if (F.getFunctionType()->isVarArg() || HasMustTailCallers ||
         HasMustTailCalls) {
       // Variadic functions will already have a va_arg function expanded inside
       // them, making them potentially very sensitive to ABI changes resulting
       // from removing arguments entirely, so don't. For example AArch64 handles
       // register and stack HFAs very differently, and this is reflected in the
       // IR which has already been generated.
       //
       // `musttail` calls to this function restrict argument removal attempts.
       // The signature of the caller must match the signature of the function.
       //
       // `musttail` calls in this function prevents us from changing its
       // signature
       Result = Live;
     } else {
       // See what the effect of this use is (recording any uses that cause
       // MaybeLive in MaybeLiveArgUses).
       Result = SurveyUses(&*AI, MaybeLiveArgUses);
     }
 
     // Mark the result.
-    MarkValue(CreateArg(&F, i), Result, MaybeLiveArgUses);
+    MarkValue(CreateArg(&F, ArgI), Result, MaybeLiveArgUses);
     // Clear the vector again for the next iteration.
     MaybeLiveArgUses.clear();
   }
 }
 
 /// MarkValue - This function marks the liveness of RA depending on L. If L is
 /// MaybeLive, it also takes all uses in MaybeLiveUses and records them in Uses,
 /// such that RA will be marked live if any use in MaybeLiveUses gets marked

 /// mark any values that are used as this function's parameters or by its return
 /// values (according to Uses) live as well.
 void DeadArgumentEliminationPass::MarkLive(const Function &F) {
   LLVM_DEBUG(dbgs() << "DeadArgumentEliminationPass - 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)
-    PropagateLiveness(CreateArg(&F, i));
+  for (unsigned ArgI = 0, E = F.arg_size(); ArgI != E; ++ArgI)
+    PropagateLiveness(CreateArg(&F, ArgI));
   // Mark all return values as live.
-  for (unsigned i = 0, e = NumRetVals(&F); i != e; ++i)
-    PropagateLiveness(CreateRet(&F, i));
+  for (unsigned Ri = 0, E = NumRetVals(&F); Ri != E; ++Ri)
+    PropagateLiveness(CreateRet(&F, Ri));
 }
 
 /// 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 DeadArgumentEliminationPass::MarkLive(const RetOrArg &RA) {
   if (LiveFunctions.count(RA.F))
     return; // Function was already marked Live.
bool DeadArgumentEliminationPass::RemoveDeadStuffFromFunction(Function *F) {
   SmallVector<AttributeSet, 8> ArgAttrVec;
   const AttributeList &PAL = F->getAttributes();
 
   // Remember which arguments are still alive.
   SmallVector<bool, 10> ArgAlive(FTy->getNumParams(), false);
   // Construct the new parameter list from non-dead arguments. Also construct
   // a new set of parameter attributes to correspond. Skip the first parameter
   // attribute, since that belongs to the return value.
-  unsigned i = 0;
-  for (Function::arg_iterator I = F->arg_begin(), E = F->arg_end();
-       I != E; ++I, ++i) {
-    RetOrArg Arg = CreateArg(F, i);
+  unsigned ArgI = 0;
+  for (Function::arg_iterator I = F->arg_begin(), E = F->arg_end(); I != E;
+       ++I, ++ArgI) {
+    RetOrArg Arg = CreateArg(F, ArgI);
     if (LiveValues.erase(Arg)) {
       Params.push_back(I->getType());
-      ArgAlive[i] = true;
-      ArgAttrVec.push_back(PAL.getParamAttributes(i));
-      HasLiveReturnedArg |= PAL.hasParamAttribute(i, Attribute::Returned);
+      ArgAlive[ArgI] = true;
+      ArgAttrVec.push_back(PAL.getParamAttributes(ArgI));
+      HasLiveReturnedArg |= PAL.hasParamAttribute(ArgI, Attribute::Returned);
     } else {
       ++NumArgumentsEliminated;
       LLVM_DEBUG(dbgs() << "DeadArgumentEliminationPass - Removing argument "
-                        << i << " (" << I->getName() << ") from "
+                        << ArgI << " (" << I->getName() << ") from "
                         << F->getName() << "\n");
     }
   }
 
   // Find out the new return value.
   Type *RetTy = FTy->getReturnType();
   Type *NRetTy = nullptr;
   unsigned RetCount = NumRetVals(F);
bool DeadArgumentEliminationPass::RemoveDeadStuffFromFunction(Function *F) {
   // the FE are places where 'returned' is basically free and almost always a
   // performance win, so the second option can just be used always for now.
   //
   // This should be revisited if 'returned' is ever applied more liberally.
   if (RetTy->isVoidTy() || HasLiveReturnedArg) {
     NRetTy = RetTy;
   } else {
     // Look at each of the original return values individually.
-    for (unsigned i = 0; i != RetCount; ++i) {
-      RetOrArg Ret = CreateRet(F, i);
+    for (unsigned Ri = 0; Ri != RetCount; ++Ri) {
+      RetOrArg Ret = CreateRet(F, Ri);
       if (LiveValues.erase(Ret)) {
-        RetTypes.push_back(getRetComponentType(F, i));
-        NewRetIdxs[i] = RetTypes.size() - 1;
+        RetTypes.push_back(getRetComponentType(F, Ri));
+        NewRetIdxs[Ri] = RetTypes.size() - 1;
       } else {
         ++NumRetValsEliminated;
         LLVM_DEBUG(
             dbgs() << "DeadArgumentEliminationPass - Removing return value "
-                   << i << " from " << F->getName() << "\n");
+                   << Ri << " from " << F->getName() << "\n");
       }
     }
     if (RetTypes.size() > 1) {
       // More than one return type? Reduce it down to size.
       if (StructType *STy = dyn_cast<StructType>(RetTy)) {
         // Make the new struct packed if we used to return a packed struct
         // already.
         NRetTy = StructType::get(STy->getContext(), RetTypes, STy->isPacked());
bool DeadArgumentEliminationPass::RemoveDeadStuffFromFunction(Function *F) {
   // it again.
   F->getParent()->getFunctionList().insert(F->getIterator(), NF);
   NF->takeName(F);
 
   // Loop over all of the callers of the function, transforming the call sites
   // to pass in a smaller number of arguments into the new function.
   std::vector<Value*> Args;
   while (!F->use_empty()) {
-    CallSite CS(F->user_back());
-    Instruction *Call = CS.getInstruction();
+    CallBase &CB = cast<CallBase>(*F->user_back());
 
     ArgAttrVec.clear();
-    const AttributeList &CallPAL = CS.getAttributes();
+    const AttributeList &CallPAL = CB.getAttributes();
 
     // Adjust the call return attributes in case the function was changed to
     // return void.
     AttrBuilder RAttrs(CallPAL.getRetAttributes());
     RAttrs.remove(AttributeFuncs::typeIncompatible(NRetTy));
     AttributeSet RetAttrs = AttributeSet::get(F->getContext(), RAttrs);
 
     // Declare these outside of the loops, so we can reuse them for the second
     // loop, which loops the varargs.
-    CallSite::arg_iterator I = CS.arg_begin();
-    unsigned i = 0;
+    auto I = CB.arg_begin();
+    unsigned Pi = 0;
     // Loop over those operands, corresponding to the normal arguments to the
     // original function, and add those that are still alive.
-    for (unsigned e = FTy->getNumParams(); i != e; ++I, ++i)
-      if (ArgAlive[i]) {
+    for (unsigned E = FTy->getNumParams(); Pi != E; ++I, ++Pi)
+      if (ArgAlive[Pi]) {
         Args.push_back(*I);
         // Get original parameter attributes, but skip return attributes.
-        AttributeSet Attrs = CallPAL.getParamAttributes(i);
+        AttributeSet Attrs = CallPAL.getParamAttributes(Pi);
         if (NRetTy != RetTy && Attrs.hasAttribute(Attribute::Returned)) {
           // If the return type has changed, then get rid of 'returned' on the
           // call site. The alternative is to make all 'returned' attributes on
           // call sites keep the return value alive just like 'returned'
           // attributes on function declaration but it's less clearly a win and
           // this is not an expected case anyway
           ArgAttrVec.push_back(AttributeSet::get(
               F->getContext(),
               AttrBuilder(Attrs).removeAttribute(Attribute::Returned)));
         } else {
           // Otherwise, use the original attributes.
           ArgAttrVec.push_back(Attrs);
         }
       }
 
     // Push any varargs arguments on the list. Don't forget their attributes.
-    for (CallSite::arg_iterator E = CS.arg_end(); I != E; ++I, ++i) {
+    for (auto E = CB.arg_end(); I != E; ++I, ++Pi) {
       Args.push_back(*I);
-      ArgAttrVec.push_back(CallPAL.getParamAttributes(i));
+      ArgAttrVec.push_back(CallPAL.getParamAttributes(Pi));
     }
 
     // Reconstruct the AttributesList based on the vector we constructed.
     assert(ArgAttrVec.size() == Args.size());
 
     // Again, be sure to remove any allocsize attributes, since their indices
     // may now be incorrect.
     AttributeSet FnAttrs = CallPAL.getFnAttributes().removeAttribute(
         F->getContext(), Attribute::AllocSize);
 
     AttributeList NewCallPAL = AttributeList::get(
         F->getContext(), FnAttrs, RetAttrs, ArgAttrVec);
 
     SmallVector<OperandBundleDef, 1> OpBundles;
-    CS.getOperandBundlesAsDefs(OpBundles);
+    CB.getOperandBundlesAsDefs(OpBundles);
 
-    CallSite NewCS;
-    if (InvokeInst *II = dyn_cast<InvokeInst>(Call)) {
-      NewCS = InvokeInst::Create(NF, II->getNormalDest(), II->getUnwindDest(),
-                                 Args, OpBundles, "", Call->getParent());
+    CallBase *NewCB = nullptr;
+    if (InvokeInst *II = dyn_cast<InvokeInst>(&CB)) {
+      NewCB = InvokeInst::Create(NF, II->getNormalDest(), II->getUnwindDest(),
+                                 Args, OpBundles, "", CB.getParent());
     } else {
-      NewCS = CallInst::Create(NFTy, NF, Args, OpBundles, "", Call);
-      cast<CallInst>(NewCS.getInstruction())
-          ->setTailCallKind(cast<CallInst>(Call)->getTailCallKind());
-    }
-    NewCS.setCallingConv(CS.getCallingConv());
-    NewCS.setAttributes(NewCallPAL);
-    NewCS->setDebugLoc(Call->getDebugLoc());
+      NewCB = CallInst::Create(NFTy, NF, Args, OpBundles, "", &CB);
+      cast<CallInst>(NewCB)->setTailCallKind(
+          cast<CallInst>(&CB)->getTailCallKind());
+    }
+    NewCB->setCallingConv(CB.getCallingConv());
+    NewCB->setAttributes(NewCallPAL);
+    NewCB->setDebugLoc(CB.getDebugLoc());
     uint64_t W;
-    if (Call->extractProfTotalWeight(W))
-      NewCS->setProfWeight(W);
+    if (CB.extractProfTotalWeight(W))
+      NewCB->setProfWeight(W);
     Args.clear();
     ArgAttrVec.clear();
 
-    Instruction *New = NewCS.getInstruction();
-    if (!Call->use_empty() || Call->isUsedByMetadata()) {
-      if (New->getType() == Call->getType()) {
+    if (!CB.use_empty() || CB.isUsedByMetadata()) {
+      if (NewCB->getType() == CB.getType()) {
         // Return type not changed? Just replace users then.
-        Call->replaceAllUsesWith(New);
-        New->takeName(Call);
-      } else if (New->getType()->isVoidTy()) {
+        CB.replaceAllUsesWith(NewCB);
+        NewCB->takeName(&CB);
+      } else if (NewCB->getType()->isVoidTy()) {
         // If the return value is dead, replace any uses of it with undef
         // (any non-debug value uses will get removed later on).
-        if (!Call->getType()->isX86_MMXTy())
-          Call->replaceAllUsesWith(UndefValue::get(Call->getType()));
+        if (!CB.getType()->isX86_MMXTy())
+          CB.replaceAllUsesWith(UndefValue::get(CB.getType()));
       } else {
         assert((RetTy->isStructTy() || RetTy->isArrayTy()) &&
                "Return type changed, but not into a void. The old return type"
                " must have been a struct or an array!");
-        Instruction *InsertPt = Call;
-        if (InvokeInst *II = dyn_cast<InvokeInst>(Call)) {
-          BasicBlock *NewEdge = SplitEdge(New->getParent(), II->getNormalDest());
+        Instruction *InsertPt = &CB;
+        if (InvokeInst *II = dyn_cast<InvokeInst>(&CB)) {
+          BasicBlock *NewEdge =
+              SplitEdge(NewCB->getParent(), II->getNormalDest());
           InsertPt = &*NewEdge->getFirstInsertionPt();
         }
 
         // We used to return a struct or array. Instead of doing smart stuff
         // with all the uses, 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) {
+        for (unsigned Ri = 0; Ri != RetCount; ++Ri)
+          if (NewRetIdxs[Ri] != -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",
+              V = ExtractValueInst::Create(NewCB, NewRetIdxs[Ri], "newret",
                                            InsertPt);
             else
               // We are now returning a single element, so just insert that
-              V = New;
+              V = NewCB;
             // Insert the value at the old position
-            RetVal = InsertValueInst::Create(RetVal, V, i, "oldret", InsertPt);
+            RetVal = InsertValueInst::Create(RetVal, V, Ri, "oldret", InsertPt);
           }
         // Now, replace all uses of the old call instruction with the return
         // struct we built
-        Call->replaceAllUsesWith(RetVal);
-        New->takeName(Call);
+        CB.replaceAllUsesWith(RetVal);
+        NewCB->takeName(&CB);
       }
     }
 
     // Finally, remove the old call from the program, reducing the use-count of
     // F.
-    Call->eraseFromParent();
+    CB.eraseFromParent();
   }
 
   // Since we have now created the new function, splice the body of the old
   // function right into the new function, leaving the old rotting hulk of the
   // function empty.
   NF->getBasicBlockList().splice(NF->begin(), F->getBasicBlockList());
 
   // Loop over the argument list, transferring uses of the old arguments over to
   // the new arguments, also transferring over the names as well.
-  i = 0;
+  ArgI = 0;
   for (Function::arg_iterator I = F->arg_begin(), E = F->arg_end(),
-       I2 = NF->arg_begin(); I != E; ++I, ++i)
-    if (ArgAlive[i]) {
+                              I2 = NF->arg_begin();
+       I != E; ++I, ++ArgI)
+    if (ArgAlive[ArgI]) {
       // If this is a live argument, move the name and users over to the new
       // version.
       I->replaceAllUsesWith(&*I2);
       I2->takeName(&*I);
       ++I2;
     } else {
       // If this argument is dead, replace any uses of it with undef
       // (any non-debug value uses will get removed later on).
       if (!I->getType()->isX86_MMXTy())
         I->replaceAllUsesWith(UndefValue::get(I->getType()));
     }
 
   // If we change the return value of the function we must rewrite any return
   // instructions.  Check this now.
   if (F->getReturnType() != NF->getReturnType())
     for (BasicBlock &BB : *NF)
       if (ReturnInst *RI = dyn_cast<ReturnInst>(BB.getTerminator())) {
-        Value *RetVal;
+        Value *RetVal = nullptr;
 
-        if (NFTy->getReturnType()->isVoidTy()) {
-          RetVal = nullptr;
-        } else {
+        if (!NFTy->getReturnType()->isVoidTy()) {
           assert(RetTy->isStructTy() || RetTy->isArrayTy());
           // The original return value was a struct or array, insert
           // extractvalue/insertvalue chains to extract only the values we need
           // to return and insert them into our new result.
           // This does generate messy code, but we'll let it to instcombine to
           // clean that up.
           Value *OldRet = RI->getOperand(0);
           // Start out building up our return value from undef
           RetVal = UndefValue::get(NRetTy);
-          for (unsigned i = 0; i != RetCount; ++i)
-            if (NewRetIdxs[i] != -1) {
-              ExtractValueInst *EV = ExtractValueInst::Create(OldRet, i,
-                                                              "oldret", RI);
+          for (unsigned RetI = 0; RetI != RetCount; ++RetI)
+            if (NewRetIdxs[RetI] != -1) {
+              ExtractValueInst *EV =
+                  ExtractValueInst::Create(OldRet, RetI, "oldret", RI);
               if (RetTypes.size() > 1) {
                 // We're still returning a struct, so reinsert the value into
                 // our new return value at the new index
 
-                RetVal = InsertValueInst::Create(RetVal, EV, NewRetIdxs[i],
+                RetVal = InsertValueInst::Create(RetVal, EV, NewRetIdxs[RetI],
                                                  "newret", RI);
               } else {
                 // We are now only returning a simple value, so just return the
                 // extracted value.
                 RetVal = EV;
               }
             }
         }