vulkan: Cleanup for shader / pipeline code
1. Collapse branch when creating pipelines to simplify code. This has a
small metric change in that VkPipelineCreation time is included in
metric now for cache misses.
2. Remove TransformedShaderModuleCacheKey. This was used by
TransformedShaderModuleCache which was deleted previously.
Bug: none
Change-Id: I21f637f0b5eeed540cd6abeae005185070bb13ab
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/241514
Reviewed-by: Loko Kung <lokokung@google.com>
Commit-Queue: Kyle Charbonneau <kylechar@google.com>
diff --git a/src/dawn/native/vulkan/ComputePipelineVk.cpp b/src/dawn/native/vulkan/ComputePipelineVk.cpp
index c8260b0..c237679 100644
--- a/src/dawn/native/vulkan/ComputePipelineVk.cpp
+++ b/src/dawn/native/vulkan/ComputePipelineVk.cpp
@@ -121,20 +121,13 @@
// Try to see if we have anything in the blob cache.
platform::metrics::DawnHistogramTimer cacheTimer(GetDevice()->GetPlatform());
Ref<PipelineCache> cache = ToBackend(GetDevice()->GetOrCreatePipelineCache(GetCacheKey()));
- if (cache->CacheHit()) {
- DAWN_TRY(CheckVkSuccess(
- device->fn.CreateComputePipelines(device->GetVkDevice(), cache->GetHandle(), 1,
- &createInfo, nullptr, &*mHandle),
- "CreateComputePipelines"));
- cacheTimer.RecordMicroseconds("Vulkan.CreateComputePipelines.CacheHit");
- } else {
- cacheTimer.Reset();
- DAWN_TRY(CheckVkSuccess(
- device->fn.CreateComputePipelines(device->GetVkDevice(), cache->GetHandle(), 1,
- &createInfo, nullptr, &*mHandle),
- "CreateComputePipelines"));
- cacheTimer.RecordMicroseconds("Vulkan.CreateComputePipelines.CacheMiss");
- }
+ DAWN_TRY(
+ CheckVkSuccess(device->fn.CreateComputePipelines(device->GetVkDevice(), cache->GetHandle(),
+ 1, &createInfo, nullptr, &*mHandle),
+ "CreateComputePipelines"));
+ cacheTimer.RecordMicroseconds(cache->CacheHit() ? "Vulkan.CreateComputePipelines.CacheHit"
+ : "Vulkan.CreateComputePipelines.CacheMiss");
+
DAWN_TRY(cache->DidCompilePipeline());
SetLabelImpl();
diff --git a/src/dawn/native/vulkan/RenderPipelineVk.cpp b/src/dawn/native/vulkan/RenderPipelineVk.cpp
index dbe5a15..4569d55 100644
--- a/src/dawn/native/vulkan/RenderPipelineVk.cpp
+++ b/src/dawn/native/vulkan/RenderPipelineVk.cpp
@@ -612,20 +612,12 @@
// Try to see if we have anything in the blob cache.
platform::metrics::DawnHistogramTimer cacheTimer(GetDevice()->GetPlatform());
Ref<PipelineCache> cache = ToBackend(GetDevice()->GetOrCreatePipelineCache(GetCacheKey()));
- if (cache->CacheHit()) {
- DAWN_TRY(CheckVkSuccess(
- device->fn.CreateGraphicsPipelines(device->GetVkDevice(), cache->GetHandle(), 1,
- &createInfo, nullptr, &*mHandle),
- "CreateGraphicsPipelines"));
- cacheTimer.RecordMicroseconds("Vulkan.CreateGraphicsPipelines.CacheHit");
- } else {
- cacheTimer.Reset();
- DAWN_TRY(CheckVkSuccess(
- device->fn.CreateGraphicsPipelines(device->GetVkDevice(), cache->GetHandle(), 1,
- &createInfo, nullptr, &*mHandle),
- "CreateGraphicsPipelines"));
- cacheTimer.RecordMicroseconds("Vulkan.CreateGraphicsPipelines.CacheMiss");
- }
+ DAWN_TRY(
+ CheckVkSuccess(device->fn.CreateGraphicsPipelines(device->GetVkDevice(), cache->GetHandle(),
+ 1, &createInfo, nullptr, &*mHandle),
+ "CreateGraphicsPipelines"));
+ cacheTimer.RecordMicroseconds(cache->CacheHit() ? "Vulkan.CreateGraphicsPipelines.CacheHit"
+ : "Vulkan.CreateGraphicsPipelines.CacheMiss");
DAWN_TRY(cache->DidCompilePipeline());
diff --git a/src/dawn/native/vulkan/ShaderModuleVk.cpp b/src/dawn/native/vulkan/ShaderModuleVk.cpp
index 258567c..623182d 100644
--- a/src/dawn/native/vulkan/ShaderModuleVk.cpp
+++ b/src/dawn/native/vulkan/ShaderModuleVk.cpp
@@ -74,31 +74,6 @@
DAWN_SERIALIZABLE(struct, CompiledSpirv, COMPILED_SPIRV_MEMBERS){};
#undef COMPILED_SPIRV_MEMBERS
-bool TransformedShaderModuleCacheKey::operator==(
- const TransformedShaderModuleCacheKey& other) const {
- if (layoutPtr != other.layoutPtr || entryPoint != other.entryPoint ||
- constants.size() != other.constants.size()) {
- return false;
- }
- if (!std::equal(constants.begin(), constants.end(), other.constants.begin())) {
- return false;
- }
- if (emitPointSize != other.emitPointSize) {
- return false;
- }
- return true;
-}
-
-size_t TransformedShaderModuleCacheKeyHashFunc::operator()(
- const TransformedShaderModuleCacheKey& key) const {
- size_t hash = 0;
- HashCombine(&hash, key.layoutPtr, key.entryPoint, key.emitPointSize);
- for (const auto& entry : key.constants) {
- HashCombine(&hash, entry.first, entry.second);
- }
- return hash;
-}
-
// static
ResultOrError<Ref<ShaderModule>> ShaderModule::Create(
Device* device,
@@ -158,14 +133,6 @@
const ImmediateConstantMask& pipelineImmediateMask) {
TRACE_EVENT0(GetDevice()->GetPlatform(), General, "ShaderModuleVk::GetHandleAndSpirv");
- // Check to see if we have the handle and spirv cached already
- // TODO(chromium:345359083): Improve the computation of the cache key. For example, it isn't
- // ideal to use `reinterpret_cast<uintptr_t>(layout)` as the layout may be freed and
- // reallocated during the runtime.
- auto cacheKey = TransformedShaderModuleCacheKey{reinterpret_cast<uintptr_t>(layout),
- programmableStage.entryPoint.c_str(),
- programmableStage.constants, emitPointSize};
-
#if TINT_BUILD_SPV_WRITER
// Creation of module and spirv is deferred to this point when using tint generator
diff --git a/src/dawn/native/vulkan/ShaderModuleVk.h b/src/dawn/native/vulkan/ShaderModuleVk.h
index ee04246..8e9b451 100644
--- a/src/dawn/native/vulkan/ShaderModuleVk.h
+++ b/src/dawn/native/vulkan/ShaderModuleVk.h
@@ -47,19 +47,6 @@
namespace vulkan {
-struct TransformedShaderModuleCacheKey {
- uintptr_t layoutPtr;
- std::string entryPoint;
- PipelineConstantEntries constants;
- bool emitPointSize;
-
- bool operator==(const TransformedShaderModuleCacheKey& other) const;
-};
-
-struct TransformedShaderModuleCacheKeyHashFunc {
- size_t operator()(const TransformedShaderModuleCacheKey& key) const;
-};
-
class Device;
class PipelineLayout;
