Skip to content

Commit 1a4b217

Browse files
SS-JIAfacebook-github-bot
authored andcommitted
Remove support for VkSemaphore
Summary: ## Changes Revert some changes to the D78360038 / #12527 stack which enabled support for submitting command buffers with an associated semaphore. ## Context The original intent was to allow command buffers to be correctly ordered when submitting multiple command buffers for model inference. Previously it was thought that the Vulkan API would not be aware of the dependency between two separate command buffer submissions, so a semaphore would be needed to ensure correct execution order between them. However, I noticed the following validation layer error on Mac: ``` Validation 0 vkQueueSubmit(): pSubmits[0].pSignalSemaphores[0] (VkSemaphore 0x10f000000010f) is being signaled by VkQueue 0x1181082a8, but it was previously signaled by VkQueue 0x1181082a8 and has not since been waited on. The Vulkan spec states: Each binary semaphore element of the pSignalSemaphores member of any element of pSubmits must be unsignaled when the semaphore signal operation it defines is executed on the device (https://vulkan.lunarg.com/doc/view/1.4.313.0/mac/antora/spec/latest/chapters/cmdbuffers.html#VUID-vkQueueSubmit-pSignalSemaphores-00067) ``` The reason this happens is because we store the `VkSemaphore` together with the `VkCommandBuffer`, and use the same `VkSemaphore` in the submit info every time the command buffer is submitted. However, there's no mechanism to reset the VkSemaphore to "unsignaled" once it's been signaled. Therefore, as-is the `VkSemaphores` do not serve any purpose after the first inference. The correct approach if we were to use semaphores is to create a new one with every submission, and not have it be attached to a specific command buffer However, after some deeper research I found that `VkSemaphore` is not actually needed to ensure correct execution order between command buffers; the command pipeline barriers that we already insert should be sufficient. My primary source is this [stackoverflow question](the correct approach if we were to use semaphores is to create a new one with every submission, and not have it be attached to a specific command buffer) which references the Vulkan API spec in the answer. Therefore, remove the `VkSemaphore` machinery since it's not required. Differential Revision: D79468286
1 parent 1d80837 commit 1a4b217

File tree

6 files changed

+8
-75
lines changed

6 files changed

+8
-75
lines changed

backends/vulkan/runtime/api/Context.cpp

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ Context::Context(vkapi::Adapter* adapter, const ContextConfig& config)
3838
querypool_(config_.query_pool_config, nullptr),
3939
// Command buffer submission
4040
cmd_mutex_{},
41-
cmd_(VK_NULL_HANDLE, VK_NULL_HANDLE, 0u),
42-
prev_semaphore_(VK_NULL_HANDLE),
41+
cmd_(VK_NULL_HANDLE, 0u),
4342
submit_count_{0u},
4443
// Memory Management
4544
buffer_clearlist_mutex_{},
@@ -196,21 +195,14 @@ void Context::register_blit(
196195
}
197196

