[FunctionAttrs] Remove a loop, NFC refactor

[sanjoy]

Summary

Remove the loop over the uses of the CallSite in ArgumentUsesTracker.
Since we have the Use * for actual argument operand, we can just use
pointer subtraction.

The time complexity remains the same though (except for a vararg
argument) -- std::advance is O(UseIndex) for the ArgumentList
iterator.

The real motivation is to make a later change adding support for operand
bundles simpler.

[chandlerc]

Generally much cleaner, minor comments below, submit whenever.

lib/Transforms/IPO/FunctionAttrs.cpp

324–325 ↗

(On Diff #39299)

Why op_begin here rather than arg_begin?

Also, its a bit odd that the use index here is the correct increment amount for the function argument iterator -- where is the function operand to the callsite?

I'd at least leave some comments to remind the reader of the surprising indexing contract of argument operands vs. function operands and the function argument list.

334–336 ↗

(On Diff #39299)

This can be:

Uses.push_back(std::next(F->arg_begin(), UseIndex));

[sanjoy]

lib/Transforms/IPO/FunctionAttrs.cpp

324–325 ↗

(On Diff #39299)

Why op_begin here rather than arg_begin?

Also, its a bit odd that the use index here is the correct increment
amount for the function argument iterator -- where is the function
operand to the callsite?

For callsites, op_begin == arg_begin. The callee and the two
successor blocks (for invokes) are the last operands to the call or
invoke instruction.

I'll change this to use arg_begin instead, that will make things
more obvious.

I'd at least leave some comments to remind the reader of the
surprising indexing contract of argument operands vs. function
operands and the function argument list.

Will do.

334–336 ↗

(On Diff #39299)

I didn't know std::next took a parameter, thanks! Will fix.

llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp

bool captured(const Use *U) override {
 
     Function *F = CS.getCalledFunction();
     if (!F || F->isDeclaration() || F->mayBeOverridden() ||
         !SCCNodes.count(F)) {
       Captured = true;
       return true;
     }
 
-    bool Found = false;
-    Function::arg_iterator AI = F->arg_begin(), AE = F->arg_end();
-    for (CallSite::arg_iterator PI = CS.arg_begin(), PE = CS.arg_end();
-         PI != PE; ++PI, ++AI) {
-      if (AI == AE) {
-        assert(F->isVarArg() && "More params than args in non-varargs call");
-        Captured = true;
-        return true;
-      }
-      if (PI == U) {
-        Uses.push_back(&*AI);
-        Found = true;
-        break;
-      }
-    }
-    assert(Found && "Capturing call-site captured nothing?");
-    (void)Found;
+    // Note: the callee and the two successor blocks *follow* the argument
+    // operands.  This means there is no need to adjust UseIndex to account for
+    // these.
+
+    unsigned UseIndex =
+        std::distance(const_cast<const Use *>(CS.arg_begin()), U);
+
+    assert(UseIndex < CS.arg_size() && "Non-argument use?");
+    if (UseIndex >= F->arg_size()) {
+
+    Uses.push_back(std::next(F->arg_begin(), UseIndex));
     return false;
   }
 
   bool Captured; // True only if certainly captured (used outside our SCC).
   SmallVector<Argument *, 4> Uses; // Uses within our SCC.
 
   const SCCNodeSet &SCCNodes;
 };