[PATCH] [GC] CodeGenPrep transform: simplify offsetable relocate

[Sanjoy Das]

Small nits inline.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:564
@@ +563,3 @@
+  // Assume that the first relocate in the basic block is relocating the base
+  // object. TODO: what can be done to get rid of this assumption?
+  IntrinsicInst *MasterRelocate = RelocateCalls[0];
----------------
This seems limiting, but correct; and a strict improvement over what we have.  We could easily have a `DenseMap<CallInst *, CallInst *>` that we build in a first pass that maps derived gc relocates to base gc relocates; and write the rest of the algorithm in terms of that.

================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:570
@@ +569,3 @@
+  unsigned BaseIdx =
+    dyn_cast<ConstantInt>(MasterRelocate->getArgOperand(1))->getZExtValue();
+  if (BaseIdx !=
----------------
Don't use `dyn_cast<>` unless you want to check if the cast succeeded or not.  I think a `cast<>` is more appropriate here.  Same applies to some later calls to `dyn_cast`.

================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:593
@@ +592,3 @@
+	Derived->getOperand(0) == Base) {
+      const int Offset = dyn_cast<ConstantInt>(Derived->getOperand(1))->getSExtValue();
+
----------------
You've assumed that the gep index is a constant int here.  Also, how about geps with multiple index operands?  And vector geps?

================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:596
@@ +595,3 @@
+      // Create a Builder and replace the target callsite with a gep
+      IRBuilder<> Builder(II->getContext());
+      Builder.SetInsertPoint(II);
----------------
This can be `IRBuilder<> Builder(II)` I think.

http://reviews.llvm.org/D6883

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