Open Kerilk opened 10 months ago
The first part of the proposal, the loader managed dispatch has been implemented in the OpenCL ICD Loader from Khronos, in ocl-icd, but also in pocl.
Corresponding pull requests can be found here:
The implementations are both forward and backward compatible, as old implementations can be used with new loaders, and new implementations can be used with old loaders as well.
A formal definition of the extension can be found here: https://github.com/KhronosGroup/OpenCL-Docs/pull/1005 An extension enabling layer de-initialization can be found here: https://github.com/KhronosGroup/OpenCL-Docs/pull/962
Thanks a lot for the review!
clIcdGetPlatformIDsKHR
instead so if an implementation was shipping with the clGetPlatformIDs
entry set to null we wouldn't have realized it. The other advantage of using a tag is that we can change it if we decide (hopefully never) to change the strategy down the road. Also, a NULL pointer could be an uninitialized value, this tag is unlikely to appear randomly in memory but is also very unlikely to be a valid function pointer on most architectures since it is not aligned (ends in 0x1).edit: expanded on rationale for tag edit2: expanded rationale on entry point query
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:
to:
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 usesclIcdGetPlatformIDsKHR
to query platforms:Dispatch through
disp_data
The check and dispatch would look like this
The
disp_data
field would be set by the loader, using the newclIcdSetPlatformDispatchDataKHR
extension function, on platforms returned byclIcdGetPlatformIDsKHR
, and populated by the newclIcdGetFunctionAddressForPlatformKHR
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:devices returned by the driver in
devices
should have theirdisp_data
pointer set to the same value as the oneplatform
currently has.This cannot reliably done by the loader since regular objects can be created by vendor extensions:
This extension could create a
cl_event
object in theevent
, and the loader would have no opportunity to set thedisp_data
pointer. So the implementation is responsible for copying thedisp_data
pointer fromcommand_queue
insideevent
.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