[PATCH] Canonicalize addrspacecast between different pointer types.

[Matt Arsenault]

On Nov 18, 2013, at 1:56 AM, Michele Scandale <michele.scandale at gmail.com> wrote:

> On 11/18/2013 03:05 AM, Matt Arsenault wrote:
>> 
>> On Nov 17, 2013, at 5:12 AM, Michele Scandale <michele.scandale at gmail.com> wrote:
>> 
>>> 
>>>  I would like to better understand the motivation of this patch: what are the benefit obtained doing a split between the change of address space and the pointer type within the new address space?
>>>  Being a bitcast just a reintepretation of the input value (no change in the bits) this behavior has been included in the addrspacecast instruction (nobody reported any objection on this).
>>> 
>> 
>> The addrspacecast instruction’s purpose is to only change the address space. Changing the type of the pointer is a logically separate operation. If separated out, the bitcast could potentially be combined with other instructions that use the value to simplify things. This is similar to how inttoptr / ptrtoint behave. You can use a different size integer than the pointer size, but instcombine splits it into the 2 logically steps of reinterpreting as an integer, and then extending or truncating to the real pointer size.
>> 
> 
> Originally the /addrspacecast/ was intended to replace every pointer to pointer cast when the two have different address spaces: http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-August/064674.html
> 
> I'm not convinced too much by the potential benefit of the semantic split you propose: I can understand the case of inttoptr and ptrtoint, but with the addrspacecast we are in a different case.
It’s probably not the most super-useful thing, but it makes sense.

> Exposing a change of pointer type doesn't seem to me useful: if I need to reinterpret it to a non pointer type an explicit /ptrtoint/ would be required, otherwise an explicit /bitcast/ would be present, but this can be fused in the current /addrspacecast/ itself.
> Do you have an example that would allow me to understand this?
Basically anything that visitBitCast could then happen after the addrspacecast. A few more transforms need to be added to look through addrspacecast, but this one will help those cooperate with the existing ones for bitcast.

> 
> Probably it would be enough to add the missing optimizations in the InstCombiner.
> 


> BTW, if we want to have a strong separation of the change of address space from the rest, I would prefer a radically different solution: a restriction of the current /addrspacecast/ instruction semantic to allow only pointers or vector of pointer with the same base-type.
> 

This would be more annoying to use. I think accepting any types and then canonicalizing it makes sense.