boostorg / compute

A C++ GPU Computing Library for OpenCL
http://boostorg.github.io/compute/
Boost Software License 1.0
1.55k stars 333 forks source link

add support to set kernel argument as NULL; fixes issue #828 #829

Closed rosenrodt closed 5 years ago

rosenrodt commented 5 years ago

Per Khronos OpenCL spec:

If the argument is declared to be a pointer of a built-in scalar or vector type, or a user defined structure type in the global or constant address space, the memory object specified as argument value must be a buffer object (or NULL).

But the following call causes error:

some_kernel.set_arg(0, NULL);

One way as suggested in #828 is to use

some_kernel.set_arg(0, sizeof(cl_mem), NULL);

Now it's just

some_kernel.set_arg(0, nullptr);
rosenrodt commented 5 years ago

@jszuppe Maybe it's better if this supports pre-C++11. Should we implement similar class to local_buffer (say null_buffer) to support setting kernel argument to NULL?

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.006%) to 84.025% when pulling 60f8555780a4f0a5254adacb315b8254a99d0e1e on rosenrodt:set-null-kernel-argument into 36c89134d4013b2e5e45bc55656a18bd6141995a on boostorg:develop.

jszuppe commented 5 years ago

Maybe it's better if this supports pre-C++11. Should we implement similar class to local_buffer (say null_buffer) to support setting kernel argument to NULL?

I would rather have just the C++11 version. It's very unlikely that someone using C++03 will find and use boost::compute::null_buffer before just using set_arg(0, sizeof(cl_mem), NULL); (not mentioning that setting a buffer to NULL is not a popular use case).

rosenrodt commented 5 years ago

We could stick to C++11. The trade-off would be that people using pre-C++11 won't be able to write nice things like kernel.set_args(b0, b1, nullptr).

If that's indeed where we want to go then I'll update the PR.

jszuppe commented 5 years ago

pre-C++11 you don't have variadic templates, you can't write kernel.set_args(b0, b1, nullptr) in C++03.

rosenrodt commented 5 years ago

I'm sold. I'll just stick with C++11 version. Please review