From 465c5aa51d7a16782e1a1ebd8d25c2a36a564ea5 Mon Sep 17 00:00:00 2001 From: Sarah Date: Fri, 23 Jul 2021 13:23:20 +0000 Subject: [PATCH] validation: clean up function attributes validation and unit-tests - clean up function decorations unit tests - clean up interpolate and invariant validation and unittest - add separate unit-tests for each shader stage input and output - add [[builtin(position)]] tests - add validation and test for: structures with 'location' decorated members cannot be used as compute shaders input Bug: tint:1007 Change-Id: I12e97e163b3a77bc76ce21faba241683eec5d917 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/58942 Kokoro: Kokoro Reviewed-by: Ben Clayton Reviewed-by: James Price --- src/resolver/builtins_validation_test.cc | 24 +- src/resolver/decoration_validation_test.cc | 689 +++++++++++++------- src/resolver/entry_point_validation_test.cc | 20 +- src/resolver/resolver.cc | 116 ++-- src/resolver/resolver.h | 3 +- 5 files changed, 531 insertions(+), 321 deletions(-) diff --git a/src/resolver/builtins_validation_test.cc b/src/resolver/builtins_validation_test.cc index 4af121dbba..38d04e04c0 100644 --- a/src/resolver/builtins_validation_test.cc +++ b/src/resolver/builtins_validation_test.cc @@ -48,6 +48,16 @@ constexpr Params ParamsFor(ast::Builtin builtin, return Params{DataType::AST, builtin, stage, is_valid}; } static constexpr Params cases[] = { + ParamsFor>(ast::Builtin::kPosition, + ast::PipelineStage::kVertex, + false), + ParamsFor>(ast::Builtin::kPosition, + ast::PipelineStage::kFragment, + true), + ParamsFor>(ast::Builtin::kPosition, + ast::PipelineStage::kCompute, + false), + ParamsFor(ast::Builtin::kVertexIndex, ast::PipelineStage::kVertex, true), @@ -183,7 +193,7 @@ INSTANTIATE_TEST_SUITE_P(ResolverBuiltinsValidationTest, TEST_F(ResolverBuiltinsValidationTest, FragDepthIsInput_Fail) { // [[stage(fragment)]] // fn fs_main( - // [[builtin(kFragDepth)]] fd: f32, + // [[builtin(frag_depth)]] fd: f32, // ) -> [[location(0)]] f32 { return 1.0; } auto* fd = Param( "fd", ty.f32(), @@ -197,9 +207,9 @@ TEST_F(ResolverBuiltinsValidationTest, FragDepthIsInput_Fail) { "fragment pipeline stage"); } -TEST_F(ResolverBuiltinsValidationTest, FragDepthIsInputStruct_Fail) { - // Struct MyInputs { - // [[builtin(front_facing)]] ff: bool; +TEST_F(ResolverBuiltinsValidationTest, FragDepthIsInputStruct_Ignored) { + // struct MyInputs { + // [[builtin(frag_depth)]] ff: f32; // }; // [[stage(fragment)]] // fn fragShader(arg: MyInputs) -> [[location(0)]] f32 { return 1.0; } @@ -211,11 +221,7 @@ TEST_F(ResolverBuiltinsValidationTest, FragDepthIsInputStruct_Fail) { Func("fragShader", {Param("arg", ty.Of(s))}, ty.f32(), {Return(1.0f)}, {Stage(ast::PipelineStage::kFragment)}, {Location(0)}); - EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ( - r()->error(), - "12:34 error: builtin(frag_depth) cannot be used in input of fragment " - "pipeline stage\nnote: while analysing entry point fragShader"); + EXPECT_TRUE(r()->Resolve()); } } // namespace StageTest diff --git a/src/resolver/decoration_validation_test.cc b/src/resolver/decoration_validation_test.cc index 48b6c8c84a..03639c7e04 100644 --- a/src/resolver/decoration_validation_test.cc +++ b/src/resolver/decoration_validation_test.cc @@ -13,11 +13,6 @@ // limitations under the License. #include "src/ast/disable_validation_decoration.h" -#include "src/ast/override_decoration.h" -#include "src/ast/return_statement.h" -#include "src/ast/stage_decoration.h" -#include "src/ast/struct_block_decoration.h" -#include "src/ast/workgroup_decoration.h" #include "src/resolver/resolver.h" #include "src/resolver/resolver_test_helper.h" @@ -55,7 +50,6 @@ using u32 = builder::u32; namespace DecorationTests { namespace { - enum class DecorationKind { kAlign, kBinding, @@ -75,7 +69,7 @@ enum class DecorationKind { kBindingAndGroup, }; -bool IsBindingDecoration(DecorationKind kind) { +static bool IsBindingDecoration(DecorationKind kind) { switch (kind) { case DecorationKind::kBinding: case DecorationKind::kGroup: @@ -106,8 +100,7 @@ static ast::DecorationList createDecorations(const Source& source, return {builder.create(source, 1u)}; case DecorationKind::kInterpolate: return {builder.Interpolate(source, ast::InterpolationType::kLinear, - ast::InterpolationSampling::kCenter), - builder.Location(0)}; + ast::InterpolationSampling::kCenter)}; case DecorationKind::kInvariant: return {builder.Invariant(source)}; case DecorationKind::kLocation: @@ -117,7 +110,7 @@ static ast::DecorationList createDecorations(const Source& source, case DecorationKind::kOffset: return {builder.create(source, 4u)}; case DecorationKind::kSize: - return {builder.create(source, 4u)}; + return {builder.create(source, 16u)}; case DecorationKind::kStage: return {builder.Stage(source, ast::PipelineStage::kCompute)}; case DecorationKind::kStride: @@ -134,6 +127,7 @@ static ast::DecorationList createDecorations(const Source& source, return {}; } +namespace FunctionInputAndOutputTests { using FunctionParameterDecorationTest = TestWithParams; TEST_P(FunctionParameterDecorationTest, IsValid) { auto& params = GetParam(); @@ -171,114 +165,6 @@ INSTANTIATE_TEST_SUITE_P( TestParams{DecorationKind::kWorkgroup, false}, TestParams{DecorationKind::kBindingAndGroup, false})); -using EntryPointParameterDecorationTest = TestWithParams; -TEST_P(EntryPointParameterDecorationTest, IsValid) { - auto& params = GetParam(); - - Func("main", - ast::VariableList{Param("a", ty.vec4(), - createDecorations({}, *this, params.kind))}, - ty.void_(), {}, - ast::DecorationList{Stage(ast::PipelineStage::kFragment)}); - - if (params.should_pass) { - EXPECT_TRUE(r()->Resolve()) << r()->error(); - } else { - EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), - "error: decoration is not valid for function parameters"); - } -} -INSTANTIATE_TEST_SUITE_P( - ResolverDecorationValidationTest, - EntryPointParameterDecorationTest, - testing::Values(TestParams{DecorationKind::kAlign, false}, - TestParams{DecorationKind::kBinding, false}, - 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}, - TestParams{DecorationKind::kSize, false}, - TestParams{DecorationKind::kStage, false}, - TestParams{DecorationKind::kStride, false}, - TestParams{DecorationKind::kStructBlock, false}, - TestParams{DecorationKind::kWorkgroup, false}, - TestParams{DecorationKind::kBindingAndGroup, false})); - -TEST_F(EntryPointParameterDecorationTest, DuplicateDecoration) { - Func("main", ast::VariableList{}, ty.f32(), ast::StatementList{Return(1.f)}, - {Stage(ast::PipelineStage::kFragment)}, - { - Location(Source{{12, 34}}, 2), - Location(Source{{56, 78}}, 3), - }); - - EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), - R"(56:78 error: duplicate location decoration -12:34 note: first decoration declared here)"); -} - -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), - }); - Func("f", {s}, ty.void_(), {}, {Stage(ast::PipelineStage::kFragment)}); - - EXPECT_TRUE(r()->Resolve()) << r()->error(); -} - -TEST_F(EntryPointParameterDecorationTest, ComputeShaderLocation) { - auto* input = Param("input", ty.vec4(), - ast::DecorationList{Location(Source{{12, 34}}, 1)}); - Func("main", {input}, ty.void_(), {}, - {Stage(ast::PipelineStage::kCompute), - create(Source{{12, 34}}, Expr(1))}); - - EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), - "12:34 error: decoration is not valid for compute shader function " - "parameters"); -} - -TEST_F(EntryPointParameterDecorationTest, InvariantWithPosition) { - auto* param = Param("p", ty.vec4(), - {Invariant(Source{{12, 34}}), - Builtin(Source{{56, 78}}, ast::Builtin::kPosition)}); - Func("main", ast::VariableList{param}, ty.vec4(), - ast::StatementList{Return(Construct(ty.vec4()))}, - ast::DecorationList{Stage(ast::PipelineStage::kFragment)}, - ast::DecorationList{ - Location(0), - }); - EXPECT_TRUE(r()->Resolve()) << r()->error(); -} - -TEST_F(EntryPointParameterDecorationTest, InvariantWithoutPosition) { - auto* param = - Param("p", ty.vec4(), {Invariant(Source{{12, 34}}), Location(0)}); - Func("main", ast::VariableList{param}, ty.vec4(), - ast::StatementList{Return(Construct(ty.vec4()))}, - ast::DecorationList{Stage(ast::PipelineStage::kFragment)}, - ast::DecorationList{ - Location(0), - }); - EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), - "12:34 error: invariant attribute must only be applied to a " - "position builtin"); -} - using FunctionReturnTypeDecorationTest = TestWithParams; TEST_P(FunctionReturnTypeDecorationTest, IsValid) { auto& params = GetParam(); @@ -313,41 +199,46 @@ INSTANTIATE_TEST_SUITE_P( TestParams{DecorationKind::kStructBlock, false}, TestParams{DecorationKind::kWorkgroup, false}, TestParams{DecorationKind::kBindingAndGroup, false})); +} // namespace FunctionInputAndOutputTests -using EntryPointReturnTypeDecorationTest = TestWithParams; -TEST_P(EntryPointReturnTypeDecorationTest, IsValid) { +namespace EntryPointInputAndOutputTests { +using ComputeShaderParameterDecorationTest = TestWithParams; +TEST_P(ComputeShaderParameterDecorationTest, IsValid) { auto& params = GetParam(); - - Func("main", ast::VariableList{}, ty.vec4(), - {Return(Construct(ty.vec4(), 1.f))}, - {Stage(ast::PipelineStage::kCompute), WorkgroupSize(1)}, - createDecorations({}, *this, params.kind)); + auto* p = Param("a", ty.vec4(), + createDecorations(Source{{12, 34}}, *this, params.kind)); + Func("main", ast::VariableList{p}, ty.void_(), {}, + {Stage(ast::PipelineStage::kCompute), WorkgroupSize(1)}); if (params.should_pass) { EXPECT_TRUE(r()->Resolve()) << r()->error(); } else { EXPECT_FALSE(r()->Resolve()); - if (params.kind == DecorationKind::kLocation || - params.kind == DecorationKind::kInterpolate) { + if (params.kind == DecorationKind::kBuiltin) { EXPECT_EQ(r()->error(), - "error: decoration is not valid for compute shader entry point " - "return types"); + "12:34 error: builtin(position) cannot be used in input of " + "compute pipeline stage"); + } else if (params.kind == DecorationKind::kInterpolate || + params.kind == DecorationKind::kLocation || + params.kind == DecorationKind::kInvariant) { + EXPECT_EQ( + r()->error(), + "12:34 error: decoration is not valid for compute shader inputs"); } else { EXPECT_EQ(r()->error(), - "error: decoration is not valid for entry point return types"); + "12:34 error: decoration is not valid for function parameters"); } } } - INSTANTIATE_TEST_SUITE_P( ResolverDecorationValidationTest, - EntryPointReturnTypeDecorationTest, + ComputeShaderParameterDecorationTest, testing::Values(TestParams{DecorationKind::kAlign, false}, TestParams{DecorationKind::kBinding, false}, - TestParams{DecorationKind::kBuiltin, true}, + TestParams{DecorationKind::kBuiltin, false}, TestParams{DecorationKind::kGroup, false}, TestParams{DecorationKind::kInterpolate, false}, - // kInvariant tested separately (requires position builtin) + TestParams{DecorationKind::kInvariant, false}, TestParams{DecorationKind::kLocation, false}, TestParams{DecorationKind::kOverride, false}, TestParams{DecorationKind::kOffset, false}, @@ -358,6 +249,271 @@ INSTANTIATE_TEST_SUITE_P( TestParams{DecorationKind::kWorkgroup, false}, TestParams{DecorationKind::kBindingAndGroup, false})); +using FragmentShaderParameterDecorationTest = TestWithParams; +TEST_P(FragmentShaderParameterDecorationTest, IsValid) { + auto& params = GetParam(); + auto decos = createDecorations(Source{{12, 34}}, *this, params.kind); + if (params.kind != DecorationKind::kBuiltin && + params.kind != DecorationKind::kLocation) { + decos.push_back(Builtin(Source{{34, 56}}, ast::Builtin::kPosition)); + } + auto* p = Param("a", ty.vec4(), decos); + Func("frag_main", {p}, ty.void_(), {}, + {Stage(ast::PipelineStage::kFragment)}); + + if (params.should_pass) { + EXPECT_TRUE(r()->Resolve()) << r()->error(); + } else { + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: decoration is not valid for function parameters"); + } +} +INSTANTIATE_TEST_SUITE_P( + ResolverDecorationValidationTest, + FragmentShaderParameterDecorationTest, + testing::Values(TestParams{DecorationKind::kAlign, false}, + TestParams{DecorationKind::kBinding, false}, + 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, false}, + TestParams{DecorationKind::kSize, false}, + TestParams{DecorationKind::kStage, false}, + TestParams{DecorationKind::kStride, false}, + TestParams{DecorationKind::kStructBlock, false}, + TestParams{DecorationKind::kWorkgroup, false}, + TestParams{DecorationKind::kBindingAndGroup, false})); + +using VertexShaderParameterDecorationTest = TestWithParams; +TEST_P(VertexShaderParameterDecorationTest, IsValid) { + auto& params = GetParam(); + auto decos = createDecorations(Source{{12, 34}}, *this, params.kind); + if (params.kind != DecorationKind::kLocation) { + decos.push_back(Location(Source{{34, 56}}, 2)); + } + auto* p = Param("a", ty.vec4(), decos); + Func("vertex_main", ast::VariableList{p}, ty.vec4(), + {Return(Construct(ty.vec4()))}, + {Stage(ast::PipelineStage::kVertex)}, + {Builtin(ast::Builtin::kPosition)}); + + if (params.should_pass) { + EXPECT_TRUE(r()->Resolve()) << r()->error(); + } else { + EXPECT_FALSE(r()->Resolve()); + if (params.kind == DecorationKind::kBuiltin) { + EXPECT_EQ(r()->error(), + "12:34 error: builtin(position) cannot be used in input of " + "vertex pipeline stage"); + } else if (params.kind == DecorationKind::kInvariant) { + EXPECT_EQ(r()->error(), + "12:34 error: invariant attribute must only be applied to a " + "position builtin"); + } else { + EXPECT_EQ(r()->error(), + "12:34 error: decoration is not valid for function parameters"); + } + } +} +INSTANTIATE_TEST_SUITE_P( + ResolverDecorationValidationTest, + VertexShaderParameterDecorationTest, + testing::Values(TestParams{DecorationKind::kAlign, false}, + TestParams{DecorationKind::kBinding, false}, + TestParams{DecorationKind::kBuiltin, false}, + TestParams{DecorationKind::kGroup, false}, + TestParams{DecorationKind::kInterpolate, true}, + TestParams{DecorationKind::kInvariant, false}, + TestParams{DecorationKind::kLocation, true}, + TestParams{DecorationKind::kOverride, false}, + TestParams{DecorationKind::kOffset, false}, + TestParams{DecorationKind::kSize, false}, + TestParams{DecorationKind::kStage, false}, + TestParams{DecorationKind::kStride, false}, + TestParams{DecorationKind::kStructBlock, false}, + TestParams{DecorationKind::kWorkgroup, false}, + TestParams{DecorationKind::kBindingAndGroup, false})); + +using ComputeShaderReturnTypeDecorationTest = TestWithParams; +TEST_P(ComputeShaderReturnTypeDecorationTest, IsValid) { + auto& params = GetParam(); + Func("main", ast::VariableList{}, ty.vec4(), + {Return(Construct(ty.vec4(), 1.f))}, + {Stage(ast::PipelineStage::kCompute), WorkgroupSize(1)}, + createDecorations(Source{{12, 34}}, *this, params.kind)); + + if (params.should_pass) { + EXPECT_TRUE(r()->Resolve()) << r()->error(); + } else { + EXPECT_FALSE(r()->Resolve()); + if (params.kind == DecorationKind::kBuiltin) { + EXPECT_EQ(r()->error(), + "12:34 error: builtin(position) cannot be used in output of " + "compute pipeline stage"); + } else if (params.kind == DecorationKind::kInterpolate || + params.kind == DecorationKind::kLocation || + params.kind == DecorationKind::kInvariant) { + EXPECT_EQ( + r()->error(), + "12:34 error: decoration is not valid for compute shader output"); + } else { + EXPECT_EQ(r()->error(), + "12:34 error: decoration is not valid for entry point return " + "types"); + } + } +} +INSTANTIATE_TEST_SUITE_P( + ResolverDecorationValidationTest, + ComputeShaderReturnTypeDecorationTest, + testing::Values(TestParams{DecorationKind::kAlign, false}, + TestParams{DecorationKind::kBinding, false}, + 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}, + TestParams{DecorationKind::kSize, false}, + TestParams{DecorationKind::kStage, false}, + TestParams{DecorationKind::kStride, false}, + TestParams{DecorationKind::kStructBlock, false}, + TestParams{DecorationKind::kWorkgroup, false}, + TestParams{DecorationKind::kBindingAndGroup, false})); + +using FragmentShaderReturnTypeDecorationTest = TestWithParams; +TEST_P(FragmentShaderReturnTypeDecorationTest, IsValid) { + auto& params = GetParam(); + auto decos = createDecorations(Source{{12, 34}}, *this, params.kind); + decos.push_back(Location(Source{{34, 56}}, 2)); + Func("frag_main", {}, ty.vec4(), {Return(Construct(ty.vec4()))}, + {Stage(ast::PipelineStage::kFragment)}, decos); + + if (params.should_pass) { + EXPECT_TRUE(r()->Resolve()) << r()->error(); + } else { + EXPECT_FALSE(r()->Resolve()); + if (params.kind == DecorationKind::kBuiltin) { + EXPECT_EQ(r()->error(), + "12:34 error: builtin(position) cannot be used in output of " + "fragment pipeline stage"); + } else if (params.kind == DecorationKind::kInvariant) { + EXPECT_EQ(r()->error(), + "12:34 error: invariant attribute must only be applied to a " + "position builtin"); + } else if (params.kind == DecorationKind::kLocation) { + EXPECT_EQ(r()->error(), + "34:56 error: duplicate location decoration\n" + "12:34 note: first decoration declared here"); + } else { + EXPECT_EQ(r()->error(), + "12:34 error: decoration is not valid for entry point return " + "types"); + } + } +} +INSTANTIATE_TEST_SUITE_P( + ResolverDecorationValidationTest, + FragmentShaderReturnTypeDecorationTest, + testing::Values(TestParams{DecorationKind::kAlign, false}, + TestParams{DecorationKind::kBinding, false}, + TestParams{DecorationKind::kBuiltin, false}, + TestParams{DecorationKind::kGroup, false}, + TestParams{DecorationKind::kInterpolate, true}, + TestParams{DecorationKind::kInvariant, false}, + TestParams{DecorationKind::kLocation, false}, + TestParams{DecorationKind::kOverride, false}, + TestParams{DecorationKind::kOffset, false}, + TestParams{DecorationKind::kSize, false}, + TestParams{DecorationKind::kStage, false}, + TestParams{DecorationKind::kStride, false}, + TestParams{DecorationKind::kStructBlock, false}, + TestParams{DecorationKind::kWorkgroup, false}, + TestParams{DecorationKind::kBindingAndGroup, false})); + +using VertexShaderReturnTypeDecorationTest = TestWithParams; +TEST_P(VertexShaderReturnTypeDecorationTest, IsValid) { + auto& params = GetParam(); + auto decos = createDecorations(Source{{12, 34}}, *this, params.kind); + // a vertex shader must include the 'position' builtin in its return type + if (params.kind != DecorationKind::kBuiltin) { + decos.push_back(Builtin(Source{{34, 56}}, ast::Builtin::kPosition)); + } + Func("vertex_main", ast::VariableList{}, ty.vec4(), + {Return(Construct(ty.vec4()))}, + {Stage(ast::PipelineStage::kVertex)}, decos); + + if (params.should_pass) { + EXPECT_TRUE(r()->Resolve()) << r()->error(); + } else { + EXPECT_FALSE(r()->Resolve()); + if (params.kind == DecorationKind::kLocation) { + EXPECT_EQ(r()->error(), + "34:56 error: multiple entry point IO attributes\n" + "12:34 note: previously consumed location(1)"); + } else { + EXPECT_EQ(r()->error(), + "12:34 error: decoration is not valid for entry point return " + "types"); + } + } +} +INSTANTIATE_TEST_SUITE_P( + ResolverDecorationValidationTest, + VertexShaderReturnTypeDecorationTest, + testing::Values(TestParams{DecorationKind::kAlign, false}, + TestParams{DecorationKind::kBinding, false}, + TestParams{DecorationKind::kBuiltin, true}, + TestParams{DecorationKind::kGroup, false}, + TestParams{DecorationKind::kInterpolate, true}, + TestParams{DecorationKind::kInvariant, true}, + TestParams{DecorationKind::kLocation, false}, + TestParams{DecorationKind::kOverride, false}, + TestParams{DecorationKind::kOffset, false}, + TestParams{DecorationKind::kSize, false}, + TestParams{DecorationKind::kStage, false}, + TestParams{DecorationKind::kStride, false}, + TestParams{DecorationKind::kStructBlock, false}, + TestParams{DecorationKind::kWorkgroup, false}, + TestParams{DecorationKind::kBindingAndGroup, false})); + +using EntryPointParameterDecorationTest = TestWithParams; +TEST_F(EntryPointParameterDecorationTest, DuplicateDecoration) { + Func("main", ast::VariableList{}, ty.f32(), ast::StatementList{Return(1.f)}, + {Stage(ast::PipelineStage::kFragment)}, + { + Location(Source{{12, 34}}, 2), + Location(Source{{56, 78}}, 3), + }); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + R"(56:78 error: duplicate location decoration +12:34 note: first decoration declared here)"); +} + +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), + }); + Func("f", {s}, ty.void_(), {}, {Stage(ast::PipelineStage::kFragment)}); + + EXPECT_TRUE(r()->Resolve()) << r()->error(); +} + +using EntryPointReturnTypeDecorationTest = ResolverTest; TEST_F(EntryPointReturnTypeDecorationTest, DuplicateDecoration) { Func("main", ast::VariableList{}, ty.f32(), ast::StatementList{Return(1.f)}, ast::DecorationList{Stage(ast::PipelineStage::kFragment)}, @@ -372,73 +528,20 @@ 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)}, +TEST_F(EntryPointReturnTypeDecorationTest, DuplicateInternalDecoration) { + Func("f", {}, ty.i32(), {Return(1)}, {Stage(ast::PipelineStage::kFragment)}, ast::DecorationList{ - Invariant(Source{{12, 34}}), - Builtin(Source{{56, 78}}, ast::Builtin::kPosition), + ASTNodes().Create( + ID(), ast::DisabledValidation::kBindingPointCollision), + ASTNodes().Create( + ID(), ast::DisabledValidation::kEntryPointParameter), }); + EXPECT_TRUE(r()->Resolve()) << r()->error(); } +} // namespace EntryPointInputAndOutputTests -TEST_F(EntryPointReturnTypeDecorationTest, InvariantWithoutPosition) { - Func("main", ast::VariableList{}, ty.vec4(), - ast::StatementList{Return(Construct(ty.vec4()))}, - ast::DecorationList{Stage(ast::PipelineStage::kVertex)}, - 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(); - - auto* arr = ty.array(ty.f32(), 0, - createDecorations(Source{{12, 34}}, *this, params.kind)); - Structure("mystruct", - { - Member("a", arr), - }, - {create()}); - - WrapInFunction(); - - if (params.should_pass) { - EXPECT_TRUE(r()->Resolve()) << r()->error(); - } else { - EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), - "12:34 error: decoration is not valid for array types"); - } -} -INSTANTIATE_TEST_SUITE_P( - ResolverDecorationValidationTest, - ArrayDecorationTest, - testing::Values(TestParams{DecorationKind::kAlign, false}, - TestParams{DecorationKind::kBinding, false}, - 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}, - TestParams{DecorationKind::kSize, false}, - TestParams{DecorationKind::kStage, false}, - TestParams{DecorationKind::kStride, true}, - TestParams{DecorationKind::kStructBlock, false}, - TestParams{DecorationKind::kWorkgroup, false}, - TestParams{DecorationKind::kBindingAndGroup, false})); - +namespace StructAndStructMemberTests { using StructDecorationTest = TestWithParams; TEST_P(StructDecorationTest, IsValid) { auto& params = GetParam(); @@ -484,19 +587,15 @@ TEST_F(StructDecorationTest, DuplicateDecoration) { create(Source{{12, 34}}), create(Source{{56, 78}}), }); - WrapInFunction(); - EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), R"(56:78 error: duplicate block decoration 12:34 note: first decoration declared here)"); } - using StructMemberDecorationTest = TestWithParams; TEST_P(StructMemberDecorationTest, IsValid) { auto& params = GetParam(); - ast::StructMemberList members; if (params.kind == DecorationKind::kBuiltin) { members.push_back( @@ -507,11 +606,8 @@ TEST_P(StructMemberDecorationTest, IsValid) { {Member("a", ty.f32(), createDecorations(Source{{12, 34}}, *this, params.kind))}); } - Structure("mystruct", members); - WrapInFunction(); - if (params.should_pass) { EXPECT_TRUE(r()->Resolve()) << r()->error(); } else { @@ -538,7 +634,6 @@ INSTANTIATE_TEST_SUITE_P( TestParams{DecorationKind::kStructBlock, false}, TestParams{DecorationKind::kWorkgroup, false}, TestParams{DecorationKind::kBindingAndGroup, false})); - TEST_F(StructMemberDecorationTest, DuplicateDecoration) { Structure("mystruct", { Member("a", ty.i32(), @@ -549,15 +644,12 @@ TEST_F(StructMemberDecorationTest, DuplicateDecoration) { Source{{56, 78}}, 8u), }), }); - WrapInFunction(); - EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), R"(56:78 error: duplicate align decoration 12:34 note: first decoration declared here)"); } - TEST_F(StructMemberDecorationTest, InvariantDecorationWithPosition) { Structure("mystruct", { Member("a", ty.vec4(), @@ -566,11 +658,9 @@ TEST_F(StructMemberDecorationTest, InvariantDecorationWithPosition) { Builtin(ast::Builtin::kPosition), }), }); - WrapInFunction(); EXPECT_TRUE(r()->Resolve()) << r()->error(); } - TEST_F(StructMemberDecorationTest, InvariantDecorationWithoutPosition) { Structure("mystruct", { Member("a", ty.vec4(), @@ -578,7 +668,6 @@ TEST_F(StructMemberDecorationTest, InvariantDecorationWithoutPosition) { Invariant(Source{{12, 34}}), }), }); - WrapInFunction(); EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), @@ -586,6 +675,49 @@ TEST_F(StructMemberDecorationTest, InvariantDecorationWithoutPosition) { "position builtin"); } +} // namespace StructAndStructMemberTests + +using ArrayDecorationTest = TestWithParams; +TEST_P(ArrayDecorationTest, IsValid) { + auto& params = GetParam(); + + auto* arr = ty.array(ty.f32(), 0, + createDecorations(Source{{12, 34}}, *this, params.kind)); + Structure("mystruct", + { + Member("a", arr), + }, + {create()}); + + WrapInFunction(); + + if (params.should_pass) { + EXPECT_TRUE(r()->Resolve()) << r()->error(); + } else { + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: decoration is not valid for array types"); + } +} +INSTANTIATE_TEST_SUITE_P( + ResolverDecorationValidationTest, + ArrayDecorationTest, + testing::Values(TestParams{DecorationKind::kAlign, false}, + TestParams{DecorationKind::kBinding, false}, + 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}, + TestParams{DecorationKind::kSize, false}, + TestParams{DecorationKind::kStage, false}, + TestParams{DecorationKind::kStride, true}, + TestParams{DecorationKind::kStructBlock, false}, + TestParams{DecorationKind::kWorkgroup, false}, + TestParams{DecorationKind::kBindingAndGroup, false})); + using VariableDecorationTest = TestWithParams; TEST_P(VariableDecorationTest, IsValid) { auto& params = GetParam(); @@ -697,41 +829,6 @@ TEST_F(ConstantDecorationTest, DuplicateDecoration) { 12:34 note: first decoration declared here)"); } -using FunctionDecorationTest = TestWithParams; -TEST_P(FunctionDecorationTest, IsValid) { - auto& params = GetParam(); - - ast::DecorationList decos = - createDecorations(Source{{12, 34}}, *this, params.kind); - Func("foo", ast::VariableList{}, ty.void_(), ast::StatementList{}, decos); - - if (params.should_pass) { - EXPECT_TRUE(r()->Resolve()) << r()->error(); - } else { - EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), - "12:34 error: decoration is not valid for functions"); - } -} -INSTANTIATE_TEST_SUITE_P( - ResolverDecorationValidationTest, - FunctionDecorationTest, - testing::Values(TestParams{DecorationKind::kAlign, false}, - TestParams{DecorationKind::kBinding, false}, - 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}, - TestParams{DecorationKind::kSize, false}, - // Skip kStage as we do not apply it in this test - TestParams{DecorationKind::kStride, false}, - TestParams{DecorationKind::kStructBlock, false}, - // Skip kWorkgroup as this is a different error - TestParams{DecorationKind::kBindingAndGroup, false})); - } // namespace } // namespace DecorationTests @@ -1046,6 +1143,108 @@ TEST_F(ResourceDecorationTest, BindingPointOnNonResource) { } // namespace } // namespace ResourceTests +namespace LocationDecorationTests { +namespace { +using LocationDecorationTests = ResolverTest; +TEST_F(LocationDecorationTests, ComputeShaderLocation_Input) { + Func("main", {}, ty.i32(), {Return(Expr(1))}, + {Stage(ast::PipelineStage::kCompute), + create(Source{{12, 34}}, Expr(1))}, + ast::DecorationList{Location(Source{{12, 34}}, 1)}); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: decoration is not valid for compute shader output"); +} + +TEST_F(LocationDecorationTests, ComputeShaderLocation_Output) { + auto* input = Param("input", ty.i32(), + ast::DecorationList{Location(Source{{12, 34}}, 0u)}); + Func("main", {input}, ty.void_(), {}, + {Stage(ast::PipelineStage::kCompute), + create(Source{{12, 34}}, Expr(1))}); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: decoration is not valid for compute shader inputs"); +} + +TEST_F(LocationDecorationTests, ComputeShaderLocationStructMember_Output) { + auto* m = Member("m", ty.i32(), + ast::DecorationList{Location(Source{{12, 34}}, 0u)}); + auto* s = Structure("S", {m}); + Func(Source{{56, 78}}, "main", {}, ty.Of(s), + ast::StatementList{Return(Expr(Construct(ty.Of(s))))}, + {Stage(ast::PipelineStage::kCompute), + create(Source{{12, 34}}, Expr(1))}); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: decoration is not valid for compute shader output\n" + "56:78 note: while analysing entry point 'main'"); +} + +TEST_F(LocationDecorationTests, ComputeShaderLocationStructMember_Input) { + auto* m = Member("m", ty.i32(), + ast::DecorationList{Location(Source{{12, 34}}, 0u)}); + auto* s = Structure("S", {m}); + auto* input = Param("input", ty.Of(s)); + Func(Source{{56, 78}}, "main", {input}, ty.void_(), {}, + {Stage(ast::PipelineStage::kCompute), + create(Source{{12, 34}}, Expr(1))}); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: decoration is not valid for compute shader inputs\n" + "56:78 note: while analysing entry point 'main'"); +} + +TEST_F(LocationDecorationTests, BadType) { + auto* p = Param(Source{{12, 34}}, "a", ty.mat2x2(), {Location(0)}); + Func("frag_main", {p}, ty.void_(), {}, + {Stage(ast::PipelineStage::kFragment)}); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: User defined entry point IO types must be a numeric " + "scalar, a numeric vector, or a structure"); +} +} // namespace +} // namespace LocationDecorationTests + +namespace InvariantDecorationTests { +namespace { +using InvariantDecorationTests = ResolverTest; +TEST_F(InvariantDecorationTests, InvariantWithPosition) { + auto* param = Param("p", ty.vec4(), + {Invariant(Source{{12, 34}}), + Builtin(Source{{56, 78}}, ast::Builtin::kPosition)}); + Func("main", ast::VariableList{param}, ty.vec4(), + ast::StatementList{Return(Construct(ty.vec4()))}, + ast::DecorationList{Stage(ast::PipelineStage::kFragment)}, + ast::DecorationList{ + Location(0), + }); + EXPECT_TRUE(r()->Resolve()) << r()->error(); +} + +TEST_F(InvariantDecorationTests, InvariantWithoutPosition) { + auto* param = + Param("p", ty.vec4(), {Invariant(Source{{12, 34}}), Location(0)}); + Func("main", ast::VariableList{param}, ty.vec4(), + ast::StatementList{Return(Construct(ty.vec4()))}, + ast::DecorationList{Stage(ast::PipelineStage::kFragment)}, + ast::DecorationList{ + Location(0), + }); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: invariant attribute must only be applied to a " + "position builtin"); +} +} // namespace +} // namespace InvariantDecorationTests + namespace WorkgroupDecorationTests { namespace { diff --git a/src/resolver/entry_point_validation_test.cc b/src/resolver/entry_point_validation_test.cc index bd0201f1a3..27c4dd699e 100644 --- a/src/resolver/entry_point_validation_test.cc +++ b/src/resolver/entry_point_validation_test.cc @@ -160,7 +160,7 @@ TEST_F(ResolverEntryPointValidationTest, EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), R"(14:52 error: multiple entry point IO attributes 13:43 note: previously consumed location(0) -12:34 note: while analysing entry point main)"); +12:34 note: while analysing entry point 'main')"); } TEST_F(ResolverEntryPointValidationTest, @@ -183,7 +183,7 @@ TEST_F(ResolverEntryPointValidationTest, EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), R"(14:52 error: missing entry point IO attribute -12:34 note: while analysing entry point main)"); +12:34 note: while analysing entry point 'main')"); } TEST_F(ResolverEntryPointValidationTest, ReturnType_Struct_NestedStruct) { @@ -209,7 +209,7 @@ TEST_F(ResolverEntryPointValidationTest, ReturnType_Struct_NestedStruct) { EXPECT_EQ( r()->error(), R"(14:52 error: entry point IO types cannot contain nested structures -12:34 note: while analysing entry point main)"); +12:34 note: while analysing entry point 'main')"); } TEST_F(ResolverEntryPointValidationTest, ReturnType_Struct_RuntimeArray) { @@ -254,7 +254,7 @@ TEST_F(ResolverEntryPointValidationTest, ReturnType_Struct_DuplicateBuiltins) { EXPECT_EQ( r()->error(), R"(12:34 error: builtin(frag_depth) attribute appears multiple times as pipeline output -12:34 note: while analysing entry point main)"); +12:34 note: while analysing entry point 'main')"); } TEST_F(ResolverEntryPointValidationTest, ReturnType_Struct_DuplicateLocation) { @@ -276,7 +276,7 @@ TEST_F(ResolverEntryPointValidationTest, ReturnType_Struct_DuplicateLocation) { EXPECT_EQ( r()->error(), R"(12:34 error: location(1) attribute appears multiple times as pipeline output -12:34 note: while analysing entry point main)"); +12:34 note: while analysing entry point 'main')"); } TEST_F(ResolverEntryPointValidationTest, ParameterAttribute_Location) { @@ -369,7 +369,7 @@ TEST_F(ResolverEntryPointValidationTest, EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), R"(14:52 error: multiple entry point IO attributes 13:43 note: previously consumed location(0) -12:34 note: while analysing entry point main)"); +12:34 note: while analysing entry point 'main')"); } TEST_F(ResolverEntryPointValidationTest, @@ -389,7 +389,7 @@ TEST_F(ResolverEntryPointValidationTest, EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), R"(14:52 error: missing entry point IO attribute -12:34 note: while analysing entry point main)"); +12:34 note: while analysing entry point 'main')"); } TEST_F(ResolverEntryPointValidationTest, Parameter_Struct_NestedStruct) { @@ -413,7 +413,7 @@ TEST_F(ResolverEntryPointValidationTest, Parameter_Struct_NestedStruct) { EXPECT_EQ( r()->error(), R"(14:52 error: entry point IO types cannot contain nested structures -12:34 note: while analysing entry point main)"); +12:34 note: while analysing entry point 'main')"); } TEST_F(ResolverEntryPointValidationTest, Parameter_Struct_RuntimeArray) { @@ -477,7 +477,7 @@ TEST_F(ResolverEntryPointValidationTest, Parameter_Struct_DuplicateBuiltins) { EXPECT_EQ( r()->error(), R"(12:34 error: builtin(sample_index) attribute appears multiple times as pipeline input -12:34 note: while analysing entry point main)"); +12:34 note: while analysing entry point 'main')"); } TEST_F(ResolverEntryPointValidationTest, Parameter_DuplicateLocation) { @@ -515,7 +515,7 @@ TEST_F(ResolverEntryPointValidationTest, Parameter_Struct_DuplicateLocation) { EXPECT_EQ( r()->error(), R"(12:34 error: location(1) attribute appears multiple times as pipeline input -12:34 note: while analysing entry point main)"); +12:34 note: while analysing entry point 'main')"); } TEST_F(ResolverEntryPointValidationTest, VertexShaderMustReturnPosition) { diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc index f7bb4e4358..26565685f6 100644 --- a/src/resolver/resolver.cc +++ b/src/resolver/resolver.cc @@ -1138,18 +1138,9 @@ bool Resolver::ValidateFunctionParameter(const ast::Function* func, "decoration is not valid for non-entry point function parameters", deco->source()); return false; - } else if (auto* interpolate = deco->As()) { - if (!ValidateInterpolateDecoration(interpolate, info->type)) { - return false; - } - } else if (deco->Is()) { - if (func->pipeline_stage() == ast::PipelineStage::kCompute) { - AddError( - "decoration is not valid for compute shader function parameters", - deco->source()); - return false; - } } else if (!deco->IsAnyOf() && (IsValidationEnabled( info->declaration->decorations(), @@ -1198,7 +1189,8 @@ bool Resolver::ValidateFunctionParameter(const ast::Function* func, bool Resolver::ValidateBuiltinDecoration(const ast::BuiltinDecoration* deco, const sem::Type* storage_type, - const bool is_input) { + const bool is_input, + const bool is_struct_member) { auto* type = storage_type->UnwrapRef(); const auto stage = current_function_ ? current_function_->declaration->pipeline_stage() @@ -1206,15 +1198,13 @@ bool Resolver::ValidateBuiltinDecoration(const ast::BuiltinDecoration* deco, std::stringstream stage_name; stage_name << stage; bool is_stage_mismatch = false; + bool is_output = !is_input; switch (deco->value()) { case ast::Builtin::kPosition: if (stage != ast::PipelineStage::kNone && - !(stage == ast::PipelineStage::kFragment && is_input) && - !(stage == ast::PipelineStage::kVertex && !is_input)) { - AddError(deco_to_str(deco) + " cannot be used in " + - (is_input ? "input of " : "output of ") + - stage_name.str() + " pipeline stage", - deco->source()); + !((is_input && stage == ast::PipelineStage::kFragment) || + (is_output && stage == ast::PipelineStage::kVertex))) { + is_stage_mismatch = true; } if (!(type->is_float_vector() && type->As()->Width() == 4)) { AddError("store type of " + deco_to_str(deco) + " must be 'vec4'", @@ -1311,12 +1301,16 @@ bool Resolver::ValidateBuiltinDecoration(const ast::BuiltinDecoration* deco, break; } - if (is_stage_mismatch) { - AddError(deco_to_str(deco) + " cannot be used in " + - (is_input ? "input of " : "output of ") + stage_name.str() + - " pipeline stage", - deco->source()); - return false; + // ignore builtin attribute on struct members to facillate data movement + // between stages + if (!is_struct_member) { + if (is_stage_mismatch) { + AddError(deco_to_str(deco) + " cannot be used in " + + (is_input ? "input of " : "output of ") + stage_name.str() + + " pipeline stage", + deco->source()); + return false; + } } return true; @@ -1409,28 +1403,14 @@ bool Resolver::ValidateFunction(const ast::Function* func, deco->source()); return false; } - - if (auto* interpolate = deco->As()) { - if (!ValidateInterpolateDecoration(interpolate, info->return_type)) { - return false; - } - } else if (deco->Is()) { - if (func->pipeline_stage() == ast::PipelineStage::kCompute) { - AddError( - "decoration is not valid for compute shader entry point return " - "types", - deco->source()); - return false; - } - } else if (!deco->IsAnyOf() && - (IsValidationEnabled( - info->declaration->decorations(), - ast::DisabledValidation::kEntryPointParameter) && - IsValidationEnabled( - info->declaration->decorations(), - ast::DisabledValidation:: - kIgnoreConstructibleFunctionParameter))) { + if (!deco->IsAnyOf() && + (IsValidationEnabled(info->declaration->decorations(), + ast::DisabledValidation::kEntryPointParameter) && + IsValidationEnabled(info->declaration->decorations(), + ast::DisabledValidation:: + kIgnoreConstructibleFunctionParameter))) { AddError("decoration is not valid for entry point return types", deco->source()); return false; @@ -1473,6 +1453,7 @@ bool Resolver::ValidateEntryPoint(const ast::Function* func, ast::Decoration* pipeline_io_attribute = nullptr; ast::InvariantDecoration* invariant_attribute = nullptr; for (auto* deco : decos) { + auto is_invalid_compute_shader_decoration = false; if (auto* builtin = deco->As()) { if (pipeline_io_attribute) { AddError("multiple entry point IO attributes", deco->source()); @@ -1490,14 +1471,14 @@ bool Resolver::ValidateEntryPoint(const ast::Function* func, func->source()); return false; } - builtins.emplace(builtin->value()); if (!ValidateBuiltinDecoration( builtin, ty, - /* is_input */ param_or_ret == ParamOrRetType::kParameter)) { + /* is_input */ param_or_ret == ParamOrRetType::kParameter, + /* is_struct_member */ is_struct_member)) { return false; } - + builtins.emplace(builtin->value()); } else if (auto* location = deco->As()) { if (pipeline_io_attribute) { AddError("multiple entry point IO attributes", deco->source()); @@ -1515,10 +1496,31 @@ bool Resolver::ValidateEntryPoint(const ast::Function* func, func->source()); return false; } + + if (func->pipeline_stage() == ast::PipelineStage::kCompute) { + is_invalid_compute_shader_decoration = true; + } locations.emplace(location->value()); + } else if (auto* interpolate = deco->As()) { + if (func->pipeline_stage() == ast::PipelineStage::kCompute) { + is_invalid_compute_shader_decoration = true; + } else if (!ValidateInterpolateDecoration(interpolate, ty)) { + return false; + } } else if (auto* invariant = deco->As()) { + if (func->pipeline_stage() == ast::PipelineStage::kCompute) { + is_invalid_compute_shader_decoration = true; + } invariant_attribute = invariant; } + if (is_invalid_compute_shader_decoration) { + std::string input_or_output = + param_or_ret == ParamOrRetType::kParameter ? "inputs" : "output"; + AddError( + "decoration is not valid for compute shader " + input_or_output, + deco->source()); + return false; + } } // Check that we saw a pipeline IO attribute iff we need one. @@ -1587,8 +1589,8 @@ bool Resolver::ValidateEntryPoint(const ast::Function* func, if (member->Type()->Is()) { AddError("entry point IO types cannot contain nested structures", member->Declaration()->source()); - AddNote("while analysing entry point " + - builder_->Symbols().NameFor(func->symbol()), + AddNote("while analysing entry point '" + + builder_->Symbols().NameFor(func->symbol()) + "'", func->source()); return false; } @@ -1597,8 +1599,8 @@ bool Resolver::ValidateEntryPoint(const ast::Function* func, if (arr->IsRuntimeSized()) { AddError("entry point IO types cannot contain runtime sized arrays", member->Declaration()->source()); - AddNote("while analysing entry point " + - builder_->Symbols().NameFor(func->symbol()), + AddNote("while analysing entry point '" + + builder_->Symbols().NameFor(func->symbol()) + "'", func->source()); return false; } @@ -1607,8 +1609,8 @@ bool Resolver::ValidateEntryPoint(const ast::Function* func, if (!validate_entry_point_decorations_inner( member->Declaration()->decorations(), member->Type(), member->Declaration()->source(), param_or_ret, true)) { - AddNote("while analysing entry point " + - builder_->Symbols().NameFor(func->symbol()), + AddNote("while analysing entry point '" + + builder_->Symbols().NameFor(func->symbol()) + "'", func->source()); return false; } @@ -3938,7 +3940,9 @@ bool Resolver::ValidateStructure(const sem::Struct* str) { invariant_attribute = invariant; } if (auto* builtin = deco->As()) { - if (!ValidateBuiltinDecoration(builtin, member->Type())) { + if (!ValidateBuiltinDecoration(builtin, member->Type(), + /* is_input */ false, + /* is_struct_member */ true)) { return false; } if (builtin->value() == ast::Builtin::kPosition) { diff --git a/src/resolver/resolver.h b/src/resolver/resolver.h index 600ef229e4..446bac6789 100644 --- a/src/resolver/resolver.h +++ b/src/resolver/resolver.h @@ -281,7 +281,8 @@ class Resolver { bool ValidateAssignment(const ast::AssignmentStatement* a); bool ValidateBuiltinDecoration(const ast::BuiltinDecoration* deco, const sem::Type* storage_type, - const bool is_input = true); + const bool is_input, + const bool is_struct_member); bool ValidateCallStatement(ast::CallStatement* stmt); bool ValidateEntryPoint(const ast::Function* func, const FunctionInfo* info); bool ValidateFunction(const ast::Function* func, const FunctionInfo* info);