Update DEPRECATED comments about offset decorations Let's keep these for the SPIR-V reader case. The way things currently work is actually nicer than attempting to generate size / align decorations in the SPIR-V reader. Change-Id: I83087c153e3b3056e737dcfbfd73ae6a0986bd7c Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44684 Commit-Queue: Ben Clayton <bclayton@google.com> Reviewed-by: David Neto <dneto@google.com>
diff --git a/src/ast/struct_member_offset_decoration.h b/src/ast/struct_member_offset_decoration.h index 3ab806d..b65b784 100644 --- a/src/ast/struct_member_offset_decoration.h +++ b/src/ast/struct_member_offset_decoration.h
@@ -21,8 +21,15 @@ namespace ast { /// A struct member offset decoration -// [DEPRECATED] - Replaced with StructMemberAlignDecoration and -// StructMemberSizeDecoration +/// @note The WGSL spec removed the `[[offset(n)]]` decoration for `[[size(n)]]` +/// and `[[align(n)]]` in https://github.com/gpuweb/gpuweb/pull/1447. However +/// this decoration is kept because the SPIR-V reader has to deal with absolute +/// offsets, and transforming these to size / align is complex and can be done +/// in a number of ways. The Resolver is responsible for consuming the size and +/// align decorations and transforming these into absolute offsets. It is +/// trivial for the Resolver to handle `[[offset(n)]]` or `[[size(n)]]` / +/// `[[align(n)]]` decorations, so this is what we do, keeping all the layout +/// logic in one place. class StructMemberOffsetDecoration : public Castable<StructMemberOffsetDecoration, Decoration> { public:
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc index 405a80b..e1f1301 100644 --- a/src/resolver/resolver.cc +++ b/src/resolver/resolver.cc
@@ -1155,9 +1155,13 @@ return nullptr; } + bool has_offset_deco = false; + bool has_align_deco = false; + bool has_size_deco = false; for (auto* deco : member->decorations()) { if (auto* o = deco->As<ast::StructMemberOffsetDecoration>()) { - // [DEPRECATED] + // Offset decorations are not part of the WGSL spec, but are emitted by + // the SPIR-V reader. if (o->offset() < struct_size) { diagnostics_.add_error("offsets must be in ascending order", o->source()); @@ -1165,6 +1169,7 @@ } offset = o->offset(); align = 1; + has_offset_deco = true; } else if (auto* a = deco->As<ast::StructMemberAlignDecoration>()) { if (a->align() <= 0 || !utils::IsPowerOfTwo(a->align())) { diagnostics_.add_error( @@ -1173,6 +1178,7 @@ return nullptr; } align = a->align(); + has_align_deco = true; } else if (auto* s = deco->As<ast::StructMemberSizeDecoration>()) { if (s->size() < size) { diagnostics_.add_error( @@ -1182,9 +1188,17 @@ return nullptr; } size = s->size(); + has_size_deco = true; } } + if (has_offset_deco && (has_align_deco || has_size_deco)) { + diagnostics_.add_error( + "offset decorations cannot be used with align or size decorations", + member->source()); + return nullptr; + } + offset = utils::RoundUp(align, offset); auto* sem_member =
diff --git a/src/resolver/validation_test.cc b/src/resolver/validation_test.cc index f3d3cbd..02965fe 100644 --- a/src/resolver/validation_test.cc +++ b/src/resolver/validation_test.cc
@@ -588,6 +588,42 @@ "12:34 error: size must be at least as big as the type's size (4)"); } +TEST_F(ResolverValidationTest, OffsetAndSizeDecoration) { + Structure("S", { + Member(Source{{12, 34}}, "a", ty.f32(), + {MemberOffset(0), MemberSize(4)}), + }); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: offset decorations cannot be used with align or size " + "decorations"); +} + +TEST_F(ResolverValidationTest, OffsetAndAlignDecoration) { + Structure("S", { + Member(Source{{12, 34}}, "a", ty.f32(), + {MemberOffset(0), MemberAlign(4)}), + }); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: offset decorations cannot be used with align or size " + "decorations"); +} + +TEST_F(ResolverValidationTest, OffsetAndAlignAndSizeDecoration) { + Structure("S", { + Member(Source{{12, 34}}, "a", ty.f32(), + {MemberOffset(0), MemberAlign(4), MemberSize(4)}), + }); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: offset decorations cannot be used with align or size " + "decorations"); +} + } // namespace } // namespace resolver } // namespace tint