Skip to content

[SYCLomatic] Fix for device_iterator trivial copyable (and device copyable) #2850

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

danhoeflinger
Copy link
Contributor

@danhoeflinger danhoeflinger commented May 22, 2025

device_iterator (USM available) is meant to be passed directly into sycl kernels, but was not technically sycl device-copyable, because of a non-default copy assignment operator (making it lose trivially copyable).
This non-default function is actually unnecessary, and can be defaulted with the same result. Making it default ensures trivial copyability and therefore sycl device-copyable. A requirement of sycl-device copyable was added to oneDPL "passed directly" types to conform to the SYCL specification.

Added tests in oneapi-src/SYCLomatic-test#915 to confirm sycl device-copyable for device_iterator and similar types.

Note:
This leaves in place an (I think unused) API for the copy constructor which doesn't make a lot of sense, but is there to match the USM_NONE version with an explicit template argument to describe the access mode of a buffer we are copying from. We could consider removing this API but I don't want to remove a public API in this PR. It is orthogonal to trivial copyability here.

@danhoeflinger danhoeflinger requested a review from a team as a code owner May 22, 2025 15:41
Copy link
Contributor

@mmichel11 mmichel11 left a comment

Choose a reason for hiding this comment

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

LGTM assuming cases in CI pass.

Copy link
Contributor

@timmiesmith timmiesmith left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you.

Copy link
Contributor

@tomflinda tomflinda left a comment

Choose a reason for hiding this comment

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

LGTM

@zhimingwang36 zhimingwang36 merged commit 6af8716 into oneapi-src:SYCLomatic May 26, 2025
4 of 7 checks passed
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.

5 participants