From 6eb2a85adf5d70978d749f42208f5876ba5c3469 Mon Sep 17 00:00:00 2001 From: David Neto Date: Wed, 5 Aug 2020 21:16:59 +0000 Subject: [PATCH] [spirv-reader] Remove support for NumWorkgroups builtin variable It was removed from WGSL MVP https://github.com/gpuweb/gpuweb/issues/920 Bug: tint:3 Change-Id: I94a584feec88dda7e310ee5d7fa01e93e26cd31d Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/25526 Reviewed-by: Ryan Harrison --- src/ast/builtin.cc | 4 ---- src/ast/builtin.h | 1 - src/reader/spirv/enum_converter.cc | 2 -- src/reader/spirv/enum_converter_test.cc | 13 ++++++------- src/reader/wgsl/parser_impl.cc | 3 --- .../wgsl/parser_impl_variable_decoration_test.cc | 1 - src/writer/msl/generator_impl.cc | 4 ---- src/writer/msl/generator_impl_test.cc | 1 - src/writer/spirv/builder.cc | 2 -- src/writer/spirv/builder_global_variable_test.cc | 1 - 10 files changed, 6 insertions(+), 26 deletions(-) diff --git a/src/ast/builtin.cc b/src/ast/builtin.cc index e579f9cc40..b0844cf771 100644 --- a/src/ast/builtin.cc +++ b/src/ast/builtin.cc @@ -47,10 +47,6 @@ std::ostream& operator<<(std::ostream& out, Builtin builtin) { out << "frag_depth"; break; } - case Builtin::kNumWorkgroups: { - out << "num_workgroups"; - break; - } case Builtin::kWorkgroupSize: { out << "workgroup_size"; break; diff --git a/src/ast/builtin.h b/src/ast/builtin.h index 511203ac59..7fb3fe5af1 100644 --- a/src/ast/builtin.h +++ b/src/ast/builtin.h @@ -29,7 +29,6 @@ enum class Builtin { kFrontFacing, kFragCoord, kFragDepth, - kNumWorkgroups, kWorkgroupSize, kLocalInvocationId, kLocalInvocationIdx, diff --git a/src/reader/spirv/enum_converter.cc b/src/reader/spirv/enum_converter.cc index b52af7bdda..2b54362305 100644 --- a/src/reader/spirv/enum_converter.cc +++ b/src/reader/spirv/enum_converter.cc @@ -80,8 +80,6 @@ ast::Builtin EnumConverter::ToBuiltin(SpvBuiltIn b) { return ast::Builtin::kFragCoord; case SpvBuiltInFragDepth: return ast::Builtin::kFragDepth; - case SpvBuiltInNumWorkgroups: - return ast::Builtin::kNumWorkgroups; case SpvBuiltInWorkgroupSize: return ast::Builtin::kWorkgroupSize; case SpvBuiltInLocalInvocationId: diff --git a/src/reader/spirv/enum_converter_test.cc b/src/reader/spirv/enum_converter_test.cc index fc9b07646c..fb9c8d0f04 100644 --- a/src/reader/spirv/enum_converter_test.cc +++ b/src/reader/spirv/enum_converter_test.cc @@ -215,8 +215,6 @@ INSTANTIATE_TEST_SUITE_P( BuiltinCase{SpvBuiltInFrontFacing, true, ast::Builtin::kFrontFacing}, BuiltinCase{SpvBuiltInFragCoord, true, ast::Builtin::kFragCoord}, BuiltinCase{SpvBuiltInFragDepth, true, ast::Builtin::kFragDepth}, - BuiltinCase{SpvBuiltInNumWorkgroups, true, - ast::Builtin::kNumWorkgroups}, BuiltinCase{SpvBuiltInWorkgroupSize, true, ast::Builtin::kWorkgroupSize}, BuiltinCase{SpvBuiltInLocalInvocationId, true, @@ -226,11 +224,12 @@ INSTANTIATE_TEST_SUITE_P( BuiltinCase{SpvBuiltInGlobalInvocationId, true, ast::Builtin::kGlobalInvocationId})); -INSTANTIATE_TEST_SUITE_P(EnumConverterBad, - SpvBuiltinTest, - testing::Values(BuiltinCase{ - static_cast(9999), false, - ast::Builtin::kNone})); +INSTANTIATE_TEST_SUITE_P( + EnumConverterBad, + SpvBuiltinTest, + testing::Values( + BuiltinCase{static_cast(9999), false, ast::Builtin::kNone}, + BuiltinCase{SpvBuiltInNumWorkgroups, false, ast::Builtin::kNone})); } // namespace } // namespace spirv diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc index b3fca96678..66f14e845f 100644 --- a/src/reader/wgsl/parser_impl.cc +++ b/src/reader/wgsl/parser_impl.cc @@ -86,9 +86,6 @@ ast::Builtin ident_to_builtin(const std::string& str) { if (str == "frag_depth") { return ast::Builtin::kFragDepth; } - if (str == "num_workgroups") { - return ast::Builtin::kNumWorkgroups; - } if (str == "workgroup_size") { return ast::Builtin::kWorkgroupSize; } diff --git a/src/reader/wgsl/parser_impl_variable_decoration_test.cc b/src/reader/wgsl/parser_impl_variable_decoration_test.cc index dc1a5b0903..6775009cca 100644 --- a/src/reader/wgsl/parser_impl_variable_decoration_test.cc +++ b/src/reader/wgsl/parser_impl_variable_decoration_test.cc @@ -101,7 +101,6 @@ INSTANTIATE_TEST_SUITE_P( BuiltinData{"front_facing", ast::Builtin::kFrontFacing}, BuiltinData{"frag_coord", ast::Builtin::kFragCoord}, BuiltinData{"frag_depth", ast::Builtin::kFragDepth}, - BuiltinData{"num_workgroups", ast::Builtin::kNumWorkgroups}, BuiltinData{"workgroup_size", ast::Builtin::kWorkgroupSize}, BuiltinData{"local_invocation_id", ast::Builtin::kLocalInvocationId}, BuiltinData{"local_invocation_idx", ast::Builtin::kLocalInvocationIdx}, diff --git a/src/writer/msl/generator_impl.cc b/src/writer/msl/generator_impl.cc index 4329e2a0bd..11f29f35ef 100644 --- a/src/writer/msl/generator_impl.cc +++ b/src/writer/msl/generator_impl.cc @@ -1258,10 +1258,6 @@ std::string GeneratorImpl::builtin_to_attribute(ast::Builtin builtin) const { return "position"; case ast::Builtin::kFragDepth: return "depth(any)"; - // TODO(dsinclair): Ignore for now, I believe it will be removed from WGSL - // https://github.com/gpuweb/gpuweb/issues/920 - case ast::Builtin::kNumWorkgroups: - return ""; // TODO(dsinclair): Ignore for now. This has been removed as a builtin // in the spec. Need to update Tint to match. // https://github.com/gpuweb/gpuweb/pull/824 diff --git a/src/writer/msl/generator_impl_test.cc b/src/writer/msl/generator_impl_test.cc index 575264cfbd..922ca9dc52 100644 --- a/src/writer/msl/generator_impl_test.cc +++ b/src/writer/msl/generator_impl_test.cc @@ -116,7 +116,6 @@ INSTANTIATE_TEST_SUITE_P( MslBuiltinData{ast::Builtin::kFrontFacing, "front_facing"}, MslBuiltinData{ast::Builtin::kFragCoord, "position"}, MslBuiltinData{ast::Builtin::kFragDepth, "depth(any)"}, - MslBuiltinData{ast::Builtin::kNumWorkgroups, ""}, MslBuiltinData{ast::Builtin::kWorkgroupSize, ""}, MslBuiltinData{ast::Builtin::kLocalInvocationId, "thread_position_in_threadgroup"}, diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc index 8183daa9a4..745cb8af12 100644 --- a/src/writer/spirv/builder.cc +++ b/src/writer/spirv/builder.cc @@ -2086,8 +2086,6 @@ SpvBuiltIn Builder::ConvertBuiltin(ast::Builtin builtin) const { return SpvBuiltInFragCoord; case ast::Builtin::kFragDepth: return SpvBuiltInFragDepth; - case ast::Builtin::kNumWorkgroups: - return SpvBuiltInNumWorkgroups; case ast::Builtin::kWorkgroupSize: return SpvBuiltInWorkgroupSize; case ast::Builtin::kLocalInvocationId: diff --git a/src/writer/spirv/builder_global_variable_test.cc b/src/writer/spirv/builder_global_variable_test.cc index 8618f39843..703dc5d862 100644 --- a/src/writer/spirv/builder_global_variable_test.cc +++ b/src/writer/spirv/builder_global_variable_test.cc @@ -387,7 +387,6 @@ INSTANTIATE_TEST_SUITE_P( BuiltinData{ast::Builtin::kFrontFacing, SpvBuiltInFrontFacing}, BuiltinData{ast::Builtin::kFragCoord, SpvBuiltInFragCoord}, BuiltinData{ast::Builtin::kFragDepth, SpvBuiltInFragDepth}, - BuiltinData{ast::Builtin::kNumWorkgroups, SpvBuiltInNumWorkgroups}, BuiltinData{ast::Builtin::kWorkgroupSize, SpvBuiltInWorkgroupSize}, BuiltinData{ast::Builtin::kLocalInvocationId, SpvBuiltInLocalInvocationId},