OpenCL-Headers icon indicating copy to clipboard operation
OpenCL-Headers copied to clipboard

Provide version macros for individual extensions

Open kpet opened this issue 1 year ago • 6 comments

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).

kpet avatar Mar 19 '24 17:03 kpet

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?

EwanC avatar Mar 26 '24 17:03 EwanC

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?

oddhack avatar Mar 27 '24 01:03 oddhack

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.

EwanC avatar Mar 27 '24 09:03 EwanC

@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.

kpet avatar Mar 27 '24 10:03 kpet

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).

oddhack avatar Mar 27 '24 11:03 oddhack

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?

kpet avatar Mar 27 '24 11:03 kpet

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

EwanC avatar Apr 05 '24 10:04 EwanC