Anastasia added inline comments.
================ Comment at: cfe/trunk/lib/Sema/SemaExprCXX.cpp:4229 + LangAS AddrSpaceR = + RHSType->getAs<BlockPointerType>()->getPointeeType().getAddressSpace(); + CastKind Kind = ---------------- rjmccall wrote: > 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. We add `__generic` addr space to pointee type of block pointer type for all block variables. However, we don't do the same for block literals. Hence we need to generate conversion from `LangAS::Default` to `LangAS::opencl_generic`... I think this just aligns with what we do for other similar cases in OpenCL. We also add `__global`/`__private` to block pointer type in block variable declarations, but we do the same for all other objects. At IR generation we generate block literals with captures as local variables in `__private` addr space and block literals without captures as global variables in `__global` addr space. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64083/new/ https://reviews.llvm.org/D64083 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
