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

Use array for clUpdateMutableCommandsKHR #245

Merged
merged 1 commit into from
Sep 6, 2024

Conversation

EwanC
Copy link
Contributor

@EwanC EwanC commented Jan 16, 2024

Proposal to pass the update configs to clUpdateMutableCommandsKHR as an array, rather than pointer changed linked list.
Generated from XML changes in KhronosGroup/OpenCL-Docs#1045

See KhronosGroup/OpenCL-Docs#1041 for motivation.

Related PRs:

EwanC added a commit to EwanC/oneapi-construction-kit that referenced this pull request Jan 17, 2024
Implementation change to prototype specification changes proposed
in KhronosGroup/OpenCL-Docs#1041

Uses OpenCL-Docs and OpenCL-Header changes from
* KhronosGroup/OpenCL-Docs#1045
* KhronosGroup/OpenCL-Headers#245
EwanC added a commit to EwanC/OpenCL-CTS that referenced this pull request Jun 19, 2024
CTS change to reflect proposed change from KhronosGroup/OpenCL-Docs#1041
Taking advantage of the extension version macros to avoid
breaking existing implementations immediately.

This reflects PRs:
* KhronosGroup/OpenCL-Docs#1045
* KhronosGroup/OpenCL-Headers#245
EwanC added a commit to EwanC/OpenCL-CTS that referenced this pull request Jun 19, 2024
CTS change to reflect proposed change from KhronosGroup/OpenCL-Docs#1041
Taking advantage of the extension version macros to avoid
breaking existing implementations immediately.

This reflects PRs:
* KhronosGroup/OpenCL-Docs#1045
* KhronosGroup/OpenCL-Headers#245
EwanC added a commit to EwanC/oneapi-construction-kit that referenced this pull request Jun 19, 2024
Implementation change to prototype specification changes proposed
in KhronosGroup/OpenCL-Docs#1041

Uses OpenCL changes from
* KhronosGroup/OpenCL-Docs#1045
* KhronosGroup/OpenCL-Headers#245
* KhronosGroup/OpenCL-CTS#1984
Proposal to pass the update configs to `clUpdateMutableCommandsKHR` as
an array, rather than pointer changed linked list.

See KhronosGroup/OpenCL-Docs#1041 for
motivation and KhronosGroup/OpenCL-Docs#1045
for OpenCL-Docs PR.
@EwanC EwanC marked this pull request as ready for review July 19, 2024 11:49
EwanC added a commit to EwanC/oneapi-construction-kit that referenced this pull request Jul 19, 2024
Implementation change to prototype specification changes proposed
in KhronosGroup/OpenCL-Docs#1041

Uses OpenCL changes from
* KhronosGroup/OpenCL-Docs#1045
* KhronosGroup/OpenCL-Headers#245
* KhronosGroup/OpenCL-CTS#1984
EwanC added a commit to EwanC/oneapi-construction-kit that referenced this pull request Jul 19, 2024
Implementation change to prototype specification changes proposed
in KhronosGroup/OpenCL-Docs#1041

Uses OpenCL changes from
* KhronosGroup/OpenCL-Docs#1045
* KhronosGroup/OpenCL-Headers#245
* KhronosGroup/OpenCL-CTS#1984
EwanC added a commit to EwanC/oneapi-construction-kit that referenced this pull request Jul 19, 2024
Implementation change to prototype specification changes proposed
in KhronosGroup/OpenCL-Docs#1041

Uses OpenCL changes from
* KhronosGroup/OpenCL-Docs#1045
* KhronosGroup/OpenCL-Headers#245
* KhronosGroup/OpenCL-CTS#1984
@EwanC EwanC requested a review from bashbaug July 24, 2024 09:30
Copy link
Contributor

@bashbaug bashbaug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Do we need to stage these changes with changes in any other repos, such as the C++ bindings (OpenCL-CLHPP) or the CTS tests?

I'll take care of updating the command buffer emulation layer.

@EwanC
Copy link
Contributor Author

EwanC commented Jul 31, 2024

LGTM

Do we need to stage these changes with changes in any other repos, such as the C++ bindings (OpenCL-CLHPP) or the CTS tests?

I'll take care of updating the command buffer emulation layer.

