[PATCH] Support of the gc.relocates for invoke statepoints

[Sanjoy Das]

Some minor comments inline.  I can't say I've fully understood this; I'll do a second, review tomorrow.


REPOSITORY
  rL LLVM

================
Comment at: include/llvm/CodeGen/FunctionLoweringInfo.h:82
@@ +81,3 @@
+  // for consistency checks.
+  DenseMap<const Value*, std::pair<bool, int>> InvokeStatepointValueSlots;
+  
----------------
Why not use a `DenseMap<const Value *, Optional<int>>`?

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:554
@@ +553,3 @@
+      SDValue sdV = Builder.getValue(V);
+      SDValue loc = Builder.StatepointLowering.getLocation(sdV);
+
----------------
LLVM conventions say variable names should be CamelCase starting with an uppercase letter.

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:574
@@ +573,3 @@
+      else {
+        Builder.FuncInfo.InvokeStatepointValueSlots[V] =
+          std::make_pair(false, 0);
----------------
So this case is for values like `alloca`s and constants, right?  If so, an assert will make things clearer.

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:579
@@ +578,3 @@
+        // CopyFromReg from undefined register in visitGcRelocate.
+        Builder.ExportFromCurrentBlock(V);
+      }
----------------
So is it correct to say that we need this because `gc_relocate` only "indirectly" uses `V`, in a manner invisible to LLVM?  For instance, if our representation of a `gc_relocate` was 

```
TOK = safepoint
i8* V.relocated = gc_relocate(TOK, i8* V.original)
```

then we would not need this since `V.original` would get exported at its def if needed?

http://reviews.llvm.org/D7798

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