-
Notifications
You must be signed in to change notification settings - Fork 443
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
Use std::size_t for requested scratch allocations in GPU backends #4551
Conversation
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.
see below (this comment cannot be deleted)
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.
scratch_unified
definition was not updated:
/var/jenkins/workspace/Kokkos/core/src/Cuda/Kokkos_Cuda_Instance.cpp:623:32: error: out-of-line definition of 'scratch_unified' does not match any declaration in 'Kokkos::Impl::CudaInternal' [clang-diagnostic-error]
Cuda::size_type *CudaInternal::scratch_unified(
^
Also these look to be mismatched:
kokkos/core/src/Cuda/Kokkos_Cuda_Instance.hpp
Lines 63 to 68 in 83acf58
CudaSpace::size_type* cuda_internal_scratch_flags( | |
const Cuda&, const CudaSpace::size_type size); | |
CudaSpace::size_type* cuda_internal_scratch_space( | |
const Cuda&, const CudaSpace::size_type size); | |
CudaSpace::size_type* cuda_internal_scratch_unified( | |
const Cuda&, const CudaSpace::size_type size); |
Otherwise looks good to me.
4734a5d
to
c43dc8a
Compare
c43dc8a
to
1e6891c
Compare
In #4532, it turned out that some test cases needed the allocation size for scratch space to be larger than
ExecutionSpace::size_type
. This pull request tries to make the change fromExecutionSpace::size_type
tostd::size_t
more uniform acrossCUDA
,HIP
andSYCL
.