Should LLVM make bitcasts between pointers with different address spaces illegal?
This will require a small clarification in the documentation and an assertion check added to the verifier, but I think this would be a good approach.
The reason being is that in different address spaces, pointers are not always the same size.
This could be limited to make it legal only if the size of the pointer in source and destination address spaces are equivalent, but that seems like more of a work-around than a proper solution.
Ideas?
Micah
Hi,
I don't think we should make bit casts between pointers with different address spaces illegal. Address spaces are not required to be disjoint. In the N1169 spec, it says
A non-null pointer into an address space A can be cast to a pointer into another address space B, but such a cast is undefined if the source pointer does not point to a location in B.
If the address spaces overlap, one should be able to bticast between them.
-- Mon Ping
What about the part about address spaces with pointers of different sizes?
Micah
Mon Ping Wang wrote:
Hi,
I don't think we should make bit casts between pointers with different
address spaces illegal. Address spaces are not required to be disjoint.
In the N1169 spec, it says
A non-null pointer into an address space A can be cast to a pointer into
another address space B, but such a cast is undefined if the source
pointer does not point to a location in B.
Use a ptrtoint + inttoptr to implement the cast.
If the address spaces overlap, one should be able to bticast between them.
I am not okay with having the verifier accept or reject a bitcast based on the contents of the TargetData. Given that we intend to permit different address spaces to have different sized pointers, we'll need to reject all cross address-space bitcasts and casts will need to go through ptrtoint+inttoptr which makes the potential for truncation and extension of the value explicit.
Is that acceptable? In particular, it still permits you to have overlapping address spaces, right?
Nick
Mon Ping Wang wrote:
Hi,
I don't think we should make bit casts between pointers with different
address spaces illegal. Address spaces are not required to be disjoint.
In the N1169 spec, it says
A non-null pointer into an address space A can be cast to a pointer into
another address space B, but such a cast is undefined if the source
pointer does not point to a location in B.
Use a ptrtoint + inttoptr to implement the cast.
I find that strange. One has to pick an intermediate integer type that is safe to convert to and from. It would seem more natural then to define a ptrtoptr operation that will truncate/extended as appropriate. It would make auto upgraded of old LLVM IR files easier to change bitcast of pointer to pointer.
If the address spaces overlap, one should be able to bticast between them.
I am not okay with having the verifier accept or reject a bitcast based on the contents of the TargetData. Given that we intend to permit different address spaces to have different sized pointers, we'll need to reject all cross address-space bitcasts and casts will need to go through ptrtoint+inttoptr which makes the potential for truncation and extension of the value explicit.
I agree that the verifier shouldn't depend on TargetData for this and bitcast is no longer appropriate to use since pointer sizes can't be used.
-- Mon Ping
Mon Ping Wang wrote:
Mon Ping Wang wrote:
Hi,
I don't think we should make bit casts between pointers with different
address spaces illegal. Address spaces are not required to be disjoint.
In the N1169 spec, it says
A non-null pointer into an address space A can be cast to a pointer into
another address space B, but such a cast is undefined if the source
pointer does not point to a location in B.
Use a ptrtoint + inttoptr to implement the cast.
I find that strange. One has to pick an intermediate integer type that is safe to convert to and from. It would seem more natural then to define a ptrtoptr operation that will truncate/extended as appropriate. It would make auto upgraded of old LLVM IR files easier to change bitcast of pointer to pointer.
Yes, that makes sense to me. It's more cumbersome to implement, what with being a new instruction, but I guess that's the price for doing it right.
The name also needs a bit of work; bitcast works for pointer-to-pointer conversions, this is specifically changing the address space. Ideas? '%bar = ptraddrspace i32 addrspace(5)* %foo to i32 addrspace(6)*' and the types must match except for the addrspace?
Nick
Mon Ping Wang wrote:
Mon Ping Wang wrote:
Hi,
I don't think we should make bit casts between pointers with different
address spaces illegal. Address spaces are not required to be disjoint.
In the N1169 spec, it says
A non-null pointer into an address space A can be cast to a pointer into
another address space B, but such a cast is undefined if the source
pointer does not point to a location in B.
Use a ptrtoint + inttoptr to implement the cast.
I find that strange. One has to pick an intermediate integer type that is safe to convert to and from. It would seem more natural then to define a ptrtoptr operation that will truncate/extended as appropriate. It would make auto upgraded of old LLVM IR files easier to change bitcast of pointer to pointer.
Yes, that makes sense to me. It's more cumbersome to implement, what with being a new instruction, but I guess that's the price for doing it right.
The name also needs a bit of work; bitcast works for pointer-to-pointer conversions, this is specifically changing the address space. Ideas? '%bar = ptraddrspace i32 addrspace(5)* %foo to i32 addrspace(6)*' and the types must match except for the addrspace?
I think you have the semantics right here that the types must match expect for the addrspace. How about addrspacecast since it is only changing the address space?
-- Mon Ping
So an instruction like addrspacecast would be the proper way to do this? I know that Eli had pointed to this being preferred.
Another quest would be, could the pointer size changes and the addrspace case be submitted separately or would they need to be part of the same patchset?
Micah