Deep copy specialization info
Previously a shared_ptr<> was used to avoid copying specialization info,
but it had the downside of requiring an additional allocation for the
VkSpecializationInfo structure itself, and treated a null pointer
differently from a structure with zero entries.
We copy the specialization info three times: to create the pipeline
cache's lookup key, to copy it into the cache when a new entry is
created, and when running spvtools optimization passes that apply the
specialization constants. Only the second one was a shallow copy, so
this change which makes it a deep copy while avoiding additional
allocations and pointer indirections most probably has no measurable
negative effect.
This change also replaces the use of vk::allocate() with
sw::allocateUninitialized(). We immediately memcpy() the same amount of
data, so it doesn't stay uninitialized. Also note that while this does
not make proper use of allocator callbacks, the old code didn't either,
and the use of the functions in the sw namespace make it clearer that
this is an unaccounted internal allocation.
Bug: b/140991626
Bug: b/172839674
Change-Id: I5962e028bbb6ad44b6dc2874eae6636bca0115c3
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/57769
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Sean Risser <srisser@google.com>
diff --git a/src/Vulkan/VkPipelineCache.cpp b/src/Vulkan/VkPipelineCache.cpp
index 817ee5c..06de756 100644
--- a/src/Vulkan/VkPipelineCache.cpp
+++ b/src/Vulkan/VkPipelineCache.cpp
@@ -19,7 +19,7 @@
namespace vk {
PipelineCache::SpirvBinaryKey::SpirvBinaryKey(const sw::SpirvBinary &insns,
- const vk::SpecializationInfo &specializationInfo,
+ const VkSpecializationInfo *specializationInfo,
bool optimize)
: insns(insns)
, specializationInfo(specializationInfo)
diff --git a/src/Vulkan/VkPipelineCache.hpp b/src/Vulkan/VkPipelineCache.hpp
index 195b284..05e8cbc 100644
--- a/src/Vulkan/VkPipelineCache.hpp
+++ b/src/Vulkan/VkPipelineCache.hpp
@@ -56,7 +56,7 @@
struct SpirvBinaryKey
{
SpirvBinaryKey(const sw::SpirvBinary &insns,
- const vk::SpecializationInfo &specializationInfo,
+ const VkSpecializationInfo *specializationInfo,
bool optimize);
bool operator<(const SpirvBinaryKey &other) const;
diff --git a/src/Vulkan/VkSpecializationInfo.cpp b/src/Vulkan/VkSpecializationInfo.cpp
index b20a6a0..dd56b17 100644
--- a/src/Vulkan/VkSpecializationInfo.cpp
+++ b/src/Vulkan/VkSpecializationInfo.cpp
@@ -12,88 +12,66 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-#include "VkPipelineCache.hpp"
+#include "VkSpecializationInfo.hpp"
+
+#include "System/Memory.hpp"
+
#include <cstring>
namespace vk {
SpecializationInfo::SpecializationInfo(const VkSpecializationInfo *specializationInfo)
{
- if(specializationInfo)
+ if(specializationInfo && specializationInfo->mapEntryCount > 0)
{
- auto *ptr = reinterpret_cast<VkSpecializationInfo *>(
- allocate(sizeof(VkSpecializationInfo), REQUIRED_MEMORY_ALIGNMENT, DEVICE_MEMORY));
- info = std::shared_ptr<VkSpecializationInfo>(ptr, Deleter());
+ info.mapEntryCount = specializationInfo->mapEntryCount;
+ size_t entriesSize = specializationInfo->mapEntryCount * sizeof(VkSpecializationMapEntry);
+ void *mapEntries = sw::allocateUninitialized(entriesSize);
+ memcpy(mapEntries, specializationInfo->pMapEntries, entriesSize);
+ info.pMapEntries = reinterpret_cast<VkSpecializationMapEntry *>(mapEntries);
- info->mapEntryCount = specializationInfo->mapEntryCount;
- info->pMapEntries = nullptr;
- info->dataSize = specializationInfo->dataSize;
- info->pData = nullptr;
-
- if(specializationInfo->mapEntryCount > 0)
- {
- size_t entriesSize = specializationInfo->mapEntryCount * sizeof(VkSpecializationMapEntry);
- VkSpecializationMapEntry *mapEntries = reinterpret_cast<VkSpecializationMapEntry *>(
- allocate(entriesSize, REQUIRED_MEMORY_ALIGNMENT, DEVICE_MEMORY));
- memcpy(mapEntries, specializationInfo->pMapEntries, entriesSize);
- info->pMapEntries = mapEntries;
- }
-
- if(specializationInfo->dataSize > 0)
- {
- void *data = allocate(specializationInfo->dataSize, REQUIRED_MEMORY_ALIGNMENT, DEVICE_MEMORY);
- memcpy(data, specializationInfo->pData, specializationInfo->dataSize);
- info->pData = data;
- }
+ info.dataSize = specializationInfo->dataSize;
+ void *data = sw::allocateUninitialized(specializationInfo->dataSize);
+ memcpy(data, specializationInfo->pData, specializationInfo->dataSize);
+ info.pData = data;
}
}
-void SpecializationInfo::Deleter::operator()(VkSpecializationInfo *info) const
+SpecializationInfo::SpecializationInfo(const SpecializationInfo ©)
+ : SpecializationInfo(©.info)
{
- if(info)
- {
- deallocate(const_cast<VkSpecializationMapEntry *>(info->pMapEntries), DEVICE_MEMORY);
- deallocate(const_cast<void *>(info->pData), DEVICE_MEMORY);
- deallocate(info, DEVICE_MEMORY);
- }
}
-bool SpecializationInfo::operator<(const SpecializationInfo &specializationInfo) const
+SpecializationInfo::~SpecializationInfo()
{
- // Check that either both or neither keys have specialization info.
- if((info.get() == nullptr) != (specializationInfo.info.get() == nullptr))
+ sw::deallocate(const_cast<VkSpecializationMapEntry *>(info.pMapEntries));
+ sw::deallocate(const_cast<void *>(info.pData));
+}
+
+bool SpecializationInfo::operator<(const SpecializationInfo &rhs) const
+{
+ if(info.mapEntryCount != rhs.info.mapEntryCount)
{
- return info.get() == nullptr;
+ return info.mapEntryCount < rhs.info.mapEntryCount;
}
- if(!info)
+ if(info.dataSize != rhs.info.dataSize)
{
- ASSERT(!specializationInfo.info);
- return false;
+ return info.dataSize < rhs.info.dataSize;
}
- if(info->mapEntryCount != specializationInfo.info->mapEntryCount)
+ if(info.mapEntryCount > 0)
{
- return info->mapEntryCount < specializationInfo.info->mapEntryCount;
- }
-
- if(info->dataSize != specializationInfo.info->dataSize)
- {
- return info->dataSize < specializationInfo.info->dataSize;
- }
-
- if(info->mapEntryCount > 0)
- {
- int cmp = memcmp(info->pMapEntries, specializationInfo.info->pMapEntries, info->mapEntryCount * sizeof(VkSpecializationMapEntry));
+ int cmp = memcmp(info.pMapEntries, rhs.info.pMapEntries, info.mapEntryCount * sizeof(VkSpecializationMapEntry));
if(cmp != 0)
{
return cmp < 0;
}
}
- if(info->dataSize > 0)
+ if(info.dataSize > 0)
{
- int cmp = memcmp(info->pData, specializationInfo.info->pData, info->dataSize);
+ int cmp = memcmp(info.pData, rhs.info.pData, info.dataSize);
if(cmp != 0)
{
return cmp < 0;
diff --git a/src/Vulkan/VkSpecializationInfo.hpp b/src/Vulkan/VkSpecializationInfo.hpp
index 0fdd43d..1e8f6e1 100644
--- a/src/Vulkan/VkSpecializationInfo.hpp
+++ b/src/Vulkan/VkSpecializationInfo.hpp
@@ -24,18 +24,19 @@
struct SpecializationInfo
{
SpecializationInfo(const VkSpecializationInfo *specializationInfo);
+ SpecializationInfo(const SpecializationInfo ©);
+
+ ~SpecializationInfo();
bool operator<(const SpecializationInfo &specializationInfo) const;
- const VkSpecializationInfo *get() const { return info.get(); }
+ const VkSpecializationInfo *get() const
+ {
+ return (info.mapEntryCount > 0) ? &info : nullptr;
+ }
private:
- struct Deleter
- {
- void operator()(VkSpecializationInfo *) const;
- };
-
- std::shared_ptr<VkSpecializationInfo> info;
+ VkSpecializationInfo info = {};
};
} // namespace vk
