Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
368 changes: 116 additions & 252 deletions src/runtime/vulkan.cpp

Large diffs are not rendered by default.

32 changes: 15 additions & 17 deletions src/runtime/vulkan_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ class VulkanMemoryAllocator;
struct VulkanShaderBinding;
struct VulkanCompiledShaderModule;
struct VulkanCompilationCacheEntry;
struct VulkanSubmission;
struct VulkanSubmissionQueue;

// --------------------------------------------------------------------------

Expand All @@ -41,11 +43,7 @@ VulkanMemoryAllocator *vk_create_memory_allocator(void *user_context, VkDevice d
const VkAllocationCallbacks *alloc_callbacks);

int vk_destroy_memory_allocator(void *user_context, VulkanMemoryAllocator *allocator);
int vk_clear_device_buffer(void *user_context,
VulkanMemoryAllocator *allocator,
VkCommandBuffer command_buffer,
VkQueue command_queue,
VkBuffer device_buffer);
void vk_clear_device_buffer(VkCommandBuffer command_buffer, VkBuffer device_buffer);
// --------------------------------------------------------------------------
// Context
// --------------------------------------------------------------------------
Expand Down Expand Up @@ -105,18 +103,18 @@ int vk_destroy_command_pool(void *user_context, VulkanMemoryAllocator *allocator
int vk_create_command_buffer(void *user_context, VulkanMemoryAllocator *allocator, VkCommandPool pool, VkCommandBuffer *command_buffer);
int vk_destroy_command_buffer(void *user_context, VulkanMemoryAllocator *allocator, VkCommandPool command_pool, VkCommandBuffer command_buffer);

struct ScopedVulkanCommandBufferAndPool;

int vk_fill_command_buffer_with_dispatch_call(void *user_context,
VkDevice device,
VkCommandBuffer command_buffer,
VkPipeline compute_pipeline,
VkPipelineLayout pipeline_layout,
VkDescriptorSet descriptor_set,
uint32_t descriptor_set_index,
int blocksX, int blocksY, int blocksZ);

int vk_submit_command_buffer(void *user_context, VkQueue queue, VkCommandBuffer command_buffer);
// -- Submissions
VulkanSubmission *vk_begin_submission(void *user_context, VulkanMemoryAllocator *allocator, VkQueue queue, uint32_t queue_family_index);
int vk_end_submission(void *user_context, VulkanMemoryAllocator *allocator, VulkanSubmission *submission, bool host_read = false);
int vk_wait_for_submissions(void *user_context, VulkanMemoryAllocator *allocator, bool retire = true);
int vk_destroy_submission_queue(void *user_context, VulkanMemoryAllocator *allocator);

void vk_record_dispatch_call(VkCommandBuffer command_buffer,
VkPipeline compute_pipeline,
VkPipelineLayout pipeline_layout,
VkDescriptorSet descriptor_set,
uint32_t descriptor_set_index,
int blocksX, int blocksY, int blocksZ);

// -- Scalar Uniform Buffer
bool vk_needs_scalar_uniform_buffer(void *user_context,
Expand Down
80 changes: 13 additions & 67 deletions src/runtime/vulkan_memory.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ class VulkanMemoryAllocator {
size_t bytes_allocated_for_regions() const;
size_t regions_allocated() const;

// The command buffers this allocator's device submits work with (see vk_begin_submission)
VulkanSubmissionQueue *submission_queue = nullptr;

private:
static constexpr uint32_t invalid_memory_heap = uint32_t(-1);
static constexpr uint32_t invalid_usage_flags = uint32_t(-1);
Expand Down Expand Up @@ -199,6 +202,7 @@ int VulkanMemoryAllocator::initialize(void *user_context,
region_byte_count = 0;
block_count = 0;
block_byte_count = 0;
submission_queue = nullptr;
BlockAllocator::MemoryAllocators allocators;
allocators.system = system_allocator;
allocators.block = {VulkanMemoryAllocator::allocate_block, VulkanMemoryAllocator::deallocate_block, VulkanMemoryAllocator::conform_block_request};
Expand Down Expand Up @@ -500,6 +504,10 @@ bool VulkanMemoryAllocator::collect(void *user_context) {
if ((device == nullptr) || (physical_device == nullptr) || (block_allocator == nullptr)) {
return false;
}
// Released regions may still be in use by submitted work: wait for it to finish
// before freeing them. (Retiring the submissions would release their staging
// buffers, which must not happen here: this may be called from inside reserve().)
vk_wait_for_submissions(user_context, this, false);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[CRITICAL] VkBuffer handles for released device regions can still be destroyed via vkDestroyBuffer while in flight on the GPU (VUID-vkDestroyBuffer-buffer-00922).

In halide_vulkan_device_free (src/runtime/vulkan.cpp:163) and halide_vulkan_device_malloc (src/runtime/vulkan.cpp:336), when halide_can_reuse_device_allocations is true, ctx.allocator->release() marks the BlockRegion as Available without waiting for in-flight submissions. Although VulkanMemoryAllocator::collect waits here, two normal reserve() paths destroy a released region's VkBuffer (VulkanMemoryAllocator::deallocate_region -> vkDestroyBuffer) without calling VulkanMemoryAllocator::collect:

  1. Region splitting: RegionAllocator::reserve -> RegionAllocator::split_block_region (src/runtime/internal/region_allocator.h:198, 382) calls deallocate_block_region_if_unused -> VulkanMemoryAllocator::deallocate_region when a smaller allocation splits a released region.
  2. Region coalescing: BlockAllocator::reserve_memory_region -> RegionAllocator::collect -> coalesce_block_regions (src/runtime/internal/block_allocator.h:277, src/runtime/internal/region_allocator.h:335-366) calls RegionAllocator::collect directly (not VulkanMemoryAllocator::collect) when a larger allocation coalesces free regions within a block.

Calling vk_wait_for_submissions(user_context, instance, false); inside VulkanMemoryAllocator::deallocate_region before vkDestroyBuffer fixes both paths while keeping same-region VkBuffer reuse wait-free.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, fixed in 98bde6f: VulkanMemoryAllocator::deallocate_region now waits for in-flight submissions (without retiring them, since it runs inside reserve()) before vkDestroyBuffer, which covers both the split and the coalesce path. Same-size reuse stays wait-free; performance_gpu_dispatch_overhead is unchanged. Added correctness_gpu_free_while_in_flight, which frees buffers while their kernels may still run and then allocates smaller and larger ones, so released regions are split and coalesced with work in flight.

🤖 Generated with Claude Code

return block_allocator->collect(this);
}

Expand All @@ -508,6 +516,7 @@ int VulkanMemoryAllocator::destroy(void *user_context) {
debug(nullptr) << "VulkanMemoryAllocator: Destroying allocator ("
<< "user_context=" << user_context << ") ... \n";
#endif
vk_destroy_submission_queue(user_context, this);
if (block_allocator != nullptr) {
BlockAllocator::destroy(user_context, block_allocator);
block_allocator = nullptr;
Expand Down Expand Up @@ -1381,6 +1390,8 @@ VulkanMemoryAllocator *vk_create_memory_allocator(void *user_context,

int vk_destroy_memory_allocator(void *user_context, VulkanMemoryAllocator *allocator) {
if (allocator != nullptr) {
// Release what the submissions hold before collecting the unused regions
vk_destroy_submission_queue(user_context, allocator);
allocator->collect(user_context);
VulkanMemoryAllocator::destroy(user_context, allocator);
allocator = nullptr;
Expand All @@ -1390,74 +1401,9 @@ int vk_destroy_memory_allocator(void *user_context, VulkanMemoryAllocator *alloc

// --------------------------------------------------------------------------

int vk_clear_device_buffer(void *user_context,
VulkanMemoryAllocator *allocator,
VkCommandBuffer command_buffer,
VkQueue command_queue,
VkBuffer device_buffer) {

#ifdef DEBUG_RUNTIME
debug(user_context)
<< " vk_clear_device_buffer (user_context: " << user_context << ", "
<< "allocator: " << (void *)allocator << ", "
<< "command_buffer: " << (void *)command_buffer << ", "
<< "command_queue: " << (void *)command_queue << ", "
<< "device_buffer: " << (void *)device_buffer << ")\n";
#endif

// begin the command buffer
VkCommandBufferBeginInfo command_buffer_begin_info =
{
VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO, // struct type
nullptr, // pointer to struct extending this
VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT, // flags
nullptr // pointer to parent command buffer
};

VkResult result = vkBeginCommandBuffer(command_buffer, &command_buffer_begin_info);
if (result != VK_SUCCESS) {
error(user_context) << "Vulkan: vkBeginCommandBuffer returned " << vk_get_error_name(result) << "\n";
return halide_error_code_generic_error;
}

// fill buffer with zero values up to the size of the buffer
// Records filling the whole buffer with zeros.
void vk_clear_device_buffer(VkCommandBuffer command_buffer, VkBuffer device_buffer) {
vkCmdFillBuffer(command_buffer, device_buffer, 0, VK_WHOLE_SIZE, 0);

// end the command buffer
result = vkEndCommandBuffer(command_buffer);
if (result != VK_SUCCESS) {
error(user_context) << "Vulkan: vkEndCommandBuffer returned " << vk_get_error_name(result) << "\n";
return halide_error_code_generic_error;
}

// submit the command buffer
VkSubmitInfo submit_info =
{
VK_STRUCTURE_TYPE_SUBMIT_INFO, // struct type
nullptr, // pointer to struct extending this
0, // wait semaphore count
nullptr, // semaphores
nullptr, // pipeline stages where semaphore waits occur
1, // how many command buffers to execute
&command_buffer, // the command buffers
0, // number of semaphores to signal
nullptr // the semaphores to signal
};

result = vkQueueSubmit(command_queue, 1, &submit_info, VK_NULL_HANDLE);
if (result != VK_SUCCESS) {
error(user_context) << "Vulkan: vkQueueSubmit returned " << vk_get_error_name(result) << "\n";
return halide_error_code_generic_error;
}

// wait for memset to finish
result = vkQueueWaitIdle(command_queue);
if (result != VK_SUCCESS) {
error(user_context) << "Vulkan: vkQueueWaitIdle returned " << vk_get_error_name(result) << "\n";
return halide_error_code_generic_error;
}

return halide_error_code_success;
}

// --------------------------------------------------------------------------
Expand Down
Loading
Loading