OpenCL-CLHPP icon indicating copy to clipboard operation
OpenCL-CLHPP copied to clipboard

Invalid call to clGetKernelWorkGroupInfo?

Open estyrke opened this issue 9 years ago • 1 comments

When calling kernel.getWorkGroupInfo<CL_KERNEL_COMPILE_WORK_GROUP_SIZE>(device, &err), this will lead to https://github.com/KhronosGroup/OpenCL-CLHPP/blob/master/input_cl2.hpp#L1052:

// Specialized GetInfoHelper for clsize_t params
template <typename Func, size_type N>
inline cl_int getInfoHelper(Func f, cl_uint name, array<size_type, N>* param, long)
{
    size_type required;
    cl_int err = f(name, 0, NULL, &required);
    if (err != CL_SUCCESS) {
        return err;
    }

When I run this with the latest Intel OpenCL runtime for Core & Xeon (version 16.1.1) CPUs, the required variable will get 0 written to it, subsequently resulting in no values being written to the return array. Instead the return array is left uninitialized, which is clearly broken.

And actually, according to the spec, this is the correct behavior:

param_value_size_ret Returns the actual size in bytes of data copied to param_value. If param_value_size_ret is NULL, it is ignored.

Note that it says "actual size in bytes of data copied to param_value", not "the actual size in bytes of data being queried by param_value" that is the case for e.g. clGetPlatformInfo.

Since it is known exactly how much space is needed statically, the simple fix seems to be removing the "required" check:

        // Specialized GetInfoHelper for clsize_t params
        template <typename Func, size_type N>
        inline cl_int getInfoHelper(Func f, cl_uint name, array<size_type, N>* param, long)
        {
            size_type required = N * sizeof(size_type);

            cl_int err = f(name, required, param->data(), NULL);
            if (err != CL_SUCCESS) {
                return err;
            }

            return CL_SUCCESS;
        }

If this is not acceptable, then a special case would be needed for clGetKernelWorkGroupInfo, and possibly others.

estyrke avatar Oct 19 '16 13:10 estyrke

I just found https://www.khronos.org/bugzilla/show_bug.cgi?id=1510, which seems to indicate that this is a bug in the documentation. I'm still not sure why the code should spend time querying for the size when it's already known though.

estyrke avatar Oct 19 '16 13:10 estyrke