Conversation
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
| end | ||
|
|
||
| # lifted from GPUArrays.jl | ||
| function Base.fill!(A::StridedView{T, N, TA, F}, x) where {T, N, TA <: AbstractGPUArray{T}, F <: ALL_FS} |
There was a problem hiding this comment.
I'm slightly hesitant about these kinds of overloads, but I think it is just ignorance:
From what I know, the point of Strided is to have some logic that makes it such that even though A might have strides that are not just the usual multidimensional ones, it tries to figure out a way to access the parent array "as linearly as possible".
If I understand this overload correctly, this is simply doing a mapping and making use of the index computations, so in that sense it is somewhat similar to the implementation for just a regular SubArray, I think?
However, I don't actually know if the same kinds of performance ideas about linear access are even valid on the GPU, so this might be completely reasonable?
Maybe we could schedule a meeting somewhere next week to discuss this, and the possible implications of deciding what type of objects to use for our strided views?
There was a problem hiding this comment.
Sure, I think that makes sense. I think coming up with a good access pattern is probably even more important on the GPU but here I've done the naive thing just to get it started, and then we should definitely do some benchmarking/profiling to see if this is really slowing us down or not.
| function Base.fill!(A::StridedView{T, N, TA, F}, x) where {T, N, TA <: AbstractGPUArray{T}, F <: ALL_FS} | ||
| isempty(A) && return A | ||
| @kernel function fill_kernel!(a, val) | ||
| idx = @index(Global, Linear) |
There was a problem hiding this comment.
Would using @index(Global, Cartesian) have been an option? Linear indexing is not particularly efficient for a StridedView?
There was a problem hiding this comment.
Yes, we can do that instead, but it also affects the launch configuration for the kernel of course. I'll try that in a fix PR that hopefully will also fix TagBot...
3 participants
No description provided.