commit 14fca80f683e8f33768f245d4141cbb96e4f1801
author Nicolas Capens <capn@google.com> Fri Oct 15 15:30:24 2021 -0400
committer Nicolas Capens <nicolascapens@google.com> Tue Oct 19 04:47:53 2021 +0000
tree 5e9042cec673763b8311b38b8112c9d31ce77541
parent 7997cbc34bb2d5db771f0d0b2497aeb665168ed2

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>

src/Vulkan/VkPipeline.cpp

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);
 

src/Vulkan/VkShaderModule.cpp

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

src/Vulkan/VkShaderModule.hpp

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)