[PATCH] D23086: [OpenCL] Generate concrete struct type for ndrange_t

[Anastasia Stulova via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D23086#515468, @yaxunl wrote:

> In https://reviews.llvm.org/D23086#515443, @Anastasia wrote:
>
> > In https://reviews.llvm.org/D23086#514279, @yaxunl wrote:
> >
> > > How about we decide if a type is ndrange_t type based on their canonical types. If the canonical type of type X is the same as the canonical type of ndrange_t type, then type X is treated as ndrange_t type. Is this reasonable?
> >
> >
> > I am not sure I understand entirely what you mean?
> >
> > Following the earlier suggestion from David, I think we can just create a struct type internally and then typedef it to ndrange_t, we can use buildImplicitRecord and addImplicitTypedef methods I believe. The latter one has already been used for other OpenCL types.
> >
> > We will have to switch to string comparisons to identify this type in SemaChecking.cpp and CGBuiltins.cpp for handling the enqueue_kernel call.
>
>
> This was not the approach we agreed upon.
>
> The approach we agreed upon was
>
> In https://reviews.llvm.org/D23086#507215, @majnemer wrote:
>
> > In https://reviews.llvm.org/D23086#507203, @yaxunl wrote:
> >
> > > How about assuming ndrange_t is a struct type defined by user and identify it by struct type name in Clang? This gives user freedom of implementing it differently than SPIR. In opencl-c.h define it as a struct type as SPIR required.
> >
> >
> > That sounds fine to me.
>
>
> The issue of your approach is that vendors lose the freedom to define ndrange_t the way they like.


Surely vendors can re-implement all OpenCL types with an implicit typedef. For example this would just work:

  typedef int queue_t;
  void bar(queue_t q);

I am afraid we will need to provide some implementation to ndrange_t in Clang itself, otherwise I don't see how it could work. Also it would be good to offer standard functionality without any extra includes just like it worked up to now for all other features.


Repository:
  rL LLVM

https://reviews.llvm.org/D23086