-
Notifications
You must be signed in to change notification settings - Fork 50
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
Added const qualifier to USM API #443
Conversation
@@ -79,8 +79,10 @@ typename sb_handle_t::event_t _gemv_impl( | |||
const auto x_vector_size = is_transposed ? _M : _N; | |||
const auto y_vector_size = is_transposed ? _N : _M; | |||
|
|||
auto mA = make_matrix_view<col_major>(_mA, _M, _N, _lda); | |||
auto vx = make_vector_view(_vx, _incx, x_vector_size); | |||
typename MatrixViewType<container_t0, index_t, col_major>::type mA = |
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.
Seems all the changes in this file is unnecessary
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.
The changes to MatrixViewType<..> and VectorViewType<..> are necessary because we cannot replace them with const auto
. The problem is that, in the case of buffer, VectorView/MatrixView cannot be const
because of the access_displacement
.
src/interface/blas1_interface.hpp
Outdated
@@ -188,7 +191,9 @@ template <typename sb_handle_t, typename container_0_t, typename container_1_t, | |||
typename sb_handle_t::event_t _asum( | |||
sb_handle_t &sb_handle, index_t _N, container_0_t _vx, increment_t _incx, | |||
container_1_t _rs, const typename sb_handle_t::event_t &_dependencies) { | |||
auto vx = make_vector_view(_vx, _incx, _N); | |||
using element_t = typename ValueType<container_0_t>::type; |
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.
Seems unnecessary
src/interface/blas1_interface.hpp
Outdated
@@ -86,7 +87,9 @@ typename sb_handle_t::event_t _copy( | |||
sb_handle_t &sb_handle, index_t _N, container_0_t _vx, increment_t _incx, | |||
container_1_t _vy, increment_t _incy, | |||
const typename sb_handle_t::event_t &_dependencies) { | |||
auto vx = make_vector_view(_vx, _incx, _N); | |||
using element_t = typename ValueType<container_0_t>::type; |
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.
Seems unnecessary
@@ -306,18 +306,19 @@ make_gemm(input_t buffer_a, input_t buffer_b, output_t buffer_c, | |||
* @Note This kernel assumes the column-major matrices | |||
* @Note This kernel uses fixed size blocks of 16, but this can be changed | |||
*/ | |||
template <bool UnitDiag, bool Upper, int BlockSize, typename matrix_t> | |||
template <bool UnitDiag, bool Upper, int BlockSize, typename lhs_t, |
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.
this is the name changing. I don't think this changes related to const qualifier
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.
This name change was required to differentiate between the input and output container types. The input in this case can be const and the output cannot be const, thus we needed separate template parameters to represent the two different types.
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.
I have added minor comment overall looks OK. Thanks for the clean up
07f1e84
to
37be9e4
Compare
* Fix negative increment * Fix build with computecpp * Update dpcpp * Revert dpcpp version * Fix sync call * Add a pointer to the first element of the vector * Fixes issue with dependencies in nrm2 (#451) * Fix nrm2 * Remove cout * Fixes issue with dependencies in TBMV (#454) This PR fixes an issue with dependencies in TBMV (similar to the fix in #451)
0e82703
to
7d7cccd
Compare
@@ -306,18 +306,19 @@ make_gemm(input_t buffer_a, input_t buffer_b, output_t buffer_c, | |||
* @Note This kernel assumes the column-major matrices | |||
* @Note This kernel uses fixed size blocks of 16, but this can be changed | |||
*/ | |||
template <bool UnitDiag, bool Upper, int BlockSize, typename matrix_t> | |||
template <bool UnitDiag, bool Upper, int BlockSize, typename lhs_t, |
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.
This name change was required to differentiate between the input and output container types. The input in this case can be const and the output cannot be const, thus we needed separate template parameters to represent the two different types.
7d7cccd
to
5d6d6b8
Compare
* Renamed py_gen_blas_unary.py file to py_gen_blas_ops.py
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.
Looks good to me. Tested it on some devices, getting compile errors with Nvidia SM>=80 though.
/src/operations/blas3/gemm_local_joint_matrix.hpp:295:18: error: no matching conversion for functional-style cast from 'container_t' (aka 'const float *') to 'multi_ptr_' (aka 'multi_ptr<float, address_t::global_space>')
All the rest seems fine, error doesn't occur when disabling joint-matrix GEMM.
This PR adds the const qualifier to input containers for the USM api as required by the oneMKL specification. --------- Co-authored-by: Muhammad Tanvir <[email protected]> Co-authored-by: Alejandro Acosta <[email protected]>
This PR adds the const qualifier to input containers for the USM api as required by the oneMKL specification. --------- Co-authored-by: Muhammad Tanvir <[email protected]> Co-authored-by: Alejandro Acosta <[email protected]>
This PR adds the
const
qualifier toinput
containers for the USM api as required by the oneMKL specification.