Skip to content

[SYCL] Force sycl::vec as device-copyable #9923

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

steffenlarsen
Copy link
Contributor

For a type to be implicitly device-copyable in SYCL it must either be trivially-copyable or explicitly specialize is_device_copyable. Currently sycl::vec isn't the former due to a work-around in the copy ctor. However, this work-around should not have an effect on whether it can actually be copied to device. As such, this commit adds an explcit specialization of is_device_copyable for vec to stay in place until the copy ctor has been fixed.

For a type to be implicitly device-copyable in SYCL it must either be
trivially-copyable or explicitly specialize `is_device_copyable`.
Currently `sycl::vec` isn't the former due to a work-around in the
copy ctor. However, this work-around should not have an effect on
whether it can actually be copied to device. As such, this commit
adds an explcit specialization of `is_device_copyable` for `vec` to stay
in place until the copy ctor has been fixed.

Signed-off-by: Larsen, Steffen <[email protected]>
@steffenlarsen steffenlarsen requested a review from a team as a code owner June 15, 2023 17:55
@steffenlarsen steffenlarsen temporarily deployed to aws June 15, 2023 18:21 — with GitHub Actions Inactive
@steffenlarsen steffenlarsen temporarily deployed to aws June 15, 2023 18:54 — with GitHub Actions Inactive
@steffenlarsen steffenlarsen temporarily deployed to aws June 15, 2023 19:27 — with GitHub Actions Inactive
@steffenlarsen steffenlarsen temporarily deployed to aws June 15, 2023 20:22 — with GitHub Actions Inactive
@rolandschulz
Copy link
Contributor

This only fixes it for vec itself. But not for a struct (defined by the user) containing a vec. Therefore this doesn't seem like a good solution.

@steffenlarsen
Copy link
Contributor Author

This only fixes it for vec itself. But not for a struct (defined by the user) containing a vec. Therefore this doesn't seem like a good solution.

Good point. This was only intended as a temporary solution, but you are right that it might not solve all the sycl::vec-related regressions introduced by the new requirements. As an alternative, #9970 reverts the requirements while we address the issue with sycl::vec.

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