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

[• chenli]

Summary

In RewriteStatepointsForGC pass, we create a gc_relocate intrinsic for each relocated pointer, and the gc_relocate has the same type with the pointer. During the creation of gc_relocate intrinsic, llvm requires to mangle its type. However, llvm does not support mangling of all possible types. RewriteStatepointsForGC will hit an assertion failure when it tries to create a gc_relocate for pointer to vector of pointers because mangling for vector of pointers is not supported.

This patch changes the way RewriteStatepointsForGC pass creates gc_relocate. For each relocated pointer, we erase the type of pointers and create an unified gc_relocate of type i8 addrspace(1)*. Then a bitcast is inserted to convert the gc_relocate to the correct type. In this way, gc_relocate does not need to deal with different types of pointers and the unsupported type mangling is no longer a problem. This change would also ease further merge when LLVM erases types of pointers and introduces an unified pointer type.

Some minor changes are also introduced to gc_relocate related part in InstCombineCalls, CodeGenPrepare, and Verifier accordingly.

[sanjoy]

Overall I think this is going in the right direction.

Some of the functions you touched don't have the right variable naming conventions (LLVM's coding style says camel case starting with upper case), it would be nice if you follow this change up with a renaming-only change that fixes the functions you touched.

lib/CodeGen/CodeGenPrepare.cpp

627

Why not directly insert ActualRelocatedBase after RelocatedBase by something like:

if (...) {
  IRBuilder::InsertPointGuard IPG(RelocatedBase->getNextNode());
  Builder.CreateBitCast ...

642

Same comment as above about directly inserting the instruction in the right place.

lib/IR/Verifier.cpp

3365

As a separate step, do you think it makes sense to erase the types of the pointers being relocated as well (when they're being fed into the statepoint call)? We'll have to do something like that anyway once LLVM has a unified ptr pointer type.

lib/Transforms/InstCombine/InstCombineCalls.cpp

1213

Maybe have a auto *GCRelocateType = cast<PointerType>(II->getType()); hoisted out?

lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

1080

As a separate change, does it make sense to get rid of gc relocates being type-polymorphic on its return type and just have one gc_relocate intrinsic.

1353

Can we remove the bitcasting logic from CreateGCRelocates and unconditionally bitcast to the type of the alloca here? I think that will simplify the logic.

[• chenli]

lib/CodeGen/CodeGenPrepare.cpp

627

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.

lib/IR/Verifier.cpp

3365

I think we should do it if it follows LLVM's direction.

lib/Transforms/InstCombine/InstCombineCalls.cpp

1213

Will do

lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

1080

That would make sense.

1353

Yes, that's a good idea. In that way only this method need to be touched.

[• chenli]

In D9592#168681, @sanjoy wrote:

Overall I think this is going in the right direction.

Some of the functions you touched don't have the right variable naming conventions (LLVM's coding style says camel case starting with upper case), it would be nice if you follow this change up with a renaming-only change that fixes the functions you touched.

I will do a follow up patch to fix names in these functions.

[sanjoy]

LGTM

lib/CodeGen/CodeGenPrepare.cpp

627

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.

[• chenli]

Update w.r.t Sanjoy's comments.

1. Hoist gc_relocate type in InstCombineCalls.cpp
2. Move bitcast logic into insertRelocationStores() in RewriteStatepointsForGC.cpp

[• chenli]

Fix some merge conflicts.

lib/CodeGen/CodeGenPrepare.cpp

for (auto &ToReplace : Targets) {
 
     SmallVector<Value *, 2> OffsetV;
     if (!getGEPSmallConstantIntOffsetV(Derived, OffsetV))
       continue;
 
     // Create a Builder and replace the target callsite with a gep
     IRBuilder<> Builder(ToReplace);
     Builder.SetCurrentDebugLocation(ToReplace->getDebugLoc());
+
+    // If gc_relocate does not match the actual type, cast it to the right type.
+    // In theory, there must be a bitcast after gc_relocate if the type does not
+    // match, and we should reuse it to get the derived pointer. But it could be
+    // cases like this:
+    // bb1:
+    //  ...
+    //  %g1 = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(...)
+    //  br label %merge
+    //
+    // bb2:
+    //  ...
+    //  %g2 = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(...)
+    //  br label %merge
+    //
+    // merge:
+    //  %p1 = phi i8 addrspace(1)* [ %g1, %bb1 ], [ %g2, %bb2 ]
+    //  %cast = bitcast i8 addrspace(1)* %p1 in to i32 addrspace(1)*
+    //
+    // In this case, we can not find the bitcast any more. So we insert a new bitcast
+    // no matter there is already one or not. In this way, we can handle all cases, and
+    // the extra bitcast should be optimized away in later passes.
+    Instruction *ActualRelocatedBase = RelocatedBase;
+    if (RelocatedBase->getType() != Base->getType()) {
+      ActualRelocatedBase =
+          cast<Instruction>(Builder.CreateBitCast(RelocatedBase, Base->getType()));
+      ActualRelocatedBase->removeFromParent();
+      ActualRelocatedBase->insertAfter(cast<Instruction>(RelocatedBase));
+    }
     Value *Replacement = Builder.CreateGEP(
-        Derived->getSourceElementType(), RelocatedBase, makeArrayRef(OffsetV));
+        Derived->getSourceElementType(), ActualRelocatedBase, makeArrayRef(OffsetV));
     Instruction *ReplacementInst = cast<Instruction>(Replacement);
     ReplacementInst->removeFromParent();
-    ReplacementInst->insertAfter(RelocatedBase);
+    ReplacementInst->insertAfter(ActualRelocatedBase);
     Replacement->takeName(ToReplace);
-    ToReplace->replaceAllUsesWith(Replacement);
+    // If the newly generated derived pointer's type does not match the original derived
+    // pointer's type, cast the new derived pointer to match it. Same reasoning as above.
+    Instruction *ActualReplacement = ReplacementInst;
+    if (ReplacementInst->getType() != ToReplace->getType()) {
+      ActualReplacement =
+          cast<Instruction>(Builder.CreateBitCast(ReplacementInst, ToReplace->getType()));
+      ActualReplacement->removeFromParent();
+      ActualReplacement->insertAfter(ReplacementInst);
+    }
+    ToReplace->replaceAllUsesWith(ActualReplacement);
     ToReplace->eraseFromParent();
 
     MadeChange = true;
   }
   return MadeChange;
 }
 
 // Turns this:

lib/IR/Verifier.cpp

Assert(GCParamArgsStart <= BaseIndex && BaseIndex < GCParamArgsEnd,
            "gc.relocate: statepoint base index doesn't fall within the "
            "'gc parameters' section of the statepoint call",
            &CI);
     Assert(GCParamArgsStart <= DerivedIndex && DerivedIndex < GCParamArgsEnd,
            "gc.relocate: statepoint derived index doesn't fall within the "
            "'gc parameters' section of the statepoint call",
            &CI);
 
-    // Assert that the result type matches the type of the relocated pointer
-    GCRelocateOperands Operands(&CI);
-    Assert(Operands.getDerivedPtr()->getType() == CI.getType(),
-           "gc.relocate: relocating a pointer shouldn't change its type", &CI);
+    // gc_relocate does not need to be the same type as the relocated pointer.
+    // It can casted to the correct type later if it's desired
     break;
   }
   };
 }
 
 /// \brief Carefully grab the subprogram from a local scope.
 ///
 /// This carefully grabs the subprogram from a local scope, avoiding the

lib/Transforms/InstCombine/InstCombineCalls.cpp

case Intrinsic::assume: {
     break;
   }
   case Intrinsic::experimental_gc_relocate: {
     // Translate facts known about a pointer before relocating into
     // facts about the relocate value, while being careful to
     // preserve relocation semantics.
     GCRelocateOperands Operands(II);
     Value *DerivedPtr = Operands.getDerivedPtr();
+    auto *GCRelocateType = cast<PointerType>(II->getType());
 
     // Remove the relocation if unused, note that this check is required
     // to prevent the cases below from looping forever.
     if (II->use_empty())
       return EraseInstFromFunction(*II);
 
     // Undef is undef, even after relocation.
     // TODO: provide a hook for this in GCStrategy.  This is clearly legal for
     // most practical collectors, but there was discussion in the review thread
     // about whether it was legal for all possible collectors.
-    if (isa<UndefValue>(DerivedPtr))
-      return ReplaceInstUsesWith(*II, DerivedPtr);
+    if (isa<UndefValue>(DerivedPtr)) {
+      // gc_relocate is uncasted. Use undef of gc_relocate's type to replace it.
+      return ReplaceInstUsesWith(*II, UndefValue::get(GCRelocateType));
+    }
 
     // The relocation of null will be null for most any collector.
     // TODO: provide a hook for this in GCStrategy.  There might be some weird
     // collector this property does not hold for.
-    if (isa<ConstantPointerNull>(DerivedPtr))
-      return ReplaceInstUsesWith(*II, DerivedPtr);
+    if (isa<ConstantPointerNull>(DerivedPtr)) {
+      // gc_relocate is uncasted. Use null-pointer of gc_relocate's type to replace it.
+      return ReplaceInstUsesWith(*II, ConstantPointerNull::get(GCRelocateType));
+    }
 
     // isKnownNonNull -> nonnull attribute
     if (isKnownNonNull(DerivedPtr))
       II->addAttribute(AttributeSet::ReturnIndex, Attribute::NonNull);
 
     // isDereferenceablePointer -> deref attribute
     if (isDereferenceablePointer(DerivedPtr, DL)) {
       if (Argument *A = dyn_cast<Argument>(DerivedPtr)) {

lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

static void CreateGCRelocates(ArrayRef<llvm::Value *> liveVariables,
   Module *M = statepointToken->getParent()->getParent()->getParent();
 
   for (unsigned i = 0; i < liveVariables.size(); i++) {
     // We generate a (potentially) unique declaration for every pointer type
     // combination.  This results is some blow up the function declarations in
     // the IR, but removes the need for argument bitcasts which shrinks the IR
     // greatly and makes it much more readable.
     SmallVector<Type *, 1> types;                 // one per 'any' type
-    types.push_back(liveVariables[i]->getType()); // result type
+    // All gc_relocate are set to i8 addrspace(1)* type. This could help avoid
+    // cases where the actual value's type mangling is not supported by llvm. A
+    // bitcast is added later to convert gc_relocate to the actual value's type.
+    types.push_back(Type::getInt8PtrTy(M->getContext(), 1));
     Value *gc_relocate_decl = Intrinsic::getDeclaration(
         M, Intrinsic::experimental_gc_relocate, types);
 
     // Generate the gc.relocate call and save the result
     Value *baseIdx =
         ConstantInt::get(Type::getInt32Ty(M->getContext()),
                          liveStart + find_index(liveVariables, basePtrs[i]));
     Value *liveIdx = ConstantInt::get(
for (User *U : gcRelocs) {
 
     GCRelocateOperands relocateOperands(relocatedValue);
     Value *originalValue =
         const_cast<Value *>(relocateOperands.getDerivedPtr());
     assert(allocaMap.count(originalValue));
     Value *alloca = allocaMap[originalValue];
 
     // Emit store into the related alloca
-    StoreInst *store = new StoreInst(relocatedValue, alloca);
-    store->insertAfter(relocatedValue);
+    // All gc_relocate are i8 addrspace(1)* typed, and it must be bitcasted to
+    // the correct type according to alloca.
+    assert(relocatedValue->getNextNode() && "Should always have one since it's not a terminator");
+    IRBuilder<> Builder(relocatedValue->getNextNode());
+    Value *CastedRelocatedValue =
+        Builder.CreateBitCast(relocatedValue, cast<AllocaInst>(alloca)->getAllocatedType(),
+        relocatedValue->hasName() ? relocatedValue->getName() + ".casted" : "");
+
+    StoreInst *store = new StoreInst(CastedRelocatedValue, alloca);
+    store->insertAfter(cast<Instruction>(CastedRelocatedValue));
 
 #ifndef NDEBUG
     visitedLiveValues.insert(originalValue);
 #endif
   }
 }
 
 /// do all the relocation update via allocas and mem2reg

test/Transforms/RewriteStatepointsForGC/base-pointers-11.ll

 ; RUN: opt %s -rewrite-statepoints-for-gc -spp-print-base-pointers  -S 2>&1 | FileCheck %s
 
 ; CHECK: derived %next base %base_obj
 
 declare void @do_safepoint()
 
 define void @test(i64 addrspace(1)* %base_obj) gc "statepoint-example" {
 entry:
   %obj = getelementptr i64, i64 addrspace(1)* %base_obj, i32 1
   br label %loop
 
 loop:                                             ; preds = %loop, %entry
 ; CHECK-LABEL: loop:
 ; CHECK: phi i64 addrspace(1)*
-; CHECK-DAG:  [ %base_obj.relocated, %loop ] 
+; CHECK-DAG:  [ %base_obj.relocated.casted, %loop ]
 ; CHECK-DAG:  [ %base_obj, %entry ]
 ; CHECK:  %current = phi i64 addrspace(1)* 
 ; CHECK-DAG:  [ %obj, %entry ]
-; CHECK-DAG:  [ %next.relocated, %loop ]
+; CHECK-DAG:  [ %next.relocated.casted, %loop ]
   %current = phi i64 addrspace(1)* [ %obj, %entry ], [ %next, %loop ]
   %next = getelementptr i64, i64 addrspace(1)* %current, i32 1
   %safepoint_token = call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @do_safepoint, i32 0, i32 0, i32 5, i32 0, i32 -1, i32 0, i32 0, i32 0)
   br label %loop
 }
 
 declare i32 @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()*, i32, i32, ...)

test/Transforms/RewriteStatepointsForGC/base-pointers.ll

 ; RUN: opt %s -rewrite-statepoints-for-gc -S 2>&1 | FileCheck %s
 
 declare i64 addrspace(1)* @generate_obj()
 declare void @use_obj(i64 addrspace(1)*)
 
 ; The rewriting needs to make %obj loop variant by inserting a phi 
 ; of the original value and it's relocation.
 define void @def_use_safepoint() gc "statepoint-example" {
 ; CHECK-LABEL: def_use_safepoint
 entry:
   %obj = call i64 addrspace(1)* @generate_obj()
   br label %loop
 
 loop:
 ; CHECK: phi i64 addrspace(1)* 
-; CHECK-DAG: [ %obj.relocated, %loop ]
+; CHECK-DAG: [ %obj.relocated.casted, %loop ]
 ; CHECK-DAG: [ %obj, %entry ]
   call void @use_obj(i64 addrspace(1)* %obj)
   %safepoint_token = call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @do_safepoint, i32 0, i32 0, i32 5, i32 0, i32 -1, i32 0, i32 0, i32 0)
   br label %loop
 }
 
 declare void @do_safepoint()
 
entry:
   br label %loop
 
 loop:                                             ; preds = %loop, %entry
 ; CHECK-LABEL: loop
 ; CHECK:   %base_phi = phi i64 addrspace(1)*
 ; CHECK-DAG: [ %base_obj, %entry ]
 ; Given the two selects are equivelent, so are their base phis - ideally,
 ; we'd have commoned these, but that's a missed optimization, not correctness.
-; CHECK-DAG: [ [[DISCARD:%base_select.*.relocated]], %loop ]
+; CHECK-DAG: [ [[DISCARD:%base_select.*.relocated.casted]], %loop ]
 ; CHECK-NOT: base_phi2
 ; CHECK: next = select
 ; CHECK: base_select
 ; CHECK: extra2 = select
 ; CHECK: base_select
 ; CHECK: statepoint
 ;; Both 'next' and 'extra2' are live across the backedge safepoint...
   %current = phi i64 addrspace(1)* [ %obj, %entry ], [ %next, %loop ]

test/Transforms/RewriteStatepointsForGC/gc_relocate_creation.ll

+; RUN: opt %s -rewrite-statepoints-for-gc -S 2>&1 | FileCheck %s
+; This test is to verify gc.relocate can handle pointer to vector of
+; pointers (<2 x i32 addrspace(1)*> addrspace(1)* in this case).
+; The old scheme to create a gc.relocate of <2 x i32 addrspace(1)*> addrspace(1)*
+; type will fail because llvm does not support mangling vector of pointers.
+; The new scheme will create all gc.relocate to i8 addrspace(1)* type and
+; then bitcast to the correct type.
+
+declare void @foo()
+declare void @use(...)
+declare i32 @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()*, i32, i32, ...)
+
+define void @test1(<2 x i32 addrspace(1)*> addrspace(1)* %obj) gc "statepoint-example" {
+entry:
+  %safepoint_token = call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @foo, i32 0, i32 0, i32 0)
+; CHECK: %obj.relocated = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(i32 %safepoint_token, i32 4, i32 4)
+; CHECK-NEXT:  %obj.relocated.casted = bitcast i8 addrspace(1)* %obj.relocated to <2 x i32 addrspace(1)*> addrspace(1)*
+  call void (...) @use(<2 x i32 addrspace(1)*> addrspace(1)* %obj)
+  ret void
+}
 No newline at end of file

