commit 7e6f3e83bc1854a57e45e94e62973437562a1fa7
author Shrek Shao <shrekshao@google.com> Mon Nov 01 20:24:02 2021 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Nov 01 20:24:02 2021 +0000
tree ed6f0b702d30d90650407f635127d5ba30d567b0
parent 643403d48e92ff1291a381a92ad8ac93145e5237

Mark pipeline overridable constants as unsafe

Until all backend implementations are done.

Chromium Test CL:
https://chromium-review.googlesource.com/c/chromium/src/+/3251494

No-Try: true
Bug: dawn:1169
Change-Id: I16d184bec03bbc9a255ad33cf11aa615b1986389
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/67422
Commit-Queue: Shrek Shao <shrekshao@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>
(cherry picked from commit f4c8a6ac9ba5f0032cec430b82080608c90d3567)
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/67600
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

src/dawn_native/Pipeline.cpp

diff --git a/src/dawn_native/Pipeline.cpp b/src/dawn_native/Pipeline.cpp
index a1f2453..49590aa 100644
--- a/src/dawn_native/Pipeline.cpp
+++ b/src/dawn_native/Pipeline.cpp
@@ -65,6 +65,12 @@
             DAWN_TRY(ValidateCompatibilityWithPipelineLayout(device, metadata, layout));
         }
 
+        if (constantCount > 0u && device->IsToggleEnabled(Toggle::DisallowUnsafeAPIs)) {
+            return DAWN_VALIDATION_ERROR(
+                "Pipeline overridable constants are disallowed because they are partially "
+                "implemented.");
+        }
+
         // Validate if overridable constants exist in shader module
         // pipelineBase is not yet constructed at this moment so iterate constants from descriptor
         for (uint32_t i = 0; i < constantCount; i++) {

src/dawn_native/ShaderModule.cpp

diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp
index 43b9f1c..8d6c59b 100644
--- a/src/dawn_native/ShaderModule.cpp
+++ b/src/dawn_native/ShaderModule.cpp
@@ -647,6 +647,10 @@
                 auto metadata = std::make_unique<EntryPointMetadata>();
 
                 if (!entryPoint.overridable_constants.empty()) {
+                    DAWN_INVALID_IF(device->IsToggleEnabled(Toggle::DisallowUnsafeAPIs),
+                                    "Pipeline overridable constants are disallowed because they "
+                                    "are partially implemented.");
+
                     const auto& name2Id = inspector.GetConstantNameToIdMap();
 
                     for (auto& c : entryPoint.overridable_constants) {

src/tests/unittests/validation/UnsafeAPIValidationTests.cpp

diff --git a/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp b/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp
index 6e22dcc..5629427 100644
--- a/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp
+++ b/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp
@@ -106,6 +106,46 @@
     }
 }
 
+// Check that pipeline overridable constants are disallowed as part of unsafe APIs.
+// TODO(dawn:1041) Remove when implementation for all backend is added
+TEST_F(UnsafeAPIValidationTest, PipelineOverridableConstants) {
+    // Create the dummy compute pipeline.
+    wgpu::ComputePipelineDescriptor pipelineDescBase;
+    pipelineDescBase.compute.entryPoint = "main";
+
+    // Control case: shader without overridable constant is allowed.
+    {
+        wgpu::ComputePipelineDescriptor pipelineDesc = pipelineDescBase;
+        pipelineDesc.compute.module =
+            utils::CreateShaderModule(device, "[[stage(compute), workgroup_size(1)]] fn main() {}");
+
+        device.CreateComputePipeline(&pipelineDesc);
+    }
+
+    // Error case: shader with overridable constant with default value
+    {
+        ASSERT_DEVICE_ERROR(utils::CreateShaderModule(device, R"(
+[[override(1000)]] let c0: u32 = 1u;
+[[override(1000)]] let c1: u32;
+
+[[stage(compute), workgroup_size(1)]] fn main() {
+    ignore(c0);
+    ignore(c1);
+})"));
+    }
+
+    // Error case: pipeline stage with constant entry is disallowed
+    {
+        wgpu::ComputePipelineDescriptor pipelineDesc = pipelineDescBase;
+        pipelineDesc.compute.module =
+            utils::CreateShaderModule(device, "[[stage(compute), workgroup_size(1)]] fn main() {}");
+        std::vector<wgpu::ConstantEntry> constants{{nullptr, "c", 1u}};
+        pipelineDesc.compute.constants = constants.data();
+        pipelineDesc.compute.constantCount = constants.size();
+        ASSERT_DEVICE_ERROR(device.CreateComputePipeline(&pipelineDesc));
+    }
+}
+
 class UnsafeQueryAPIValidationTest : public ValidationTest {
   protected:
     WGPUDevice CreateTestDevice() override {