[dawn] Add RequiredExtensionsMustBeEnabled shader module validation test The test ensures that each WGSL language extension compiles when the required specific feature is enabled. Also: - Clean up test code - Add an optional CRTP template param to ValidationTestWithParam so that a ValidationTest-derived base can be provided when needed, which was needed for this CL. Change-Id: I2fd04a82bbaa7271e2db983c678602c22fbb23a8 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/278215 Reviewed-by: Corentin Wallez <cwallez@chromium.org> Commit-Queue: Antonio Maiorano <amaiorano@google.com>
diff --git a/src/dawn/tests/BUILD.gn b/src/dawn/tests/BUILD.gn index 894b885..188d3c3 100644 --- a/src/dawn/tests/BUILD.gn +++ b/src/dawn/tests/BUILD.gn
@@ -85,6 +85,12 @@ "${googletest_dir}/googlemock/include", "${googletest_dir}/googletest/include", ] + if (dawn_standalone && is_clang) { + cflags = [ + # gmock emits deprecated-copy-dtor warnings when using ::testing::Combine + "-Wno-deprecated-copy-dtor", + ] + } } static_library("gmock") {
diff --git a/src/dawn/tests/unittests/validation/ShaderModuleValidationTests.cpp b/src/dawn/tests/unittests/validation/ShaderModuleValidationTests.cpp index 9eac76e..bcd2e63 100644 --- a/src/dawn/tests/unittests/validation/ShaderModuleValidationTests.cpp +++ b/src/dawn/tests/unittests/validation/ShaderModuleValidationTests.cpp
@@ -29,6 +29,7 @@ #include <memory> #include <sstream> #include <string> +#include <tuple> #include <utility> #include <vector> @@ -864,42 +865,42 @@ const char* wgslName; // Is this WGSL extension experimental, i.e. guarded by AllowUnsafeAPIs toggle bool isExperimental; - // The WebGPU features that required to enable this extension, set to empty if no feature + // The WebGPU features required to enable this extension, set to empty if no feature // required. - const std::vector<const char*> requiredFeatureNames; - // The WGSL extensions dependency that required to enable this extension, set to empty if no + const std::vector<wgpu::FeatureName> requiredFeatureNames; + // The WGSL extensions dependency required to enable this extension, set to empty if no // dependency. const std::vector<const char*> dependingExtensionNames; }; +std::ostream& operator<<(std::ostream& os, const WGSLExtensionInfo& info) { + return os << "WGSLExtensionInfo{wgslName: " << info.wgslName + << ", isExperimental: " << info.isExperimental + << ", requiredFeatureNames size: " << info.requiredFeatureNames.size() + << ", dependingExtensionNames size: " << info.dependingExtensionNames.size() << "}"; +} -const struct WGSLExtensionInfo kExtensions[] = { - {"f16", false, {"shader-f16"}, {}}, - {"clip_distances", false, {"clip-distances"}, {}}, - {"dual_source_blending", false, {"dual-source-blending"}, {}}, - {"subgroups", false, {"subgroups"}, {}}, - {"primitive_index", false, {"primitive-index"}, {}}, - {"chromium_experimental_pixel_local", true, {"pixel-local-storage-coherent"}, {}}, +// clang-format off +const WGSLExtensionInfo kExtensions[] = { + {"f16", false, {wgpu::FeatureName::ShaderF16}, {}}, + {"clip_distances", false, {wgpu::FeatureName::ClipDistances}, {}}, + {"dual_source_blending", false, {wgpu::FeatureName::DualSourceBlending}, {}}, + {"subgroups", false, {wgpu::FeatureName::Subgroups}, {}}, + {"primitive_index", false, {wgpu::FeatureName::PrimitiveIndex}, {}}, + {"chromium_experimental_pixel_local", true, {wgpu::FeatureName::PixelLocalStorageCoherent}, {}}, {"chromium_disable_uniformity_analysis", true, {}, {}}, {"chromium_internal_graphite", true, {}, {}}, - {"chromium_experimental_framebuffer_fetch", true, {"framebuffer-fetch"}, {}}, - {"chromium_experimental_subgroup_matrix", true, {"chromium-experimental-subgroup-matrix"}, {}}, - {"chromium_experimental_dynamic_binding", true, {"chromium-experimental-bindless"}, {}} + {"chromium_experimental_framebuffer_fetch", true, {wgpu::FeatureName::FramebufferFetch}, {}}, + {"chromium_experimental_subgroup_matrix", true, {wgpu::FeatureName::ChromiumExperimentalSubgroupMatrix}, {}}, + {"chromium_experimental_dynamic_binding", true, {wgpu::FeatureName::ChromiumExperimentalBindless}, {}} // Currently the following WGSL extensions are not enabled under any situation. /* {"chromium_internal_relaxed_uniform_layout", true, {}}, */ }; +// clang-format on -std::string EnableDependingWGSLExtensions(const WGSLExtensionInfo& extension) { - std::stringstream s; - for (const char* dependency : extension.dependingExtensionNames) { - s << "enable " << dependency << ";\n"; - } - return s.str(); -} - -class ShaderModuleExtensionValidationTestBase : public ValidationTest { +class ShaderModuleExtensionValidationTest : public ValidationTest { protected: // Skip tests if using Wire, because some features are not supported by the wire and cause the // device creation failed. @@ -917,11 +918,25 @@ } return requiredFeatures; } + + std::string EnabledExtensionsShader(const WGSLExtensionInfo& extension) { + std::stringstream s; + for (const char* dependency : extension.dependingExtensionNames) { + s << "enable " << dependency << ";\n"; + } + s << "enable " << extension.wgslName << ";\n"; + s << "@compute @workgroup_size(1) fn main() {}"; + return s.str(); + } }; -// Test validating WGSL extension on safe device with no feature required. +template <typename T> +class ShaderModuleExtensionValidationTestWithParams + : public ValidationTestWithParam<T, ShaderModuleExtensionValidationTest> {}; + +// Test validating WGSL extension on safe device with no required features. class ShaderModuleExtensionValidationTestSafeNoFeature - : public ShaderModuleExtensionValidationTestBase { + : public ShaderModuleExtensionValidationTest { protected: bool AllowUnsafeAPIs() override { return false; } std::vector<wgpu::FeatureName> GetRequiredFeatures() override { return {}; } @@ -930,10 +945,7 @@ TEST_F(ShaderModuleExtensionValidationTestSafeNoFeature, OnlyStableExtensionsRequiringNoFeatureAllowed) { for (auto& extension : kExtensions) { - std::string wgsl = EnableDependingWGSLExtensions(extension) + std::string("enable ") + - extension.wgslName + R"(; - -@compute @workgroup_size(1) fn main() {})"; + std::string wgsl = EnabledExtensionsShader(extension); // On a safe device with no feature required, only stable extensions requiring no features // are allowed. @@ -945,9 +957,9 @@ } } -// Test validating WGSL extension on unsafe device with no feature required. +// Test validating WGSL extension on unsafe device with no required features. class ShaderModuleExtensionValidationTestUnsafeNoFeature - : public ShaderModuleExtensionValidationTestBase { + : public ShaderModuleExtensionValidationTest { protected: std::vector<wgpu::FeatureName> GetRequiredFeatures() override { return {}; } }; @@ -955,10 +967,7 @@ TEST_F(ShaderModuleExtensionValidationTestUnsafeNoFeature, OnlyExtensionsRequiringNoFeatureAllowed) { for (auto& extension : kExtensions) { - std::string wgsl = EnableDependingWGSLExtensions(extension) + std::string("enable ") + - extension.wgslName + R"(; - -@compute @workgroup_size(1) fn main() {})"; + std::string wgsl = EnabledExtensionsShader(extension); // On an unsafe device with no feature required, only extensions requiring no features are // allowed. @@ -970,9 +979,9 @@ } } -// Test validating WGSL extension on safe device with all features required. +// Test validating WGSL extension on safe device with required features set to all. class ShaderModuleExtensionValidationTestSafeAllFeatures - : public ShaderModuleExtensionValidationTestBase { + : public ShaderModuleExtensionValidationTest { protected: bool AllowUnsafeAPIs() override { return false; } std::vector<wgpu::FeatureName> GetRequiredFeatures() override { return GetAllFeatures(); } @@ -980,10 +989,7 @@ TEST_F(ShaderModuleExtensionValidationTestSafeAllFeatures, OnlyStableExtensionsAllowed) { for (auto& extension : kExtensions) { - std::string wgsl = EnableDependingWGSLExtensions(extension) + std::string("enable ") + - extension.wgslName + R"(; - -@compute @workgroup_size(1) fn main() {})"; + std::string wgsl = EnabledExtensionsShader(extension); // On a safe device with all feature required, only stable extensions are allowed. if (!extension.isExperimental) { @@ -994,19 +1000,16 @@ } } -// Test validating WGSL extension on unsafe device with all features required. +// Test validating WGSL extension on unsafe device with required features set to all. class ShaderModuleExtensionValidationTestUnsafeAllFeatures - : public ShaderModuleExtensionValidationTestBase { + : public ShaderModuleExtensionValidationTest { protected: std::vector<wgpu::FeatureName> GetRequiredFeatures() override { return GetAllFeatures(); } }; TEST_F(ShaderModuleExtensionValidationTestUnsafeAllFeatures, AllExtensionsAllowed) { for (auto& extension : kExtensions) { - std::string wgsl = EnableDependingWGSLExtensions(extension) + std::string("enable ") + - extension.wgslName + R"(; - -@compute @workgroup_size(1) fn main() {})"; + std::string wgsl = EnabledExtensionsShader(extension); // On an unsafe device with all feature required, all extensions are allowed. utils::CreateShaderModule(device, wgsl.c_str()); @@ -1030,5 +1033,38 @@ device.CreateShaderModule(&desc); } +// Test that each WGSL language extension requires a specific feature to be enabled +class ShaderModuleExtensionValidationTestUnsafeOnlyRequiredFeatures + : public ShaderModuleExtensionValidationTestWithParams<std::tuple<WGSLExtensionInfo, bool>> { + public: + std::vector<wgpu::FeatureName> GetRequiredFeatures() override { + auto [extension, enabled] = GetParam(); + if (enabled) { + return extension.requiredFeatureNames; + } + return {}; + } +}; + +TEST_P(ShaderModuleExtensionValidationTestUnsafeOnlyRequiredFeatures, + RequiredExtensionsMustBeEnabled) { + auto [extension, enabled] = GetParam(); + std::string wgsl = EnabledExtensionsShader(extension); + + if (enabled || extension.requiredFeatureNames.empty()) { + // On an unsafe device with the required feature enabled, or if it doesn't require one, the + // wgsl extension should be valid + utils::CreateShaderModule(device, wgsl.c_str()); + } else { + // On an unsafe device with the required feature not enabled, the wgsl extension should be + // invalid + ASSERT_DEVICE_ERROR(utils::CreateShaderModule(device, wgsl.c_str())); + } +} +INSTANTIATE_TEST_SUITE_P(, + ShaderModuleExtensionValidationTestUnsafeOnlyRequiredFeatures, + ::testing::Combine(::testing::ValuesIn(kExtensions), + ::testing::Values(true, false))); + } // anonymous namespace } // namespace dawn
diff --git a/src/dawn/tests/unittests/validation/ValidationTest.h b/src/dawn/tests/unittests/validation/ValidationTest.h index f03b547..6438451 100644 --- a/src/dawn/tests/unittests/validation/ValidationTest.h +++ b/src/dawn/tests/unittests/validation/ValidationTest.h
@@ -212,7 +212,7 @@ bool mExpectDestruction = false; }; -template <typename T> -class ValidationTestWithParam : public ValidationTest, public testing::WithParamInterface<T> {}; +template <typename T, typename Base = ValidationTest> +class ValidationTestWithParam : public Base, public testing::WithParamInterface<T> {}; #endif // SRC_DAWN_TESTS_UNITTESTS_VALIDATION_VALIDATIONTEST_H_