From cb41b8f97f12341af2c5667b3c059767fa14b2de Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Mon, 26 Sep 2022 15:54:55 +0000 Subject: [PATCH] Convert `align` attribute to expressions. This CL updates the align attribute to parse expressions. Bug: tint:1633 Change-Id: I5412180ff62e6b286b35ea3297e2b4f136899960 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/102841 Reviewed-by: Ben Clayton Kokoro: Kokoro Commit-Queue: Dan Sinclair --- src/tint/inspector/inspector_test.cc | 4 +- src/tint/reader/wgsl/parser_impl.cc | 11 +-- .../reader/wgsl/parser_impl_error_msg_test.cc | 14 +-- .../wgsl/parser_impl_struct_body_decl_test.cc | 4 +- ...arser_impl_struct_member_attribute_test.cc | 37 ++++++-- .../resolver/attribute_validation_test.cc | 89 ++++++++++++++++++- src/tint/resolver/dependency_graph.cc | 10 ++- src/tint/resolver/dependency_graph_test.cc | 5 +- src/tint/resolver/resolver.cc | 9 +- .../storage_class_layout_validation_test.cc | 20 ++--- src/tint/resolver/struct_layout_test.cc | 14 +-- src/tint/resolver/validation_test.cc | 23 +++-- src/tint/transform/std140.cc | 2 +- src/tint/transform/std140_test.cc | 4 +- .../generator_impl_storage_buffer_test.cc | 6 +- .../writer/msl/generator_impl_type_test.cc | 8 +- src/tint/writer/spirv/builder_type_test.cc | 2 +- 17 files changed, 194 insertions(+), 68 deletions(-) diff --git a/src/tint/inspector/inspector_test.cc b/src/tint/inspector/inspector_test.cc index 5a190f6080..b892302749 100644 --- a/src/tint/inspector/inspector_test.cc +++ b/src/tint/inspector/inspector_test.cc @@ -1811,7 +1811,7 @@ TEST_F(InspectorGetUniformBufferResourceBindingsTest, ContainingArray) { "foo_type", utils::Vector{ Member("0i32", ty.i32()), - Member("b", ty.array(ty.u32(), 4_u, /*stride*/ 16), utils::Vector{MemberAlign(16_u)}), + Member("b", ty.array(ty.u32(), 4_u, /*stride*/ 16), utils::Vector{MemberAlign(16_i)}), }); AddUniformBuffer("foo_ub", ty.Of(foo_struct_type), 0, 0); @@ -3239,7 +3239,7 @@ TEST_F(InspectorGetWorkgroupStorageSizeTest, StructAlignment) { // here the struct is expected to occupy 1024 bytes of workgroup storage. const auto* wg_struct_type = MakeStructTypeFromMembers( "WgStruct", utils::Vector{ - MakeStructMember(0, ty.f32(), utils::Vector{MemberAlign(1024_u)}), + MakeStructMember(0, ty.f32(), utils::Vector{MemberAlign(1024_i)}), }); AddWorkgroupStorage("wg_struct_var", ty.Of(wg_struct_type)); diff --git a/src/tint/reader/wgsl/parser_impl.cc b/src/tint/reader/wgsl/parser_impl.cc index 2c72d6a0c3..cb7cb6a120 100644 --- a/src/tint/reader/wgsl/parser_impl.cc +++ b/src/tint/reader/wgsl/parser_impl.cc @@ -3435,15 +3435,16 @@ Maybe ParserImpl::attribute() { if (t == "align") { const char* use = "align attribute"; return expect_paren_block(use, [&]() -> Result { - auto val = expect_positive_sint(use); - if (val.errored) { + auto expr = expression(); + if (expr.errored) { return Failure::kErrored; } + if (!expr.matched) { + return add_error(peek(), "expected align expression"); + } match(Token::Type::kComma); - return create( - t.source(), create( - val.value, ast::IntLiteralExpression::Suffix::kNone)); + return create(t.source(), expr.value); }); } diff --git a/src/tint/reader/wgsl/parser_impl_error_msg_test.cc b/src/tint/reader/wgsl/parser_impl_error_msg_test.cc index ac2161c1ad..9fad55c008 100644 --- a/src/tint/reader/wgsl/parser_impl_error_msg_test.cc +++ b/src/tint/reader/wgsl/parser_impl_error_msg_test.cc @@ -851,17 +851,9 @@ struct S { 1 : i32, }; } TEST_F(ParserImplErrorTest, GlobalDeclStructMemberAlignInvaldValue) { - EXPECT("struct S { @align(x) i : i32, };", - R"(test.wgsl:1:19 error: expected signed integer literal for align attribute -struct S { @align(x) i : i32, }; - ^ -)"); -} - -TEST_F(ParserImplErrorTest, GlobalDeclStructMemberAlignNegativeValue) { - EXPECT("struct S { @align(-2) i : i32, };", - R"(test.wgsl:1:19 error: align attribute must be positive -struct S { @align(-2) i : i32, }; + EXPECT("struct S { @align(fn) i : i32, };", + R"(test.wgsl:1:19 error: expected align expression +struct S { @align(fn) i : i32, }; ^^ )"); } diff --git a/src/tint/reader/wgsl/parser_impl_struct_body_decl_test.cc b/src/tint/reader/wgsl/parser_impl_struct_body_decl_test.cc index 3499b5cb00..ce56a676c9 100644 --- a/src/tint/reader/wgsl/parser_impl_struct_body_decl_test.cc +++ b/src/tint/reader/wgsl/parser_impl_struct_body_decl_test.cc @@ -60,12 +60,12 @@ TEST_F(ParserImplTest, StructBodyDecl_ParsesEmpty) { TEST_F(ParserImplTest, StructBodyDecl_InvalidAlign) { auto p = parser(R"( { - @align(nan) a : i32, + @align(if) a : i32, })"); auto m = p->expect_struct_body_decl(); ASSERT_TRUE(p->has_error()); ASSERT_TRUE(m.errored); - EXPECT_EQ(p->error(), "3:10: expected signed integer literal for align attribute"); + EXPECT_EQ(p->error(), "3:10: expected align expression"); } TEST_F(ParserImplTest, StructBodyDecl_InvalidSize) { diff --git a/src/tint/reader/wgsl/parser_impl_struct_member_attribute_test.cc b/src/tint/reader/wgsl/parser_impl_struct_member_attribute_test.cc index 9bafaf0f44..f6957e4c66 100644 --- a/src/tint/reader/wgsl/parser_impl_struct_member_attribute_test.cc +++ b/src/tint/reader/wgsl/parser_impl_struct_member_attribute_test.cc @@ -110,13 +110,39 @@ TEST_F(ParserImplTest, Attribute_Align) { ast::IntLiteralExpression::Suffix::kNone); } +TEST_F(ParserImplTest, Attribute_Align_Expression) { + auto p = parser("align(4 + 5)"); + auto attr = p->attribute(); + EXPECT_TRUE(attr.matched); + EXPECT_FALSE(attr.errored); + ASSERT_FALSE(p->has_error()) << p->error(); + ASSERT_NE(attr.value, nullptr); + + auto* member_attr = attr.value->As(); + ASSERT_NE(member_attr, nullptr); + ASSERT_TRUE(member_attr->Is()); + + auto* o = member_attr->As(); + ASSERT_TRUE(o->expr->Is()); + auto* expr = o->expr->As(); + + EXPECT_EQ(ast::BinaryOp::kAdd, expr->op); + auto* v = expr->lhs->As(); + ASSERT_NE(nullptr, v); + EXPECT_EQ(v->value, 4u); + + v = expr->rhs->As(); + ASSERT_NE(nullptr, v); + EXPECT_EQ(v->value, 5u); +} + TEST_F(ParserImplTest, Attribute_Align_TrailingComma) { auto p = parser("align(4,)"); auto attr = p->attribute(); EXPECT_TRUE(attr.matched); EXPECT_FALSE(attr.errored); ASSERT_NE(attr.value, nullptr); - ASSERT_FALSE(p->has_error()); + ASSERT_FALSE(p->has_error()) << p->error(); auto* member_attr = attr.value->As(); ASSERT_NE(member_attr, nullptr); @@ -157,17 +183,18 @@ TEST_F(ParserImplTest, Attribute_Align_MissingValue) { EXPECT_TRUE(attr.errored); EXPECT_EQ(attr.value, nullptr); EXPECT_TRUE(p->has_error()); - EXPECT_EQ(p->error(), "1:7: expected signed integer literal for align attribute"); + EXPECT_EQ(p->error(), "1:7: expected align expression"); } -TEST_F(ParserImplTest, Attribute_Align_MissingInvalid) { - auto p = parser("align(nan)"); +TEST_F(ParserImplTest, Attribute_Align_ExpressionInvalid) { + auto p = parser("align(4 + 5 << 6)"); auto attr = p->attribute(); EXPECT_FALSE(attr.matched); EXPECT_TRUE(attr.errored); EXPECT_EQ(attr.value, nullptr); EXPECT_TRUE(p->has_error()); - EXPECT_EQ(p->error(), "1:7: expected signed integer literal for align attribute"); + + EXPECT_EQ(p->error(), "1:13: expected ')' for align attribute"); } } // namespace diff --git a/src/tint/resolver/attribute_validation_test.cc b/src/tint/resolver/attribute_validation_test.cc index e648562012..8d2067381f 100644 --- a/src/tint/resolver/attribute_validation_test.cc +++ b/src/tint/resolver/attribute_validation_test.cc @@ -89,7 +89,7 @@ static utils::Vector createAttributes(const Source& so AttributeKind kind) { switch (kind) { case AttributeKind::kAlign: - return {builder.MemberAlign(source, 4_u)}; + return {builder.MemberAlign(source, 4_i)}; case AttributeKind::kBinding: return {builder.Binding(source, 1_a)}; case AttributeKind::kBuiltin: @@ -627,8 +627,8 @@ TEST_F(StructMemberAttributeTest, DuplicateAttribute) { Structure("mystruct", utils::Vector{ Member("a", ty.i32(), utils::Vector{ - MemberAlign(Source{{12, 34}}, 4_u), - MemberAlign(Source{{56, 78}}, 8_u), + MemberAlign(Source{{12, 34}}, 4_i), + MemberAlign(Source{{56, 78}}, 8_i), }), }); EXPECT_FALSE(r()->Resolve()); @@ -659,6 +659,89 @@ TEST_F(StructMemberAttributeTest, InvariantAttributeWithoutPosition) { "position builtin"); } +TEST_F(StructMemberAttributeTest, Align_Attribute_Const) { + GlobalConst("val", ty.i32(), Expr(1_i)); + + Structure("mystruct", utils::Vector{Member("a", ty.f32(), utils::Vector{MemberAlign("val")})}); + EXPECT_TRUE(r()->Resolve()) << r()->error(); +} + +TEST_F(StructMemberAttributeTest, Align_Attribute_ConstNegative) { + GlobalConst("val", ty.i32(), Expr(-2_i)); + + Structure("mystruct", utils::Vector{Member( + "a", ty.f32(), utils::Vector{MemberAlign(Source{{12, 34}}, "val")})}); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + R"(12:34 error: 'align' value must be a positive, power-of-two integer)"); +} + +TEST_F(StructMemberAttributeTest, Align_Attribute_ConstPowerOfTwo) { + GlobalConst("val", ty.i32(), Expr(3_i)); + + Structure("mystruct", utils::Vector{Member( + "a", ty.f32(), utils::Vector{MemberAlign(Source{{12, 34}}, "val")})}); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + R"(12:34 error: 'align' value must be a positive, power-of-two integer)"); +} + +TEST_F(StructMemberAttributeTest, Align_Attribute_ConstF32) { + GlobalConst("val", ty.f32(), Expr(1.23_f)); + + Structure("mystruct", utils::Vector{Member( + "a", ty.f32(), utils::Vector{MemberAlign(Source{{12, 34}}, "val")})}); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), R"(12:34 error: 'align' must be an i32 value)"); +} + +TEST_F(StructMemberAttributeTest, Align_Attribute_ConstU32) { + GlobalConst("val", ty.u32(), Expr(2_u)); + + Structure("mystruct", utils::Vector{Member( + "a", ty.f32(), utils::Vector{MemberAlign(Source{{12, 34}}, "val")})}); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), R"(12:34 error: 'align' must be an i32 value)"); +} + +TEST_F(StructMemberAttributeTest, Align_Attribute_ConstAInt) { + GlobalConst("val", Expr(2_a)); + + Structure("mystruct", utils::Vector{Member( + "a", ty.f32(), utils::Vector{MemberAlign(Source{{12, 34}}, "val")})}); + EXPECT_TRUE(r()->Resolve()) << r()->error(); +} + +TEST_F(StructMemberAttributeTest, Align_Attribute_ConstAFloat) { + GlobalConst("val", Expr(2.0_a)); + + Structure("mystruct", utils::Vector{Member( + "a", ty.f32(), utils::Vector{MemberAlign(Source{{12, 34}}, "val")})}); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), R"(12:34 error: 'align' must be an i32 value)"); +} + +TEST_F(StructMemberAttributeTest, Align_Attribute_Var) { + GlobalVar(Source{{1, 2}}, "val", ty.f32(), ast::StorageClass::kPrivate, ast::Access::kUndefined, + Expr(1.23_f)); + + Structure(Source{{6, 4}}, "mystruct", + utils::Vector{Member(Source{{12, 5}}, "a", ty.f32(), + utils::Vector{MemberAlign(Expr(Source{{12, 35}}, "val"))})}); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), R"(12:35 error: var 'val' cannot be referenced at module-scope +1:2 note: var 'val' declared here)"); +} + +TEST_F(StructMemberAttributeTest, Align_Attribute_Override) { + Override("val", ty.f32(), Expr(1.23_f)); + + Structure("mystruct", utils::Vector{Member( + "a", ty.f32(), utils::Vector{MemberAlign(Source{{12, 34}}, "val")})}); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), R"(12:34 error: 'align' must be an i32 value)"); +} + } // namespace StructAndStructMemberTests using ArrayAttributeTest = TestWithParams; diff --git a/src/tint/resolver/dependency_graph.cc b/src/tint/resolver/dependency_graph.cc index 823dcada11..0b391c55f4 100644 --- a/src/tint/resolver/dependency_graph.cc +++ b/src/tint/resolver/dependency_graph.cc @@ -182,6 +182,7 @@ class DependencyScanner { [&](const ast::Struct* str) { Declare(str->name, str); for (auto* member : str->members) { + TraverseAttributes(member->attributes); TraverseType(member->type); } }, @@ -421,11 +422,16 @@ class DependencyScanner { TraverseExpression(wg->z); return; } + if (auto* align = attr->As()) { + TraverseExpression(align->expr); + return; + } + if (attr->IsAnyOf()) { + ast::StrideAttribute, ast::StructMemberOffsetAttribute, + ast::StructMemberSizeAttribute>()) { return; } diff --git a/src/tint/resolver/dependency_graph_test.cc b/src/tint/resolver/dependency_graph_test.cc index 224d342b12..c9ffc91039 100644 --- a/src/tint/resolver/dependency_graph_test.cc +++ b/src/tint/resolver/dependency_graph_test.cc @@ -1229,7 +1229,10 @@ TEST_F(ResolverDependencyGraphTraversalTest, SymbolsReached) { #define F add_use(func_decl, Expr(func_sym), __LINE__, "F()") Alias(Sym(), T); - Structure(Sym(), utils::Vector{Member(Sym(), T)}); + Structure(Sym(), // + utils::Vector{ + Member(Sym(), T, utils::Vector{MemberAlign(V)}) // + }); GlobalVar(Sym(), T, V); GlobalConst(Sym(), T, V); Func(Sym(), // diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc index ded0afa596..3102292691 100644 --- a/src/tint/resolver/resolver.cc +++ b/src/tint/resolver/resolver.cc @@ -2821,15 +2821,20 @@ sem::Struct* Resolver::Structure(const ast::Struct* str) { if (!materialized) { return nullptr; } + if (!materialized->Type()->Is()) { + AddError("'align' must be an i32 value", a->source); + return nullptr; + } + auto const_value = materialized->ConstantValue(); if (!const_value) { - AddError("'align' must be constant expression", a->expr->source); + AddError("'align' must be constant expression", a->source); return nullptr; } auto value = const_value->As(); if (value <= 0 || !utils::IsPowerOfTwo(value)) { - AddError("align value must be a positive, power-of-two integer", a->source); + AddError("'align' value must be a positive, power-of-two integer", a->source); return nullptr; } align = const_value->As(); diff --git a/src/tint/resolver/storage_class_layout_validation_test.cc b/src/tint/resolver/storage_class_layout_validation_test.cc index 91d0d5d021..818a7d0190 100644 --- a/src/tint/resolver/storage_class_layout_validation_test.cc +++ b/src/tint/resolver/storage_class_layout_validation_test.cc @@ -36,7 +36,7 @@ TEST_F(ResolverStorageClassLayoutValidationTest, StorageBuffer_UnalignedMember) Structure(Source{{12, 34}}, "S", utils::Vector{ Member("a", ty.f32(), utils::Vector{MemberSize(5_a)}), - Member(Source{{34, 56}}, "b", ty.f32(), utils::Vector{MemberAlign(1_u)}), + Member(Source{{34, 56}}, "b", ty.f32(), utils::Vector{MemberAlign(1_i)}), }); GlobalVar(Source{{78, 90}}, "a", ty.type_name("S"), ast::StorageClass::kStorage, Group(0_a), @@ -66,7 +66,7 @@ TEST_F(ResolverStorageClassLayoutValidationTest, StorageBuffer_UnalignedMember_S Structure(Source{{12, 34}}, "S", utils::Vector{ Member("a", ty.f32(), utils::Vector{MemberSize(5_a)}), - Member(Source{{34, 56}}, "b", ty.f32(), utils::Vector{MemberAlign(4_u)}), + Member(Source{{34, 56}}, "b", ty.f32(), utils::Vector{MemberAlign(4_i)}), }); GlobalVar(Source{{78, 90}}, "a", ty.type_name("S"), ast::StorageClass::kStorage, Group(0_a), @@ -142,7 +142,7 @@ TEST_F(ResolverStorageClassLayoutValidationTest, utils::Vector{ Member("scalar", ty.f32()), Member(Source{{56, 78}}, "inner", ty.type_name("Inner"), - utils::Vector{MemberAlign(16_u)}), + utils::Vector{MemberAlign(16_i)}), }); GlobalVar(Source{{78, 90}}, "a", ty.type_name("Outer"), ast::StorageClass::kUniform, Group(0_a), @@ -201,7 +201,7 @@ TEST_F(ResolverStorageClassLayoutValidationTest, UniformBuffer_UnalignedMember_A utils::Vector{ Member("scalar", ty.f32()), Member(Source{{34, 56}}, "inner", ty.type_name("Inner"), - utils::Vector{MemberAlign(16_u)}), + utils::Vector{MemberAlign(16_i)}), }); GlobalVar(Source{{78, 90}}, "a", ty.type_name("Outer"), ast::StorageClass::kUniform, Group(0_a), @@ -227,7 +227,7 @@ TEST_F(ResolverStorageClassLayoutValidationTest, UniformBuffer_MembersOffsetNotM Structure(Source{{12, 34}}, "Inner", utils::Vector{ - Member("scalar", ty.i32(), utils::Vector{MemberAlign(1_u), MemberSize(5_a)}), + Member("scalar", ty.i32(), utils::Vector{MemberAlign(1_i), MemberSize(5_a)}), }); Structure(Source{{34, 56}}, "Outer", @@ -279,7 +279,7 @@ TEST_F(ResolverStorageClassLayoutValidationTest, Member("a", ty.i32()), Member("b", ty.i32()), Member("c", ty.i32()), - Member("scalar", ty.i32(), utils::Vector{MemberAlign(1_u), MemberSize(5_a)}), + Member("scalar", ty.i32(), utils::Vector{MemberAlign(1_i), MemberSize(5_a)}), }); Structure(Source{{34, 56}}, "Outer", @@ -327,13 +327,13 @@ TEST_F(ResolverStorageClassLayoutValidationTest, Structure(Source{{12, 34}}, "Inner", utils::Vector{ - Member("scalar", ty.i32(), utils::Vector{MemberAlign(1_u), MemberSize(5_a)}), + Member("scalar", ty.i32(), utils::Vector{MemberAlign(1_i), MemberSize(5_a)}), }); Structure(Source{{34, 56}}, "Outer", utils::Vector{ Member(Source{{56, 78}}, "inner", ty.type_name("Inner")), - Member(Source{{78, 90}}, "scalar", ty.i32(), utils::Vector{MemberAlign(16_u)}), + Member(Source{{78, 90}}, "scalar", ty.i32(), utils::Vector{MemberAlign(16_i)}), }); GlobalVar(Source{{22, 34}}, "a", ty.type_name("Outer"), ast::StorageClass::kUniform, Group(0_a), @@ -551,7 +551,7 @@ TEST_F(ResolverStorageClassLayoutValidationTest, PushConstant_UnalignedMember) { Structure( Source{{12, 34}}, "S", utils::Vector{Member("a", ty.f32(), utils::Vector{MemberSize(5_a)}), - Member(Source{{34, 56}}, "b", ty.f32(), utils::Vector{MemberAlign(1_u)})}); + Member(Source{{34, 56}}, "b", ty.f32(), utils::Vector{MemberAlign(1_i)})}); GlobalVar(Source{{78, 90}}, "a", ty.type_name("S"), ast::StorageClass::kPushConstant); ASSERT_FALSE(r()->Resolve()); @@ -576,7 +576,7 @@ TEST_F(ResolverStorageClassLayoutValidationTest, PushConstant_Aligned) { // var a : S; Enable(ast::Extension::kChromiumExperimentalPushConstant); Structure("S", utils::Vector{Member("a", ty.f32(), utils::Vector{MemberSize(5_a)}), - Member("b", ty.f32(), utils::Vector{MemberAlign(4_u)})}); + Member("b", ty.f32(), utils::Vector{MemberAlign(4_i)})}); GlobalVar("a", ty.type_name("S"), ast::StorageClass::kPushConstant); ASSERT_TRUE(r()->Resolve()) << r()->error(); diff --git a/src/tint/resolver/struct_layout_test.cc b/src/tint/resolver/struct_layout_test.cc index 5b5ab686e8..1e5ec45464 100644 --- a/src/tint/resolver/struct_layout_test.cc +++ b/src/tint/resolver/struct_layout_test.cc @@ -498,15 +498,15 @@ TEST_F(ResolverStructLayoutTest, SizeAttributes) { TEST_F(ResolverStructLayoutTest, AlignAttributes) { auto* inner = Structure("Inner", utils::Vector{ - Member("a", ty.f32(), utils::Vector{MemberAlign(8_u)}), - Member("b", ty.f32(), utils::Vector{MemberAlign(16_u)}), - Member("c", ty.f32(), utils::Vector{MemberAlign(4_u)}), + Member("a", ty.f32(), utils::Vector{MemberAlign(8_i)}), + Member("b", ty.f32(), utils::Vector{MemberAlign(16_i)}), + Member("c", ty.f32(), utils::Vector{MemberAlign(4_i)}), }); auto* s = Structure("S", utils::Vector{ - Member("a", ty.f32(), utils::Vector{MemberAlign(4_u)}), - Member("b", ty.u32(), utils::Vector{MemberAlign(8_u)}), + Member("a", ty.f32(), utils::Vector{MemberAlign(4_i)}), + Member("b", ty.u32(), utils::Vector{MemberAlign(8_i)}), Member("c", ty.Of(inner)), - Member("d", ty.i32(), utils::Vector{MemberAlign(32_u)}), + Member("d", ty.i32(), utils::Vector{MemberAlign(32_i)}), }); ASSERT_TRUE(r()->Resolve()) << r()->error(); @@ -536,7 +536,7 @@ TEST_F(ResolverStructLayoutTest, AlignAttributes) { TEST_F(ResolverStructLayoutTest, StructWithLotsOfPadding) { auto* s = Structure("S", utils::Vector{ - Member("a", ty.i32(), utils::Vector{MemberAlign(1024_u)}), + Member("a", ty.i32(), utils::Vector{MemberAlign(1024_i)}), }); ASSERT_TRUE(r()->Resolve()) << r()->error(); diff --git a/src/tint/resolver/validation_test.cc b/src/tint/resolver/validation_test.cc index 90b1664615..825fbc9eb7 100644 --- a/src/tint/resolver/validation_test.cc +++ b/src/tint/resolver/validation_test.cc @@ -1237,22 +1237,31 @@ TEST_F(ResolverValidationTest, StructMemberDuplicateNamePass) { EXPECT_TRUE(r()->Resolve()); } -TEST_F(ResolverValidationTest, NonPOTStructMemberAlignAttribute) { +TEST_F(ResolverValidationTest, NegativeStructMemberAlignAttribute) { Structure("S", utils::Vector{ - Member("a", ty.f32(), utils::Vector{MemberAlign(Source{{12, 34}}, 3_u)}), + Member("a", ty.f32(), utils::Vector{MemberAlign(Source{{12, 34}}, -2_i)}), }); EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), "12:34 error: align value must be a positive, power-of-two integer"); + EXPECT_EQ(r()->error(), "12:34 error: 'align' value must be a positive, power-of-two integer"); +} + +TEST_F(ResolverValidationTest, NonPOTStructMemberAlignAttribute) { + Structure("S", utils::Vector{ + Member("a", ty.f32(), utils::Vector{MemberAlign(Source{{12, 34}}, 3_i)}), + }); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), "12:34 error: 'align' value must be a positive, power-of-two integer"); } TEST_F(ResolverValidationTest, ZeroStructMemberAlignAttribute) { Structure("S", utils::Vector{ - Member("a", ty.f32(), utils::Vector{MemberAlign(Source{{12, 34}}, 0_u)}), + Member("a", ty.f32(), utils::Vector{MemberAlign(Source{{12, 34}}, 0_i)}), }); EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), "12:34 error: align value must be a positive, power-of-two integer"); + EXPECT_EQ(r()->error(), "12:34 error: 'align' value must be a positive, power-of-two integer"); } TEST_F(ResolverValidationTest, ZeroStructMemberSizeAttribute) { @@ -1279,7 +1288,7 @@ TEST_F(ResolverValidationTest, OffsetAndSizeAttribute) { TEST_F(ResolverValidationTest, OffsetAndAlignAttribute) { Structure("S", utils::Vector{ Member(Source{{12, 34}}, "a", ty.f32(), - utils::Vector{MemberOffset(0_a), MemberAlign(4_u)}), + utils::Vector{MemberOffset(0_a), MemberAlign(4_i)}), }); EXPECT_FALSE(r()->Resolve()); @@ -1291,7 +1300,7 @@ TEST_F(ResolverValidationTest, OffsetAndAlignAttribute) { TEST_F(ResolverValidationTest, OffsetAndAlignAndSizeAttribute) { Structure("S", utils::Vector{ Member(Source{{12, 34}}, "a", ty.f32(), - utils::Vector{MemberOffset(0_a), MemberAlign(4_u), MemberSize(4_a)}), + utils::Vector{MemberOffset(0_a), MemberAlign(4_i), MemberSize(4_a)}), }); EXPECT_FALSE(r()->Resolve()); diff --git a/src/tint/transform/std140.cc b/src/tint/transform/std140.cc index 1f495f0107..66857bca9e 100644 --- a/src/tint/transform/std140.cc +++ b/src/tint/transform/std140.cc @@ -460,7 +460,7 @@ struct Std140::State { // The matrix was @align() annotated with a larger alignment // than the natural alignment for the matrix. This extra padding // needs to be applied to the first column vector. - attributes.Push(b.MemberAlign(u32(align))); + attributes.Push(b.MemberAlign(i32(align))); } if ((i == num_columns - 1) && mat->Size() != size) { // The matrix was @size() annotated with a larger size than the diff --git a/src/tint/transform/std140_test.cc b/src/tint/transform/std140_test.cc index 97936ad9fb..6e0b170308 100644 --- a/src/tint/transform/std140_test.cc +++ b/src/tint/transform/std140_test.cc @@ -200,7 +200,7 @@ struct S { struct S_std140 { before : i32, - @align(128u) + @align(128i) m_0 : vec2, m_1 : vec2, m_2 : vec2, @@ -272,7 +272,7 @@ struct S { struct S_std140 { before : i32, - @align(128u) + @align(128i) m_0 : vec2, m_1 : vec2, @size(112) diff --git a/src/tint/writer/glsl/generator_impl_storage_buffer_test.cc b/src/tint/writer/glsl/generator_impl_storage_buffer_test.cc index f6f127fb57..78d5859aea 100644 --- a/src/tint/writer/glsl/generator_impl_storage_buffer_test.cc +++ b/src/tint/writer/glsl/generator_impl_storage_buffer_test.cc @@ -33,9 +33,9 @@ void TestAlign(ProgramBuilder* ctx) { // @group(0) @binding(0) var nephews : Nephews; auto* nephews = ctx->Structure( "Nephews", utils::Vector{ - ctx->Member("huey", ctx->ty.f32(), utils::Vector{ctx->MemberAlign(256_u)}), - ctx->Member("dewey", ctx->ty.f32(), utils::Vector{ctx->MemberAlign(256_u)}), - ctx->Member("louie", ctx->ty.f32(), utils::Vector{ctx->MemberAlign(256_u)}), + ctx->Member("huey", ctx->ty.f32(), utils::Vector{ctx->MemberAlign(256_i)}), + ctx->Member("dewey", ctx->ty.f32(), utils::Vector{ctx->MemberAlign(256_i)}), + ctx->Member("louie", ctx->ty.f32(), utils::Vector{ctx->MemberAlign(256_i)}), }); ctx->GlobalVar("nephews", ctx->ty.Of(nephews), ast::StorageClass::kStorage, ctx->Binding(0_a), ctx->Group(0_a)); diff --git a/src/tint/writer/msl/generator_impl_type_test.cc b/src/tint/writer/msl/generator_impl_type_test.cc index c8f4f6d4df..a8d9fc6855 100644 --- a/src/tint/writer/msl/generator_impl_type_test.cc +++ b/src/tint/writer/msl/generator_impl_type_test.cc @@ -255,7 +255,7 @@ TEST_F(MslGeneratorImplTest, EmitType_Struct_Layout_NonComposites) { auto* s = Structure( "S", utils::Vector{ Member("a", ty.i32(), utils::Vector{MemberSize(32_a)}), - Member("b", ty.f32(), utils::Vector{MemberAlign(128_u), MemberSize(128_a)}), + Member("b", ty.f32(), utils::Vector{MemberAlign(128_i), MemberSize(128_a)}), Member("c", ty.vec2()), Member("d", ty.u32()), Member("e", ty.vec3()), @@ -372,7 +372,7 @@ TEST_F(MslGeneratorImplTest, EmitType_Struct_Layout_Structures) { auto* inner_x = Structure("inner_x", utils::Vector{ Member("a", ty.i32()), - Member("b", ty.f32(), utils::Vector{MemberAlign(512_u)}), + Member("b", ty.f32(), utils::Vector{MemberAlign(512_i)}), }); // inner_y: size(516), align(4) @@ -460,7 +460,7 @@ TEST_F(MslGeneratorImplTest, EmitType_Struct_Layout_ArrayDefaultStride) { // inner: size(1024), align(512) auto* inner = Structure("inner", utils::Vector{ Member("a", ty.i32()), - Member("b", ty.f32(), utils::Vector{MemberAlign(512_u)}), + Member("b", ty.f32(), utils::Vector{MemberAlign(512_i)}), }); // array_x: size(28), align(4) @@ -598,7 +598,7 @@ TEST_F(MslGeneratorImplTest, AttemptTintPadSymbolCollision) { // uses symbols tint_pad_[0..9] and tint_pad_[20..35] Member("tint_pad_2", ty.i32(), utils::Vector{MemberSize(32_a)}), Member("tint_pad_20", ty.f32(), - utils::Vector{MemberAlign(128_u), MemberSize(128_u)}), + utils::Vector{MemberAlign(128_i), MemberSize(128_u)}), Member("tint_pad_33", ty.vec2()), Member("tint_pad_1", ty.u32()), Member("tint_pad_3", ty.vec3()), diff --git a/src/tint/writer/spirv/builder_type_test.cc b/src/tint/writer/spirv/builder_type_test.cc index 9b6126842f..ea530d2530 100644 --- a/src/tint/writer/spirv/builder_type_test.cc +++ b/src/tint/writer/spirv/builder_type_test.cc @@ -336,7 +336,7 @@ OpMemberName %1 0 "a" TEST_F(BuilderTest_Type, GenerateStruct_DecoratedMembers) { auto* s = Structure("S", utils::Vector{ Member("a", ty.f32()), - Member("b", ty.f32(), utils::Vector{MemberAlign(8_u)}), + Member("b", ty.f32(), utils::Vector{MemberAlign(8_i)}), }); spirv::Builder& b = Build();