-
Notifications
You must be signed in to change notification settings - Fork 109
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
oneMKL: replace unsupported sycl::vector_class with std:vector in USM API #381
Conversation
@@ -139,7 +139,7 @@ gemm (USM version) | |||
const fp beta, | |||
const fp *C, | |||
const std::int64_t ldc, | |||
const sycl::vector_class<sycl::event> &dependencies = {}); | |||
const std::vector<sycl::event> &dependencies = {}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like for sparse blas, the &dependencies was previously aligned with other arg names, I don't see this pattern for blas, lapack, dft, rng, stats or vm. Do you think we can got through these few and realign the args? not super important, but could be good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
@@ -154,7 +154,7 @@ compute_backward (USM version) | |||
template <typename descriptor_type, typename data_type> | |||
sycl::event compute_backward( descriptor_type &desc, | |||
data_type *inout, | |||
const cl::sycl::vector_class<cl::sycl::event> &dependencies = {}); | |||
const std::vector<cl::sycl::event> &dependencies = {}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually looks like dft also had args tab aligned. can we realign &dependencies with other args ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @mkrainiuk this PR looks good. I can confirm you caught all the references to sycl::vector_class. Short of a few stylistic changes with whitespace alignment, I approve this PR
The class
sycl::vector_class
has been removed from SYCL 2020 and the standard classstd::vector
should be used instead (SYCL 2020 spec p491).This PR updates oneMKL USM API to align with SYCL 2020.