[PATCH] Statepoint infrastructure for garbage collection

[Philip Reames]

morisset, I think I've addressed all your comments.  Let me know if you see anything I missed.

================
Comment at: include/llvm/CodeGen/MachineInstr.h:91
@@ -90,2 +90,3 @@
 
-  uint8_t NumMemRefs;                   // Information on memory references.
+  // BEGIN CHANGE: NumMemRefs was a uint8_t, which is too
+  // small to hold the number of gc ptrs in some statepoints.
----------------
morisset wrote:
> What are these 'BEGIN CHANGE/END CHANGE'  comments ?
Removed.

================
Comment at: include/llvm/IR/Intrinsics.td:500
@@ +499,3 @@
+                                llvm_vararg_ty],
+                               [Throws]>;
+
----------------
morisset wrote:
> Is this necessary ? I don't see in the documentation any case where the statepoint can throw an exception. And if the goal is just to say it can have side-effects on memory, that is the default if there is no annotation according to Intrinsics.td
Removed. This was an artefact of work to support invokable statepoints which aren't part of this patch. 

================
Comment at: include/llvm/IR/Intrinsics.td:502
@@ +501,3 @@
+
+def int_gc_result_int : Intrinsic<[llvm_anyint_ty], [llvm_i32_ty]>;
+def int_gc_result_float : Intrinsic<[llvm_anyfloat_ty], [llvm_i32_ty]>;
----------------
morisset wrote:
> These (and gc_relocate) should probably have the attribute IntrNoMem if I understood correctly your documentation.
You're right.  This change will need more testing that I can easily give it at the moment.  Do you mind if this change lands separately once this is integrated and I've merged it back into our tree?

================
Comment at: lib/IR/Verifier.cpp:1120
@@ +1119,3 @@
+      if (const Instruction* I = dyn_cast<Instruction>(U)) {
+        if (isStatepoint( const_cast<Instruction*>(I))) {
+          ok = true;
----------------
morisset wrote:
> It is not clear to me why the const_cast is required: I is a const Instruction *, and isStatepoint accepts a const Instruction *.
It's not.  This is just old code that hadn't been updated.  Fixed.  Thanks.

================
Comment at: lib/Target/X86/X86MCInstLower.cpp:862
@@ +861,3 @@
+    // This is likelly to be patchable call, so we need to ensure that it will
+    // not cross 16-byte boundary as VM requires from us.
+    // 8-byte alignment is enough because this instruction is 5-byte long and
----------------
morisset wrote:
> Is this invariant something general to LLVM and documented as such, or specific to your application (I am asking because the comment talks about a 'VM')?
This is preliminary support for atomically patchable calls.  For now, I just documented the 32 bit offset requirement and removed the rest.  This will be a separate change proposal down the road.

http://reviews.llvm.org/D5683