Repository navigation
[SYCL][UR] Bindless Image -- win32 named handles converted to UTF-8 when passed to L0 - #23199
cperkinsintel wants to merge 10 commits into
Conversation
…ort for VULKAN semaphore under test (even though it is not yet supported by driver)
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
This PR fixes Win32 named external semaphore import on Level Zero by converting UTF-16 NT object names from SYCL/UR into UTF-8 strings expected by the L0 semaphore import path, and adds/updates end-to-end coverage and documentation for named-handle interop.
Changes:
- Add Win32-only UTF-16 → UTF-8 conversion for named semaphore import in the Level Zero UR adapter.
- Add a new Vulkan E2E test for named timeline semaphore import (including non-ASCII name coverage).
- Re-enable/extend the D3D12 named semaphore E2E test and clarify docs around
resource_win32_namestring type/lifetime.
| File | Description |
|---|---|
| unified-runtime/source/adapters/level_zero/common/image_common.cpp | Introduces Win32-only UTF-16→UTF-8 conversion and applies it to the semaphore-by-name import path. |
| sycl/test-e2e/bindless_images/vulkan_interop/vulkan_win32_named_timeline_semaphore.cpp | New E2E Vulkan test validating named timeline semaphore import and UTF-16 round-trip behavior. |
| sycl/test-e2e/bindless_images/dx12_interop/D3D12_win32_named_semaphore.cpp | Re-enables the test for Intel driver and adds non-ASCII name coverage + safer wide-name printing. |
| sycl/test-e2e/bindless_images/dx12_interop/D3D12_win32_named_memory.cpp | Updates unsupported tracker and clarifies why named memory import remains unsupported. |
| sycl/doc/extensions/experimental/sycl_ext_oneapi_bindless_images.asciidoc | Documents resource_win32_name::name as LPCWSTR and clarifies storage lifetime expectations. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
dyniols
left a comment
dyniols
left a comment
There was a problem hiding this comment.
Bindless images part LGTM.
I agree with @ldorau that the issue with the string that it must outlive the import call has to be resolved before merge.
https://github.com/intel/llvm/pull/23199/changes#r4193219873
| D3D12NamedFence extFence = | ||
| createNamedExportableFence(d3dCtx, L"Global\\SYCLTestNamedFence"); | ||
| // Non-ASCII name (Chiqué气) verifies UTF-16 codepoints round-trip through | ||
| // import But rather than "Chiqué气" directly, we use escapes so MSVC |
ldorau
changed the title
|
I fixed typo in the title: "Bindlesss" -> "Bindless" |
|
@intel/llvm-gatekeepers please consider merging |
win32 named handles are normally UTF-16, but on the L0 driver side the ones for semaphores are UTF-8. Ultimately, this will be redesigned, but the API is out there, published, and there is nothing to be done about it in the short term. So SYCL is taking the UTF-16 and converting to UTF-8.