[PATCH] D64083: [OpenCL][Sema] Improve address space support for blocks

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: cfe/trunk/lib/Sema/SemaExprCXX.cpp:4229
+    LangAS AddrSpaceR =
+        RHSType->getAs<BlockPointerType>()->getPointeeType().getAddressSpace();
+    CastKind Kind =
----------------
Anastasia wrote:
> rjmccall wrote:
> > Anastasia wrote:
> > > rjmccall wrote:
> > > > All of this can be much simpler:
> > > > 
> > > > ```
> > > > LangAS AddrSpaceL = ToType->castAs<BlockPointerType>()->getPointeeType().getAddressSpace();
> > > > LangAS AddrSpaceR = FromType->castAs<BlockPointerType>()->getPointeeType().getAddressSpace();
> > > > ```
> > > > 
> > > > Is there something actually checking the validity of this address-space cast somewhere?
> > > > Is there something actually checking the validity of this address-space cast somewhere?
> > > 
> > > The address spaces for blocks are currently added by clang implicitly. It doesn't seem possible to write kernel code qualifying blocks with address spaces. Although I have to say the error is not given properly because the parser gets confused at least for the examples I have tried. The OpenCL spec doesn't detail much regarding this use case. Potentially this is the area for improvement.
> > > 
> > > So for now we can add an assert to check the cast validity if you think it makes sense and maybe a FIXME in the  code to explain that address spaces aren't working with blocks....
> > > The address spaces for blocks are currently added by clang implicitly. It doesn't seem possible to write kernel code qualifying blocks with address spaces.
> > 
> > There's no way that just fell out from the existing implementation; it was a change someone must have made for OpenCL.  Unless you care about blocks existing in multiple address spaces — which, given that you depend on eliminating them, I'm pretty sure you don't — the compiler just shouldn't do that if it's causing you problems.
> So the reasons why we add addr spaces to blocks is that if they are declared in program scope they will be inferred as `__global` AS and if they are declared in kernel scope they are inferred as `__private` AS.
> 
> When there is a common code i.e. we pass block into a function or invoke the block we use generic AS so that blocks in different addr spaces can be work correctly but we are adding addr space cast.
> 
> This is the review that added this logic for OpenCL C: https://reviews.llvm.org/D28814
> 
> However in C++ we follow slightly different program path and therefore addr space cast isn't performed correctly.
Okay, so users can't write block pointer types with explicit address spaces.  What exactly do you mean by "declaring" a block?  Do you mean that block pointer *types* are inferred to have different qualification based on where they're written, or do you mean that block *literals* have different qualification based on where they're written?  Because I'm not sure the latter is actually distinguishable from a language model in which blocks are always pointers into `__generic` and the compiler just immediately promotes them when emitting the expression.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64083