Add revision to XML extensions #1161
Conversation
EwanC
force-pushed
the
xml_extension_revision
branch
from
2f68c9b
to
9b8e913
Compare
| @@ -6983,7 +6983,7 @@ server's OpenCL/api-docs repository. | |||
| <enum name="CL_DEVICE_INTEGER_DOT_PRODUCT_ACCELERATION_PROPERTIES_4x8BIT_PACKED_KHR"/> | |||
| </require> | |||
| </extension> | |||
| <extension name="cl_khr_semaphore" revision="0.9.1" supported="opencl" depends="CL_VERSION_1_2" ratified="opencl"> | |||
There was a problem hiding this comment.
Are you sure this is intentional? There are a few more extensions that already had a revision attribute, and have been reset to 1.0.0.
There was a problem hiding this comment.
Yes this is intentional - I manually looked through the extensions while I was doing this and changed the revisions in the xml tag to match the spec. Several of the semaphore and external memory family of extension tags needed bumped to 1.0.0. Saying that, because I did this manually I still could have made a mistake so double checking these revisions are correct should be verified in review.
EwanC
force-pushed
the
xml_extension_revision
branch
from
9b8e913
to
eef0acf
Compare
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
xml_extension_revision
branch
from
eef0acf
to
3026686
Compare
| @@ -5622,7 +5622,7 @@ server's OpenCL/api-docs repository. | |||
| <command name="clIcdGetPlatformIDsKHR"/> | |||
| </require> | |||
| </extension> | |||
| <extension name="cl_loader_layers" supported="opencl"> | |||
| <extension name="cl_loader_layers" revision="1.0.0" supported="opencl"> | |||
There was a problem hiding this comment.
I've set this to 1.0.0 but can find the spec for it
There was a problem hiding this comment.
I think the latest spec is here: https://github.com/KhronosGroup/OpenCL-Docs/blob/main/extensions/cl_loader_layers.asciidoc
Looks like version 1.0.0 should be fine.
@Kerilk should we build an HTML version of this spec and post it on the registry? We published an HTML spec for cl_loader_info, but it looks like we haven't published cl_loader_layers.
There was a problem hiding this comment.
I think we can merge this as-is to unblock progress. We can always make addtional tweaks later.
In particular, I think it'd be best for most vendor extensions to go from 0.0.0 to 1.0.0, at least for extensions that are supported by newer drivers that report version 1.0.0 or newer for the supported extension version. This will avoid awkward situations where the reported extension version is "newer" than the version in the headers. I think it'd be fine for each of the vendors to do this for their own extensions, though, or we can do this in one follow-on PR if all vendors are comfortable bumping their extensions to version 1.0.0.
|
Merging as discussed in the May 21st teleconference. |
This PR actions KhronosGroup/OpenCL-Headers#248 to mandate a revision field in the XML tag for extensions. This allows a version macro to be generated with headers PR KhronosGroup/OpenCL-Headers#251
To reduce the scope of this change only KHR extensions are given a revision, based on the semantic version defined in their respective specifications. For EXT and vendor extensions where using semantic versioning is less standardized, a placeholder
0.0.0value is used until they can be updated by the owner as part of #1168The XML schema is updated to make the revision field mandatory in the XML entry for extensions, in the future this means that the rendered extension specifications could generated the revision based on this - as tracked by #1169
The existence of the macro is also advertised as a note in the versioning sectioning of the spec.