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

[Philip Reames via llvm-commits]

On 12/27/2015 11:11 AM, Philip Reames wrote:
> I think you need to leave in:
>
> if (!CI->isNoopCast(CI->getModule()->getDataLayout()))
> -      return false;
>
> This is needed to catch things like addrspacecast.
p.s. Please add a test which shows the early bail being exercised and 
would have failed with this line removed. Realized I should probably say 
that explicitly since I was assuming it would be done for the LGTM.
>
> 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
>>
>>
>