test/Transforms/RewriteStatepointsForGC/live-vector.ll

 ; Test that we can correctly handle vectors of pointers in statepoint 
 ; rewriting.  Currently, we scalarize, but that's an implementation detail.
 ; RUN: opt %s -rewrite-statepoints-for-gc -S | FileCheck  %s
 
 ; A non-vector relocation for comparison
 define i64 addrspace(1)* @test(i64 addrspace(1)* %obj) gc "statepoint-example" {
 ; CHECK-LABEL: test
 ; CHECK: gc.statepoint
 ; CHECK-NEXT: gc.relocate
-; CHECK-NEXT: ret i64 addrspace(1)* %obj.relocated
+; CHECK-NEXT: bitcast
+; CHECK-NEXT: ret i64 addrspace(1)* %obj.relocated.casted
 entry:
   %safepoint_token = call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @do_safepoint, i32 0, i32 0, i32 0)
   ret i64 addrspace(1)* %obj
 }
 
 ; A base vector from a argument
 define <2 x i64 addrspace(1)*> @test2(<2 x i64 addrspace(1)*> %obj) gc "statepoint-example" {
 ; CHECK-LABEL: test2
 ; CHECK: extractelement
 ; CHECK-NEXT: extractelement
 ; CHECK-NEXT: gc.statepoint
 ; CHECK-NEXT: gc.relocate
+; CHECK-NEXT: bitcast
 ; CHECK-NEXT: gc.relocate
+; CHECK-NEXT: bitcast
 ; CHECK-NEXT: insertelement
 ; CHECK-NEXT: insertelement
-; CHECK-NEXT: ret <2 x i64 addrspace(1)*> %5
+; CHECK-NEXT: ret <2 x i64 addrspace(1)*> %7
 entry:
   %safepoint_token = call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @do_safepoint, i32 0, i32 0, i32 0)
   ret <2 x i64 addrspace(1)*> %obj
 }
 
 ; A base vector from a load
 define <2 x i64 addrspace(1)*> @test3(<2 x i64 addrspace(1)*>* %ptr) gc "statepoint-example" {
 ; CHECK-LABEL: test3
 ; CHECK: load
 ; CHECK-NEXT: extractelement
 ; CHECK-NEXT: extractelement
 ; CHECK-NEXT: gc.statepoint
 ; CHECK-NEXT: gc.relocate
+; CHECK-NEXT: bitcast
 ; CHECK-NEXT: gc.relocate
+; CHECK-NEXT: bitcast
 ; CHECK-NEXT: insertelement
 ; CHECK-NEXT: insertelement
-; CHECK-NEXT: ret <2 x i64 addrspace(1)*> %5
+; CHECK-NEXT: ret <2 x i64 addrspace(1)*> %7
 entry:
   %obj = load <2 x i64 addrspace(1)*>, <2 x i64 addrspace(1)*>* %ptr
   %safepoint_token = call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @do_safepoint, i32 0, i32 0, i32 0)
   ret <2 x i64 addrspace(1)*> %obj
 }
 
 declare i32 @fake_personality_function()
 
 ; When a statepoint is an invoke rather than a call
 define <2 x i64 addrspace(1)*> @test4(<2 x i64 addrspace(1)*>* %ptr) gc "statepoint-example" {
 ; CHECK-LABEL: test4
 ; CHECK: load
 ; CHECK-NEXT: extractelement
 ; CHECK-NEXT: extractelement
 ; CHECK-NEXT: gc.statepoint
 entry:
   %obj = load <2 x i64 addrspace(1)*>, <2 x i64 addrspace(1)*>* %ptr
   invoke i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @do_safepoint, i32 0, i32 0, i32 0)
           to label %normal_return unwind label %exceptional_return
 
 ; CHECK-LABEL: normal_return:
 ; CHECK: gc.relocate
+; CHECK-NEXT: bitcast
 ; CHECK-NEXT: gc.relocate
+; CHECK-NEXT: bitcast
 ; CHECK-NEXT: insertelement
 ; CHECK-NEXT: insertelement
-; CHECK-NEXT: ret <2 x i64 addrspace(1)*> %6
+; CHECK-NEXT: ret <2 x i64 addrspace(1)*> %8
 normal_return:                                    ; preds = %entry
   ret <2 x i64 addrspace(1)*> %obj
 
 ; CHECK-LABEL: exceptional_return:
 ; CHECK: gc.relocate
+; CHECK-NEXT: bitcast
 ; CHECK-NEXT: gc.relocate
+; CHECK-NEXT: bitcast
 ; CHECK-NEXT: insertelement
 ; CHECK-NEXT: insertelement
-; CHECK-NEXT: ret <2 x i64 addrspace(1)*> %10
+; CHECK-NEXT: ret <2 x i64 addrspace(1)*> %14
 exceptional_return:                               ; preds = %entry
   %landing_pad4 = landingpad { i8*, i32 } personality i32 ()* @fake_personality_function
           cleanup
   ret <2 x i64 addrspace(1)*> %obj
 }
 
 declare void @do_safepoint()
 
 declare i32 @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()*, i32, i32, ...)

test/Transforms/RewriteStatepointsForGC/liveness-basics.ll

 ; A collection of liveness test cases to ensure we're reporting the
 ; correct live values at statepoints
 ; RUN: opt -rewrite-statepoints-for-gc -S < %s | FileCheck %s
 
 
 ; Tests to make sure we consider %obj live in both the taken and untaken 
 ; predeccessor of merge.
 define i64 addrspace(1)* @test1(i1 %cmp, i64 addrspace(1)* %obj) gc "statepoint-example" {
 ; CHECK-LABEL: @test1
 entry:
   br i1 %cmp, label %taken, label %untaken
 
 taken:
 ; CHECK-LABEL: taken:
 ; CHECK-NEXT: gc.statepoint
-; CHECK-NEXT: %obj.relocated = call coldcc i64 addrspace(1)*
+; CHECK-NEXT: %obj.relocated = call coldcc i8 addrspace(1)*
+; CHECK-NEXT: bitcast
 ; CHECK-NEXT: br label %merge
   call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @foo, i32 0, i32 0, i32 0)
   br label %merge
 
 untaken:
 ; CHECK-LABEL: untaken:
 ; CHECK-NEXT: gc.statepoint
-; CHECK-NEXT: %obj.relocated1 = call coldcc i64 addrspace(1)*
+; CHECK-NEXT: %obj.relocated1 = call coldcc i8 addrspace(1)*
+; CHECK-NEXT: bitcast
 ; CHECK-NEXT: br label %merge
   call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @foo, i32 0, i32 0, i32 0)
   br label %merge
 
 merge:
 ; CHECK-LABEL: merge:
