[vulkan] Fix subregion memory offsets to respect buffer alignment (ha…

…lide#7576) * Fix buffer alignment constraints for subregion allocations (some drivers report a minimum alignment for the buffer that is larger than the storage or uniform storage offset alignemnt) Cleanup region offset and size constraints * Clang tidy/format pass --------- Co-authored-by: Derek Gerstmann <[email protected]>
ardier · Mar 3, 2024 · eacc94c · eacc94c
1 parent 6966caa
commit eacc94c
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 9 deletions.
diff --git a/src/runtime/internal/region_allocator.h b/src/runtime/internal/region_allocator.h
@@ -415,14 +415,14 @@ BlockRegion *RegionAllocator::split_block_region(void *user_context, BlockRegion
     }
 
     size_t actual_alignment = conform_alignment(alignment, block->memory.properties.alignment);
-    size_t actual_size = conform_size(block_region->memory.offset, size, actual_alignment, block->memory.properties.nearest_multiple);
-    size_t actual_offset = aligned_offset(block_region->memory.offset + size, actual_alignment);
-    size_t empty_size = block_region->memory.size - actual_size;
+    size_t split_size = conform_size(block_region->memory.offset, size, actual_alignment, block->memory.properties.nearest_multiple);
+    size_t split_offset = aligned_offset(block_region->memory.offset + size, actual_alignment);
+    size_t empty_size = block_region->memory.size - split_size;
 
 #ifdef DEBUG_RUNTIME_INTERNAL
     debug(user_context) << "RegionAllocator: Conforming size and alignment \n"
                         << " requested_size=" << (uint32_t)size << "\n"
-                        << " actual_size=" << (uint32_t)actual_size << "\n"
+                        << " split_size=" << (uint32_t)split_size << "\n"
                         << " requested_alignment=" << (uint32_t)alignment << " "
                         << " required_alignment=" << (uint32_t)block->memory.properties.alignment << " "
                         << " actual_alignment=" << (uint32_t)actual_alignment << ")\n";
@@ -431,13 +431,13 @@ BlockRegion *RegionAllocator::split_block_region(void *user_context, BlockRegion
 #ifdef DEBUG_RUNTIME_INTERNAL
     debug(user_context) << "RegionAllocator: Splitting "
                         << "current region (offset=" << (int32_t)block_region->memory.offset << " size=" << (int32_t)(block_region->memory.size) << " bytes) "
-                        << "to create empty region (offset=" << (int32_t)actual_offset << " size=" << (int32_t)(empty_size) << " bytes)!\n";
+                        << "to create empty region (offset=" << (int32_t)split_offset << " size=" << (int32_t)(empty_size) << " bytes)!\n";
 #endif
 
     BlockRegion *next_region = block_region->next_ptr;
     BlockRegion *empty_region = create_block_region(user_context,
                                                     block_region->memory.properties,
-                                                    actual_offset, empty_size,
+                                                    split_offset, empty_size,
                                                     block_region->memory.dedicated);
     halide_abort_if_false(user_context, empty_region != nullptr);
 
@@ -476,9 +476,13 @@ BlockRegion *RegionAllocator::create_block_region(void *user_context, const Memo
                         << "block_region=" << (void *)(block_region) << ") ...\n";
 #endif
 
+    size_t actual_alignment = conform_alignment(properties.alignment, block->memory.properties.alignment);
+    size_t actual_size = conform_size(offset, size, actual_alignment, block->memory.properties.nearest_multiple);
+    size_t actual_offset = aligned_offset(offset, actual_alignment);
+
     block_region->memory.handle = nullptr;
-    block_region->memory.offset = offset;
-    block_region->memory.size = size;
+    block_region->memory.offset = actual_offset;
+    block_region->memory.size = actual_size;
     block_region->memory.properties = properties;
     block_region->memory.dedicated = dedicated;
     block_region->status = AllocationStatus::Available;

diff --git a/src/runtime/vulkan_memory.h b/src/runtime/vulkan_memory.h
@@ -620,11 +620,14 @@ int VulkanMemoryAllocator::allocate_block(void *instance_ptr, MemoryBlock *block
                    << "dedicated=" << (block->dedicated ? "true" : "false") << ")\n";
 #endif
 
+    // Enforce any alignment constrainst reported by the device limits for each usage type
     if (usage_flags & VK_BUFFER_USAGE_STORAGE_BUFFER_BIT) {
         block->properties.alignment = instance->physical_device_limits.minStorageBufferOffsetAlignment;
     } else if (usage_flags & VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT) {
         block->properties.alignment = instance->physical_device_limits.minUniformBufferOffsetAlignment;
-    } else {
+    }
+    // Some drivers appear to report a buffer alignment constraint (regardless of usage) that can be larger than either of the above
+    if (memory_requirements.alignment > block->properties.alignment) {
         block->properties.alignment = memory_requirements.alignment;
     }
     block->handle = (void *)device_memory;