[PATCH] Statepoint infrastructure for garbage collection

[Juergen Ributzka]

Hi Philip,

I looked only at parts of the patch so far. I guess certain things could have been broken out into smaller patches that are not part of the statepoints itself, such as the changes to MachineInstr, intrinsic munging, etc.

In general I noticed that the patch requires some love when it comes to the LLVM coding standards (e.g. CamelCase for variables). Besides that nitpicking the code looks really nice.

Please see my inline comments and questions.

Thanks

Cheers,
Juergen

================
Comment at: include/llvm/CodeGen/MachineInstr.h:91
@@ -90,3 +90,3 @@
 
-  uint8_t NumMemRefs;                   // Information on memory references.
+  uint16_t NumMemRefs;                  // Information on memory references.
   mmo_iterator MemRefs;
----------------
Are you really hitting the limitations of uint8_t for mem references? Changing this by one byte increases the overall size of a MachineInstr from 64 bytes to 72 bytes (due to padding).

================
Comment at: include/llvm/Target/Target.td:859
@@ +858,3 @@
+  let mayStore = 1;
+  let isNotDuplicable = 1;
+  let hasSideEffects = 1;
----------------
Is this a hard requirement for STATEPOINTs to work?

================
Comment at: include/llvm/Target/TargetFrameLowering.h:202-203
@@ -201,1 +201,4 @@
 
+  /// Same as above, except that the 'base register' will always be RSP, not
+  /// RBP on x86.  This should really be a parameterizable choice.
+  virtual int getFrameIndexReferenceForGC(const MachineFunction &MF, int FI,
----------------
Yup, couldn't agree more.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:7330-7334
@@ +7329,7 @@
+  args.insert(args.end(), arg_begin, arg_end); 
+  CallInst* tmp = CallInst::Create(ActualCallee, args);
+  tmp->setTailCall(CI.isTailCall());
+  tmp->setCallingConv(CI.getCallingConv());
+  tmp->setAttributes(CI.getAttributes());
+  Builder.LowerCallTo(tmp, Builder.getValue(ActualCallee), false);
+
----------------
I think we had something similar like this before. I think we changed it because we didn't wan't to modify the LLVM IR by creating a new function call at this point.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:7351
@@ +7350,3 @@
+  delete tmp;
+  tmp = NULL;
+
----------------
nullptr

http://reviews.llvm.org/D5683