Skip to content
This repository has been archived by the owner on May 27, 2021. It is now read-only.

Allow transfer of array views backed by contiguous memory (fix #76) #77

Merged
merged 3 commits into from
Jan 10, 2018
Merged

Allow transfer of array views backed by contiguous memory (fix #76) #77

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
55c15ad
Permit copying views of arrays with unit stride
samuelpowell Jan 4, 2018
0f15f4d
Capture all type parameters
samuelpowell Jan 4, 2018
10d2cba
Restrict to dense contiguous views, add tests
samuelpowell Jan 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Permit copying views of arrays with unit stride
  • Loading branch information
@samuelpowell
samuelpowell committed Jan 4, 2018
commit 55c15adc9acded2194819efc967a6b1b79d8de1e
35 changes: 35 additions & 0 deletions src/array.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,41 @@ const copyfun = VERSION >= v"0.7.0-DEV.3057" ? :(copyto!) : :(copy!)
Mem.transfer!(dst.buf, src.buf, length(src) * sizeof(T))
return dst
end

"""
$copyfun{T}(dst::CuArray{T}, src::SubArray{T,N,A,I,true})

Copy an array view from a host array `src` to a device array `dst` in place. Both arrays
should have an equal length, and the stide of the view must be unity in all dimensions.
"""
function Base.$copyfun(dst::CuArray{T}, src::SubArray{T,N,A,I,true}) where T
if length(dst) != length(src)
throw(ArgumentError("Inconsistent array length."))
end
if any(strides(src) .!= 1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fail for any multidimensional array, where even a dense array has a stride bigger than one along higher dimensions.

strides is not entirely safe yet, but see JuliaLang/julia#25321 (which should make it safe). If you rely on an unsafe strides, you might get wrong answers or segfaults.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timholy good point, thanks. I was concentrating on the particular problem I have in #76 and hadn't properly considered this.

I could fix this to work for views which only restrict the trailing dimension to a contiguous region, but that's too peculiar. Alternatively, I could restrict the method only to operate on views which result in a vector, which would still be useful.

That said, if strides isn't safe, then that might be a showstopper. Feel free to close the PR if that's the case - I can always implement these methods privately in my package until things stabilise.

throw(ArgumentError("Transfers to an array view require unit stride."))
end
Mem.upload!(dst.buf, pointer(src), length(src) * sizeof(T))
return dst
end

"""
$copyfun{T}(dst::SubArray{T,N,A,I,true}, src::CuArray{T})

Copy an array from a device array `src` to a host array view `dst` in place. Both arrays
should have an equal length, and the stride of the view must be unity in all dimensions.
"""
function Base.$copyfun(dst::SubArray{T,N,A,I,true}, src::CuArray{T}) where T
if length(dst) != length(src)
throw(ArgumentError("Inconsistent array length."))
end
if any(strides(dst) .!= 1)
throw(ArgumentError("Transfers to an array view require unit stride."))
end
Mem.download!(pointer(dst), src.buf, length(src) * sizeof(T))
return dst
end

end


Expand Down