cl_khr_external_semaphore: Clarify language #938
Conversation
|
|
|
Leaving this open until the discussion around re-importing semaphore handles is closed. |
|
@lakshmih If we still need this PR, we need to resolve conflicts at TOT. |
lakshmih
force-pushed
the
clarify_cl_khr_external_semaphore
branch
from
c97958c
to
8aba57a
Compare
Refined the cl_khr_external_semaphore spec. Removed references to permanence which appear to have been leveraged from the Vulkan spec but don’t apply to the OpenCL spec in its current form.
lakshmih
force-pushed
the
clarify_cl_khr_external_semaphore
branch
from
8aba57a
to
19e0a28
Compare
There was a problem hiding this comment.
This mostly looks good to me, and I really like the updated description, nice work!
If it helps other reviewers, the relevant Vulkan control for temporary vs. permanent import is VK_SEMAPHORE_IMPORT_TEMPORARY_BIT_KHR. Since there is no OpenCL equivalent, I agree we should not make any references to application control of temporary or permanent imports.
Do we still need to introduce the concept of a temporary import though, since SYNC_FD imports are temporary? Or, is this also not applicable to OpenCL, because we only support creating a semaphore from an external handle, and do not support importing an external handle into an already created semaphore?
(Note: clReImportSemaphoreSyncFdKHR is specified to behave as-if the semaphore object was destroyed and re-created with the new handle, so conceptually it is still creating a semaphore.)
| |==== | ||
| | Handle Type | Transference | Permanence | ||
| | Handle Type | Transference |
There was a problem hiding this comment.
Since we're modifying the number of table columns I think we also need to update the table contents for CL_SEMAPHORE_HANDLE_SYNC_FD_KHR below.
|
As mentioned in an earlier comment, OpenCL does not importing an external handle into an already created semaphore. This avoids the need for introducing the concept of temporary handle import. |
| must not affect any other semaphore or payload. | ||
|
|
||
| Please refer to handle specific documentation for more details on transference requirements per | ||
| handle type. |
There was a problem hiding this comment.
This feels like a good step forward. I'm wondering whether we should mention re-import commands in this paragraph. In the end the command was made specific to sync_fd while this paragraph is documents generic bits of behaviour.
|
Merging as discussed in the July 9th teleconference. |
This would have enabled the CI to catch a markup issue introduced by KhronosGroup#938. Signed-off-by: Kevin Petit <[email protected]> Change-Id: I49de3eaf623117f7c29d1019dedf5b342766a029
Introduced by KhronosGroup#938 Signed-off-by: Kevin Petit <[email protected]> Change-Id: Ibef16bceb5398c49a14e88818a45236d0e17acf0
Introduced by #938 Change-Id: Ibef16bceb5398c49a14e88818a45236d0e17acf0 Signed-off-by: Kevin Petit <[email protected]>
This would have enabled the CI to catch a markup issue introduced by KhronosGroup#938. Signed-off-by: Kevin Petit <[email protected]> Change-Id: I49de3eaf623117f7c29d1019dedf5b342766a029
* Fail spec creation if asciidoctor errors are encountered This would have enabled the CI to catch a markup issue introduced by #938. Signed-off-by: Kevin Petit <[email protected]> Change-Id: I49de3eaf623117f7c29d1019dedf5b342766a029 * attempt to fix asciidoctor errors in API spec Change-Id: I0f9cbeddb72e0d76ba508b336d91c4ee640d77ad --------- Signed-off-by: Kevin Petit <[email protected]>
Refined the cl_khr_external_semaphore spec. Removed references to permanence which appear to have been leveraged from the Vulkan spec but don’t apply to the OpenCL spec in its current form.