Refactor vk_semaphore to be a shared_ptr object

[sredman]

When allocating a vk_semaphore, we would return the pointer to the object in the vector's memory space. This was prone to change when the vector was reallocated to resize, resulting in the pointer pointing to nonsense.

Also add the vk_device which owns the semaphore to the struct, for use at cleanup time, to allow semaphores to be owned by the non-main device.

Finally, adds helpers for setting and awaiting the semaphores.

This change was prepared with the help of Github Copilot. I don't remember how much. This change is part of a larger change I'm working on. I understand the change and am prepared to discuss it with a human reviewer.

[jeffbolznv]

Semaphores are currently unused. What's the end goal here?

[sredman]

Semaphores are currently unused. What's the end goal here?

I have a fully working POC for row split matrix multiply: sredman#1 which relies on semaphores for cross-device synchronization.

IMO that branch has far too many changes to be able to submit as a PR, as well as a lot of junk from me experimenting which should never go to PR. This PR is the first of several as I'm trying to meaningfully break it down. Unfortunately it is likely to be a lot of meaningless churn until the actual split operation lands.

[jeffbolznv]

OK. Be warned that there's a plan to implement backend-agnostic tensor parallelism, so there's a chance this ends up not being needed.

[sredman]

OK. Be warned that there's a plan to implement backend-agnostic tensor parallelism, so there's a chance this ends up not being needed.

That would be very cool! Is there somewhere I can read more about that work? I imagine the building blocks I have would be necessary/beneficial for that effort as well.

[jeffbolznv]

I think @JohannesGaessler is working on it, but I don't know any more details.

[JohannesGaessler]

I primarily work on the CUDA backend but I am implementing tensor parallelism at a ggml backend level so it should also work for Vulkan as long as the Vulkan backend implements support for events. The implementation is done as a "meta" backend that internally wraps multiple "simple" backends. As of right now I think I have the interface and the allocation logic down and am working on orchestrating the computations and synchronizations. A --split-mode row like for the CUDA backend should not be implemented for other backend and I will remove that functionality from the CUDA backend once I have a replacement.

[sredman]

I primarily work on the CUDA backend

Thank you for your work. I remember now seeing your name in a lot of git blame entries. I spent a lot of time studying the CUDA split row implementation.

... it should also work for Vulkan as long as the Vulkan backend implements support for events.

