Skip to content

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

Merged
uditagarwal97 merged 3 commits into
intel:syclfrom
crystarm:fix/external-semaphore-regular-cl
Oct 19, 2026
Merged

uditagarwal97 merged 3 commits into
intel:syclfrom
crystarm:fix/external-semaphore-regular-cl

Conversation

@crystarm

@crystarm crystarm commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

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
crystarm marked this pull request as ready for review October 7, 2026 14:04
@crystarm
crystarm requested a review from a team as a code owner October 7, 2026 14:04
@crystarm
crystarm requested a review from cperkinsintel October 7, 2026 14:04
@crystarm crystarm closed this Oct 7, 2026
@crystarm crystarm reopened this Oct 7, 2026
@crystarm
crystarm marked this pull request as draft October 7, 2026 14:07
@crystarm

crystarm commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

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.

@crystarm
crystarm marked this pull request as ready for review October 7, 2026 14:13
@dyniols
dyniols requested a balanced review from Copilot October 7, 2026 19:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

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

1 open finding
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.

Comment on lines +87 to +88
D3D12Context d3dCtx = createD3D12Context();
D3D12ExportableFence extFence = createExportableFence(d3dCtx);

@iclsrc iclsrc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

Comment on lines +2336 to +2338
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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔵 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.

MarkerAtomicRef(*marker).store(0);

try {
constexpr uint64_t D3DSignalValue = 1;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔵 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.

@crystarm

crystarm commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

@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.

@crystarm
crystarm requested a review from a team as a code owner October 12, 2026 13:43
@crystarm

Copy link
Copy Markdown
Contributor Author

I moved external semaphore operations to a separate regular command list in 8782f82. This should fix the Linux Arc CI failure.

@ldorau ldorau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

UR part LGTM

@sys-ce-bb

Copy link
Copy Markdown
Contributor

@intel/llvm-gatekeepers please consider merging

@uditagarwal97
uditagarwal97 merged commit a6c4ca9 into intel:sycl Oct 19, 2026
59 of 62 checks passed
@sarnex

sarnex commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Hi @crystarm, we are seeing this fail in postcommit on Linux DG2, it seems to be sporadic, can you please take a look?
https://github.com/intel/llvm/actions/runs/39276053486/job/118151812031

FAIL: SYCL :: bindless_images/vulkan_interop/external_semaphore_regular_cl.cpp (2607 of 2621)
    ******************** TEST 'SYCL :: bindless_images/vulkan_interop/external_semaphore_regular_cl.cpp' FAILED ********************
    Exit Code: 1
    
    Command Output (stdout):
    --
    # RUN: at line 12
    /__w/llvm/llvm/toolchain/bin/dpclang++ -Werror  -fsycl -fsycl-targets=spir64  /__w/llvm/llvm/llvm/sycl/test-e2e/bindless_images/vulkan_interop/external_semaphore_regular_cl.cpp -I /opt/vulkan/x86_64/include -L /opt/vulkan/x86_64/lib -lvulkan -o /__w/llvm/llvm/build-e2e/bindless_images/vulkan_interop/Output/external_semaphore_regular_cl.cpp.tmp.out  -Wno-ignored-attributes
    # executed command: /__w/llvm/llvm/toolchain/bin/dpclang++ -Werror -fsycl -fsycl-targets=spir64 /__w/llvm/llvm/llvm/sycl/test-e2e/bindless_images/vulkan_interop/external_semaphore_regular_cl.cpp -I /opt/vulkan/x86_64/include -L /opt/vulkan/x86_64/lib -lvulkan -o /__w/llvm/llvm/build-e2e/bindless_images/vulkan_interop/Output/external_semaphore_regular_cl.cpp.tmp.out -Wno-ignored-attributes
    # note: command had no output on stdout or stderr
    # RUN: at line 13
    env UR_L0_BATCH_SIZE=8 env ONEAPI_DEVICE_SELECTOR=level_zero:gpu  /__w/llvm/llvm/build-e2e/bindless_images/vulkan_interop/Output/external_semaphore_regular_cl.cpp.tmp.out
    # executed command: env UR_L0_BATCH_SIZE=8 env ONEAPI_DEVICE_SELECTOR=level_zero:gpu /__w/llvm/llvm/build-e2e/bindless_images/vulkan_interop/Output/external_semaphore_regular_cl.cpp.tmp.out
    # .---command stderr------------
    # | [Vulkan Validation Layer]: libnvidia-gpucomp.so.565.57.01: cannot open shared object file: No such file or directory
    # | [Vulkan Validation Layer]: loader_icd_scan: Failed loading library associated with ICD JSON libGLX_nvidia.so.0. Ignoring this JSON
    # | The external semaphore wait did not submit the preceding batch.
    # `-----------------------------
    # error: command failed with exit status: 1


Note the vulkan validation layer errors is not related to the actual problem, you can ignore those lines.

@crystarm

Copy link
Copy Markdown
Contributor Author

Hi @sarnex!!!! (⸝⸝> ᴗ•⸝⸝)
Thanks for the report! I investigated this and opened #23561.

The failing assertion uses a shared-USM marker kernel to infer that the external semaphore wait submitted the preceding batch. This is an indirect check and has been failing sporadically in the DG2 postcommit configuration.

The follow-up replaces it with a direct Level Zero trace check. It requires two submissions around the external semaphore wait: one for the preceding regular command list and one for the wait itself. The existing semaphore semantic checks are preserved.

@sarnex

sarnex commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Thanks!

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.

bindless_images/vulkan_interop/external_semaphore_regular_cl_fails.cpp hangs on Linux with driver 26.35.39758.10

9 participants