Fix conform request in the Vulkan memory allocator to avoid rounding up on subsequent passes.#9223
Open
johningve wants to merge 5 commits into
Open
Fix conform request in the Vulkan memory allocator to avoid rounding up on subsequent passes.#9223johningve wants to merge 5 commits into
johningve wants to merge 5 commits into
Conversation
conform() used request->alignment to decide whether to raise the
region's nearest_multiple, but it also writes the conformed alignment
back into the request. Therefore, conforming an already-conformed
request can produce a larger size because the second pass sees a larger
request->alignment and decides to raise the nearest_multiple.
The conform method of the vulkan memory allocator is called by the
runtime block allocator on multiple levels, using requests that were
already conformed, so different layers see mismatching sizes.
The mismatch causes issues observable on drivers reporting 64-byte
buffer alignment (Mesa lavapipe/RADV): a 96-byte buffer gets a 64-byte
region, and the transfer overruns it. The Khronos validation layer flags
it as VUID-vkCmdCopyBuffer-size-00115/00116 ("size (96) is greater than
the buffer size (64)"). These errors are present before this commit and
gone after it.
This commit changes conform() to use the conformed alignment instead of
request->alignment, to decide whether to raise nearest_multiple.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Previously it was possible that a region's conformed size exceeded the conformed size of its block. This could happen because a block's size was conformed by conform_block_request (rounding to the config nearest_multiple, then overwriting with VkMemoryRequirements.size), while regions carved from the block were conformed by a separate path that also rounds up to the device buffer alignment. The two paths disagree: for a buffer whose size is a multiple of the config nearest_multiple but not of the device alignment, the block was sized smaller than the region request it had to hold. On drivers reporting 64-byte buffer alignment (Mesa lavapipe/RADV), a 9338976-byte buffer (a multiple of 32 but not 64) produced a 9338976-byte block, while the region inside it conformed to 9339008. RegionAllocator::reserve then failed its headroom check, collect() retried, a second block failed identically, and allocation failed with "Failed to allocate device memory!". This commit fixes the issue by factoring the request-conforming arithmetic into a shared helper and calling it from both the block and region conform paths, so a block is always sized to hold a conformed region request of the same size. Fixes halide#8868 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
derek-gerstmann
changed the title
derek-gerstmann
requested changes
Jul 20, 2026
derek-gerstmann
left a comment
derek-gerstmann
left a comment
Contributor
There was a problem hiding this comment.
Thanks very much for the contribution! Yes, the conform request cleary should not be rounding up to the next nearest multiple on subsequent passes after the alignment is already adjusted.
Since the Vulkan Memory Allocator uses the block allocator interface, could you add some test cases to the runtime/internal tests for the block_allocator/region_allocator?
| size_t actual_alignment = conform_alignment(request->alignment, memory_requirements->alignment); | ||
|
|
||
| // Ensure the request ends on an aligned address. | ||
| // NOTE: use the conformed alignment and not the input alignment. Otherwise, conform is not idempotent: |
Contributor
There was a problem hiding this comment.
Oooh, good catch! Yes, the block allocator assumes that the conform is idempotent, and it shouldn't be rounding up to the next greatest multiple on subsequent passes if the alignment is already conformed.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9223 +/- ##
=======================================
Coverage ? 69.64%
=======================================
Files ? 255
Lines ? 78885
Branches ? 18859
=======================================
Hits ? 54941
Misses ? 18300
Partials ? 5644 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This commit moves the conform implementation from the vulkan_memory.h header into the memory_resources.h header so that it can be tested by the runtime/internal block+region allocator test. A test case is added to check that conform is idempotent over a number of different inputs, as well as testing the specific values from drivers reporting 64-bit alignment that revealed the previous version of conform to not be idempotent. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The error messages in allocate_block say "unable to deallocate". Likely copy-pasted from deallcate_block.
The debug print of find_block_region has block->memory.allocation.size. But this does not compile. block->memory has a size member itself, so use that instead.
johningve
force-pushed
the
vulkan-allocator-fixes
branch
from
July 21, 2026 09:20
6baa469 to
07c6b6c
Compare
Author
I added a commit that moves the conform function into memory_resources.h so that it can be used in the runtime/internal test: [vulkan] Move conform implementation to memory_resources.h and add tests |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes two bugs in the vulkan memory allocator, both related to conform. I ran into these issues when testing out the Vulkan backend with Mesa (llvmpipe / RADV).
One of the bugs was already reported: #8868
I did not add any tests because I could not see any existing tests for the Vulkan-specific runtime internals, and coming up with a test strategy for this is not something I feel prepared to do at this point since I'm new to the project.
I used Claude to help root-cause the bugs and then implemented the fixes myself.
Checklist