Turn on using spvc by default

BUG=dawn:291

Change-Id: I9ebf34388abc6f5ff443a430f5bd79507c095520
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/15463
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

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

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

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;
 

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

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.

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,
+                mSpvcContext.InitializeForMsl(descriptor->code, descriptor->codeSize, options),
-                                              GetMSLCompileOptions()),
                 "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

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<void**>(&compiler));
+            status = context->GetCompiler(reinterpret_cast<void**>(&compiler));
             if (status != shaderc_spvc_status_success) {
                 return DAWN_VALIDATION_ERROR("Unable to get cross compiler");
             }

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

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"));

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);