masahi commented on a change in pull request #7833:
URL: https://github.com/apache/tvm/pull/7833#discussion_r612869601
##########
File path: src/runtime/vulkan/vulkan.cc
##########
@@ -60,35 +91,21 @@ class VulkanThreadEntry {
pool.reset();
streams_.clear();
for (const auto& kv : staging_buffers_) {
- if (!kv.second) {
- continue;
- }
- auto& buf = *(kv.second);
- if (buf.host_addr != nullptr) {
- vkUnmapMemory(buf.device, buf.memory);
- }
- if (buf.memory != VK_NULL_HANDLE) {
- vkFreeMemory(buf.device, buf.memory, nullptr);
- }
- if (buf.buffer != VK_NULL_HANDLE) {
- vkDestroyBuffer(buf.device, buf.buffer, nullptr);
- }
+ DeleteHostVisibleBuffer(kv.second.get());
Review comment:
Yes it is likely a bug. The code actually uses the id to look up only
GPU context (`VulkanDeviceAPI`, see below), so it doesn't make sense to pass a
CPU device ID here. It was working just because both IDs are 0.
https://github.com/apache/tvm/blob/e8369881ef69a46eaafd143a1c9bfa0eba2eb003/src/runtime/vulkan/vulkan.cc#L1116-L1119
I've fixed above points and added sync after UBO realloc in
https://github.com/apache/tvm/pull/7833/commits/9df431ff4480fac3edd9422bdb4af2c34a12f5f8
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
