Making SPIR-V ingestion and validation optional

Browsers won't be exposing the ability to pass SPIR-V shaders, and the
ability to consume and validate them is adding a non-trivial amount to
the browser binary size on platforms like Android. To avoid that
overhead, this change puts those features behind a flag so that browser
usage can easily omit them.

Bug: dawn:286
Change-Id: Idf70683f2c4ccf479b723c00ba6914e27e4f765f
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/117241
Commit-Queue: Brandon Jones <bajones@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

scripts/dawn_features.gni

@@ -96,6 +96,10 @@ declare_args() {
   # assume to have one present at the system level.
   dawn_enable_vulkan_loader =
       dawn_enable_vulkan && (is_mac || (is_linux && !is_android))
+
+  # Disable SPIR-V validation on Android because it adds a significant amount
+  # to the binary size, and Tint's output should be well-formed.
+  dawn_enable_spirv_validation = dawn_enable_vulkan && !is_android
 }
 
 # UWP only supports CoreWindow for windowing

src/dawn/native/BUILD.gn

@@ -151,6 +151,9 @@ source_set("sources") {
     ":utils_gen",
     "${dawn_root}/src/dawn/common",
     "${dawn_root}/src/tint:libtint",
+
+    # TODO(dawn:286): These should only be necessary if SPIR-V validation is
+    # enabled with dawn_enable_spirv_validation
     "${dawn_spirv_tools_dir}:spvtools_opt",
     "${dawn_spirv_tools_dir}:spvtools_val",
   ]
@@ -158,7 +161,10 @@ source_set("sources") {
   libs = []
   data_deps = []
 
-  configs += [ ":internal" ]
+  configs += [
+    ":internal",
+    "${dawn_root}/src/tint:tint_public_config",
+  ]
 
   # Enable -Wglobal-constructors here only, instead of in internal_config,
   # because gtest and some other targets don't build with it.
@@ -527,7 +533,8 @@ source_set("sources") {
     ]
   }
 
-  if (dawn_enable_opengl || dawn_enable_vulkan) {
+  if ((dawn_enable_opengl || dawn_enable_vulkan) &&
+      dawn_enable_spirv_validation) {
     sources += [
       "SpirvValidation.cpp",
       "SpirvValidation.h",
@@ -726,6 +733,9 @@ source_set("sources") {
           [ "${dawn_swiftshader_dir}/src/Vulkan:swiftshader_libvulkan" ]
       defines += [ "DAWN_ENABLE_SWIFTSHADER" ]
     }
+    if (dawn_enable_spirv_validation) {
+      defines += [ "DAWN_ENABLE_SPIRV_VALIDATION" ]
+    }
   }
 
   if (use_angle) {

src/dawn/native/Device.cpp

@@ -619,8 +619,9 @@ BlobCache* DeviceBase::GetBlobCache() {
     // generate cache keys. We can lift the dependency once we also cache frontend parsing,
     // transformations, and reflection.
     return mAdapter->GetInstance()->GetBlobCache(!IsToggleEnabled(Toggle::DisableBlobCache));
-#endif
+#else
     return mAdapter->GetInstance()->GetBlobCache(false);
+#endif
 }
 
 Blob DeviceBase::LoadCachedBlob(const CacheKey& key) {

src/dawn/native/ShaderModule.cpp

@@ -311,9 +311,9 @@ ResultOrError<tint::Program> ParseWGSL(const tint::Source::File* file,
 #endif
 }
 
+#if TINT_BUILD_SPV_READER
 ResultOrError<tint::Program> ParseSPIRV(const std::vector<uint32_t>& spirv,
                                         OwnedCompilationMessages* outMessages) {
-#if TINT_BUILD_SPV_READER
     tint::Program program = tint::reader::spirv::Parse(spirv);
     if (outMessages != nullptr) {
         DAWN_TRY(outMessages->AddMessages(program.Diagnostics()));
@@ -324,11 +324,8 @@ ResultOrError<tint::Program> ParseSPIRV(const std::vector<uint32_t>& spirv,
     }
 
     return std::move(program);
-#else
-    return DAWN_VALIDATION_ERROR("TINT_BUILD_SPV_READER is not defined.");
-
-#endif
 }
+#endif  // TINT_BUILD_SPV_READER
 
 std::vector<uint64_t> GetBindGroupMinBufferSizes(const BindingGroupInfoMap& shaderBindings,
                                                  const BindGroupLayoutBase* layout) {
@@ -903,17 +900,23 @@ MaybeError ValidateAndParseShaderModule(DeviceBase* device,
     DAWN_INVALID_IF(chainedDescriptor == nullptr,
                     "Shader module descriptor missing chained descriptor");
 
-    // For now only a single SPIRV or WGSL subdescriptor is allowed.
+// For now only a single WGSL (or SPIRV, if enabled) subdescriptor is allowed.
+#if TINT_BUILD_SPV_READER
     DAWN_TRY(ValidateSingleSType(chainedDescriptor, wgpu::SType::ShaderModuleSPIRVDescriptor,
                                  wgpu::SType::ShaderModuleWGSLDescriptor));
+#else
+    DAWN_TRY(ValidateSingleSType(chainedDescriptor, wgpu::SType::ShaderModuleWGSLDescriptor));
+#endif
 
     ScopedTintICEHandler scopedICEHandler(device);
 
-    const ShaderModuleSPIRVDescriptor* spirvDesc = nullptr;
-    FindInChain(chainedDescriptor, &spirvDesc);
     const ShaderModuleWGSLDescriptor* wgslDesc = nullptr;
     FindInChain(chainedDescriptor, &wgslDesc);
 
+#if TINT_BUILD_SPV_READER
+    const ShaderModuleSPIRVDescriptor* spirvDesc = nullptr;
+    FindInChain(chainedDescriptor, &spirvDesc);
+
     // We have a temporary toggle to force the SPIRV ingestion to go through a WGSL
     // intermediate step. It is done by switching the spirvDesc for a wgslDesc below.
     ShaderModuleWGSLDescriptor newWgslDesc;
@@ -937,7 +940,7 @@ MaybeError ValidateAndParseShaderModule(DeviceBase* device,
         device->EmitLog(
             WGPULoggingType_Info,
             "Toggle::ForceWGSLStep skipped because TINT_BUILD_WGSL_WRITER is not defined\n");
-#endif
+#endif  // TINT_BUILD_WGSL_WRITER
     }
 
     if (spirvDesc) {
@@ -947,20 +950,25 @@ MaybeError ValidateAndParseShaderModule(DeviceBase* device,
         tint::Program program;
         DAWN_TRY_ASSIGN(program, ParseSPIRV(spirv, outMessages));
         parseResult->tintProgram = std::make_unique<tint::Program>(std::move(program));
-    } else if (wgslDesc) {
-        auto tintSource = std::make_unique<TintSource>("", wgslDesc->source);
 
-        if (device->IsToggleEnabled(Toggle::DumpShaders)) {
-            std::ostringstream dumpedMsg;
-            dumpedMsg << "// Dumped WGSL:" << std::endl << wgslDesc->source;
-            device->EmitLog(WGPULoggingType_Info, dumpedMsg.str().c_str());
-        }
-
-        tint::Program program;
-        DAWN_TRY_ASSIGN(program, ParseWGSL(&tintSource->file, outMessages));
-        parseResult->tintProgram = std::make_unique<tint::Program>(std::move(program));
-        parseResult->tintSource = std::move(tintSource);
+        return {};
     }
+#endif  // TINT_BUILD_SPV_READER
+
+    ASSERT(wgslDesc != nullptr);
+
+    auto tintSource = std::make_unique<TintSource>("", wgslDesc->source);
+
+    if (device->IsToggleEnabled(Toggle::DumpShaders)) {
+        std::ostringstream dumpedMsg;
+        dumpedMsg << "// Dumped WGSL:" << std::endl << wgslDesc->source;
+        device->EmitLog(WGPULoggingType_Info, dumpedMsg.str().c_str());
+    }
+
+    tint::Program program;
+    DAWN_TRY_ASSIGN(program, ParseWGSL(&tintSource->file, outMessages));
+    parseResult->tintProgram = std::make_unique<tint::Program>(std::move(program));
+    parseResult->tintSource = std::move(tintSource);
 
     return {};
 }

src/dawn/native/Texture.cpp

@@ -84,14 +84,13 @@ MaybeError ValidateCanViewTextureAs(const DeviceBase* device,
             "The formats must be compatible, and the view format "
             "must be passed in the list of view formats on texture creation.",
             viewFormat.format, format.format);
-    } else {
-        // The view format is compatible, but not in the list.
-        return DAWN_VALIDATION_ERROR(
-            "%s was not created with the texture view format (%s) "
-            "in the list of compatible view formats.",
-            texture, viewFormat.format);
     }
-    return {};
+
+    // The view format is compatible, but not in the list.
+    return DAWN_VALIDATION_ERROR(
+        "%s was not created with the texture view format (%s) "
+        "in the list of compatible view formats.",
+        texture, viewFormat.format);
 }
 
 bool IsTextureViewDimensionCompatibleWithTextureDimension(

src/dawn/native/vulkan/ShaderModuleVk.cpp

@@ -360,8 +360,10 @@ ResultOrError<ShaderModule::ModuleAndSpirv> ShaderModule::GetHandleAndSpirv(
             return result;
         });
 
+#ifdef DAWN_ENABLE_SPIRV_VALIDATION
     DAWN_TRY(ValidateSpirv(GetDevice(), compilation->spirv.data(), compilation->spirv.size(),
                            GetDevice()->IsToggleEnabled(Toggle::DumpShaders)));
+#endif
 
     VkShaderModuleCreateInfo createInfo;
     createInfo.sType = VK_STRUCTURE_TYPE_SHADER_MODULE_CREATE_INFO;

src/dawn/tests/unittests/validation/ShaderModuleValidationTests.cpp

@@ -23,6 +23,7 @@
 
 class ShaderModuleValidationTest : public ValidationTest {};
 
+#if TINT_BUILD_SPV_READER
 // Test case with a simpler shader that should successfully be created
 TEST_F(ShaderModuleValidationTest, CreationSuccess) {
     const char* shader = R"(
@@ -55,13 +56,6 @@ TEST_F(ShaderModuleValidationTest, CreationSuccess) {
     utils::CreateShaderModuleFromASM(device, shader);
 }
 
-// Test that it is invalid to create a shader module with no chained descriptor. (It must be
-// WGSL or SPIRV, not empty)
-TEST_F(ShaderModuleValidationTest, NoChainedDescriptor) {
-    wgpu::ShaderModuleDescriptor desc = {};
-    ASSERT_DEVICE_ERROR(device.CreateShaderModule(&desc));
-}
-
 // Test that it is not allowed to use combined texture and sampler.
 TEST_F(ShaderModuleValidationTest, CombinedTextureAndSampler) {
     // SPIR-V ASM produced by glslang for the following fragment shader:
@@ -143,6 +137,14 @@ TEST_F(ShaderModuleValidationTest, MultisampledArrayTexture) {
 
     ASSERT_DEVICE_ERROR(utils::CreateShaderModuleFromASM(device, shader));
 }
+#endif  // TINT_BUILD_SPV_READER
+
+// Test that it is invalid to create a shader module with no chained descriptor. (It must be
+// WGSL or SPIRV, not empty)
+TEST_F(ShaderModuleValidationTest, NoChainedDescriptor) {
+    wgpu::ShaderModuleDescriptor desc = {};
+    ASSERT_DEVICE_ERROR(device.CreateShaderModule(&desc));
+}
 
 // Tests that shader module compilation messages can be queried.
 TEST_F(ShaderModuleValidationTest, GetCompilationMessages) {

src/dawn/utils/BUILD.gn

@@ -21,7 +21,10 @@ import("${dawn_root}/scripts/dawn_features.gni")
 ###############################################################################
 
 static_library("utils") {
-  configs += [ "${dawn_root}/src/dawn/common:internal_config" ]
+  configs += [
+    "${dawn_root}/src/dawn/common:internal_config",
+    "${dawn_root}/src/tint:tint_public_config",
+  ]
 
   sources = [
     "ComboRenderBundleEncoderDescriptor.cpp",
@@ -51,6 +54,7 @@ static_library("utils") {
     "${dawn_root}/src/dawn/wire",
     "${dawn_spirv_tools_dir}:spvtools_opt",
   ]
+
   libs = []
   frameworks = []
 

src/dawn/utils/WGPUHelpers.cpp

@@ -24,7 +24,9 @@
 #include "dawn/common/Log.h"
 #include "dawn/common/Numeric.h"
 
+#if TINT_BUILD_SPV_READER
 #include "spirv-tools/optimizer.hpp"
+#endif
 
 namespace {
 std::array<float, 12> kYuvToRGBMatrixBT709 = {1.164384f, 0.0f,       1.792741f,  -0.972945f,
@@ -38,6 +40,7 @@ std::array<float, 7> kGammaEncodeSrgb = {1 / 2.4, 1.137119, 0.0, 12.92, 0.003130
 }  // namespace
 
 namespace utils {
+#if TINT_BUILD_SPV_READER
 wgpu::ShaderModule CreateShaderModuleFromASM(const wgpu::Device& device, const char* source) {
     // Use SPIRV-Tools's C API to assemble the SPIR-V assembly text to binary. Because the types
     // aren't RAII, we don't return directly on success and instead always go through the code
@@ -73,6 +76,7 @@ wgpu::ShaderModule CreateShaderModuleFromASM(const wgpu::Device& device, const c
 
     return result;
 }
+#endif
 
 wgpu::ShaderModule CreateShaderModule(const wgpu::Device& device, const char* source) {
     wgpu::ShaderModuleWGSLDescriptor wgslDesc;

src/dawn/utils/WGPUHelpers.h

@@ -27,7 +27,9 @@ namespace utils {
 
 enum Expectation { Success, Failure };
 
+#if TINT_BUILD_SPV_READER
 wgpu::ShaderModule CreateShaderModuleFromASM(const wgpu::Device& device, const char* source);
+#endif
 wgpu::ShaderModule CreateShaderModule(const wgpu::Device& device, const char* source);
 
 wgpu::Buffer CreateBufferFromData(const wgpu::Device& device,

src/tint/BUILD.gn

@@ -1024,12 +1024,19 @@ if (tint_build_unittests) {
     } else {
       sources = [ "test_main.cc" ]
       configs += [ ":tint_unittests_config" ]
-      deps += [
-        ":libtint",
-        ":tint_unittests_hlsl_writer_src",
-        ":tint_unittests_msl_writer_src",
-        ":tint_unittests_spv_reader_src",
-      ]
+      deps += [ ":libtint" ]
+
+      if (tint_build_hlsl_writer) {
+        deps += [ ":tint_unittests_hlsl_writer_src" ]
+      }
+
+      if (tint_build_msl_writer) {
+        deps += [ ":tint_unittests_msl_writer_src" ]
+      }
+
+      if (tint_build_spv_reader) {
+        deps += [ ":tint_unittests_spv_reader_src" ]
+      }
     }
   }