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>
diff --git a/scripts/dawn_features.gni b/scripts/dawn_features.gni
index 5eec131..b72e39e 100644
--- a/scripts/dawn_features.gni
+++ b/scripts/dawn_features.gni
@@ -96,6 +96,10 @@
# 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
diff --git a/src/dawn/native/BUILD.gn b/src/dawn/native/BUILD.gn
index b0651ea..95bf264 100644
--- a/src/dawn/native/BUILD.gn
+++ b/src/dawn/native/BUILD.gn
@@ -151,6 +151,9 @@
":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 @@
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 @@
]
}
- 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 @@
[ "${dawn_swiftshader_dir}/src/Vulkan:swiftshader_libvulkan" ]
defines += [ "DAWN_ENABLE_SWIFTSHADER" ]
}
+ if (dawn_enable_spirv_validation) {
+ defines += [ "DAWN_ENABLE_SPIRV_VALIDATION" ]
+ }
}
if (use_angle) {
diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp
index a0d096a..e924d53 100644
--- a/src/dawn/native/Device.cpp
+++ b/src/dawn/native/Device.cpp
@@ -619,8 +619,9 @@
// 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) {
diff --git a/src/dawn/native/ShaderModule.cpp b/src/dawn/native/ShaderModule.cpp
index 6c04065..2d48f14 100644
--- a/src/dawn/native/ShaderModule.cpp
+++ b/src/dawn/native/ShaderModule.cpp
@@ -311,9 +311,9 @@
#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 @@
}
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 @@
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 @@
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 @@
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 {};
}
diff --git a/src/dawn/native/Texture.cpp b/src/dawn/native/Texture.cpp
index c462009..1ac38ed 100644
--- a/src/dawn/native/Texture.cpp
+++ b/src/dawn/native/Texture.cpp
@@ -84,14 +84,13 @@
"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(
diff --git a/src/dawn/native/vulkan/ShaderModuleVk.cpp b/src/dawn/native/vulkan/ShaderModuleVk.cpp
index 39fd0df..5f55569 100644
--- a/src/dawn/native/vulkan/ShaderModuleVk.cpp
+++ b/src/dawn/native/vulkan/ShaderModuleVk.cpp
@@ -360,8 +360,10 @@
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;
diff --git a/src/dawn/tests/unittests/validation/ShaderModuleValidationTests.cpp b/src/dawn/tests/unittests/validation/ShaderModuleValidationTests.cpp
index d96b910..448bfad 100644
--- a/src/dawn/tests/unittests/validation/ShaderModuleValidationTests.cpp
+++ b/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 @@
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 @@
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) {
diff --git a/src/dawn/utils/BUILD.gn b/src/dawn/utils/BUILD.gn
index e74473d..02820bb 100644
--- a/src/dawn/utils/BUILD.gn
+++ b/src/dawn/utils/BUILD.gn
@@ -21,7 +21,10 @@
###############################################################################
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 @@
"${dawn_root}/src/dawn/wire",
"${dawn_spirv_tools_dir}:spvtools_opt",
]
+
libs = []
frameworks = []
diff --git a/src/dawn/utils/WGPUHelpers.cpp b/src/dawn/utils/WGPUHelpers.cpp
index 8cdf718..d6b1a4a 100644
--- a/src/dawn/utils/WGPUHelpers.cpp
+++ b/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 @@
} // 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 @@
return result;
}
+#endif
wgpu::ShaderModule CreateShaderModule(const wgpu::Device& device, const char* source) {
wgpu::ShaderModuleWGSLDescriptor wgslDesc;
diff --git a/src/dawn/utils/WGPUHelpers.h b/src/dawn/utils/WGPUHelpers.h
index b33f9d7..d56dc59 100644
--- a/src/dawn/utils/WGPUHelpers.h
+++ b/src/dawn/utils/WGPUHelpers.h
@@ -27,7 +27,9 @@
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,
diff --git a/src/tint/BUILD.gn b/src/tint/BUILD.gn
index f3fd25b..941fc83 100644
--- a/src/tint/BUILD.gn
+++ b/src/tint/BUILD.gn
@@ -1024,12 +1024,19 @@
} 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" ]
+ }
}
}
