[IR] Add getArg() method to Function class

[hmwildermuth]

Summary

Adds a method which, when called with function.getArg(i), returns an Argument* to the i'th argument.

[tejohnson]

Suggest sending this change with patch that uses and tests it.

[hmwildermuth]

Add test

[hmwildermuth]

In D64925#1599789, @tejohnson wrote:

Suggest sending this change with patch that uses and tests it.

@tejohnson any more recommendations?

[tejohnson]

In D64925#1602949, @hmwildermuth wrote:

In D64925#1599789, @tejohnson wrote:

Suggest sending this change with patch that uses and tests it.

@tejohnson any more recommendations?

It looks fine but is there a plan for a follow on patch that uses this new facility in the code (not just in a unit test)? Wondering since I see that the type of arg_begin() is actually "Argument *" so there is already a fair amount of code that just does something like either arg_begin()[index] or arg_begin()+index. Not opposed to adding a new interface for this, but wondering what the motivating case is.

Also, for future patches - please upload patches with context (see https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface) to make them easier to review.

[hmwildermuth]

In D64925#1604439, @tejohnson wrote:

In D64925#1602949, @hmwildermuth wrote:

In D64925#1599789, @tejohnson wrote:

Suggest sending this change with patch that uses and tests it.

@tejohnson any more recommendations?

It looks fine but is there a plan for a follow on patch that uses this new facility in the code (not just in a unit test)? Wondering since I see that the type of arg_begin() is actually "Argument *" so there is already a fair amount of code that just does something like either arg_begin()[index] or arg_begin()+index. Not opposed to adding a new interface for this, but wondering what the motivating case is.

Also, for future patches - please upload patches with context (see https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface) to make them easier to review.

Thank you for your tip on the patch context, this is my first patch.

The idea is that the method name should be as descriptive as possible for the use; something like arg_begin()[index] is much less descriptive than getArg(index). The reason I originally wrote the patch is because I was writing an LLVM pass and needed a specific argument to a function, but the documentation was unclear to me at the time on how to do it and the use of arg_begin and the arg_iterator type added unnecessary confusion. There also seem to be similar implementations in other classes, e.g. getOperandList () and getOperandUse(index) in the User class, so there is somewhat of a precedent for this kind of facility.

In terms of using this facility, I have no distinct plan but I may go through and replace arg_begin()[index] as I come across them, and would recommend that others should as well to improve readability.

[tejohnson]

lgtm

[hmwildermuth]

I can't seem to commit it myself, could someone else do it for me? Potentially @tejohnson

[tejohnson]

In D64925#1608736, @hmwildermuth wrote:

I can't seem to commit it myself, could someone else do it for me? Potentially @tejohnson

Sure, I will commit for you tomorrow. To get commit access for future patches see https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access

llvm/trunk/include/llvm/IR/Function.h

arg_iterator arg_end() {
     CheckLazyArguments();
     return Arguments + NumArgs;
   }
   const_arg_iterator arg_end() const {
     CheckLazyArguments();
     return Arguments + NumArgs;
   }
 
+  Argument* getArg(unsigned i) const {
+    assert (i < NumArgs && "getArg() out of range!");
+    CheckLazyArguments();
+    return Arguments + i;
+  }
+
   iterator_range<arg_iterator> args() {
     return make_range(arg_begin(), arg_end());
   }
   iterator_range<const_arg_iterator> args() const {
     return make_range(arg_begin(), arg_end());
   }
 
 /// @}

llvm/trunk/unittests/IR/FunctionTest.cpp

TEST(FunctionTest, hasLazyArguments) {
   EXPECT_FALSE(F->arg_empty());
   EXPECT_TRUE(F->hasLazyArguments());
   EXPECT_EQ(2u, F->arg_size());
   EXPECT_TRUE(F->hasLazyArguments());
 
   // The argument list should be populated at first access.
   (void)F->arg_begin();
   EXPECT_FALSE(F->hasLazyArguments());
+
+  // Checking that getArg gets the arguments from F1 in the correct order.
+  unsigned i = 0;
+  for (Argument &A : F->args()) {
+    EXPECT_EQ(&A, F->getArg(i));
+    ++i;
+  }
+  EXPECT_FALSE(F->hasLazyArguments());
 }
 
 TEST(FunctionTest, stealArgumentListFrom) {
   LLVMContext C;
 
   Type *ArgTypes[] = {Type::getInt8Ty(C), Type::getInt32Ty(C)};
   FunctionType *FTy = FunctionType::get(Type::getVoidTy(C), ArgTypes, false);
   std::unique_ptr<Function> F1(