From 21744d0fb8d76383e08471859ae776e677c18375 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Tue, 21 Apr 2020 07:57:30 +0000 Subject: [PATCH] Make all backend::ShaderModule get SPIRV from the frontend This will make it easier to support SPIRV as a chained sub-descriptor of ShaderModuleDescriptor in follow-up CLs. Also fix a couple style and formatting issues. Bug: dawn:22 Change-Id: Iddaf1f87edee65687e17670b70024835918a0382 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/19864 Commit-Queue: Corentin Wallez Reviewed-by: Kai Ninomiya --- src/dawn_native/ShaderModule.cpp | 16 +++++++++---- src/dawn_native/ShaderModule.h | 11 ++++----- src/dawn_native/d3d12/ShaderModuleD3D12.cpp | 20 ++++++++++------- src/dawn_native/d3d12/ShaderModuleD3D12.h | 4 +--- src/dawn_native/metal/ShaderModuleMTL.h | 7 +----- src/dawn_native/metal/ShaderModuleMTL.mm | 25 ++++++++++++--------- src/dawn_native/null/DeviceNull.cpp | 10 ++++----- src/dawn_native/opengl/ShaderModuleGL.cpp | 22 +++++++++--------- src/dawn_native/opengl/ShaderModuleGL.h | 2 +- src/dawn_native/vulkan/ShaderModuleVk.cpp | 14 +++++++----- src/dawn_native/vulkan/ShaderModuleVk.h | 2 +- 11 files changed, 70 insertions(+), 63 deletions(-) diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp index 41f84d9511..713dd956ef 100644 --- a/src/dawn_native/ShaderModule.cpp +++ b/src/dawn_native/ShaderModule.cpp @@ -324,7 +324,7 @@ namespace dawn_native { // ShaderModuleBase ShaderModuleBase::ShaderModuleBase(DeviceBase* device, const ShaderModuleDescriptor* descriptor) - : CachedObject(device), mCode(descriptor->code, descriptor->code + descriptor->codeSize) { + : CachedObject(device), mSpirv(descriptor->code, descriptor->code + descriptor->codeSize) { mFragmentOutputFormatBaseTypes.fill(Format::Other); if (GetDevice()->IsToggleEnabled(Toggle::UseSpvcParser)) { mSpvcContext.SetUseSpvcParser(true); @@ -836,7 +836,7 @@ namespace dawn_native { size_t ShaderModuleBase::HashFunc::operator()(const ShaderModuleBase* module) const { size_t hash = 0; - for (uint32_t word : module->mCode) { + for (uint32_t word : module->mSpirv) { HashCombine(&hash, word); } @@ -845,7 +845,7 @@ namespace dawn_native { bool ShaderModuleBase::EqualityFunc::operator()(const ShaderModuleBase* a, const ShaderModuleBase* b) const { - return a->mCode == b->mCode; + return a->mSpirv == b->mSpirv; } MaybeError ShaderModuleBase::CheckSpvcSuccess(shaderc_spvc_status status, @@ -856,7 +856,15 @@ namespace dawn_native { return {}; } - shaderc_spvc::CompileOptions ShaderModuleBase::GetCompileOptions() { + shaderc_spvc::Context* ShaderModuleBase::GetContext() { + return &mSpvcContext; + } + + const std::vector& ShaderModuleBase::GetSpirv() const { + return mSpirv; + } + + shaderc_spvc::CompileOptions ShaderModuleBase::GetCompileOptions() const { shaderc_spvc::CompileOptions options; options.SetValidate(GetDevice()->IsValidationEnabled()); return options; diff --git a/src/dawn_native/ShaderModule.h b/src/dawn_native/ShaderModule.h index 0653bbfd17..b3fd83974b 100644 --- a/src/dawn_native/ShaderModule.h +++ b/src/dawn_native/ShaderModule.h @@ -83,13 +83,12 @@ namespace dawn_native { bool operator()(const ShaderModuleBase* a, const ShaderModuleBase* b) const; }; - shaderc_spvc::Context* GetContext() { - return &mSpvcContext; - } + shaderc_spvc::Context* GetContext(); + const std::vector& GetSpirv() const; protected: static MaybeError CheckSpvcSuccess(shaderc_spvc_status status, const char* error_msg); - shaderc_spvc::CompileOptions GetCompileOptions(); + shaderc_spvc::CompileOptions GetCompileOptions() const; shaderc_spvc::Context mSpvcContext; @@ -103,9 +102,7 @@ namespace dawn_native { MaybeError ExtractSpirvInfoWithSpvc(); MaybeError ExtractSpirvInfoWithSpirvCross(const spirv_cross::Compiler& compiler); - // TODO(cwallez@chromium.org): The code is only stored for deduplication. We could maybe - // store a cryptographic hash of the code instead? - std::vector mCode; + std::vector mSpirv; ModuleBindingInfo mBindingInfo; std::bitset mUsedVertexAttributes; diff --git a/src/dawn_native/d3d12/ShaderModuleD3D12.cpp b/src/dawn_native/d3d12/ShaderModuleD3D12.cpp index d9289cacc2..e006d6fe53 100644 --- a/src/dawn_native/d3d12/ShaderModuleD3D12.cpp +++ b/src/dawn_native/d3d12/ShaderModuleD3D12.cpp @@ -28,7 +28,7 @@ namespace dawn_native { namespace d3d12 { ResultOrError ShaderModule::Create(Device* device, const ShaderModuleDescriptor* descriptor) { Ref module = AcquireRef(new ShaderModule(device, descriptor)); - DAWN_TRY(module->Initialize(descriptor)); + DAWN_TRY(module->Initialize()); return module.Detach(); } @@ -36,8 +36,9 @@ namespace dawn_native { namespace d3d12 { : ShaderModuleBase(device, descriptor) { } - MaybeError ShaderModule::Initialize(const ShaderModuleDescriptor* descriptor) { - mSpirv.assign(descriptor->code, descriptor->code + descriptor->codeSize); + MaybeError ShaderModule::Initialize() { + const std::vector& spirv = GetSpirv(); + if (GetDevice()->IsToggleEnabled(Toggle::UseSpvc)) { shaderc_spvc::CompileOptions options = GetCompileOptions(); @@ -52,7 +53,7 @@ namespace dawn_native { namespace d3d12 { options.SetHLSLPointSizeCompat(true); DAWN_TRY(CheckSpvcSuccess( - mSpvcContext.InitializeForHlsl(descriptor->code, descriptor->codeSize, options), + mSpvcContext.InitializeForHlsl(spirv.data(), spirv.size(), options), "Unable to initialize instance of spvc")); spirv_cross::Compiler* compiler; @@ -60,14 +61,17 @@ namespace dawn_native { namespace d3d12 { "Unable to get cross compiler")); DAWN_TRY(ExtractSpirvInfo(*compiler)); } else { - spirv_cross::CompilerHLSL compiler(descriptor->code, descriptor->codeSize); + spirv_cross::CompilerHLSL compiler(spirv); DAWN_TRY(ExtractSpirvInfo(compiler)); } return {}; } ResultOrError ShaderModule::GetHLSLSource(PipelineLayout* layout) { - std::unique_ptr compiler_impl; + ASSERT(!IsError()); + const std::vector& spirv = GetSpirv(); + + std::unique_ptr compilerImpl; spirv_cross::CompilerHLSL* compiler = nullptr; if (!GetDevice()->IsToggleEnabled(Toggle::UseSpvc)) { // If these options are changed, the values in DawnSPIRVCrossHLSLFastFuzzer.cpp need to @@ -87,8 +91,8 @@ namespace dawn_native { namespace d3d12 { options_hlsl.point_coord_compat = true; options_hlsl.point_size_compat = true; - compiler_impl = std::make_unique(mSpirv); - compiler = compiler_impl.get(); + compilerImpl = std::make_unique(spirv); + compiler = compilerImpl.get(); compiler->set_common_options(options_glsl); compiler->set_hlsl_options(options_hlsl); } diff --git a/src/dawn_native/d3d12/ShaderModuleD3D12.h b/src/dawn_native/d3d12/ShaderModuleD3D12.h index 289c2dba60..e34d8815a4 100644 --- a/src/dawn_native/d3d12/ShaderModuleD3D12.h +++ b/src/dawn_native/d3d12/ShaderModuleD3D12.h @@ -32,9 +32,7 @@ namespace dawn_native { namespace d3d12 { private: ShaderModule(Device* device, const ShaderModuleDescriptor* descriptor); ~ShaderModule() override = default; - MaybeError Initialize(const ShaderModuleDescriptor* descriptor); - - std::vector mSpirv; + MaybeError Initialize(); }; }} // namespace dawn_native::d3d12 diff --git a/src/dawn_native/metal/ShaderModuleMTL.h b/src/dawn_native/metal/ShaderModuleMTL.h index 53e9f7d484..d4d41abc68 100644 --- a/src/dawn_native/metal/ShaderModuleMTL.h +++ b/src/dawn_native/metal/ShaderModuleMTL.h @@ -51,14 +51,9 @@ namespace dawn_native { namespace metal { private: ShaderModule(Device* device, const ShaderModuleDescriptor* descriptor); ~ShaderModule() override = default; - MaybeError Initialize(const ShaderModuleDescriptor* descriptor); + MaybeError Initialize(); shaderc_spvc::CompileOptions GetMSLCompileOptions(); - - // Calling compile on CompilerMSL somehow changes internal state that makes subsequent - // compiles return invalid MSL. We keep the spirv around and recreate the compiler everytime - // we need to use it. - std::vector mSpirv; }; }} // namespace dawn_native::metal diff --git a/src/dawn_native/metal/ShaderModuleMTL.mm b/src/dawn_native/metal/ShaderModuleMTL.mm index a06adccbbb..d0a8bfc97a 100644 --- a/src/dawn_native/metal/ShaderModuleMTL.mm +++ b/src/dawn_native/metal/ShaderModuleMTL.mm @@ -58,7 +58,7 @@ namespace dawn_native { namespace metal { ResultOrError ShaderModule::Create(Device* device, const ShaderModuleDescriptor* descriptor) { Ref module = AcquireRef(new ShaderModule(device, descriptor)); - DAWN_TRY(module->Initialize(descriptor)); + DAWN_TRY(module->Initialize()); return module.Detach(); } @@ -66,21 +66,22 @@ namespace dawn_native { namespace metal { : ShaderModuleBase(device, descriptor) { } - MaybeError ShaderModule::Initialize(const ShaderModuleDescriptor* descriptor) { - mSpirv.assign(descriptor->code, descriptor->code + descriptor->codeSize); + MaybeError ShaderModule::Initialize() { + const std::vector& spirv = GetSpirv(); + if (GetDevice()->IsToggleEnabled(Toggle::UseSpvc)) { shaderc_spvc::CompileOptions options = GetMSLCompileOptions(); - DAWN_TRY(CheckSpvcSuccess( - mSpvcContext.InitializeForMsl(descriptor->code, descriptor->codeSize, options), - "Unable to initialize instance of spvc")); + DAWN_TRY( + CheckSpvcSuccess(mSpvcContext.InitializeForMsl(spirv.data(), spirv.size(), options), + "Unable to initialize instance of spvc")); spirv_cross::CompilerMSL* compiler; DAWN_TRY(CheckSpvcSuccess(mSpvcContext.GetCompiler(reinterpret_cast(&compiler)), "Unable to get cross compiler")); DAWN_TRY(ExtractSpirvInfo(*compiler)); } else { - spirv_cross::CompilerMSL compiler(mSpirv); + spirv_cross::CompilerMSL compiler(spirv); DAWN_TRY(ExtractSpirvInfo(compiler)); } return {}; @@ -92,13 +93,15 @@ namespace dawn_native { namespace metal { ShaderModule::MetalFunctionData* out) { ASSERT(!IsError()); ASSERT(out); - std::unique_ptr compiler_impl; + const std::vector& spirv = GetSpirv(); + + std::unique_ptr compilerImpl; spirv_cross::CompilerMSL* compiler; if (GetDevice()->IsToggleEnabled(Toggle::UseSpvc)) { // Initializing the compiler is needed every call, because this method uses reflection // to mutate the compiler's IR. DAWN_TRY(CheckSpvcSuccess( - mSpvcContext.InitializeForMsl(mSpirv.data(), mSpirv.size(), GetMSLCompileOptions()), + mSpvcContext.InitializeForMsl(spirv.data(), spirv.size(), GetMSLCompileOptions()), "Unable to initialize instance of spvc")); DAWN_TRY(CheckSpvcSuccess(mSpvcContext.GetCompiler(reinterpret_cast(&compiler)), "Unable to get cross compiler")); @@ -118,8 +121,8 @@ namespace dawn_native { namespace metal { // the shader storage buffer lengths. options_msl.buffer_size_buffer_index = kBufferLengthBufferSlot; - compiler_impl = std::make_unique(mSpirv); - compiler = compiler_impl.get(); + compilerImpl = std::make_unique(spirv); + compiler = compilerImpl.get(); compiler->set_msl_options(options_msl); } diff --git a/src/dawn_native/null/DeviceNull.cpp b/src/dawn_native/null/DeviceNull.cpp index 79da48cf10..701e720f7e 100644 --- a/src/dawn_native/null/DeviceNull.cpp +++ b/src/dawn_native/null/DeviceNull.cpp @@ -125,14 +125,14 @@ namespace dawn_native { namespace null { } ResultOrError Device::CreateShaderModuleImpl( const ShaderModuleDescriptor* descriptor) { - auto module = new ShaderModule(this, descriptor); + Ref module = AcquireRef(new ShaderModule(this, descriptor)); if (IsToggleEnabled(Toggle::UseSpvc)) { shaderc_spvc::CompileOptions options; options.SetValidate(IsValidationEnabled()); shaderc_spvc::Context* context = module->GetContext(); - shaderc_spvc_status status = - context->InitializeForGlsl(descriptor->code, descriptor->codeSize, options); + shaderc_spvc_status status = context->InitializeForGlsl( + module->GetSpirv().data(), module->GetSpirv().size(), options); if (status != shaderc_spvc_status_success) { return DAWN_VALIDATION_ERROR("Unable to initialize instance of spvc"); } @@ -144,10 +144,10 @@ namespace dawn_native { namespace null { } DAWN_TRY(module->ExtractSpirvInfo(*compiler)); } else { - spirv_cross::Compiler compiler(descriptor->code, descriptor->codeSize); + spirv_cross::Compiler compiler(module->GetSpirv()); DAWN_TRY(module->ExtractSpirvInfo(compiler)); } - return module; + return module.Detach(); } ResultOrError Device::CreateSwapChainImpl( const SwapChainDescriptor* descriptor) { diff --git a/src/dawn_native/opengl/ShaderModuleGL.cpp b/src/dawn_native/opengl/ShaderModuleGL.cpp index 86398c9087..8979716751 100644 --- a/src/dawn_native/opengl/ShaderModuleGL.cpp +++ b/src/dawn_native/opengl/ShaderModuleGL.cpp @@ -51,7 +51,7 @@ namespace dawn_native { namespace opengl { ResultOrError ShaderModule::Create(Device* device, const ShaderModuleDescriptor* descriptor) { Ref module = AcquireRef(new ShaderModule(device, descriptor)); - DAWN_TRY(module->Initialize(descriptor)); + DAWN_TRY(module->Initialize()); return module.Detach(); } @@ -67,10 +67,11 @@ namespace dawn_native { namespace opengl { : ShaderModuleBase(device, descriptor) { } - MaybeError ShaderModule::Initialize(const ShaderModuleDescriptor* descriptor) { - std::unique_ptr compiler_impl; - spirv_cross::CompilerGLSL* compiler; + MaybeError ShaderModule::Initialize() { + const std::vector& spirv = GetSpirv(); + std::unique_ptr compilerImpl; + spirv_cross::CompilerGLSL* compiler; if (GetDevice()->IsToggleEnabled(Toggle::UseSpvc)) { // If these options are changed, the values in DawnSPIRVCrossGLSLFastFuzzer.cpp need to // be updated. @@ -90,7 +91,7 @@ namespace dawn_native { namespace opengl { options.SetGLSLLanguageVersion(440); #endif DAWN_TRY(CheckSpvcSuccess( - mSpvcContext.InitializeForGlsl(descriptor->code, descriptor->codeSize, options), + mSpvcContext.InitializeForGlsl(spirv.data(), spirv.size(), options), "Unable to initialize instance of spvc")); DAWN_TRY(CheckSpvcSuccess(mSpvcContext.GetCompiler(reinterpret_cast(&compiler)), "Unable to get cross compiler")); @@ -108,15 +109,14 @@ namespace dawn_native { namespace opengl { // TODO(cwallez@chromium.org): discover the backing context version and use that. #if defined(DAWN_PLATFORM_APPLE) - options.version = 410; + options.version = 410; #else - options.version = 440; + options.version = 440; #endif - compiler_impl = - std::make_unique(descriptor->code, descriptor->codeSize); - compiler = compiler_impl.get(); - compiler->set_common_options(options); + compilerImpl = std::make_unique(spirv); + compiler = compilerImpl.get(); + compiler->set_common_options(options); } DAWN_TRY(ExtractSpirvInfo(*compiler)); diff --git a/src/dawn_native/opengl/ShaderModuleGL.h b/src/dawn_native/opengl/ShaderModuleGL.h index 849f2da112..9e2b5c9e14 100644 --- a/src/dawn_native/opengl/ShaderModuleGL.h +++ b/src/dawn_native/opengl/ShaderModuleGL.h @@ -51,7 +51,7 @@ namespace dawn_native { namespace opengl { private: ShaderModule(Device* device, const ShaderModuleDescriptor* descriptor); ~ShaderModule() override = default; - MaybeError Initialize(const ShaderModuleDescriptor* descriptor); + MaybeError Initialize(); CombinedSamplerInfo mCombinedInfo; std::string mGlslSource; diff --git a/src/dawn_native/vulkan/ShaderModuleVk.cpp b/src/dawn_native/vulkan/ShaderModuleVk.cpp index 8cc8265120..36f9db815b 100644 --- a/src/dawn_native/vulkan/ShaderModuleVk.cpp +++ b/src/dawn_native/vulkan/ShaderModuleVk.cpp @@ -28,7 +28,7 @@ namespace dawn_native { namespace vulkan { Ref module = AcquireRef(new ShaderModule(device, descriptor)); if (!module) return DAWN_VALIDATION_ERROR("Unable to create ShaderModule"); - DAWN_TRY(module->Initialize(descriptor)); + DAWN_TRY(module->Initialize()); return module.Detach(); } @@ -36,14 +36,16 @@ namespace dawn_native { namespace vulkan { : ShaderModuleBase(device, descriptor) { } - MaybeError ShaderModule::Initialize(const ShaderModuleDescriptor* descriptor) { + MaybeError ShaderModule::Initialize() { + const std::vector& spirv = GetSpirv(); + // Use SPIRV-Cross to extract info from the SPIRV even if Vulkan consumes SPIRV. We want to // have a translation step eventually anyway. if (GetDevice()->IsToggleEnabled(Toggle::UseSpvc)) { shaderc_spvc::CompileOptions options = GetCompileOptions(); DAWN_TRY(CheckSpvcSuccess( - mSpvcContext.InitializeForVulkan(descriptor->code, descriptor->codeSize, options), + mSpvcContext.InitializeForVulkan(spirv.data(), spirv.size(), options), "Unable to initialize instance of spvc")); spirv_cross::Compiler* compiler; @@ -51,7 +53,7 @@ namespace dawn_native { namespace vulkan { "Unable to get cross compiler")); DAWN_TRY(ExtractSpirvInfo(*compiler)); } else { - spirv_cross::Compiler compiler(descriptor->code, descriptor->codeSize); + spirv_cross::Compiler compiler(spirv); DAWN_TRY(ExtractSpirvInfo(compiler)); } @@ -69,8 +71,8 @@ namespace dawn_native { namespace vulkan { createInfo.codeSize = vulkanSource.size() * sizeof(uint32_t); createInfo.pCode = vulkanSource.data(); } else { - createInfo.codeSize = descriptor->codeSize * sizeof(uint32_t); - createInfo.pCode = descriptor->code; + createInfo.codeSize = spirv.size() * sizeof(uint32_t); + createInfo.pCode = spirv.data(); } Device* device = ToBackend(GetDevice()); diff --git a/src/dawn_native/vulkan/ShaderModuleVk.h b/src/dawn_native/vulkan/ShaderModuleVk.h index 962ccf8146..720cc5e232 100644 --- a/src/dawn_native/vulkan/ShaderModuleVk.h +++ b/src/dawn_native/vulkan/ShaderModuleVk.h @@ -34,7 +34,7 @@ namespace dawn_native { namespace vulkan { private: ShaderModule(Device* device, const ShaderModuleDescriptor* descriptor); ~ShaderModule() override; - MaybeError Initialize(const ShaderModuleDescriptor* descriptor); + MaybeError Initialize(); VkShaderModule mHandle = VK_NULL_HANDLE; };