Skip to content
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

Add CudaUVMSpace specializations for cuBLAS IAMAX and SCAL #758

Closed
vqd8a opened this issue Jul 2, 2020 · 9 comments
Closed

Add CudaUVMSpace specializations for cuBLAS IAMAX and SCAL #758

vqd8a opened this issue Jul 2, 2020 · 9 comments

Comments

@vqd8a
Copy link
Contributor

vqd8a commented Jul 2, 2020

Requested by GEMMA for integrating GEMMA and ADELUS.

@srajama1
Copy link
Contributor

srajama1 commented Jul 2, 2020

Oh man, why do they need UVM when I am trying to remove it in other places.

@crtrott
Copy link
Member

crtrott commented Jul 2, 2020

That's for ETI? Because what else would be the specialization?

@vqd8a
Copy link
Contributor Author

vqd8a commented Jul 2, 2020

It is for cuBLAS TPL. Kokkos-kernels' IAMAX and SCAL have no CudaUVMSpace specializations when calling cuBLAS TPL. We only have CudaSpace specializations. For example:

KOKKOSBLAS1_ZIAMAX_TPL_SPEC_DECL_CUBLAS( unsigned long, Kokkos::LayoutLeft, Kokkos::CudaSpace, true)

When CudaUVMSpace is used, the fall-back implementations would be called, which might be slower than cuBLAS.
I did a simple fix in #759 by adding CudaUVMSpace specializations to the TPL files: KokkosBlas1_iamax_tpl_spec_decl.hpp, KokkosBlas1_iamax_tpl_spec_avail.hpp, and KokkosBlas1_scal_tpl_spec_decl.hpp, KokkosBlas1_scal_tpl_spec_avail.hpp

@vqd8a vqd8a changed the title Add CudaUVMSpace specializations for IAMAX and SCAL Add CudaUVMSpace specializations for cuBLAS IAMAX and SCAL Jul 2, 2020
@crtrott
Copy link
Member

crtrott commented Jul 2, 2020

why isn't this type-erased before it this that (i.e. use Device<Cuda,AnonymousSpace>)?

@vqd8a
Copy link
Contributor Author

vqd8a commented Jul 2, 2020

Does it require re-write the whole TPLs? I just followed what was done in #397 and #399 for GEMM for a quick fix.

@vqd8a
Copy link
Contributor Author

vqd8a commented Jul 2, 2020

As pointed by @ndellingwood, there is an open issue #144 about AnonymousSpace but it was backlogged. I feel it is somehow beyond the scope of my PR. But should using AnonymousSpace be reprioritized? @srajama1 @crtrott

@crtrott
Copy link
Member

crtrott commented Jul 2, 2020

yeah its fine not to do it in your PR. but generally it might be worthwhile to look at that.

@vqd8a
Copy link
Contributor Author

vqd8a commented Jul 2, 2020

Thanks @crtrott

@vqd8a
Copy link
Contributor Author

vqd8a commented Jul 2, 2020

I will look at that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants