[vulkan] Simplify vk::Semaphore implementation.

src/Vulkan/VkSemaphore.cpp

@@ -33,229 +33,197 @@
 
 #include <functional>
 #include <memory>
-#include <mutex>
 #include <utility>
 
 namespace vk {
 
-// An implementation of VkSemaphore based on Marl primitives.
-class Semaphore::Impl
+namespace {
+
+struct SemaphoreCreateInfo
 {
-public:
+	bool exportSemaphore = false;
+
 	// Create a new instance. The external instance will be allocated only
 	// the pCreateInfo->pNext chain indicates it needs to be exported.
-	Impl(const VkSemaphoreCreateInfo *pCreateInfo)
+	SemaphoreCreateInfo(const VkSemaphoreCreateInfo *pCreateInfo)
 	{
-		bool exportSemaphore = false;
 		for(const auto *nextInfo = reinterpret_cast<const VkBaseInStructure *>(pCreateInfo->pNext);
 		    nextInfo != nullptr; nextInfo = nextInfo->pNext)
 		{
 			if(nextInfo->sType == VK_STRUCTURE_TYPE_EXPORT_SEMAPHORE_CREATE_INFO)
 			{
 				const auto *exportInfo = reinterpret_cast<const VkExportSemaphoreCreateInfo *>(nextInfo);
-				if(exportInfo->handleTypes != External::kExternalSemaphoreHandleType)
+				exportSemaphore = true;
+				if(exportInfo->handleTypes != Semaphore::External::kExternalSemaphoreHandleType)
 				{
 					UNIMPLEMENTED("exportInfo->handleTypes");
 				}
-				exportSemaphore = true;
 				break;
 			}
 		}
-
-		if(exportSemaphore)
-		{
-			allocateExternalNoInit();
-			external->init();
-		}
 	}
+};
 
-	~Impl()
-	{
-		deallocateExternal();
-	}
+}  // namespace
 
-	// Deallocate the External semaphore if any.
-	void deallocateExternal()
+void Semaphore::wait()
+{
+	if(external)
 	{
-		if(external)
+		if(!external->tryWait())
 		{
-			external->~External();
-			external = nullptr;
+			// Dispatch the external wait to a background thread.
+			// Even if this creates a new thread on each
+			// call, it is assumed that this is negligible
+			// compared with the actual semaphore wait()
+			// operation.
+			marl::blocking_call([this]() {
+				external->wait();
+			});
 		}
-	}
 
-	// Allocate the external semaphore.
-	// Note that this does not allocate the internal resource, which must be
-	// performed by calling external->init(), or importing one using
-	// a platform-specific external->importXXX(...) method.
-	void allocateExternalNoInit()
-	{
-		external = new(externalStorage) External();
-	}
-
-	void wait()
-	{
-		if(external)
-		{
-			if(!external->tryWait())
-			{
-				// Dispatch the external wait to a background thread.
-				// Even if this creates a new thread on each
-				// call, it is assumed that this is negligible
-				// compared with the actual semaphore wait()
-				// operation.
-				marl::blocking_call([this]() {
-					external->wait();
-				});
-			}
-
-			// If the import was temporary, reset the semaphore to its
-			// permanent state by getting rid of |external|.
-			// See "6.4.5. Importing Semaphore Payloads" in Vulkan 1.1 spec.
-			if(temporaryImport)
-			{
-				deallocateExternal();
-				temporaryImport = false;
-			}
-		}
-		else
+		// If the import was temporary, reset the semaphore to its
+		// permanent state by getting rid of |external|.
+		// See "6.4.5. Importing Semaphore Payloads" in Vulkan 1.1 spec.
+		if(temporaryImport)
 		{
-			waitInternal();
+			deallocateExternal();
+			temporaryImport = false;
 		}
 	}
-
-	void signal()
+	else
 	{
-		if(external)
-		{
-			// Assumes that signalling an external semaphore is non-blocking,
-			// so it can be performed directly either from a fiber or thread.
-			external->signal();
-		}
-		else
-		{
-			signalInternal();
-		}
+		waitInternal();
 	}
+}
 
-private:
-	// Necessary to make ::importXXX() and ::exportXXX() simpler.
-	friend Semaphore;
-
-	void waitInternal()
+void Semaphore::signal()
+{
+	if(external)
 	{
-		// Wait on the marl condition variable only.
-		std::unique_lock<std::mutex> lock(mutex);
-		condition.wait(lock, [this] { return this->signaled; });
-		signaled = false;  // Vulkan requires resetting after waiting.
+		// Assumes that signalling an external semaphore is non-blocking,
+		// so it can be performed directly either from a fiber or thread.
+		external->signal();
 	}
-
-	void signalInternal()
+	else
 	{
-		// Signal the marl condition variable only.
-		std::unique_lock<std::mutex> lock(mutex);
-		if(!signaled)
-		{
-			signaled = true;
-			condition.notify_one();
-		}
+		signalInternal();
 	}
+}
 
-	// Implementation of a non-external semaphore based on Marl.
-	std::mutex mutex;
-	marl::ConditionVariable condition;
-	bool signaled = false;
-
-	// Optional external semaphore data might be referenced and stored here.
-	External *external = nullptr;
-
-	// Set to true if |external| comes from a temporary import.
-	bool temporaryImport = false;
+void Semaphore::waitInternal()
+{
+	// Wait on the marl condition variable only.
+	std::unique_lock<std::mutex> lock(mutex);
+	condition.wait(lock, [this] { return this->signaled; });
+	signaled = false;  // Vulkan requires resetting after waiting.
+}
 
-	alignas(External) char externalStorage[sizeof(External)];
-};
+void Semaphore::signalInternal()
+{
+	// Signal the marl condition variable only.
+	std::unique_lock<std::mutex> lock(mutex);
+	if(!signaled)
+	{
+		signaled = true;
+		condition.notify_one();
+	}
+}
 
-Semaphore::Semaphore(const VkSemaphoreCreateInfo *pCreateInfo, void *mem)
+Semaphore::Semaphore(const VkSemaphoreCreateInfo *pCreateInfo, void *mem, const VkAllocationCallbacks *pAllocator)
+    : allocator(pAllocator)
 {
-	impl = new(mem) Impl(pCreateInfo);
+	SemaphoreCreateInfo info(pCreateInfo);
+	if(info.exportSemaphore)
+	{
+		allocateExternal();
+		external->init();
+	}
 }
 
 void Semaphore::destroy(const VkAllocationCallbacks *pAllocator)
 {
-	impl->~Impl();
-	vk::deallocate(impl, pAllocator);
+	deallocateExternal();
 }
 
 size_t Semaphore::ComputeRequiredAllocationSize(const VkSemaphoreCreateInfo *pCreateInfo)
 {
-	return sizeof(Semaphore::Impl);
+	// Semaphore::External instance is created and destroyed on demand so return 0 here.
+	return 0;
 }
 
-void Semaphore::wait()
+void Semaphore::allocateExternal()
 {
-	impl->wait();
+	ASSERT(external == nullptr);
+	external = reinterpret_cast<Semaphore::External *>(
+	    vk::allocate(sizeof(Semaphore::External), vk::REQUIRED_MEMORY_ALIGNMENT, allocator));
+	new(external) Semaphore::External();
 }
 
-void Semaphore::signal()
+void Semaphore::deallocateExternal()
 {
-	impl->signal();
+	if(external)
+	{
+		vk::deallocate(external, allocator);
+		external = nullptr;
+	}
 }
 
 #if SWIFTSHADER_EXTERNAL_SEMAPHORE_OPAQUE_FD
-VkResult Semaphore::importFd(int fd, bool temporaryImport)
+VkResult Semaphore::importFd(int fd, bool tempImport)
 {
-	std::unique_lock<std::mutex> lock(impl->mutex);
-	if(!impl->external)
+	std::unique_lock<std::mutex> lock(mutex);
+	if(!external)
 	{
-		impl->allocateExternalNoInit();
+		allocateExternal();
 	}
-	VkResult result = impl->external->importFd(fd);
+	VkResult result = external->importFd(fd);
 	if(result != VK_SUCCESS)
 	{
-		impl->deallocateExternal();
+		deallocateExternal();
 	}
 	else
 	{
-		impl->temporaryImport = temporaryImport;
+		temporaryImport = tempImport;
 	}
 	return result;
 }
 
-VkResult Semaphore::exportFd(int *pFd) const
+VkResult Semaphore::exportFd(int *pFd)
 {
-	std::unique_lock<std::mutex> lock(impl->mutex);
-	if(!impl->external)
+	std::unique_lock<std::mutex> lock(mutex);
+	if(!external)
 	{
 		TRACE("Cannot export non-external semaphore");
 		return VK_ERROR_INVALID_EXTERNAL_HANDLE;
 	}
-	return impl->external->exportFd(pFd);
+	return external->exportFd(pFd);
 }
 #endif  // SWIFTSHADER_EXTERNAL_SEMAPHORE_OPAQUE_FD
 
 #if VK_USE_PLATFORM_FUCHSIA
