vulkan: implement ADD1, ARANGE, FILL, SOFTPLUS, STEP, ROUND, CEIL, FLOOR, TRUNC #17319
Conversation
giuseppe
force-pushed
the
add-more-ops-vulkan
branch
from
67689fd to
e59509c
Compare
giuseppe
changed the title
github-actions
bot
added
documentation
ggml/src/ggml-vulkan/ggml-vulkan.cpp
Outdated
| CREATE_UNARY_RTE(exp) | ||
| #undef CREATE_UNARY_RTE | ||
|
|
||
| ggml_vk_create_pipeline(device, device->pipeline_add1_f16_f16, "add1_f16_f16", add1_f16_f16_len, add1_f16_f16_data, "main", 3, sizeof(vk_op_binary_push_constants), {256, 1, 1}, {}, 1); |
There was a problem hiding this comment.
Each invocation does 2 iterations and the WG has 256 threads, so I think this should be 512?
| layout (binding = 0) writeonly buffer D {D_TYPE data_d[];}; | ||
|
|
||
| void main() { | ||
| const uint i = gl_GlobalInvocationID.z * 262144 + gl_GlobalInvocationID.y * 512 + gl_GlobalInvocationID.x; |
There was a problem hiding this comment.
I think these index calculations rely on this logic in ggml_vk_op_f32, but arange doesn't go through ggml_vk_op_f32:
if (ne > 262144) {
elements = { 512, 512, CEIL_DIV(ne, 262144) };
} else if (ne > 512) {
elements = { 512, CEIL_DIV(ne, 512), 1 };
} else {
elements = { ne, 1, 1 };
}
giuseppe
force-pushed
the
add-more-ops-vulkan
branch
from
e59509c to
26eeb8a
Compare
|
@jeffbolznv thanks for the review. Addressed and pushed a new version |
| layout (binding = 0) writeonly buffer D {D_TYPE data_d[];}; | ||
|
|
||
| void main() { | ||
| const uint i = gl_GlobalInvocationID.z * 262144 + gl_GlobalInvocationID.y * 512 + gl_GlobalInvocationID.x; |
There was a problem hiding this comment.
I think this one has the same issue.
There was a problem hiding this comment.
fixed as well! Thanks
giuseppe
force-pushed
the
add-more-ops-vulkan
branch
from
26eeb8a to
d7df09c
Compare
jeffbolznv
left a comment
jeffbolznv
left a comment
There was a problem hiding this comment.
The changes look good to me now. Do you think it makes sense to add a few larger test cases for these to catch those bugs?
do you've any suggestions on what to add? I've tried a few new cases with more elements, but they don' fail with the older version of the PR |
|
Nothing specific, just that they would need to be more than 256k elements. |
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
giuseppe
force-pushed
the
add-more-ops-vulkan
branch
from
d7df09c to
2300634
Compare
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
giuseppe
force-pushed
the
add-more-ops-vulkan
branch
from
2300634 to
15bcb5e
Compare
added new test cases |
2 participants
mostly mechanical changes, except ROUND that doesn't match directly to Vulkan as there is no equivalent rounding mode (at least didn't manage to find it)