-; CHECK-NEXT: %.0 = phi i64 addrspace(1)* [ %obj.relocated, %taken ], [ %obj.relocated1, %untaken ]
+; CHECK-NEXT: %.0 = phi i64 addrspace(1)* [ %obj.relocated.casted, %taken ], [ %obj.relocated1.casted, %untaken ]
 ; CHECK-NEXT: ret i64 addrspace(1)* %.0
   ret i64 addrspace(1)* %obj
 }
 
 ; A local kill should not effect liveness in predecessor block
 define i64 addrspace(1)* @test2(i1 %cmp, i64 addrspace(1)** %loc) gc "statepoint-example" {
 ; CHECK-LABEL: @test2
 entry:
 ; CHECK-LABEL: entry:
 ; CHECK-NEXT:  gc.statepoint
 ; CHECK-NEXT:  br
   call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @foo, i32 0, i32 0, i32 0)
   br i1 %cmp, label %taken, label %untaken
 
 taken:
 ; CHECK-LABEL: taken:
 ; CHECK-NEXT:  %obj = load
 ; CHECK-NEXT:  gc.statepoint
 ; CHECK-NEXT:  gc.relocate
-; CHECK-NEXT:  ret i64 addrspace(1)* %obj.relocated
+; CHECK-NEXT: bitcast
+; CHECK-NEXT:  ret i64 addrspace(1)* %obj.relocated.casted
 
   %obj = load i64 addrspace(1)*, i64 addrspace(1)** %loc
   call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @foo, i32 0, i32 0, i32 0)
   ret i64 addrspace(1)* %obj
 
 untaken:
   ret i64 addrspace(1)* null
 }
 
 ; A local kill should effect values live from a successor phi.  Also, we
 ; should only propagate liveness from a phi to the appropriate predecessors.
 define i64 addrspace(1)* @test3(i1 %cmp, i64 addrspace(1)** %loc) gc "statepoint-example" {
 ; CHECK-LABEL: @test3
 entry:
   br i1 %cmp, label %taken, label %untaken
 
 taken:
 ; CHECK-LABEL: taken:
 ; CHECK-NEXT: gc.statepoint
 ; CHECK-NEXT: %obj = load
 ; CHECK-NEXT: gc.statepoint
-; CHECK-NEXT: %obj.relocated = call coldcc i64 addrspace(1)*
+; CHECK-NEXT: %obj.relocated = call coldcc i8 addrspace(1)*
+; CHECK-NEXT: bitcast
 ; CHECK-NEXT: br label %merge
   call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @foo, i32 0, i32 0, i32 0)
   %obj = load i64 addrspace(1)*, i64 addrspace(1)** %loc
   call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @foo, i32 0, i32 0, i32 0)
   br label %merge
 
 untaken:
 ; CHECK-LABEL: taken:

 ; even if the base pointer is otherwise unused.
 define i64 addrspace(1)* @test4(i1 %cmp, i64 addrspace(1)* %obj) gc "statepoint-example" {
 ; CHECK-LABEL: @test4
 entry:
 ; CHECK-LABEL: entry:
 ; CHECK-NEXT:  %derived = getelementptr
 ; CHECK-NEXT:  gc.statepoint
 ; CHECK-NEXT:  %derived.relocated = 
+; CHECK-NEXT:  bitcast
 ; CHECK-NEXT:  %obj.relocated =
+; CHECK-NEXT:  bitcast
 ; CHECK-NEXT:  gc.statepoint
 ; CHECK-NEXT:  %derived.relocated1 = 
+; CHECK-NEXT:  bitcast
 ; Note: It's legal to relocate obj again, but not strictly needed
 ; CHECK-NEXT:  %obj.relocated2 =
-; CHECK-NEXT:  ret i64 addrspace(1)* %derived.relocated1
+; CHECK-NEXT:  bitcast
+; CHECK-NEXT:  ret i64 addrspace(1)* %derived.relocated1.casted
 ; 
   %derived = getelementptr i64, i64 addrspace(1)* %obj, i64 8
   call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @foo, i32 0, i32 0, i32 0)
 
   call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @foo, i32 0, i32 0, i32 0)
   ret i64 addrspace(1)* %derived
 }
 
 declare void @consume(...) readonly
 
 ; Make sure that a phi def visited during iteration is considered a kill.
 ; Also, liveness after base pointer analysis can change based on new uses,
 ; not just new defs.
 define i64 addrspace(1)* @test5(i1 %cmp, i64 addrspace(1)* %obj) gc "statepoint-example" {
 ; CHECK-LABEL: @test5
 entry:
   br i1 %cmp, label %taken, label %untaken
 
 taken:
 ; CHECK-LABEL: taken:
 ; CHECK-NEXT: gc.statepoint
-; CHECK-NEXT: %obj.relocated = call coldcc i64 addrspace(1)*
+; CHECK-NEXT: %obj.relocated = call coldcc i8 addrspace(1)*
+; CHECK-NEXT: bitcast
 ; CHECK-NEXT: br label %merge
   call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @foo, i32 0, i32 0, i32 0)
   br label %merge
 
 untaken:
 ; CHECK-LABEL: untaken:
 ; CHECK-NEXT: br label %merge
   br label %merge