There is already some basic support for ggml events using Vulkan events. However, Vulkan events are extremely limited purpose (https://docs.vulkan.org/refpages/latest/refpages/source/VkEvent.html)

Events are a synchronization primitive that can be used to insert a fine-grained dependency between commands submitted to the same queue

AFAIU, this means they are limited to use within a single device, maybe within a single physical device group I don't know how queues work in a physical device group, I haven't gotten that far in the docs and I don't have a physical device group to play with anyway!

Even assuming this does work with physical device groups, Vulkan is very restrictive to when it will form device groups. AFAIU, it must be the case that devices share some hardware interconnect to form a physical device group. I believe this is even more restrictive than the ROCm/CUDA implementation, since my GPUs were doing row split under ROCm but do not form a physical device group in Vulkan. Further, it will never be the case that devices from multiple vendors can be a physical device group, which rules out a lot of cool usecases, like say laptops with AMD UMA iGPU and nvidia dGPU. My implementation, while currently extremely naive, could support this.

Anyway, I recognize that I am a guest here and I don't want to trample on your plans. If you do not want this implementation, I will not try to push for it.

Is there a better place to discuss this? Should I create a github discussion?

[jeffbolznv]

We currently implement ggml events using both VkEvents and VkFences. We could also have each ggml event use a VkSemaphore, there just hasn't been a need for that yet.

I agree we don't want to rely on device groups, but I think it's possible to export/import semaphores across devices (maybe with some limitations) so I think it should be possible to use them. I expect we'll need to make some changes along these lines in order to be able to support the backend-agnostic implementation.

[sredman]

Yes, semaphores can be exported cross-device. I have that code working too. I was planning to submit it as its own PR after this one 🙂
https://github.com/sredman/llama.cpp/pull/1/changes#diff-35a5049d5eebe22eda1e0d661bd87639b31aafeba62deeaaaca9c13ec3e71d11R2574

The semantics are a bit annoying though. Before you can export a semaphore, the source device needs to submit the vk_context which is going to signal the semaphore. The only way I could figure to solve this was this submit mid-operation 😕 .

The more difficult challenge is cross-device data movement. I have GPU -> CPU -> GPU copy because it was easy to implement. I haven't played with better options, but from my understanding DMA should be possible.

[jeffbolznv]

If this is going to be a shared_ptr should it have a destructor?

[sredman]

I am not super familiar with how shared_ptr. Does it impact the need for a destructor? I thought an explicit destructor was not required in this case since this struct doesn't need any special cleanup beyond just letting each of the containing items be destructed.

[jeffbolznv]

I don't think the members' destructors actually do anything. e.g. vk_buffer_struct has a destructor to destroy the buffer and free the memory.

[jeffbolznv]

Do we actually need binary semaphores or can we always use timeline semaphores?

[sredman]

I went the other way, with only binary semaphores 🙂 . I believe you can always replace binary semaphores with timeline semaphores. But I couldn't think of a compelling usecase for timeline semaphores. Maybe, as you mention below, one use would be to keep one semaphore per queue for the life of the application, assuming I am wrong about export/import ownership rules.

[jeffbolznv]

I know this is pre-existing, but I doubt we'll want to destroy semaphores in ggml_vk_graph_cleanup (i.e. every token) rather than in ggml_vk_cleanup. Seems like we should be able to create one semaphore per queue and keep it alive for the lifetime of the device? And if we do that do we need these to be shared_ptrs at all?

[sredman]

Assuming the end goal is to use semaphores for cross-device signalling, I think destroying and recreating is inevitable. AFAIU, when you import a semaphore, that device owns it, so it wouldn't be possible to just keep one per queue.
https://docs.vulkan.org/refpages/latest/refpages/source/vkImportSemaphoreFdKHR.html

Importing a semaphore payload from a file descriptor transfers ownership of the file descriptor from the application to the Vulkan implementation.

(It's a little ambiguous. I took it to mean that ownership transfers from the first GPU to the application, and then from the application to the second GPU, but maybe I'm wrong about that)

[jeffbolznv]

That's talking about the file descriptor ownership, not the semaphore payload. If the semaphore payload has reference semantics (e.g. VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_OPAQUE_FD_BIT) then all semaphores sharing that payload will act as if they're the same semaphore. I think it should be possible to use a single timeline semaphore per queue, and should be less overhead than creating/destroying semaphores often.

[sredman]

Thanks. Would you mind if I changed this PR to add semaphore import plus this shared_ptr change? Since that's basically my value proposition anyway.

[jeffbolznv]

Will it be with timeline semaphores? Having the code available to do cross-device semaphores I think will be necessary for tensor parallelism regardless of how it's implemented.

[sredman]

I don't have that implemented yet, but if that's what you want I'll (hopefully) get back to this in a week or two and build the idea of per-queue semaphores and PCIe DMA.

[JohannesGaessler]

Regarding my work on multi GPU code: I have a working prototype and I think I won't need to make use of ggml_backend_event_t after all. Instead I'm using ggml_backend_tensor_copy which (in the CUDA backend) has a defined behavior w.r.t. synchronization. For the final implementation I think I'll add something like ggml_backend_tensor_shfl which lets 2 backends exchange tensor data between each other and define how the synchronization behavior of that should be. I'll also add something like ggml_backend_tensor_allreduce for CUDA so that something like NCCL can optionally be used if the backend supports it.

[sredman]

I'm going to close this for now; I have not yet had a chance to get back to it. I see #19378 is working for Vulkan without this support so apparently this synchronization tool is not needed.