Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

[C++4OpenCL] Add extra diagnostics for kernel argument types

Authored by olestrohm on Apr 14 2021, 5:13 AM.



Adds extra error diagnostics when using unsupported types in kernel arguments.

This fixes

Diff Detail

Event Timeline

olestrohm created this revision.Apr 14 2021, 5:13 AM
olestrohm requested review of this revision.Apr 14 2021, 5:13 AM
Anastasia added inline comments.Apr 14 2021, 5:46 AM

Let's also test atomic and void pointer type since you are checking it in your patch?

This should be extended to references btw since clang implements them as a pointer type so we can allow that too.

Similarly for by-value parameters - let's test an OpenCL type too.

olestrohm updated this revision to Diff 337450.Apr 14 2021, 7:55 AM

Added more exhaustive tests, as well as fixed the diagnostic to allow reference types.

svenvh added inline comments.Apr 15 2021, 4:00 AM

I am wondering if this should be made conditional on C++ mode? Or is there no possible way that this new return InvalidKernelParam can be triggered from OpenCL C mode?

LGTM from my side. I will leave final approval to Sven.


I think it shouldn't because C types are POD types which is a superset of standard layout types.

olestrohm added inline comments.Apr 15 2021, 9:14 AM

I believe these rules are correct, but just in case they are not, I will make it conditional on C++ mode to not accidentally break OpenCL C.

olestrohm updated this revision to Diff 338032.Apr 16 2021, 2:26 AM
olestrohm set the repository for this revision to rG LLVM Github Monorepo.

Restricted the checks to C++ for OpenCL and added another test for OpenCL vector types.

svenvh accepted this revision.Apr 16 2021, 5:47 AM

LGTM, thanks!

This revision is now accepted and ready to land.Apr 16 2021, 5:47 AM