[SYCL][E2E][Doc] Test external semaphores on regular command lists by crystarm · Pull Request #23399 · intel/llvm

crystarm · 2026-10-07T13:48:50Z

Follow-up to #22811
Fixes #23242
Related to #23249

Summary

This is a test and documentation follow-up to #22811, which made Level Zero external semaphore wait and signal operations non-batchable. That change allows external semaphore operations to work correctly on queues backed by regular command lists when supported by the driver.

With newer drivers, the existing negative tests became invalid. They expected external semaphore operations on regular command lists to throw, but the operations are now accepted and submitted. In the Vulkan test, this resulted in a real wait being submitted for an unsignaled semaphore, followed by wait_and_throw(), which caused the test to hang as reported in #23242.

This PR replaces the obsolete negative tests with bounded positive tests and updates the bindless-images extension documentation to allow external semaphore operations on in-order queues backed by regular command lists.

Changes

Replace the Vulkan and D3D12 external_semaphore_regular_cl_fails.cpp negative tests with positive external_semaphore_regular_cl.cpp tests.
Explicitly create in-order queues with
sycl::ext::intel::property::queue::no_immediate_command_list.
Set UR_L0_BATCH_SIZE=8 to exercise the regular command-list batching path.
Verify that an external semaphore wait submits the preceding command-list batch by placing a USM marker kernel before the wait.
Verify the actual wait semantics through the event returned by
ext_oneapi_wait_external_semaphore:
- the event must remain incomplete before the external semaphore is signaled;
- the event must complete after the Vulkan or D3D12 signal.
Verify external semaphore signaling in the opposite SYCL-to-Vulkan/D3D12 direction.
Use bounded polling and native API timeouts so a regression fails instead of hanging indefinitely.
Exercise Vulkan timeline semaphore interop on both Linux and Windows:
- timeline_fd on Linux;
- timeline_win32_nt_handle on Windows.
Keep separate D3D12 fence coverage on Windows through
win32_nt_dx12_fence.
Use a Vulkan timeline semaphore because binary semaphore sharing remains disabled on Linux by CMPLRLLVM-78008.
Update sycl_ext_oneapi_bindless_images to require only an in-order queue for external semaphore operations, removing the immediate-command-list requirement.
Update comments in existing bindless-images tests that continue to use immediate command lists as part of their existing configuration.

Relationship to #22811

Before #22811, an external semaphore wait or signal appended to a regular command list could remain in an open batch. A wait could deadlock because the external producer could not release a command list that had not been submitted, while an external consumer could fail to observe a signal that remained batched.

#22811 made these operations non-batchable, forcing the regular command list to be submitted. The tests added by this PR provide regression coverage for both directions:

the marker before the external wait checks that the preceding batch is submitted;
the wait-event checks that synchronization is not ignored;
the native Vulkan/D3D12 wait checks that the SYCL external signal is submitted and observed.

Driver requirements

Regular command-list external semaphore support is gated on:

Linux driver 39758;
Windows driver 101.9030.

Windows Gen12 configurations remain marked as expected failures and continue to be tracked by #23249. Therefore, this PR is related to #23249 but does not close it.

crystarm · 2026-10-07T14:13:01Z

Important! The new test uses a timeline semaphore because binary semaphores have a known driver issue on Linux (CMPLRLLVM-78008). Therefore, this PR verifies the general batching fix for regular command lists, but does not cover the original binary/opaque_fd path from #23242.

Copilot

🟡 Changes recommended

The D3D12 test may create its fence on a different physical adapter than the selected SYCL device.

1 open finding

Create D3D12 device on adapter matching selected SYCL device · New

What changed in this PR

Updates bindless-image interop documentation and E2E coverage to support external semaphores on regular Level Zero command lists.

Changes:

Replaces obsolete negative tests with bounded Vulkan and D3D12 synchronization tests.
Exercises regular command-list batching and bidirectional semaphore signaling.
Updates extension documentation and existing test comments.

