Anastasia marked an inline comment as done.
Anastasia added inline comments.
================
Comment at: lib/Sema/SemaCast.cpp:2224
+ } else if (IsLValueCast) {
Kind = CK_LValueBitCast;
} else if (DestType->isObjCObjectPointerType()) {
----------------
Anastasia wrote:
> ebevhan wrote:
> > Anastasia wrote:
> > > ebevhan wrote:
> > > > This might not be applicable to this patch, but just something I
> > > > noticed.
> > > >
> > > > So `reinterpret_cast` only operates on pointers when dealing with
> > > > address spaces. What about something like
> > > > ```
> > > > T a;
> > > > T& a_ref = reinterpret_cast<T&>(a);
> > > > ```
> > > > `reinterpret_cast` on an lvalue like this is equivalent to
> > > > `*reinterpret_cast<T*>(&a)`. So for AS code:
> > > > ```
> > > > __private T x;
> > > > __generic T& ref = reinterpret_cast<__generic T&>(x);
> > > > ```
> > > > This should work, since `*reinterpret_cast<__generic T*>(&x)` is valid,
> > > > correct?
> > > >
> > > > What if we have the reference cast case with a different address space
> > > > like this? Doesn't the `IsLValueCast` check need to be first?
> > > >
> > > > What if we have the reference cast case with a different address space
> > > > like this? Doesn't the IsLValueCast check need to be first?
> > >
> > > Ok, let me see if I understand you. I changed `__generic` -> `__global`
> > > since it's invalid and the error is produced as follows:
> > > test.cl:7:21: error: reinterpret_cast from 'int' to '__global int &' is
> > > not allowed
> > > __global int& ref = reinterpret_cast<__global int&>(x);
> > >
> > > Is this not what we are expecting here?
> > My last sentence could have been a bit clearer. Yes, for the
> > `__private`->`__global` case, it should error. But I was thinking more of
> > the case where the AS conversion is valid, like `__private`->`__generic`.
> > Then we will do the AS conversion, but we should have done both an AS
> > conversion and an `LValueBitCast`, because we need to both convert the
> > 'pointer' and also dereference it.
> Hmm... it seems like here we can only have one cast kind... I guess
> `CK_LValueBitCast` leads to the generation of `bitcast` in the IR? But
> `addrspacecast` can perform both bit reinterpretation and address
> translation, so perhaps it makes sense to only have an address space
> conversion in this case? Unless some other logic is needed elsewhere before
> CodeGen... I will try to construct a test case in plain C++.
I am not sure I understood the problem you are describing, may be I need an
example...
But the valid address space conversion example in OpenCL
```
__private T x = ...;
__generic T& ref = reinterpret_cast<__generic T&>(x);
```
produces correctly `addrspacecast`
```
%0 = load i32*, i32** %x.addr, align 4
%1 = addrspacecast i32* %0 to i32 addrspace(4)*
store i32 addrspace(4)* %1, i32 addrspace(4)** %ref
```
However, if we keep checking `CK_LValueBitCast` first clang attempts to
generate `bitcast` but fails because pointers are in different address spaces.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58346/new/
https://reviews.llvm.org/D58346
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
