diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp index 6e41e56414..cabea0fbd8 100644 --- a/src/dawn_native/Device.cpp +++ b/src/dawn_native/Device.cpp @@ -692,6 +692,7 @@ namespace dawn_native { void DeviceBase::SetDefaultToggles() { // Sets the default-enabled toggles mTogglesSet.SetToggle(Toggle::LazyClearResourceOnFirstUse, true); + mTogglesSet.SetToggle(Toggle::UseSpvc, true); } // Implementation details of object creation diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp index 605b055fec..bed169bb39 100644 --- a/src/dawn_native/ShaderModule.cpp +++ b/src/dawn_native/ShaderModule.cpp @@ -590,4 +590,10 @@ namespace dawn_native { return {}; } + shaderc_spvc::CompileOptions ShaderModuleBase::GetCompileOptions() { + shaderc_spvc::CompileOptions options; + options.SetValidate(GetDevice()->IsValidationEnabled()); + return options; + } + } // namespace dawn_native diff --git a/src/dawn_native/ShaderModule.h b/src/dawn_native/ShaderModule.h index 2c94ef9f1b..096d528ee1 100644 --- a/src/dawn_native/ShaderModule.h +++ b/src/dawn_native/ShaderModule.h @@ -81,8 +81,13 @@ namespace dawn_native { bool operator()(const ShaderModuleBase* a, const ShaderModuleBase* b) const; }; + shaderc_spvc::Context* GetContext() { + return &mSpvcContext; + } + protected: static MaybeError CheckSpvcSuccess(shaderc_spvc_status status, const char* error_msg); + shaderc_spvc::CompileOptions GetCompileOptions(); shaderc_spvc::Context mSpvcContext; diff --git a/src/dawn_native/d3d12/ShaderModuleD3D12.cpp b/src/dawn_native/d3d12/ShaderModuleD3D12.cpp index f6fefff819..6a6075da86 100644 --- a/src/dawn_native/d3d12/ShaderModuleD3D12.cpp +++ b/src/dawn_native/d3d12/ShaderModuleD3D12.cpp @@ -41,7 +41,7 @@ namespace dawn_native { namespace d3d12 { MaybeError ShaderModule::Initialize(const ShaderModuleDescriptor* descriptor) { mSpirv.assign(descriptor->code, descriptor->code + descriptor->codeSize); if (GetDevice()->IsToggleEnabled(Toggle::UseSpvc)) { - shaderc_spvc::CompileOptions options; + shaderc_spvc::CompileOptions options = GetCompileOptions(); options.SetHLSLShaderModel(51); // PointCoord and PointSize are not supported in HLSL diff --git a/src/dawn_native/metal/ShaderModuleMTL.h b/src/dawn_native/metal/ShaderModuleMTL.h index ef8dd38f4e..2e447c1796 100644 --- a/src/dawn_native/metal/ShaderModuleMTL.h +++ b/src/dawn_native/metal/ShaderModuleMTL.h @@ -52,6 +52,8 @@ namespace dawn_native { namespace metal { ShaderModule(Device* device, const ShaderModuleDescriptor* descriptor); MaybeError Initialize(const ShaderModuleDescriptor* descriptor); + 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. diff --git a/src/dawn_native/metal/ShaderModuleMTL.mm b/src/dawn_native/metal/ShaderModuleMTL.mm index 679ea021e6..5028934c68 100644 --- a/src/dawn_native/metal/ShaderModuleMTL.mm +++ b/src/dawn_native/metal/ShaderModuleMTL.mm @@ -52,25 +52,6 @@ namespace dawn_native { namespace metal { return shaderc_spvc_execution_model_invalid; } } - - shaderc_spvc::CompileOptions GetMSLCompileOptions() { - // If these options are changed, the values in DawnSPIRVCrossGLSLFastFuzzer.cpp need to - // be updated. - shaderc_spvc::CompileOptions options; - - // Disable PointSize builtin for https://bugs.chromium.org/p/dawn/issues/detail?id=146 - // Because Metal will reject PointSize builtin if the shader is compiled into a render - // pipeline that uses a non-point topology. - // TODO (hao.x.li@intel.com): Remove this once WebGPU requires there is no - // gl_PointSize builtin (https://github.com/gpuweb/gpuweb/issues/332). - options.SetMSLEnablePointSizeBuiltIn(false); - - // Always use vertex buffer 30 (the last one in the vertex buffer table) to contain - // the shader storage buffer lengths. - options.SetMSLBufferSizeBufferIndex(kBufferLengthBufferSlot); - - return options; - } } // namespace // static @@ -90,9 +71,10 @@ namespace dawn_native { namespace metal { MaybeError ShaderModule::Initialize(const ShaderModuleDescriptor* descriptor) { mSpirv.assign(descriptor->code, descriptor->code + descriptor->codeSize); if (GetDevice()->IsToggleEnabled(Toggle::UseSpvc)) { + shaderc_spvc::CompileOptions options = GetMSLCompileOptions(); + DAWN_TRY(CheckSpvcSuccess( - mSpvcContext.InitializeForMsl(descriptor->code, descriptor->codeSize, - GetMSLCompileOptions()), + mSpvcContext.InitializeForMsl(descriptor->code, descriptor->codeSize, options), "Unable to initialize instance of spvc")); spirv_cross::CompilerMSL* compiler; @@ -244,4 +226,23 @@ namespace dawn_native { namespace metal { return {}; } + shaderc_spvc::CompileOptions ShaderModule::GetMSLCompileOptions() { + // If these options are changed, the values in DawnSPIRVCrossGLSLFastFuzzer.cpp need to + // be updated. + shaderc_spvc::CompileOptions options = GetCompileOptions(); + + // Disable PointSize builtin for https://bugs.chromium.org/p/dawn/issues/detail?id=146 + // Because Metal will reject PointSize builtin if the shader is compiled into a render + // pipeline that uses a non-point topology. + // TODO (hao.x.li@intel.com): Remove this once WebGPU requires there is no + // gl_PointSize builtin (https://github.com/gpuweb/gpuweb/issues/332). + options.SetMSLEnablePointSizeBuiltIn(false); + + // Always use vertex buffer 30 (the last one in the vertex buffer table) to contain + // the shader storage buffer lengths. + options.SetMSLBufferSizeBufferIndex(kBufferLengthBufferSlot); + + return options; + } + }} // namespace dawn_native::metal diff --git a/src/dawn_native/null/DeviceNull.cpp b/src/dawn_native/null/DeviceNull.cpp index c7586eba6d..64bde466e8 100644 --- a/src/dawn_native/null/DeviceNull.cpp +++ b/src/dawn_native/null/DeviceNull.cpp @@ -133,15 +133,16 @@ namespace dawn_native { namespace null { if (IsToggleEnabled(Toggle::UseSpvc)) { shaderc_spvc::CompileOptions options; - shaderc_spvc::Context context; + options.SetValidate(IsValidationEnabled()); + shaderc_spvc::Context* context = module->GetContext(); shaderc_spvc_status status = - context.InitializeForGlsl(descriptor->code, descriptor->codeSize, options); + context->InitializeForGlsl(descriptor->code, descriptor->codeSize, options); if (status != shaderc_spvc_status_success) { return DAWN_VALIDATION_ERROR("Unable to initialize instance of spvc"); } spirv_cross::Compiler* compiler; - status = context.GetCompiler(reinterpret_cast(&compiler)); + status = context->GetCompiler(reinterpret_cast(&compiler)); if (status != shaderc_spvc_status_success) { return DAWN_VALIDATION_ERROR("Unable to get cross compiler"); } diff --git a/src/dawn_native/opengl/ShaderModuleGL.cpp b/src/dawn_native/opengl/ShaderModuleGL.cpp index ac2f52269f..67e6ebf38c 100644 --- a/src/dawn_native/opengl/ShaderModuleGL.cpp +++ b/src/dawn_native/opengl/ShaderModuleGL.cpp @@ -76,7 +76,7 @@ namespace dawn_native { namespace opengl { if (GetDevice()->IsToggleEnabled(Toggle::UseSpvc)) { // If these options are changed, the values in DawnSPIRVCrossGLSLFastFuzzer.cpp need to // be updated. - shaderc_spvc::CompileOptions options; + shaderc_spvc::CompileOptions options = GetCompileOptions(); // The range of Z-coordinate in the clipping volume of OpenGL is [-w, w], while it is // [0, w] in D3D12, Metal and Vulkan, so we should normalize it in shaders in all diff --git a/src/dawn_native/vulkan/ShaderModuleVk.cpp b/src/dawn_native/vulkan/ShaderModuleVk.cpp index a9e717ce9c..60c6ba6e19 100644 --- a/src/dawn_native/vulkan/ShaderModuleVk.cpp +++ b/src/dawn_native/vulkan/ShaderModuleVk.cpp @@ -40,7 +40,8 @@ namespace dawn_native { namespace vulkan { // 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; + shaderc_spvc::CompileOptions options = GetCompileOptions(); + DAWN_TRY(CheckSpvcSuccess( mSpvcContext.InitializeForVulkan(descriptor->code, descriptor->codeSize, options), "Unable to initialize instance of spvc")); diff --git a/src/tests/DawnTest.cpp b/src/tests/DawnTest.cpp index 78b1deeb19..3896f2ad75 100644 --- a/src/tests/DawnTest.cpp +++ b/src/tests/DawnTest.cpp @@ -522,6 +522,9 @@ void DawnTestBase::SetUp() { if (gTestEnv->IsSpvcBeingUsed()) { ASSERT(gTestEnv->GetInstance()->GetToggleInfo(kUseSpvcToggle) != nullptr); deviceDescriptor.forceEnabledToggles.push_back(kUseSpvcToggle); + } else { + ASSERT(gTestEnv->GetInstance()->GetToggleInfo(kUseSpvcToggle) != nullptr); + deviceDescriptor.forceDisabledToggles.push_back(kUseSpvcToggle); } backendDevice = mBackendAdapter.CreateDevice(&deviceDescriptor);