[PATCH] D49723: [OpenCL] Check for invalid kernel arguments in array types

Ayal Zaks via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Feb 12 09:47:42 PST 2023


Ayal added a comment.

In D49723#4116783 <https://reviews.llvm.org/D49723#4116783>, @asavonic wrote:

> In D49723#4116435 <https://reviews.llvm.org/D49723#4116435>, @Ayal wrote:
>
>> This is admittedly a couple of years old by now, but wonder about that other intended patch - Clang still seems to consider pointers in struct arguments to be illegal in CL2.0 (and CL3.0) - please see https://godbolt.org/z/E87z66h1d
>
> Yeah, the patch got lost somewhere, I'm sorry...

Sure, trying to resurrect it in a comment above, and in D143849 <https://reviews.llvm.org/D143849>.

> TBH, I don't know why pointers in structs or arrays were disallowed in the first place. Even OpenCL 1.2 does not say it explicitly, although there is a bit suspicious point in s6.9.p:
>
>   Arguments to kernel functions that are declared to be a struct or union do not allow
>   OpenCL objects to be passed as elements of the struct or union
>
> It does not say "pointers", just "OpenCL objects". Sounds more like events or images to me, not pointers.

Yeah, there's room to improve clarity of the spec. Passing pointers indirectly requires them to point to same/share address space, as provided by SVM starting in OpenCL2.0.



================
Comment at: lib/Sema/SemaDecl.cpp:8218
       // do not allow OpenCL objects to be passed as elements of the struct or
       // union.
       if (ParamType == PtrKernelParam || ParamType == PtrPtrKernelParam ||
----------------
Suggest to (rebase and) update this as follows, based on
https://registry.khronos.org/OpenCL/sdk/2.0/docs/man/xhtml/restrictions.html
```
// union. This restriction was lifted in OpenCL v2.0 with the introduction
// of SVM.
if (S.getLangOpts().getOpenCLCompatibleVersion() > 120 &&
    (ParamType == PtrKernelParam || ParamType == PtrPtrKernelParam))
  continue;
```


Repository:
  rC Clang

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

https://reviews.llvm.org/D49723



More information about the cfe-commits mailing list