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
