-
Notifications
You must be signed in to change notification settings - Fork 41
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
Allow StructArray arguments in CUDAnative kernels. #87
Conversation
This uses a generated function to avoid a dynamic call for `getindex` allowing it to be called in a CUDAnative kernel.
@@ -134,11 +134,11 @@ Base.axes(s::StructArray) = axes(fieldarrays(s)[1]) | |||
Base.axes(s::StructArray{<:Any, <:Any, <:EmptyTup}) = (1:0,) | |||
|
|||
get_ith(cols::NamedTuple, I...) = get_ith(Tuple(cols), I...) | |||
function get_ith(cols::NTuple{N, Any}, I...) where N | |||
ntuple(N) do i |
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.
Might need to be
ntuple(Val(N))` do i
Base.@_inline_meta
@inbounds res = getfield(cols, i)[I...]
end
But the generated function works as well.
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.
I tried
function get_ith(cols::NTuple{N, Any}, I...) where N
ntuple(Val(N)) do i
Base.@_inline_meta
@inbounds res = getfield(cols, i)[I...]
end
end
but still got a dynamic function
ERROR: LoadError: InvalidIRError: compiling kernel!(StructArray{SHermitianCompact{2,Float64,3},1,NamedTuple{(:lowertriangle,),Tuple{StructArray{SArray{Tuple{3},Float64,1,3},1,NamedTuple{(:data,),Tuple{StructArray{Tuple{Float64,Float64,Float64},1,Tuple{CuDeviceArray{Float64,1,CUDAnative.AS.Global},CuDeviceArray{Float64,1,CUDAnative.AS.Global},CuDeviceArray{Float64,1,CUDAnative.AS.Global}},Int64}}},Int64}}},Int64}, StructArray{SHermitianCompact{2,Float64,3},1,NamedTuple{(:lowertriangle,),Tuple{StructArray{SArray{Tuple{3},Float64,1,3},1,NamedTuple{(:data,),Tuple{StructArray{Tuple{Float64,Float64,Float64},1,Tuple{CuDeviceArray{Float64,1,CUDAnative.AS.Global},CuDeviceArray{Float64,1,CUDAnative.AS.Global},CuDeviceArray{Float64,1,CUDAnative.AS.Global}},Int64}}},Int64}}},Int64}) resulted in invalid LLVM IR
Reason: unsupported dynamic function invocation (call to ntuple(f, ::Val{1}) in Base at ntuple.jl:41)
Stacktrace:
[1] get_ith at /home/lucas/julia/dev/StructArrays/src/structarray.jl:138
[2] get_ith at /home/lucas/julia/dev/StructArrays/src/structarray.jl:136
[3] getindex at /home/lucas/julia/dev/StructArrays/src/structarray.jl:153
[4] kernel! at /home/lucas/research/code/Heptapus.jl/examples/structarrays/try.jl:14
Reason: unsupported call to the Julia runtime (call to jl_f_getfield)
Stacktrace:
[1] getindex at /home/lucas/julia/dev/StructArrays/src/structarray.jl:153
[2] kernel! at /home/lucas/research/code/Heptapus.jl/examples/structarrays/try.jl:14
Reason: unsupported dynamic function invocation (call to foreachfield)
Stacktrace:
[1] getindex at /home/lucas/julia/dev/StructArrays/src/structarray.jl:153
[2] kernel! at /home/lucas/research/code/Heptapus.jl/examples/structarrays/try.jl:14
Reason: unsupported dynamic function invocation (call to foreachfield)
Stacktrace:
[1] setindex! at /home/lucas/julia/dev/StructArrays/src/structarray.jl:168
[2] kernel! at /home/lucas/research/code/Heptapus.jl/examples/structarrays/try.jl:14
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.
Generally, I would prefer a normal function over a generated function. What is the cause of "dynamic function invocation"? Is it due to the Never mind, splatting seems fine.I...
splatting?
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.
I agree it would be nice to avoid generated functions. I am not sure why this has to be generated but there is definitely something wrong with what I did for foreachfield
as the test suite doesn't pass. Maybe that is causing issues. If you get a chance, could you take a peak at what I did and see if you can spot something astray?
This allows `setindex!` on `StructArray`s to be used in `CUDAnative` kernels.
53e160f
to
2aee4cc
Compare
Looks good from my end |
end | ||
end | ||
|
||
@generated function foreachfield(::Type{T}, f, xs...) where {T<:Tup} |
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.
This introduces a lot of code complexity, while the original implementation is just a few lines of code: I'm not completely sure I understand conceptually what this refactor is trying to do. Is there an intuitive explanation as to way the original foreachfield
is not suitable for CUDAnative while this implementation works?
This introduces a lot of code complexity, while the original
implementation is just a few lines of code: I'm not completely sure I
understand conceptually what this refactor is trying to do. Is there
an intuitive explanation as to way the original `foreachfield` is not
suitable for CUDAnative while this implementation works?
This is trying to identify all of the fields that have arrays associated
with them at compile time. This is avoiding the current run time
recursion that is causing dynamic function calls and thus the code not
compile on the GPU.
|
Closing this in favor of #114. |
These changes allow
StructArray
s to be use as arguments toCUDAnative
kernels and allowsgetindex
andsetindex!
to be called. For example the following code now works