Skip to content

[SYCL][UR] Remove redundant SYCL shadow copy of user host pointers - #22169

Closed
mateuszpn wants to merge 9 commits into
intel:syclfrom
mateuszpn:triple-buffer-ur
Closed

[SYCL][UR] Remove redundant SYCL shadow copy of user host pointers#22169
mateuszpn wants to merge 9 commits into
intel:syclfrom
mateuszpn:triple-buffer-ur

Conversation

@mateuszpn

@mateuszpn mateuszpn commented May 29, 2026

Copy link
Copy Markdown
Contributor

Problem. A SYCL buffer built from a misaligned user host pointer caused both SYCL and the UR adapter to allocate their own storage, leaving three live copies of the data — the user's memory, a SYCL shadow copy, and the adapter's device/USM allocation — kept in sync by two copy-back chains. The shadow copy dated from when SYCL assumed OpenCL's strict CL_MEM_USE_HOST_PTR alignment rules; adapters now handle such pointers themselves.

Fix. SYCL passes the user pointer to UR unchanged; each adapter decides whether to use it directly or allocate and copy.

  • SYCL (sycl_mem_obj_t): no more pre-aligning. The only shadow copy kept is for a read-only pointer that later gets a write accessor (a language rule — never write through const).
  • Level Zero: maybeImportUSM now returns success only when doZeUSMImport actually returns ZE_RESULT_SUCCESS, so a silently-rejected (e.g. misaligned) pointer falls back to allocate-and-copy instead of a bogus alias.
  • OpenCL / CUDA: no change — OpenCL's driver handles it internally; CUDA always copies.

Testing. New E2E test Regression/buffer_shadow_copy_platform_policy.cpp covers read, write-back, read-only immutability, and mid-life host_accessor map/unmap, for aligned and misaligned pointers (offsets 1/4/64 B) with use_host_ptr set.

@mateuszpn
mateuszpn marked this pull request as ready for review June 23, 2026 14:31
@mateuszpn
mateuszpn requested review from a team as code owners June 23, 2026 14:31

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

So CUDA/HIP/OpenCL adapters require no changes?

Comment on lines +37 to +39
// doZeUSMImport silently ignores driver-level failures (e.g., misaligned
// ptr), so re-query to confirm the import actually succeeded before
// reporting it to callers.

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.

I noticed that we don't actually verify the return value from the underlying L0 call. Maybe we can just check that instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@mateuszpn

Copy link
Copy Markdown
Contributor Author

So CUDA/HIP/OpenCL adapters require no changes?

CUDA: no change — never zero-copies (USE_HOST_PTR disabled), always copies.
OpenCL: no change — passes the misaligned-pointer tests, so the driver handles misalignment internally (copying when it can't alias); behaviorally confirmed, not proven by mechanism.
HIP: no change expected, but untested

@mateuszpn
mateuszpn requested a review from pbalcer June 26, 2026 08:25
@mateuszpn mateuszpn changed the title [SYCL][UR] Buffers allocation and copy-back moved to adapters [SYCL][UR] Remove redundant SYCL shadow copy of user host pointers Jun 30, 2026
@kswiecicki

Copy link
Copy Markdown
Contributor

@slawekptak, could you take a look at this please.

@mateuszpn

Copy link
Copy Markdown
Contributor Author

@intel/llvm-reviewers-runtime I'd appreciate your feedback

@mateuszpn

Copy link
Copy Markdown
Contributor Author

@cperkinsintel Are you able to take a look?

@kweronsx

Copy link
Copy Markdown
Contributor

Work on this issue will be continued here: #23050

@kweronsx kweronsx closed this Aug 28, 2026
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