From eb5d3e147d396ef2e0c3baa43913d16d8d795587 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Thu, 8 Oct 2020 19:34:25 +0000 Subject: [PATCH] [wgsl-reader] Allow array decorations to have multiple blocks. This CL updates the WGSL parser to allow array decorations to accept multiple blocks. The stride decoration on arrays was turned into a proper decoration object instead of just storing the stride directly. Bug: tint:240 Change-Id: I6cdc7400d8847e3e043b846ea5c9f86cb795cf86 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/29780 Commit-Queue: dan sinclair Reviewed-by: Sarah Mashayekhi Reviewed-by: David Neto --- BUILD.gn | 5 + src/CMakeLists.txt | 9 +- src/ast/array_decoration.cc | 38 ++++++++ src/ast/array_decoration.h | 51 ++++++++++ src/ast/stride_decoration.cc | 33 +++++++ src/ast/stride_decoration.h | 51 ++++++++++ src/ast/stride_decoration_test.cc | 37 ++++++++ src/ast/struct_decoration.h | 1 + src/ast/type/array_type.cc | 22 ++++- src/ast/type/array_type.h | 18 ++-- src/ast/type/array_type_test.cc | 6 +- src/reader/spirv/parser_impl.cc | 5 +- src/reader/wgsl/parser_impl.cc | 93 ++++++++++++------- src/reader/wgsl/parser_impl.h | 9 +- src/reader/wgsl/parser_impl_type_decl_test.cc | 39 ++++++++ src/transform/vertex_pulling_transform.cc | 6 +- .../generator_impl_member_accessor_test.cc | 37 ++++++-- src/writer/spirv/builder_type_test.cc | 7 +- src/writer/wgsl/generator_impl.cc | 7 +- src/writer/wgsl/generator_impl_type_test.cc | 22 ++++- 20 files changed, 431 insertions(+), 65 deletions(-) create mode 100644 src/ast/array_decoration.cc create mode 100644 src/ast/array_decoration.h create mode 100644 src/ast/stride_decoration.cc create mode 100644 src/ast/stride_decoration.h create mode 100644 src/ast/stride_decoration_test.cc diff --git a/BUILD.gn b/BUILD.gn index 591365013f..e2a82377e5 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -219,6 +219,8 @@ source_set("libtint_core_src") { sources = [ "src/ast/array_accessor_expression.cc", "src/ast/array_accessor_expression.h", + "src/ast/array_decoration.cc", + "src/ast/array_decoration.h", "src/ast/assignment_statement.cc", "src/ast/assignment_statement.h", "src/ast/binary_expression.cc", @@ -303,6 +305,8 @@ source_set("libtint_core_src") { "src/ast/statement.h", "src/ast/storage_class.cc", "src/ast/storage_class.h", + "src/ast/stride_decoration.cc", + "src/ast/stride_decoration.h", "src/ast/struct.cc", "src/ast/struct.h", "src/ast/struct_decoration.cc", @@ -727,6 +731,7 @@ source_set("tint_unittests_core_src") { "src/ast/set_decoration_test.cc", "src/ast/sint_literal_test.cc", "src/ast/stage_decoration_test.cc", + "src/ast/stride_decoration_test.cc", "src/ast/struct_member_offset_decoration_test.cc", "src/ast/struct_member_test.cc", "src/ast/struct_test.cc", diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index cb04a05523..c4b003f521 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -40,6 +40,8 @@ set(TINT_LIB_SRCS ../include/tint/tint.h ast/array_accessor_expression.cc ast/array_accessor_expression.h + ast/array_decoration.cc + ast/array_decoration.h ast/assignment_statement.cc ast/assignment_statement.h ast/binary_expression.cc @@ -124,10 +126,12 @@ set(TINT_LIB_SRCS ast/statement.h ast/storage_class.cc ast/storage_class.h - ast/struct_decoration.cc - ast/struct_decoration.h + ast/stride_decoration.cc + ast/stride_decoration.h ast/struct.cc ast/struct.h + ast/struct_decoration.cc + ast/struct_decoration.h ast/struct_member.cc ast/struct_member.h ast/struct_member_decoration.cc @@ -337,6 +341,7 @@ set(TINT_TEST_SRCS ast/set_decoration_test.cc ast/sint_literal_test.cc ast/stage_decoration_test.cc + ast/stride_decoration_test.cc ast/struct_member_test.cc ast/struct_member_offset_decoration_test.cc ast/struct_test.cc diff --git a/src/ast/array_decoration.cc b/src/ast/array_decoration.cc new file mode 100644 index 0000000000..8402439fbb --- /dev/null +++ b/src/ast/array_decoration.cc @@ -0,0 +1,38 @@ +// Copyright 2020 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/ast/array_decoration.h" + +#include + +#include "src/ast/stride_decoration.h" + +namespace tint { +namespace ast { + +ArrayDecoration::ArrayDecoration() = default; + +ArrayDecoration::~ArrayDecoration() = default; + +bool ArrayDecoration::IsStride() const { + return false; +} + +StrideDecoration* ArrayDecoration::AsStride() { + assert(IsStride()); + return static_cast(this); +} + +} // namespace ast +} // namespace tint diff --git a/src/ast/array_decoration.h b/src/ast/array_decoration.h new file mode 100644 index 0000000000..579668b0c7 --- /dev/null +++ b/src/ast/array_decoration.h @@ -0,0 +1,51 @@ +// Copyright 2020 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. + +#ifndef SRC_AST_ARRAY_DECORATION_H_ +#define SRC_AST_ARRAY_DECORATION_H_ + +#include +#include +#include + +namespace tint { +namespace ast { + +class StrideDecoration; + +/// A decoration attached to an array +class ArrayDecoration { + public: + virtual ~ArrayDecoration(); + + /// @returns true if this is a stride decoration + virtual bool IsStride() const; + + /// @returns the decoration as a stride decoration + StrideDecoration* AsStride(); + + /// @returns the decoration as a string + virtual std::string to_str() const = 0; + + protected: + ArrayDecoration(); +}; + +/// A list of unique array decorations +using ArrayDecorationList = std::vector>; + +} // namespace ast +} // namespace tint + +#endif // SRC_AST_ARRAY_DECORATION_H_ diff --git a/src/ast/stride_decoration.cc b/src/ast/stride_decoration.cc new file mode 100644 index 0000000000..a68cbcdc07 --- /dev/null +++ b/src/ast/stride_decoration.cc @@ -0,0 +1,33 @@ +// Copyright 2020 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/ast/stride_decoration.h" + +namespace tint { +namespace ast { + +StrideDecoration::StrideDecoration(uint32_t stride) : stride_(stride) {} + +bool StrideDecoration::IsStride() const { + return true; +} + +StrideDecoration::~StrideDecoration() = default; + +std::string StrideDecoration::to_str() const { + return "stride " + std::to_string(stride_); +} + +} // namespace ast +} // namespace tint diff --git a/src/ast/stride_decoration.h b/src/ast/stride_decoration.h new file mode 100644 index 0000000000..a8a9cb2eed --- /dev/null +++ b/src/ast/stride_decoration.h @@ -0,0 +1,51 @@ +// Copyright 2020 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. + +#ifndef SRC_AST_STRIDE_DECORATION_H_ +#define SRC_AST_STRIDE_DECORATION_H_ + +#include + +#include + +#include "src/ast/array_decoration.h" + +namespace tint { +namespace ast { + +/// A stride decoration +class StrideDecoration : public ArrayDecoration { + public: + /// constructor + /// @param stride the stride value + explicit StrideDecoration(uint32_t stride); + ~StrideDecoration() override; + + /// @returns true if this is a stride decoration + bool IsStride() const override; + + /// @returns the stride value + uint32_t stride() const { return stride_; } + + /// @returns the decoration as a string + std::string to_str() const override; + + private: + uint32_t stride_; +}; + +} // namespace ast +} // namespace tint + +#endif // SRC_AST_STRIDE_DECORATION_H_ diff --git a/src/ast/stride_decoration_test.cc b/src/ast/stride_decoration_test.cc new file mode 100644 index 0000000000..8209c3d4d3 --- /dev/null +++ b/src/ast/stride_decoration_test.cc @@ -0,0 +1,37 @@ +// Copyright 2020 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/ast/stride_decoration.h" + +#include "gtest/gtest.h" + +namespace tint { +namespace ast { +namespace { + +using StrideDecorationTest = testing::Test; + +TEST_F(StrideDecorationTest, Creation) { + StrideDecoration d{2}; + EXPECT_EQ(2u, d.stride()); +} + +TEST_F(StrideDecorationTest, Is) { + StrideDecoration d{2}; + EXPECT_TRUE(d.IsStride()); +} + +} // namespace +} // namespace ast +} // namespace tint diff --git a/src/ast/struct_decoration.h b/src/ast/struct_decoration.h index fecea8a25f..20139ac874 100644 --- a/src/ast/struct_decoration.h +++ b/src/ast/struct_decoration.h @@ -16,6 +16,7 @@ #define SRC_AST_STRUCT_DECORATION_H_ #include +#include namespace tint { namespace ast { diff --git a/src/ast/type/array_type.cc b/src/ast/type/array_type.cc index 683f6aa333..71b386138a 100644 --- a/src/ast/type/array_type.cc +++ b/src/ast/type/array_type.cc @@ -14,6 +14,8 @@ #include "src/ast/type/array_type.h" +#include "src/ast/stride_decoration.h" + namespace tint { namespace ast { namespace type { @@ -31,6 +33,24 @@ bool ArrayType::IsArray() const { return true; } +uint32_t ArrayType::array_stride() const { + for (const auto& deco : decos_) { + if (deco->IsStride()) { + return deco->AsStride()->stride(); + } + } + return 0; +} + +bool ArrayType::has_array_stride() const { + for (const auto& deco : decos_) { + if (deco->IsStride()) { + return true; + } + } + return false; +} + std::string ArrayType::type_name() const { assert(subtype_); @@ -38,7 +58,7 @@ std::string ArrayType::type_name() const { if (!IsRuntimeArray()) type_name += "_" + std::to_string(size_); if (has_array_stride()) - type_name += "_stride_" + std::to_string(array_stride_); + type_name += "_stride_" + std::to_string(array_stride()); return type_name; } diff --git a/src/ast/type/array_type.h b/src/ast/type/array_type.h index e89684c307..9a84d1e901 100644 --- a/src/ast/type/array_type.h +++ b/src/ast/type/array_type.h @@ -19,6 +19,7 @@ #include +#include "src/ast/array_decoration.h" #include "src/ast/type/type.h" namespace tint { @@ -45,13 +46,18 @@ class ArrayType : public Type { /// i.e. the size is determined at runtime bool IsRuntimeArray() const { return size_ == 0; } - /// Sets the array stride - /// @param stride the stride to set - void set_array_stride(uint32_t stride) { array_stride_ = stride; } + /// Sets the array decorations + /// @param decos the decorations to set + void set_decorations(ast::ArrayDecorationList decos) { + decos_ = std::move(decos); + } + /// @returns the array decorations + const ArrayDecorationList& decorations() const { return decos_; } + /// @returns the array stride or 0 if none set. - uint32_t array_stride() const { return array_stride_; } + uint32_t array_stride() const; /// @returns true if the array has a stride set - bool has_array_stride() const { return array_stride_ != 0; } + bool has_array_stride() const; /// @returns the array type Type* type() const { return subtype_; } @@ -64,7 +70,7 @@ class ArrayType : public Type { private: Type* subtype_ = nullptr; uint32_t size_ = 0; - uint32_t array_stride_ = 0; + ast::ArrayDecorationList decos_; }; } // namespace type diff --git a/src/ast/type/array_type_test.cc b/src/ast/type/array_type_test.cc index 478333084a..b76cb081ee 100644 --- a/src/ast/type/array_type_test.cc +++ b/src/ast/type/array_type_test.cc @@ -15,6 +15,7 @@ #include "src/ast/type/array_type.h" #include "gtest/gtest.h" +#include "src/ast/stride_decoration.h" #include "src/ast/type/i32_type.h" #include "src/ast/type/u32_type.h" @@ -75,8 +76,11 @@ TEST_F(ArrayTypeTest, TypeName_RuntimeArray) { TEST_F(ArrayTypeTest, TypeName_WithStride) { I32Type i32; + ArrayDecorationList decos; + decos.push_back(std::make_unique(16)); + ArrayType arr{&i32, 3}; - arr.set_array_stride(16); + arr.set_decorations(std::move(decos)); EXPECT_EQ(arr.type_name(), "__array__i32_3_stride_16"); } diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc index 92c6a29910..0861b2457d 100644 --- a/src/reader/spirv/parser_impl.cc +++ b/src/reader/spirv/parser_impl.cc @@ -45,6 +45,7 @@ #include "src/ast/scalar_constructor_expression.h" #include "src/ast/set_decoration.h" #include "src/ast/sint_literal.h" +#include "src/ast/stride_decoration.h" #include "src/ast/struct.h" #include "src/ast/struct_decoration.h" #include "src/ast/struct_member.h" @@ -772,7 +773,9 @@ bool ParserImpl::ApplyArrayDecorations( return Fail() << "invalid array type ID " << type_id << ": multiple ArrayStride decorations"; } - ast_type->set_array_stride(stride); + ast::ArrayDecorationList decos; + decos.push_back(std::make_unique(stride)); + ast_type->set_decorations(std::move(decos)); } else { return Fail() << "invalid array type ID " << type_id << ": unknown decoration " diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc index 032c14cfce..362219c77f 100644 --- a/src/reader/wgsl/parser_impl.cc +++ b/src/reader/wgsl/parser_impl.cc @@ -41,6 +41,7 @@ #include "src/ast/set_decoration.h" #include "src/ast/sint_literal.h" #include "src/ast/stage_decoration.h" +#include "src/ast/stride_decoration.h" #include "src/ast/struct_member_offset_decoration.h" #include "src/ast/switch_statement.h" #include "src/ast/type/alias_type.h" @@ -1105,9 +1106,9 @@ ast::type::AliasType* ParserImpl::type_alias() { // | VEC3 LESS_THAN type_decl GREATER_THAN // | VEC4 LESS_THAN type_decl GREATER_THAN // | PTR LESS_THAN storage_class, type_decl GREATER_THAN -// | array_decoration_list? ARRAY LESS_THAN type_decl COMMA +// | array_decoration_list* ARRAY LESS_THAN type_decl COMMA // INT_LITERAL GREATER_THAN -// | array_decoration_list? ARRAY LESS_THAN type_decl +// | array_decoration_list* ARRAY LESS_THAN type_decl // GREATER_THAN // | MAT2x2 LESS_THAN type_decl GREATER_THAN // | MAT2x3 LESS_THAN type_decl GREATER_THAN @@ -1153,19 +1154,28 @@ ast::type::Type* ParserImpl::type_decl() { return type_decl_pointer(t); } - auto deco = array_decoration_list(); + ast::ArrayDecorationList decos; + for (;;) { + size_t s = decos.size(); + if (!array_decoration_list(decos)) { + return nullptr; + } + if (decos.size() == s) { + break; + } + } if (has_error()) { return nullptr; } - if (deco != 0) { + if (!decos.empty()) { t = peek(); } - if (deco != 0 && !t.IsArray()) { + if (!decos.empty() && !t.IsArray()) { set_error(t, "found array decoration but no array"); return nullptr; } if (t.IsArray()) { - return type_decl_array(t, deco); + return type_decl_array(t, std::move(decos)); } if (t.IsMat2x2() || t.IsMat2x3() || t.IsMat2x4() || t.IsMat3x2() || t.IsMat3x3() || t.IsMat3x4() || t.IsMat4x2() || t.IsMat4x3() || @@ -1258,7 +1268,8 @@ ast::type::Type* ParserImpl::type_decl_vector(Token t) { std::make_unique(subtype, count)); } -ast::type::Type* ParserImpl::type_decl_array(Token t, uint32_t stride) { +ast::type::Type* ParserImpl::type_decl_array(Token t, + ast::ArrayDecorationList decos) { next(); // Consume the peek t = next(); @@ -1296,9 +1307,7 @@ ast::type::Type* ParserImpl::type_decl_array(Token t, uint32_t stride) { } auto ty = std::make_unique(subtype, size); - if (stride != 0) { - ty->set_array_stride(stride); - } + ty->set_decorations(std::move(decos)); return ctx_.type_mgr().Get(std::move(ty)); } @@ -1309,48 +1318,62 @@ ast::type::Type* ParserImpl::type_decl_array(Token t, uint32_t stride) { // // As there is currently only one decoration I'm combining these for now. // we can split apart later if needed. -uint32_t ParserImpl::array_decoration_list() { +bool ParserImpl::array_decoration_list(ast::ArrayDecorationList& decos) { auto t = peek(); if (!t.IsAttrLeft()) { - return 0; + return true; } t = peek(1); if (!t.IsStride()) { - return 0; + return true; } next(); // consume the peek of [[ - next(); // consume the peek of stride - t = next(); - if (!t.IsParenLeft()) { - set_error(t, "missing ( for stride attribute"); - return 0; - } + for (;;) { + t = next(); + if (!t.IsStride()) { + set_error(t, "unknown array decoration"); + return false; + } - t = next(); - if (!t.IsSintLiteral()) { - set_error(t, "missing value for stride decoration"); - return 0; - } - if (t.to_i32() < 0) { - set_error(t, "invalid stride value: " + t.to_str()); - return 0; - } - uint32_t stride = static_cast(t.to_i32()); + t = next(); + if (!t.IsParenLeft()) { + set_error(t, "missing ( for stride attribute"); + return false; + } - t = next(); - if (!t.IsParenRight()) { - set_error(t, "missing ) for stride attribute"); - return 0; + t = next(); + if (!t.IsSintLiteral()) { + set_error(t, "missing value for stride decoration"); + return false; + } + if (t.to_i32() < 0) { + set_error(t, "invalid stride value: " + t.to_str()); + return false; + } + uint32_t stride = static_cast(t.to_i32()); + decos.push_back(std::make_unique(stride)); + + t = next(); + if (!t.IsParenRight()) { + set_error(t, "missing ) for stride attribute"); + return false; + } + + t = peek(); + if (!t.IsComma()) { + break; + } + next(); // Consume the peek } t = next(); if (!t.IsAttrRight()) { set_error(t, "missing ]] for array decoration"); - return 0; + return false; } - return stride; + return true; } ast::type::Type* ParserImpl::type_decl_matrix(Token t) { diff --git a/src/reader/wgsl/parser_impl.h b/src/reader/wgsl/parser_impl.h index 2d53423ac7..b4a1bbca29 100644 --- a/src/reader/wgsl/parser_impl.h +++ b/src/reader/wgsl/parser_impl.h @@ -21,6 +21,7 @@ #include #include +#include "src/ast/array_decoration.h" #include "src/ast/assignment_statement.h" #include "src/ast/builtin.h" #include "src/ast/call_statement.h" @@ -172,8 +173,8 @@ class ParserImpl { std::unique_ptr struct_member(); /// Parses a `struct_member_decoration_decl` grammar element, appending newly /// parsed decorations to the end of |decos|. - /// @params decos the decoration list - /// @returns the list of decorations + /// @param decos the decoration list + /// @returns true if parsing was successful. bool struct_member_decoration_decl(ast::StructMemberDecorationList& decos); /// Parses a `struct_member_decoration` grammar element /// @returns the decoration or nullptr if none found @@ -395,8 +396,8 @@ class ParserImpl { private: ast::type::Type* type_decl_pointer(Token t); ast::type::Type* type_decl_vector(Token t); - ast::type::Type* type_decl_array(Token t, uint32_t stride); - uint32_t array_decoration_list(); + ast::type::Type* type_decl_array(Token t, ast::ArrayDecorationList decos); + bool array_decoration_list(ast::ArrayDecorationList& decos); ast::type::Type* type_decl_matrix(Token t); std::unique_ptr const_expr_internal( diff --git a/src/reader/wgsl/parser_impl_type_decl_test.cc b/src/reader/wgsl/parser_impl_type_decl_test.cc index b4c96aad36..f3848038e2 100644 --- a/src/reader/wgsl/parser_impl_type_decl_test.cc +++ b/src/reader/wgsl/parser_impl_type_decl_test.cc @@ -13,6 +13,7 @@ // limitations under the License. #include "gtest/gtest.h" +#include "src/ast/stride_decoration.h" #include "src/ast/type/alias_type.h" #include "src/ast/type/array_type.h" #include "src/ast/type/bool_type.h" @@ -423,6 +424,44 @@ TEST_F(ParserImplTest, TypeDecl_Array_Runtime_Stride) { EXPECT_EQ(a->array_stride(), 16u); } +TEST_F(ParserImplTest, TypeDecl_Array_MultipleDecorations_OneBlock) { + auto* p = parser("[[stride(16), stride(32)]] array"); + auto* t = p->type_decl(); + ASSERT_NE(t, nullptr) << p->error(); + ASSERT_FALSE(p->has_error()); + ASSERT_TRUE(t->IsArray()); + + auto* a = t->AsArray(); + ASSERT_TRUE(a->IsRuntimeArray()); + ASSERT_TRUE(a->type()->IsF32()); + + auto& decos = a->decorations(); + ASSERT_EQ(decos.size(), 2u); + EXPECT_TRUE(decos[0]->IsStride()); + EXPECT_EQ(decos[0]->AsStride()->stride(), 16u); + EXPECT_TRUE(decos[1]->IsStride()); + EXPECT_EQ(decos[1]->AsStride()->stride(), 32u); +} + +TEST_F(ParserImplTest, TypeDecl_Array_MultipleDecorations_MultipleBlocks) { + auto* p = parser("[[stride(16)]] [[stride(32)]] array"); + auto* t = p->type_decl(); + ASSERT_NE(t, nullptr) << p->error(); + ASSERT_FALSE(p->has_error()); + ASSERT_TRUE(t->IsArray()); + + auto* a = t->AsArray(); + ASSERT_TRUE(a->IsRuntimeArray()); + ASSERT_TRUE(a->type()->IsF32()); + + auto& decos = a->decorations(); + ASSERT_EQ(decos.size(), 2u); + EXPECT_TRUE(decos[0]->IsStride()); + EXPECT_EQ(decos[0]->AsStride()->stride(), 16u); + EXPECT_TRUE(decos[1]->IsStride()); + EXPECT_EQ(decos[1]->AsStride()->stride(), 32u); +} + TEST_F(ParserImplTest, TypeDecl_Array_Decoration_MissingArray) { auto* p = parser("[[stride(16)]] f32"); auto* t = p->type_decl(); diff --git a/src/transform/vertex_pulling_transform.cc b/src/transform/vertex_pulling_transform.cc index 5e4d7ce7ca..303a06253e 100644 --- a/src/transform/vertex_pulling_transform.cc +++ b/src/transform/vertex_pulling_transform.cc @@ -21,6 +21,7 @@ #include "src/ast/decorated_variable.h" #include "src/ast/member_accessor_expression.h" #include "src/ast/scalar_constructor_expression.h" +#include "src/ast/stride_decoration.h" #include "src/ast/struct.h" #include "src/ast/struct_decoration.h" #include "src/ast/struct_member.h" @@ -215,7 +216,10 @@ void VertexPullingTransform::AddVertexStorageBuffers() { // TODO(idanr): Make this readonly https://github.com/gpuweb/gpuweb/issues/935 // The array inside the struct definition auto internal_array = std::make_unique(GetU32Type()); - internal_array->set_array_stride(4u); + ast::ArrayDecorationList ary_decos; + ary_decos.push_back(std::make_unique(4u)); + internal_array->set_decorations(std::move(ary_decos)); + auto* internal_array_type = ctx_->type_mgr().Get(std::move(internal_array)); // Creating the struct type diff --git a/src/writer/hlsl/generator_impl_member_accessor_test.cc b/src/writer/hlsl/generator_impl_member_accessor_test.cc index 040804c116..a55bedd82b 100644 --- a/src/writer/hlsl/generator_impl_member_accessor_test.cc +++ b/src/writer/hlsl/generator_impl_member_accessor_test.cc @@ -24,6 +24,7 @@ #include "src/ast/module.h" #include "src/ast/scalar_constructor_expression.h" #include "src/ast/sint_literal.h" +#include "src/ast/stride_decoration.h" #include "src/ast/struct.h" #include "src/ast/struct_member.h" #include "src/ast/struct_member_offset_decoration.h" @@ -525,7 +526,9 @@ TEST_F(HlslGeneratorImplTest_MemberAccessor, ast::type::F32Type f32; ast::type::I32Type i32; ast::type::ArrayType ary(&i32, 5); - ary.set_array_stride(4); + ast::ArrayDecorationList decos; + decos.push_back(std::make_unique(4)); + ary.set_decorations(std::move(decos)); ast::StructMemberList members; ast::StructMemberDecorationList a_deco; @@ -573,7 +576,9 @@ TEST_F(HlslGeneratorImplTest_MemberAccessor, ast::type::F32Type f32; ast::type::I32Type i32; ast::type::ArrayType ary(&i32, 5); - ary.set_array_stride(4); + ast::ArrayDecorationList decos; + decos.push_back(std::make_unique(4)); + ary.set_decorations(std::move(decos)); ast::StructMemberList members; ast::StructMemberDecorationList a_deco; @@ -684,7 +689,9 @@ TEST_F(HlslGeneratorImplTest_MemberAccessor, ast::type::F32Type f32; ast::type::I32Type i32; ast::type::ArrayType ary(&i32, 5); - ary.set_array_stride(4); + ast::ArrayDecorationList decos; + decos.push_back(std::make_unique(4)); + ary.set_decorations(std::move(decos)); ast::StructMemberList members; ast::StructMemberDecorationList a_deco; @@ -937,7 +944,9 @@ TEST_F(HlslGeneratorImplTest_MemberAccessor, data.set_name("Data"); ast::type::ArrayType ary(&data, 4); - ary.set_array_stride(32); + ast::ArrayDecorationList decos; + decos.push_back(std::make_unique(32)); + ary.set_decorations(std::move(decos)); deco.push_back(std::make_unique(0)); members.push_back( @@ -1010,7 +1019,9 @@ TEST_F(HlslGeneratorImplTest_MemberAccessor, data.set_name("Data"); ast::type::ArrayType ary(&data, 4); - ary.set_array_stride(32); + ast::ArrayDecorationList decos; + decos.push_back(std::make_unique(32)); + ary.set_decorations(std::move(decos)); deco.push_back(std::make_unique(0)); members.push_back( @@ -1086,7 +1097,9 @@ TEST_F( data.set_name("Data"); ast::type::ArrayType ary(&data, 4); - ary.set_array_stride(32); + ast::ArrayDecorationList decos; + decos.push_back(std::make_unique(32)); + ary.set_decorations(std::move(decos)); deco.push_back(std::make_unique(0)); members.push_back( @@ -1161,7 +1174,9 @@ TEST_F(HlslGeneratorImplTest_MemberAccessor, data.set_name("Data"); ast::type::ArrayType ary(&data, 4); - ary.set_array_stride(32); + ast::ArrayDecorationList decos; + decos.push_back(std::make_unique(32)); + ary.set_decorations(std::move(decos)); deco.push_back(std::make_unique(0)); members.push_back( @@ -1237,7 +1252,9 @@ TEST_F(HlslGeneratorImplTest_MemberAccessor, data.set_name("Data"); ast::type::ArrayType ary(&data, 4); - ary.set_array_stride(32); + ast::ArrayDecorationList decos; + decos.push_back(std::make_unique(32)); + ary.set_decorations(std::move(decos)); deco.push_back(std::make_unique(0)); members.push_back( @@ -1329,7 +1346,9 @@ TEST_F(HlslGeneratorImplTest_MemberAccessor, data.set_name("Data"); ast::type::ArrayType ary(&data, 4); - ary.set_array_stride(32); + ast::ArrayDecorationList decos; + decos.push_back(std::make_unique(32)); + ary.set_decorations(std::move(decos)); deco.push_back(std::make_unique(0)); members.push_back( diff --git a/src/writer/spirv/builder_type_test.cc b/src/writer/spirv/builder_type_test.cc index 3a7e84d909..7c2ef16c8f 100644 --- a/src/writer/spirv/builder_type_test.cc +++ b/src/writer/spirv/builder_type_test.cc @@ -16,6 +16,7 @@ #include "gtest/gtest.h" #include "src/ast/identifier_expression.h" +#include "src/ast/stride_decoration.h" #include "src/ast/struct.h" #include "src/ast/struct_member.h" #include "src/ast/struct_member_offset_decoration.h" @@ -128,8 +129,12 @@ TEST_F(BuilderTest_Type, GenerateArray) { TEST_F(BuilderTest_Type, GenerateArray_WithStride) { ast::type::I32Type i32; + + ast::ArrayDecorationList decos; + decos.push_back(std::make_unique(16u)); + ast::type::ArrayType ary(&i32, 4); - ary.set_array_stride(16u); + ary.set_decorations(std::move(decos)); ast::Module mod; Builder b(&mod); diff --git a/src/writer/wgsl/generator_impl.cc b/src/writer/wgsl/generator_impl.cc index 187d06ec8e..ae92972ca3 100644 --- a/src/writer/wgsl/generator_impl.cc +++ b/src/writer/wgsl/generator_impl.cc @@ -46,6 +46,7 @@ #include "src/ast/sint_literal.h" #include "src/ast/stage_decoration.h" #include "src/ast/statement.h" +#include "src/ast/stride_decoration.h" #include "src/ast/struct.h" #include "src/ast/struct_member.h" #include "src/ast/struct_member_offset_decoration.h" @@ -395,8 +396,10 @@ bool GeneratorImpl::EmitType(ast::type::Type* type) { } else if (type->IsArray()) { auto* ary = type->AsArray(); - if (ary->has_array_stride()) { - out_ << "[[stride(" << ary->array_stride() << ")]] "; + for (const auto& deco : ary->decorations()) { + if (deco->IsStride()) { + out_ << "[[stride(" << deco->AsStride()->stride() << ")]] "; + } } out_ << "array<"; diff --git a/src/writer/wgsl/generator_impl_type_test.cc b/src/writer/wgsl/generator_impl_type_test.cc index d7360b34bc..456c478e84 100644 --- a/src/writer/wgsl/generator_impl_type_test.cc +++ b/src/writer/wgsl/generator_impl_type_test.cc @@ -13,6 +13,7 @@ // limitations under the License. #include "gtest/gtest.h" +#include "src/ast/stride_decoration.h" #include "src/ast/struct.h" #include "src/ast/struct_decoration.h" #include "src/ast/struct_member.h" @@ -60,16 +61,33 @@ TEST_F(WgslGeneratorImplTest, EmitType_Array) { EXPECT_EQ(g.result(), "array"); } -TEST_F(WgslGeneratorImplTest, EmitType_Array_WithStride) { +TEST_F(WgslGeneratorImplTest, EmitType_Array_Decoration) { ast::type::BoolType b; + ast::ArrayDecorationList decos; + decos.push_back(std::make_unique(16u)); + ast::type::ArrayType a(&b, 4); - a.set_array_stride(16); + a.set_decorations(std::move(decos)); GeneratorImpl g; ASSERT_TRUE(g.EmitType(&a)) << g.error(); EXPECT_EQ(g.result(), "[[stride(16)]] array"); } +TEST_F(WgslGeneratorImplTest, EmitType_Array_MultipleDecorations) { + ast::type::BoolType b; + ast::ArrayDecorationList decos; + decos.push_back(std::make_unique(16u)); + decos.push_back(std::make_unique(32u)); + + ast::type::ArrayType a(&b, 4); + a.set_decorations(std::move(decos)); + + GeneratorImpl g; + ASSERT_TRUE(g.EmitType(&a)) << g.error(); + EXPECT_EQ(g.result(), "[[stride(16)]] [[stride(32)]] array"); +} + TEST_F(WgslGeneratorImplTest, EmitType_RuntimeArray) { ast::type::BoolType b; ast::type::ArrayType a(&b);