Clean up statepoint argument accessors.

[pgavlin]

Summary

This change cleans up some assertions and removes the '# transition args' and '# deopt args' parameters from their respective iterators. These arguments are now accessed solely through 'getNumTotal{GCTransition,VMS}Args'.

[pgavlin]

Sanjoy, this might conflict with D9546. I'm happy to wait until that patch has landed before checking this in.

[reames]

LGTM. Thanks for the cleanup work!

llvm/trunk/include/llvm/IR/Statepoint.h

int getNumCallArgs() {
     return cast<ConstantInt>(NumCallArgsVal)->getZExtValue();
   }
 
   typename CallSiteTy::arg_iterator call_args_begin() {
     assert(CallArgsBeginPos <= (int)StatepointCS.arg_size());
     return StatepointCS.arg_begin() + CallArgsBeginPos;
   }
   typename CallSiteTy::arg_iterator call_args_end() {
-    int Offset = CallArgsBeginPos + getNumCallArgs();
-    assert(Offset <= (int)StatepointCS.arg_size());
-    return StatepointCS.arg_begin() + Offset;
+    auto I = call_args_begin() + getNumCallArgs();
+    assert((StatepointCS.arg_end() - I) >= 0);
+    return I;
   }
 
   /// range adapter for call arguments
   iterator_range<arg_iterator> call_args() {
     return iterator_range<arg_iterator>(call_args_begin(), call_args_end());
   }
 
   /// Number of GC transition args.
   int getNumTotalGCTransitionArgs() {
-    const Value *NumGCTransitionArgs = *gc_transition_args_begin();
+    const Value *NumGCTransitionArgs = *call_args_end();
     return cast<ConstantInt>(NumGCTransitionArgs)->getZExtValue();
   }
   typename CallSiteTy::arg_iterator gc_transition_args_begin() {
-    int Offset = call_args_end() - StatepointCS.arg_begin();
-    assert(Offset <= (int)StatepointCS.arg_size());
-    return StatepointCS.arg_begin() + Offset;
+    auto I = call_args_end() + 1;
+    assert((StatepointCS.arg_end() - I) >= 0);
+    return I;
   }
   typename CallSiteTy::arg_iterator gc_transition_args_end() {
-    int Offset = (gc_transition_args_begin() + 1 +
-                  getNumTotalGCTransitionArgs()) - StatepointCS.arg_begin();
-    assert(Offset <= (int)StatepointCS.arg_size());
-    return StatepointCS.arg_begin() + Offset;
+    auto I = gc_transition_args_begin() + getNumTotalGCTransitionArgs();
+    assert((StatepointCS.arg_end() - I) >= 0);
+    return I;
   }
 
   /// range adapter for GC transition arguments
   iterator_range<arg_iterator> gc_transition_args() {
     return iterator_range<arg_iterator>(gc_transition_args_begin(),
                                         gc_transition_args_end());
   }
 
   /// Number of additional arguments excluding those intended
   /// for garbage collection.
   int getNumTotalVMSArgs() {
-    Value *NumVMSArgs = *vm_state_begin();
+    const Value *NumVMSArgs = *gc_transition_args_end();
     return cast<ConstantInt>(NumVMSArgs)->getZExtValue();
   }
 
   typename CallSiteTy::arg_iterator vm_state_begin() {
-    return gc_transition_args_end();
+    auto I = gc_transition_args_end() + 1;
+    assert((StatepointCS.arg_end() - I) >= 0);
+    return I;
   }
   typename CallSiteTy::arg_iterator vm_state_end() {
-    int Offset = (gc_transition_args_end() + 1 + getNumTotalVMSArgs()) -
-                 StatepointCS.arg_begin();
-    assert(Offset <= (int)StatepointCS.arg_size());
-    return StatepointCS.arg_begin() + Offset;
+    auto I = vm_state_begin() + getNumTotalVMSArgs();
+    assert((StatepointCS.arg_end() - I) >= 0);
+    return I;
   }
 
   /// range adapter for vm state arguments
   iterator_range<arg_iterator> vm_state_args() {
     return iterator_range<arg_iterator>(vm_state_begin(), vm_state_end());
   }
 
   typename CallSiteTy::arg_iterator gc_args_begin() { return vm_state_end(); }

llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp

#ifndef NDEBUG
   }
 #endif
 
   // Before we actually start lowering (and allocating spill slots for values),
   // reserve any stack slots which we judge to be profitable to reuse for a
   // particular value.  This is purely an optimization over the code below and
   // doesn't change semantics at all.  It is important for performance that we
   // reserve slots for both deopt and gc values before lowering either.
-  for (auto I = StatepointSite.vm_state_begin() + 1,
-            E = StatepointSite.vm_state_end();
-       I != E; ++I) {
-    Value *V = *I;
+  for (const Value *V : StatepointSite.vm_state_args()) {
     SDValue Incoming = Builder.getValue(V);
     reservePreviousStackSlotForValue(Incoming, Builder);
   }
   for (unsigned i = 0; i < Bases.size(); ++i) {
     const Value *Base = Bases[i];
     reservePreviousStackSlotForValue(Builder.getValue(Base), Builder);
 
     const Value *Ptr = Ptrs[i];
     reservePreviousStackSlotForValue(Builder.getValue(Ptr), Builder);
   }
 
   // First, prefix the list with the number of unique values to be
   // lowered.  Note that this is the number of *Values* not the
   // number of SDValues required to lower them.
   const int NumVMSArgs = StatepointSite.getNumTotalVMSArgs();
   pushStackMapConstant(Ops, Builder, NumVMSArgs);
 
-  assert(NumVMSArgs + 1 == std::distance(StatepointSite.vm_state_begin(),
-                                         StatepointSite.vm_state_end()));
+  assert(NumVMSArgs == std::distance(StatepointSite.vm_state_begin(),
 
   // The vm state arguments are lowered in an opaque manner.  We do
   // not know what type of values are contained within.  We skip the
   // first one since that happens to be the total number we lowered
   // explicitly just above.  We could have left it in the loop and
   // not done it explicitly, but it's far easier to understand this
   // way.
-  for (auto I = StatepointSite.vm_state_begin() + 1,
-            E = StatepointSite.vm_state_end();
-       I != E; ++I) {
-    const Value *V = *I;
+  for (const Value *V : StatepointSite.vm_state_args()) {
     SDValue Incoming = Builder.getValue(V);
     lowerIncomingStatepointValue(Incoming, Ops, Builder);
   }
 
   // Finally, go ahead and lower all the gc arguments.  There's no prefixed
   // length for this one.  After lowering, we'll have the base and pointer
   // arrays interwoven with each (lowered) base pointer immediately followed by
   // it's (lowered) derived pointer.  i.e
const bool IsGCTransition =
           (uint64_t)StatepointFlags::GCTransition;
   if (IsGCTransition) {
     SmallVector<SDValue, 8> TSOps;
 
     // Add chain
     TSOps.push_back(Chain);
 
     // Add GC transition arguments
-    for (auto I = ISP.gc_transition_args_begin() + 1,
-              E = ISP.gc_transition_args_end();
-         I != E; ++I) {
-      TSOps.push_back(getValue(*I));
-      if ((*I)->getType()->isPointerTy())
-        TSOps.push_back(DAG.getSrcValue(*I));
+    for (const Value *V : ISP.gc_transition_args()) {
+      TSOps.push_back(getValue(V));
+      if (V->getType()->isPointerTy())
+        TSOps.push_back(DAG.getSrcValue(V));
     }
 
     // Add glue if necessary
     if (CallHasIncomingGlue)
       TSOps.push_back(Glue);
 
     SDVTList NodeTys = DAG.getVTList(MVT::Other, MVT::Glue);
 
#endif
   // the operands to the GC_TRANSITION_END node.
   if (IsGCTransition) {
     SmallVector<SDValue, 8> TEOps;
 
     // Add chain
     TEOps.push_back(SDValue(StatepointMCNode, 0));
 
     // Add GC transition arguments
-    for (auto I = ISP.gc_transition_args_begin() + 1,
-              E = ISP.gc_transition_args_end();
-         I != E; ++I) {
-      TEOps.push_back(getValue(*I));
-      if ((*I)->getType()->isPointerTy())
-        TEOps.push_back(DAG.getSrcValue(*I));
+    for (const Value *V : ISP.gc_transition_args()) {
+      TEOps.push_back(getValue(V));
+      if (V->getType()->isPointerTy())
+        TEOps.push_back(DAG.getSrcValue(V));
     }
 
     // Add glue
     TEOps.push_back(SDValue(StatepointMCNode, 1));
 
     SDVTList NodeTys = DAG.getVTList(MVT::Other, MVT::Glue);
 
     SDValue GCTransitionStart =