198197
void Context::submit_cmd_to_gpu(VkFence fence_handle, const bool final_use) {
199-
// Wait semaphore would be previous command buffer's signal semaphore
200-
VkSemaphore wait_semaphore = prev_semaphore_;
201-
// Signal semaphore for the the current command buffer
202-
VkSemaphore signal_semaphore = cmd_.get_signal_semaphore();
203-
// Next command buffer would wait on this command buffer's signal semaphore
204-
prev_semaphore_ = signal_semaphore;
205-
206198
if (cmd_) {
207199
cmd_.end();
208200
adapter_p_->submit_cmd(
209201
queue_,
210202
cmd_.get_submit_handle(final_use),
211203
fence_handle,
212-
wait_semaphore,
213-
signal_semaphore);
204+
VK_NULL_HANDLE,
205+
VK_NULL_HANDLE);
214206

215207
submit_count_ = 0u;
216208
}
@@ -226,8 +218,6 @@ void Context::flush() {
226218
if (cmd_) {
227219
cmd_.invalidate();
228220
}
229-
// Reset previous command buffer semaphore
230-
prev_semaphore_ = VK_NULL_HANDLE;
231221

232222
std::lock_guard<std::mutex> bufferlist_lock(buffer_clearlist_mutex_);
233223
std::lock_guard<std::mutex> imagelist_lock(image_clearlist_mutex_);

backends/vulkan/runtime/api/Context.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,6 @@ class Context final {
6868
// Command buffers submission
6969
std::mutex cmd_mutex_;
7070
vkapi::CommandBuffer cmd_;
71-
// Semaphore for the previously submitted command buffer, if any
72-
VkSemaphore prev_semaphore_;
7371
uint32_t submit_count_;
7472
// Memory Management
7573
std::mutex buffer_clearlist_mutex_;

backends/vulkan/runtime/graph/ComputeGraph.cpp

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -776,36 +776,22 @@ void ComputeGraph::submit_current_cmd_and_wait(const bool final_use) {
776776
context_->fences().return_fence(fence);
777777
}
778778

779-
void ComputeGraph::submit_cmd(
780-
vkapi::CommandBuffer& cmd_buf,
781-
VkSemaphore wait_semaphore,
782-
VkSemaphore signal_semaphore,
783-
VkFence fence) {
779+
void ComputeGraph::submit_cmd(vkapi::CommandBuffer& cmd_buf, VkFence fence) {
784780
if (cmd_buf) {
785781
cmd_buf.end();
786782
context_->adapter_ptr()->submit_cmd(
787-
context_->queue(),
788-
cmd_buf.get_submit_handle(false),
789-
fence,
790-
wait_semaphore,
791-
signal_semaphore);
783+
context_->queue(), cmd_buf.get_submit_handle(false), fence);
792784
}
793785
}
794786

795787
void ComputeGraph::submit_deferred_cmds_and_wait() {
796-
VkSemaphore prev_semaphore = VK_NULL_HANDLE;
797788
vkapi::VulkanFence fence = context_->fences().get_fence();
798789

799790
for (uint32_t i = 0; i < deferred_cmd_list_.size(); i++) {
800791
auto& cmd = deferred_cmd_list_[i];
801-
VkSemaphore wait_semaphore = prev_semaphore;
802-
VkSemaphore signal_semaphore = cmd.get_signal_semaphore();
803-
prev_semaphore = signal_semaphore;
804792

805793
submit_cmd(
806794
cmd,
807-
wait_semaphore,
808-
signal_semaphore,
809795
i == (deferred_cmd_list_.size() - 1) ? fence.get_submit_handle()
810796
: VK_NULL_HANDLE);
811797
}

backends/vulkan/runtime/graph/ComputeGraph.h

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -857,11 +857,7 @@ class ComputeGraph final {
857857
/*
858858
* Submit one command buffer to the GPU.
859859
*/
860-
void submit_cmd(
861-
vkapi::CommandBuffer& cmd_buf,
862-
VkSemaphore wait_semaphore,
863-
VkSemaphore signal_semaphore,
864-
VkFence fence);
860+
void submit_cmd(vkapi::CommandBuffer& cmd_buf, VkFence fence);
865861

866862
/*
867863
* Submits all the commands gathered in deferred_cmd_bufs_ to the GPU.

backends/vulkan/runtime/vk_api/Command.cpp

Lines changed: 1 addition & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -20,34 +20,28 @@ namespace vkapi {
2020

2121
CommandBuffer::CommandBuffer(
2222
VkCommandBuffer handle,
23-
VkSemaphore semaphore,
2423
const VkCommandBufferUsageFlags flags)
2524
: handle_(handle),
26-
signal_semaphore_(semaphore),
2725
flags_(flags),
2826
state_(CommandBuffer::State::NEW),
2927
bound_{} {}
3028

3129
CommandBuffer::CommandBuffer(CommandBuffer&& other) noexcept
3230
: handle_(other.handle_),
33-
signal_semaphore_(other.signal_semaphore_),
3431
flags_(other.flags_),
3532
state_(other.state_),
3633
bound_(other.bound_) {
3734
other.handle_ = VK_NULL_HANDLE;
38-
other.signal_semaphore_ = VK_NULL_HANDLE;
3935
other.bound_.reset();
4036
}
4137

4238
CommandBuffer& CommandBuffer::operator=(CommandBuffer&& other) noexcept {
4339
handle_ = other.handle_;
44-
signal_semaphore_ = other.signal_semaphore_;
4540
flags_ = other.flags_;
4641
state_ = other.state_;
4742
bound_ = other.bound_;
4843

4944
other.handle_ = VK_NULL_HANDLE;
50-
other.signal_semaphore_ = VK_NULL_HANDLE;
5145
other.bound_.reset();
5246
other.state_ = CommandBuffer::State::INVALID;
5347

@@ -310,12 +304,6 @@ CommandPool::~CommandPool() {
310304
if (pool_ == VK_NULL_HANDLE) {
311305
return;
312306
}
313-
for (auto& semaphore : semaphores_) {
314-
if (semaphore != VK_NULL_HANDLE) {
315-
vkDestroySemaphore(device_, semaphore, nullptr);
316-
}
317-
}
318-
319307
vkDestroyCommandPool(device_, pool_, nullptr);
320308
}
321309

@@ -326,15 +314,14 @@ CommandBuffer CommandPool::get_new_cmd(bool reusable) {
326314
allocate_new_batch(config_.cmd_pool_batch_size);
327315

328316
VkCommandBuffer handle = buffers_[in_use_];
329-
VkSemaphore semaphore = semaphores_[in_use_];
330317

331318
VkCommandBufferUsageFlags cmd_flags = 0u;
332319
if (!reusable) {
333320
cmd_flags |= VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT;
334321
}
335322

336323
in_use_++;
337-
return CommandBuffer(handle, semaphore, cmd_flags);
324+
return CommandBuffer(handle, cmd_flags);
338325
}
339326

340327
void CommandPool::flush() {
@@ -350,7 +337,6 @@ void CommandPool::allocate_new_batch(const uint32_t count) {
350337
}
351338

352339
buffers_.resize(buffers_.size() + count);
353-
semaphores_.resize(buffers_.size() + count);
354340

355341
const VkCommandBufferAllocateInfo allocate_info{
356342
VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO, // sType
@@ -362,17 +348,6 @@ void CommandPool::allocate_new_batch(const uint32_t count) {
362348

363349
VK_CHECK(vkAllocateCommandBuffers(
364350
device_, &allocate_info, buffers_.data() + in_use_));
365-
366-
const VkSemaphoreCreateInfo semaphoreCreateInfo = {
367-
VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO, nullptr, 0};
368-
369-
for (uint32_t i = 0; i < count; i++) {
370-
VK_CHECK(vkCreateSemaphore(
371-
device_,
372-
&semaphoreCreateInfo,
373-
nullptr,
374-
semaphores_.data() + in_use_ + i));
375-
}
376351
}
377352

378353
} // namespace vkapi

backends/vulkan/runtime/vk_api/Command.h

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,7 @@ namespace vkapi {
2626

2727
class CommandBuffer final {
2828
public:
29-
explicit CommandBuffer(
30-
VkCommandBuffer,
31-
VkSemaphore,
32-
const VkCommandBufferUsageFlags);
29+
explicit CommandBuffer(VkCommandBuffer, const VkCommandBufferUsageFlags);
3330

3431
CommandBuffer(const CommandBuffer&) = delete;
3532
CommandBuffer& operator=(const CommandBuffer&) = delete;
@@ -73,8 +70,6 @@ class CommandBuffer final {
7370

7471
private:
7572
VkCommandBuffer handle_;
76-
// Semaphore to signal when the command buffer has completed execution
77-
VkSemaphore signal_semaphore_;
7873
VkCommandBufferUsageFlags flags_;
7974
State state_;
8075
Bound bound_;
@@ -86,7 +81,6 @@ class CommandBuffer final {
8681

8782
inline void invalidate() {
8883
handle_ = VK_NULL_HANDLE;
89-
signal_semaphore_ = VK_NULL_HANDLE;
9084
bound_.reset();
9185
}
9286

@@ -106,10 +100,6 @@ class CommandBuffer final {
106100

107101
VkCommandBuffer get_submit_handle(const bool final_use = false);
108102

109-
VkSemaphore get_signal_semaphore() const {
110-
return signal_semaphore_;
111-
}
112-
113103
inline operator bool() const {
114104
return handle_ != VK_NULL_HANDLE;
115105
}
@@ -140,8 +130,6 @@ class CommandPool final {
140130
// New Buffers
141131
std::mutex mutex_;
142132
std::vector<VkCommandBuffer> buffers_;
143-
// Semaphores corresponding to the command buffers
144-
std::vector<VkSemaphore> semaphores_;
145133
size_t in_use_;
146134

147135
public:

0 commit comments

Comments
 (0)