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

Anastasia Stulova via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 15 03:42:36 PDT 2019


Anastasia added a comment.

In D64083#1583110 <https://reviews.llvm.org/D64083#1583110>, @rjmccall wrote:

> In D64083#1583109 <https://reviews.llvm.org/D64083#1583109>, @rjmccall wrote:
>
> > Okay, so it sounds like *from the language perspective* all block pointers are actually pointers into `__generic`, and the thing with literals is just an implementation detail that's been inappropriately expressed in the AST.  The better implementation strategy is to make sure that (1) the AST uses the size/alignment of a `__generic` pointer for a block pointer and (2) IRGen implicitly converts block literals to `__generic` pointers when it emits them, and then there's no such thing as a block pointer to a qualified type.
>
>
> This is actually important to get right in C++ because of `auto` and templates; you really don't want non-generic block pointer types to become an expressible thing in the language.


Yes, I agree. Right now it fails but the diagnostic is wrong.

In D64083#1583110 <https://reviews.llvm.org/D64083#1583110>, @rjmccall wrote:

> In D64083#1583109 <https://reviews.llvm.org/D64083#1583109>, @rjmccall wrote:
>
> > Okay, so it sounds like *from the language perspective* all block pointers are actually pointers into `__generic`, and the thing with literals is just an implementation detail that's been inappropriately expressed in the AST.  The better implementation strategy is to make sure that (1) the AST uses the size/alignment of a `__generic` pointer for a block pointer and (2) IRGen implicitly converts block literals to `__generic` pointers when it emits them, and then there's no such thing as a block pointer to a qualified type.
>
>
> This is actually important to get right in C++ because of `auto` and templates; you really don't want non-generic block pointer types to become an expressible thing in the language.


I agree. I have opened a bug to follow up on this: https://bugs.llvm.org/show_bug.cgi?id=42621. We can still simplify the code in this patch for now, although I expect in the future it won't be needed.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64083





More information about the llvm-commits mailing list