Code Generation for Extension Headers

[bashbaug]

This PR adds code generation for extension headers. Instead of writing the extension headers manually, the extension headers are instead generated from the XML grammar in the OpenCL-Docs repo.

I've found and fixed a few bugs in the XML file - see KhronosGroup/OpenCL-Docs#576 - but it's possible a few still remain, or something is busted in the code generation itself. It would be great if each vendor could review their extensions to ensure nothing looks amiss. I'll keep this a draft PR for a bit until everyone has a chance to review.

I'd also be interested in feedback regarding the header layout itself. I'm pretty happy with the way things look, but if we'd like to make changes, it's fairly easy to do so.

Fixes #113, which is the initial discussion regarding generated extension headers.

Fixes #86 by adding the extension the extension preprocessor define unconditionally.

Fixes #111 by adding a symbolic name for all extensions in the XML file. Note however that there still may be some extensions missing in the XML file.

Fixes #224 by adding CL_CALLBACK to clSetMemObjectDestructorAPPLE.

Fixes part of #124 by changing the include guards for the extension headers.

Remaining things to do:

• Merge define CL_NO_EXTENSION_PROTOTYPES OpenCL-CTS#1325
• Merge add the CL_NO_NON_ICD_DISPATCH_EXTENSION_PROTOTYPES compile definition OpenCL-ICD-Loader#213
• clCreateFromGLBuffer parameter errcode_ret (see: Code Generation for Extension Headers #161 (comment))
• cl_ext_image_from_buffer and the #ifdef CL_VERSION_3_0 guard (see: Code Generation for Extension Headers #161 (comment))
• clSetMemObjectDestructorAPPLE (see: XML and header discrepancy for clSetMemObjectDestructorAPPLE #224)
• Reconcile clCreateProgramWithILKHR_fn, clCreateProgramWithILKHR, and CL_API_SUFFIX__VERSION_1_2 (see: Code Generation for Extension Headers #161 (comment))
• Add CL_VERSION_1_2 guard to cl_arm_get_core_id (see: Code Generation for Extension Headers #161 (comment))
• Add CL_API_SUFFIX__VERSION_1_0 to clCreateBufferWithPropertiesINTEL_fn and clCreateBufferWithPropertiesINTEL (see: https://github.com/KhronosGroup/OpenCL-Headers/pull/161#issuecomment-1480235421)\
• Add CL_API_SUFFIX__VERSION_1_0 to clEnqueue*HostPipeINTEL (see: Code Generation for Extension Headers #161 (comment))
• Figure out if anything is needed for large bitfield bits, such as CL_QUEUE_THREAD_LOCAL_EXEC_ENABLE_INTEL (see: Code Generation for Extension Headers #161 (comment))
• Figure out what to do with NO_PROTOTYPES guards (see: Code Generation for Extension Headers #161 (comment)).
• Update the README to document the new NO_PROTOTYPE guards.

[EwanC]

Tried this out on the cl.xml file from an internal docs MR, and it worked nicely at generating the cl_ext.h header. Thanks 😃

A minor nice-to-have would be a requirements.txt file with Mako in it to make Python dependencies more visible.

[bashbaug]

Excellent, that's encouraging.

I added a "requirements.txt" - good suggestion, thanks!

This has been a "draft" for a month and a half now, so it's probably ready for a review soon.

[Kerilk]

I think maintaining coherency through the project is important. We could change the way the loader names it's pointers types from pfn_* to *_fn, and change these in the header as well, the only projects that should be using those definitions are ocl-icd (which has it's own copy of the headers) and the Khronos loader, where we can make the switch at the same time.
If that seems reasonable to you I will prepare two pull requests.

[bashbaug]

I think maintaining coherency through the project is important.

Yeah, I agree, that's one of the key benefits of generating these headers vs. authoring them manually.

It's easy to start generating the layers header again if we decide to switch its naming convention (and switch the ICD loader at the same time). I mostly pushed the change to stop generating it a) so the ICD loader builds wouldn't be broken if we decided to commit these changes as-is, and b) so I wouldn't keep breaking my own builds as I switched from generated headers to the older non-generated headers!

I think an unfortunate reality is that some code in the wild is going to be broken when we align to a common convention, and this layers case is just one example, but hopefully the breakages are minimal.

[Kerilk]

I was trying some clang-format configurations that could be used as an additional pass after generation, and found some settings that might work. I attached the configuration file.
clang-format.txt

[Kerilk]

@bashbaug I see here that you generate function declarations for all extension functions (unless I am mistaken), and not only for those that are exported by the loader (I realize this was already the case for some extension functions already). I assume someone trying to use those symbols would encounter a link error.

Shouldn't only functions that can be linked against have a formal declaration in the headers?

Also, maybe we could simplify the headers + cl_icd.h by defining the function types and then using this type to define symbols and pointers:

typedef cl_int CL_API_CALL clRetainDeviceEXT_t(cl_device_id device) CL_API_SUFFIX__VERSION_1_0;
extern clRetainDeviceEXT_t clRetainDeviceEXT;
typedef clRetainDeviceEXT_t *clRetainDeviceEXT_fn;

And in cl_icd.h:

typedef clRetainDeviceEXT_t *cl_api_clRetainDeviceEXT;

This would avoid the duplication of function parameters declarations. We could do the same for non extensions APIs, as we need pointers to those also for the dispatch table (or any indirect call mechanism we may want to implement really...).

[bashbaug]

Shouldn't only functions that can be linked against have a formal declaration in the headers?

This is a very good question. I think it depends how we expect the extension headers to be used, especially when they are not in the ICD loader dispatch table and hence need to be queried and called indirectly.

I haven't quite finished this in a general sense, but the libusm library I use in my USM samples shows one of the ways that the extension headers can be used: the library provides a definition of the USM extension functions so an application can simply call them like usual. A fancier version could a) automatically query a function pointers on first use vs. requiring an application to call a function to initialize them directly, and/or b) handle multiple versions of the function pointers when multiple platforms support the extension.

If we want to support something similar for the extension functions then I think the headers are fine as-is. I can certainly envision scenarios where we don't want to include the function declarations, though.

FWIW, the Vulkan headers appear to include the function pointer typedef unconditionally, but the function prototypes selectively based on whether VK_NO_PROTOTYPES is defined. This is true for both core API functions and extension APIs. We could do something similar in these headers, if desired.

• Vulkan Core API Example
• Vulkan Extension API Example

[Kerilk]

I haven't quite finished this in a general sense, but the libusm library I use in my USM samples shows one of the ways that the extension headers can be used: the library provides a definition of the USM extension functions so an application can simply call them like usual. A fancier version could a) automatically query a function pointers on first use vs. requiring an application to call a function to initialize them directly, and/or b) handle multiple versions of the function pointers when multiple platforms support the extension

I am very curious about b. Do you have a lightweight strategy in mind to achieve this? I assume you could hash by the dispatch table/it's first entry, but maybe there is a better alternative.

If we want to support something similar for the extension functions then I think the headers are fine as-is. I can certainly envision scenarios where we don't want to include the function declarations, though.

Yes here, again, there are tradeoffs. I was also wondering it providing function typdefs, allowing people to easily make those declarations should they feel the need to, would not help alleviate the issue.

FWIW, the Vulkan headers appear to include the function pointer typedef unconditionally, but the function prototypes selectively based on whether VK_NO_PROTOTYPES is defined. This is true for both core API functions and extension APIs. We could do something similar in these headers, if desired.

* [Vulkan Core API Example](https://github.com/KhronosGroup/Vulkan-Headers/blob/9af411e83fb08cd2bddc3ec771de89416a96cb91/include/vulkan/vulkan_core.h#L3564)

* [Vulkan Extension API Example](https://github.com/KhronosGroup/Vulkan-Headers/blob/9af411e83fb08cd2bddc3ec771de89416a96cb91/include/vulkan/vulkan_core.h#L6056)

That looks very appealing to me. Maybe we could do that and add a level of granularity to allow distinguishing between loader exported APIs and extensions that have no entry in the dispatch table.

[bashbaug]

I am very curious about b. Do you have a lightweight strategy in mind to achieve this? I assume you could hash by the dispatch table/it's first entry, but maybe there is a better alternative.

Sorry for the slow reply.

My initial thought is to walk up the hierarchy using OpenCL queries until I find the platform. This is the same strategy I currently use in the OpenCL intercept layer to find its platform-specific set of function pointers. This isn't exactly cheap or lightweight but it's been acceptable so far. The one case that is a little icky is getting the platform from an OpenCL context, since in the general case this can require a dynamic allocation to query the N devices associated with the context and then the platform from one of the N devices, but we can avoid the dynamic allocation for most common cases:

https://github.com/intel/opencl-intercept-layer/blob/master/intercept/src/intercept.h#L1277

It'd be really nice to be able to query the platform from the context directly!

If this becomes a problem we can look into fancier alternatives. Your idea to hash by the dispatch table is a good one.

[bashbaug]

I have a few updates related to this PR:

1. I added the CL_NO_PROTOTYPES change described above so it is easy to have the function pointer typedefs only but not the function declarations.
2. I have several CTS-related PRs open to fix issues related to generated headers. They should be merged before merging the generated headers:
   • add a prefix to OpenCL extension names OpenCL-CTS#1311
   • do not directly use media sharing extension function names OpenCL-CTS#1313
3. @Kerilk, if you're curious, I have a POC extension loader library that makes use of the function declarations here:
   • https://github.com/bashbaug/opencl-extension-loader

[bashbaug]

We've been using these scripts to generate headers for most new extensions and it seems to be working well. It would be great to merge this PR to improve consistency with older extensions and to avoid needing to grab the generation script out of a PR. Maybe after the next tag?

[Kerilk]

Looks good to me, aside the comment about cl_layer.h. Thanks a lot for that.

remarks:

• We should have the same kind of make targets for generating the loader files
• Maybe when generating cl.h we can add a different define guard for prototypes (CL_NO_PROTOTYPE_CORE)?
• I still think we should switch to function typedef, but this hill can wait.

[Kerilk]

@Kerilk, if you're curious, I have a POC extension loader library that makes use of the function declarations here:

* https://github.com/bashbaug/opencl-extension-loader

I took a good look, and I like it a lot.

[bashbaug]

I rebased to pick up #226.

[bashbaug]

Merging as discussed in the April 11th teleconference.