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

Fix CL_INVALID_CONTEXT command-buffer error definitions #1149

Merged
merged 2 commits into from
Jul 16, 2024

Conversation

EwanC
Copy link
Contributor

@EwanC EwanC commented Apr 8, 2024

See issue #1147 documenting that the error specification for CL_INVALID_CONTEXT doesn't take into account the variation when
cl_khr_command_buffer_multi_device is enabled.

Doing this change also picked up that the error wording for clCommandSVMMemcpyKHR and clCommandSVMMemFillKHR referenced the kernel parameter which doesn't exist.

Closes #1147

EwanC added 2 commits May 22, 2024 10:05
See issue KhronosGroup#1147
documenting that the error specification for `CL_INVALID_CONTEXT`
doesn't take into account the variation when
`cl_khr_command_buffer_multi_device` is enabled.

Doing this change also picked up that the error wording for
`clCommandSVMMemcpyKHR` and `clCommandSVMMemFillKHR` referenced
the _kernel_ parameter which doesn't exist.
* Remove extraneous `cl_khr_command_buffer_multi_device` precondition
  from error wording.
* Change "enabled" terminology to "supported" with regards to
  extensions.
@EwanC EwanC added the cl_khr_command_buffer Relating to the command-buffer family of extension label May 22, 2024
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.

Thanks! LGTM.

@bashbaug
Copy link
Contributor

Could of follow-on questions:

Thanks!

@EwanC
Copy link
Contributor Author

EwanC commented May 23, 2024

Does this PR fix Function clCommandBarrierWithWaitListKHR error code CL_INVALID_CONTEXT #1147? If so, note that it's not setup to close the issue when it's merged.

That's the intention, i've added "Closes #1147" to the PR description, which should be one of the magic words for GitHub to do this automatically.

Should we merge this sooner rather than later to unblock tests (Added negative tests for clCommandBarrierWithWaitListKHR OpenCL-CTS#1937) or can this wait another week or so?

I think this PR can wait because it's specifying relatively edge case behaviour, so I don't think waiting a week or so will affect anyone. The negative testing PRs already assume the behaviour in this PR and only stress the wording outside of the multi-device guards, this is because we've yet to do any cl_khr_command_buffer_multi_device testing so introducing it as part of negative testing even without this spec change would be a fair amount of scope increase to the task.

@bashbaug
Copy link
Contributor

Merging as discussed in the July 16th teleconference.

@bashbaug bashbaug merged commit fed48e7 into KhronosGroup:main Jul 16, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cl_khr_command_buffer Relating to the command-buffer family of extension
Development

Successfully merging this pull request may close these issues.

Function clCommandBarrierWithWaitListKHR error code CL_INVALID_CONTEXT
2 participants