Repository navigation
vulkan: add a barrier before record_clone writes the dst buffer - #7151
Open
kongxinjun wants to merge 1 commit into
Open
kongxinjun wants to merge 1 commit into
kongxinjun wants to merge 1 commit into
Conversation
A VkMat released while recording returns its memory to the blob allocator right away, so the dst of a buffer copy may be memory that an earlier command in the same command buffer still reads or writes. record_clone recorded no barrier for dst, and the compute barriers in between do not cover the transfer stage. Synchronization validation reports WRITE_AFTER_READ and WRITE_AFTER_WRITE on vkCmdCopyBuffer in test_mat_batch.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Member
|
|
This branch has not been deployed
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
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.
在 Apple M5 Pro(macOS 26.6.2,MoltenVK 1.4.2)上开 VK_LAYER_KHRONOS_validation 的同步校验(validate_sync)跑 test_mat_batch,test_batch_forward_scale_external 会报两处:
record_clone 对 src 会按 access_flags / stage_flags 加 barrier,对 dst 没有加(注释是 barrier device any @ null)。但 dst 不一定是新内存:录制过程中 VkMat 一释放,内存就回到 VkBlobAllocator,同一个命令缓冲里的下一次分配可能拿到同一段,而之前用到它的命令这时还没执行。
把 allocator.cpp 里注释掉的 VkBlobAllocator M/F 日志打开,再在 record_clone 里打印 src 和 dst 的偏移,出问题的那段是这样的(每个 batch 先把 data 和 scale 各 clone 一份,跑 scale,再 clone 到输出):
dispatch 之间复用内存一般没问题:后一个 dispatch 读前一个的输出时会有 compute 到 compute 的 barrier,它的执行依赖覆盖了前面所有的 compute 命令。但 vkCmdCopyBuffer 在 transfer 阶段,这些 barrier 覆盖不到,所以两次 copy 落在同一段回收的内存上时,中间没有任何同步。
改动是在 record_clone 写 dst 之前加一个 buffer barrier,src 是 COMPUTE | TRANSFER 阶段的 SHADER_WRITE | TRANSFER_WRITE,dst 是 TRANSFER 阶段的 TRANSFER_WRITE。host 到 device 的 record_clone(const Mat&, VkMat&) 最后也走这里。record_clone(const VkImageMat&, VkMat&) 里是同样的写法,ncnn 里没有调用,就没有动。
改了以后同步校验不再报这两处。不开校验层时测试结果和改之前一样,只有 test_squeezenet 因为 #4742 的路径问题失败。在 MoltenVK 上没有看到输出出错,这个问题是校验层报出来的。