Extract out a SelectionDAGBuilder::LowerAsStatepoint; NFC

[sanjoy]

Summary

This is a step towards implementing "direct" lowering of calls and
invokes with deopt operand bundles into STATEPOINT nodes (as opposed to
having them mandatorily pass through RewriteStatepointsForGC, which is
the case today).

This change extracts out a SelectionDAGBuilder::LowerAsStatepoint
helper function that is able to lower a "statepoint like thing", and
uses it to lower gc.statepoint calls. This is an NFC now, but in a
later change we will use LowerAsStatepoint to directly lower calls and
invokes with operand bundles without going through an intermediate
gc.statepoint IR representation.

FYI: I expect SelectionDAGBuilder::StatepointInfo will evolve as I add
support for lowering non gc.statepoints, right now it is fairly tightly
coupled with an IR level gc.statepoint.

[reames]

A couple of minor comments for now. I'm going to return to this Monday. FYI, the organization of this is really bugging me. Hopefully, I'll be able to explain why with a bit more thought. :)

lib/CodeGen/SelectionDAG/StatepointLowering.cpp

249

minor: this looks like an unrelated NFCI change. Please separate and land to simplify review.

345

This is going to be confusing named if reused. For a non-statepoint call with deopt, the GCResult instruction will just be the instruction itself.

375

I think this is valid for a non-statepoint void call as well.

392–393

again, nfci and separate.

580

I think the only thing we're using GCArgs for is this loop. Might be better to just pass in the explicit set of spill slots.

[sanjoy]

• Factored out and landed the s/Value/GCRelocateInst/ bits
• Add back some verification code I had accidentally removed

[sanjoy]

Hi Philip,

In D18106#373810, @reames wrote:

A couple of minor comments for now. I'm going to return to this Monday. FYI, the organization of this is really bugging me. Hopefully, I'll be able to explain why with a bit more thought. :)

If you're concerned about this not being general enough to support
deopt bundle lowering also (i.e. the coupling is too tight), then I
think you're right -- as I've mentioned in the commit message, I'm
fairly sure that I'll have to evolve the LowerAsStatepoint interface
somewhat to support both use cases. If you want, I can wait on
getting this reviewed till I'm done with the full implementation of
deopt bundle lowering (and get them reviewed at the same time, but as
different patches).

Or are you concerned about some other aspect of this patch?

[reames]

The structure of the StatepointInfo struct strongly hints to me that we have a coupling of the API with the implementation. I understand this is an initial step, but I think it would be good to clean up some of the more messy aspects before moving on. I've tried to give a couple of specific suggestions below.

In general, I think we should be asking the question: why is StatepointInfo different from CallLoweringInfo? Merging them might be worth seriously considering. (I'd suggest NOT doing that in this patch, but keeping that goal in mind and merging them in a later step might be good.)

You might also look closely at the lowerInvokable function in SelectionDAGBuilder.

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h

739

The fact we need to pass these in explicitly hints at a broken API. I'll scatter comments prefixed with gc-relocate: below with suggestions.

775

These two fields hint at a problematic coupling of the interface. Why can't this simply be a list of arguments?

lib/CodeGen/SelectionDAG/StatepointLowering.cpp

345

Could this block of code be lifted into the caller and the GCResult param removed? It seems unlikely we'll want this for the deopt only use.

528

gc-relocate: this looks like it should really be an assertion in the visitGCRelocate code. Separate and commit?

596

gc-relocate: It looks like the only thing we're using the relocates for here is to identify the derived pointer. Since we've already done that once, can this become a loop over the derived pointers? Separate and commit?

642

gc-relocate: With all the other users removed, might this code and the startNewStatepoint call be lifted into the parent?

675

Why do we need the IsStatepoint flag? Can't an empty flags convey the same information?

[sanjoy]

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h

775

This is because SelectionDAGBuilder::lowerCallOperands takes a CallSite, unsigned ArgIdx, unsigned NumArgs. Passing in an ArrayRef<Value *> CallArgs is definitely cleaner, but we'll have to generalize lowerCallOperands before that happens.

Edit: I have an idea -- how about we stop using lowerCallOperands and use lowerInvokable instead? Then I can embed TargetLowering::CallLoweringInfo into StatepointInfo and reduce the conceptual duplication. In fact, the nesting could ultimately be: StatepointLoweringInfo contains DeoptLoweringInfo contains CallLoweringInfo. I'll try to code that up to see if it works.

lib/CodeGen/SelectionDAG/StatepointLowering.cpp

345

Yes, I'll do that separately if you don't mind.

596

It is also used below in if (Relocate->getParent() != StatepointInstr->getParent())

642

We still need GCRelocates in lowerStatepointInfoMetaArgs, as noted earlier.

675

Good point, I'll remove this.

[sanjoy]

I moved some code around, and now have a separation between the
information needed to lower the call itself and the "meta" statepoint
information. Out of this I'll try to split out the
populateCallLoweringInfo (and land them if they look trivial).

However, there is one snag -- the Chain parameter of
CallLoweringInfo. With this new organization, we create the
CallLoweringInfo instance earlier than the spills, the CALL_SEQ
nodes end up anchoring to an earlier chain (since
populateCallLoweringInfo set CLI.Chain to DAG.getRoot()) and the
spill code gets DCE'd away.

In the current form of the patch, this is "fixed" by re-setting the
chain later, after we've emitted the spill code. But then the
CallLoweringInfo instance we create is invalid to use till we re-set
the chain which, which is ugly.

The Right(TM) fix to this is to (I think) pass in an explicit Chain
parameter to all the places that consume a CLI, since the Chain is not
part of the thing we're lowering, but a part of the location we want
to lower it at. However, that is a large (but simple) API change
touching all targets.

[reames]

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h

723

Comments about what these do would help...

728

I'm not sure lowerCallOperands is a useful abstraction once you've extracted populateCallLoweringInfo. Might be worth just inlining it at the two call sites.

[sanjoy]

Rebase over http://reviews.llvm.org/rL263663

I think at this point we've reached the point of diminshing returns
for clean-ups before checkin. Philip - what do you think about
checking this in now, and working on it in tree?

[reames]

LGTM. I'm happy to see this evolve further in tree. Having the other user will also make it far more obvious where we've gotten the API wrong. :)

minor comments below. Can be done either before submission or as separate changes depending on your preference.

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h

752

Why explicitly initial this and none of the other fields?

lib/CodeGen/SelectionDAG/StatepointLowering.cpp

506–507

minor: I wouldn't bother changing the function name in this way.

580

This comment still applies. A follow on commit is fine.

858

Might be good to lift these two to the top of the function.

[sanjoy]

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h

752

Will fix -- the only ones with missing initializers right now are StatepointInstr and EHPadBB (the rest of the arguments are objects that should initialize themselves).

lib/CodeGen/SelectionDAG/StatepointLowering.cpp

506–507

Will fix before checkin.

580

Ah, sorry, I somehow missed seeing the comment earlier. I'll check in a follow-on change.

858

Will fix before checkin.

[sanjoy]

lib/CodeGen/SelectionDAG/StatepointLowering.cpp

580

Thinking about it, the current behavior seems somewhat problematic --
whether an incoming gc pointer gets the special "already a spill slot"
treatment depends on whether SelectionDAG lowers it to a
FrameIndexSDNode or not; and that in turn depends on whether the
llvm::Value is statically an llvm::AllocaInst or not. So if I
start with a gc.statepoint that has an alloca as a gc pointer,
something like LCSSA (say) could obscure the llvm::AllocaInst behind
a PHI, and getValue will give us a CopyFromReg, and this would
change behavior.

I think even in the best case we can't do what this bit of code is
trying to do (assuming I understand its intent correctly) -- you could
have a PHI of two different allocas flowing in, into the
gc.statepoint (this could have been created due to tail merging, for
instance) and then we won't even have a fixed frame index that we can
report.

The caveat to the above is that I've assumed that the special
treatment of alloca's is needed for correctness. If this is merely a
performance optimization, then the problem is easier.

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h

public:
 
   std::pair<SDValue, SDValue> lowerCallOperands(
           ImmutableCallSite CS,
           unsigned ArgIdx,
           unsigned NumArgs,
           SDValue Callee,
           Type *ReturnTy,
           const BasicBlock *EHPadBB = nullptr,
           bool IsPatchPoint = false);
 
   /// UpdateSplitBlock - When an MBB was split during scheduling, update the
   /// references that need to refer to the last resulting block.
   void UpdateSplitBlock(MachineBasicBlock *First, MachineBasicBlock *Last);
 
