Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide version macros for individual extensions #248

Closed
kpet opened this issue Mar 19, 2024 · 8 comments
Closed

Provide version macros for individual extensions #248

kpet opened this issue Mar 19, 2024 · 8 comments
Assignees

Comments

@kpet
Copy link
Contributor

kpet commented Mar 19, 2024

We should provide macros for the version of individual extensions so applications can more easily adapt to changes to definitions for extensions (backwards compatible or not).

@EwanC
Copy link
Contributor

EwanC commented Mar 26, 2024

Want to confirm I understand this proposal because I agree having clear messaging to users on how to deal with API changes is a nice improvement.

The goal is that users will be able to write code like this?

// Application code.
#if CL_FOO_KHR_VERSION == 1
  clFooKHR(bar);
#elif CL_FOO_KHR_VERSION == 2
  clFooKHR(baz);
#endif

And in cl_ext.h we'd define the macro like:

// cl_ext.h
#define CL_FOO_KHR_VERSION N
// APIs for revision N ....
// No APIs for revisions earlier than N

And ideally the version macro would be generated from the revision component in the XML <extension> entry using gen_headers.py.

Is that what you had in mind?

@oddhack
Copy link
Contributor

oddhack commented Mar 27, 2024

It seems like you would need a runtime query for the actual extension version supported by the driver, rather than statically compiling against the version that happens to be in the header file you built the app with?

@EwanC
Copy link
Contributor

EwanC commented Mar 27, 2024

That's a good point, a runtime could return a different signature from clGetExtensionFunctionAddressForPlatform than the signature that the app was built with.

We do have an existing runtime query CL_DEVICE_EXTENSIONS_WITH_VERSION that we could leverage.

@kpet
Copy link
Contributor Author

kpet commented Mar 27, 2024

@EwanC That's exactly what I had in mind.

@oddhack We already have runtime queries but we're missing a mechanism for compile-time checks similar to the *_SPEC_VERSION macros that the Vulkan headers provide. It's particularly problematic for provisional extensions that are less stable and for which breaking changes can occur between released versions of the headers.

@oddhack
Copy link
Contributor

oddhack commented Mar 27, 2024

FWIW, the Vulkan experience led to our current approach where we disallow breaking changes in extension revisions - they are only to indicate fixes to spec language, not behavior. If a shipping extensions is fundamentally broken then we may do a new extension that replaces or modifies it, which is thankfully infrequent.

There are two exceptions I can think of offhand:

  • Occasionally a vendor ships a new vendor extension and almost immediately realizes something was wrong with it. Since the burden is entirely on that vendor, if they want to make a breaking change that's their call.
  • Provisional extensions can change in an arbitrary fashion and as you say, we do have the compile-time macro to rely on, but not the runtime query as you do. The burden is on the ISV to ensure they are using a driver consistent with their header, which has not led to complaints so far (or at least, not that I'm aware of - vendors may be getting negative feedback we don't hear about).

@kpet
Copy link
Contributor Author

kpet commented Mar 27, 2024

One of the cases we discussed was that of an application that does not bundle headers. Different environments could use a different released version of the headers, possibly with incompatible or missing definitions. Having a macro could be helpful in some scenarios. An application could add preprocessor checks and emit warnings/errors if the version is not as expected. They could also potentially adapt to behaviour changes or other compile-time breaking changes. The one thing they could not always do is build a single binary that would work with a range of versions (at least not without providing definitions themselves).

Another case we discussed was that of backwards-compatible additions to extensions that we do allow in CL. v2 of an extension could add a new enum or command whose use could be cleanly guarded by a version macro (it is possible in some cases to use #ifdef on the definitions themselves but not always).

but not the runtime query as you do

Couldn't VkExtensionProperties::specVersion be used for this?

@EwanC EwanC self-assigned this Apr 5, 2024
@EwanC
Copy link
Contributor

EwanC commented Apr 5, 2024

Assigning myself to try draft this up, because I think it effectively blocks KhronosGroup/OpenCL-Docs#1045 if want users to have a good experience of handling that API break.

EwanC added a commit to EwanC/OpenCL-Headers that referenced this issue May 7, 2024
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.
EwanC added a commit to EwanC/OpenCL-Docs that referenced this issue 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 added a commit to EwanC/OpenCL-Headers that referenced this issue May 8, 2024
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.
EwanC added a commit to EwanC/OpenCL-Docs that referenced this issue 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 added a commit to EwanC/OpenCL-Headers that referenced this issue May 8, 2024
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.
EwanC added a commit to KhronosGroup/OpenCL-Docs that referenced this issue 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 issue 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-Headers that referenced this issue May 15, 2024
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.
bashbaug pushed a commit to KhronosGroup/OpenCL-Docs that referenced this issue 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.
EwanC added a commit to EwanC/OpenCL-Headers that referenced this issue May 21, 2024
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.
bashbaug pushed a commit that referenced this issue Jun 18, 2024
* Introduce extension Macros

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.

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.

* Fix CI issues

Clang format changes and other fixes
@EwanC
Copy link
Contributor

EwanC commented Jun 18, 2024

@EwanC EwanC closed this as completed Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants