From 508d2491d014376550595ee9aef355e6ddfb9355 Mon Sep 17 00:00:00 2001 From: James Price Date: Mon, 12 Jul 2021 12:12:32 +0000 Subject: [PATCH] validation: Validate invariant attribute This is only valid on structure members and entry point return types. Additionally, this can only be applied to a position builtin. Bug: tint:772 Change-Id: Iffd774ca768ab66373678ecf0eb57c766767e92b Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/57641 Commit-Queue: Ben Clayton Kokoro: Kokoro Reviewed-by: Ben Clayton --- src/program_builder.h | 14 ++++++++ src/resolver/decoration_validation_test.cc | 39 ++++++++++++++++++++++ src/resolver/resolver.cc | 18 ++++++++-- 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/src/program_builder.h b/src/program_builder.h index dcf311c910..f0b0e059c8 100644 --- a/src/program_builder.h +++ b/src/program_builder.h @@ -39,6 +39,7 @@ #include "src/ast/i32.h" #include "src/ast/if_statement.h" #include "src/ast/interpolate_decoration.h" +#include "src/ast/invariant_decoration.h" #include "src/ast/loop_statement.h" #include "src/ast/matrix.h" #include "src/ast/member_accessor_expression.h" @@ -2042,6 +2043,19 @@ class ProgramBuilder { return create(source_, type, sampling); } + /// Creates an ast::InvariantDecoration + /// @param source the source information + /// @returns the invariant decoration pointer + ast::InvariantDecoration* Invariant(const Source& source) { + return create(source); + } + + /// Creates an ast::InvariantDecoration + /// @returns the invariant decoration pointer + ast::InvariantDecoration* Invariant() { + return create(source_); + } + /// Creates an ast::LocationDecoration /// @param source the source information /// @param location the location value diff --git a/src/resolver/decoration_validation_test.cc b/src/resolver/decoration_validation_test.cc index 61f8d36834..6b893ac89c 100644 --- a/src/resolver/decoration_validation_test.cc +++ b/src/resolver/decoration_validation_test.cc @@ -62,6 +62,7 @@ enum class DecorationKind { kBuiltin, kGroup, kInterpolate, + kInvariant, kLocation, kOverride, kOffset, @@ -107,6 +108,8 @@ static ast::DecorationList createDecorations(const Source& source, return {builder.Interpolate(source, ast::InterpolationType::kLinear, ast::InterpolationSampling::kCenter), builder.Location(0)}; + case DecorationKind::kInvariant: + return {builder.Invariant(source)}; case DecorationKind::kLocation: return {builder.Location(source, 1)}; case DecorationKind::kOverride: @@ -157,6 +160,7 @@ INSTANTIATE_TEST_SUITE_P( TestParams{DecorationKind::kBuiltin, false}, TestParams{DecorationKind::kGroup, false}, TestParams{DecorationKind::kInterpolate, false}, + TestParams{DecorationKind::kInvariant, false}, TestParams{DecorationKind::kLocation, false}, TestParams{DecorationKind::kOverride, false}, TestParams{DecorationKind::kOffset, false}, @@ -193,6 +197,7 @@ INSTANTIATE_TEST_SUITE_P( TestParams{DecorationKind::kBuiltin, true}, TestParams{DecorationKind::kGroup, false}, TestParams{DecorationKind::kInterpolate, true}, + TestParams{DecorationKind::kInvariant, false}, TestParams{DecorationKind::kLocation, true}, TestParams{DecorationKind::kOverride, false}, TestParams{DecorationKind::kOffset, false}, @@ -257,6 +262,7 @@ INSTANTIATE_TEST_SUITE_P( TestParams{DecorationKind::kBuiltin, false}, TestParams{DecorationKind::kGroup, false}, TestParams{DecorationKind::kInterpolate, false}, + TestParams{DecorationKind::kInvariant, false}, TestParams{DecorationKind::kLocation, false}, TestParams{DecorationKind::kOverride, false}, TestParams{DecorationKind::kOffset, false}, @@ -293,6 +299,7 @@ INSTANTIATE_TEST_SUITE_P( TestParams{DecorationKind::kBuiltin, true}, TestParams{DecorationKind::kGroup, false}, TestParams{DecorationKind::kInterpolate, true}, + // kInvariant tested separately (requires position builtin) TestParams{DecorationKind::kLocation, true}, TestParams{DecorationKind::kOverride, false}, TestParams{DecorationKind::kOffset, false}, @@ -317,6 +324,32 @@ TEST_F(EntryPointReturnTypeDecorationTest, DuplicateDecoration) { 12:34 note: first decoration declared here)"); } +TEST_F(EntryPointReturnTypeDecorationTest, InvariantWithPosition) { + Func("main", ast::VariableList{}, ty.vec4(), + ast::StatementList{Return(Construct(ty.vec4()))}, + ast::DecorationList{Stage(ast::PipelineStage::kVertex)}, + ast::DecorationList{ + Invariant(Source{{12, 34}}), + Builtin(Source{{56, 78}}, ast::Builtin::kPosition), + }); + EXPECT_TRUE(r()->Resolve()) << r()->error(); +} + +TEST_F(EntryPointReturnTypeDecorationTest, InvariantWithoutPosition) { + Func("main", ast::VariableList{}, ty.vec4(), + ast::StatementList{Return(Construct(ty.vec4()))}, + ast::DecorationList{Stage(ast::PipelineStage::kFragment)}, + ast::DecorationList{ + Invariant(Source{{12, 34}}), + Location(Source{{56, 78}}, 0), + }); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: invariant attribute must only be applied to a " + "position builtin"); +} + using ArrayDecorationTest = TestWithParams; TEST_P(ArrayDecorationTest, IsValid) { auto& params = GetParam(); @@ -347,6 +380,7 @@ INSTANTIATE_TEST_SUITE_P( TestParams{DecorationKind::kBuiltin, false}, TestParams{DecorationKind::kGroup, false}, TestParams{DecorationKind::kInterpolate, false}, + TestParams{DecorationKind::kInvariant, false}, TestParams{DecorationKind::kLocation, false}, TestParams{DecorationKind::kOverride, false}, TestParams{DecorationKind::kOffset, false}, @@ -382,6 +416,7 @@ INSTANTIATE_TEST_SUITE_P( TestParams{DecorationKind::kBuiltin, false}, TestParams{DecorationKind::kGroup, false}, TestParams{DecorationKind::kInterpolate, false}, + TestParams{DecorationKind::kInvariant, false}, TestParams{DecorationKind::kLocation, false}, TestParams{DecorationKind::kOverride, false}, TestParams{DecorationKind::kOffset, false}, @@ -445,6 +480,7 @@ INSTANTIATE_TEST_SUITE_P( TestParams{DecorationKind::kBuiltin, true}, TestParams{DecorationKind::kGroup, false}, TestParams{DecorationKind::kInterpolate, true}, + TestParams{DecorationKind::kInvariant, true}, TestParams{DecorationKind::kLocation, true}, TestParams{DecorationKind::kOverride, false}, TestParams{DecorationKind::kOffset, true}, @@ -507,6 +543,7 @@ INSTANTIATE_TEST_SUITE_P( TestParams{DecorationKind::kBuiltin, false}, TestParams{DecorationKind::kGroup, false}, TestParams{DecorationKind::kInterpolate, false}, + TestParams{DecorationKind::kInvariant, false}, TestParams{DecorationKind::kLocation, false}, TestParams{DecorationKind::kOverride, false}, TestParams{DecorationKind::kOffset, false}, @@ -558,6 +595,7 @@ INSTANTIATE_TEST_SUITE_P( TestParams{DecorationKind::kBuiltin, false}, TestParams{DecorationKind::kGroup, false}, TestParams{DecorationKind::kInterpolate, false}, + TestParams{DecorationKind::kInvariant, false}, TestParams{DecorationKind::kLocation, false}, TestParams{DecorationKind::kOverride, true}, TestParams{DecorationKind::kOffset, false}, @@ -607,6 +645,7 @@ INSTANTIATE_TEST_SUITE_P( TestParams{DecorationKind::kBuiltin, false}, TestParams{DecorationKind::kGroup, false}, TestParams{DecorationKind::kInterpolate, false}, + TestParams{DecorationKind::kInvariant, false}, TestParams{DecorationKind::kLocation, false}, TestParams{DecorationKind::kOverride, false}, TestParams{DecorationKind::kOffset, false}, diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc index 0c48f04c74..fe559f36c0 100644 --- a/src/resolver/resolver.cc +++ b/src/resolver/resolver.cc @@ -722,7 +722,7 @@ bool Resolver::ValidateGlobalVariable(const VariableInfo* info) { } else { bool is_shader_io_decoration = deco->IsAnyOf(); + ast::InvariantDecoration, ast::LocationDecoration>(); bool has_io_storage_class = info->storage_class == ast::StorageClass::kInput || info->storage_class == ast::StorageClass::kOutput; @@ -1194,6 +1194,7 @@ bool Resolver::ValidateFunction(const ast::Function* func, return false; } } else if (!deco->IsAnyOf() && (IsValidationEnabled( info->declaration->decorations(), @@ -1242,6 +1243,7 @@ bool Resolver::ValidateEntryPoint(const ast::Function* func, // Scan decorations for pipeline IO attributes. // Check for overlap with attributes that have been seen previously. ast::Decoration* pipeline_io_attribute = nullptr; + ast::InvariantDecoration* invariant_attribute = nullptr; for (auto* deco : decos) { if (auto* builtin = deco->As()) { if (pipeline_io_attribute) { @@ -1286,6 +1288,8 @@ bool Resolver::ValidateEntryPoint(const ast::Function* func, return false; } locations.emplace(location->value()); + } else if (auto* invariant = deco->As()) { + invariant_attribute = invariant; } } @@ -1312,10 +1316,19 @@ bool Resolver::ValidateEntryPoint(const ast::Function* func, return false; } + auto* builtin = pipeline_io_attribute->As(); + if (invariant_attribute && + !(builtin && builtin->value() == ast::Builtin::kPosition)) { + AddError( + "invariant attribute must only be applied to a position builtin", + invariant_attribute->source()); + return false; + } + // Check that all user defined attributes are numeric scalars, vectors // of numeric scalars. // Testing for being a struct is handled by the if portion above. - if (!pipeline_io_attribute->Is()) { + if (!builtin) { if (!ty->is_numeric_scalar_or_vector()) { AddError( "User defined entry point IO types must be a numeric scalar, " @@ -3558,6 +3571,7 @@ bool Resolver::ValidateStructure(const sem::Struct* str) { for (auto* deco : member->Declaration()->decorations()) { if (!(deco->Is() || deco->Is() || + deco->Is() || deco->Is() || deco->Is() || deco->Is() ||