Separate device memory allocations from host allocations
vk::allocate() was previously used for Vulkan object allocation,
internal 'host' memory allocations, and 'device' memory allocations. The
`DEVICE_MEMORY` constant was intended to help separate the latter from
the former two, but was being used for various host memory allocations.
Host memory allocations should all call the allocation callbacks
provided by the application, if any. To make it clearer where we're
currently not providing these callbacks, `DEVICE_MEMORY` has been
replaced by `NULL_ALLOCATION_CALLBACKS`. For true device memory
allocations, vk::allocateDeviceMemory() was added.
Note that this change does not alter the underlying sw::allocate*() call
used for either host or device memory allocations. Both still use zero-
initialized memory at this point.
Bug: b/140991626
Change-Id: I4224ad2597248af3dd8a5311da61361b434f32c3
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/57708
Tested-by: Nicolas Capens <nicolascapens@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
diff --git a/src/Device/Renderer.cpp b/src/Device/Renderer.cpp
index c1c52ad..2402337 100644
--- a/src/Device/Renderer.cpp
+++ b/src/Device/Renderer.cpp
@@ -174,12 +174,12 @@
void *Renderer::operator new(size_t size)
{
ASSERT(size == sizeof(Renderer)); // This operator can't be called from a derived class
- return vk::allocate(sizeof(Renderer), alignof(Renderer), vk::DEVICE_MEMORY, VK_SYSTEM_ALLOCATION_SCOPE_DEVICE);
+ return vk::allocate(sizeof(Renderer), alignof(Renderer), vk::NULL_ALLOCATION_CALLBACKS, VK_SYSTEM_ALLOCATION_SCOPE_DEVICE);
}
void Renderer::operator delete(void *mem)
{
- vk::deallocate(mem, vk::DEVICE_MEMORY);
+ vk::deallocate(mem, vk::NULL_ALLOCATION_CALLBACKS);
}
void Renderer::draw(const vk::GraphicsPipeline *pipeline, const vk::DynamicState &dynamicState, unsigned int count, int baseVertex,
diff --git a/src/Vulkan/VkCommandPool.cpp b/src/Vulkan/VkCommandPool.cpp
index a7d6fe2..e747df0 100644
--- a/src/Vulkan/VkCommandPool.cpp
+++ b/src/Vulkan/VkCommandPool.cpp
@@ -29,7 +29,7 @@
// Free command Buffers allocated in allocateCommandBuffers
for(auto commandBuffer : commandBuffers)
{
- vk::destroy(commandBuffer, DEVICE_MEMORY);
+ vk::destroy(commandBuffer, NULL_ALLOCATION_CALLBACKS);
}
}
@@ -42,9 +42,9 @@
{
for(uint32_t i = 0; i < commandBufferCount; i++)
{
- // FIXME (b/119409619): use an allocator here so we can control all memory allocations
+ // TODO(b/119409619): Allocate command buffers from the pool memory.
void *deviceMemory = vk::allocate(sizeof(DispatchableCommandBuffer), REQUIRED_MEMORY_ALIGNMENT,
- DEVICE_MEMORY, DispatchableCommandBuffer::GetAllocationScope());
+ NULL_ALLOCATION_CALLBACKS, DispatchableCommandBuffer::GetAllocationScope());
ASSERT(deviceMemory);
DispatchableCommandBuffer *commandBuffer = new(deviceMemory) DispatchableCommandBuffer(device, level);
if(commandBuffer)
@@ -55,12 +55,14 @@
{
for(uint32_t j = 0; j < i; j++)
{
- vk::destroy(pCommandBuffers[j], DEVICE_MEMORY);
+ vk::destroy(pCommandBuffers[j], NULL_ALLOCATION_CALLBACKS);
}
+
for(uint32_t j = 0; j < commandBufferCount; j++)
{
pCommandBuffers[j] = VK_NULL_HANDLE;
}
+
return VK_ERROR_OUT_OF_DEVICE_MEMORY;
}
}
@@ -72,10 +74,10 @@
void CommandPool::freeCommandBuffers(uint32_t commandBufferCount, const VkCommandBuffer *pCommandBuffers)
{
- for(uint32_t i = 0; i < commandBufferCount; ++i)
+ for(uint32_t i = 0; i < commandBufferCount; i++)
{
commandBuffers.erase(pCommandBuffers[i]);
- vk::destroy(pCommandBuffers[i], DEVICE_MEMORY);
+ vk::destroy(pCommandBuffers[i], NULL_ALLOCATION_CALLBACKS);
}
}
diff --git a/src/Vulkan/VkDeviceMemory.cpp b/src/Vulkan/VkDeviceMemory.cpp
index 20fc6cd..de386dd 100644
--- a/src/Vulkan/VkDeviceMemory.cpp
+++ b/src/Vulkan/VkDeviceMemory.cpp
@@ -13,13 +13,13 @@
// limitations under the License.
#include "VkDeviceMemory.hpp"
+
#include "VkBuffer.hpp"
+#include "VkConfig.hpp"
#include "VkDevice.hpp"
#include "VkImage.hpp"
#include "VkStringify.hpp"
-#include "VkConfig.hpp"
-
// Host-allocated memory and host-mapped foreign memory
class ExternalMemoryHost : public vk::DeviceMemory, public vk::ObjectBase<ExternalMemoryHost, VkDeviceMemory>
{
@@ -374,7 +374,7 @@
// and sets |*pBuffer|.
VkResult DeviceMemory::allocate(size_t size, void **pBuffer)
{
- buffer = vk::allocate(size, REQUIRED_MEMORY_ALIGNMENT, DEVICE_MEMORY);
+ buffer = vk::allocateDeviceMemory(size, REQUIRED_MEMORY_ALIGNMENT);
if(!buffer)
{
return VK_ERROR_OUT_OF_DEVICE_MEMORY;
@@ -387,7 +387,7 @@
// Deallocate previously allocated memory at |buffer|.
void DeviceMemory::deallocate(void *buffer, size_t size)
{
- vk::deallocate(buffer, DEVICE_MEMORY);
+ vk::deallocateDeviceMemory(buffer);
buffer = nullptr;
}
diff --git a/src/Vulkan/VkDeviceMemoryExternalAndroid.cpp b/src/Vulkan/VkDeviceMemoryExternalAndroid.cpp
index 9085814..0b9b2fb 100644
--- a/src/Vulkan/VkDeviceMemoryExternalAndroid.cpp
+++ b/src/Vulkan/VkDeviceMemoryExternalAndroid.cpp
@@ -452,14 +452,14 @@
VkImage Image;
- result = vk::Image::Create(vk::DEVICE_MEMORY, &info, &Image, vk::Cast(device));
+ result = vk::Image::Create(vk::NULL_ALLOCATION_CALLBACKS, &info, &Image, vk::Cast(device));
if(result != VK_SUCCESS)
{
return result;
}
pProperties->allocationSize = vk::Cast(Image)->getMemoryRequirements().size;
- vk::destroy(Image, vk::DEVICE_MEMORY);
+ vk::destroy(Image, vk::NULL_ALLOCATION_CALLBACKS);
}
return result;
diff --git a/src/Vulkan/VkMemory.cpp b/src/Vulkan/VkMemory.cpp
index 3ff3b45..2b8518e 100644
--- a/src/Vulkan/VkMemory.cpp
+++ b/src/Vulkan/VkMemory.cpp
@@ -22,6 +22,18 @@
namespace vk {
+void *allocateDeviceMemory(size_t count, size_t alignment)
+{
+ // TODO(b/140991626): Use allocateZeroOrPoison() instead of allocateZero() to detect MemorySanitizer errors.
+ // TODO(b/140991626): Use allocateUninitialized() instead of allocateZeroOrPoison() to improve startup peformance.
+ return sw::allocateZero(count, alignment);
+}
+
+void deallocateDeviceMemory(void *ptr)
+{
+ sw::deallocate(ptr);
+}
+
void *allocate(size_t count, size_t alignment, const VkAllocationCallbacks *pAllocator, VkSystemAllocationScope allocationScope)
{
// TODO(b/140991626): Use allocateZeroOrPoison() instead of allocateZero() to detect MemorySanitizer errors.
diff --git a/src/Vulkan/VkMemory.hpp b/src/Vulkan/VkMemory.hpp
index f3108ac..91d0ae3 100644
--- a/src/Vulkan/VkMemory.hpp
+++ b/src/Vulkan/VkMemory.hpp
@@ -19,14 +19,23 @@
namespace vk {
-void *allocate(size_t count, size_t alignment, const VkAllocationCallbacks *pAllocator,
- VkSystemAllocationScope allocationScope = VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
+// TODO(b/192449828): Pass VkDeviceDeviceMemoryReportCreateInfoEXT into these functions to
+// centralize device memory report callback usage.
+void *allocateDeviceMemory(size_t bytes, size_t alignment);
+void deallocateDeviceMemory(void *ptr);
+
+// TODO(b/201798871): Fix host allocation callback usage. Uses of this symbolic constant indicate
+// places where we should use an allocator instead of unaccounted memory allocations.
+constexpr VkAllocationCallbacks *NULL_ALLOCATION_CALLBACKS = nullptr;
+
+void *allocate(size_t bytes, size_t alignment, const VkAllocationCallbacks *pAllocator,
+ VkSystemAllocationScope allocationScope);
void deallocate(void *ptr, const VkAllocationCallbacks *pAllocator);
template<typename T>
-T *allocate(size_t count, const VkAllocationCallbacks *pAllocator)
+T *allocate(size_t bytes, const VkAllocationCallbacks *pAllocator)
{
- return static_cast<T *>(allocate(count, alignof(T), pAllocator, T::GetAllocationScope()));
+ return static_cast<T *>(allocate(bytes, alignof(T), pAllocator, T::GetAllocationScope()));
}
} // namespace vk
diff --git a/src/Vulkan/VkObject.hpp b/src/Vulkan/VkObject.hpp
index dece90d..5019387 100644
--- a/src/Vulkan/VkObject.hpp
+++ b/src/Vulkan/VkObject.hpp
@@ -40,9 +40,6 @@
return { static_cast<uint64_t>(reinterpret_cast<uintptr_t>(object)) };
}
-// For use in the placement new to make it verbose that we're allocating an object using device memory
-static constexpr VkAllocationCallbacks *DEVICE_MEMORY = nullptr;
-
template<typename T, typename VkT, typename CreateInfo, typename... ExtendedInfo>
static VkResult Create(const VkAllocationCallbacks *pAllocator, const CreateInfo *pCreateInfo, VkT *outObject, ExtendedInfo... extendedInfo)
{
diff --git a/src/Vulkan/VkPipelineCache.hpp b/src/Vulkan/VkPipelineCache.hpp
index 05e8cbc..9a60954 100644
--- a/src/Vulkan/VkPipelineCache.hpp
+++ b/src/Vulkan/VkPipelineCache.hpp
@@ -44,6 +44,8 @@
class PipelineCache : public Object<PipelineCache, VkPipelineCache>
{
public:
+ static constexpr VkSystemAllocationScope GetAllocationScope() { return VK_SYSTEM_ALLOCATION_SCOPE_CACHE; }
+
PipelineCache(const VkPipelineCacheCreateInfo *pCreateInfo, void *mem);
virtual ~PipelineCache();
void destroy(const VkAllocationCallbacks *pAllocator);
diff --git a/src/Vulkan/VkQueue.cpp b/src/Vulkan/VkQueue.cpp
index 918b467..6b3ceb1 100644
--- a/src/Vulkan/VkQueue.cpp
+++ b/src/Vulkan/VkQueue.cpp
@@ -62,7 +62,7 @@
}
uint8_t *mem = static_cast<uint8_t *>(
- vk::allocate(totalSize, vk::REQUIRED_MEMORY_ALIGNMENT, vk::DEVICE_MEMORY, vk::Fence::GetAllocationScope()));
+ vk::allocate(totalSize, vk::REQUIRED_MEMORY_ALIGNMENT, vk::NULL_ALLOCATION_CALLBACKS, vk::Fence::GetAllocationScope()));
auto submits = new(mem) VkSubmitInfo[submitCount];
memcpy(mem, pSubmits, submitSize);
@@ -303,7 +303,7 @@
{
auto v = toDelete.tryTake();
if(!v.second) { break; }
- vk::deallocate(v.first, DEVICE_MEMORY);
+ vk::deallocate(v.first, NULL_ALLOCATION_CALLBACKS);
}
}
diff --git a/src/Vulkan/VkSemaphore.cpp b/src/Vulkan/VkSemaphore.cpp
index da46181..872f285 100644
--- a/src/Vulkan/VkSemaphore.cpp
+++ b/src/Vulkan/VkSemaphore.cpp
@@ -224,7 +224,7 @@
BinarySemaphore::External *BinarySemaphore::allocateExternal()
{
auto *ext = reinterpret_cast<BinarySemaphore::External *>(
- vk::allocate(sizeof(EXTERNAL), alignof(EXTERNAL), allocator));
+ vk::allocate(sizeof(EXTERNAL), alignof(EXTERNAL), allocator, VK_SYSTEM_ALLOCATION_SCOPE_OBJECT));
new(ext) EXTERNAL();
return ext;
}
