This is an archive of the discontinued LLVM Phabricator instance.

[OpenCL] Allow pointer-to-pointer kernel args beyond CL 1.2
ClosedPublic

Authored by svenvh on Nov 25 2020, 4:25 AM.

Details

Summary

The restriction on pointer-to-pointer kernel arguments has been
relaxed in OpenCL 2.0. Apply the same address space restrictions for
pointer argument types to the inner pointer types.

Diff Detail

Event Timeline

svenvh created this revision.Nov 25 2020, 4:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 25 2020, 4:25 AM
Herald added a subscriber: yaxunl. · View Herald Transcript
svenvh requested review of this revision.Nov 25 2020, 4:25 AM
Anastasia added inline comments.Nov 26 2020, 8:43 AM
clang/lib/Sema/SemaDecl.cpp
8638

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?

8669

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.

svenvh updated this revision to Diff 308075.Nov 27 2020, 9:05 AM

Recursively call getOpenCLKernelParameterType to check address spaces.

svenvh added inline comments.Nov 27 2020, 9:09 AM
clang/lib/Sema/SemaDecl.cpp
8638
8669

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.

Anastasia accepted this revision.Nov 30 2020, 3:19 AM

LGTM! Thanks!

The testing can be improved before committing!

clang/lib/Sema/SemaDecl.cpp
8638

Great! Thanks!

clang/test/SemaOpenCL/invalid-kernel-parameters.cl
13

Btw this was missing in the original testing, could we add a line with __constant and __local? Or perhaps just replace the first two global in this line.

This revision is now accepted and ready to land.Nov 30 2020, 3:19 AM
svenvh added inline comments.Nov 30 2020, 8:24 AM
clang/test/SemaOpenCL/invalid-kernel-parameters.cl
13

Sure, will do!

This revision was automatically updated to reflect the committed changes.