KhronosGroup / OpenCL-Headers

Khronos OpenCL-Headers
Apache License 2.0
666 stars 236 forks source link

Update extension headers #235

Closed kpet closed 1 year ago

kpet commented 1 year ago
kpet commented 1 year ago

Thanks, I've approved your CTS PR.

bashbaug commented 1 year ago

We need to merge this PR or #236 (or both) ASAP because header generation is currently broken. Any preference? Are we OK removing CL_COMMAND_BUFFER_STATE_INVALID_KHR?

EwanC commented 1 year ago

This loooks good to me, although removing CL_COMMAND_BUFFER_STATE_INVALID_KHR is a breaking change. Are we sure we're ready to do this now?

My quick GitHub search shows that at least a few other projects appear to be using this enum, such as POCL.

Note also, if we merge https://github.com/KhronosGroup/OpenCL-Headers/pull/236 first we won't need the update to the Mako template (though it won't hurt anything, either).

We need to merge this PR or #236 (or both) ASAP because header generation is currently broken. Any preference? Are we OK removing CL_COMMAND_BUFFER_STATE_INVALID_KHR?

Is there any precedence for the timing of when provisional extensions make breaking changes? Thinking that it might be best to align with when we do a maintenance release, so that if application developers see an error and look at the hosted cl_khr_command_buffer spec then the change to remove the enum is reflected there. That's not a strong preference though if others have opinions, I also don't object to merging now.

bashbaug commented 1 year ago

Is there any precedence for the timing of when provisional extensions make breaking changes?

I've gone back and forth on this one. My current thinking is: we have the tagged headers if folks want to pull headers corresponding to a specific spec version, so we're probably OK making breaking changes?

The main use-case that this breaks is if someone wants to pull in the latest headers to get support for the latest extensions (like this PR, for cl_ext_image_raw10_raw12), but doesn't want to pull in any breaking changes, too.

Actions we should take regardless - I'll volunteer to do this once we decide on a direction:

As an aside, we made another breaking change recently that will affect the headers, so this isn't just a command buffer issue: https://github.com/KhronosGroup/OpenCL-Docs/pull/956

bashbaug commented 1 year ago

I created https://github.com/KhronosGroup/OpenCL-Headers/pull/238 to describe our policy regarding breaking backward compatibility. It also contains a suggestion to use the tagged or released headers if backward compatibility is important.

Assuming this looks good and accurately describes our policy we can merge this PR (and break code that is using CL_COMMAND_BUFFER_STATE_INVALID_KHR).

bashbaug commented 1 year ago

Merging as discussed in the September 12th teleconference.