Remove getArgumentList() in favor of arg_begin(), args(), etc

[rnk]

Summary

Users often call getArgumentList().size(), which is a linear way to get
the number of function arguments. arg_size(), on the other hand, is
constant time.

In general, the fact that arguments are stored in an iplist is an
implementation detail, so I've removed it from the Function interface
and moved all other users to the argument container APIs (arg_begin(),
arg_end(), args(), arg_size()).

[chandlerc]

LGTM once split.

include/llvm/IR/Argument.h

40–41

This change seems unrelated, land separately? (Likely simple enough to be obvious.)

include/llvm/IR/Argument.h

 class Argument : public Value, public ilist_node<Argument> {
   virtual void anchor();
   Function *Parent;
 
   friend class SymbolTableListTraits<Argument>;
   void setParent(Function *parent);
 
 public:
-  /// If \p F is specified, the argument is inserted at the end of the argument
-  /// list for \p F.
-  explicit Argument(Type *Ty, const Twine &Name = "", Function *F = nullptr);
+  /// Argument constructor.
+  explicit Argument(Type *Ty, const Twine &Name = "");
 
   inline const Function *getParent() const { return Parent; }
   inline       Function *getParent()       { return Parent; }
 
   /// Return the index of this formal argument in its containing function.
   ///
   /// For example in "void foo(int a, float b)" a is 0 and b is 1.
   unsigned getArgNo() const;

include/llvm/IR/Function.h

public:
   void eraseFromParent() override;
 
   /// Steal arguments from another function.
   ///
   /// Drop this function's arguments and splice in the ones from \c Src.
   /// Requires that this has no function body.
   void stealArgumentListFrom(Function &Src);
 
-  /// Get the underlying elements of the Function... the basic block list is
-  /// empty for external functions.
-  ///
-  const ArgumentListType &getArgumentList() const {
-    CheckLazyArguments();
-    return ArgumentList;
-  }
-  ArgumentListType &getArgumentList() {
-    CheckLazyArguments();
-    return ArgumentList;
-  }
-
   static ArgumentListType Function::*getSublistAccess(Argument*) {
     return &Function::ArgumentList;
   }
 
+  /// Get the underlying elements of the Function... the basic block list is
+  /// empty for external functions.
+  ///
   const BasicBlockListType &getBasicBlockList() const { return BasicBlocks; }
         BasicBlockListType &getBasicBlockList()       { return BasicBlocks; }
 
   static BasicBlockListType Function::*getSublistAccess(BasicBlock*) {
     return &Function::BasicBlocks;
   }
 
   const BasicBlock       &getEntryBlock() const   { return front(); }

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

 findArgumentCopyElisionCandidates(const DataLayout &DL,
                                   FunctionLoweringInfo *FuncInfo,
                                   ArgCopyElisionMapTy &ArgCopyElisionCandidates) {
   // Record the state of every static alloca used in the entry block. Argument
   // allocas are all used in the entry block, so we need approximately as many
   // entries as we have arguments.
   enum StaticAllocaInfo { Unknown, Clobbered, Elidable };
   SmallDenseMap<const AllocaInst *, StaticAllocaInfo, 8> StaticAllocas;
-  unsigned NumArgs = FuncInfo->Fn->getArgumentList().size();
+  unsigned NumArgs = FuncInfo->Fn->arg_size();
   StaticAllocas.reserve(NumArgs * 2);
 
   auto GetInfoIfStaticAlloca = [&](const Value *V) -> StaticAllocaInfo * {
     if (!V)
       return nullptr;
     V = V->stripPointerCasts();
     const auto *AI = dyn_cast<AllocaInst>(V);
     if (!AI || !AI->isStaticAlloca() || !FuncInfo->StaticAllocaMap.count(AI))

lib/IR/Function.cpp

 template class llvm::SymbolTableListTraits<BasicBlock>;
 
 //===----------------------------------------------------------------------===//
 // Argument Implementation
 //===----------------------------------------------------------------------===//
 
 void Argument::anchor() { }
 
-Argument::Argument(Type *Ty, const Twine &Name, Function *Par)
-  : Value(Ty, Value::ArgumentVal) {
-  Parent = nullptr;
-
-  if (Par)
-    Par->getArgumentList().push_back(this);
+Argument::Argument(Type *Ty, const Twine &Name)
+    : Value(Ty, Value::ArgumentVal), Parent(nullptr) {
   setName(Name);
 }
 
 void Argument::setParent(Function *parent) {
   Parent = parent;
 }
 
 unsigned Argument::getArgNo() const {

lib/Target/AArch64/AArch64CallLowering.cpp

bool AArch64CallLowering::lowerReturn(MachineIRBuilder &MIRBuilder,
 
   MIRBuilder.insertInstr(MIB);
   return Success;
 }
 
 bool AArch64CallLowering::lowerFormalArguments(MachineIRBuilder &MIRBuilder,
                                                const Function &F,
                                                ArrayRef<unsigned> VRegs) const {
-  auto &Args = F.getArgumentList();
   MachineFunction &MF = MIRBuilder.getMF();
   MachineBasicBlock &MBB = MIRBuilder.getMBB();
   MachineRegisterInfo &MRI = MF.getRegInfo();
   auto &DL = F.getParent()->getDataLayout();
 
   SmallVector<ArgInfo, 8> SplitArgs;
   unsigned i = 0;
-  for (auto &Arg : Args) {
+  for (auto &Arg : F.args()) {
     ArgInfo OrigArg{VRegs[i], Arg.getType()};
     setArgFlags(OrigArg, i + 1, DL, F);
     bool Split = false;
     LLT Ty = MRI.getType(VRegs[i]);
     unsigned Dst = VRegs[i];
 
     splitToValueTypes(OrigArg, SplitArgs, DL, MRI,
                       [&](unsigned Reg, uint64_t Offset) {

lib/Target/ARM/ARMCallLowering.cpp

bool ARMCallLowering::lowerFormalArguments(MachineIRBuilder &MIRBuilder,
 
   if (Subtarget->isThumb())
     return false;
 
   // FIXME: Support soft float (when we're ready to generate libcalls)
   if (Subtarget->useSoftFloat() || !Subtarget->hasVFP2())
     return false;
 
-  auto &Args = F.getArgumentList();
-  for (auto &Arg : Args)
+  for (auto &Arg : F.args())
     if (!isSupportedType(DL, TLI, Arg.getType()))
       return false;
 
   CCAssignFn *AssignFn =
       TLI.CCAssignFnForCall(F.getCallingConv(), F.isVarArg());
 
   SmallVector<ArgInfo, 8> ArgInfos;
   unsigned Idx = 0;
-  for (auto &Arg : Args) {
+  for (auto &Arg : F.args()) {
     ArgInfo AInfo(VRegs[Idx], Arg.getType());
     setArgFlags(AInfo, Idx + 1, DL, F);
     splitToValueTypes(AInfo, ArgInfos, DL, MF.getRegInfo());
     Idx++;
   }
 
   FormalArgHandler ArgHandler(MIRBuilder, MIRBuilder.getMF().getRegInfo(),
                               AssignFn);

lib/Target/AVR/AVRInstrumentFunctions.cpp

static void BuildSignatureCall(StringRef SymName, BasicBlock &BB, Function &F) {
 
   FunctionType *FnType = FunctionType::get(Type::getVoidTy(Ctx),
     {Type::getInt8PtrTy(Ctx), I16}, false);
 
   Constant *Fn = F.getParent()->getOrInsertFunction(SymName, FnType);
   Value *FunctionName = CreateStringPtr(BB, F.getName());
 
   Value *Args[] = {FunctionName,
-                   ConstantInt::get(I16, F.getArgumentList().size())};
+                   ConstantInt::get(I16, F.arg_size())};
   CallInst::Create(Fn, Args, "", &BB);
 }
 
 /// Builds instructions to call into an external function to
 /// notify about a function signature beginning.
 static void BuildBeginSignature(BasicBlock &BB, Function &F) {
   return BuildSignatureCall(symbols::BEGIN_FUNCTION_SIGNATURE, BB, F);
 }

lib/Target/Mips/MipsOs16.cpp

static  bool needsFPFromSig(Function &F) {
   switch (RetType->getTypeID()) {
   case Type::FloatTyID:
   case Type::DoubleTyID:
     return true;
   default:
     ;
   }
   if (F.arg_size() >=1) {
-    Argument &Arg = F.getArgumentList().front();
+    Argument &Arg = *F.arg_begin();
     switch (Arg.getType()->getTypeID()) {
     case Type::FloatTyID:
     case Type::DoubleTyID:
       return true;
     default:
       ;
     }
   }

lib/Target/PowerPC/PPCISelLowering.cpp

if (CalculateStackSlotUsed(Param.VT, Param.ArgVT, Param.Flags,
                                Subtarget.hasQPX()))
       return true;
   }
   return false;
 }
 
 static bool
 hasSameArgumentList(const Function *CallerFn, ImmutableCallSite *CS) {
-  if (CS->arg_size() != CallerFn->getArgumentList().size())
+  if (CS->arg_size() != CallerFn->arg_size())
     return false;
 
   ImmutableCallSite::arg_iterator CalleeArgIter = CS->arg_begin();
   ImmutableCallSite::arg_iterator CalleeArgEnd = CS->arg_end();
   Function::const_arg_iterator CallerArgIter = CallerFn->arg_begin();
 
   for (; CalleeArgIter != CalleeArgEnd; ++CalleeArgIter, ++CallerArgIter) {
     const Value* CalleeArg = *CalleeArgIter;

lib/Target/X86/X86CallLowering.cpp

if (F.isVarArg())
     return false;
 
   MachineFunction &MF = MIRBuilder.getMF();
   MachineRegisterInfo &MRI = MF.getRegInfo();
   auto DL = MF.getDataLayout();
 
   SmallVector<ArgInfo, 8> SplitArgs;
   unsigned Idx = 0;
-  for (auto &Arg : F.getArgumentList()) {
+  for (auto &Arg : F.args()) {
     ArgInfo OrigArg(VRegs[Idx], Arg.getType());
     setArgFlags(OrigArg, Idx + 1, DL, F);
     LLT Ty = MRI.getType(VRegs[Idx]);
     unsigned Dst = VRegs[Idx];
     bool Split = false;
     splitToValueTypes(OrigArg, SplitArgs, DL, MRI,
                       [&](unsigned Reg, uint64_t Offset) {
                         if (!Split) {

lib/Transforms/Coroutines/CoroElide.cpp

if (auto *Call = dyn_cast<CallInst>(&I))
           report_fatal_error("Call referring to the coroutine frame cannot be "
                              "marked as musttail");
         Call->setTailCall(false);
       }
 }
 
 // Given a resume function @f.resume(%f.frame* %frame), returns %f.frame type.
 static Type *getFrameType(Function *Resume) {
-  auto *ArgType = Resume->getArgumentList().front().getType();
+  auto *ArgType = Resume->arg_begin()->getType();
   return cast<PointerType>(ArgType)->getElementType();
 }
 
 // Finds first non alloca instruction in the entry block of a function.
 static Instruction *getFirstNonAllocaInTheEntryBlock(Function *F) {
   for (Instruction &I : F->getEntryBlock())
     if (!isa<AllocaInst>(&I))
       return &I;

lib/Transforms/Coroutines/CoroFrame.cpp

void coro::buildCoroutineFrame(Function &F, Shape &Shape) {
 
   // Rewrite materializable instructions to be materialized at the use point.
   std::sort(Spills.begin(), Spills.end());
   DEBUG(dump("Materializations", Spills));
   rewriteMaterializableInstructions(Builder, Spills);
 
   // Collect the spills for arguments and other not-materializable values.
   Spills.clear();
-  for (Argument &A : F.getArgumentList())
+  for (Argument &A : F.args())
     for (User *U : A.users())
       if (Checker.isDefinitionAcrossSuspend(A, U))
         Spills.emplace_back(&A, U);
 
   for (Instruction &I : instructions(F)) {
     // Values returned from coroutine structure intrinsics should not be part
     // of the Coroutine Frame.
     if (isCoroutineStructureIntrinsic(I))

lib/Transforms/Coroutines/CoroSplit.cpp

Function *NewF =
                        F.getName() + Suffix, M);
   NewF->addAttribute(1, Attribute::NonNull);
   NewF->addAttribute(1, Attribute::NoAlias);
 
   ValueToValueMapTy VMap;
   // Replace all args with undefs. The buildCoroutineFrame algorithm already
   // rewritten access to the args that occurs after suspend points with loads
   // and stores to/from the coroutine frame.
-  for (Argument &A : F.getArgumentList())
+  for (Argument &A : F.args())
     VMap[&A] = UndefValue::get(A.getType());
 
   SmallVector<ReturnInst *, 4> Returns;
 
   if (DISubprogram *SP = F.getSubprogram()) {
     // If we have debug info, add mapping for the metadata nodes that should not
     // be cloned by CloneFunctionInfo.
     auto &MD = VMap.MD();
static Function *createClone(Function &F, Twine Suffix, coro::Shape &Shape,
 
   // Clear all predecessors of the new entry block.
   auto *Switch = cast<SwitchInst>(VMap[Shape.ResumeSwitch]);
   Entry->replaceAllUsesWith(Switch->getDefaultDest());
 
   IRBuilder<> Builder(&NewF->getEntryBlock().front());
 
   // Remap frame pointer.
-  Argument *NewFramePtr = &NewF->getArgumentList().front();
+  Argument *NewFramePtr = &*NewF->arg_begin();
   Value *OldFramePtr = cast<Value>(VMap[Shape.FramePtr]);
   NewFramePtr->takeName(OldFramePtr);
   OldFramePtr->replaceAllUsesWith(NewFramePtr);
 
   // Remap vFrame pointer.
   auto *NewVFrame = Builder.CreateBitCast(
       NewFramePtr, Type::getInt8PtrTy(Builder.getContext()), "vFrame");
   Value *OldVFrame = cast<Value>(VMap[Shape.CoroBegin]);

lib/Transforms/Instrumentation/DataFlowSanitizer.cpp

Constant *DataFlowSanitizer::getOrBuildTrampolineFunction(FunctionType *FT,
   Function *F = dyn_cast<Function>(C);
   if (F && F->isDeclaration()) {
     F->setLinkage(GlobalValue::LinkOnceODRLinkage);
     BasicBlock *BB = BasicBlock::Create(*Ctx, "entry", F);
     std::vector<Value *> Args;
     Function::arg_iterator AI = F->arg_begin(); ++AI;
     for (unsigned N = FT->getNumParams(); N != 0; ++AI, --N)
       Args.push_back(&*AI);
-    CallInst *CI =
-        CallInst::Create(&F->getArgumentList().front(), Args, "", BB);
+    CallInst *CI = CallInst::Create(&*F->arg_begin(), Args, "", BB);
     ReturnInst *RI;
     if (FT->getReturnType()->isVoidTy())
       RI = ReturnInst::Create(*Ctx, BB);
     else
       RI = ReturnInst::Create(*Ctx, CI, BB);
 
     DFSanFunction DFSF(*this, F, /*IsNativeABI=*/true);
     Function::arg_iterator ValAI = F->arg_begin(), ShadowAI = AI; ++ValAI;
     for (unsigned N = FT->getNumParams(); N != 0; ++ValAI, ++ShadowAI, --N)
       DFSF.ValShadowMap[&*ValAI] = &*ShadowAI;
     DFSanVisitor(DFSF).visitCallInst(*CI);
     if (!FT->getReturnType()->isVoidTy())
       new StoreInst(DFSF.getShadow(RI->getReturnValue()),
-                    &F->getArgumentList().back(), RI);
+                    &*std::prev(F->arg_end()), RI);
   }
 
   return C;
 }
 
 bool DataFlowSanitizer::runOnModule(Module &M) {
   if (ABIList.isIn(M, "skip"))
     return false;
if (Argument *A = dyn_cast<Argument>(V)) {
         Instruction *ArgTLSPos =
             DFS.ArgTLS ? &*F->getEntryBlock().begin()
                        : cast<Instruction>(ArgTLSPtr)->getNextNode();
         IRBuilder<> IRB(ArgTLSPos);
         Shadow = IRB.CreateLoad(getArgTLS(A->getArgNo(), ArgTLSPos));
         break;
       }
       case DataFlowSanitizer::IA_Args: {
-        unsigned ArgIdx = A->getArgNo() + F->getArgumentList().size() / 2;
+        unsigned ArgIdx = A->getArgNo() + F->arg_size() / 2;
         Function::arg_iterator i = F->arg_begin();
         while (ArgIdx--)
           ++i;
         Shadow = &*i;
         assert(Shadow->getType() == DFS.ShadowTy);
         break;
       }
       }

lib/Transforms/Utils/InlineFunction.cpp

Function::iterator FirstNewBlock;
 
     assert(CalledFunc->arg_size() == CS.arg_size() &&
            "No varargs calls can be inlined!");
 
     // Calculate the vector of arguments to pass into the function cloner, which
     // matches up the formal to the actual argument values.
     CallSite::arg_iterator AI = CS.arg_begin();
     unsigned ArgNo = 0;
-    for (Function::const_arg_iterator I = CalledFunc->arg_begin(),
+    for (Function::arg_iterator I = CalledFunc->arg_begin(),
          E = CalledFunc->arg_end(); I != E; ++I, ++AI, ++ArgNo) {
       Value *ActualArg = *AI;
 
       // When byval arguments actually inlined, we need to make the copy implied
       // by them explicit.  However, we don't do this if the callee is readonly
       // or readnone, because the copy would be unneeded: the callee doesn't
       // modify the struct.
       if (CS.isByValArgument(ArgNo)) {

unittests/IR/ValueTest.cpp

const char *ModuleString = "define void @f(i32 %x, i32 %y) {\n"
                              "  ret void\n"
                              "}\n";
   SMDiagnostic Err;
   std::unique_ptr<Module> M = parseAssemblyString(ModuleString, Err, C);
 
   Function *F = M->getFunction("f");
 
   EXPECT_FALSE(F->isUsedInBasicBlock(&F->front()));
-  EXPECT_TRUE((++F->arg_begin())->isUsedInBasicBlock(&F->front()));
+  EXPECT_TRUE(std::next(F->arg_begin())->isUsedInBasicBlock(&F->front()));
   EXPECT_TRUE(F->arg_begin()->isUsedInBasicBlock(&F->front()));
 }
 
 TEST(GlobalTest, CreateAddressSpace) {
   LLVMContext Ctx;
   std::unique_ptr<Module> M(new Module("TestModule", Ctx));
   Type *Int8Ty = Type::getInt8Ty(Ctx);
   Type *Int32Ty = Type::getInt32Ty(Ctx);

unittests/Transforms/Utils/IntegerDivision.cpp

TEST(IntegerDivision, SDiv) {
   LLVMContext C;
   Module M("test division", C);
   IRBuilder<> Builder(C);
 
   SmallVector<Type*, 2> ArgTys(2, Builder.getInt32Ty());
   Function *F = Function::Create(FunctionType::get(Builder.getInt32Ty(),
                                                    ArgTys, false),
                                  GlobalValue::ExternalLinkage, "F", &M);
-  assert(F->getArgumentList().size() == 2);
+  assert(F->arg_size() == 2);
 
   BasicBlock *BB = BasicBlock::Create(C, "", F);
   Builder.SetInsertPoint(BB);
 
   Function::arg_iterator AI = F->arg_begin();
   Value *A = &*AI++;
   Value *B = &*AI++;
 
TEST(IntegerDivision, UDiv) {
   LLVMContext C;
   Module M("test division", C);
   IRBuilder<> Builder(C);
 
   SmallVector<Type*, 2> ArgTys(2, Builder.getInt32Ty());
   Function *F = Function::Create(FunctionType::get(Builder.getInt32Ty(),
                                                    ArgTys, false),
                                  GlobalValue::ExternalLinkage, "F", &M);
-  assert(F->getArgumentList().size() == 2);
+  assert(F->arg_size() == 2);
 
   BasicBlock *BB = BasicBlock::Create(C, "", F);
   Builder.SetInsertPoint(BB);
 
   Function::arg_iterator AI = F->arg_begin();
   Value *A = &*AI++;
   Value *B = &*AI++;
 
TEST(IntegerDivision, SRem) {
   LLVMContext C;
   Module M("test remainder", C);
   IRBuilder<> Builder(C);
 
   SmallVector<Type*, 2> ArgTys(2, Builder.getInt32Ty());
   Function *F = Function::Create(FunctionType::get(Builder.getInt32Ty(),
                                                    ArgTys, false),
                                  GlobalValue::ExternalLinkage, "F", &M);
-  assert(F->getArgumentList().size() == 2);
+  assert(F->arg_size() == 2);
 
   BasicBlock *BB = BasicBlock::Create(C, "", F);
   Builder.SetInsertPoint(BB);
 
   Function::arg_iterator AI = F->arg_begin();
   Value *A = &*AI++;
   Value *B = &*AI++;
 
TEST(IntegerDivision, URem) {
   LLVMContext C;
   Module M("test remainder", C);
   IRBuilder<> Builder(C);
 
   SmallVector<Type*, 2> ArgTys(2, Builder.getInt32Ty());
   Function *F = Function::Create(FunctionType::get(Builder.getInt32Ty(),
                                                    ArgTys, false),
                                  GlobalValue::ExternalLinkage, "F", &M);
-  assert(F->getArgumentList().size() == 2);
+  assert(F->arg_size() == 2);
 
   BasicBlock *BB = BasicBlock::Create(C, "", F);
   Builder.SetInsertPoint(BB);
 
   Function::arg_iterator AI = F->arg_begin();
   Value *A = &*AI++;
   Value *B = &*AI++;
 
TEST(IntegerDivision, SDiv64) {
   LLVMContext C;
   Module M("test division", C);
   IRBuilder<> Builder(C);
 
   SmallVector<Type*, 2> ArgTys(2, Builder.getInt64Ty());
   Function *F = Function::Create(FunctionType::get(Builder.getInt64Ty(),
                                                    ArgTys, false),
                                  GlobalValue::ExternalLinkage, "F", &M);
-  assert(F->getArgumentList().size() == 2);
+  assert(F->arg_size() == 2);
 
   BasicBlock *BB = BasicBlock::Create(C, "", F);
   Builder.SetInsertPoint(BB);
 
   Function::arg_iterator AI = F->arg_begin();
   Value *A = &*AI++;
   Value *B = &*AI++;
 
TEST(IntegerDivision, UDiv64) {
   LLVMContext C;
   Module M("test division", C);
   IRBuilder<> Builder(C);
 
   SmallVector<Type*, 2> ArgTys(2, Builder.getInt64Ty());
   Function *F = Function::Create(FunctionType::get(Builder.getInt64Ty(),
                                                    ArgTys, false),
                                  GlobalValue::ExternalLinkage, "F", &M);
-  assert(F->getArgumentList().size() == 2);
+  assert(F->arg_size() == 2);
 
   BasicBlock *BB = BasicBlock::Create(C, "", F);
   Builder.SetInsertPoint(BB);
 
   Function::arg_iterator AI = F->arg_begin();
   Value *A = &*AI++;
   Value *B = &*AI++;
 
TEST(IntegerDivision, SRem64) {
   LLVMContext C;
   Module M("test remainder", C);
   IRBuilder<> Builder(C);
 
   SmallVector<Type*, 2> ArgTys(2, Builder.getInt64Ty());
   Function *F = Function::Create(FunctionType::get(Builder.getInt64Ty(),
                                                    ArgTys, false),
                                  GlobalValue::ExternalLinkage, "F", &M);
-  assert(F->getArgumentList().size() == 2);
+  assert(F->arg_size() == 2);
 
   BasicBlock *BB = BasicBlock::Create(C, "", F);
   Builder.SetInsertPoint(BB);
 
   Function::arg_iterator AI = F->arg_begin();
   Value *A = &*AI++;
   Value *B = &*AI++;
 
TEST(IntegerDivision, URem64) {
   LLVMContext C;
   Module M("test remainder", C);
   IRBuilder<> Builder(C);
 
   SmallVector<Type*, 2> ArgTys(2, Builder.getInt64Ty());
   Function *F = Function::Create(FunctionType::get(Builder.getInt64Ty(),
                                                    ArgTys, false),
                                  GlobalValue::ExternalLinkage, "F", &M);
-  assert(F->getArgumentList().size() == 2);
+  assert(F->arg_size() == 2);
 
   BasicBlock *BB = BasicBlock::Create(C, "", F);
   Builder.SetInsertPoint(BB);
 
   Function::arg_iterator AI = F->arg_begin();
   Value *A = &*AI++;
   Value *B = &*AI++;