-VkResult Semaphore::importHandle(zx_handle_t handle, bool temporaryImport)
+VkResult Semaphore::importHandle(zx_handle_t handle, bool tempImport)
 {
-	std::unique_lock<std::mutex> lock(impl->mutex);
-	if(!impl->external)
+	std::unique_lock<std::mutex> lock(mutex);
+	if(!external)
 	{
-		impl->allocateExternalNoInit();
+		allocateExternal();
 	}
 	// NOTE: Imports are just moving a handle so cannot fail.
-	impl->external->importHandle(handle);
-	impl->temporaryImport = temporaryImport;
+	external->importHandle(handle);
+	temporaryImport = tempImport;
 	return VK_SUCCESS;
 }
 
-VkResult Semaphore::exportHandle(zx_handle_t *pHandle) const
+VkResult Semaphore::exportHandle(zx_handle_t *pHandle)
 {
-	std::unique_lock<std::mutex> lock(impl->mutex);
-	if(!impl->external)
+	std::unique_lock<std::mutex> lock(mutex);
+	if(!external)
 	{
 		TRACE("Cannot export non-external semaphore");
 		return VK_ERROR_INVALID_EXTERNAL_HANDLE;
 	}
-	return impl->external->exportHandle(pHandle);
+	return external->exportHandle(pHandle);
 }
 #endif  // VK_USE_PLATFORM_FUCHSIA
 

src/Vulkan/VkSemaphore.hpp

@@ -18,6 +18,9 @@
 #include "VkConfig.h"
 #include "VkObject.hpp"
 
+#include "marl/conditionvariable.h"
+#include <mutex>
+
 #if VK_USE_PLATFORM_FUCHSIA
 #	include <zircon/types.h>
 #endif
@@ -27,7 +30,7 @@ namespace vk {
 class Semaphore : public Object<Semaphore, VkSemaphore>
 {
 public:
-	Semaphore(const VkSemaphoreCreateInfo *pCreateInfo, void *mem);
+	Semaphore(const VkSemaphoreCreateInfo *pCreateInfo, void *mem, const VkAllocationCallbacks *pAllocator);
 	void destroy(const VkAllocationCallbacks *pAllocator);
 
 	static size_t ComputeRequiredAllocationSize(const VkSemaphoreCreateInfo *pCreateInfo);
@@ -44,18 +47,30 @@ public:
 
 #if SWIFTSHADER_EXTERNAL_SEMAPHORE_OPAQUE_FD
 	VkResult importFd(int fd, bool temporaryImport);
-	VkResult exportFd(int *pFd) const;
+	VkResult exportFd(int *pFd);
 #endif
 
 #if VK_USE_PLATFORM_FUCHSIA
 	VkResult importHandle(zx_handle_t handle, bool temporaryImport);
-	VkResult exportHandle(zx_handle_t *pHandle) const;
+	VkResult exportHandle(zx_handle_t *pHandle);
 #endif
 
-private:
 	class External;
-	class Impl;
-	Impl *impl = nullptr;
+
+private:
+	void waitInternal();
+	void signalInternal();
+
+	void allocateExternal();
+	void deallocateExternal();
+
+	const VkAllocationCallbacks *allocator = nullptr;
+	std::mutex mutex;
+	marl::ConditionVariable condition;
+	bool signaled = false;
+
+	External *external = nullptr;
+	bool temporaryImport = false;
 };
 
 static inline Semaphore *Cast(VkSemaphore object)

src/Vulkan/libVulkan.cpp

@@ -1082,7 +1082,7 @@ VKAPI_ATTR VkResult VKAPI_CALL vkCreateSemaphore(VkDevice device, const VkSemaph
 		UNIMPLEMENTED("pCreateInfo->flags");
 	}
 
-	return vk::Semaphore::Create(pAllocator, pCreateInfo, pSemaphore);
+	return vk::Semaphore::Create(pAllocator, pCreateInfo, pSemaphore, pAllocator);
 }
 
 VKAPI_ATTR void VKAPI_CALL vkDestroySemaphore(VkDevice device, VkSemaphore semaphore, const VkAllocationCallbacks *pAllocator)