Allow qualifying the OpenCL library calls with a namespace

[fricc33]

Allow qualifying the OpenCL library calls with a namespace (CL_WRAPPER_NS) and to use function pointers to access the dynamic library (CL_USE_FUNCTION_POINTERS)

Dear author of opencl.hpp, I have a proposal for a change that would allow for easier integration with embedded systems such as Android.

On Android the user is not able to directly link against the OpenCL libraries, rather the app will have to discover the dll at run time and bind to the library at initialization using function pointers defined in cl_icd.h.

This approach causes problems when trying to use opencl.hpp, which expects canonical names for the OpenCL calls (e.g.: ::clGetContextInfo) as we will have to define a different name for the function pointers to avoid clashes.

My solution to this problem is to define a separate C++ namespace for the function pointers, and refer to that namespace within opencl.hpp (see attached icd wrapper for reference).

The namespace can be defined through a preprocessor macro (CL_WRAPPER_NS) which can be left undefined by default, leaving the present opencl.hpp behavior unmodified. Wherever OpenCL calls are defined within the opencl.hpp source they are prefixed with the namespace macro (e.g.: CL_WRAPPER_NS::clGetContextInfo).

Finally, in the GetInfoFunctor0 and GetInfoFunctor1 structs, opencl.hpp uses function pointers already, which now become pointer to pointer breaking the code. We need to explicitly dereference the first indirection, for that I added another preprocessor macro (CL_USE_FUNCTION_POINTERS) which needs to be defined by the caller:

template <typename Func, typename Arg0>
struct GetInfoFunctor0
{
    Func f_; const Arg0& arg0_;
    cl_int operator ()(
        cl_uint param, size_type size, void* value, size_type* size_ret)
#if defined(CL_USE_FUNCTION_POINTERS)
    { return (*f_)(arg0_, param, size, value, size_ret); }
#else
    { return f_(arg0_, param, size, value, size_ret); }
#endif
};

This approach has been working very well in my Android applications and I thought it might be of use for others.

Thank you very much for this very useful library, cheers,

• Fabio

gls_icd_wrapper.zip

[CLAassistant]

All committers have signed the CLA.

[bashbaug]

Thank you very much for your contribution! I've left a few comments and suggestions for improvement.

[bashbaug]

Thanks, the latest version addressed all of my comments. I have a slight preference for a more descriptive name for the new macro (currently CL_HPP_WRAPPER_NS), but I can live with it as-is and I'm not sure how much to bike-shed this anyhow. LGTM!

I'd like a couple of others from the working group to add their review and people are starting to disappear for vacation, but we'll aim to get this merged early in January.

Thanks again!

[fricc33]

Hi Ben, thank you very much for the review =) I was thinking about a more descriptive name, maybe CL_HPP_DLL_WRAPPER_NS, or CL_HPP_DLL_PTR_NS? Thanks again, cheers, - Fabio

…

On Fri, Dec 17, 2021 at 9:49 AM Ben Ashbaugh ***@***.***> wrote: ***@***.**** approved this pull request. Thanks, the latest version addressed all of my comments. I have a slight preference for a more descriptive name for the new macro (currently CL_HPP_WRAPPER_NS), but I can live with it as-is and I'm not sure how much to bike-shed this anyhow. LGTM! I'd like a couple of others from the working group to add their review and people are starting to disappear for vacation, but we'll aim to get this merged early in January. Thanks again! — Reply to this email directly, view it on GitHub <#170 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHSPIRWLCKQTZWZK5RXJ6RLUROH3LANCNFSM5JZ2SDBQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you authored the thread.Message ID: ***@***.***>

[bashbaug]

Thank you for your patience, and sorry for taking so long to get back to you.

We discussed this PR in our working group meeting and our recommended name for the new macro is CL_HPP_OPENCL_API_NAMESPACE. You've been very patient so I'm happy to help with the renaming if this would be helpful, likely as a PR to your branch. I see this PR needs a rebase now also.

Let me know how you'd like to proceed. As soon as the renaming is complete we're all set to merge. Thanks again!

[bashbaug]

I've created a PR against your branch with the renaming in case it's helpful: Glass-Imaging#1

I've also checked the rebase and it doesn't look too bad (the conflict is due to the recent static_cast -> const_cast changes).