From 0e0979e89badb42f4ff9dc80b0ada2391ff1fc34 Mon Sep 17 00:00:00 2001 From: David Neto Date: Tue, 16 Jun 2020 23:47:05 +0000 Subject: [PATCH] [spirv-reader] Add ArrayStride Bug: tint:3 Change-Id: Ib174795d1b055b33bfc94205173dbc5a88bb92cb Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/23262 Reviewed-by: dan sinclair --- src/reader/spirv/parser_impl.cc | 44 +++++- src/reader/spirv/parser_impl.h | 7 + .../spirv/parser_impl_convert_type_test.cc | 132 +++++++++++++++++- 3 files changed, 175 insertions(+), 8 deletions(-) diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc index 870bd7998e..c90a620c50 100644 --- a/src/reader/spirv/parser_impl.cc +++ b/src/reader/spirv/parser_impl.cc @@ -589,18 +589,19 @@ ast::type::Type* ParserImpl::ConvertType( ast::type::Type* ParserImpl::ConvertType( const spvtools::opt::analysis::RuntimeArray* rtarr_ty) { - // TODO(dneto): Handle ArrayStride. Blocked by crbug.com/tint/30 auto* ast_elem_ty = ConvertType(type_mgr_->GetId(rtarr_ty->element_type())); if (ast_elem_ty == nullptr) { return nullptr; } - return ctx_.type_mgr().Get( - std::make_unique(ast_elem_ty)); + auto ast_type = std::make_unique(ast_elem_ty); + if (!ApplyArrayDecorations(rtarr_ty, ast_type.get())) { + return nullptr; + } + return ctx_.type_mgr().Get(std::move(ast_type)); } ast::type::Type* ParserImpl::ConvertType( const spvtools::opt::analysis::Array* arr_ty) { - // TODO(dneto): Handle ArrayStride. Blocked by crbug.com/tint/30 auto* ast_elem_ty = ConvertType(type_mgr_->GetId(arr_ty->element_type())); if (ast_elem_ty == nullptr) { return nullptr; @@ -632,8 +633,39 @@ ast::type::Type* ParserImpl::ConvertType( << num_elem; return nullptr; } - return ctx_.type_mgr().Get(std::make_unique( - ast_elem_ty, static_cast(num_elem))); + auto ast_type = std::make_unique( + ast_elem_ty, static_cast(num_elem)); + if (!ApplyArrayDecorations(arr_ty, ast_type.get())) { + return nullptr; + } + return ctx_.type_mgr().Get(std::move(ast_type)); +} + +bool ParserImpl::ApplyArrayDecorations( + const spvtools::opt::analysis::Type* spv_type, + ast::type::ArrayType* ast_type) { + const auto type_id = type_mgr_->GetId(spv_type); + for (auto& decoration : this->GetDecorationsFor(type_id)) { + if (decoration.size() == 2 && decoration[0] == SpvDecorationArrayStride) { + const auto stride = decoration[1]; + if (stride == 0) { + return Fail() << "invalid array type ID " << type_id + << ": ArrayStride can't be 0"; + } + if (ast_type->has_array_stride()) { + return Fail() << "invalid array type ID " << type_id + << ": multiple ArrayStride decorations"; + } + ast_type->set_array_stride(stride); + } else { + return Fail() << "invalid array type ID " << type_id + << ": unknown decoration " + << (decoration.empty() ? "(empty)" + : std::to_string(decoration[0])) + << " with " << decoration.size() << " total words"; + } + } + return true; } ast::type::Type* ParserImpl::ConvertType( diff --git a/src/reader/spirv/parser_impl.h b/src/reader/spirv/parser_impl.h index 4a4e70d3dc..b269455f3b 100644 --- a/src/reader/spirv/parser_impl.h +++ b/src/reader/spirv/parser_impl.h @@ -303,6 +303,13 @@ class ParserImpl : Reader { /// Converts a specific SPIR-V type to a Tint type. Pointer case ast::type::Type* ConvertType(const spvtools::opt::analysis::Pointer* ptr_ty); + /// Applies SPIR-V decorations to the given array or runtime-array type. + /// @param spv_type the SPIR-V aray or runtime-array type. + /// @param ast_type non-null; the AST type to apply decorations to + /// @returns true on success. + bool ApplyArrayDecorations(const spvtools::opt::analysis::Type* spv_type, + ast::type::ArrayType* ast_type); + // The SPIR-V binary we're parsing std::vector spv_binary_; diff --git a/src/reader/spirv/parser_impl_convert_type_test.cc b/src/reader/spirv/parser_impl_convert_type_test.cc index 84e80dbd8a..8ee2f1d012 100644 --- a/src/reader/spirv/parser_impl_convert_type_test.cc +++ b/src/reader/spirv/parser_impl_convert_type_test.cc @@ -342,13 +342,74 @@ TEST_F(SpvParserTest, ConvertType_RuntimeArray) { auto* arr_type = type->AsArray(); EXPECT_TRUE(arr_type->IsRuntimeArray()); ASSERT_NE(arr_type, nullptr); - ASSERT_EQ(arr_type->size(), 0u); + EXPECT_EQ(arr_type->size(), 0u); + EXPECT_EQ(arr_type->array_stride(), 0u); + EXPECT_FALSE(arr_type->has_array_stride()); auto* elem_type = arr_type->type(); ASSERT_NE(elem_type, nullptr); EXPECT_TRUE(elem_type->IsU32()); EXPECT_TRUE(p->error().empty()); } +TEST_F(SpvParserTest, ConvertType_RuntimeArray_InvalidDecoration) { + auto* p = parser(test::Assemble(R"( + OpDecorate %10 Block + %uint = OpTypeInt 32 0 + %10 = OpTypeRuntimeArray %uint + )")); + EXPECT_TRUE(p->BuildInternalModule()); + auto* type = p->ConvertType(10); + EXPECT_EQ(type, nullptr); + EXPECT_THAT( + p->error(), + Eq("invalid array type ID 10: unknown decoration 2 with 1 total words")); +} + +TEST_F(SpvParserTest, ConvertType_RuntimeArray_ArrayStride_Valid) { + auto* p = parser(test::Assemble(R"( + OpDecorate %10 ArrayStride 64 + %uint = OpTypeInt 32 0 + %10 = OpTypeRuntimeArray %uint + )")); + EXPECT_TRUE(p->BuildInternalModule()); + auto* type = p->ConvertType(10); + ASSERT_NE(type, nullptr); + auto* arr_type = type->AsArray(); + EXPECT_TRUE(arr_type->IsRuntimeArray()); + ASSERT_NE(arr_type, nullptr); + EXPECT_EQ(arr_type->array_stride(), 64u); + EXPECT_TRUE(arr_type->has_array_stride()); + EXPECT_TRUE(p->error().empty()); +} + +TEST_F(SpvParserTest, ConvertType_RuntimeArray_ArrayStride_ZeroIsError) { + auto* p = parser(test::Assemble(R"( + OpDecorate %10 ArrayStride 0 + %uint = OpTypeInt 32 0 + %10 = OpTypeRuntimeArray %uint + )")); + EXPECT_TRUE(p->BuildInternalModule()); + auto* type = p->ConvertType(10); + EXPECT_EQ(type, nullptr); + EXPECT_THAT(p->error(), + Eq("invalid array type ID 10: ArrayStride can't be 0")); +} + +TEST_F(SpvParserTest, + ConvertType_RuntimeArray_ArrayStride_SpecifiedTwiceIsError) { + auto* p = parser(test::Assemble(R"( + OpDecorate %10 ArrayStride 64 + OpDecorate %10 ArrayStride 64 + %uint = OpTypeInt 32 0 + %10 = OpTypeRuntimeArray %uint + )")); + EXPECT_TRUE(p->BuildInternalModule()); + auto* type = p->ConvertType(10); + EXPECT_EQ(type, nullptr); + EXPECT_THAT(p->error(), + Eq("invalid array type ID 10: multiple ArrayStride decorations")); +} + TEST_F(SpvParserTest, ConvertType_Array) { auto* p = parser(test::Assemble(R"( %uint = OpTypeInt 32 0 @@ -363,7 +424,9 @@ TEST_F(SpvParserTest, ConvertType_Array) { auto* arr_type = type->AsArray(); EXPECT_FALSE(arr_type->IsRuntimeArray()); ASSERT_NE(arr_type, nullptr); - ASSERT_EQ(arr_type->size(), 42u); + EXPECT_EQ(arr_type->size(), 42u); + EXPECT_EQ(arr_type->array_stride(), 0u); + EXPECT_FALSE(arr_type->has_array_stride()); auto* elem_type = arr_type->type(); ASSERT_NE(elem_type, nullptr); EXPECT_TRUE(elem_type->IsU32()); @@ -419,6 +482,71 @@ TEST_F(SpvParserTest, ConvertType_ArrayBadTooBig) { EXPECT_THAT(p->error(), Eq("unhandled integer width: 64")); } +TEST_F(SpvParserTest, ConvertType_Array_InvalidDecoration) { + auto* p = parser(test::Assemble(R"( + OpDecorate %10 Block + %uint = OpTypeInt 32 0 + %uint_5 = OpConstant %uint 5 + %10 = OpTypeArray %uint %uint_5 + )")); + EXPECT_TRUE(p->BuildInternalModule()); + auto* type = p->ConvertType(10); + EXPECT_EQ(type, nullptr); + EXPECT_THAT( + p->error(), + Eq("invalid array type ID 10: unknown decoration 2 with 1 total words")); +} + +TEST_F(SpvParserTest, ConvertType_ArrayStride_Valid) { + auto* p = parser(test::Assemble(R"( + OpDecorate %10 ArrayStride 8 + %uint = OpTypeInt 32 0 + %uint_5 = OpConstant %uint 5 + %10 = OpTypeArray %uint %uint_5 + )")); + EXPECT_TRUE(p->BuildInternalModule()); + + auto* type = p->ConvertType(10); + ASSERT_NE(type, nullptr); + EXPECT_TRUE(type->IsArray()); + auto* arr_type = type->AsArray(); + ASSERT_NE(arr_type, nullptr); + ASSERT_EQ(arr_type->array_stride(), 8u); + EXPECT_TRUE(arr_type->has_array_stride()); + EXPECT_TRUE(p->error().empty()); +} + +TEST_F(SpvParserTest, ConvertType_ArrayStride_ZeroIsError) { + auto* p = parser(test::Assemble(R"( + OpDecorate %10 ArrayStride 0 + %uint = OpTypeInt 32 0 + %uint_5 = OpConstant %uint 5 + %10 = OpTypeArray %uint %uint_5 + )")); + EXPECT_TRUE(p->BuildInternalModule()); + + auto* type = p->ConvertType(10); + ASSERT_EQ(type, nullptr); + EXPECT_THAT(p->error(), + Eq("invalid array type ID 10: ArrayStride can't be 0")); +} + +TEST_F(SpvParserTest, ConvertType_ArrayStride_SpecifiedTwiceIsError) { + auto* p = parser(test::Assemble(R"( + OpDecorate %10 ArrayStride 4 + OpDecorate %10 ArrayStride 4 + %uint = OpTypeInt 32 0 + %uint_5 = OpConstant %uint 5 + %10 = OpTypeArray %uint %uint_5 + )")); + EXPECT_TRUE(p->BuildInternalModule()); + + auto* type = p->ConvertType(10); + ASSERT_EQ(type, nullptr); + EXPECT_THAT(p->error(), + Eq("invalid array type ID 10: multiple ArrayStride decorations")); +} + TEST_F(SpvParserTest, ConvertType_StructTwoMembers) { auto* p = parser(test::Assemble(R"( %uint = OpTypeInt 32 0