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

[Philip Reames]

Meta comment: I can't tell if this code is wrong, but it's not obviously correct.  Your mixing renames, code motion, and restructuring unnecessarily.  This makes it much harder to review for correctness.


================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:225
@@ -224,3 @@
-/// reference lowered call result
-static SDNode *lowerCallFromStatepoint(ImmutableStatepoint StatepointSite,
-                                       MachineBasicBlock *LandingPad,
----------------
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.)

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:526
@@ +525,3 @@
+    assert(CallEnd->getOpcode() == ISD::CALLSEQ_END && "expected!");
+  }
+  SDNode *Call = CallEnd->getOperand(0).getNode();
----------------
This block looks like code which could and should be commoned into a helper function for statepoints and patchpoints.  possible: findCallNode?

================
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
----------------
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.  

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

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:580
@@ -653,1 +579,3 @@
 
+  // Update the NodeMap.
+  if (CS.isInvoke()) {
----------------
You're missing a check on whether the underlying call produces a value.

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:591
@@ +590,3 @@
+
+    RegsForValue RFV(*DAG.getContext(), DAG.getTargetLoweringInfo(), Reg,
+                     ISP.actualReturnType());
----------------
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?

http://reviews.llvm.org/D9480

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