Khronos Public Bugzilla

Bug 200

Summary: Feedback on OpenCL C++ bindings
Product: OpenCL Reporter: thomas7755
Component: Man Pages, Reference Card & Other DocumentationAssignee: Benedict Gaster <bgaster>
Status: RESOLVED FIXED QA Contact: OpenCL Working Group <opencl>
Severity: normal    
Priority: P3 CC: bmerry, devrel, stapostol
Version: unspecified   
Target Milestone: ---   
Hardware: All   
OS: All   

Description thomas7755 2009-08-24 03:46:19 PDT
When handling multiple devices it would be handy to be able to maintain a vector of kernel functors, where each functor is associated with a command queue:

std::vector<cl::KernelFunctor> kernelFunctors;
for (int d = 0 ; d < commandQueues.size() ; d++)
{
    kernelFunctors.push_back(kernel.bind(commandQueues[d], globalNDRange, localNDRange));
}

Any ideas for how to implement something like this, given that this won't work without being able to copy KernelFunctors?
Comment 1 Jon Leech 2009-08-24 05:07:18 PDT
Reassign to Benedict.
Comment 2 Benedict Gaster 2009-08-24 23:15:43 PDT
I have updated the C++ bindings to provide copy constructor for KernelFunctor and the following program now compiles without error:

#include <CL/cl.hpp>


int main(void)
{
    std::vector<cl::CommandQueue> commandQueues;
    cl::Kernel kernel;

    cl::NDRange globalNDRange;
    cl::NDRange localNDRange;

    std::vector<cl::KernelFunctor> kernelFunctors;
    for (int d = 0 ; d < commandQueues.size() ; d++) {
        kernelFunctors.push_back(
            kernel.bind(
                commandQueues[d], 
                globalNDRange,
                localNDRange));
    }
}

Please let me know if you have any problems using it and thanks for feedback.
Comment 3 thomas7755 2009-08-28 10:50:56 PDT
This doesn't work as hoped unfortunately. The KernelFunctor has a reference to a Kernel and a CommandQueue, and so strange things start happening when you try reseat the reference in the assignment operator.

I can get the original code to execute by changing the copy constructor to:

inline KernelFunctor::KernelFunctor(const KernelFunctor& rhs) :
    kernel_(rhs.kernel_),
    queue_(rhs.queue_),
    offset_(rhs.offset_),
    global_(rhs.global_),
    local_(rhs.local_),
    err_(rhs.err_)
{
    //*this = rhs;
}

(i.e. complete the initializer list and remove the call to the assignment operator)

In this case the assignment operator is never called, but it must still be present as stl::vectors mandate it. Perhaps the assignment operator should fail if rhs != lhs. KernelFunctor could maintain a pointer instead of a reference, but that's possibly not as elegant.
Comment 4 Benedict Gaster 2009-08-28 10:58:57 PDT
Indeed. Let me think about this some more and I will get back with a solution. Might be best to simply remove the reference. I do not what to have pointers.
Comment 5 thomas7755 2009-09-10 11:59:32 PDT
(In reply to comment #4)
> Indeed. Let me think about this some more and I will get back with a solution.
> Might be best to simply remove the reference. I do not what to have pointers.

Have you had any thoughts on this? I agree it would be preferable to avoid pointers. My suggestion would be to provide the corrected copy constructor and make the assignment illegal.
Comment 6 Benedict Gaster 2009-09-10 12:04:13 PDT
I'm working on  fix for this, not using ponters, and will update the C++ bindings with some other fixes in the next few days.
Comment 7 Bruce Merry 2013-10-31 03:08:37 PDT
The kernel functors in cl.hpp were reworked several years ago. Since there has been no activity on this bug I'm going to close it as fixed, but feel free to reopen it if the new kernel functors still have problems.