Skip to content

[SYCL] Add support for get_native for buffer and fix backend_return_t #5881

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

Merged
merged 9 commits into from
Apr 22, 2022

Conversation

maximdimakov
Copy link
Contributor

Add support for sycl::get_native(sycl::buffer) accordingly to the spec.
Also, fixed backend_return_t for buffer for OpenCL backend.
Signed-off-by: mdimakov [email protected]

@maximdimakov maximdimakov marked this pull request as ready for review March 25, 2022 12:16
@maximdimakov maximdimakov requested a review from a team as a code owner March 25, 2022 12:16
@maximdimakov
Copy link
Contributor Author

/verify with intel/llvm-test-suite#945

@maximdimakov maximdimakov requested a review from gmlueck March 28, 2022 19:12
@maximdimakov
Copy link
Contributor Author

/verify with intel/llvm-test-suite#945

@gmlueck
Copy link
Contributor

gmlueck commented Mar 29, 2022

In #4952, we were concerned about breaking ABI / API compatibility when we changed the return type of get_native for event. Doesn't the same issue apply here?

@maximdimakov
Copy link
Contributor Author

@gmlueck Yes, I think we should apply the same logic

@maximdimakov maximdimakov requested a review from a team as a code owner April 4, 2022 14:40
gmlueck
gmlueck previously approved these changes Apr 4, 2022
Copy link
Contributor

@gmlueck gmlueck left a comment

Choose a reason for hiding this comment

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

LGTM, but someone else should review

@maximdimakov
Copy link
Contributor Author

@s-kanaev could you please review?

@maximdimakov
Copy link
Contributor Author

/verify with intel/llvm-test-suite#945

Comment on lines 81 to 84
template <typename DataT, int Dimensions, typename AllocatorT>
struct interop<backend::opencl, buffer<DataT, Dimensions, AllocatorT>> {
using type = cl_mem;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this type used? Is it needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems it is not used anywhere. I removed it

@againull againull merged commit 8b3c8c4 into intel:sycl Apr 22, 2022
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