From 8d7551cc27838a84debffc37bb452bec8c45ef2f Mon Sep 17 00:00:00 2001 From: James Price Date: Thu, 28 Oct 2021 15:00:39 +0000 Subject: [PATCH] Add a helper for DisableValidationDecoration This is slightly different from the other helpers, since this AST node does not have a source. Change-Id: I56e72eb0f26f80d52be9e4f51dd42c3d5149cb00 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/67644 Kokoro: Kokoro Reviewed-by: Ben Clayton --- src/program_builder.h | 10 ++++++ src/resolver/decoration_validation_test.cc | 24 ++++++-------- src/transform/binding_remapper.cc | 3 +- src/transform/calculate_array_length.cc | 6 ++-- src/transform/canonicalize_entry_point_io.cc | 6 ++-- src/transform/decompose_memory_access.cc | 27 ++++++---------- .../decompose_strided_matrix_test.cc | 30 ++++++------------ .../module_scope_var_to_entry_point_param.cc | 31 ++++++------------- 8 files changed, 52 insertions(+), 85 deletions(-) diff --git a/src/program_builder.h b/src/program_builder.h index d78c0779f3..abe9a6e99b 100644 --- a/src/program_builder.h +++ b/src/program_builder.h @@ -35,6 +35,7 @@ #include "src/ast/case_statement.h" #include "src/ast/depth_multisampled_texture.h" #include "src/ast/depth_texture.h" +#include "src/ast/disable_validation_decoration.h" #include "src/ast/external_texture.h" #include "src/ast/f32.h" #include "src/ast/float_literal.h" @@ -2411,6 +2412,15 @@ class ProgramBuilder { Expr(std::forward(z))); } + /// Creates an ast::DisableValidationDecoration + /// @param validation the validation to disable + /// @returns the disable validation decoration pointer + const ast::DisableValidationDecoration* Disable( + ast::DisabledValidation validation) { + return ASTNodes().Create(ID(), + validation); + } + /// Sets the current builder source to `src` /// @param src the Source used for future create() calls void SetSource(const Source& src) { diff --git a/src/resolver/decoration_validation_test.cc b/src/resolver/decoration_validation_test.cc index 49f7cbae8d..d4641a553c 100644 --- a/src/resolver/decoration_validation_test.cc +++ b/src/resolver/decoration_validation_test.cc @@ -498,16 +498,13 @@ TEST_F(EntryPointParameterDecorationTest, DuplicateDecoration) { } TEST_F(EntryPointParameterDecorationTest, DuplicateInternalDecoration) { - auto* s = - Param("s", ty.sampler(ast::SamplerKind::kSampler), - ast::DecorationList{ - create(0), - create(0), - ASTNodes().Create( - ID(), ast::DisabledValidation::kBindingPointCollision), - ASTNodes().Create( - ID(), ast::DisabledValidation::kEntryPointParameter), - }); + auto* s = Param("s", ty.sampler(ast::SamplerKind::kSampler), + ast::DecorationList{ + create(0), + create(0), + Disable(ast::DisabledValidation::kBindingPointCollision), + Disable(ast::DisabledValidation::kEntryPointParameter), + }); Func("f", {s}, ty.void_(), {}, {Stage(ast::PipelineStage::kFragment)}); EXPECT_TRUE(r()->Resolve()) << r()->error(); @@ -531,10 +528,8 @@ TEST_F(EntryPointReturnTypeDecorationTest, DuplicateDecoration) { TEST_F(EntryPointReturnTypeDecorationTest, DuplicateInternalDecoration) { Func("f", {}, ty.i32(), {Return(1)}, {Stage(ast::PipelineStage::kFragment)}, ast::DecorationList{ - ASTNodes().Create( - ID(), ast::DisabledValidation::kBindingPointCollision), - ASTNodes().Create( - ID(), ast::DisabledValidation::kEntryPointParameter), + Disable(ast::DisabledValidation::kBindingPointCollision), + Disable(ast::DisabledValidation::kEntryPointParameter), }); EXPECT_TRUE(r()->Resolve()) << r()->error(); @@ -1156,7 +1151,6 @@ TEST_F(ResourceDecorationTest, BindingPointOnNonResource) { } // namespace } // namespace ResourceTests - namespace InvariantDecorationTests { namespace { using InvariantDecorationTests = ResolverTest; diff --git a/src/transform/binding_remapper.cc b/src/transform/binding_remapper.cc index ef77fed65f..7689cbaa6b 100644 --- a/src/transform/binding_remapper.cc +++ b/src/transform/binding_remapper.cc @@ -140,8 +140,7 @@ void BindingRemapper::Run(CloneContext& ctx, const DataMap& inputs, DataMap&) { // Add `DisableValidationDecoration`s if required if (add_collision_deco.count(bp)) { auto* decoration = - ctx.dst->ASTNodes().Create( - ctx.dst->ID(), ast::DisabledValidation::kBindingPointCollision); + ctx.dst->Disable(ast::DisabledValidation::kBindingPointCollision); ctx.InsertBefore(var->decorations, *var->decorations.begin(), decoration); } diff --git a/src/transform/calculate_array_length.cc b/src/transform/calculate_array_length.cc index d82e2320ee..82f8fcb85d 100644 --- a/src/transform/calculate_array_length.cc +++ b/src/transform/calculate_array_length.cc @@ -82,10 +82,8 @@ void CalculateArrayLength::Run(CloneContext& ctx, const DataMap&, DataMap&) { auto name = ctx.dst->Sym(); auto* buffer_typename = ctx.dst->ty.type_name(ctx.Clone(buffer_type->Declaration()->name)); - auto* disable_validation = - ctx.dst->ASTNodes().Create( - ctx.dst->ID(), - ast::DisabledValidation::kIgnoreConstructibleFunctionParameter); + auto* disable_validation = ctx.dst->Disable( + ast::DisabledValidation::kIgnoreConstructibleFunctionParameter); auto* func = ctx.dst->create( name, ast::VariableList{ diff --git a/src/transform/canonicalize_entry_point_io.cc b/src/transform/canonicalize_entry_point_io.cc index 09e279aa8a..af54484501 100644 --- a/src/transform/canonicalize_entry_point_io.cc +++ b/src/transform/canonicalize_entry_point_io.cc @@ -173,8 +173,7 @@ struct CanonicalizeEntryPointIO::State { // Disable validation for use of the `input` storage class. attributes.push_back( - ctx.dst->ASTNodes().Create( - ctx.dst->ID(), ast::DisabledValidation::kIgnoreStorageClass)); + ctx.dst->Disable(ast::DisabledValidation::kIgnoreStorageClass)); // Create the global variable and use its value for the shader input. auto symbol = ctx.dst->Symbols().New(name); @@ -417,8 +416,7 @@ struct CanonicalizeEntryPointIO::State { // Disable validation for use of the `output` storage class. ast::DecorationList attributes = std::move(outval.attributes); attributes.push_back( - ctx.dst->ASTNodes().Create( - ctx.dst->ID(), ast::DisabledValidation::kIgnoreStorageClass)); + ctx.dst->Disable(ast::DisabledValidation::kIgnoreStorageClass)); // Create the global variable and assign it the output value. auto name = ctx.dst->Symbols().New(outval.name); diff --git a/src/transform/decompose_memory_access.cc b/src/transform/decompose_memory_access.cc index 450f8c07bd..a524ee4f1d 100644 --- a/src/transform/decompose_memory_access.cc +++ b/src/transform/decompose_memory_access.cc @@ -451,10 +451,8 @@ struct DecomposeMemoryAccess::State { return utils::GetOrCreate( load_funcs, LoadStoreKey{storage_class, buf_ty, el_ty}, [&] { auto* buf_ast_ty = CreateASTTypeFor(ctx, buf_ty); - auto* disable_validation = - b.ASTNodes().Create( - b.ID(), ast::DisabledValidation:: - kIgnoreConstructibleFunctionParameter); + auto* disable_validation = b.Disable( + ast::DisabledValidation::kIgnoreConstructibleFunctionParameter); ast::VariableList params = { // Note: The buffer parameter requires the StorageClass in @@ -476,8 +474,7 @@ struct DecomposeMemoryAccess::State { name, params, el_ast_ty, nullptr, ast::DecorationList{ intrinsic, - b.ASTNodes().Create( - b.ID(), ast::DisabledValidation::kFunctionHasNoBody), + b.Disable(ast::DisabledValidation::kFunctionHasNoBody), }, ast::DecorationList{}); b.AST().AddFunction(func); @@ -554,10 +551,8 @@ struct DecomposeMemoryAccess::State { store_funcs, LoadStoreKey{storage_class, buf_ty, el_ty}, [&] { auto* buf_ast_ty = CreateASTTypeFor(ctx, buf_ty); auto* el_ast_ty = CreateASTTypeFor(ctx, el_ty); - auto* disable_validation = - b.ASTNodes().Create( - b.ID(), ast::DisabledValidation:: - kIgnoreConstructibleFunctionParameter); + auto* disable_validation = b.Disable( + ast::DisabledValidation::kIgnoreConstructibleFunctionParameter); ast::VariableList params{ // Note: The buffer parameter requires the StorageClass in // order for HLSL to emit this as a ByteAddressBuffer. @@ -578,8 +573,7 @@ struct DecomposeMemoryAccess::State { name, params, b.ty.void_(), nullptr, ast::DecorationList{ intrinsic, - b.ASTNodes().Create( - b.ID(), ast::DisabledValidation::kFunctionHasNoBody), + b.Disable(ast::DisabledValidation::kFunctionHasNoBody), }, ast::DecorationList{}); b.AST().AddFunction(func); @@ -655,10 +649,8 @@ struct DecomposeMemoryAccess::State { auto op = intrinsic->Type(); return utils::GetOrCreate(atomic_funcs, AtomicKey{buf_ty, el_ty, op}, [&] { auto* buf_ast_ty = CreateASTTypeFor(ctx, buf_ty); - auto* disable_validation = - b.ASTNodes().Create( - b.ID(), - ast::DisabledValidation::kIgnoreConstructibleFunctionParameter); + auto* disable_validation = b.Disable( + ast::DisabledValidation::kIgnoreConstructibleFunctionParameter); // The first parameter to all WGSL atomics is the expression to the // atomic. This is replaced with two parameters: the buffer and offset. @@ -691,8 +683,7 @@ struct DecomposeMemoryAccess::State { b.Sym(), params, ret_ty, nullptr, ast::DecorationList{ atomic, - b.ASTNodes().Create( - b.ID(), ast::DisabledValidation::kFunctionHasNoBody), + b.Disable(ast::DisabledValidation::kFunctionHasNoBody), }, ast::DecorationList{}); diff --git a/src/transform/decompose_strided_matrix_test.cc b/src/transform/decompose_strided_matrix_test.cc index 8f5d13e88b..5c34ff727c 100644 --- a/src/transform/decompose_strided_matrix_test.cc +++ b/src/transform/decompose_strided_matrix_test.cc @@ -82,8 +82,7 @@ TEST_F(DecomposeStridedMatrixTest, ReadUniformMatrix) { { b.create(16), b.create(32), - b.ASTNodes().Create( - b.ID(), ast::DisabledValidation::kIgnoreStrideDecoration), + b.Disable(ast::DisabledValidation::kIgnoreStrideDecoration), }), }, { @@ -149,8 +148,7 @@ TEST_F(DecomposeStridedMatrixTest, ReadUniformColumn) { { b.create(16), b.create(32), - b.ASTNodes().Create( - b.ID(), ast::DisabledValidation::kIgnoreStrideDecoration), + b.Disable(ast::DisabledValidation::kIgnoreStrideDecoration), }), }, { @@ -212,8 +210,7 @@ TEST_F(DecomposeStridedMatrixTest, ReadUniformMatrix_DefaultStride) { { b.create(16), b.create(8), - b.ASTNodes().Create( - b.ID(), ast::DisabledValidation::kIgnoreStrideDecoration), + b.Disable(ast::DisabledValidation::kIgnoreStrideDecoration), }), }, { @@ -276,8 +273,7 @@ TEST_F(DecomposeStridedMatrixTest, ReadStorageMatrix) { { b.create(8), b.create(32), - b.ASTNodes().Create( - b.ID(), ast::DisabledValidation::kIgnoreStrideDecoration), + b.Disable(ast::DisabledValidation::kIgnoreStrideDecoration), }), }, { @@ -343,8 +339,7 @@ TEST_F(DecomposeStridedMatrixTest, ReadStorageColumn) { { b.create(16), b.create(32), - b.ASTNodes().Create( - b.ID(), ast::DisabledValidation::kIgnoreStrideDecoration), + b.Disable(ast::DisabledValidation::kIgnoreStrideDecoration), }), }, { @@ -406,8 +401,7 @@ TEST_F(DecomposeStridedMatrixTest, WriteStorageMatrix) { { b.create(8), b.create(32), - b.ASTNodes().Create( - b.ID(), ast::DisabledValidation::kIgnoreStrideDecoration), + b.Disable(ast::DisabledValidation::kIgnoreStrideDecoration), }), }, { @@ -474,8 +468,7 @@ TEST_F(DecomposeStridedMatrixTest, WriteStorageColumn) { { b.create(8), b.create(32), - b.ASTNodes().Create( - b.ID(), ast::DisabledValidation::kIgnoreStrideDecoration), + b.Disable(ast::DisabledValidation::kIgnoreStrideDecoration), }), }, { @@ -543,8 +536,7 @@ TEST_F(DecomposeStridedMatrixTest, ReadWriteViaPointerLets) { { b.create(8), b.create(32), - b.ASTNodes().Create( - b.ID(), ast::DisabledValidation::kIgnoreStrideDecoration), + b.Disable(ast::DisabledValidation::kIgnoreStrideDecoration), }), }, { @@ -626,8 +618,7 @@ TEST_F(DecomposeStridedMatrixTest, ReadPrivateMatrix) { { b.create(8), b.create(32), - b.ASTNodes().Create( - b.ID(), ast::DisabledValidation::kIgnoreStrideDecoration), + b.Disable(ast::DisabledValidation::kIgnoreStrideDecoration), }), }); b.Global("s", b.ty.Of(S), ast::StorageClass::kPrivate); @@ -684,8 +675,7 @@ TEST_F(DecomposeStridedMatrixTest, WritePrivateMatrix) { { b.create(8), b.create(32), - b.ASTNodes().Create( - b.ID(), ast::DisabledValidation::kIgnoreStrideDecoration), + b.Disable(ast::DisabledValidation::kIgnoreStrideDecoration), }), }); b.Global("s", b.ty.Of(S), ast::StorageClass::kPrivate); diff --git a/src/transform/module_scope_var_to_entry_point_param.cc b/src/transform/module_scope_var_to_entry_point_param.cc index 5ac99396e0..891dc32a9c 100644 --- a/src/transform/module_scope_var_to_entry_point_param.cc +++ b/src/transform/module_scope_var_to_entry_point_param.cc @@ -186,9 +186,7 @@ struct ModuleScopeVarToEntryPointParam::State { // For a texture or sampler variable, redeclare it as an entry point // parameter. Disable entry point parameter validation. auto* disable_validation = - ctx.dst->ASTNodes().Create( - ctx.dst->ID(), - ast::DisabledValidation::kEntryPointParameter); + ctx.dst->Disable(ast::DisabledValidation::kEntryPointParameter); auto decos = ctx.Clone(var->Declaration()->decorations); decos.push_back(disable_validation); auto* param = ctx.dst->Param(new_var_symbol, store_type(), decos); @@ -198,14 +196,10 @@ struct ModuleScopeVarToEntryPointParam::State { // Variables into the Storage and Uniform storage classes are // redeclared as entry point parameters with a pointer type. auto attributes = ctx.Clone(var->Declaration()->decorations); + attributes.push_back(ctx.dst->Disable( + ast::DisabledValidation::kEntryPointParameter)); attributes.push_back( - ctx.dst->ASTNodes().Create( - ctx.dst->ID(), - ast::DisabledValidation::kEntryPointParameter)); - attributes.push_back( - ctx.dst->ASTNodes().Create( - ctx.dst->ID(), - ast::DisabledValidation::kIgnoreStorageClass)); + ctx.dst->Disable(ast::DisabledValidation::kIgnoreStorageClass)); auto* param_type = ctx.dst->ty.pointer( store_type(), sc, var->Declaration()->declared_access); auto* param = @@ -241,9 +235,7 @@ struct ModuleScopeVarToEntryPointParam::State { // redeclared at function scope. Disable storage class validation on // this variable. auto* disable_validation = - ctx.dst->ASTNodes().Create( - ctx.dst->ID(), - ast::DisabledValidation::kIgnoreStorageClass); + ctx.dst->Disable(ast::DisabledValidation::kIgnoreStorageClass); auto* constructor = ctx.Clone(var->Declaration()->constructor); auto* local_var = ctx.dst->Var(new_var_symbol, store_type(), sc, constructor, @@ -264,13 +256,9 @@ struct ModuleScopeVarToEntryPointParam::State { // Disable validation of the parameter's storage class and of // arguments passed it. attributes.push_back( - ctx.dst->ASTNodes().Create( - ctx.dst->ID(), - ast::DisabledValidation::kIgnoreStorageClass)); - attributes.push_back( - ctx.dst->ASTNodes().Create( - ctx.dst->ID(), - ast::DisabledValidation::kIgnoreInvalidPointerArgument)); + ctx.dst->Disable(ast::DisabledValidation::kIgnoreStorageClass)); + attributes.push_back(ctx.dst->Disable( + ast::DisabledValidation::kIgnoreInvalidPointerArgument)); } ctx.InsertBack( func_ast->params, @@ -311,8 +299,7 @@ struct ModuleScopeVarToEntryPointParam::State { auto* param_type = ctx.dst->ty.pointer(ctx.dst->ty.Of(str), ast::StorageClass::kWorkgroup); auto* disable_validation = - ctx.dst->ASTNodes().Create( - ctx.dst->ID(), ast::DisabledValidation::kEntryPointParameter); + ctx.dst->Disable(ast::DisabledValidation::kEntryPointParameter); auto* param = ctx.dst->Param(workgroup_param(), param_type, {disable_validation}); ctx.InsertFront(func_ast->params, param);