File	Description
`sycl/test-e2e/bindless_images/vulkan_interop/vulkan_sycl_image_unsampled_timeline_semaphore.cpp`	Updates queue requirement comment.
`sycl/test-e2e/bindless_images/vulkan_interop/vulkan_sycl_image_interop_write_3d_unsampled.cpp`	Updates queue requirement comment.
`sycl/test-e2e/bindless_images/vulkan_interop/vulkan_sycl_image_interop_write_2d_unsampled.cpp`	Updates queue requirement comment.
`sycl/test-e2e/bindless_images/vulkan_interop/vulkan_sycl_image_interop_write_1d_unsampled.cpp`	Updates queue requirement comment.
`sycl/test-e2e/bindless_images/vulkan_interop/vulkan_sycl_image_interop_read_3d.cpp`	Updates queue requirement comment.
`sycl/test-e2e/bindless_images/vulkan_interop/vulkan_sycl_image_interop_read_2d.cpp`	Updates queue requirement comment.
`sycl/test-e2e/bindless_images/vulkan_interop/vulkan_sycl_image_interop_read_1d.cpp`	Updates queue requirement comment.
`sycl/test-e2e/bindless_images/vulkan_interop/vulkan_sycl_buffer.cpp`	Updates queue requirement comment.
`sycl/test-e2e/bindless_images/vulkan_interop/vulkan_sycl_buffer_timeline_semaphore.cpp`	Updates queue requirement comment.
`sycl/test-e2e/bindless_images/vulkan_interop/vulkan_sycl_buffer_binary_semaphore.cpp`	Updates queue requirement comment.
`sycl/test-e2e/bindless_images/vulkan_interop/vulkan_sycl_2d_arithmetic.cpp`	Updates queue requirement comment.
`sycl/test-e2e/bindless_images/vulkan_interop/external_semaphore_regular_cl.cpp`	Adds bounded Vulkan timeline-semaphore coverage.
`sycl/test-e2e/bindless_images/vulkan_interop/external_semaphore_regular_cl_fails.cpp`	Removes obsolete negative test.
`sycl/test-e2e/bindless_images/examples/example_6_import_memory_and_semaphores.cpp`	Removes the immediate-list requirement from the example.
`sycl/test-e2e/bindless_images/dx12_interop/external_semaphore_regular_cl.cpp`	Adds bounded D3D12 fence coverage.
`sycl/test-e2e/bindless_images/dx12_interop/external_semaphore_regular_cl_fails.cpp`	Removes obsolete negative test.
`sycl/test-e2e/bindless_images/dx12_interop/D3D12_win32_named_semaphore.cpp`	Updates queue requirement comment.
`sycl/test-e2e/bindless_images/dx12_interop/D3D12_sycl_interop_3D_write_unsampled.cpp`	Updates queue requirement comment.
`sycl/test-e2e/bindless_images/dx12_interop/D3D12_sycl_interop_3D_read.cpp`	Updates queue requirement comment.
`sycl/test-e2e/bindless_images/dx12_interop/D3D12_sycl_interop_2D_write_unsampled.cpp`	Updates queue requirement comment.
`sycl/test-e2e/bindless_images/dx12_interop/D3D12_sycl_interop_2D_read.cpp`	Updates queue requirement comment.
`sycl/test-e2e/bindless_images/dx12_interop/D3D12_sycl_interop_2D_arithmetic.cpp`	Updates queue requirement comment.
`sycl/test-e2e/bindless_images/dx12_interop/D3D12_sycl_interop_1D_write_unsampled.cpp`	Updates queue requirement comment.
`sycl/test-e2e/bindless_images/dx12_interop/D3D12_sycl_interop_1D_read.cpp`	Updates queue requirement comment.
`sycl/test-e2e/bindless_images/dx12_interop/D3D12_sycl_buffer_timeline_semaphore.cpp`	Updates queue requirement comment.
`sycl/test-e2e/bindless_images/dx11_interop/read_write_unsampled.cpp`	Updates queue requirement comment.
`sycl/doc/extensions/experimental/sycl_ext_oneapi_bindless_images.asciidoc`	Permits regular command lists and records revision 6.13.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+  D3D12Context d3dCtx = createD3D12Context();
+  D3D12ExportableFence extFence = createExportableFence(d3dCtx);


iclsrc

Review summary: This PR drops the spec requirement that queues using external semaphores be built with immediate_command_list. It replaces the old *_regular_cl_fails.cpp tests with positive DX12 and Vulkan tests that run on an in-order queue with no_immediate_command_list, and it updates the comments in the existing interop tests to match. The L0 v1 (image.cpp) and v2 (queue_batched.cpp) adapters already submit external semaphore wait/signal immediately instead of batching them, so the spec change is consistent with the runtime.

