[PATCH] SROA fix to avoid invalid bitcast generation when converting a load from a double pointer bitcasted with an address space change of the pointee pointee

[Chandler Carruth]

On Fri, Mar 7, 2014 at 10:27 AM, Michele Scandale <
michele.scandale at gmail.com> wrote:

>   I tested it at commit 122a970111b8ec66ae330f2c218ae951dddaf75b, and
> still I have an assertion on "castIsValid": the problem is exactly on
> pointer of pointer where the bitcast changes the address space of the
> pointee pointee.
>
>     define void @union_cvt_float2(<2 x float> addrspace(1)* %p) {
>      %p.addr = alloca <2 x float> addrspace(1)*, align 8
>      store <2 x float> addrspace(1)* %p, <2 x float> addrspace(1)**
> %p.addr, align 8
>      %1 = bitcast <2 x float> addrspace(1)** %p.addr to <2 x float>**
>      %2 = load <2 x float>** %1, align 8
>      call void @ext_call_float2(<2 x float>* %2)
>      ret void
>     }
>
>   In this example SROA wants to replace `%2 = load <2 x float>** %1, align
> 8` with `%2 = bitcast <2 x float> addrspace(1)* %p to <2 x float>*`, but
> this is an invalid instruction!
>   The input IR does the following:
>   -  bitcast the address of the stack location associated to the parameter
> (`<2 x float> addrspace(1)*`) to an address pointing to another type (`<2 x
> float>*`)
>   -  load a `<2 x float>*` from the bitcasted address
>
>   The conservative assumption is that `sizeof(<2 x float> addrspace(1)*)
> != <2 x float>*`: in this case only the bits in the overlapped region are
> the same, thus IMHO the right transformation to preserve the semantic is to
> use ptrtoint+inttoptr using as middle type an integer type that can fully
> contains the original value.
>

The lang ref says for bitcast: "The conversion is done as if the value had
been stored to memory and read back as type ty2." However, bitcast requires
the same number of bits in both arguments.

So, I see two problems here:

1) I don't understand why we don't allow bitcasting between address spaces
when they are the same size. We have the datalayout embedded in the module,
we should be able to verify this, etc etc. Breaking the same-size "as if"
semantics of bitcast seems a pointless limitation.

2) I suspect the isCastValid checks need to be enhanced to detect
differently sized pointer types and not allow a value-cast lowering of
those.

-Chandler
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140307/98daded2/attachment.html>