[PATCH] D92091: [OpenCL] Allow pointer-to-pointer kernel args beyond CL 1.2

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added inline comments.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:8638
     // array, this recursive call only happens once.
     return getOpenCLKernelParameterType(S, QualType(UnderlyingTy, 0));
   }
----------------
Anastasia wrote:
> Btw I am surprised that we recurse to check the underlying type of an array but not the pointer. I guess this spec wording implies that pointed types should be checked too?
> 
> > The   size   in   bytes   of   these   types   areimplementation-defined  and  in  addition  can  also  be  different  for  the  OpenCL  device  and  the host processor making it difficult to allocate buffer objects to be passed as arguments to a kernel declared as pointer to these types.
> 
> 
> Also I can't find anything specific to the arrays...
> 
> Do you think we need to chase this with the spec?
I have raised https://github.com/KhronosGroup/OpenCL-Docs/issues/504


================
Comment at: clang/lib/Sema/SemaDecl.cpp:8669
+    // Loop through inner pointer types to ensure address spaces are valid.
+    QualType NestedPT = PT->getPointeeType();
+    do {
----------------
Anastasia wrote:
> I feel this breaks the original flow a little bit. It would have been nicer if this could be part of `getOpenCLKernelParameterType`, but it might become more complex then... We would probably need to introduce something like `ValidPtrPtrKernelParam` and `InvalidAddrSpacePtrPtrKernelParam`.
> 
Agreed.  I have simplified the patch a bit, by not distinguishing between the outer pointer-to-pointer type and any inner pointer-to-pointer types for the diagnostic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92091