Avoid recompiling identical SPIR-V binaries
We previously took the serial ID from the unoptimized SPIR-V module, for
use as part of the Routine cache keys. If a pipeline is created with
specialization constants, we assumed them to produce unique optimized
SPIR-V binaries, regardless of their actual values, and obtained a new
serial ID each time. If a shader module was used with identical
specialization data, we would redundantly JIT-compile the same routine if
no other state used by the pipeline stage was different.
This change takes advantage of the fact that the pipeline cache doesn't
add a new entry (of optimized SPIR-V) for an already optimized module
unless the values of the specialization constants are different and
thus would produce a distinct binary (assuming said specialization
constants affect the code). A new serial ID is now associated with every
new SPIR-V binary. For a (pipeline) cache hit, the existing entry's
ID is used as part of the lookup key for the routine cache.
Note an application has to use a VkPipelineCache object to take
advantage of this.
When there's no pipeline cache, we still inherit the ID from the
unoptimized binary, to avoid Reactor routine recompiles.
Bug: b/172839674
Change-Id: I18b937800ea021235081bba0d827436265816414
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/57988
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
diff --git a/src/Device/PixelProcessor.cpp b/src/Device/PixelProcessor.cpp
index 617c0ad..fb492f9 100644
--- a/src/Device/PixelProcessor.cpp
+++ b/src/Device/PixelProcessor.cpp
@@ -91,7 +91,7 @@
if(fragmentShader)
{
- state.shaderID = fragmentShader->getSerialID();
+ state.shaderID = fragmentShader->getIdentifier();
state.pipelineLayoutIdentifier = pipelineState.getPipelineLayout()->identifier;
}
else
diff --git a/src/Device/VertexProcessor.cpp b/src/Device/VertexProcessor.cpp
index 68bdfc5..833fa6a 100644
--- a/src/Device/VertexProcessor.cpp
+++ b/src/Device/VertexProcessor.cpp
@@ -69,7 +69,7 @@
{
State state;
- state.shaderID = vertexShader->getSerialID();
+ state.shaderID = vertexShader->getIdentifier();
state.pipelineLayoutIdentifier = pipelineState.getPipelineLayout()->identifier;
state.robustBufferAccess = pipelineState.getRobustBufferAccess();
state.isPoint = pipelineState.getTopology() == VK_PRIMITIVE_TOPOLOGY_POINT_LIST;
diff --git a/src/Pipeline/BUILD.gn b/src/Pipeline/BUILD.gn
index d88e255..6b44f4b 100644
--- a/src/Pipeline/BUILD.gn
+++ b/src/Pipeline/BUILD.gn
@@ -40,6 +40,7 @@
"SamplerCore.cpp",
"SetupRoutine.cpp",
"ShaderCore.cpp",
+ "SpirvBinary.cpp",
"SpirvShader.cpp",
"SpirvShaderArithmetic.cpp",
"SpirvShaderControlFlow.cpp",
diff --git a/src/Pipeline/CMakeLists.txt b/src/Pipeline/CMakeLists.txt
index 4daf9e1..2b5b15c 100644
--- a/src/Pipeline/CMakeLists.txt
+++ b/src/Pipeline/CMakeLists.txt
@@ -33,6 +33,7 @@
ShaderCore.cpp
ShaderCore.hpp
SpirvBinary.hpp
+ SpirvBinary.cpp
SpirvID.hpp
SpirvShader.cpp
SpirvShader.hpp
diff --git a/src/Pipeline/SpirvBinary.cpp b/src/Pipeline/SpirvBinary.cpp
new file mode 100644
index 0000000..88ade4d
--- /dev/null
+++ b/src/Pipeline/SpirvBinary.cpp
@@ -0,0 +1,36 @@
+// Copyright 2021 The SwiftShader Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "SpirvBinary.hpp"
+
+namespace sw {
+
+std::atomic<uint32_t> SpirvBinary::serialCounter(1); // Start at 1, 0 is invalid shader.
+
+SpirvBinary::SpirvBinary()
+ : identifier(serialCounter++)
+{}
+
+SpirvBinary::SpirvBinary(const uint32_t *binary, uint32_t wordCount)
+ : std::vector<uint32_t>(binary, binary + wordCount)
+ , identifier(serialCounter++)
+{
+}
+
+void SpirvBinary::inheritIdentifier(const SpirvBinary &binary)
+{
+ identifier = binary.identifier;
+}
+
+} // namespace sw
\ No newline at end of file
diff --git a/src/Pipeline/SpirvBinary.hpp b/src/Pipeline/SpirvBinary.hpp
index b4766bb..9ff4536 100644
--- a/src/Pipeline/SpirvBinary.hpp
+++ b/src/Pipeline/SpirvBinary.hpp
@@ -1,4 +1,4 @@
-// Copyright 2018 The SwiftShader Authors. All Rights Reserved.
+// Copyright 2021 The SwiftShader Authors. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
@@ -15,6 +15,7 @@
#ifndef sw_SpirvBinary_hpp
#define sw_SpirvBinary_hpp
+#include <atomic>
#include <cstdint>
#include <vector>
@@ -23,12 +24,17 @@
class SpirvBinary : public std::vector<uint32_t>
{
public:
- SpirvBinary() = default;
+ SpirvBinary();
+ SpirvBinary(const uint32_t *binary, uint32_t wordCount);
- SpirvBinary(const uint32_t *binary, uint32_t wordCount)
- : std::vector<uint32_t>(binary, binary + wordCount)
- {
- }
+ inline uint32_t getIdentifier() const { return identifier; };
+
+ void inheritIdentifier(const SpirvBinary &binary);
+
+private:
+ static std::atomic<uint32_t> serialCounter;
+
+ uint32_t identifier;
};
} // namespace sw
diff --git a/src/Pipeline/SpirvShader.cpp b/src/Pipeline/SpirvShader.cpp
index 4d434ce..c56fd38 100644
--- a/src/Pipeline/SpirvShader.cpp
+++ b/src/Pipeline/SpirvShader.cpp
@@ -26,7 +26,6 @@
namespace sw {
SpirvShader::SpirvShader(
- uint32_t codeSerialID,
VkShaderStageFlagBits pipelineStage,
const char *entryPointName,
SpirvBinary const &insns,
@@ -37,7 +36,6 @@
: insns{ insns }
, inputs{ MAX_INTERFACE_COMPONENTS }
, outputs{ MAX_INTERFACE_COMPONENTS }
- , codeSerialID(codeSerialID)
, robustBufferAccess(robustBufferAccess)
{
ASSERT(insns.size() > 0);
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index 151e69d..9e5a42d 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -557,13 +557,12 @@
// This method is for retrieving an ID that uniquely identifies the
// shader entry point represented by this object.
- uint64_t getSerialID() const
+ uint64_t getIdentifier() const
{
- return ((uint64_t)entryPoint.value() << 32) | codeSerialID;
+ return ((uint64_t)entryPoint.value() << 32) | insns.getIdentifier();
}
- SpirvShader(uint32_t codeSerialID,
- VkShaderStageFlagBits stage,
+ SpirvShader(VkShaderStageFlagBits stage,
const char *entryPointName,
SpirvBinary const &insns,
const vk::RenderPass *renderPass,
@@ -835,7 +834,6 @@
WorkgroupMemory workgroupMemory;
private:
- const uint32_t codeSerialID;
const bool robustBufferAccess;
Function::ID entryPoint;
diff --git a/src/Vulkan/VkPipeline.cpp b/src/Vulkan/VkPipeline.cpp
index 168f6a3..9c06350 100644
--- a/src/Vulkan/VkPipeline.cpp
+++ b/src/Vulkan/VkPipeline.cpp
@@ -35,7 +35,7 @@
// optimizeSpirv() applies and freezes specializations into constants, and runs spirv-opt.
sw::SpirvBinary optimizeSpirv(const vk::PipelineCache::SpirvBinaryKey &key)
{
- const sw::SpirvBinary &code = key.getInsns();
+ const sw::SpirvBinary &code = key.getBinary();
const VkSpecializationInfo *specializationInfo = key.getSpecializationInfo();
bool optimize = key.getOptimization();
@@ -232,14 +232,17 @@
else
{
spirv = optimizeSpirv(key);
+
+ // If the pipeline does not have specialization constants, there's a 1-to-1 mapping between the unoptimized and optimized SPIR-V,
+ // so we should use a 1-to-1 mapping of the identifiers to avoid JIT routine recompiles.
+ if(!key.getSpecializationInfo())
+ {
+ spirv.inheritIdentifier(key.getBinary());
+ }
}
- // If the pipeline has specialization constants, assume they're unique and
- // use a new serial ID so the shader gets recompiled.
- uint32_t codeSerialID = (key.getSpecializationInfo() ? vk::ShaderModule::nextSerialID() : module->getSerialID());
-
// TODO(b/201798871): use allocator.
- auto shader = std::make_shared<sw::SpirvShader>(codeSerialID, pStage->stage, pStage->pName, spirv,
+ auto shader = std::make_shared<sw::SpirvShader>(pStage->stage, pStage->pName, spirv,
vk::Cast(pCreateInfo->renderPass), pCreateInfo->subpass, robustBufferAccess, dbgctx);
setShader(pStage->stage, shader);
@@ -289,17 +292,20 @@
else
{
spirv = optimizeSpirv(shaderKey);
+
+ // If the pipeline does not have specialization constants, there's a 1-to-1 mapping between the unoptimized and optimized SPIR-V,
+ // so we should use a 1-to-1 mapping of the identifiers to avoid JIT routine recompiles.
+ if(!shaderKey.getSpecializationInfo())
+ {
+ spirv.inheritIdentifier(shaderKey.getBinary());
+ }
}
- // If the pipeline has specialization constants, assume they're unique and
- // use a new serial ID so the shader gets recompiled.
- uint32_t codeSerialID = (stage.pSpecializationInfo ? vk::ShaderModule::nextSerialID() : module->getSerialID());
-
// TODO(b/201798871): use allocator.
- shader = std::make_shared<sw::SpirvShader>(codeSerialID, stage.stage, stage.pName, spirv,
+ shader = std::make_shared<sw::SpirvShader>(stage.stage, stage.pName, spirv,
nullptr, 0, robustBufferAccess, dbgctx);
- const PipelineCache::ComputeProgramKey programKey(shader->getSerialID(), layout->identifier);
+ const PipelineCache::ComputeProgramKey programKey(shader->getIdentifier(), layout->identifier);
if(pPipelineCache)
{
diff --git a/src/Vulkan/VkPipelineCache.cpp b/src/Vulkan/VkPipelineCache.cpp
index c4e9c31..c187632 100644
--- a/src/Vulkan/VkPipelineCache.cpp
+++ b/src/Vulkan/VkPipelineCache.cpp
@@ -18,10 +18,10 @@
namespace vk {
-PipelineCache::SpirvBinaryKey::SpirvBinaryKey(const sw::SpirvBinary &insns,
+PipelineCache::SpirvBinaryKey::SpirvBinaryKey(const sw::SpirvBinary &spirv,
const VkSpecializationInfo *specializationInfo,
bool optimize)
- : insns(insns)
+ : spirv(spirv)
, specializationInfo(specializationInfo)
, optimize(optimize)
{
@@ -29,12 +29,12 @@
bool PipelineCache::SpirvBinaryKey::operator<(const SpirvBinaryKey &other) const
{
- if(insns.size() != other.insns.size())
+ if(spirv.size() != other.spirv.size())
{
- return insns.size() < other.insns.size();
+ return spirv.size() < other.spirv.size();
}
- int cmp = memcmp(insns.data(), other.insns.data(), insns.size() * sizeof(uint32_t));
+ int cmp = memcmp(spirv.data(), other.spirv.data(), spirv.size() * sizeof(uint32_t));
if(cmp != 0)
{
return cmp < 0;
diff --git a/src/Vulkan/VkPipelineCache.hpp b/src/Vulkan/VkPipelineCache.hpp
index 9a60954..d875764 100644
--- a/src/Vulkan/VkPipelineCache.hpp
+++ b/src/Vulkan/VkPipelineCache.hpp
@@ -57,18 +57,18 @@
struct SpirvBinaryKey
{
- SpirvBinaryKey(const sw::SpirvBinary &insns,
+ SpirvBinaryKey(const sw::SpirvBinary &spirv,
const VkSpecializationInfo *specializationInfo,
bool optimize);
bool operator<(const SpirvBinaryKey &other) const;
- const sw::SpirvBinary &getInsns() const { return insns; }
+ const sw::SpirvBinary &getBinary() const { return spirv; }
const VkSpecializationInfo *getSpecializationInfo() const { return specializationInfo.get(); }
bool getOptimization() const { return optimize; }
private:
- const sw::SpirvBinary insns;
+ const sw::SpirvBinary spirv;
const vk::SpecializationInfo specializationInfo;
const bool optimize;
};
diff --git a/src/Vulkan/VkShaderModule.cpp b/src/Vulkan/VkShaderModule.cpp
index b49151d..edf495b 100644
--- a/src/Vulkan/VkShaderModule.cpp
+++ b/src/Vulkan/VkShaderModule.cpp
@@ -20,11 +20,8 @@
namespace vk {
-std::atomic<uint32_t> ShaderModule::serialCounter(1); // Start at 1, 0 is invalid shader.
-
ShaderModule::ShaderModule(const VkShaderModuleCreateInfo *pCreateInfo, void *mem)
- : serialID(nextSerialID())
- , binary(pCreateInfo->pCode, pCreateInfo->codeSize / sizeof(uint32_t))
+ : binary(pCreateInfo->pCode, pCreateInfo->codeSize / sizeof(uint32_t))
{
#if !defined(NDEBUG) || defined(DCHECK_ALWAYS_ON)
spvtools::SpirvTools spirvTools(SPIRV_VERSION);
diff --git a/src/Vulkan/VkShaderModule.hpp b/src/Vulkan/VkShaderModule.hpp
index 611aebf..2bba35d 100644
--- a/src/Vulkan/VkShaderModule.hpp
+++ b/src/Vulkan/VkShaderModule.hpp
@@ -18,8 +18,6 @@
#include "VkObject.hpp"
#include "Pipeline/SpirvBinary.hpp"
-#include <atomic>
-
namespace vk {
class ShaderModule : public Object<ShaderModule, VkShaderModule>
@@ -30,13 +28,7 @@
static size_t ComputeRequiredAllocationSize(const VkShaderModuleCreateInfo *pCreateInfo);
const sw::SpirvBinary &getBinary() const { return binary; }
- uint32_t getSerialID() const { return serialID; }
- static uint32_t nextSerialID() { return serialCounter++; }
-
private:
- const uint32_t serialID;
- static std::atomic<uint32_t> serialCounter;
-
sw::SpirvBinary binary;
};