The new tests are well designed. UR_L0_BATCH_SIZE=8 together with the marker kernel checks that the wait flushes the earlier batch. Checking that the wait event stays incomplete before the external signal confirms it really blocks. The bounded timeouts with std::_Exit keep a regression from hanging CI. I have no blocking issues. The minor points are below.

iclsrc · 2026-10-08T14:03:21Z

+within SYCL requires the SYCL queue to have been constructed with the
+`sycl::property::queue::in_order` property. The semaphore synchronization
+mechanism is not supported on the default out-of-order queues.


🔵 Suggestion: With this change the spec no longer restricts external semaphore use to immediate command lists. The new tests, however, need REQUIRES-INTEL-DRIVER: lin: 39758, win: 101.9030. On older L0 drivers, a semaphore wait on a regular command list could hang silently, and nothing in the spec or the runtime warns the user. Could you add an implementation note here (or a runtime check or diagnostic in the L0 adapter) saying that regular command list support depends on the backend or driver? That way users on older drivers aren't caught out.

iclsrc · 2026-10-08T14:03:22Z

+  MarkerAtomicRef(*marker).store(0);
+
+  try {
+    constexpr uint64_t D3DSignalValue = 1;


🔵 Suggestion: D3DSignalValue = 1 silently relies on signalExportableFence() incrementing extFence.fenceValue from 0 to 1. If someone later adds another signal before this point, or changes the helper, the wait value and the value actually signaled will no longer match, and the test will fail with a misleading timeout. Consider using extFence.fenceValue + 1 here, or asserting extFence.fenceValue == D3DSignalValue after the signal at line 112, so the dependency is visible in the code.

… lists

crystarm · 2026-10-09T11:44:52Z

@iclsrc Thank you for the review!! ₍ᐢ. .ᐢ₎ ₊˚⊹♡

Addressed:

Per Copilot's suggestion, the D3D12 device is now created on the DXGI adapter matching the selected SYCL device by LUID.
Per @iclsrc's suggestions, documented the backend/driver requirements for external semaphores on regular command lists and derived the fence signal values from the current fence state instead of hardcoding them.

[SYCL][E2E][Doc] Test external semaphores on regular command lists

11ccf69

crystarm marked this pull request as ready for review October 7, 2026 14:04

crystarm requested a review from a team as a code owner October 7, 2026 14:04

crystarm requested a review from cperkinsintel October 7, 2026 14:04

crystarm closed this Oct 7, 2026

crystarm reopened this Oct 7, 2026

crystarm marked this pull request as draft October 7, 2026 14:07

crystarm marked this pull request as ready for review October 7, 2026 14:13

dyniols requested a balanced review from Copilot October 7, 2026 19:40

Copilot started reviewing on behalf of dyniols October 7, 2026 19:41 View session

Copilot AI reviewed Oct 7, 2026

View reviewed changes

Comment thread sycl/test-e2e/bindless_images/dx12_interop/external_semaphore_regular_cl.cpp Outdated

Comment on lines +87 to +88

D3D12Context d3dCtx = createD3D12Context();

D3D12ExportableFence extFence = createExportableFence(d3dCtx);

dyniols added the claude-review label Oct 8, 2026

iclsrc reviewed Oct 8, 2026

View reviewed changes

iclsrc removed the claude-review label Oct 8, 2026

[SYCL][E2E][Doc] Harden external semaphore testing on regular command…

db9eccf

… lists

Navigation Menu

Repository navigation

[SYCL][E2E][Doc] Test external semaphores on regular command lists - #23399

[SYCL][E2E][Doc] Test external semaphores on regular command lists#23399
crystarm wants to merge 2 commits into
intel:syclfrom
crystarm:fix/external-semaphore-regular-cl

crystarm commented Oct 7, 2026

Uh oh!

crystarm commented Oct 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

iclsrc left a comment

Uh oh!

iclsrc Oct 8, 2026

Uh oh!

iclsrc Oct 8, 2026

Uh oh!

crystarm commented Oct 9, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		D3D12Context d3dCtx = createD3D12Context();
		D3D12ExportableFence extFence = createExportableFence(d3dCtx);

Conversation

crystarm commented Oct 7, 2026

Summary

Changes

Relationship to #22811

Driver requirements

Uh oh!

crystarm commented Oct 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

iclsrc left a comment

Choose a reason for hiding this comment

Uh oh!

iclsrc Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

iclsrc Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

crystarm commented Oct 9, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants