[vulkan] Simplify vk::Semaphore implementation.
This CL simplifies the implementation of vk::Semaphore by getting
rid of vk::Semaphore::Impl (moving its fields into vk::Semaphore
itself).
This requires a minor change to the vk::Semaphore constructor which
now takes an allocator parameter. The latter is used to allocate
the "External" instance on demand. Before the CL, said instance
was 'allocated' from a fixed storage area inside the Impl class.
Note that this doesn't change the external semaphore's behaviour.
In particular, only one implementation can live in the source tree
at the moment, while the Vulkan specification makes it clear that
it should be possible to export a single VkSemaphore to several types
of platform-specific handles (though this doesn't seem to be tested
by the Vulkan-CTS). This last issue will be addressed in a future CL.
Bug: 140421736
Change-Id: I3610d9e7e8cb8e49368b658d157408cbd23ee6db
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/39052
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Tested-by: David Turner <digit@google.com>
diff --git a/src/Vulkan/VkSemaphore.cpp b/src/Vulkan/VkSemaphore.cpp
index c46e399..f4b9062 100644
--- a/src/Vulkan/VkSemaphore.cpp
+++ b/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();
- }
-
- // Deallocate the External semaphore if any.
- void deallocateExternal()
- {
- if(external)
- {
- external->~External();
- external = nullptr;
- }
- }
-
- // 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
- {
- waitInternal();
- }
- }
-
- void signal()
- {
- 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();
- }
- }
-
-private:
- // Necessary to make ::importXXX() and ::exportXXX() simpler.
- friend Semaphore;
-
- void 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.
- }
-
- void signalInternal()
- {
- // Signal the marl condition variable only.
- std::unique_lock<std::mutex> lock(mutex);
- if(!signaled)
- {
- signaled = true;
- condition.notify_one();
- }
- }
-
- // 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;
-
- alignas(External) char externalStorage[sizeof(External)];
};
-Semaphore::Semaphore(const VkSemaphoreCreateInfo *pCreateInfo, void *mem)
-{
- impl = new(mem) Impl(pCreateInfo);
-}
-
-void Semaphore::destroy(const VkAllocationCallbacks *pAllocator)
-{
- impl->~Impl();
- vk::deallocate(impl, pAllocator);
-}
-
-size_t Semaphore::ComputeRequiredAllocationSize(const VkSemaphoreCreateInfo *pCreateInfo)
-{
- return sizeof(Semaphore::Impl);
-}
+} // namespace
void Semaphore::wait()
{
- impl->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
+ {
+ waitInternal();
+ }
}
void Semaphore::signal()
{
- impl->signal();
-}
-
-#if SWIFTSHADER_EXTERNAL_SEMAPHORE_OPAQUE_FD
-VkResult Semaphore::importFd(int fd, bool temporaryImport)
-{
- std::unique_lock<std::mutex> lock(impl->mutex);
- if(!impl->external)
+ if(external)
{
- impl->allocateExternalNoInit();
- }
- VkResult result = impl->external->importFd(fd);
- if(result != VK_SUCCESS)
- {
- impl->deallocateExternal();
+ // Assumes that signalling an external semaphore is non-blocking,
+ // so it can be performed directly either from a fiber or thread.
+ external->signal();
}
else
{
- impl->temporaryImport = temporaryImport;
+ signalInternal();
+ }
+}
+
+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.
+}
+
+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, const VkAllocationCallbacks *pAllocator)
+ : allocator(pAllocator)
+{
+ SemaphoreCreateInfo info(pCreateInfo);
+ if(info.exportSemaphore)
+ {
+ allocateExternal();
+ external->init();
+ }
+}
+
+void Semaphore::destroy(const VkAllocationCallbacks *pAllocator)
+{
+ deallocateExternal();
+}
+
+size_t Semaphore::ComputeRequiredAllocationSize(const VkSemaphoreCreateInfo *pCreateInfo)
+{
+ // Semaphore::External instance is created and destroyed on demand so return 0 here.
+ return 0;
+}
+
+void Semaphore::allocateExternal()
+{
+ ASSERT(external == nullptr);
+ external = reinterpret_cast<Semaphore::External *>(
+ vk::allocate(sizeof(Semaphore::External), vk::REQUIRED_MEMORY_ALIGNMENT, allocator));
+ new(external) Semaphore::External();
+}
+
+void Semaphore::deallocateExternal()
+{
+ if(external)
+ {
+ vk::deallocate(external, allocator);
+ external = nullptr;
+ }
+}
+
+#if SWIFTSHADER_EXTERNAL_SEMAPHORE_OPAQUE_FD
+VkResult Semaphore::importFd(int fd, bool tempImport)
+{
+ std::unique_lock<std::mutex> lock(mutex);
+ if(!external)
+ {
+ allocateExternal();
+ }
+ VkResult result = external->importFd(fd);
+ if(result != VK_SUCCESS)
+ {
+ deallocateExternal();
+ }
+ else
+ {
+ 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
diff --git a/src/Vulkan/VkSemaphore.hpp b/src/Vulkan/VkSemaphore.hpp
index 257c9f5..18ebc71 100644
--- a/src/Vulkan/VkSemaphore.hpp
+++ b/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 @@
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 @@
#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)
diff --git a/src/Vulkan/libVulkan.cpp b/src/Vulkan/libVulkan.cpp
index 40d3412..8c5484d 100644
--- a/src/Vulkan/libVulkan.cpp
+++ b/src/Vulkan/libVulkan.cpp
@@ -1082,7 +1082,7 @@
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)
