Introduce extension macros #251
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
EwanC
added a commit
to EwanC/OpenCL-Docs
that referenced
this pull request
May 8, 2024
This extension adds the revision field to the XML tag for extensions. This allows a version macro to be generated with: * KhronosGroup/OpenCL-Headers#251 * KhronosGroup/OpenCL-Headers#248 Marked as **Draft** as I have a couple of open questions: * Should we be adding this to vendor & ext extension entries in the XML too? Some vendor extensions aren't semantically versioned and are instead a value that's incremented, we could use this as the major value in semantic versioning. * Should this revision entry be used to generate any part of the human readable specification? This would keep the values in sync rather than the specification revision being bumped and then the XML revision being forgotten about.
EwanC
force-pushed
the
extension_version_macro
branch
from
8eaabe8
to
e6597c3
Compare
EwanC
added a commit
to EwanC/OpenCL-Docs
that referenced
this pull request
May 8, 2024
This extension adds the revision field to the XML tag for extensions. This allows a version macro to be generated with: * KhronosGroup/OpenCL-Headers#251 * KhronosGroup/OpenCL-Headers#248 Marked as **Draft** as I have a couple of open questions: * Should we be adding this to vendor & ext extension entries in the XML too? Some vendor extensions aren't semantically versioned and are instead a value that's incremented, we could use this as the major value in semantic versioning. * Should this revision entry be used to generate any part of the human readable specification? This would keep the values in sync rather than the specification revision being bumped and then the XML revision being forgotten about.
EwanC
force-pushed
the
extension_version_macro
branch
from
e6597c3
to
f442d65
Compare
EwanC
added a commit
to KhronosGroup/OpenCL-Docs
that referenced
this pull request
May 15, 2024
This extension adds the revision field to the XML tag for extensions. This allows a version macro to be generated with: * KhronosGroup/OpenCL-Headers#251 * KhronosGroup/OpenCL-Headers#248 KHR extensions are given a revision based on the semantic version of the spec. However other extensions don't use semantic versioning, and so are given a placeholder `0.0.0` value until they can be updated by the owner. The XML schema is also updated to make the revision field mandatory in the XML entry for extensions and the existence of the macro this enables is advertised to users in the spec.
EwanC
added a commit
to EwanC/OpenCL-Docs
that referenced
this pull request
May 15, 2024
This extension adds the revision field to the XML tag for extensions. This allows a version macro to be generated with: * KhronosGroup/OpenCL-Headers#251 * KhronosGroup/OpenCL-Headers#248 KHR extensions are given a revision based on the semantic version of the spec. However other extensions don't use semantic versioning, and so are given a placeholder `0.0.0` value until they can be updated by the owner. The XML schema is also updated to make the revision field mandatory in the XML entry for extensions and the existence of the macro this enables is advertised to users in the spec.
EwanC
force-pushed
the
extension_version_macro
branch
2 times, most recently
from
05de75f
to
b2adfe1
Compare
EwanC
commented
May 15, 2024
| @@ -1776,21 +1903,12 @@ clEnqueueReleaseExternalMemObjectsKHR( | |||
| #define CL_KHR_EXTERNAL_MEMORY_DMA_BUF_EXTENSION_NAME \ | |||
| "cl_khr_external_memory_dma_buf" | |||
|
|
|||
| /* cl_external_memory_handle_type_khr */ | |||
There was a problem hiding this comment.
cl_khr_external_memory_dx has dissapeared after copying over cl_ext.h wholesale from the gen_headers.py output - However it looks like this was intended KhronosGroup/OpenCL-Docs#1160
| @@ -1864,23 +1992,17 @@ clGetSemaphoreHandleForTypeKHR( | |||
|
|
|||
| #endif /* !defined(CL_NO_NON_ICD_DISPATCH_EXTENSION_PROTOTYPES) */ | |||
|
|
|||
| /*************************************************************** | |||
| * cl_khr_external_semaphore_dx_fence | |||
There was a problem hiding this comment.
cl_khr_external_semaphore_dx_fence has dissapeared after copying over cl_ext.h wholesale from the gen_headers.py output - However it looks like this was intended KhronosGroup/OpenCL-Docs#1160
Merged
bashbaug
pushed a commit
to KhronosGroup/OpenCL-Docs
that referenced
this pull request
May 21, 2024
This extension adds the revision field to the XML tag for extensions. This allows a version macro to be generated with: * KhronosGroup/OpenCL-Headers#251 * KhronosGroup/OpenCL-Headers#248 KHR extensions are given a revision based on the semantic version of the spec. However other extensions don't use semantic versioning, and so are given a placeholder `0.0.0` value until they can be updated by the owner. The XML schema is also updated to make the revision field mandatory in the XML entry for extensions and the existence of the macro this enables is advertised to users in the spec.
Implement idea from KhronosGroup#248 to add an version macro to the extension headers so that users can guard application code using the macro to ensure the correct APIs are being used. Extensions can then increment the version when they change the APIs without breaking user code. See [CL_MAKE_VERSION](https://registry.khronos.org/OpenCL/specs/3.0-unified/html/OpenCL_API.html#CL_MAKE_VERSION) for how the macro version is defined.
Clang format changes and other fixes
EwanC
force-pushed
the
extension_version_macro
branch
from
b2adfe1
to
45a04ef
Compare
bashbaug
approved these changes
Jun 11, 2024
There was a problem hiding this comment.
LGTM.
I think cl_ext.h is going to change slightly when/if we remove the automated formatting checks - see #253 - but no need to hold up this PR until the automated formatting checks are merged. It'll just make for a few more diffs next time we re-generate headers.
Un-guarding CL_MAKE_VERSION means that a pre-OpenCL 3.0 application that uses CL_MAKE_VERSION intentionally or unintentially (say, if it meant to use CL_MAKE_VERSION_KHR instead) would build with newer headers and fail to build with older headers, but the risk of that happening is probably low enough that we shouldn't worry too much about it. I don't have a better solution, anyhow.
|
Merging as discussed in the June 18th teleconference. |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implement idea from #248 to add an version macro to the extension headers so that users can guard application code using the macro to ensure the correct APIs are being used. This uses the XML from KhronosGroup/OpenCL-Docs#1161 which forces extension to set a revision value.
Extensions can then increment the version when they change the APIs without breaking user code, providing applications have guarded their usage.
See CL_MAKE_VERSION for how the macro version is defined. Note that the definitions of this macro and
cl_versionhave been taken out of theCL_VERSION_3_0version guard to allow them to always be used by an application. This is because there functionality is implemented in the headers and not affected in any way by the OpenCL implementation being used, and the OpenCL version that implementation supports.