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

Import OpenCL exported semaphore in Vulkan #1898

Merged
merged 5 commits into from
May 28, 2024

Conversation

saurabhnv
Copy link
Contributor

Except for SYNC_FD, current implementation doesn't import exported OpenCL semaphore in Vulkan and ends up doing signal and wait on essentially two unrelated semaphores (one created in OpenCL and one in Vulkan).
Since OpenCL exports the semaphore, import that in Vulkan to perform signal/wait on the same underlying payload.

throw std::runtime_error("Failed to import semaphore in Vulkan\n");
}
}
#ifdef _WIN32
Copy link
Contributor

Choose a reason for hiding this comment

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

Use a switch or an else-if branch to explicitly match the windows handle enum type. If possible eliminate the ifdef, I made an effort to reduce the reliance of ifdefs to select semaphore handle types, and instead query the the platform for supported extensions. If eliminating the ifdef entirely is not possible, can its scope be reduced? What is the behavior if not WIN32 but also not opaque fd?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, actually I wasn't aware of your efforts made in that direction, will try to remove the ifdef.
As for your second question (I guess you are referring to sync_fd?), it still gets re-imported during every signal/wait operation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in recent commit, couldn't eliminate it completely as it uses vkImportSemaphoreWin32HandleKHR, a windows specific def as per vulkan header.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, looks good. I will test and update if anything fails on my end.

Copy link
Contributor

Choose a reason for hiding this comment

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

This change is fine. Lets merge.

Except for SYNC_FD, current implementation doesn't import exported OpenCL
semaphore in vulkan and ends up doing signal and wait on essentially two
unrelated semaphores (one created in OpenCL and one in Vulkan).

* For linux FD and windows KMT/NT handle types, import the exported
  OpenCL semaphore in Vulkan after its creation.
* Add vkImportSemaphoreWin32HandleKHR in VK_FUNC_LIST
* If importable is asked then create one.
* If exportable is asked then check support for that particular
  handleType, if supported then create one otherwise fallback to
  importable semaphore.
* Replace direct creation of Importable/Exportable semaphore with the macro.
* Use Importable when testing buffer_multiImport_diffCtx
@saurabhnv saurabhnv force-pushed the importable_exportable_semaphore branch 2 times, most recently from 18536cd to 57f90bb Compare April 19, 2024 08:34
@saurabhnv saurabhnv force-pushed the importable_exportable_semaphore branch from 57f90bb to fab800a Compare April 19, 2024 08:45
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 May 28th memory TSG.

@bashbaug bashbaug merged commit 6807e16 into KhronosGroup:main May 28, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants