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>

src/dawn/native/d3d12/ShaderModuleD3D12.cpp

@@ -327,8 +327,8 @@ ResultOrError<std::string> TranslateToHLSL(
     }
 
     if (r.substituteOverrideConfig) {
-        // This needs to run after SingleEntryPoint transform to get rid of overrides not used for
+        // This needs to run after SingleEntryPoint transform which removes unused overrides for
-        // the current entry point.
+        // current entry point.
         transformManager.Add<tint::transform::SubstituteOverride>();
         transformInputs.Add<tint::transform::SubstituteOverride::Config>(
             std::move(r.substituteOverrideConfig).value());

src/dawn/native/metal/ShaderModuleMTL.mm

@@ -206,8 +206,8 @@ ResultOrError<CacheResult<MslCompilation>> TranslateToMSL(
             }
 
             if (r.substituteOverrideConfig) {
-                // This needs to run after SingleEntryPoint transform to get rid of overrides not
+                // This needs to run after SingleEntryPoint transform which removes unused overrides
-                // used for the current entry point.
+                // for current entry point.
                 transformManager.Add<tint::transform::SubstituteOverride>();
                 transformInputs.Add<tint::transform::SubstituteOverride::Config>(
                     std::move(r.substituteOverrideConfig).value());

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 @@ MaybeError Queue::WriteBufferImpl(BufferBase* buffer,
 
 // 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 {};
 }
 

src/dawn/native/opengl/ShaderModuleGL.cpp

@@ -60,8 +60,11 @@ using BindingMap = std::unordered_map<tint::sem::BindingPoint, tint::sem::Bindin
 #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)
@@ -168,11 +171,21 @@ ResultOrError<GLuint> ShaderModule::CompileShader(const OpenGLFunctions& gl,
         }
     }
 
+    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 @@ ResultOrError<GLuint> ShaderModule::CompileShader(const OpenGLFunctions& gl,
                     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);

src/dawn/native/vulkan/ShaderModuleVk.cpp

@@ -311,8 +311,8 @@ ResultOrError<ShaderModule::ModuleAndSpirv> ShaderModule::GetHandleAndSpirv(
                     r.newBindingsMap);
             }
             if (r.substituteOverrideConfig) {
-                // This needs to run after SingleEntryPoint transform to get rid of overrides not
+                // This needs to run after SingleEntryPoint transform which removes unused overrides
-                // used for the current entry point.
+                // for current entry point.
                 transformManager.Add<tint::transform::SubstituteOverride>();
                 transformInputs.Add<tint::transform::SubstituteOverride::Config>(
                     std::move(r.substituteOverrideConfig).value());

src/dawn/tests/end2end/ShaderTests.cpp

@@ -395,9 +395,6 @@ fn main(@location(0) pos : vec4<f32>) -> @builtin(position) vec4<f32> {
 
 // 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 @@ struct Buf {
 
 // 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 @@ struct Buf {
 
 // 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 @@ struct Buf {
 
 // 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 @@ struct Buf {
 // 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 @@ struct Buf {
 
 // 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 @@ struct Buf {
 // 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;

src/dawn/tests/end2end/ShaderValidationTests.cpp

@@ -392,4 +392,10 @@ TEST_P(WorkgroupSizeValidationTest, DISABLED_ValidationAfterOverrideStorageSize)
     CheckPipelineWithWorkgroupStorage(false, 0, maxMat4Count + 1);
 }
 
-DAWN_INSTANTIATE_TEST(WorkgroupSizeValidationTest, D3D12Backend(), MetalBackend(), VulkanBackend());
+DAWN_INSTANTIATE_TEST(WorkgroupSizeValidationTest,
+                      D3D12Backend(),
+                      MetalBackend(),
+                      NullBackend(),
+                      OpenGLBackend(),
+                      OpenGLESBackend(),
+                      VulkanBackend());

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

@@ -119,6 +119,11 @@ TEST_F(ComputePipelineOverridableConstantsValidationTest, ConstantsIdentifierLoo
         std::vector<wgpu::ConstantEntry> constants{{nullptr, "1000", 0}};
         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}};