[PATCH] Extend the statepoint intrinsic to allow statepoints to be marked as transitions from GC-aware code to code that is not GC-aware.

[Philip Reames]

This is looking really close to ready to land.  Not quite there yet, but probably only one more round of updates.

I did notice that there are no tests specifically for the lowering of statepoints with transition arguments.  You should be able to write tests using statepoint-example, even if they're not the most useful tests in the world.  At minimum, we can make sure the backend doesn't crash when presented with such a statepoint.  The challenge in testing this really makes me think we should have the CLR GC strategy be included as a builtin GC.


================
Comment at: docs/Statepoints.rst:249
@@ -210,1 +248,3 @@
 
+Assuming that support for generating the necessary transition code for the
+"statepoint-example" GC strategy, the generated X86 assembly would be:
----------------
This paragraph gets confusing.  I think you'd be better off introducing a hypothetical GC, describe briefly how you might add support for such a GC, and then show the resulting code.  Just make it clear that such a GC is hypothetical.  :)

You might also mention where this is actually used.  (i.e. the CLR GC support.)  

I'd be tempted to have you guys actually add your strategy so that we can have the actual support conditional on the gc name and just use that for our example.  

================
Comment at: docs/Statepoints.rst:315
@@ +314,3 @@
+  +-------+---------------------------------------------------+
+  | Bit # | Usage                                             |
+  +=======+===================================================+
----------------
LSB?  MSB?  What order of the bits are you using?

================
Comment at: docs/Statepoints.rst:320
@@ +319,3 @@
+  +-------+---------------------------------------------------+
+  |  1-63 | Unused.                                           |
+  +-------+---------------------------------------------------+
----------------
Currently unused, but required to be unset to allow possible future extensions.  

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:672
@@ +671,3 @@
+      (Flags & (uint64_t)StatepointFlags::MaskAll) == 0 && "unknown flag used");
+  const int Shift = 1;
+  static_assert(
----------------
This shift should be define in StackMaps.h for the StatepointOperands class with a more clear name.  

The other option is to just split this operand into 2.  I have no idea why I masked them to start with, that's just unnecessary complexity.  Might be better to do this as a separate change though.

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:708
@@ +707,3 @@
+  // 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).
----------------
You could shorten this comment if desired by just referencing the format already defined for GC_TRANSITION_START.

http://reviews.llvm.org/D9501

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