Skip to content

Conversation

@wenju-he
Copy link
Contributor

@wenju-he wenju-he commented Dec 9, 2024

The test extends sampled_images.cpp test with semaphore import/wait. Only sycl waiting for vulkan semaphore is tested.
Vulkan waiting for sycl semaphore isn't added, because the test's output is buffer and interop between vulkan and sycl buffers isn't supported.

…e.cpp

The test extends sampled_images.cpp test with semaphore import/wait.
Only sycl waiting for vulkan semaphore is tested.
Vulkan waiting for sycl semaphore isn't added, because the test's output
is buffer and interop between vulkan and sycl buffers isn't supported.
@wenju-he wenju-he requested a review from a team as a code owner December 9, 2024 02:20
@@ -0,0 +1,9 @@
// REQUIRES: cuda
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// REQUIRES: cuda
// REQUIRES: aspect-ext_oneapi_bindless_images
// REQUIRES: aspect-ext_oneapi_external_semaphore_import

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done. Changed to cuda || (windows && level_zero && aspect-ext_oneapi_bindless_images)


#ifdef TEST_SEMAPHORE_IMPORT
// Extension: wait for imported semaphore
q.ext_oneapi_wait_external_semaphore(handles.sycl_wait_external_semaphore);

Choose a reason for hiding this comment

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

so to clarify, the sequence is:

  1. submit VK operations and signal the semaphore
  2. SYCL waits on sycl_wait_external_semaphore to ensure VK operations are complete before queuing up any compute operation
  3. q.wait_and_throw() is simply to wait on the sycl Q for compute to complete right? we don't have any signal on an external semaphore that VK will wait on here?

Choose a reason for hiding this comment

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

in the unsampled test case we have a sycl_done_semaphore that is passed into q.submit() but Im not sure who is waiting on that: https://github.com/intel/llvm/blob/sycl/sycl/test-e2e/bindless_images/vulkan_interop/unsampled_images.cpp#L251

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so to clarify, the sequence is:

  1. submit VK operations and signal the semaphore
  2. SYCL waits on sycl_wait_external_semaphore to ensure VK operations are complete before queuing up any compute operation
  3. q.wait_and_throw() is simply to wait on the sycl Q for compute to complete right? we don't have any signal on an external semaphore that VK will wait on here?

yes, correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in the unsampled test case we have a sycl_done_semaphore that is passed into q.submit() but Im not sure who is waiting on that: https://github.com/intel/llvm/blob/sycl/sycl/test-e2e/bindless_images/vulkan_interop/unsampled_images.cpp#L251

in that test vulkan is waiting on the semaphore at

submission.pWaitSemaphores = &syclDoneSemaphore;
, i.e. right before reading vulkan image for verification.

This test doesn't have this workflow because as explained in the commit message this test uses sycl buffer as output and there is no interop support between sycl buffer and vulkan buffer.

@wenju-he
Copy link
Contributor Author

@intel/llvm-gatekeepers please merge, thanks

@martygrant martygrant merged commit dec8c40 into intel:sycl Dec 13, 2024
14 checks passed
@wenju-he wenju-he deleted the vulkan_interop-sampled_images.cpp-semaphore branch December 13, 2024 11:43
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.

4 participants