test/Transforms/RewriteStatepointsForGC/preprocess.ll

 ; RUN: opt -rewrite-statepoints-for-gc -S < %s | FileCheck %s
 
 declare void @consume(...)
 
 ; Test to make sure we destroy LCSSA's single entry phi nodes before
 ; running liveness
 define void @test6(i64 addrspace(1)* %obj) gc "statepoint-example" {
 ; CHECK-LABEL: @test6
 entry:
   br label %next
 
 next:
 ; CHECK-LABEL: next:
 ; CHECK-NEXT: gc.statepoint
 ; CHECK-NEXT: gc.relocate
-; CHECK-NEXT: @consume(i64 addrspace(1)* %obj.relocated)
-; CHECK-NEXT: @consume(i64 addrspace(1)* %obj.relocated)
+; CHECK-NEXT: bitcast
+; CHECK-NEXT: @consume(i64 addrspace(1)* %obj.relocated.casted)
+; CHECK-NEXT: @consume(i64 addrspace(1)* %obj.relocated.casted)
   %obj2 = phi i64 addrspace(1)* [ %obj, %entry ]
   call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @foo, i32 0, i32 0, i32 0)
   call void (...) @consume(i64 addrspace(1)* %obj2)
   call void (...) @consume(i64 addrspace(1)* %obj)
   ret void
 }
 
 declare void @some_call(i64 addrspace(1)*)

test/Transforms/RewriteStatepointsForGC/relocate_invoke_result.ll

 unwind_dest: 
   %lpad = landingpad { i8*, i32 } personality i32* ()* @fake_personality_function
           cleanup
   resume { i8*, i32 } undef
 
 normal_dest:
 ;; CHECK-LABEL: normal_dest:
 ;; CHECK-NEXT: gc.statepoint
-;; CHECK-NEXT: %obj.relocated = call coldcc i64* addrspace(1)*                                      
+;; CHECK-NEXT: %obj.relocated = call coldcc i8 addrspace(1)*
+;; CHECK-NEXT: bitcast
   %safepoint_token = call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @gc_call, i32 0, i32 0, i32 5, i32 0, i32 -1, i32 0, i32 0, i32 0)
   ret i64* addrspace(1)* %obj
 }
 
 declare i32 @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()*, i32, i32, ...)
 
 

test/Transforms/RewriteStatepointsForGC/relocation.ll

 ; RUN: opt %s -rewrite-statepoints-for-gc -S 2>&1 | FileCheck %s
 
 
 declare void @foo()
 declare void @use(...)
 
 define i64 addrspace(1)* @test1(i64 addrspace(1)* %obj, i64 addrspace(1)* %obj2, i1 %condition) gc "statepoint-example" {
 entry:
 ; CHECK-LABEL: @test1
 ; CHECK-DAG: %obj.relocated
 ; CHECK-DAG: %obj2.relocated
   %safepoint_token = call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @foo, i32 0, i32 0, i32 0)
   br label %joint
 
 joint:
 ; CHECK-LABEL: joint:
-; CHECK: %phi1 = phi i64 addrspace(1)* [ %obj.relocated, %entry ], [ %obj3, %joint2 ]
+; CHECK: %phi1 = phi i64 addrspace(1)* [ %obj.relocated.casted, %entry ], [ %obj3, %joint2 ]
   %phi1 = phi i64 addrspace(1)* [ %obj, %entry ], [ %obj3, %joint2 ]
   br i1 %condition, label %use, label %joint2
 
 use:
   br label %joint2
 
 joint2:
 ; CHECK-LABEL: joint2:
