From f0c150b01bd832ea2922c49b37860cca5e23cc83 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Fri, 3 Jun 2022 02:47:40 +0000 Subject: [PATCH] Add parsing of shorter stage attributes. This CL adds the ability to parse the `@compute`, `@fragment` and `@vertex` attrbutes. The `@stage(...)` are still accepted and are not marked as deprecated yet. Most tests are still using `@stage(..)` except for a testing one. Bug: tint:1503 Change-Id: I85cad5996605035e83109b021ffb13db98b1a144 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/92480 Reviewed-by: Dan Sinclair Reviewed-by: Antonio Maiorano Auto-Submit: Dan Sinclair Kokoro: Kokoro Commit-Queue: Dan Sinclair Reviewed-by: Ryan Harrison --- src/tint/reader/wgsl/parser_impl.cc | 29 ++++++++++++++ ...arser_impl_function_attribute_list_test.cc | 28 ++++++++++++- .../parser_impl_function_attribute_test.cc | 39 +++++++++++++++++++ .../transform/canonicalize_entry_point_io.h | 4 +- .../module_scope_var_to_entry_point_param.h | 4 +- .../shader_io/compute_input_builtins.wgsl | 2 +- 6 files changed, 100 insertions(+), 6 deletions(-) diff --git a/src/tint/reader/wgsl/parser_impl.cc b/src/tint/reader/wgsl/parser_impl.cc index 4ad0f76e40..a28b7987fb 100644 --- a/src/tint/reader/wgsl/parser_impl.cc +++ b/src/tint/reader/wgsl/parser_impl.cc @@ -3258,9 +3258,38 @@ Maybe ParserImpl::attribute() { return Failure::kErrored; } + // TODO(crbug.com/tint/1503): Enable this once all the Dawn and CTS + // tests are updated to use the new format so we can avoid spamming + // the log files. + if ((false)) { + std::string warning = "stage should use @"; + switch (stage.value) { + case ast::PipelineStage::kVertex: + warning += "vertex"; + break; + case ast::PipelineStage::kFragment: + warning += "fragment"; + break; + case ast::PipelineStage::kCompute: + warning += "compute"; + break; + case ast::PipelineStage::kNone: + break; + } + deprecated(t.source(), warning); + } return create(t.source(), stage.value); }); } + if (t == kComputeStage) { + return create(t.source(), ast::PipelineStage::kCompute); + } + if (t == kVertexStage) { + return create(t.source(), ast::PipelineStage::kVertex); + } + if (t == kFragmentStage) { + return create(t.source(), ast::PipelineStage::kFragment); + } if (t == kSizeAttribute) { const char* use = "size attribute"; diff --git a/src/tint/reader/wgsl/parser_impl_function_attribute_list_test.cc b/src/tint/reader/wgsl/parser_impl_function_attribute_list_test.cc index 356241b209..83ebeac1c6 100644 --- a/src/tint/reader/wgsl/parser_impl_function_attribute_list_test.cc +++ b/src/tint/reader/wgsl/parser_impl_function_attribute_list_test.cc @@ -18,7 +18,7 @@ namespace tint::reader::wgsl { namespace { -TEST_F(ParserImplTest, AttributeList_Parses) { +TEST_F(ParserImplTest, AttributeList_Parses_Stage) { auto p = parser("@workgroup_size(2) @stage(compute)"); auto attrs = p->attribute_list(); EXPECT_FALSE(p->has_error()) << p->error(); @@ -44,6 +44,32 @@ TEST_F(ParserImplTest, AttributeList_Parses) { EXPECT_EQ(attr_1->As()->stage, ast::PipelineStage::kCompute); } +TEST_F(ParserImplTest, AttributeList_Parses) { + auto p = parser("@workgroup_size(2) @compute"); + auto attrs = p->attribute_list(); + EXPECT_FALSE(p->has_error()) << p->error(); + EXPECT_FALSE(attrs.errored); + EXPECT_TRUE(attrs.matched); + ASSERT_EQ(attrs.value.size(), 2u); + + auto* attr_0 = attrs.value[0]->As(); + auto* attr_1 = attrs.value[1]->As(); + ASSERT_NE(attr_0, nullptr); + ASSERT_NE(attr_1, nullptr); + + ASSERT_TRUE(attr_0->Is()); + const ast::Expression* x = attr_0->As()->x; + ASSERT_NE(x, nullptr); + auto* x_literal = x->As(); + ASSERT_NE(x_literal, nullptr); + ASSERT_TRUE(x_literal->Is()); + EXPECT_EQ(x_literal->As()->value, 2); + EXPECT_EQ(x_literal->As()->suffix, + ast::IntLiteralExpression::Suffix::kNone); + ASSERT_TRUE(attr_1->Is()); + EXPECT_EQ(attr_1->As()->stage, ast::PipelineStage::kCompute); +} + TEST_F(ParserImplTest, AttributeList_Invalid) { auto p = parser("@invalid"); auto attrs = p->attribute_list(); diff --git a/src/tint/reader/wgsl/parser_impl_function_attribute_test.cc b/src/tint/reader/wgsl/parser_impl_function_attribute_test.cc index d2585b3062..15be222a70 100644 --- a/src/tint/reader/wgsl/parser_impl_function_attribute_test.cc +++ b/src/tint/reader/wgsl/parser_impl_function_attribute_test.cc @@ -265,5 +265,44 @@ TEST_F(ParserImplTest, Attribute_Stage_MissingRightParen) { EXPECT_EQ(p->error(), "1:14: expected ')' for stage attribute"); } +TEST_F(ParserImplTest, Attribute_Compute) { + auto p = parser("compute"); + auto attr = p->attribute(); + EXPECT_TRUE(attr.matched); + EXPECT_FALSE(attr.errored); + ASSERT_NE(attr.value, nullptr) << p->error(); + ASSERT_FALSE(p->has_error()); + auto* func_attr = attr.value->As(); + ASSERT_NE(func_attr, nullptr); + ASSERT_TRUE(func_attr->Is()); + EXPECT_EQ(func_attr->As()->stage, ast::PipelineStage::kCompute); +} + +TEST_F(ParserImplTest, Attribute_Vertex) { + auto p = parser("vertex"); + auto attr = p->attribute(); + EXPECT_TRUE(attr.matched); + EXPECT_FALSE(attr.errored); + ASSERT_NE(attr.value, nullptr) << p->error(); + ASSERT_FALSE(p->has_error()); + auto* func_attr = attr.value->As(); + ASSERT_NE(func_attr, nullptr); + ASSERT_TRUE(func_attr->Is()); + EXPECT_EQ(func_attr->As()->stage, ast::PipelineStage::kVertex); +} + +TEST_F(ParserImplTest, Attribute_Fragment) { + auto p = parser("fragment"); + auto attr = p->attribute(); + EXPECT_TRUE(attr.matched); + EXPECT_FALSE(attr.errored); + ASSERT_NE(attr.value, nullptr) << p->error(); + ASSERT_FALSE(p->has_error()); + auto* func_attr = attr.value->As(); + ASSERT_NE(func_attr, nullptr); + ASSERT_TRUE(func_attr->Is()); + EXPECT_EQ(func_attr->As()->stage, ast::PipelineStage::kFragment); +} + } // namespace } // namespace tint::reader::wgsl diff --git a/src/tint/transform/canonicalize_entry_point_io.h b/src/tint/transform/canonicalize_entry_point_io.h index 64a10f2106..95f8b197df 100644 --- a/src/tint/transform/canonicalize_entry_point_io.h +++ b/src/tint/transform/canonicalize_entry_point_io.h @@ -34,7 +34,7 @@ namespace tint::transform { /// @location(2) loc2 : vec4; /// }; /// -/// @stage(fragment) +/// @fragment /// fn frag_main(@builtin(position) coord : vec4, /// locations : Locations) -> @location(0) f32 { /// if (coord.w > 1.0) { @@ -71,7 +71,7 @@ namespace tint::transform { /// return col; /// } /// -/// @stage(fragment) +/// @fragment /// fn frag_main(in : frag_main_in) -> frag_main_out { /// let inner_retval = frag_main_inner(in.coord, Locations(in.loc1, in.loc2)); /// var wrapper_result : frag_main_out; diff --git a/src/tint/transform/module_scope_var_to_entry_point_param.h b/src/tint/transform/module_scope_var_to_entry_point_param.h index f268197431..e3a50f4013 100644 --- a/src/tint/transform/module_scope_var_to_entry_point_param.h +++ b/src/tint/transform/module_scope_var_to_entry_point_param.h @@ -43,7 +43,7 @@ namespace tint::transform { /// p = p + f; /// } /// -/// @stage(compute) @workgroup_size(1) +/// @compute @workgroup_size(1) /// fn main() { /// foo(); /// } @@ -55,7 +55,7 @@ namespace tint::transform { /// *p = *p + (*sptr).f; /// } /// -/// @stage(compute) @workgroup_size(1) +/// @compute @workgroup_size(1) /// fn main(sptr : ptr) { /// var p : f32 = 2.0; /// foo(&p, sptr); diff --git a/test/tint/shader_io/compute_input_builtins.wgsl b/test/tint/shader_io/compute_input_builtins.wgsl index 77f209eea6..dfcd8bccb1 100644 --- a/test/tint/shader_io/compute_input_builtins.wgsl +++ b/test/tint/shader_io/compute_input_builtins.wgsl @@ -1,4 +1,4 @@ -@stage(compute) @workgroup_size(1) +@compute @workgroup_size(1) fn main( @builtin(local_invocation_id) local_invocation_id : vec3, @builtin(local_invocation_index) local_invocation_index : u32,