From 8cd5c611fcc7d78c4871443128a1b18597443737 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Fri, 24 Feb 2023 21:02:40 +0000 Subject: [PATCH] tint/reader/wgsl: Remove element_count_expression() It wasn't actually called, as array is parsed like any other templated type. Fixed: tint:1850 Change-Id: I2f1c8b8990cf7f3a5f8b09316e4dc539dbe6535a Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/121520 Kokoro: Kokoro Reviewed-by: David Neto Commit-Queue: Ben Clayton --- src/tint/BUILD.gn | 1 - src/tint/CMakeLists.txt | 1 - src/tint/reader/wgsl/parser_impl.cc | 31 ------- src/tint/reader/wgsl/parser_impl.h | 3 - ...rser_impl_element_count_expression_test.cc | 81 ------------------- test/tint/array/size.wgsl | 7 +- test/tint/array/size.wgsl.expected.dxc.hlsl | 7 +- test/tint/array/size.wgsl.expected.fxc.hlsl | 7 +- test/tint/array/size.wgsl.expected.glsl | 7 +- test/tint/array/size.wgsl.expected.msl | 7 +- test/tint/array/size.wgsl.expected.spvasm | 16 ++-- test/tint/array/size.wgsl.expected.wgsl | 7 +- 12 files changed, 41 insertions(+), 134 deletions(-) delete mode 100644 src/tint/reader/wgsl/parser_impl_element_count_expression_test.cc diff --git a/src/tint/BUILD.gn b/src/tint/BUILD.gn index a93678f7fb..662f90d14f 100644 --- a/src/tint/BUILD.gn +++ b/src/tint/BUILD.gn @@ -1696,7 +1696,6 @@ if (tint_build_unittests) { "reader/wgsl/parser_impl_diagnostic_attribute_test.cc", "reader/wgsl/parser_impl_diagnostic_control_test.cc", "reader/wgsl/parser_impl_diagnostic_directive_test.cc", - "reader/wgsl/parser_impl_element_count_expression_test.cc", "reader/wgsl/parser_impl_enable_directive_test.cc", "reader/wgsl/parser_impl_error_msg_test.cc", "reader/wgsl/parser_impl_error_resync_test.cc", diff --git a/src/tint/CMakeLists.txt b/src/tint/CMakeLists.txt index a3835b7379..658d14f031 100644 --- a/src/tint/CMakeLists.txt +++ b/src/tint/CMakeLists.txt @@ -1076,7 +1076,6 @@ if(TINT_BUILD_TESTS) reader/wgsl/parser_impl_diagnostic_attribute_test.cc reader/wgsl/parser_impl_diagnostic_control_test.cc reader/wgsl/parser_impl_diagnostic_directive_test.cc - reader/wgsl/parser_impl_element_count_expression_test.cc reader/wgsl/parser_impl_enable_directive_test.cc reader/wgsl/parser_impl_error_msg_test.cc reader/wgsl/parser_impl_error_resync_test.cc diff --git a/src/tint/reader/wgsl/parser_impl.cc b/src/tint/reader/wgsl/parser_impl.cc index 13b47414c0..53d29f1016 100644 --- a/src/tint/reader/wgsl/parser_impl.cc +++ b/src/tint/reader/wgsl/parser_impl.cc @@ -2354,37 +2354,6 @@ Expect ParserImpl::expect_math_expression_post_unary_exp return expect_additive_expression_post_unary_expression(rhs.value); } -// element_count_expression -// : unary_expression math_expression.post.unary_expression -// | unary_expression bitwise_expression.post.unary_expression -// -// Note, this moves the `( multiplicative_operator unary_expression )* ( additive_operator -// unary_expression ( multiplicative_operator unary_expression )* )*` expression for the first -// branch out into helper methods. -Maybe ParserImpl::element_count_expression() { - auto lhs = unary_expression(); - if (lhs.errored) { - return Failure::kErrored; - } - if (!lhs.matched) { - return Failure::kNoMatch; - } - - auto bitwise = bitwise_expression_post_unary_expression(lhs.value); - if (bitwise.errored) { - return Failure::kErrored; - } - if (bitwise.matched) { - return bitwise.value; - } - - auto math = expect_math_expression_post_unary_expression(lhs.value); - if (math.errored) { - return Failure::kErrored; - } - return math.value; -} - // shift_expression // : unary_expression shift_expression.post.unary_expression Maybe ParserImpl::shift_expression() { diff --git a/src/tint/reader/wgsl/parser_impl.h b/src/tint/reader/wgsl/parser_impl.h index e26ffbd80d..bfc2f9a234 100644 --- a/src/tint/reader/wgsl/parser_impl.h +++ b/src/tint/reader/wgsl/parser_impl.h @@ -587,9 +587,6 @@ class ParserImpl { /// @returns the parsed expression or `lhs` if no match Expect expect_math_expression_post_unary_expression( const ast::Expression* lhs); - /// Parses an `element_count_expression` grammar element - /// @returns the parsed expression or nullptr - Maybe element_count_expression(); /// Parses a `unary_expression shift.post.unary_expression` /// @returns the parsed expression or nullptr Maybe shift_expression(); diff --git a/src/tint/reader/wgsl/parser_impl_element_count_expression_test.cc b/src/tint/reader/wgsl/parser_impl_element_count_expression_test.cc deleted file mode 100644 index b629ef7358..0000000000 --- a/src/tint/reader/wgsl/parser_impl_element_count_expression_test.cc +++ /dev/null @@ -1,81 +0,0 @@ -// Copyright 2022 The Tint Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "src/tint/reader/wgsl/parser_impl_test_helper.h" - -namespace tint::reader::wgsl { -namespace { - -TEST_F(ParserImplTest, ElementCountExpression_Math) { - auto p = parser("a * b"); - auto e = p->element_count_expression(); - EXPECT_TRUE(e.matched); - EXPECT_FALSE(e.errored); - EXPECT_FALSE(p->has_error()) << p->error(); - ASSERT_NE(e.value, nullptr); - - ASSERT_TRUE(e->Is()); - auto* rel = e->As(); - EXPECT_EQ(ast::BinaryOp::kMultiply, rel->op); - - ASSERT_TRUE(rel->lhs->Is()); - auto* ident_expr = rel->lhs->As(); - EXPECT_EQ(ident_expr->identifier->symbol, p->builder().Symbols().Get("a")); - - ASSERT_TRUE(rel->rhs->Is()); - ident_expr = rel->rhs->As(); - EXPECT_EQ(ident_expr->identifier->symbol, p->builder().Symbols().Get("b")); -} - -TEST_F(ParserImplTest, ElementCountExpression_Bitwise) { - auto p = parser("a | true"); - auto e = p->element_count_expression(); - EXPECT_TRUE(e.matched); - EXPECT_FALSE(e.errored); - EXPECT_FALSE(p->has_error()) << p->error(); - ASSERT_NE(e.value, nullptr); - - ASSERT_TRUE(e->Is()); - auto* rel = e->As(); - EXPECT_EQ(ast::BinaryOp::kOr, rel->op); - - ASSERT_TRUE(rel->lhs->Is()); - auto* ident_expr = rel->lhs->As(); - EXPECT_EQ(ident_expr->identifier->symbol, p->builder().Symbols().Get("a")); - - ASSERT_TRUE(rel->rhs->Is()); - ASSERT_TRUE(rel->rhs->As()->value); -} - -TEST_F(ParserImplTest, ElementCountExpression_NoMatch) { - auto p = parser("if (a) { }"); - auto e = p->element_count_expression(); - EXPECT_FALSE(e.matched); - EXPECT_FALSE(e.errored); - EXPECT_FALSE(p->has_error()) << p->error(); - ASSERT_EQ(e.value, nullptr); -} - -TEST_F(ParserImplTest, ElementCountExpression_InvalidRHS) { - auto p = parser("a * if"); - auto e = p->element_count_expression(); - EXPECT_FALSE(e.matched); - EXPECT_TRUE(e.errored); - EXPECT_TRUE(p->has_error()); - ASSERT_EQ(e.value, nullptr); - EXPECT_EQ("1:5: unable to parse right side of * expression", p->error()); -} - -} // namespace -} // namespace tint::reader::wgsl diff --git a/test/tint/array/size.wgsl b/test/tint/array/size.wgsl index 4b2ec371a8..4a7c5ee95f 100644 --- a/test/tint/array/size.wgsl +++ b/test/tint/array/size.wgsl @@ -7,8 +7,11 @@ fn main() { var unsigned_literal : array; var signed_constant : array; var unsigned_constant : array; + var shr_const_expr : array; // Ensure that the types are compatible. - signed_literal = unsigned_constant; - signed_constant = unsigned_literal; + unsigned_literal = signed_literal; + signed_constant = signed_literal; + unsigned_constant = signed_literal; + shr_const_expr = signed_literal; } diff --git a/test/tint/array/size.wgsl.expected.dxc.hlsl b/test/tint/array/size.wgsl.expected.dxc.hlsl index 4753dde800..34f544895a 100644 --- a/test/tint/array/size.wgsl.expected.dxc.hlsl +++ b/test/tint/array/size.wgsl.expected.dxc.hlsl @@ -3,7 +3,10 @@ void main() { float unsigned_literal[4] = (float[4])0; float signed_constant[4] = (float[4])0; float unsigned_constant[4] = (float[4])0; - signed_literal = unsigned_constant; - signed_constant = unsigned_literal; + float shr_const_expr[4] = (float[4])0; + unsigned_literal = signed_literal; + signed_constant = signed_literal; + unsigned_constant = signed_literal; + shr_const_expr = signed_literal; return; } diff --git a/test/tint/array/size.wgsl.expected.fxc.hlsl b/test/tint/array/size.wgsl.expected.fxc.hlsl index 4753dde800..34f544895a 100644 --- a/test/tint/array/size.wgsl.expected.fxc.hlsl +++ b/test/tint/array/size.wgsl.expected.fxc.hlsl @@ -3,7 +3,10 @@ void main() { float unsigned_literal[4] = (float[4])0; float signed_constant[4] = (float[4])0; float unsigned_constant[4] = (float[4])0; - signed_literal = unsigned_constant; - signed_constant = unsigned_literal; + float shr_const_expr[4] = (float[4])0; + unsigned_literal = signed_literal; + signed_constant = signed_literal; + unsigned_constant = signed_literal; + shr_const_expr = signed_literal; return; } diff --git a/test/tint/array/size.wgsl.expected.glsl b/test/tint/array/size.wgsl.expected.glsl index 132511d617..10d6d6c6b6 100644 --- a/test/tint/array/size.wgsl.expected.glsl +++ b/test/tint/array/size.wgsl.expected.glsl @@ -6,8 +6,11 @@ void tint_symbol() { float unsigned_literal[4] = float[4](0.0f, 0.0f, 0.0f, 0.0f); float signed_constant[4] = float[4](0.0f, 0.0f, 0.0f, 0.0f); float unsigned_constant[4] = float[4](0.0f, 0.0f, 0.0f, 0.0f); - signed_literal = unsigned_constant; - signed_constant = unsigned_literal; + float shr_const_expr[4] = float[4](0.0f, 0.0f, 0.0f, 0.0f); + unsigned_literal = signed_literal; + signed_constant = signed_literal; + unsigned_constant = signed_literal; + shr_const_expr = signed_literal; } void main() { diff --git a/test/tint/array/size.wgsl.expected.msl b/test/tint/array/size.wgsl.expected.msl index f72c5aa9ba..f3277f26c8 100644 --- a/test/tint/array/size.wgsl.expected.msl +++ b/test/tint/array/size.wgsl.expected.msl @@ -19,8 +19,11 @@ fragment void tint_symbol() { tint_array unsigned_literal = {}; tint_array signed_constant = {}; tint_array unsigned_constant = {}; - signed_literal = unsigned_constant; - signed_constant = unsigned_literal; + tint_array shr_const_expr = {}; + unsigned_literal = signed_literal; + signed_constant = signed_literal; + unsigned_constant = signed_literal; + shr_const_expr = signed_literal; return; } diff --git a/test/tint/array/size.wgsl.expected.spvasm b/test/tint/array/size.wgsl.expected.spvasm index ac1a0c93cb..2bc675d734 100644 --- a/test/tint/array/size.wgsl.expected.spvasm +++ b/test/tint/array/size.wgsl.expected.spvasm @@ -1,7 +1,7 @@ ; SPIR-V ; Version: 1.3 ; Generator: Google Tint Compiler; 0 -; Bound: 17 +; Bound: 20 ; Schema: 0 OpCapability Shader OpMemoryModel Logical GLSL450 @@ -12,6 +12,7 @@ OpName %unsigned_literal "unsigned_literal" OpName %signed_constant "signed_constant" OpName %unsigned_constant "unsigned_constant" + OpName %shr_const_expr "shr_const_expr" OpDecorate %_arr_float_uint_4 ArrayStride 4 %void = OpTypeVoid %1 = OpTypeFunction %void @@ -27,9 +28,14 @@ %unsigned_literal = OpVariable %_ptr_Function__arr_float_uint_4 Function %11 %signed_constant = OpVariable %_ptr_Function__arr_float_uint_4 Function %11 %unsigned_constant = OpVariable %_ptr_Function__arr_float_uint_4 Function %11 - %15 = OpLoad %_arr_float_uint_4 %unsigned_constant - OpStore %signed_literal %15 - %16 = OpLoad %_arr_float_uint_4 %unsigned_literal - OpStore %signed_constant %16 +%shr_const_expr = OpVariable %_ptr_Function__arr_float_uint_4 Function %11 + %16 = OpLoad %_arr_float_uint_4 %signed_literal + OpStore %unsigned_literal %16 + %17 = OpLoad %_arr_float_uint_4 %signed_literal + OpStore %signed_constant %17 + %18 = OpLoad %_arr_float_uint_4 %signed_literal + OpStore %unsigned_constant %18 + %19 = OpLoad %_arr_float_uint_4 %signed_literal + OpStore %shr_const_expr %19 OpReturn OpFunctionEnd diff --git a/test/tint/array/size.wgsl.expected.wgsl b/test/tint/array/size.wgsl.expected.wgsl index a82c3773b9..f9eee82ec4 100644 --- a/test/tint/array/size.wgsl.expected.wgsl +++ b/test/tint/array/size.wgsl.expected.wgsl @@ -8,6 +8,9 @@ fn main() { var unsigned_literal : array; var signed_constant : array; var unsigned_constant : array; - signed_literal = unsigned_constant; - signed_constant = unsigned_literal; + var shr_const_expr : array; + unsigned_literal = signed_literal; + signed_constant = signed_literal; + unsigned_constant = signed_literal; + shr_const_expr = signed_literal; }