Validate multiply of invalid vector/matrix sizes Added tests that test all combos of vec*mat, mat*vec, and mat*mat for 2, 3, and 4 dimensions. Bug: tint:698 Change-Id: I4a407228261cf8ea2a93bc7077544e5a9244d854 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46660 Reviewed-by: Ben Clayton <bclayton@google.com> Kokoro: Kokoro <noreply+kokoro@google.com> Commit-Queue: Antonio Maiorano <amaiorano@google.com>
diff --git a/src/program_builder.h b/src/program_builder.h index 76c823d..613fb62 100644 --- a/src/program_builder.h +++ b/src/program_builder.h
@@ -928,7 +928,7 @@ /// @param rhs the right hand argument to the addition operation /// @returns a `ast::BinaryExpression` summing the arguments `lhs` and `rhs` template <typename LHS, typename RHS> - ast::Expression* Add(LHS&& lhs, RHS&& rhs) { + ast::BinaryExpression* Add(LHS&& lhs, RHS&& rhs) { return create<ast::BinaryExpression>(ast::BinaryOp::kAdd, Expr(std::forward<LHS>(lhs)), Expr(std::forward<RHS>(rhs))); @@ -938,7 +938,7 @@ /// @param rhs the right hand argument to the subtraction operation /// @returns a `ast::BinaryExpression` subtracting `rhs` from `lhs` template <typename LHS, typename RHS> - ast::Expression* Sub(LHS&& lhs, RHS&& rhs) { + ast::BinaryExpression* Sub(LHS&& lhs, RHS&& rhs) { return create<ast::BinaryExpression>(ast::BinaryOp::kSubtract, Expr(std::forward<LHS>(lhs)), Expr(std::forward<RHS>(rhs))); @@ -948,17 +948,28 @@ /// @param rhs the right hand argument to the multiplication operation /// @returns a `ast::BinaryExpression` multiplying `rhs` from `lhs` template <typename LHS, typename RHS> - ast::Expression* Mul(LHS&& lhs, RHS&& rhs) { + ast::BinaryExpression* Mul(LHS&& lhs, RHS&& rhs) { return create<ast::BinaryExpression>(ast::BinaryOp::kMultiply, Expr(std::forward<LHS>(lhs)), Expr(std::forward<RHS>(rhs))); } + /// @param source the source information + /// @param lhs the left hand argument to the multiplication operation + /// @param rhs the right hand argument to the multiplication operation + /// @returns a `ast::BinaryExpression` multiplying `rhs` from `lhs` + template <typename LHS, typename RHS> + ast::BinaryExpression* Mul(const Source& source, LHS&& lhs, RHS&& rhs) { + return create<ast::BinaryExpression>(source, ast::BinaryOp::kMultiply, + Expr(std::forward<LHS>(lhs)), + Expr(std::forward<RHS>(rhs))); + } + /// @param arr the array argument for the array accessor expression /// @param idx the index argument for the array accessor expression /// @returns a `ast::ArrayAccessorExpression` that indexes `arr` with `idx` template <typename ARR, typename IDX> - ast::Expression* IndexAccessor(ARR&& arr, IDX&& idx) { + ast::ArrayAccessorExpression* IndexAccessor(ARR&& arr, IDX&& idx) { return create<ast::ArrayAccessorExpression>(Expr(std::forward<ARR>(arr)), Expr(std::forward<IDX>(idx))); }
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc index 1158ad6..0c79dd7 100644 --- a/src/resolver/resolver.cc +++ b/src/resolver/resolver.cc
@@ -1040,8 +1040,10 @@ auto* rhs_vec_elem_type = rhs_vec ? rhs_vec->type()->UnwrapAliasIfNeeded() : nullptr; - const bool matching_vec_elem_types = lhs_vec_elem_type && rhs_vec_elem_type && - (lhs_vec_elem_type == rhs_vec_elem_type); + const bool matching_vec_elem_types = + lhs_vec_elem_type && rhs_vec_elem_type && + (lhs_vec_elem_type == rhs_vec_elem_type) && + (lhs_vec->size() == rhs_vec->size()); const bool matching_types = matching_vec_elem_types || (lhs_type == rhs_type); @@ -1106,19 +1108,22 @@ // Vector times matrix if (lhs_vec_elem_type && lhs_vec_elem_type->Is<F32>() && - rhs_mat_elem_type && rhs_mat_elem_type->Is<F32>()) { + rhs_mat_elem_type && rhs_mat_elem_type->Is<F32>() && + (lhs_vec->size() == rhs_mat->rows())) { return true; } // Matrix times vector if (lhs_mat_elem_type && lhs_mat_elem_type->Is<F32>() && - rhs_vec_elem_type && rhs_vec_elem_type->Is<F32>()) { + rhs_vec_elem_type && rhs_vec_elem_type->Is<F32>() && + (lhs_mat->columns() == rhs_vec->size())) { return true; } // Matrix times matrix if (lhs_mat_elem_type && lhs_mat_elem_type->Is<F32>() && - rhs_mat_elem_type && rhs_mat_elem_type->Is<F32>()) { + rhs_mat_elem_type && rhs_mat_elem_type->Is<F32>() && + (lhs_mat->columns() == rhs_mat->rows())) { return true; } }
diff --git a/src/resolver/resolver_test.cc b/src/resolver/resolver_test.cc index 12d019e..7ee58c6 100644 --- a/src/resolver/resolver_test.cc +++ b/src/resolver/resolver_test.cc
@@ -1263,11 +1263,11 @@ Global("lhs", lhs_type, ast::StorageClass::kNone); Global("rhs", rhs_type, ast::StorageClass::kNone); - auto* expr = create<ast::BinaryExpression>( - Source{Source::Location{12, 34}}, params.op, Expr("lhs"), Expr("rhs")); + auto* expr = create<ast::BinaryExpression>(Source{{12, 34}}, params.op, + Expr("lhs"), Expr("rhs")); WrapInFunction(expr); - ASSERT_FALSE(r()->Resolve()) << r()->error(); + ASSERT_FALSE(r()->Resolve()); ASSERT_EQ(r()->error(), "12:34 error: Binary expression operand types are invalid for " "this operation: " + @@ -1280,6 +1280,99 @@ Expr_Binary_Test_Invalid, testing::Combine(testing::ValuesIn(all_valid_cases), testing::ValuesIn(all_create_type_funcs))); + +using Expr_Binary_Test_Invalid_VectorMatrixMultiply = + ResolverTestWithParam<std::tuple<bool, uint32_t, uint32_t, uint32_t>>; +TEST_P(Expr_Binary_Test_Invalid_VectorMatrixMultiply, All) { + bool vec_by_mat = std::get<0>(GetParam()); + uint32_t vec_size = std::get<1>(GetParam()); + uint32_t mat_rows = std::get<2>(GetParam()); + uint32_t mat_cols = std::get<3>(GetParam()); + + type::Type* lhs_type; + type::Type* rhs_type; + type::Type* result_type; + bool is_valid_expr; + + if (vec_by_mat) { + lhs_type = create<type::Vector>(ty.f32(), vec_size); + rhs_type = create<type::Matrix>(ty.f32(), mat_rows, mat_cols); + result_type = create<type::Vector>(ty.f32(), mat_cols); + is_valid_expr = vec_size == mat_rows; + } else { + lhs_type = create<type::Matrix>(ty.f32(), mat_rows, mat_cols); + rhs_type = create<type::Vector>(ty.f32(), vec_size); + result_type = create<type::Vector>(ty.f32(), mat_rows); + is_valid_expr = vec_size == mat_cols; + } + + Global("lhs", lhs_type, ast::StorageClass::kNone); + Global("rhs", rhs_type, ast::StorageClass::kNone); + + auto* expr = Mul(Source{{12, 34}}, Expr("lhs"), Expr("rhs")); + WrapInFunction(expr); + + if (is_valid_expr) { + ASSERT_TRUE(r()->Resolve()) << r()->error(); + ASSERT_TRUE(TypeOf(expr) == result_type); + } else { + ASSERT_FALSE(r()->Resolve()); + ASSERT_EQ(r()->error(), + "12:34 error: Binary expression operand types are invalid for " + "this operation: " + + lhs_type->FriendlyName(Symbols()) + " " + + FriendlyName(expr->op()) + " " + + rhs_type->FriendlyName(Symbols())); + } +} +auto all_dimension_values = testing::Values(2u, 3u, 4u); +INSTANTIATE_TEST_SUITE_P(ResolverTest, + Expr_Binary_Test_Invalid_VectorMatrixMultiply, + testing::Combine(testing::Values(true, false), + all_dimension_values, + all_dimension_values, + all_dimension_values)); + +using Expr_Binary_Test_Invalid_MatrixMatrixMultiply = + ResolverTestWithParam<std::tuple<uint32_t, uint32_t, uint32_t, uint32_t>>; +TEST_P(Expr_Binary_Test_Invalid_MatrixMatrixMultiply, All) { + uint32_t lhs_mat_rows = std::get<0>(GetParam()); + uint32_t lhs_mat_cols = std::get<1>(GetParam()); + uint32_t rhs_mat_rows = std::get<2>(GetParam()); + uint32_t rhs_mat_cols = std::get<3>(GetParam()); + + auto* lhs_type = create<type::Matrix>(ty.f32(), lhs_mat_rows, lhs_mat_cols); + auto* rhs_type = create<type::Matrix>(ty.f32(), rhs_mat_rows, rhs_mat_cols); + auto* result_type = + create<type::Matrix>(ty.f32(), lhs_mat_rows, rhs_mat_cols); + + Global("lhs", lhs_type, ast::StorageClass::kNone); + Global("rhs", rhs_type, ast::StorageClass::kNone); + + auto* expr = Mul(Source{{12, 34}}, Expr("lhs"), Expr("rhs")); + WrapInFunction(expr); + + bool is_valid_expr = lhs_mat_cols == rhs_mat_rows; + if (is_valid_expr) { + ASSERT_TRUE(r()->Resolve()) << r()->error(); + ASSERT_TRUE(TypeOf(expr) == result_type); + } else { + ASSERT_FALSE(r()->Resolve()); + ASSERT_EQ(r()->error(), + "12:34 error: Binary expression operand types are invalid for " + "this operation: " + + lhs_type->FriendlyName(Symbols()) + " " + + FriendlyName(expr->op()) + " " + + rhs_type->FriendlyName(Symbols())); + } +} +INSTANTIATE_TEST_SUITE_P(ResolverTest, + Expr_Binary_Test_Invalid_MatrixMatrixMultiply, + testing::Combine(all_dimension_values, + all_dimension_values, + all_dimension_values, + all_dimension_values)); + } // namespace ExprBinaryTest using UnaryOpExpressionTest = ResolverTestWithParam<ast::UnaryOp>;