Add Overrides implementation for OpenGL/OpenGLES backend
As we are using SubstituteOverride, it is easy to add
overridable constants support for OpenGL/OpenGLES.
Also add validate workgroup size for null backend.
Bug: dawn:1537, dawn:1504
Change-Id: I293f10b9a6c606aee6c0ed25b1d966bc56a0b88d
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/101800
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Shrek Shao <shrekshao@google.com>
diff --git a/src/dawn/native/d3d12/ShaderModuleD3D12.cpp b/src/dawn/native/d3d12/ShaderModuleD3D12.cpp
index f67167e..a5392a1 100644
--- a/src/dawn/native/d3d12/ShaderModuleD3D12.cpp
+++ b/src/dawn/native/d3d12/ShaderModuleD3D12.cpp
@@ -327,8 +327,8 @@
}
if (r.substituteOverrideConfig) {
- // This needs to run after SingleEntryPoint transform to get rid of overrides not used for
- // the current entry point.
+ // This needs to run after SingleEntryPoint transform which removes unused overrides for
+ // current entry point.
transformManager.Add<tint::transform::SubstituteOverride>();
transformInputs.Add<tint::transform::SubstituteOverride::Config>(
std::move(r.substituteOverrideConfig).value());
diff --git a/src/dawn/native/metal/ShaderModuleMTL.mm b/src/dawn/native/metal/ShaderModuleMTL.mm
index b0f59a3..e07cd1d 100644
--- a/src/dawn/native/metal/ShaderModuleMTL.mm
+++ b/src/dawn/native/metal/ShaderModuleMTL.mm
@@ -206,8 +206,8 @@
}
if (r.substituteOverrideConfig) {
- // This needs to run after SingleEntryPoint transform to get rid of overrides not
- // used for the current entry point.
+ // This needs to run after SingleEntryPoint transform which removes unused overrides
+ // for current entry point.
transformManager.Add<tint::transform::SubstituteOverride>();
transformInputs.Add<tint::transform::SubstituteOverride::Config>(
std::move(r.substituteOverrideConfig).value());
diff --git a/src/dawn/native/null/DeviceNull.cpp b/src/dawn/native/null/DeviceNull.cpp
index 79d5f66..badb570 100644
--- a/src/dawn/native/null/DeviceNull.cpp
+++ b/src/dawn/native/null/DeviceNull.cpp
@@ -22,6 +22,9 @@
#include "dawn/native/ErrorData.h"
#include "dawn/native/Instance.h"
#include "dawn/native/Surface.h"
+#include "dawn/native/TintUtils.h"
+
+#include "tint/tint.h"
namespace dawn::native::null {
@@ -383,6 +386,40 @@
// ComputePipeline
MaybeError ComputePipeline::Initialize() {
+ const ProgrammableStage& computeStage = GetStage(SingleShaderStage::Compute);
+
+ tint::Program transformedProgram;
+ const tint::Program* program;
+ if (!computeStage.metadata->overrides.empty()) {
+ tint::transform::Manager transformManager;
+ tint::transform::DataMap transformInputs;
+
+ transformManager.Add<tint::transform::SingleEntryPoint>();
+ transformInputs.Add<tint::transform::SingleEntryPoint::Config>(
+ computeStage.entryPoint.c_str());
+
+ // This needs to run after SingleEntryPoint transform which removes unused overrides for
+ // current entry point.
+ transformManager.Add<tint::transform::SubstituteOverride>();
+ transformInputs.Add<tint::transform::SubstituteOverride::Config>(
+ BuildSubstituteOverridesTransformConfig(computeStage));
+
+ DAWN_TRY_ASSIGN(transformedProgram,
+ RunTransforms(&transformManager, computeStage.module->GetTintProgram(),
+ transformInputs, nullptr, nullptr));
+
+ program = &transformedProgram;
+ } else {
+ program = computeStage.module->GetTintProgram();
+ }
+
+ // Do the workgroup size validation as it is actually backend agnostic.
+ const CombinedLimits& limits = GetDevice()->GetLimits();
+ Extent3D _;
+ DAWN_TRY_ASSIGN(
+ _, ValidateComputeStageWorkgroupSize(*program, computeStage.entryPoint.c_str(),
+ LimitsForCompilationRequest::Create(limits.v1)));
+
return {};
}
diff --git a/src/dawn/native/opengl/ShaderModuleGL.cpp b/src/dawn/native/opengl/ShaderModuleGL.cpp
index 172d401..caaf863 100644
--- a/src/dawn/native/opengl/ShaderModuleGL.cpp
+++ b/src/dawn/native/opengl/ShaderModuleGL.cpp
@@ -57,13 +57,16 @@
using BindingMap = std::unordered_map<tint::sem::BindingPoint, tint::sem::BindingPoint>;
-#define GLSL_COMPILATION_REQUEST_MEMBERS(X) \
- X(const tint::Program*, inputProgram) \
- X(std::string, entryPointName) \
- X(tint::transform::MultiplanarExternalTexture::BindingsMap, externalTextureBindings) \
- X(BindingMap, glBindings) \
- X(opengl::OpenGLVersion::Standard, glVersionStandard) \
- X(uint32_t, glVersionMajor) \
+#define GLSL_COMPILATION_REQUEST_MEMBERS(X) \
+ X(const tint::Program*, inputProgram) \
+ X(std::string, entryPointName) \
+ X(SingleShaderStage, stage) \
+ X(tint::transform::MultiplanarExternalTexture::BindingsMap, externalTextureBindings) \
+ X(BindingMap, glBindings) \
+ X(std::optional<tint::transform::SubstituteOverride::Config>, substituteOverrideConfig) \
+ X(LimitsForCompilationRequest, limits) \
+ X(opengl::OpenGLVersion::Standard, glVersionStandard) \
+ X(uint32_t, glVersionMajor) \
X(uint32_t, glVersionMinor)
DAWN_MAKE_CACHE_REQUEST(GLSLCompilationRequest, GLSL_COMPILATION_REQUEST_MEMBERS);
@@ -168,11 +171,21 @@
}
}
+ std::optional<tint::transform::SubstituteOverride::Config> substituteOverrideConfig;
+ if (!programmableStage.metadata->overrides.empty()) {
+ substituteOverrideConfig = BuildSubstituteOverridesTransformConfig(programmableStage);
+ }
+
+ const CombinedLimits& limits = GetDevice()->GetLimits();
+
GLSLCompilationRequest req = {};
req.inputProgram = GetTintProgram();
+ req.stage = stage;
req.entryPointName = programmableStage.entryPoint;
req.externalTextureBindings = BuildExternalTextureTransformBindings(layout);
req.glBindings = std::move(glBindings);
+ req.substituteOverrideConfig = std::move(substituteOverrideConfig);
+ req.limits = LimitsForCompilationRequest::Create(limits.v1);
req.glVersionStandard = version.GetStandard();
req.glVersionMajor = version.GetMajor();
req.glVersionMinor = version.GetMinor();
@@ -190,10 +203,27 @@
std::move(r.externalTextureBindings));
}
+ if (r.substituteOverrideConfig) {
+ transformManager.Add<tint::transform::SingleEntryPoint>();
+ transformInputs.Add<tint::transform::SingleEntryPoint::Config>(r.entryPointName);
+ // This needs to run after SingleEntryPoint transform which removes unused overrides
+ // for current entry point.
+ transformManager.Add<tint::transform::SubstituteOverride>();
+ transformInputs.Add<tint::transform::SubstituteOverride::Config>(
+ std::move(r.substituteOverrideConfig).value());
+ }
+
tint::Program program;
DAWN_TRY_ASSIGN(program, RunTransforms(&transformManager, r.inputProgram,
transformInputs, nullptr, nullptr));
+ if (r.stage == SingleShaderStage::Compute) {
+ // Validate workgroup size after program runs transforms.
+ Extent3D _;
+ DAWN_TRY_ASSIGN(_, ValidateComputeStageWorkgroupSize(
+ program, r.entryPointName.c_str(), r.limits));
+ }
+
tint::writer::glsl::Options tintOptions;
tintOptions.version = tint::writer::glsl::Version(ToTintGLStandard(r.glVersionStandard),
r.glVersionMajor, r.glVersionMinor);
diff --git a/src/dawn/native/vulkan/ShaderModuleVk.cpp b/src/dawn/native/vulkan/ShaderModuleVk.cpp
index 6588627..0ec0092 100644
--- a/src/dawn/native/vulkan/ShaderModuleVk.cpp
+++ b/src/dawn/native/vulkan/ShaderModuleVk.cpp
@@ -311,8 +311,8 @@
r.newBindingsMap);
}
if (r.substituteOverrideConfig) {
- // This needs to run after SingleEntryPoint transform to get rid of overrides not
- // used for the current entry point.
+ // This needs to run after SingleEntryPoint transform which removes unused overrides
+ // for current entry point.
transformManager.Add<tint::transform::SubstituteOverride>();
transformInputs.Add<tint::transform::SubstituteOverride::Config>(
std::move(r.substituteOverrideConfig).value());
diff --git a/src/dawn/tests/end2end/ShaderTests.cpp b/src/dawn/tests/end2end/ShaderTests.cpp
index 45ba077..3ce2759 100644
--- a/src/dawn/tests/end2end/ShaderTests.cpp
+++ b/src/dawn/tests/end2end/ShaderTests.cpp
@@ -395,9 +395,6 @@
// Test overridable constants without numeric identifiers
TEST_P(ShaderTests, OverridableConstants) {
- DAWN_TEST_UNSUPPORTED_IF(IsOpenGL());
- DAWN_TEST_UNSUPPORTED_IF(IsOpenGLES());
-
uint32_t const kCount = 11;
std::vector<uint32_t> expected(kCount);
std::iota(expected.begin(), expected.end(), 0);
@@ -473,9 +470,6 @@
// Test one shader shared by two pipelines with different constants overridden
TEST_P(ShaderTests, OverridableConstantsSharedShader) {
- DAWN_TEST_UNSUPPORTED_IF(IsOpenGL());
- DAWN_TEST_UNSUPPORTED_IF(IsOpenGLES());
-
std::vector<uint32_t> expected1{1};
wgpu::Buffer buffer1 = CreateBuffer(expected1.size());
std::vector<uint32_t> expected2{2};
@@ -530,9 +524,6 @@
// Test overridable constants work with workgroup size
TEST_P(ShaderTests, OverridableConstantsWorkgroupSize) {
- DAWN_TEST_UNSUPPORTED_IF(IsOpenGL());
- DAWN_TEST_UNSUPPORTED_IF(IsOpenGLES());
-
std::string shader = R"(
override x: u32;
@@ -594,9 +585,6 @@
// Test overridable constants with numeric identifiers
TEST_P(ShaderTests, OverridableConstantsNumericIdentifiers) {
- DAWN_TEST_UNSUPPORTED_IF(IsOpenGL());
- DAWN_TEST_UNSUPPORTED_IF(IsOpenGLES());
-
uint32_t const kCount = 4;
std::vector<uint32_t> expected{1u, 2u, 3u, 0u};
wgpu::Buffer buffer = CreateBuffer(kCount);
@@ -651,9 +639,6 @@
// Test overridable constants precision
// D3D12 HLSL shader uses defines so we want float number to have enough precision
TEST_P(ShaderTests, OverridableConstantsPrecision) {
- DAWN_TEST_UNSUPPORTED_IF(IsOpenGL());
- DAWN_TEST_UNSUPPORTED_IF(IsOpenGLES());
-
uint32_t const kCount = 2;
float const kValue1 = 3.14159;
float const kValue2 = 3.141592653589793238;
@@ -702,9 +687,6 @@
// Test overridable constants for different entry points
TEST_P(ShaderTests, OverridableConstantsMultipleEntryPoints) {
- DAWN_TEST_UNSUPPORTED_IF(IsOpenGL());
- DAWN_TEST_UNSUPPORTED_IF(IsOpenGLES());
-
uint32_t const kCount = 1;
std::vector<uint32_t> expected1{1u};
std::vector<uint32_t> expected2{2u};
@@ -807,9 +789,6 @@
// Draw a triangle covering the render target, with vertex position and color values from
// overridable constants
TEST_P(ShaderTests, OverridableConstantsRenderPipeline) {
- DAWN_TEST_UNSUPPORTED_IF(IsOpenGL());
- DAWN_TEST_UNSUPPORTED_IF(IsOpenGLES());
-
wgpu::ShaderModule vsModule = utils::CreateShaderModule(device, R"(
@id(1111) override xright: f32;
@id(2222) override ytop: f32;
diff --git a/src/dawn/tests/end2end/ShaderValidationTests.cpp b/src/dawn/tests/end2end/ShaderValidationTests.cpp
index c8b03f3..e2e4365 100644
--- a/src/dawn/tests/end2end/ShaderValidationTests.cpp
+++ b/src/dawn/tests/end2end/ShaderValidationTests.cpp
@@ -392,4 +392,10 @@
CheckPipelineWithWorkgroupStorage(false, 0, maxMat4Count + 1);
}
-DAWN_INSTANTIATE_TEST(WorkgroupSizeValidationTest, D3D12Backend(), MetalBackend(), VulkanBackend());
+DAWN_INSTANTIATE_TEST(WorkgroupSizeValidationTest,
+ D3D12Backend(),
+ MetalBackend(),
+ NullBackend(),
+ OpenGLBackend(),
+ OpenGLESBackend(),
+ VulkanBackend());
diff --git a/src/dawn/tests/unittests/validation/OverridableConstantsValidationTests.cpp b/src/dawn/tests/unittests/validation/OverridableConstantsValidationTests.cpp
index 882bf25..c0f9eec 100644
--- a/src/dawn/tests/unittests/validation/OverridableConstantsValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/OverridableConstantsValidationTests.cpp
@@ -120,6 +120,11 @@
TestCreatePipeline(constants);
}
{
+ // Error: c10 already has a constant numeric id specified
+ std::vector<wgpu::ConstantEntry> constants{{nullptr, "c10", 0}};
+ ASSERT_DEVICE_ERROR(TestCreatePipeline(constants));
+ }
+ {
// Error: constant numeric id not specified
std::vector<wgpu::ConstantEntry> constants{{nullptr, "9999", 0}};
ASSERT_DEVICE_ERROR(TestCreatePipeline(constants));