-; CHECK: %phi2 = phi i64 addrspace(1)* [ %obj.relocated, %use ], [ %obj2.relocated, %joint ]
-; CHECK: %obj3 = getelementptr i64, i64 addrspace(1)* %obj2.relocated, i32 1
+; CHECK: %phi2 = phi i64 addrspace(1)* [ %obj.relocated.casted, %use ], [ %obj2.relocated.casted, %joint ]
+; CHECK: %obj3 = getelementptr i64, i64 addrspace(1)* %obj2.relocated.casted, i32 1
   %phi2 = phi i64 addrspace(1)* [ %obj, %use ], [ %obj2, %joint ]
   %obj3 = getelementptr i64, i64 addrspace(1)* %obj2, i32 1
   br label %joint
 }
 
 declare i64 addrspace(1)* @generate_obj()
 
 declare void @consume_obj(i64 addrspace(1)*)
 
 declare i1 @rt()
 
 define void @test2() gc "statepoint-example" {
 ; CHECK-LABEL: @test2
 entry:
   %obj_init = call i64 addrspace(1)* @generate_obj()
   %obj = getelementptr i64, i64 addrspace(1)* %obj_init, i32 42
   br label %loop
 
 loop:
 ; CHECK: loop:
-; CHECK-DAG: [ %obj_init.relocated, %loop.backedge ]
+; CHECK-DAG: [ %obj_init.relocated.casted, %loop.backedge ]
 ; CHECK-DAG: [ %obj_init, %entry ]
-; CHECK-DAG: [ %obj.relocated, %loop.backedge ]
+; CHECK-DAG: [ %obj.relocated.casted, %loop.backedge ]
 ; CHECK-DAG: [ %obj, %entry ]
   %index = phi i32 [ 0, %entry ], [ %index.inc, %loop.backedge ]
 ; CHECK-NOT: %location = getelementptr i64, i64 addrspace(1)* %obj, i32 %index
   %location = getelementptr i64, i64 addrspace(1)* %obj, i32 %index
   call void @consume_obj(i64 addrspace(1)* %location)
   %index.inc = add i32 %index, 1
   %condition = call i1 @rt()
   br i1 %condition, label %loop_x, label %loop_y

 ; Make sure a use in a statepoint gets properly relocated at a previous one.  
 ; This is basically just making sure that statepoints aren't accidentally 
 ; treated specially.
 define void @test3(i64 addrspace(1)* %obj) gc "statepoint-example" {
 entry:
 ; CHECK-LABEL: @test3
 ; CHECK: gc.statepoint
 ; CHECK-NEXT: gc.relocate
+; CHECK-NEXT: bitcast
 ; CHECK-NEXT: gc.statepoint
   %safepoint_token = call i32 (void (i64)*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidi64f(void (i64)* undef, i32 1, i32 0, i64 undef, i32 5, i32 0, i32 -1, i32 0, i32 0, i32 0)
   %safepoint_token1 = call i32 (i32 (i64 addrspace(1)*)*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_i32p1i64f(i32 (i64 addrspace(1)*)* undef, i32 1, i32 0, i64 addrspace(1)* %obj, i32 5, i32 0, i32 -1, i32 0, i32 0, i32 0)
   ret void
 }
 
 ; Check specifically for the case where the result of a statepoint needs to 
 ; be relocated itself
branch2:
   br i1 %condition, label %callbb, label %join2
 
 callbb:
   %safepoint_token = call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @foo, i32 0, i32 0, i32 5, i32 0, i32 -1, i32 0, i32 0, i32 0)
   br label %join
 
 join:
 ; CHECK-LABEL: join:
-; CHECK: phi i64 addrspace(1)* [ %obj.relocated, %callbb ], [ %obj, %entry ]
+; CHECK: phi i64 addrspace(1)* [ %obj.relocated.casted, %callbb ], [ %obj, %entry ]
 ; CHECK: phi i64 addrspace(1)* 
 ; CHECK-DAG: [ %obj, %entry ]
-; CHECK-DAG: [ %obj2.relocated, %callbb ]
+; CHECK-DAG: [ %obj2.relocated.casted, %callbb ]
   ; This is a phi outside the dominator region of the new defs inserted by
   ; the safepoint, BUT we can't stop the search here or we miss the second
   ; phi below.
   %phi1 = phi i64 addrspace(1)* [ %obj, %entry ], [ %obj2, %callbb ]
   br label %join2
 
 join2:
 ; CHECK-LABEL: join2: