vulkan: implement several ops relevant for ggml_opt #11769
Conversation
github-actions
bot
added
testing
| count += uint(data_a[idx] == data_b[idx]); | ||
| } | ||
|
|
||
| atomicAdd(data_d[0], D_TYPE(count)); |
There was a problem hiding this comment.
This shader crashes my Intel A770. I assume it's this atomicAdd. Maybe there is a way to avoid it?
There was a problem hiding this comment.
I'm not sure how to perform reduction with multiple workgroups without adding an extra buffer.
Maybe doing a single atomic per warp helps with your crash?
Does it also crash with this variant : a1633e4 ?
There was a problem hiding this comment.
It's surprising this crashes because int32 atomics in compute shaders are required in vulkan 1.0. Does it crash during the compile or while executing? Maybe the compiler would handle uint better?
There was a problem hiding this comment.
It's surprising this crashes because int32 atomics in compute shaders are required in vulkan 1.0. Does it crash during the compile or while executing? Maybe the compiler would handle uint better?
It crashes with a vk::DeviceLostError on execution.
I'm not sure how to perform reduction with multiple workgroups without adding an extra buffer. Maybe doing a single atomic per warp helps with your crash?
Does it also crash with this variant : a1633e4 ?
Sadly yes. But this isn't your problem, probably just another driver bug in mesa ANV.
ggml/src/ggml-vulkan/ggml-vulkan.cpp
Outdated
| } | ||
|
|
||
| static void ggml_vk_count_equal(ggml_backend_vk_context * ctx, vk_context& subctx, const ggml_tensor * src0, const ggml_tensor * src1, ggml_tensor * dst, bool dryrun = false) { | ||
| ggml_backend_tensor_memset(dst, 0, 0, ggml_nbytes(dst)); |
There was a problem hiding this comment.
This looks like it'll record to a separate command buffer and run out of order. Is this intended?
There was a problem hiding this comment.
Good catch, this is definitely not right. If a memset is needed, you have to implement an async variant of ggml_vk_buffer_memset and use that. ggml_backend_tensor_memset just comes back around to ggml_backend_vk_buffer_memset_tensor, which calls the synchronous ggml_vk_buffer_memset.
| for (uint i2 = i12; i2 < p.ne02; i2 += p.ne12) { | ||
| for (uint i1 = i11; i1 < p.ne01; i1 += p.ne11) { | ||
| for (uint i0 = i10; i0 < p.ne00; i0 += p.ne10) { | ||
| acc += data_a[i3*p.nb03 + i2*p.nb02 + i1*p.nb01 + i0*p.nb00]; |
There was a problem hiding this comment.
Is get_aoffset() needed here? (I don't know)
|
The Intel crash can be ignored. Once you resolve the memset, this can be merged. |
|
Thanks for the review |
This PR implements several GGML opcodes which are possibly relevant for #10544 (SUM, ARGMAX, SUB, COUNT_EQUAL, OPT_STEP_ADAMW, REPEAT_BACK).
After these patches, it is possible to run
test-optusing the Vulkan backend (with a few failures maybe caused by rounding issues?).Several issues were identified in test-backend-ops:
-b CPU)Several issues were identified in Vulkan CHECK_RESULTS mode: