KhronosGroup / KhronosGroup/OpenCL-CLHPP

Command Buffer Extension API Corner Cases

Open
#232 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
425
Forks
133
Avg merge
3h 30m
Merged PRs (30d)
1

Description

Related to #202.

I hit an interesting corner case today trying to update the `opencl.hpp` mirror I have in my SimpleOpenCLSamples repo. As background, I previously enabled very basic query and reference counting support for command buffers and semaphores, though nothing fancy like the full command buffer and semaphore support added recently. Rather than querying extension API function pointers in `opencl.hpp`, I relied on the extension loader to query the extension API function pointers.

Because I didn't have any fancy support for creating command buffers I used the constructor taking a `cl_command_buffer_khr` to construct my `cl::CommandBuffer`:

https://github.com/bashbaug/SimpleOpenCLSamples/blob/a434b4edcede9c6215e509f01eee7f736632993a/samples/12_commandbufferspp/main.cpp#L161

```c++
cl::CommandBuffer cmdbuf{clCreateCommandBufferKHR(
1,
&commandQueue(),
NULL,
NULL)};
```

This gave me the nice reference counting support and the ability to do C++ queries - cool.

When I tried to update to the latest `opencl.hpp` though, I hit a problem:

1. This constructor doesn't initialize the command buffer function pointers ([link](https://github.com/KhronosGroup/OpenCL-CLHPP/blob/0f420fb265d5d3a64d68deceabfd3689f7322e30/include/CL/opencl.hpp#L10742)).
2. Even if we wanted to initialize the command buffer function pointers in this constructor, we have no way of doing so, because we can't get a platform (or anything we could use to query a platform) to use to initialize the command buffer function pointers.

We have all of the right `nullptr` checks so we won't crash even when the function pointers are uninitialized, but the queries and any reference counting won't work as expected.

I don't think there's a great way to fix this, unfortunately. Some options to consider:

1. Do nothing - this is an odd usage and a corner case.
2. Remove this constructor - it's dangerous.
3. Add a parameter to this constructor (could be optional?) that gives us something we could use to query function pointers.
4. Provide an expliict function that applications can use to initialize extension API function pointers to handle cases like this one where the extension function pointers aren't initialized automatically.

Of these options I think I like (4) the best, but they all seem pretty unsatisfying, so maybe there's another better option instead?

PS: I encountered this issue for command buffers but I'm pretty sure semaphores could have the same problem.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with the CommandBuffer constructor in include/CL/opencl.hpp at the linked location, then review related issue #202 and the command-buffer and semaphore extension-function initialization paths. The issue needs an agreed API approach for initializing extension pointers when constructed from a raw handle, with queries and reference counting behaving correctly.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
api
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.