[PATCH] D151087: [Clang] Permit address space casts with 'reinterpret_cast' in C++

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

In D151087#4362059 <https://reviews.llvm.org/D151087#4362059>, @aaron.ballman wrote:

> Based on all this, I think we should go with `__addrspace_cast` as a named cast and not allow the conversion through `reinterpret_cast` unless going to/from a `[u]intptr_t`.

I think this sounds good. Most of the building blocks for it should already be in place in the form of OpenCL's addrspace_cast. It would remove reinterpret_cast's ability to perform safe conversions, though. Could that affect something else inadvertently? ICS or SCS?

There are other C++ casts that deal with address spaces today. static_cast can also do it when converting from a void pointer, for example. Should it also lose the ability?

In D151087#4362135 <https://reviews.llvm.org/D151087#4362135>, @arsenm wrote:

> In D151087#4361237 <https://reviews.llvm.org/D151087#4361237>, @ebevhan wrote:
>
>> What is now a reinterpret_cast? An address space conversion, or a bitcast? It's not as straightforward as it might seem.
>
> This is the most straightforward part. It's a bitcast.

And because of that, it wouldn't be possible to perform address space conversions between such pointers using reinterpret_cast.

In D151087#4362216 <https://reviews.llvm.org/D151087#4362216>, @jhuber6 wrote:

> I'm not sure if casting away an address space is always legal, it's generally what we do in LLVM-IR.

The TR says this:

  There is no requirement that named address spaces (intrinsic or otherwise) be subsets of the
  generic address space.

but also what Aaron pasted in his earlier comment:

  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.

So, according to the report, converting to the generic address space would always be valid, but might be undefined. I've never understood why the TR did it this way; there's no reason to allow such conversions if we know they are undefined at compile time. Conversions that are undecidable (from superset to subset) are one thing, but the report makes no validity restrictions on converting between disjoint spaces.

Undefined could technically mean "emit an error", but this isn't what Clang does today, and lots of tests and probably code depends on it working like that now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151087/new/

https://reviews.llvm.org/D151087