[PATCH] [StatepointLowering] Don't create temporary instructions. NFCI.

[Sanjoy Das]

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:225
@@ -224,3 @@
-/// reference lowered call result
-static SDNode *lowerCallFromStatepoint(ImmutableStatepoint StatepointSite,
-                                       MachineBasicBlock *LandingPad,
----------------
reames wrote:
> To simplify review, please place the new code into the same helper.  (I think the helper should be there regardless, but it also reduces the diff.)
will do.

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:526
@@ +525,3 @@
+    assert(CallEnd->getOpcode() == ISD::CALLSEQ_END && "expected!");
+  }
+  SDNode *Call = CallEnd->getOperand(0).getNode();
----------------
reames wrote:
> This block looks like code which could and should be commoned into a helper function for statepoints and patchpoints.  possible: findCallNode?
I'll do that once this is in, if you're okay with that.

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:537
@@ -597,10 +536,3 @@
   SmallVector<SDValue, 40> Ops;
 
   // Get number of arguments incoming directly into call node
----------------
reames wrote:
> I would prefer the glue node changes go in separately.  You do not need further review for them, but they're unrelated to the tricky part of this review and should be separated.  
Will do.

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:571
@@ -646,1 +570,3 @@
+  if (HasGlue)
+    Ops.push_back(*(Call->op_end() - 1));
 
----------------
reames wrote:
> Use getGluedNode please.
Sure.

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:580
@@ -653,1 +579,3 @@
 
+  // Update the NodeMap.
+  if (CS.isInvoke()) {
----------------
reames wrote:
> You're missing a check on whether the underlying call produces a value.
Yes.  I wonder why this didn't break any tests.

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:591
@@ +590,3 @@
+
+    RegsForValue RFV(*DAG.getContext(), DAG.getTargetLoweringInfo(), Reg,
+                     ISP.actualReturnType());
----------------
reames wrote:
> I'm pretty sure using the entry point of the chain here is wrong.  I don't understand the motivation for this change.  Can you please separate or revert this part?
The node copying the return value to a virtual register has a data dependence on the call, and it can thus be scheduled anywhere after the call and before the terminator instruction, but otherwise has no control dependencies.  Adding it to `PendingExports` takes care of the latter.

`SelectionDAGBuilder::CopyValueToVirtualRegister` does the same thing.

http://reviews.llvm.org/D9480

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/