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: ASSIGNED --- QA Contact: OpenCL Working Group <opencl>
Severity: normal    
Priority: P3 CC: 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.