[llvm] r256132 - [RS4GC] Add an assert which fails if there is a (yet unsupported) addrspacecast.

[Manuel Jacob via llvm-commits]

CC'ing llvm-commits this time

On 2015-12-21 02:48, Philip Reames wrote:
> It's really not clear to me that stripPointerCasts should be stripping
> addrspacecast at all.  I could see arguments either way, but my bias
> would be that it shouldn't unless there's a good reason.  Too many
> things are likely to rely on the stripped value having the same bit
> width (for instance).

Yes, it's not clear at all.  I actually added a variant of 
stripPointerCasts which doesn't strip addrspacecast.  However, stopping 
the frontend from emitting addrspacecast in the first place turned out 
to be a better idea.  This is why I added the assert instead.  I thought 
it's better than failing with a strange error later.

Note that changing the behaviour of stripPointerCasts() should also 
imply changing the behaviour of CreatePointerCast to maintain symmetry 
between these two operations.  Looking at it from this perspective, I'm 
not sure it still makes sense.  On the other hand, it's already 
unsymmetric because CreatePointerCast() can also create ptrtoint.

BTW, why does findBaseDefiningValue() use stripPointerCasts() in the 
first place?  I think findBaseDefiningValue() is able to handle all 
cases which stripPointerCasts() can handle (except addrspacecast, of 
course).

-Manuel

> 
> Philip
> 
> On 12/20/2015 05:26 PM, Manuel Jacob via llvm-commits wrote:
>> Author: mjacob
>> Date: Sun Dec 20 19:26:46 2015
>> New Revision: 256132
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=256132&view=rev
>> Log:
>> [RS4GC] Add an assert which fails if there is a (yet unsupported) 
>> addrspacecast.
>> 
>> The slightly strange indentation comes from clang-format.
>> 
>> Modified:
>>      llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
>> 
>> Modified: llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp?rev=256132&r1=256131&r2=256132&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp 
>> (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp Sun 
>> Dec 20 19:26:46 2015
>> @@ -454,6 +454,11 @@ static BaseDefiningValueResult findBaseD
>>       if (CastInst *CI = dyn_cast<CastInst>(I)) {
>>       Value *Def = CI->stripPointerCasts();
>> +    // If stripping pointer casts changes the address space there is 
>> an
>> +    // addrspacecast in between.
>> +    assert(cast<PointerType>(Def->getType())->getAddressSpace() ==
>> +               cast<PointerType>(CI->getType())->getAddressSpace() &&
>> +           "unsupported addrspacecast");
>>       // If we find a cast instruction here, it means we've found a 
>> cast which is
>>       // not simply a pointer cast (i.e. an inttoptr).  We don't know 
>> how to
>>       // handle int->ptr conversion.
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits