KhronosGroup / KhronosGroup/OpenCL-Docs

cl_khr_icd 2.0.0 Discussion

Open
#1,003 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
421
Forks
132
Avg merge
5d 13h
Merged PRs (30d)
11

Description

This is a discussion thread, the intent is to gather feedback on the new dispatching strategy proposed at the Montreal F2F, slides are below.

The gist of the idea is:

Fix Dispatching Issues

the current dispatch strategy leaves us open to segfaults, as the dispatch tables are not managed by the loader, and thus the loader has minimal information about them. The idea is to switch to loader managed dispatch as Vulkan does.

Change OpenCL objects layout

From:

struct _cl_platform_id { cl_icd_dispatch *dispatch; };

to:

struct _cl_platform_id {
  cl_icd_dispatch *dispatch;
  void            *dispatch_data;
};

The loader will be able to distinguish objects supporting the new dispatching strategy by using the first entry in the dispatch table, clGetPlatformIDs, that is not currently used by the loader since it uses clIcdGetPlatformIDsKHR to query platforms:

typedef struct _cl_icd_dispatch {
  /* OpenCL 1.0 */
  clGetPlatformIDs_t *clGetPlatformIDs;
  clGetPlatformInfo_t *clGetPlatformInfo;
  clGetDeviceIDs_t *clGetDeviceIDs;
  /* ... */
}
Dispatch through disp_data

The check and dispatch would look like this

  /* Dispatch test on 64 bit architecture */
  if ((uintptr_t)platform->dispatch->clGetPlatformIDs == 0x4F50454E434C3331) { // (ASCII “OPENCL31”)
    /* New dispatch through `disp_data` */
  } else {
    /* Old dispatch through `dispatch` */
  }

The disp_data field would be set by the loader, using the new clIcdSetPlatformDispatchDataKHR extension function, on platforms returned by clIcdGetPlatformIDsKHR, and populated by the new clIcdGetFunctionAddressForPlatformKHR extension function. This new function allows querying regular entry points for a platform.

Vendor Responsible to Propagate disp_data

This disp_data has to be propagated to child object, and this is left to the implementation to do. For instance, when calling:

cl_int clGetDeviceIDs(
  cl_platform_id platform,
  cl_device_type device_type,
  cl_uint num_entries,
  cl_device_id* devices,
  cl_uint* num_devices);

devices returned by the driver in devices should have their disp_data pointer set to the same value as the one platform currently has.

This cannot reliably done by the loader since regular objects can be created by vendor extensions:

cl_int clEnqueueSVMUnmapARM(
  cl_command_queue command_queue,
  void* svm_ptr,
  cl_uint num_events_in_wait_list,
  const cl_event* event_wait_list,
  cl_event* event);

This extension could create a cl_event object in the event, and the loader would have no opportunity to set the disp_data pointer. So the implementation is responsible for copying the disp_data pointer from command_queue inside event.

Fix Library Behavior

The loader is leaking memory as well as vendor library handles.

Destructors

Leverage library destructors to benefit from the system management of library lifetime (like Vulkan).

Library Unloading

Only unload vendor libraries that advertise this capability (through cl_khr_icd2 or another dedicated extension e.g. cl_khr_icd_unloadable).

Layer unloading

Add Necessary new layer destruction API and deprecate old version of the API.

Conclusion

This should address the main concern about the loader. Instance and application managed dispatch can be tackled once we decide on this part of the proposal. Loader managed dispatch is a prerequisite for instances and instance layers.

Please let me know your feedback, and don't hesitate to poke holes in the proposal.

OpenCL ICD Loader - Status and Perspective - 2023-10-27.pdf

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the proposed _cl_platform_id layout, cl_icd_dispatch entries, and the clIcdSetPlatformDispatchDataKHR and clIcdGetFunctionAddressForPlatformKHR entry points in this discussion, then read the attached OpenCL ICD Loader status PDF. Done would require an agreed direction for loader-managed dispatch, library and layer unloading, and the related extension changes.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.