[PATCH] D56066: [OpenCL] Address space for default class members

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: lib/Sema/SemaInit.cpp:4539
+  if (InitCategory.isPRValue() || InitCategory.isXValue())
+    T1Quals.removeAddressSpace();
+
----------------
ebevhan wrote:
> rjmccall wrote:
> > rjmccall wrote:
> > > I can understand why a pr-value wouldn't have an address space, but an x-value's address space is surely important.
> > No, wait, I think this is more deeply wrong.  We're initializing a reference to type `cv1 T1`, and if we're initializing it with a temporary, we're dropping the address space from `cv1`?  That's definitely wrong.
> > 
> > If we're starting with a pr-value, reference-initialization normally has to start by initializing a temporary.  (The exception is it's a class type with a conversion operator to a reference type; C++ is abysmally complicated.  Let's ignore this for a moment.)  Temporaries are always in the default address space (the address space of local variables) because the language (neither OpenCL nor Embedded C) does not give us any facility for allocating temporary memory anywhere else.  So reference-initialization from a pr-value creates a temporary in the default address space and then attempts to initialize the destination reference type with that temporary.  That initialization should fail if there's no conversion from the default address space to the destination address space.  For example, consider initializing a `const int __global &` from a pr-value: this should clearly not succeed, because we have no way of putting a temporary in the global address space.  That reference can only be initialized with a gl-value that's already in the `__global` address space.
> > 
> > On the other hand, we can successfully initialize a `const int &` with a gl-value of type `const int __global` not by direct reference initialization but by loading from the operand and then materializing the result into a new temporary.
> > 
> > I think what this means is:
> > 
> > - We need to be doing this checking as if pr-values were in `__private`.  This includes both (1) expressions that were originally pr-values and (2) expressions that have been converted to pr-values due to a failure to perform direct reference initialization.
> > 
> > - We need to make sure that reference compatibility correctly prevents references from being bound to gl-values in incompatible address spaces.
> > On the other hand, we can successfully initialize a `const int &` with a gl-value of type `const int __global` not by direct reference initialization but by loading from the operand and then materializing the result into a new temporary.
> 
> Is this the right thing to do? It means that initializing such a reference would incur a cost under the hood that might be unnoticed/unwanted by the user. 
Hmm.  I was going to say that it's consistent with the normal behavior of C++, but looking at the actual rule, it's more complicated than that: C++ will only do this if the types are not reference-related or if the gl-value is a bit-field.  So this would only be allowed if the reference type was e.g. `const long &`.  It's a strange rule, but it's straightforward to stay consistent with it.

I think we will eventually find ourselves needing a special-case rule for copy-initializing and copy-assigning trivially-copyable types, but we're not there yet.


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

https://reviews.llvm.org/D56066