+  /// Describes a gc.statepoint or a gc.statepoint like thing for the purposes
+  /// of lowering into a STATEPOINT node.  Right now it only abstracts an actual
+  /// gc.statepoint, but that will change in the future.
+  struct StatepointInfo {
+    /// Bases[i] is the base pointer for Ptrs[i].  Together they denote the set
+    /// of gc pointers this STATEPOINT has to relocate.
+    ArrayRef<const Value *> Bases;
+    ArrayRef<const Value *> Ptrs;
+
+    /// The set of gc.relocate calls associated with this gc.statepoint.
+    ArrayRef<const GCRelocateInst *> GCRelocates;
+
+    /// The full list of gc arguments to the gc.statepoint being lowered.
+    ArrayRef<const Use> GCArgs;
+
+    /// The list of gc transition arguments present in the gc.statepoint being
+    /// lowered.
+    ArrayRef<const Use> GCTransitionArgs;
+
+    /// The ID that the resulting STATEPOINT instruction has to report.
+    unsigned ID;
+
+    /// The actual destination of the gc.statepoint call or invoke.
+    const Value *CalledValue;
+
+    /// The gc.result associated with this gc.statepoint call, if any.
+    const Instruction *GCResult;
+
+    /// The deoptimization state associated with this gc.statepoint call, if
+    /// any.
+    ArrayRef<const Use> DeoptState;
+
+    /// The "raw" CallSite for this gc.statepoint call.
+    ImmutableCallSite CS;
+
+    /// Denotes that this instance of StatepointInfo represents an actual
+    /// gc.statepoint.  Currently always true.
+    bool IsStatepoint;
+
+    /// Flags associated with the gc.statepoint.
+    uint64_t StatepointFlags;
+
+    /// The number of patchable bytes the gc.statepoint needs to get lowered
+    /// into.
+    unsigned NumPatchBytes;
+
+    /// The call arguments start at this index in the argument list of the raw
+    /// call site.
+    unsigned CallArgBeginIdx;
+
+    /// The number of call arguments.
+    unsigned NumCallArgs;
+
+    /// The exception handling unwind destination, in case this represents an
+    /// invoke of gc.statepoint.
+    const BasicBlock *EHPadBB;
+
+    const BasicBlock *getParent() const { return CS.getParent(); }
+    const Instruction *getInstruction() const { return CS.getInstruction(); }
+    Type *getReturnType() const {
+      auto *FTy = cast<FunctionType>(
+        cast<PointerType>(CalledValue->getType())->getElementType());
+      return FTy->getReturnType();
+    }
+  };
+
+  /// Lower \p SI into a STATEPOINT instruction.
+  void LowerAsStatepoint(const StatepointInfo &SI);
+
   // This function is responsible for the whole statepoint lowering process.
   // It uniformly handles invoke and call statepoints.
   void LowerStatepoint(ImmutableStatepoint Statepoint,
                        const BasicBlock *EHPadBB = nullptr);
 private:
   std::pair<SDValue, SDValue>
   lowerInvokable(TargetLowering::CallLoweringInfo &CLI,
                  const BasicBlock *EHPadBB = nullptr);

lib/CodeGen/SelectionDAG/StatepointLowering.cpp

static void reservePreviousStackSlotForValue(const Value *IncomingValue,
   Builder.StatepointLowering.setLocation(Incoming, Loc);
 }
 
 /// Remove any duplicate (as SDValues) from the derived pointer pairs.  This
 /// is not required for correctness.  It's purpose is to reduce the size of
 /// StackMap section.  It has no effect on the number of spill slots required
 /// or the actual lowering.
 static void
 removeDuplicatesGCPtrs(SmallVectorImpl<const Value *> &Bases,
                        SmallVectorImpl<const Value *> &Ptrs,
                        SmallVectorImpl<const GCRelocateInst *> &Relocs,
                        SelectionDAGBuilder &Builder) {
 
   // This is horribly inefficient, but I don't care right now
   SmallSet<SDValue, 32> Seen;
 
   SmallVector<const Value *, 64> NewBases, NewPtrs;
removeDuplicatesGCPtrs(SmallVectorImpl<const Value *> &Bases,
   assert(Ptrs.size() == Bases.size());
   assert(Ptrs.size() == Relocs.size());
 }
 
 /// Extract call from statepoint, lower it and return pointer to the
 /// call node. Also update NodeMap so that getValue(statepoint) will
 /// reference lowered call result
 static SDNode *
-lowerCallFromStatepoint(ImmutableStatepoint ISP, const BasicBlock *EHPadBB,
+lowerCallFromStatepointInfo(const SelectionDAGBuilder::StatepointInfo &SI,
                             SelectionDAGBuilder &Builder,
                             SmallVectorImpl<SDValue> &PendingExports) {
-
-  ImmutableCallSite CS(ISP.getCallSite());
-
   SDValue ActualCallee;
 
-  if (ISP.getNumPatchBytes() > 0) {
+  if (SI.NumPatchBytes > 0) {
     // If we've been asked to emit a nop sequence instead of a call instruction
     // for this statepoint then don't lower the call target, but use a constant
     // `null` instead.  Not lowering the call target lets statepoint clients get
     // away without providing a physical address for the symbolic call target at
     // link time.
 
     const auto &TLI = Builder.DAG.getTargetLoweringInfo();
     const auto &DL = Builder.DAG.getDataLayout();
 
-    unsigned AS = ISP.getCalledValue()->getType()->getPointerAddressSpace();
+    unsigned AS = SI.CalledValue->getType()->getPointerAddressSpace();
     ActualCallee = Builder.DAG.getConstant(0, Builder.getCurSDLoc(),
                                            TLI.getPointerTy(DL, AS));
   } else {
-    ActualCallee = Builder.getValue(ISP.getCalledValue());
+    ActualCallee = Builder.getValue(SI.CalledValue);
   }
 
-  assert(CS.getCallingConv() != CallingConv::AnyReg &&
+  assert(SI.CS.getCallingConv() != CallingConv::AnyReg &&
          "anyregcc is not supported on statepoints!");
 
-  Type *DefTy = ISP.getActualReturnType();
+  Type *DefTy = SI.getReturnType();
   bool HasDef = !DefTy->isVoidTy();
 
   SDValue ReturnValue, CallEndVal;
   std::tie(ReturnValue, CallEndVal) = Builder.lowerCallOperands(
-      ISP.getCallSite(), ImmutableStatepoint::CallArgsBeginPos,
-      ISP.getNumCallArgs(), ActualCallee, DefTy, EHPadBB,
-      false /* IsPatchPoint */);
+      SI.CS, SI.CallArgBeginIdx, SI.NumCallArgs, ActualCallee, DefTy,
+      SI.EHPadBB, false /* IsPatchPoint */);
 
   SDNode *CallEnd = CallEndVal.getNode();
 
   // Get a call instruction from the call sequence chain.  Tail calls are not
   // allowed.  The following code is essentially reverse engineering X86's
   // LowerCallTo.
   //
   // We are expecting DAG to have the following form:
if (HasDef) {
     else
       while (CallEnd->getOpcode() == ISD::CopyFromReg)
         CallEnd = CallEnd->getOperand(0).getNode();
   }
 
   assert(CallEnd->getOpcode() == ISD::CALLSEQ_END && "expected!");
 
   // Export the result value if needed
-  const Instruction *GCResult = ISP.getGCResult();
+  const Instruction *GCResult = SI.GCResult;
   if (HasDef && GCResult) {
-    if (GCResult->getParent() != CS.getParent()) {
+    if (GCResult->getParent() != SI.CS.getParent()) {
       // Result value will be used in a different basic block so we need to
       // export it now.
       // Default exporting mechanism will not work here because statepoint call
       // has a different type than the actual call. It means that by default
       // llvm will create export register of the wrong type (always i32 in our
       // case). So instead we need to create export register with correct type
       // manually.
       // TODO: To eliminate this problem we can remove gc.result intrinsics
       //       completely and make statepoint call to return a tuple.
-      unsigned Reg = Builder.FuncInfo.CreateRegs(ISP.getActualReturnType());
-      RegsForValue RFV(
-          *Builder.DAG.getContext(), Builder.DAG.getTargetLoweringInfo(),
-          Builder.DAG.getDataLayout(), Reg, ISP.getActualReturnType());
+      unsigned Reg = Builder.FuncInfo.CreateRegs(SI.getReturnType());
+      RegsForValue RFV(*Builder.DAG.getContext(),
+                       Builder.DAG.getTargetLoweringInfo(),
+                       Builder.DAG.getDataLayout(), Reg, SI.getReturnType());
       SDValue Chain = Builder.DAG.getEntryNode();
 
       RFV.getCopyToRegs(ReturnValue, Builder.DAG, Builder.getCurSDLoc(), Chain,
                         nullptr);
       PendingExports.push_back(Chain);
-      Builder.FuncInfo.ValueMap[CS.getInstruction()] = Reg;
+      Builder.FuncInfo.ValueMap[SI.getInstruction()] = Reg;
     } else {
       // Result value will be used in a same basic block. Don't export it or
       // perform any explicit register copies.
       // We'll replace the actuall call node shortly. gc_result will grab
       // this value.
-      Builder.setValue(CS.getInstruction(), ReturnValue);
+      Builder.setValue(SI.getInstruction(), ReturnValue);
     }
-  } else {
+  } else if (SI.IsStatepoint) {
     // The token value is never used from here on, just generate a poison value
-    Builder.setValue(CS.getInstruction(),
+    Builder.setValue(SI.getInstruction(),
                      Builder.DAG.getIntPtrConstant(-1, Builder.getCurSDLoc()));
   }
 
   return CallEnd->getOperand(0).getNode();
 }
 
 /// Callect all gc pointers coming into statepoint intrinsic, clean them up,
 /// and return two arrays:
 ///   Bases - base pointers incoming to this statepoint
 ///   Ptrs - derived pointers incoming to this statepoint
 ///   Relocs - the gc_relocate corresponding to each base/ptr pair
 /// Elements of this arrays should be in one-to-one correspondence with each
 /// other i.e Bases[i], Ptrs[i] are from the same gcrelocate call
 static void getIncomingStatepointGCValues(
     SmallVectorImpl<const Value *> &Bases, SmallVectorImpl<const Value *> &Ptrs,
     SmallVectorImpl<const GCRelocateInst *> &Relocs,
     ImmutableStatepoint StatepointSite, SelectionDAGBuilder &Builder) {
   for (const GCRelocateInst *Relocate : StatepointSite.getRelocates()) {
     Relocs.push_back(Relocate);
     Bases.push_back(Relocate->getBasePtr());
     Ptrs.push_back(Relocate->getDerivedPtr());
   }
 
   // Remove any redundant llvm::Values which map to the same SDValue as another
   // input.  Also has the effect of removing duplicates in the original

 
 /// Lower deopt state and gc pointer arguments of the statepoint.  The actual
 /// lowering is described in lowerIncomingStatepointValue.  This function is
 /// responsible for lowering everything in the right position and playing some
 /// tricks to avoid redundant stack manipulation where possible.  On
 /// completion, 'Ops' will contain ready to use operands for machine code
 /// statepoint. The chain nodes will have already been created and the DAG root
 /// will be set to the last value spilled (if any were).
-static void lowerStatepointMetaArgs(SmallVectorImpl<SDValue> &Ops,
-                                    ImmutableStatepoint StatepointSite,
-                                    SelectionDAGBuilder &Builder) {
+static void
+lowerStatepointInfoMetaArgs(SmallVectorImpl<SDValue> &Ops,
+                            const SelectionDAGBuilder::StatepointInfo &SI,
 
   // Lower the deopt and gc arguments for this statepoint.  Layout will
   // be: deopt argument length, deopt arguments.., gc arguments...
-
-  SmallVector<const Value *, 64> Bases, Ptrs;
-  SmallVector<const GCRelocateInst *, 64> Relocations;
-  getIncomingStatepointGCValues(Bases, Ptrs, Relocations, StatepointSite,
-                                Builder);
-
 #ifndef NDEBUG
   // Check that each of the gc pointer and bases we've gotten out of the
   // safepoint is something the strategy thinks might be a pointer (or vector
   // of pointers) into the GC heap.  This is basically just here to help catch
   // errors during statepoint insertion. TODO: This should actually be in the
   // Verifier, but we can't get to the GCStrategy from there (yet).
   GCStrategy &S = Builder.GFI->getStrategy();
-  for (const Value *V : Bases) {
+  for (const Value *V : SI.Bases) {
     auto Opt = S.isGCManagedPointer(V->getType()->getScalarType());
     if (Opt.hasValue()) {
       assert(Opt.getValue() &&
              "non gc managed base pointer found in statepoint");
     }
   }
-  for (const Value *V : Ptrs) {
+  for (const Value *V : SI.Ptrs) {
     auto Opt = S.isGCManagedPointer(V->getType()->getScalarType());
     if (Opt.hasValue()) {
       assert(Opt.getValue() &&
              "non gc managed derived pointer found in statepoint");
     }
   }
-  for (const GCRelocateInst *GCR : Relocations) {
+  for (const GCRelocateInst *GCR : SI.GCRelocates) {
     auto Opt = S.isGCManagedPointer(GCR->getType()->getScalarType());
     if (Opt.hasValue()) {
       assert(Opt.getValue() && "non gc managed pointer relocated");
     }
   }
 #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 (const Value *V : StatepointSite.vm_state_args()) {
+  for (const Value *V : SI.DeoptState) {
     reservePreviousStackSlotForValue(V, Builder);
   }
-  for (unsigned i = 0; i < Bases.size(); ++i) {
-    reservePreviousStackSlotForValue(Bases[i], Builder);
-    reservePreviousStackSlotForValue(Ptrs[i], Builder);
+  for (unsigned i = 0; i < SI.Bases.size(); ++i) {
+    reservePreviousStackSlotForValue(SI.Bases[i], Builder);
+    reservePreviousStackSlotForValue(SI.Ptrs[i], 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();
+  const int NumVMSArgs = SI.DeoptState.size();
   pushStackMapConstant(Ops, Builder, NumVMSArgs);
 
-  assert(NumVMSArgs == std::distance(StatepointSite.vm_state_begin(),
-                                     StatepointSite.vm_state_end()));
-
-  // 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 (const Value *V : StatepointSite.vm_state_args()) {
+  // The vm state arguments are lowered in an opaque manner.  We do not know
+  // what type of values are contained within.
+  for (const Value *V : SI.DeoptState) {
     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
   // (base[0], ptr[0], base[1], ptr[1], ...)
-  for (unsigned i = 0; i < Bases.size(); ++i) {
-    const Value *Base = Bases[i];
+  for (unsigned i = 0; i < SI.Bases.size(); ++i) {
+    const Value *Base = SI.Bases[i];
     lowerIncomingStatepointValue(Builder.getValue(Base), Ops, Builder);
 
-    const Value *Ptr = Ptrs[i];
+    const Value *Ptr = SI.Ptrs[i];
     lowerIncomingStatepointValue(Builder.getValue(Ptr), Ops, Builder);
   }
 
   // If there are any explicit spill slots passed to the statepoint, record
   // them, but otherwise do not do anything special.  These are user provided
   // allocas and give control over placement to the consumer.  In this case,
   // it is the contents of the slot which may get updated, not the pointer to
   // the alloca
-  for (Value *V : StatepointSite.gc_args()) {
+  for (Value *V : SI.GCArgs) {
     SDValue Incoming = Builder.getValue(V);
     if (FrameIndexSDNode *FI = dyn_cast<FrameIndexSDNode>(Incoming)) {
       // This handles allocas as arguments to the statepoint
       Ops.push_back(Builder.DAG.getTargetFrameIndex(FI->getIndex(),
                                                     Incoming.getValueType()));
     }
   }
 
   // Record computed locations for all lowered values.
   // This can not be embedded in lowering loops as we need to record *all*
   // values, while previous loops account only values with unique SDValues.
-  const Instruction *StatepointInstr =
-    StatepointSite.getCallSite().getInstruction();
+  const Instruction *StatepointInstr = SI.getInstruction();
   auto &SpillMap = Builder.FuncInfo.StatepointRelocatedValues[StatepointInstr];
 
-  for (const GCRelocateInst *Relocate : StatepointSite.getRelocates()) {
+  for (const GCRelocateInst *Relocate : SI.GCRelocates) {
     const Value *V = Relocate->getDerivedPtr();
     SDValue SDV = Builder.getValue(V);
     SDValue Loc = Builder.StatepointLowering.getLocation(SDV);
 
     if (Loc.getNode()) {
       SpillMap[V] = cast<FrameIndexSDNode>(Loc)->getIndex();
     } else {
       // Record value as visited, but not spilled. This is case for allocas
       // and constants. For this values we can avoid emitting spill load while

 void SelectionDAGBuilder::visitStatepoint(const CallInst &CI) {
   // Check some preconditions for sanity
   assert(isStatepoint(&CI) &&
          "Function called must be the statepoint function");
 
   LowerStatepoint(ImmutableStatepoint(&CI));
 }
 
-void SelectionDAGBuilder::LowerStatepoint(
-    ImmutableStatepoint ISP, const BasicBlock *EHPadBB /*= nullptr*/) {
+void SelectionDAGBuilder::LowerAsStatepoint(
+    const SelectionDAGBuilder::StatepointInfo &SI) {
   // The basic scheme here is that information about both the original call and
   // the safepoint is encoded in the CallInst.  We create a temporary call and
   // lower it, then reverse engineer the calling sequence.
 
   NumOfStatepoints++;
   // Clear state
   StatepointLowering.startNewStatepoint(*this);
 
-  ImmutableCallSite CS(ISP.getCallSite());
-
-#ifndef NDEBUG
-  // Consistency check. Check only relocates in the same basic block as thier
-  // statepoint.
-  for (const User *U : CS->users()) {
-    const CallInst *Call = cast<CallInst>(U);
-    if (isa<GCRelocateInst>(Call) && Call->getParent() == CS.getParent())
-      StatepointLowering.scheduleRelocCall(*Call);
-  }
-#endif
-
 #ifndef NDEBUG
-  // If this is a malformed statepoint, report it early to simplify debugging.
-  // This should catch any IR level mistake that's made when constructing or
-  // transforming statepoints.
-  ISP.verify();
-
-  // Check that the associated GCStrategy expects to encounter statepoints.
-  assert(GFI->getStrategy().useStatepoints() &&
-         "GCStrategy does not expect to encounter statepoints");
+  for (auto *Reloc : SI.GCRelocates)
+    if (Reloc->getParent() == SI.getParent())
+      StatepointLowering.scheduleRelocCall(*Reloc);
 #endif
 
   // Lower statepoint vmstate and gcstate arguments
   SmallVector<SDValue, 10> LoweredMetaArgs;
-  lowerStatepointMetaArgs(LoweredMetaArgs, ISP, *this);
+  lowerStatepointInfoMetaArgs(LoweredMetaArgs, SI, *this);
 
   // Get call node, we will replace it later with statepoint
-  SDNode *CallNode =
-      lowerCallFromStatepoint(ISP, EHPadBB, *this, PendingExports);
+  SDNode *CallNode = lowerCallFromStatepointInfo(SI, *this, PendingExports);
 
   // Construct the actual GC_TRANSITION_START, STATEPOINT, and GC_TRANSITION_END
   // nodes with all the appropriate arguments and return values.
 
   // Call Node: Chain, Target, {Args}, RegMask, [Glue]
   SDValue Chain = CallNode->getOperand(0);
 
   SDValue Glue;
   bool CallHasIncomingGlue = CallNode->getGluedNode();
   if (CallHasIncomingGlue) {
     // Glue is always last operand
     Glue = CallNode->getOperand(CallNode->getNumOperands() - 1);
   }
 
   // Build the GC_TRANSITION_START node if necessary.
   //
   // The operands to the GC_TRANSITION_{START,END} nodes are laid out in the
   // order in which they appear in the call to the statepoint intrinsic. If
   // any of the operands is a pointer-typed, that operand is immediately
   // followed by a SRCVALUE for the pointer that may be used during lowering
   // (e.g. to form MachinePointerInfo values for loads/stores).
   const bool IsGCTransition =
-      (ISP.getFlags() & (uint64_t)StatepointFlags::GCTransition) ==
+      SI.IsStatepoint &&
+      (SI.StatepointFlags & (uint64_t)StatepointFlags::GCTransition) ==
           (uint64_t)StatepointFlags::GCTransition;
   if (IsGCTransition) {
     SmallVector<SDValue, 8> TSOps;
 
     // Add chain
     TSOps.push_back(Chain);
 
     // Add GC transition arguments
-    for (const Value *V : ISP.gc_transition_args()) {
+    for (const Value *V : SI.GCTransitionArgs) {
       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);
 
     SDValue GCTransitionStart =
         DAG.getNode(ISD::GC_TRANSITION_START, getCurSDLoc(), NodeTys, TSOps);
 
     Chain = GCTransitionStart.getValue(0);
     Glue = GCTransitionStart.getValue(1);
   }
 
   // TODO: Currently, all of these operands are being marked as read/write in
   // PrologEpilougeInserter.cpp, we should special case the VMState arguments
   // and flags to be read-only.
   SmallVector<SDValue, 40> Ops;
 
   // Add the <id> and <numBytes> constants.
-  Ops.push_back(DAG.getTargetConstant(ISP.getID(), getCurSDLoc(), MVT::i64));
+  Ops.push_back(DAG.getTargetConstant(SI.ID, getCurSDLoc(), MVT::i64));
   Ops.push_back(
-      DAG.getTargetConstant(ISP.getNumPatchBytes(), getCurSDLoc(), MVT::i32));
+      DAG.getTargetConstant(SI.NumPatchBytes, getCurSDLoc(), MVT::i32));
 
   // Calculate and push starting position of vmstate arguments
   // Get number of arguments incoming directly into call node
   unsigned NumCallRegArgs =
       CallNode->getNumOperands() - (CallHasIncomingGlue ? 4 : 3);
   Ops.push_back(DAG.getTargetConstant(NumCallRegArgs, getCurSDLoc(), MVT::i32));
 
   // Add call target
   SDValue CallTarget = SDValue(CallNode->getOperand(1).getNode(), 0);
   Ops.push_back(CallTarget);
 
   // Add call arguments
   // Get position of register mask in the call
   SDNode::op_iterator RegMaskIt;
   if (CallHasIncomingGlue)
     RegMaskIt = CallNode->op_end() - 2;
   else
     RegMaskIt = CallNode->op_end() - 1;
   Ops.insert(Ops.end(), CallNode->op_begin() + 2, RegMaskIt);
 
   // Add a constant argument for the calling convention
-  pushStackMapConstant(Ops, *this, CS.getCallingConv());
+  pushStackMapConstant(Ops, *this, SI.CS.getCallingConv());
 
   // Add a constant argument for the flags
-  uint64_t Flags = ISP.getFlags();
+  uint64_t Flags = SI.StatepointFlags;
   assert(((Flags & ~(uint64_t)StatepointFlags::MaskAll) == 0) &&
          "Unknown flag used");
   pushStackMapConstant(Ops, *this, Flags);
 
   // Insert all vmstate and gcstate arguments
   Ops.insert(Ops.end(), LoweredMetaArgs.begin(), LoweredMetaArgs.end());
 
   // Add register mask from call node
#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 (const Value *V : ISP.gc_transition_args()) {
+    for (const Value *V : SI.GCTransitionArgs) {
       TEOps.push_back(getValue(V));
       if (V->getType()->isPointerTy())
         TEOps.push_back(DAG.getSrcValue(V));
     }
 
     // Add glue
     TEOps.push_back(SDValue(StatepointMCNode, 1));
 
#endif
 
   // TODO: A better future implementation would be to emit a single variable
   // argument, variable return value STATEPOINT node here and then hookup the
   // return value of each gc.relocate to the respective output of the
   // previously emitted STATEPOINT value.  Unfortunately, this doesn't appear
   // to actually be possible today.
 }
 
+void
+SelectionDAGBuilder::LowerStatepoint(ImmutableStatepoint ISP,
+                                     const BasicBlock *EHPadBB /*= nullptr*/) {
+  SmallVector<const Value *, 16> Bases;
+  SmallVector<const Value *, 16> Ptrs;
+  SmallVector<const GCRelocateInst *, 16> GCRelocates;
+
+  getIncomingStatepointGCValues(Bases, Ptrs, GCRelocates, ISP, *this);
+
+  StatepointInfo SI;
+  SI.Bases = Bases;
+  SI.Ptrs = Ptrs;
+  SI.GCRelocates = GCRelocates;
+  SI.GCArgs = ArrayRef<const Use>(ISP.gc_args_begin(), ISP.gc_args_end());
+  SI.GCTransitionArgs =
+      ArrayRef<const Use>(ISP.gc_args_begin(), ISP.gc_args_end());
+  SI.ID = ISP.getID();
+  SI.CalledValue = ISP.getCalledValue();
+  SI.GCResult = ISP.getGCResult();
+  SI.DeoptState = ArrayRef<const Use>(ISP.vm_state_begin(), ISP.vm_state_end());
+  SI.CS = ISP.getCallSite();
+  SI.IsStatepoint = true;
+  SI.StatepointFlags = ISP.getFlags();
+  SI.NumPatchBytes = ISP.getNumPatchBytes();
+  SI.CallArgBeginIdx = ImmutableStatepoint::CallArgsBeginPos;
+  SI.NumCallArgs = ISP.getNumCallArgs();
+  SI.EHPadBB = EHPadBB;
+
+#ifndef NDEBUG
+  // If this is a malformed statepoint, report it early to simplify debugging.
+  // This should catch any IR level mistake that's made when constructing or
+  // transforming statepoints.
+  ISP.verify();
+
+  // Check that the associated GCStrategy expects to encounter statepoints.
+  assert(GFI->getStrategy().useStatepoints() &&
+         "GCStrategy does not expect to encounter statepoints");
+#endif
+
+  LowerAsStatepoint(SI);
+}
+
 void SelectionDAGBuilder::visitGCResult(const CallInst &CI) {
   // The result value of the gc_result is simply the result of the actual
   // call.  We've already emitted this, so just grab the value.
   Instruction *I = cast<Instruction>(CI.getArgOperand(0));
   assert(isStatepoint(I) && "first argument must be a statepoint token");
 
   if (I->getParent() != CI.getParent()) {
     // Statepoint is in different basic block so we should have stored call
     // result in a virtual register.
     // We can not use default getValue() functionality to copy value from this
     // register because statepoint and actuall call return types can be
     // different, and getValue() will use CopyFromReg of the wrong type,
     // which is always i32 in our case.
     PointerType *CalleeType = cast<PointerType>(
         ImmutableStatepoint(I).getCalledValue()->getType());
     Type *RetTy =
         cast<FunctionType>(CalleeType->getElementType())->getReturnType();
     SDValue CopyFromReg = getCopyFromRegs(I, RetTy);