[PATCH] D15789: [RS4GC] Fix rematerialization of bitcast of bitcast.

[Philip Reames via llvm-commits]

I think you need to leave in:

if (!CI->isNoopCast(CI->getModule()->getDataLayout()))
-      return false;

This is needed to catch things like addrspacecast.

Other than that, LGTM.



On 12/27/2015 05:58 AM, Manuel Jacob wrote:
> mjacob created this revision.
> mjacob added reviewers: reames, igor-laevsky.
> mjacob added subscribers: llvm-commits, alex.
> Herald added a subscriber: sanjoy.
>
> Previously, only the outer (last) bitcast was rematerialized, resulting in a
> use of the unrelocated inner (first) bitcast after the statepoint.  See the
> test case for an example.
>
> http://reviews.llvm.org/D15789
>
> Files:
>    lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
>    test/Transforms/RewriteStatepointsForGC/rematerialize-derived-pointers.ll
>
> Index: test/Transforms/RewriteStatepointsForGC/rematerialize-derived-pointers.ll
> ===================================================================
> --- test/Transforms/RewriteStatepointsForGC/rematerialize-derived-pointers.ll
> +++ test/Transforms/RewriteStatepointsForGC/rematerialize-derived-pointers.ll
> @@ -47,6 +47,22 @@
>     ret void
>   }
>   
> +define void @"test_bitcast_bitcast"(i32 addrspace(1)* %base) gc "statepoint-example" {
> +; CHECK-LABEL: test_bitcast_bitcast
> +entry:
> +  %ptr1 = bitcast i32 addrspace(1)* %base to i64 addrspace(1)*
> +  %ptr2 = bitcast i64 addrspace(1)* %ptr1 to i16 addrspace(1)*
> +  ; CHECK: bitcast i32 addrspace(1)* %base to i64 addrspace(1)*
> +  %sp = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @do_safepoint, i32 0, i32 0, i32 0, i32 0)
> +  ; CHECK: %base.relocated = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %sp, i32 7, i32 7)
> +  ; CHECK: %base.relocated.casted = bitcast i8 addrspace(1)* %base.relocated to i32 addrspace(1)*
> +  ; CHECK: bitcast i32 addrspace(1)* %base.relocated.casted to i64 addrspace(1)*
> +  ; CHECK: bitcast i64 addrspace(1)* %ptr1.remat to i16 addrspace(1)*
> +  call void @use_obj32(i32 addrspace(1)* %base)
> +  call void @use_obj16(i16 addrspace(1)* %ptr2)
> +  ret void
> +}
> +
>   define void @"test_bitcast_gep"(i32 addrspace(1)* %base) gc "statepoint-example" {
>   ; CHECK-LABEL: test_bitcast_gep
>   entry:
> Index: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
> ===================================================================
> --- lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
> +++ lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
> @@ -2101,16 +2101,9 @@
>     }
>   
>     if (CastInst *CI = dyn_cast<CastInst>(CurrentValue)) {
> -    Value *Def = CI->stripPointerCasts();
> -
> -    // This two checks are basically similar. First one is here for the
> -    // consistency with findBasePointers logic.
> -    assert(!isa<CastInst>(Def) && "not a pointer cast found");
> -    if (!CI->isNoopCast(CI->getModule()->getDataLayout()))
> -      return false;
> -
>       ChainToBase.push_back(CI);
> -    return findRematerializableChainToBasePointer(ChainToBase, Def, BaseValue);
> +    return findRematerializableChainToBasePointer(ChainToBase,
> +                                                  CI->getOperand(0), BaseValue);
>     }
>   
>     // Not supported instruction in the chain
>
>