Add validation for pipeline overrides value type error for i32/u32/f32
Follow WebIDL TypeError for float and EnforceRange. Generate
a validation error is the number is not representable.
Bug: dawn:1597
Change-Id: I9a683f65ed0bfadb936d5de358670b01a2036848
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/112920
Reviewed-by: Austin Eng <enga@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Shrek Shao <shrekshao@google.com>
diff --git a/src/dawn/common/Numeric.h b/src/dawn/common/Numeric.h
index f4b084b..6387f48 100644
--- a/src/dawn/common/Numeric.h
+++ b/src/dawn/common/Numeric.h
@@ -51,4 +51,18 @@
return static_cast<Dst>(value);
}
+template <typename T>
+bool IsDoubleValueRepresentable(double value) {
+ if constexpr (std::is_same_v<T, float> || std::is_integral_v<T>) {
+ // Following WebIDL 3.3.6.[EnforceRange] for integral
+ // Following WebIDL 3.2.5.float for float
+ // TODO(crbug.com/1396194): now follows what blink does but may need revisit.
+ constexpr double kLowest = static_cast<double>(std::numeric_limits<T>::lowest());
+ constexpr double kMax = static_cast<double>(std::numeric_limits<T>::max());
+ return kLowest <= value && value <= kMax;
+ } else {
+ static_assert(sizeof(T) != sizeof(T), "Unsupported type");
+ }
+}
+
#endif // SRC_DAWN_COMMON_NUMERIC_H_
diff --git a/src/dawn/native/ComputePipeline.cpp b/src/dawn/native/ComputePipeline.cpp
index ef84b2c..a4b669b 100644
--- a/src/dawn/native/ComputePipeline.cpp
+++ b/src/dawn/native/ComputePipeline.cpp
@@ -30,10 +30,13 @@
DAWN_TRY(device->ValidateObject(descriptor->layout));
}
- return ValidateProgrammableStage(
- device, descriptor->compute.module, descriptor->compute.entryPoint,
- descriptor->compute.constantCount, descriptor->compute.constants, descriptor->layout,
- SingleShaderStage::Compute);
+ DAWN_TRY_CONTEXT(ValidateProgrammableStage(
+ device, descriptor->compute.module, descriptor->compute.entryPoint,
+ descriptor->compute.constantCount, descriptor->compute.constants,
+ descriptor->layout, SingleShaderStage::Compute),
+ "validating compute stage (%s, entryPoint: %s).", descriptor->compute.module,
+ descriptor->compute.entryPoint);
+ return {};
}
// ComputePipelineBase
diff --git a/src/dawn/native/Pipeline.cpp b/src/dawn/native/Pipeline.cpp
index 443ae3d..581472d 100644
--- a/src/dawn/native/Pipeline.cpp
+++ b/src/dawn/native/Pipeline.cpp
@@ -68,8 +68,38 @@
"Pipeline overridable constant \"%s\" not found in %s.", constants[i].key,
module);
DAWN_INVALID_IF(!std::isfinite(constants[i].value),
- "Pipeline overridable constant \"%s\" with value (%f) is not finite",
- constants[i].key, constants[i].value);
+ "Pipeline overridable constant \"%s\" with value (%f) is not finite in %s",
+ constants[i].key, constants[i].value, module);
+
+ // Validate if constant value can be represented by the given scalar type in shader
+ auto type = metadata.overrides.at(constants[i].key).type;
+ switch (type) {
+ case EntryPointMetadata::Override::Type::Float32:
+ DAWN_INVALID_IF(!IsDoubleValueRepresentable<float>(constants[i].value),
+ "Pipeline overridable constant \"%s\" with value (%f) is not "
+ "representable in type (%s)",
+ constants[i].key, constants[i].value, "f32");
+ break;
+ case EntryPointMetadata::Override::Type::Int32:
+ DAWN_INVALID_IF(!IsDoubleValueRepresentable<int32_t>(constants[i].value),
+ "Pipeline overridable constant \"%s\" with value (%f) is not "
+ "representable in type (%s)",
+ constants[i].key, constants[i].value,
+ type == EntryPointMetadata::Override::Type::Int32 ? "i32" : "b");
+ break;
+ case EntryPointMetadata::Override::Type::Uint32:
+ DAWN_INVALID_IF(!IsDoubleValueRepresentable<uint32_t>(constants[i].value),
+ "Pipeline overridable constant \"%s\" with value (%f) is not "
+ "representable in type (%s)",
+ constants[i].key, constants[i].value, "u32");
+ break;
+ case EntryPointMetadata::Override::Type::Boolean:
+ // Conversion to boolean can't fail
+ // https://webidl.spec.whatwg.org/#es-boolean
+ break;
+ default:
+ UNREACHABLE();
+ }
if (stageInitializedConstantIdentifiers.count(constants[i].key) == 0) {
if (metadata.uninitializedOverrides.count(constants[i].key) > 0) {
@@ -79,8 +109,7 @@
} else {
// There are duplicate initializations
return DAWN_VALIDATION_ERROR(
- "Pipeline overridable constants \"%s\" is set more than once in %s",
- constants[i].key, module);
+ "Pipeline overridable constants \"%s\" is set more than once", constants[i].key);
}
}
@@ -102,9 +131,9 @@
}
return DAWN_VALIDATION_ERROR(
- "There are uninitialized pipeline overridable constants in shader module %s, their "
+ "There are uninitialized pipeline overridable constants, their "
"identifiers:[%s]",
- module, uninitializedConstantsArray);
+ uninitializedConstantsArray);
}
return {};
diff --git a/src/dawn/native/RenderPipeline.cpp b/src/dawn/native/RenderPipeline.cpp
index baa6c85..9b0ff96 100644
--- a/src/dawn/native/RenderPipeline.cpp
+++ b/src/dawn/native/RenderPipeline.cpp
@@ -128,7 +128,7 @@
DAWN_TRY_CONTEXT(ValidateProgrammableStage(device, descriptor->module, descriptor->entryPoint,
descriptor->constantCount, descriptor->constants,
layout, SingleShaderStage::Vertex),
- "validating vertex stage (module: %s, entryPoint: %s).", descriptor->module,
+ "validating vertex stage (%s, entryPoint: %s).", descriptor->module,
descriptor->entryPoint);
const EntryPointMetadata& vertexMetadata =
descriptor->module->GetEntryPoint(descriptor->entryPoint);
@@ -341,7 +341,7 @@
DAWN_TRY_CONTEXT(ValidateProgrammableStage(device, descriptor->module, descriptor->entryPoint,
descriptor->constantCount, descriptor->constants,
layout, SingleShaderStage::Fragment),
- "validating fragment stage (module: %s, entryPoint: %s).", descriptor->module,
+ "validating fragment stage (%s, entryPoint: %s).", descriptor->module,
descriptor->entryPoint);
uint32_t maxColorAttachments = device->GetLimits().v1.maxColorAttachments;
diff --git a/src/dawn/native/ShaderModule.cpp b/src/dawn/native/ShaderModule.cpp
index b05f1b8..1ca7c7c 100644
--- a/src/dawn/native/ShaderModule.cpp
+++ b/src/dawn/native/ShaderModule.cpp
@@ -533,24 +533,8 @@
for (auto& c : entryPoint.overrides) {
auto id = name2Id.at(c.name);
- OverrideScalar defaultValue;
- if (c.is_initialized) {
- // if it is initialized, the scalar must exist
- const auto& scalar = id2Scalar.at(id);
- if (scalar.IsBool()) {
- defaultValue.b = scalar.AsBool();
- } else if (scalar.IsU32()) {
- defaultValue.u32 = scalar.AsU32();
- } else if (scalar.IsI32()) {
- defaultValue.i32 = scalar.AsI32();
- } else if (scalar.IsFloat()) {
- defaultValue.f32 = scalar.AsFloat();
- } else {
- UNREACHABLE();
- }
- }
EntryPointMetadata::Override override = {id, FromTintOverrideType(c.type),
- c.is_initialized, defaultValue};
+ c.is_initialized};
std::string identifier = c.is_id_specified ? std::to_string(override.id.value) : c.name;
metadata->overrides[identifier] = override;
diff --git a/src/dawn/native/ShaderModule.h b/src/dawn/native/ShaderModule.h
index a1f34ac..af63e85 100644
--- a/src/dawn/native/ShaderModule.h
+++ b/src/dawn/native/ShaderModule.h
@@ -227,11 +227,6 @@
// Then it is required for the pipeline stage to have a constant record to initialize a
// value
bool isInitialized;
-
- // Store the default initialized value in shader
- // This is used by metal backend as the function_constant does not have dafault values
- // Initialized when isInitialized == true
- OverrideScalar defaultValue;
};
using OverridesMap = std::unordered_map<std::string, Override>;
diff --git a/src/dawn/tests/unittests/validation/OverridableConstantsValidationTests.cpp b/src/dawn/tests/unittests/validation/OverridableConstantsValidationTests.cpp
index f7c061f..542bb06 100644
--- a/src/dawn/tests/unittests/validation/OverridableConstantsValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/OverridableConstantsValidationTests.cpp
@@ -223,31 +223,72 @@
TEST_F(ComputePipelineOverridableConstantsValidationTest, InvalidValue) {
SetUpShadersWithDefaultValueConstants();
{
- // Error:: NaN
+ // Error: NaN
std::vector<wgpu::ConstantEntry> constants{{nullptr, "c3", std::nan("")}};
ASSERT_DEVICE_ERROR(TestCreatePipeline(constants));
}
{
- // Error:: -NaN
+ // Error: -NaN
std::vector<wgpu::ConstantEntry> constants{{nullptr, "c3", -std::nan("")}};
ASSERT_DEVICE_ERROR(TestCreatePipeline(constants));
}
{
- // Error:: Inf
+ // Error: Inf
std::vector<wgpu::ConstantEntry> constants{
{nullptr, "c3", std::numeric_limits<double>::infinity()}};
ASSERT_DEVICE_ERROR(TestCreatePipeline(constants));
}
{
- // Error:: -Inf
+ // Error: -Inf
std::vector<wgpu::ConstantEntry> constants{
{nullptr, "c3", -std::numeric_limits<double>::infinity()}};
ASSERT_DEVICE_ERROR(TestCreatePipeline(constants));
}
{
- // Valid:: Max
+ // Valid: Max
std::vector<wgpu::ConstantEntry> constants{
{nullptr, "c3", std::numeric_limits<float>::max()}};
TestCreatePipeline(constants);
}
+ {
+ // Valid: Lowest
+ std::vector<wgpu::ConstantEntry> constants{
+ {nullptr, "c3", std::numeric_limits<float>::lowest()}};
+ TestCreatePipeline(constants);
+ }
+}
+
+// Test that values that are not representable by WGSL type i32/u32/f16/f32
+TEST_F(ComputePipelineOverridableConstantsValidationTest, OutofRangeValue) {
+ SetUpShadersWithDefaultValueConstants();
+ {
+ // Error: 1.79769e+308 cannot be represented by f32
+ std::vector<wgpu::ConstantEntry> constants{
+ {nullptr, "c3", std::numeric_limits<double>::max()}};
+ ASSERT_DEVICE_ERROR(TestCreatePipeline(constants));
+ }
+ {
+ // Error: i32 out of range
+ std::vector<wgpu::ConstantEntry> constants{
+ {nullptr, "c5", static_cast<double>(std::numeric_limits<int32_t>::max()) + 1.0}};
+ ASSERT_DEVICE_ERROR(TestCreatePipeline(constants));
+ }
+ {
+ // Error: i32 out of range (negative)
+ std::vector<wgpu::ConstantEntry> constants{
+ {nullptr, "c5", static_cast<double>(std::numeric_limits<int32_t>::lowest()) - 1.0}};
+ ASSERT_DEVICE_ERROR(TestCreatePipeline(constants));
+ }
+ {
+ // Error: u32 out of range
+ std::vector<wgpu::ConstantEntry> constants{
+ {nullptr, "c8", static_cast<double>(std::numeric_limits<uint32_t>::max()) + 1.0}};
+ ASSERT_DEVICE_ERROR(TestCreatePipeline(constants));
+ }
+ {
+ // Valid: conversion to boolean can't fail
+ std::vector<wgpu::ConstantEntry> constants{
+ {nullptr, "c0", static_cast<double>(std::numeric_limits<int32_t>::max()) + 1.0}};
+ TestCreatePipeline(constants);
+ }
}
