Store the SPIR-V binary of a vk::ShaderModule in an sw::SpirvBinary Previously we stored it as a raw block of memory, but SPIRV-Tools and SpirvShader expect an std::vector<uint32_t> so we would copy it on each access. This change eliminates the copy, and prepares for making the serial ID a part of SpirvBinary to improve pipeline cache reuse. Bug: b/172839674 Change-Id: I052bb10a35fb863f835e316e4f648b26575bc398 Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/58269 Kokoro-Result: kokoro <noreply+kokoro@google.com> Tested-by: Nicolas Capens <nicolascapens@google.com> Reviewed-by: Alexis Hétu <sugoi@google.com>
diff --git a/src/Vulkan/VkPipeline.cpp b/src/Vulkan/VkPipeline.cpp index 7fad7b5..168f6a3 100644 --- a/src/Vulkan/VkPipeline.cpp +++ b/src/Vulkan/VkPipeline.cpp
@@ -219,7 +219,7 @@ const bool optimize = !dbgctx; const ShaderModule *module = vk::Cast(pStage->module); - const PipelineCache::SpirvBinaryKey key(module->getCode(), pStage->pSpecializationInfo, optimize); + const PipelineCache::SpirvBinaryKey key(module->getBinary(), pStage->pSpecializationInfo, optimize); sw::SpirvBinary spirv; @@ -238,7 +238,7 @@ // use a new serial ID so the shader gets recompiled. uint32_t codeSerialID = (key.getSpecializationInfo() ? vk::ShaderModule::nextSerialID() : module->getSerialID()); - // TODO(b/119409619): use allocator. + // TODO(b/201798871): use allocator. auto shader = std::make_shared<sw::SpirvShader>(codeSerialID, pStage->stage, pStage->pName, spirv, vk::Cast(pCreateInfo->renderPass), pCreateInfo->subpass, robustBufferAccess, dbgctx); @@ -276,7 +276,7 @@ // instructions. const bool optimize = !dbgctx; - const PipelineCache::SpirvBinaryKey shaderKey(module->getCode(), stage.pSpecializationInfo, optimize); + const PipelineCache::SpirvBinaryKey shaderKey(module->getBinary(), stage.pSpecializationInfo, optimize); sw::SpirvBinary spirv; @@ -295,7 +295,7 @@ // use a new serial ID so the shader gets recompiled. uint32_t codeSerialID = (stage.pSpecializationInfo ? vk::ShaderModule::nextSerialID() : module->getSerialID()); - // TODO(b/119409619): use allocator. + // TODO(b/201798871): use allocator. shader = std::make_shared<sw::SpirvShader>(codeSerialID, stage.stage, stage.pName, spirv, nullptr, 0, robustBufferAccess, dbgctx);
diff --git a/src/Vulkan/VkShaderModule.cpp b/src/Vulkan/VkShaderModule.cpp index a49f824..b49151d 100644 --- a/src/Vulkan/VkShaderModule.cpp +++ b/src/Vulkan/VkShaderModule.cpp
@@ -24,29 +24,21 @@ ShaderModule::ShaderModule(const VkShaderModuleCreateInfo *pCreateInfo, void *mem) : serialID(nextSerialID()) - , code(reinterpret_cast<uint32_t *>(mem)) + , binary(pCreateInfo->pCode, pCreateInfo->codeSize / sizeof(uint32_t)) { - memcpy(code, pCreateInfo->pCode, pCreateInfo->codeSize); - wordCount = static_cast<uint32_t>(pCreateInfo->codeSize / sizeof(uint32_t)); - #if !defined(NDEBUG) || defined(DCHECK_ALWAYS_ON) spvtools::SpirvTools spirvTools(SPIRV_VERSION); spvtools::ValidatorOptions validatorOptions = {}; validatorOptions.SetScalarBlockLayout(true); // VK_EXT_scalar_block_layout validatorOptions.SetUniformBufferStandardLayout(true); // VK_KHR_uniform_buffer_standard_layout - ASSERT(spirvTools.Validate(code, wordCount, validatorOptions)); // The SPIR-V code passed to vkCreateShaderModule must be valid (b/158228522) + ASSERT(spirvTools.Validate(binary.data(), binary.size(), validatorOptions)); // The SPIR-V code passed to vkCreateShaderModule must be valid (b/158228522) #endif } -void ShaderModule::destroy(const VkAllocationCallbacks *pAllocator) -{ - vk::freeHostMemory(code, pAllocator); -} - size_t ShaderModule::ComputeRequiredAllocationSize(const VkShaderModuleCreateInfo *pCreateInfo) { - return pCreateInfo->codeSize; + return 0; } } // namespace vk
diff --git a/src/Vulkan/VkShaderModule.hpp b/src/Vulkan/VkShaderModule.hpp index 2081f07..611aebf 100644 --- a/src/Vulkan/VkShaderModule.hpp +++ b/src/Vulkan/VkShaderModule.hpp
@@ -19,11 +19,6 @@ #include "Pipeline/SpirvBinary.hpp" #include <atomic> -#include <vector> - -namespace rr { -class Routine; -} namespace vk { @@ -31,12 +26,9 @@ { public: ShaderModule(const VkShaderModuleCreateInfo *pCreateInfo, void *mem); - void destroy(const VkAllocationCallbacks *pAllocator); static size_t ComputeRequiredAllocationSize(const VkShaderModuleCreateInfo *pCreateInfo); - // TODO: reconsider boundary of ShaderModule class; try to avoid 'expose the - // guts' operations, and this copy. - sw::SpirvBinary getCode() const { return sw::SpirvBinary(code, wordCount); } + const sw::SpirvBinary &getBinary() const { return binary; } uint32_t getSerialID() const { return serialID; } static uint32_t nextSerialID() { return serialCounter++; } @@ -45,8 +37,7 @@ const uint32_t serialID; static std::atomic<uint32_t> serialCounter; - uint32_t *code = nullptr; - uint32_t wordCount = 0; + sw::SpirvBinary binary; }; static inline ShaderModule *Cast(VkShaderModule object)