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>
diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp
index 6e41e56..cabea0f 100644
--- a/src/dawn_native/Device.cpp
+++ b/src/dawn_native/Device.cpp
@@ -692,6 +692,7 @@
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 605b055..bed169b 100644
--- a/src/dawn_native/ShaderModule.cpp
+++ b/src/dawn_native/ShaderModule.cpp
@@ -590,4 +590,10 @@
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 2c94ef9..096d528 100644
--- a/src/dawn_native/ShaderModule.h
+++ b/src/dawn_native/ShaderModule.h
@@ -81,8 +81,13 @@
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 f6fefff..6a6075d 100644
--- a/src/dawn_native/d3d12/ShaderModuleD3D12.cpp
+++ b/src/dawn_native/d3d12/ShaderModuleD3D12.cpp
@@ -41,7 +41,7 @@
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 ef8dd38..2e447c1 100644
--- a/src/dawn_native/metal/ShaderModuleMTL.h
+++ b/src/dawn_native/metal/ShaderModuleMTL.h
@@ -52,6 +52,8 @@
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 679ea02..5028934 100644
--- a/src/dawn_native/metal/ShaderModuleMTL.mm
+++ b/src/dawn_native/metal/ShaderModuleMTL.mm
@@ -52,25 +52,6 @@
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 @@
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 @@
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 c7586eb..64bde46 100644
--- a/src/dawn_native/null/DeviceNull.cpp
+++ b/src/dawn_native/null/DeviceNull.cpp
@@ -133,15 +133,16 @@
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");
}
diff --git a/src/dawn_native/opengl/ShaderModuleGL.cpp b/src/dawn_native/opengl/ShaderModuleGL.cpp
index ac2f522..67e6ebf 100644
--- a/src/dawn_native/opengl/ShaderModuleGL.cpp
+++ b/src/dawn_native/opengl/ShaderModuleGL.cpp
@@ -76,7 +76,7 @@
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 a9e717c..60c6ba6 100644
--- a/src/dawn_native/vulkan/ShaderModuleVk.cpp
+++ b/src/dawn_native/vulkan/ShaderModuleVk.cpp
@@ -40,7 +40,8 @@
// 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 78b1dee..3896f2a 100644
--- a/src/tests/DawnTest.cpp
+++ b/src/tests/DawnTest.cpp
@@ -522,6 +522,9 @@
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);
