[PATCH] [RewriteStatepointsForGC] Fix a bug on creating gc_relocate for pointer to vector of pointers

[Sanjoy Das]

LGTM


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:627
@@ +626,3 @@
+    if (RelocatedBase->getType() != Base->getType()) {
+      ActualRelocatedBase =
+          cast<Instruction>(Builder.CreateBitCast(RelocatedBase, Base->getType()));
----------------
chenli wrote:
> sanjoy wrote:
> > Why not directly insert `ActualRelocatedBase` after `RelocatedBase` by something like:
> > 
> > ```
> > if (...) {
> >   IRBuilder::InsertPointGuard IPG(RelocatedBase->getNextNode());
> >   Builder.CreateBitCast ...
> > ```
> I basically try to follow the existing code:
> 
> 
> ```
> Value *Replacement = Builder.CreateGEP(
>        Derived->getSourceElementType(), RelocatedBase, makeArrayRef(OffsetV));
> Instruction *ReplacementInst = cast<Instruction>(Replacement);
> ReplacementInst->removeFromParent();
> ReplacementInst->insertAfter(RelocatedBase);
> ```
> 
> I wouldn't mind to use either of the styles, but I would prefer to do a separate patch if we need to change.
Sure, I think fixing all of this together in a separate change is a good idea.  The Create->Remove->ReInsert flow seems very odd to me, unless there's a reason for it I can't see.

http://reviews.llvm.org/D9592

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