Should probably be staged with CTS PR KhronosGroup/OpenCL-CTS#1984, I don't have a OpenCL-CLHPP PR but will look into making one there. I'm not that familiar with the C++ bindings, but I'd expect that would break without an update.

Edit: created a OpenCL-CLHPP PR here will update KhronosGroup/OpenCL-CLHPP#298

EwanC added a commit to EwanC/OpenCL-CLHPP that referenced this pull request Jul 31, 2024
In OpenCL-Docs PR KhronosGroup/OpenCL-Docs#1045
the API for `cl_khr_command_buffer_mutable_dispatch` API
`clUpdateMutableCommandsKHR` changed in a breaking way.

When the headers update OpenCL-Headers PR KhronosGroup/OpenCL-Headers#245
the bindings will break if they are not updated to reflect this
change.

Bindings updated in this PR to pass to arrays of templated length to
the C++ method. The underlying `.data()` pointers of these parameters can be
passed through to the OpenCL API.
EwanC added a commit to EwanC/OpenCL-CLHPP that referenced this pull request Aug 1, 2024
In OpenCL-Docs PR KhronosGroup/OpenCL-Docs#1045
the API for `cl_khr_command_buffer_mutable_dispatch` API
`clUpdateMutableCommandsKHR` changed in a breaking way.

When the headers update OpenCL-Headers PR KhronosGroup/OpenCL-Headers#245
the bindings will break if they are not updated to reflect this
change.

Bindings updated in this PR to pass to arrays of templated length to
the C++ method. The underlying `.data()` pointers of these parameters can be
passed through to the OpenCL API.
EwanC added a commit to EwanC/OpenCL-CTS that referenced this pull request Sep 5, 2024
CTS change to reflect proposed change from KhronosGroup/OpenCL-Docs#1041
Taking advantage of the extension version macros to avoid
breaking existing implementations immediately.

This reflects PRs:
* KhronosGroup/OpenCL-Docs#1045
* KhronosGroup/OpenCL-Headers#245
Copy link
Contributor

@bashbaug bashbaug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging as discussed in the September 3rd teleconference + email.

@bashbaug bashbaug merged commit 7258b9e into KhronosGroup:main Sep 6, 2024
77 checks passed
bashbaug pushed a commit to KhronosGroup/OpenCL-CLHPP that referenced this pull request Sep 6, 2024
In OpenCL-Docs PR KhronosGroup/OpenCL-Docs#1045
the API for `cl_khr_command_buffer_mutable_dispatch` API
`clUpdateMutableCommandsKHR` changed in a breaking way.

When the headers update OpenCL-Headers PR KhronosGroup/OpenCL-Headers#245
the bindings will break if they are not updated to reflect this
change.

Bindings updated in this PR to pass to arrays of templated length to
the C++ method. The underlying `.data()` pointers of these parameters can be
passed through to the OpenCL API.
bashbaug pushed a commit to KhronosGroup/OpenCL-CTS that referenced this pull request Sep 6, 2024
CTS changes to reflect the spec changes merged in
KhronosGroup/OpenCL-Docs#1045 and requires
header updates from
KhronosGroup/OpenCL-Headers#245

Tested using OCK implementation from
codeplaysoftware/oneapi-construction-kit#501
EwanC added a commit to Bensuo/unified-runtime that referenced this pull request Sep 16, 2024
Update OpenCL adapter code to reflect the 2 API breaking changes
to the command-buffer family of extensions that have been
made upstream:

* [Add properties parameter to all command-buffer commands](KhronosGroup/OpenCL-Headers#260)
* [Use array for
  clUpdateMutableCommandsKHR](KhronosGroup/OpenCL-Headers#245)
EwanC added a commit to Bensuo/unified-runtime that referenced this pull request Sep 16, 2024
Update OpenCL adapter code to reflect the 2 API breaking changes
to the command-buffer family of extensions that have been
made upstream:

* [Add properties parameter to all command-buffer commands](KhronosGroup/OpenCL-Headers#260)
* [Use array for
  clUpdateMutableCommandsKHR](KhronosGroup/OpenCL-Headers#245)
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

Successfully merging this pull request may close these issues.

3 participants