refactor template order allocMappedBuf
#2270
Conversation
|
please update alpaka/docs/source/dev/backends.rst Line 386 in 8dbe7c4 It was also forgotten during the change from platform type to platform instance. Maybe we should use here |
|
please update alpaka/docs/source/dev/backends.rst Line 366 in 8dbe7c4 alpaka::allocMappedBuf<TElement>(host, platform, extents) 1D, 2D, 3D supported! |
I |
SimeonEhrig
force-pushed
the
RefactorAllocMappedBuf
branch
from
f2defff
to
bc8241b
Compare
docs/source/dev/backends.rst
Outdated
| @@ -363,7 +363,7 @@ The following tables list the functions available in the `CUDA Runtime API <http | |||
| +----------------------------+--------------------------------------------------------------------------------------------+ | |||
| | cudaGetSymbolSize | -- | | |||
| +----------------------------+--------------------------------------------------------------------------------------------+ | |||
| | cudaHostAlloc | alpaka::allocMappedBuf<TPlatform, TElement>(host, extents) 1D, 2D, 3D supported! | | |||
| | cudaHostAlloc | alpaka::allocMappedBuf<TElement, TIdx>(host, extents) 1D, 2D, 3D supported! | | |||
There was a problem hiding this comment.
Sry my comment was wrong this should be allocMappedBufIfSupported. There is no strong need that this memory is mapped.
allocMappedBuf is not available for any accelerator or device.
SimeonEhrig
force-pushed
the
RefactorAllocMappedBuf
branch
from
bc8241b
to
bc82094
Compare
docs/source/dev/backends.rst
Outdated
| @@ -363,7 +363,7 @@ The following tables list the functions available in the `CUDA Runtime API <http | |||
| +----------------------------+--------------------------------------------------------------------------------------------+ | |||
| | cudaGetSymbolSize | -- | | |||
| +----------------------------+--------------------------------------------------------------------------------------------+ | |||
| | cudaHostAlloc | alpaka::allocMappedBuf<TPlatform, TElement>(host, extents) 1D, 2D, 3D supported! | | |||
| | cudaHostAlloc | alpaka::allocMappedBufIfSupported<TElement, TIdx>(host, extents) 1D, 2D, 3D supported! | | |||
There was a problem hiding this comment.
This too should be just
| | cudaHostAlloc | alpaka::allocMappedBufIfSupported<TElement, TIdx>(host, extents) 1D, 2D, 3D supported! | | |
| | cudaHostAlloc | alpaka::allocMappedBuf<TElement, TIdx>(host, extents) 1D, 2D, 3D supported! | |
There was a problem hiding this comment.
I do not think that allocMappedBuf is correct here. allocMappedBuf can fail if mapped memory is not supported for the current device, so the code will not run with all devices. allocMappedBufIfSupported will be safe for all kinds of devices.
The CUDA API function cudaHostAlloc is pinning memory but does not guarantee that the memory is visible on the GPU device, so the best fitting alpaka equivalent is allocMappedBufIfSupported
There was a problem hiding this comment.
The CUDA API function
cudaHostAllocis pinning memory but does not guarantee that the memory is visible on the GPU device, so the best fitting alpaka equivalent isallocMappedBufIfSupported
It does guarantee that. From the CUDA Runtime API:
__host__ cudaError_t cudaHostAlloc ( void** pHost, size_t size, unsigned int flags )Description
Allocates size bytes of host memory that is page-locked and accessible to the device.
There was a problem hiding this comment.
In any case, cudaMallocHost() and cudaHostAlloc() are identical, so they should map to the same functionality in alpaka.
There was a problem hiding this comment.
Ohh I was not aware that cudaMallocHost is the C++-API equivalent of cudaHostAlloc. I used always cudaHostAlloc with flags to create mapped memory.
In this case, IMO both should be equal. The problem I had is that for both CUDA methods, you can configure the behavior via a flag where we have only named functions. Maybe we should add allocMappedBufIfSupported, allocMappedBuf, and allocMappedBuf into the documentation and the user must decide what fitts best for their problem because the method used for the replacement depends on the flags used in the native CUDA function call.
There was a problem hiding this comment.
I changed it back to alpaka::allocMappedBuf<TPlatform, TElement>(host, extents) and wrote a for foot note for alpaka::allocMappedBufIfSupported<TPlatform, TElement>(host, extents)
SimeonEhrig
force-pushed
the
RefactorAllocMappedBuf
branch
from
bc82094
to
84e6a12
Compare
docs/source/dev/backends.rst
Outdated
| @@ -363,7 +363,7 @@ The following tables list the functions available in the `CUDA Runtime API <http | |||
| +----------------------------+--------------------------------------------------------------------------------------------+ | |||
| | cudaGetSymbolSize | -- | | |||
| +----------------------------+--------------------------------------------------------------------------------------------+ | |||
| | cudaHostAlloc | alpaka::allocMappedBuf<TPlatform, TElement>(host, extents) 1D, 2D, 3D supported! | | |||
| | cudaHostAlloc | alpaka::allocMappedBuf<TElement, TIdx>(host, extents) 1D, 2D, 3D supported! [1] | | |||
There was a problem hiding this comment.
| | cudaHostAlloc | alpaka::allocMappedBuf<TElement, TIdx>(host, extents) 1D, 2D, 3D supported! [1] | | |
| | cudaHostAlloc | alpaka::allocMappedBuf<TElement, TIdx>(host, platform, extents) 1D, 2D, 3D supported! [1] | |
SimeonEhrig
force-pushed
the
RefactorAllocMappedBuf
branch
from
84e6a12
to
7173395
Compare
Move template TPlatform as the last template. There is no need to provide the platform template signature if we pass the platform as an instance.
SimeonEhrig
force-pushed
the
RefactorAllocMappedBuf
branch
from
7173395
to
9ff7917
Compare
Move template TPlatform as the last template. There is no need to provide the platform template signature if we pass the platform as an instance.
follow up of #2162