Dawn: Support streaming ShaderModuleParseResult This CL add StreamOut support for std::optional, and add a wrapper UnsafeUnserializedValue to replace CacheKey::UnsafeUnkeyedValue and hold unserializable value that will do nothing when streaming in and will set to default constructed value when streamed out to. These help making ShaderModuleParseResult serializable for shader module blob cache. Bug: 42240459, 402772740 Change-Id: Ic6199e46a1be34c4d03047af361dfab1ef807817 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/244094 Reviewed-by: Corentin Wallez <cwallez@chromium.org> Commit-Queue: Zhaoming Jiang <zhaoming.jiang@microsoft.com>
diff --git a/src/dawn/native/CacheKey.h b/src/dawn/native/CacheKey.h index b5e32db..d8b5d47 100644 --- a/src/dawn/native/CacheKey.h +++ b/src/dawn/native/CacheKey.h
@@ -40,34 +40,8 @@ using stream::ByteVectorSink::ByteVectorSink; enum class Type { ComputePipeline, RenderPipeline, Shader }; - - template <typename T> - class UnsafeUnkeyedValue { - public: - UnsafeUnkeyedValue() = default; - // NOLINTNEXTLINE(runtime/explicit) allow implicit construction to decrease verbosity - UnsafeUnkeyedValue(T&& value) : mValue(std::forward<T>(value)) {} - - const T& UnsafeGetValue() const { return mValue; } - - // Friend definition of StreamIn which can be found by ADL to override - // stream::StreamIn<T>. - friend constexpr void StreamIn(stream::Sink*, const UnsafeUnkeyedValue&) {} - - // Enabling DAWN_SERIALIZABLE classes with UnsafeUnkeyedValue member to use default equality - // operator. Equality comparison always returns true for the same type UnsafeUnkeyedValues. - bool operator==(const UnsafeUnkeyedValue<T>& other) const { return true; } - - private: - T mValue; - }; }; -template <typename T> -CacheKey::UnsafeUnkeyedValue<T> UnsafeUnkeyedValue(T&& value) { - return CacheKey::UnsafeUnkeyedValue<T>(std::forward<T>(value)); -} - } // namespace dawn::native #endif // SRC_DAWN_NATIVE_CACHEKEY_H_
diff --git a/src/dawn/native/Serializable.h b/src/dawn/native/Serializable.h index 72dafb2..88918c2 100644 --- a/src/dawn/native/Serializable.h +++ b/src/dawn/native/Serializable.h
@@ -63,6 +63,46 @@ return CreateBlob(std::move(sink)); } }; + +// UnsafeUnserializedValue holds a value of type T that does nothing when StreamIn to a sink, calls +// default constructor when StreamOut from a source, and always compares as equal between objects of +// the same type UnsafeUnserializedValue<T>. This is used for members in DAWN_SERIALIZABLE or +// DAWN_MAKE_CACHE_REQUEST to prevent a member to get streamed into cache key, or enable having +// unserializabled fields that get computed by cache missed function and returned together with +// cached fields. +template <typename T> +class UnsafeUnserializedValue { + public: + UnsafeUnserializedValue() = default; + explicit UnsafeUnserializedValue(T&& value) : mValue(std::forward<T>(value)) {} + UnsafeUnserializedValue(const UnsafeUnserializedValue<T>& other) + : mValue(other.UnsafeGetValue()) {} + UnsafeUnserializedValue<T>& operator=(UnsafeUnserializedValue<T>&& other) { + mValue = std::move(other.UnsafeGetValue()); + return *this; + } + + constexpr const T& UnsafeGetValue() const { return mValue; } + constexpr T& UnsafeGetValue() { return mValue; } + + friend constexpr void StreamIn(stream::Sink*, const UnsafeUnserializedValue<T>&) {} + friend MaybeError StreamOut(stream::Source*, UnsafeUnserializedValue<T>* out) { + // Call default constructor to initialize the value. + out->mValue = T(); + return {}; + } + // Enabling DAWN_SERIALIZABLE classes with UnsafeUnserializedOptional member to use default + // equality operator. Equality comparison always returns true for the same type. + bool operator==(const UnsafeUnserializedValue<T>& other) const { return true; } + + protected: + T mValue; +}; + +// Template deduction guide for UnsafeUnserializedValue to enable deducting type of +// UnsafeUnserializedValue(T{}) to UnsafeUnserializedValue<T>. +template <typename T> +UnsafeUnserializedValue(T&& value) -> UnsafeUnserializedValue<std::decay_t<T>>; } // namespace dawn::native // Helper macro to define a struct or class along with VisitAll methods to call
diff --git a/src/dawn/native/d3d/D3DCompilationRequest.h b/src/dawn/native/d3d/D3DCompilationRequest.h index a8d04d8..bd3c8b9 100644 --- a/src/dawn/native/d3d/D3DCompilationRequest.h +++ b/src/dawn/native/d3d/D3DCompilationRequest.h
@@ -60,38 +60,38 @@ using InterStageShaderVariablesMask = std::bitset<tint::hlsl::writer::kMaxInterStageLocations>; using SubstituteOverrideConfig = std::unordered_map<tint::OverrideId, double>; -#define HLSL_COMPILATION_REQUEST_MEMBERS(X) \ - X(ShaderModuleBase::ShaderModuleHash, shaderModuleHash) \ - X(CacheKey::UnsafeUnkeyedValue<ShaderModuleBase::ScopedUseTintProgram>, inputProgram) \ - X(std::string_view, entryPointName) \ - X(SingleShaderStage, stage) \ - X(uint32_t, shaderModel) \ - X(uint32_t, compileFlags) \ - X(Compiler, compiler) \ - X(uint64_t, compilerVersion) \ - X(std::wstring_view, dxcShaderProfile) \ - X(std::string_view, fxcShaderProfile) \ - X(uint32_t, firstIndexOffsetShaderRegister) \ - X(uint32_t, firstIndexOffsetRegisterSpace) \ - X(tint::hlsl::writer::Options, tintOptions) \ - X(SubstituteOverrideConfig, substituteOverrideConfig) \ - X(LimitsForCompilationRequest, limits) \ - X(CacheKey::UnsafeUnkeyedValue<LimitsForCompilationRequest>, adapterSupportedLimits) \ - X(uint32_t, maxSubgroupSize) \ - X(bool, disableSymbolRenaming) \ - X(bool, dumpShaders) \ +#define HLSL_COMPILATION_REQUEST_MEMBERS(X) \ + X(ShaderModuleBase::ShaderModuleHash, shaderModuleHash) \ + X(UnsafeUnserializedValue<ShaderModuleBase::ScopedUseTintProgram>, inputProgram) \ + X(std::string_view, entryPointName) \ + X(SingleShaderStage, stage) \ + X(uint32_t, shaderModel) \ + X(uint32_t, compileFlags) \ + X(Compiler, compiler) \ + X(uint64_t, compilerVersion) \ + X(std::wstring_view, dxcShaderProfile) \ + X(std::string_view, fxcShaderProfile) \ + X(uint32_t, firstIndexOffsetShaderRegister) \ + X(uint32_t, firstIndexOffsetRegisterSpace) \ + X(tint::hlsl::writer::Options, tintOptions) \ + X(SubstituteOverrideConfig, substituteOverrideConfig) \ + X(LimitsForCompilationRequest, limits) \ + X(UnsafeUnserializedValue<LimitsForCompilationRequest>, adapterSupportedLimits) \ + X(uint32_t, maxSubgroupSize) \ + X(bool, disableSymbolRenaming) \ + X(bool, dumpShaders) \ X(bool, useTintIR) -#define D3D_BYTECODE_COMPILATION_REQUEST_MEMBERS(X) \ - X(bool, hasShaderF16Feature) \ - X(uint32_t, compileFlags) \ - X(Compiler, compiler) \ - X(uint64_t, compilerVersion) \ - X(std::wstring_view, dxcShaderProfile) \ - X(std::string_view, fxcShaderProfile) \ - X(CacheKey::UnsafeUnkeyedValue<pD3DCompile>, d3dCompile) \ - X(CacheKey::UnsafeUnkeyedValue<IDxcLibrary*>, dxcLibrary) \ - X(CacheKey::UnsafeUnkeyedValue<IDxcCompiler3*>, dxcCompiler) +#define D3D_BYTECODE_COMPILATION_REQUEST_MEMBERS(X) \ + X(bool, hasShaderF16Feature) \ + X(uint32_t, compileFlags) \ + X(Compiler, compiler) \ + X(uint64_t, compilerVersion) \ + X(std::wstring_view, dxcShaderProfile) \ + X(std::string_view, fxcShaderProfile) \ + X(UnsafeUnserializedValue<pD3DCompile>, d3dCompile) \ + X(UnsafeUnserializedValue<IDxcLibrary*>, dxcLibrary) \ + X(UnsafeUnserializedValue<IDxcCompiler3*>, dxcCompiler) DAWN_SERIALIZABLE(struct, HlslCompilationRequest, HLSL_COMPILATION_REQUEST_MEMBERS){}; #undef HLSL_COMPILATION_REQUEST_MEMBERS @@ -104,7 +104,7 @@ #define D3D_COMPILATION_REQUEST_MEMBERS(X) \ X(HlslCompilationRequest, hlsl) \ X(D3DBytecodeCompilationRequest, bytecode) \ - X(CacheKey::UnsafeUnkeyedValue<dawn::platform::Platform*>, tracePlatform) + X(UnsafeUnserializedValue<dawn::platform::Platform*>, tracePlatform) DAWN_MAKE_CACHE_REQUEST(D3DCompilationRequest, D3D_COMPILATION_REQUEST_MEMBERS); #undef D3D_COMPILATION_REQUEST_MEMBERS
diff --git a/src/dawn/native/d3d/ShaderUtils.cpp b/src/dawn/native/d3d/ShaderUtils.cpp index db399b3..93ad32a 100644 --- a/src/dawn/native/d3d/ShaderUtils.cpp +++ b/src/dawn/native/d3d/ShaderUtils.cpp
@@ -228,7 +228,7 @@ } MaybeError TranslateToHLSL(d3d::HlslCompilationRequest r, - CacheKey::UnsafeUnkeyedValue<dawn::platform::Platform*> tracePlatform, + UnsafeUnserializedValue<dawn::platform::Platform*> tracePlatform, CompiledShader* compiledShader) { tint::ast::transform::Manager transformManager; tint::ast::transform::DataMap transformInputs;
diff --git a/src/dawn/native/d3d11/ShaderModuleD3D11.cpp b/src/dawn/native/d3d11/ShaderModuleD3D11.cpp index d5c7073..c8ff479 100644 --- a/src/dawn/native/d3d11/ShaderModuleD3D11.cpp +++ b/src/dawn/native/d3d11/ShaderModuleD3D11.cpp
@@ -92,7 +92,7 @@ const bool useTintIR = device->IsToggleEnabled(Toggle::UseTintIR); d3d::D3DCompilationRequest req = {}; - req.tracePlatform = UnsafeUnkeyedValue(device->GetPlatform()); + req.tracePlatform = UnsafeUnserializedValue(device->GetPlatform()); req.hlsl.shaderModel = 50; req.hlsl.disableSymbolRenaming = device->IsToggleEnabled(Toggle::DisableSymbolRenaming); req.hlsl.dumpShaders = device->IsToggleEnabled(Toggle::DumpShaders); @@ -104,7 +104,8 @@ // D3D11 only supports FXC. req.bytecode.compiler = d3d::Compiler::FXC; - req.bytecode.d3dCompile = std::move(pD3DCompile{device->GetFunctions()->d3dCompile}); + req.bytecode.d3dCompile = + UnsafeUnserializedValue(pD3DCompile{device->GetFunctions()->d3dCompile}); req.bytecode.compilerVersion = D3D_COMPILER_VERSION; DAWN_ASSERT(device->GetDeviceInfo().shaderModel == 50); switch (stage) { @@ -192,7 +193,7 @@ } req.hlsl.shaderModuleHash = GetHash(); - req.hlsl.inputProgram = UseTintProgram(); + req.hlsl.inputProgram = UnsafeUnserializedValue(UseTintProgram()); req.hlsl.entryPointName = programmableStage.entryPoint.c_str(); req.hlsl.stage = stage; @@ -228,8 +229,8 @@ req.hlsl.substituteOverrideConfig = BuildSubstituteOverridesTransformConfig(programmableStage); req.hlsl.limits = LimitsForCompilationRequest::Create(device->GetLimits().v1); - req.hlsl.adapterSupportedLimits = - LimitsForCompilationRequest::Create(device->GetAdapter()->GetLimits().v1); + req.hlsl.adapterSupportedLimits = UnsafeUnserializedValue( + LimitsForCompilationRequest::Create(device->GetAdapter()->GetLimits().v1)); req.hlsl.maxSubgroupSize = device->GetAdapter()->GetPhysicalDevice()->GetSubgroupMaxSize(); req.hlsl.tintOptions.disable_robustness = !device->IsRobustnessEnabled();
diff --git a/src/dawn/native/d3d12/ShaderModuleD3D12.cpp b/src/dawn/native/d3d12/ShaderModuleD3D12.cpp index ca4b0d6..d4bf39e 100644 --- a/src/dawn/native/d3d12/ShaderModuleD3D12.cpp +++ b/src/dawn/native/d3d12/ShaderModuleD3D12.cpp
@@ -134,7 +134,7 @@ const bool useTintIR = device->IsToggleEnabled(Toggle::UseTintIR); d3d::D3DCompilationRequest req = {}; - req.tracePlatform = UnsafeUnkeyedValue(device->GetPlatform()); + req.tracePlatform = UnsafeUnserializedValue(device->GetPlatform()); req.hlsl.shaderModel = ToBackend(device->GetPhysicalDevice()) ->GetAppliedShaderModelUnderToggles(device->GetTogglesState()); req.hlsl.disableSymbolRenaming = device->IsToggleEnabled(Toggle::DisableSymbolRenaming); @@ -154,13 +154,14 @@ ToBackend(device->GetPhysicalDevice())->GetBackend()->GetDxcVersion(); req.bytecode.compiler = d3d::Compiler::DXC; - req.bytecode.dxcLibrary = device->GetDxcLibrary().Get(); - req.bytecode.dxcCompiler = device->GetDxcCompiler().Get(); + req.bytecode.dxcLibrary = UnsafeUnserializedValue(device->GetDxcLibrary().Get()); + req.bytecode.dxcCompiler = UnsafeUnserializedValue(device->GetDxcCompiler().Get()); req.bytecode.compilerVersion = dxcVersionInfo.DxcCompilerVersion; req.bytecode.dxcShaderProfile = device->GetDxcShaderProfiles()[stage]; } else { req.bytecode.compiler = d3d::Compiler::FXC; - req.bytecode.d3dCompile = std::move(pD3DCompile{device->GetFunctions()->d3dCompile}); + req.bytecode.d3dCompile = + UnsafeUnserializedValue(pD3DCompile{device->GetFunctions()->d3dCompile}); req.bytecode.compilerVersion = D3D_COMPILER_VERSION; switch (stage) { case SingleShaderStage::Vertex: @@ -322,7 +323,7 @@ } req.hlsl.shaderModuleHash = GetHash(); - req.hlsl.inputProgram = UseTintProgram(); + req.hlsl.inputProgram = UnsafeUnserializedValue(UseTintProgram()); req.hlsl.entryPointName = programmableStage.entryPoint.c_str(); req.hlsl.stage = stage; if (!useTintIR) { @@ -380,8 +381,8 @@ device->IsToggleEnabled(Toggle::EnableIntegerRangeAnalysisInRobustness); req.hlsl.limits = LimitsForCompilationRequest::Create(device->GetLimits().v1); - req.hlsl.adapterSupportedLimits = - LimitsForCompilationRequest::Create(device->GetAdapter()->GetLimits().v1); + req.hlsl.adapterSupportedLimits = UnsafeUnserializedValue( + LimitsForCompilationRequest::Create(device->GetAdapter()->GetLimits().v1)); req.hlsl.maxSubgroupSize = device->GetAdapter()->GetPhysicalDevice()->GetSubgroupMaxSize(); CacheResult<d3d::CompiledShader> compiledShader;
diff --git a/src/dawn/native/metal/ShaderModuleMTL.mm b/src/dawn/native/metal/ShaderModuleMTL.mm index 4b96fa6..d487537 100644 --- a/src/dawn/native/metal/ShaderModuleMTL.mm +++ b/src/dawn/native/metal/ShaderModuleMTL.mm
@@ -55,19 +55,19 @@ using OptionalVertexPullingTransformConfig = std::optional<tint::VertexPullingConfig>; using SubstituteOverrideConfig = std::unordered_map<tint::OverrideId, double>; -#define MSL_COMPILATION_REQUEST_MEMBERS(X) \ - X(SingleShaderStage, stage) \ - X(ShaderModuleBase::ShaderModuleHash, shaderModuleHash) \ - X(CacheKey::UnsafeUnkeyedValue<ShaderModuleBase::ScopedUseTintProgram>, inputProgram) \ - X(SubstituteOverrideConfig, substituteOverrideConfig) \ - X(LimitsForCompilationRequest, limits) \ - X(CacheKey::UnsafeUnkeyedValue<LimitsForCompilationRequest>, adapterSupportedLimits) \ - X(uint32_t, maxSubgroupSize) \ - X(std::string, entryPointName) \ - X(bool, usesSubgroupMatrix) \ - X(bool, disableSymbolRenaming) \ - X(tint::msl::writer::Options, tintOptions) \ - X(CacheKey::UnsafeUnkeyedValue<dawn::platform::Platform*>, platform) +#define MSL_COMPILATION_REQUEST_MEMBERS(X) \ + X(SingleShaderStage, stage) \ + X(ShaderModuleBase::ShaderModuleHash, shaderModuleHash) \ + X(UnsafeUnserializedValue<ShaderModuleBase::ScopedUseTintProgram>, inputProgram) \ + X(SubstituteOverrideConfig, substituteOverrideConfig) \ + X(LimitsForCompilationRequest, limits) \ + X(UnsafeUnserializedValue<LimitsForCompilationRequest>, adapterSupportedLimits) \ + X(uint32_t, maxSubgroupSize) \ + X(std::string, entryPointName) \ + X(bool, usesSubgroupMatrix) \ + X(bool, disableSymbolRenaming) \ + X(tint::msl::writer::Options, tintOptions) \ + X(UnsafeUnserializedValue<dawn::platform::Platform*>, platform) DAWN_MAKE_CACHE_REQUEST(MslCompilationRequest, MSL_COMPILATION_REQUEST_MEMBERS); #undef MSL_COMPILATION_REQUEST_MEMBERS @@ -275,12 +275,12 @@ MslCompilationRequest req = {}; req.stage = stage; req.shaderModuleHash = programmableStage.module->GetHash(); - req.inputProgram = programmableStage.module->UseTintProgram(); + req.inputProgram = UnsafeUnserializedValue(programmableStage.module->UseTintProgram()); req.substituteOverrideConfig = BuildSubstituteOverridesTransformConfig(programmableStage); req.entryPointName = programmableStage.entryPoint.c_str(); req.disableSymbolRenaming = device->IsToggleEnabled(Toggle::DisableSymbolRenaming); req.usesSubgroupMatrix = programmableStage.metadata->usesSubgroupMatrix; - req.platform = UnsafeUnkeyedValue(device->GetPlatform()); + req.platform = UnsafeUnserializedValue(device->GetPlatform()); req.tintOptions.strip_all_names = !req.disableSymbolRenaming; req.tintOptions.remapped_entry_point_name = device->GetIsolatedEntryPointName(); @@ -305,8 +305,8 @@ device->IsToggleEnabled(Toggle::EnableIntegerRangeAnalysisInRobustness); req.limits = LimitsForCompilationRequest::Create(device->GetLimits().v1); - req.adapterSupportedLimits = - LimitsForCompilationRequest::Create(device->GetAdapter()->GetLimits().v1); + req.adapterSupportedLimits = UnsafeUnserializedValue( + LimitsForCompilationRequest::Create(device->GetAdapter()->GetLimits().v1)); req.maxSubgroupSize = device->GetAdapter()->GetPhysicalDevice()->GetSubgroupMaxSize(); CacheResult<MslCompilation> mslCompilation;
diff --git a/src/dawn/native/opengl/ShaderModuleGL.cpp b/src/dawn/native/opengl/ShaderModuleGL.cpp index 62cacee..41109c7 100644 --- a/src/dawn/native/opengl/ShaderModuleGL.cpp +++ b/src/dawn/native/opengl/ShaderModuleGL.cpp
@@ -92,18 +92,18 @@ using InterstageLocationAndName = std::pair<uint32_t, std::string>; using SubstituteOverrideConfig = std::unordered_map<tint::OverrideId, double>; -#define GLSL_COMPILATION_REQUEST_MEMBERS(X) \ - X(ShaderModuleBase::ShaderModuleHash, shaderModuleHash) \ - X(CacheKey::UnsafeUnkeyedValue<ShaderModuleBase::ScopedUseTintProgram>, inputProgram) \ - X(std::string, entryPointName) \ - X(SingleShaderStage, stage) \ - X(SubstituteOverrideConfig, substituteOverrideConfig) \ - X(LimitsForCompilationRequest, limits) \ - X(CacheKey::UnsafeUnkeyedValue<LimitsForCompilationRequest>, adapterSupportedLimits) \ - X(bool, disableSymbolRenaming) \ - X(std::vector<InterstageLocationAndName>, interstageVariables) \ - X(tint::glsl::writer::Options, tintOptions) \ - X(CacheKey::UnsafeUnkeyedValue<dawn::platform::Platform*>, platform) +#define GLSL_COMPILATION_REQUEST_MEMBERS(X) \ + X(ShaderModuleBase::ShaderModuleHash, shaderModuleHash) \ + X(UnsafeUnserializedValue<ShaderModuleBase::ScopedUseTintProgram>, inputProgram) \ + X(std::string, entryPointName) \ + X(SingleShaderStage, stage) \ + X(SubstituteOverrideConfig, substituteOverrideConfig) \ + X(LimitsForCompilationRequest, limits) \ + X(UnsafeUnserializedValue<LimitsForCompilationRequest>, adapterSupportedLimits) \ + X(bool, disableSymbolRenaming) \ + X(std::vector<InterstageLocationAndName>, interstageVariables) \ + X(tint::glsl::writer::Options, tintOptions) \ + X(UnsafeUnserializedValue<dawn::platform::Platform*>, platform) DAWN_MAKE_CACHE_REQUEST(GLSLCompilationRequest, GLSL_COMPILATION_REQUEST_MEMBERS); #undef GLSL_COMPILATION_REQUEST_MEMBERS @@ -475,7 +475,7 @@ GLSLCompilationRequest req = {}; req.shaderModuleHash = GetHash(); - req.inputProgram = UseTintProgram(); + req.inputProgram = UnsafeUnserializedValue(UseTintProgram()); // Since (non-Vulkan) GLSL does not support descriptor sets, generate a // mapping from the original group/binding pair to a binding-only @@ -512,8 +512,8 @@ req.entryPointName = programmableStage.entryPoint; req.substituteOverrideConfig = BuildSubstituteOverridesTransformConfig(programmableStage); req.limits = LimitsForCompilationRequest::Create(GetDevice()->GetLimits().v1); - req.adapterSupportedLimits = - LimitsForCompilationRequest::Create(GetDevice()->GetAdapter()->GetLimits().v1); + req.adapterSupportedLimits = UnsafeUnserializedValue( + LimitsForCompilationRequest::Create(GetDevice()->GetAdapter()->GetLimits().v1)); if (GetDevice()->IsToggleEnabled(Toggle::GLUseArrayLengthFromUniform)) { *needsSSBOLengthUniformBuffer = @@ -527,7 +527,7 @@ } } - req.platform = UnsafeUnkeyedValue(GetDevice()->GetPlatform()); + req.platform = UnsafeUnserializedValue(GetDevice()->GetPlatform()); req.tintOptions.version = tint::glsl::writer::Version(ToTintGLStandard(version.GetStandard()), version.GetMajor(), version.GetMinor());
diff --git a/src/dawn/native/stream/Stream.cpp b/src/dawn/native/stream/Stream.cpp index b3a5cb0..1682b63 100644 --- a/src/dawn/native/stream/Stream.cpp +++ b/src/dawn/native/stream/Stream.cpp
@@ -33,12 +33,6 @@ namespace dawn::native::stream { -constexpr void StreamIn(Sink* s) {} - -MaybeError StreamOut(Source* s) { - return {}; -} - template <> void Stream<std::string>::Write(Sink* s, const std::string& t) { StreamIn(s, t.length());
diff --git a/src/dawn/native/stream/Stream.h b/src/dawn/native/stream/Stream.h index 3ffd37d..acdf30f 100644 --- a/src/dawn/native/stream/Stream.h +++ b/src/dawn/native/stream/Stream.h
@@ -33,13 +33,12 @@ #include <functional> #include <limits> #include <memory> +#include <optional> #include <unordered_map> #include <unordered_set> #include <utility> #include <vector> -#include <optional> - #include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" #include "dawn/common/Platform.h" @@ -106,11 +105,13 @@ // Helper to call StreamIn on an empty parameter pack, e.g. for a DAWN_SERIALIZABLE struct with no // member. Do nothing. -constexpr void StreamIn(Sink* s); +inline constexpr void StreamIn(Sink* s) {} // Helper to call StreamOut on an empty parameter pack, e.g. for a DAWN_SERIALIZABLE struct with no // member. Do nothing and return success. -MaybeError StreamOut(Source* s); +inline MaybeError StreamOut(Source* s) { + return {}; +} // Stream specialization for fundamental types. template <typename T> @@ -254,9 +255,10 @@ bool notNullptr; DAWN_TRY(StreamOut(source, ¬Nullptr)); if (notNullptr) { - T out; - DAWN_TRY(StreamOut(source, &out)); - *t = std::make_unique<T>(std::move(out)); + // Avoid using copy or move constructor of T. + std::unique_ptr<T> out = std::make_unique<T>(); + DAWN_TRY(StreamOut(source, out.get())); + *t = std::move(out); } else { *t = nullptr; } @@ -285,6 +287,18 @@ StreamIn(sink, *t); } } + static MaybeError Read(Source* source, std::optional<T>* t) { + bool hasValue; + DAWN_TRY(StreamOut(source, &hasValue)); + if (hasValue) { + T out; + DAWN_TRY(StreamOut(source, &out)); + *t = std::move(out); + } else { + t->reset(); + } + return {}; + } }; // Stream specialization for fixed arrays of fundamental types.
diff --git a/src/dawn/native/vulkan/ShaderModuleVk.cpp b/src/dawn/native/vulkan/ShaderModuleVk.cpp index 7de811a..45a2f08 100644 --- a/src/dawn/native/vulkan/ShaderModuleVk.cpp +++ b/src/dawn/native/vulkan/ShaderModuleVk.cpp
@@ -116,18 +116,18 @@ using SubstituteOverrideConfig = std::unordered_map<tint::OverrideId, double>; -#define SPIRV_COMPILATION_REQUEST_MEMBERS(X) \ - X(SingleShaderStage, stage) \ - X(ShaderModuleBase::ShaderModuleHash, shaderModuleHash) \ - X(CacheKey::UnsafeUnkeyedValue<ShaderModuleBase::ScopedUseTintProgram>, inputProgram) \ - X(SubstituteOverrideConfig, substituteOverrideConfig) \ - X(LimitsForCompilationRequest, limits) \ - X(CacheKey::UnsafeUnkeyedValue<LimitsForCompilationRequest>, adapterSupportedLimits) \ - X(uint32_t, maxSubgroupSize) \ - X(std::string_view, entryPointName) \ - X(bool, usesSubgroupMatrix) \ - X(tint::spirv::writer::Options, tintOptions) \ - X(CacheKey::UnsafeUnkeyedValue<dawn::platform::Platform*>, platform) +#define SPIRV_COMPILATION_REQUEST_MEMBERS(X) \ + X(SingleShaderStage, stage) \ + X(ShaderModuleBase::ShaderModuleHash, shaderModuleHash) \ + X(UnsafeUnserializedValue<ShaderModuleBase::ScopedUseTintProgram>, inputProgram) \ + X(SubstituteOverrideConfig, substituteOverrideConfig) \ + X(LimitsForCompilationRequest, limits) \ + X(UnsafeUnserializedValue<LimitsForCompilationRequest>, adapterSupportedLimits) \ + X(uint32_t, maxSubgroupSize) \ + X(std::string_view, entryPointName) \ + X(bool, usesSubgroupMatrix) \ + X(tint::spirv::writer::Options, tintOptions) \ + X(UnsafeUnserializedValue<dawn::platform::Platform*>, platform) DAWN_MAKE_CACHE_REQUEST(SpirvCompilationRequest, SPIRV_COMPILATION_REQUEST_MEMBERS); #undef SPIRV_COMPILATION_REQUEST_MEMBERS @@ -244,9 +244,9 @@ SpirvCompilationRequest req = {}; req.stage = stage; req.shaderModuleHash = GetHash(); - req.inputProgram = UseTintProgram(); + req.inputProgram = UnsafeUnserializedValue(UseTintProgram()); req.entryPointName = programmableStage.entryPoint; - req.platform = UnsafeUnkeyedValue(GetDevice()->GetPlatform()); + req.platform = UnsafeUnserializedValue(GetDevice()->GetPlatform()); req.substituteOverrideConfig = BuildSubstituteOverridesTransformConfig(programmableStage); req.usesSubgroupMatrix = programmableStage.metadata->usesSubgroupMatrix; @@ -306,8 +306,8 @@ GetDevice()->IsToggleEnabled(Toggle::EnableIntegerRangeAnalysisInRobustness); req.limits = LimitsForCompilationRequest::Create(GetDevice()->GetLimits().v1); - req.adapterSupportedLimits = - LimitsForCompilationRequest::Create(GetDevice()->GetAdapter()->GetLimits().v1); + req.adapterSupportedLimits = UnsafeUnserializedValue( + LimitsForCompilationRequest::Create(GetDevice()->GetAdapter()->GetLimits().v1)); req.maxSubgroupSize = GetDevice()->GetAdapter()->GetPhysicalDevice()->GetSubgroupMaxSize(); CacheResult<CompiledSpirv> compilation;
diff --git a/src/dawn/tests/unittests/native/CacheRequestTests.cpp b/src/dawn/tests/unittests/native/CacheRequestTests.cpp index 152fa61..e144a1a 100644 --- a/src/dawn/tests/unittests/native/CacheRequestTests.cpp +++ b/src/dawn/tests/unittests/native/CacheRequestTests.cpp
@@ -60,12 +60,12 @@ int value; }; -#define REQUEST_MEMBERS(X) \ - X(int, a) \ - X(float, b) \ - X(std::vector<unsigned int>, c) \ - X(CacheKey::UnsafeUnkeyedValue<int*>, d) \ - X(CacheKey::UnsafeUnkeyedValue<Foo>, e) +#define REQUEST_MEMBERS(X) \ + X(int, a) \ + X(float, b) \ + X(std::vector<unsigned int>, c) \ + X(UnsafeUnserializedValue<int*>, d) \ + X(UnsafeUnserializedValue<Foo>, e) DAWN_MAKE_CACHE_REQUEST(CacheRequestForTesting, REQUEST_MEMBERS); @@ -123,18 +123,18 @@ EXPECT_EQ(memcmp(result.GetCacheKey().data(), expectedKey.data(), expectedKey.size()), 0); } -// Test that members that are wrapped in UnsafeUnkeyedValue do not impact the key. +// Test that members that are wrapped in UnsafeUnserializedValue do not impact the key. TEST_F(CacheRequestTests, CacheKeyIgnoresUnsafeIgnoredValue) { - // Make two requests with different UnsafeUnkeyedValues (UnsafeUnkeyed is declared on the struct - // definition). + // Make two requests with different UnsafeUnserializedValue (UnsafeUnkeyed is declared on the + // struct definition). int v1, v2; CacheRequestForTesting req1; - req1.d = &v1; - req1.e = Foo{42}; + req1.d = UnsafeUnserializedValue(&v1); + req1.e = UnsafeUnserializedValue(Foo{42}); CacheRequestForTesting req2; - req2.d = &v2; - req2.e = Foo{24}; + req2.d = UnsafeUnserializedValue(&v2); + req2.e = UnsafeUnserializedValue(Foo{24}); EXPECT_CALL(mMockCache, LoadData(_, _, nullptr, 0)).WillOnce(Return(0)).WillOnce(Return(0));
diff --git a/src/dawn/tests/unittests/native/StreamTests.cpp b/src/dawn/tests/unittests/native/StreamTests.cpp index e3b270f..88feeef 100644 --- a/src/dawn/tests/unittests/native/StreamTests.cpp +++ b/src/dawn/tests/unittests/native/StreamTests.cpp
@@ -436,6 +436,17 @@ EXPECT_CACHE_KEY_EQ(bp, expected); } +// Test that ByteVectorSink serialization skip UnsafeUnserializedValue as expected. +TEST(SerializeTests, UnsafeUnserializedValue) { + std::pair<uint32_t, UnsafeUnserializedValue<uint32_t>> input{123, 456}; + + ByteVectorSink expected; + // The second UnsafeUnserializedValue<uint32_t> is not serialized. + StreamIn(&expected, uint32_t(123)); + + EXPECT_CACHE_KEY_EQ(input, expected); +} + // Test that serializing then deserializing a param pack yields the same values. TEST(StreamTests, SerializeDeserializeParamPack) { int a = 1; @@ -585,6 +596,32 @@ } } +// Test that serializing then deserializing a UnsafeUnserializedValue<T> yields the default +// constructed T. +TEST(StreamTests, UnsafeUnserializedValue) { + using ValueType = std::string; + using Type = UnsafeUnserializedValue<ValueType>; + ValueType init1 = "hello"; + ValueType init2 = "world"; + + Type in = Type(ValueType(init1)); + ASSERT_EQ(in.UnsafeGetValue(), init1); + + ByteVectorSink sink; + StreamIn(&sink, in); + + BlobSource src(CreateBlob(sink)); + // Initialize the UnsafeUnserializedValue to a value different from default constructed. When + // deserializing, it should be assigned to default constructed value. + Type out = Type(ValueType(init2)); + ASSERT_EQ(out.UnsafeGetValue(), init2); + // Do the deserialization. + auto err = StreamOut(&src, &out); + EXPECT_FALSE(err.IsError()); + // Check that the UnsafeUnserializedValue was set to default constructed value. + EXPECT_EQ(out.UnsafeGetValue(), ValueType()); +} + template <size_t N> std::bitset<N - 1> BitsetFromBitString(const char (&str)[N]) { // N - 1 because the last character is the null terminator. @@ -614,6 +651,8 @@ BitsetFromBitString("100110010101011001100110101011001100101010110011001011011"), BitsetFromBitString("000110010101011000100110101011001100101010010011001010100"), BitsetFromBitString("111111111111111111111111111111111111111111111111111111111"), 0}, + // Test std::optional. + std::vector<std::optional<std::string>>{std::nullopt, "", "abc"}, // Test unordered_maps. std::vector<std::unordered_map<int, int>>{{}, {{4, 5}, {6, 8}, {99, 42}, {0, 0}},