tint/resolver: Validate @size only on creation-fixed footprint member types.
Fixed: tint:1623
Change-Id: Id553dfe57effb4084a16fd6e6b02ad0cef85f836
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/106224
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
diff --git a/src/tint/resolver/attribute_validation_test.cc b/src/tint/resolver/attribute_validation_test.cc
index 212f594..86d6825 100644
--- a/src/tint/resolver/attribute_validation_test.cc
+++ b/src/tint/resolver/attribute_validation_test.cc
@@ -809,11 +809,25 @@
TEST_F(StructMemberAttributeTest, Size_Attribute_Override) {
Override("val", ty.f32(), Expr(1.23_f));
- Structure("mystruct", utils::Vector{Member("a", ty.f32(), utils::Vector{MemberSize("val")})});
+ Structure("mystruct",
+ utils::Vector{
+ Member("a", ty.f32(), utils::Vector{MemberSize(Expr(Source{{12, 34}}, "val"))}),
+ });
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(
r()->error(),
- R"(error: @size requires a const-expression, but expression is an override-expression)");
+ R"(12:34 error: @size requires a const-expression, but expression is an override-expression)");
+}
+
+TEST_F(StructMemberAttributeTest, Size_On_RuntimeSizedArray) {
+ Structure("mystruct",
+ utils::Vector{
+ Member("a", ty.array<i32>(), utils::Vector{MemberSize(Source{{12, 34}}, 8_a)}),
+ });
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(12:34 error: @size can only be applied to members where the member's type size can be fully determined at shader creation time)");
}
} // namespace StructAndStructMemberTests
diff --git a/src/tint/resolver/validator.cc b/src/tint/resolver/validator.cc
index 6dab977..7f1cecf 100644
--- a/src/tint/resolver/validator.cc
+++ b/src/tint/resolver/validator.cc
@@ -2186,46 +2186,69 @@
const ast::InvariantAttribute* invariant_attribute = nullptr;
const ast::InterpolateAttribute* interpolate_attribute = nullptr;
for (auto* attr : member->Declaration()->attributes) {
- if (!attr->IsAnyOf<ast::BuiltinAttribute, //
- ast::InternalAttribute, //
- ast::InterpolateAttribute, //
- ast::InvariantAttribute, //
- ast::LocationAttribute, //
- ast::StructMemberOffsetAttribute, //
- ast::StructMemberSizeAttribute, //
- ast::StructMemberAlignAttribute>()) {
- if (attr->Is<ast::StrideAttribute>() &&
- IsValidationDisabled(member->Declaration()->attributes,
- ast::DisabledValidation::kIgnoreStrideAttribute)) {
- continue;
- }
- AddError("attribute is not valid for structure members", attr->source);
+ bool ok = Switch(
+ attr, //
+ [&](const ast::InvariantAttribute* invariant) {
+ invariant_attribute = invariant;
+ return true;
+ },
+ [&](const ast::LocationAttribute* location) {
+ has_location = true;
+ TINT_ASSERT(Resolver, member->Location().has_value());
+ if (!LocationAttribute(location, member->Location().value(), member->Type(),
+ locations, stage, member->Declaration()->source)) {
+ return false;
+ }
+ return true;
+ },
+ [&](const ast::BuiltinAttribute* builtin) {
+ if (!BuiltinAttribute(builtin, member->Type(), stage,
+ /* is_input */ false)) {
+ return false;
+ }
+ if (builtin->builtin == ast::BuiltinValue::kPosition) {
+ has_position = true;
+ }
+ return true;
+ },
+ [&](const ast::InterpolateAttribute* interpolate) {
+ interpolate_attribute = interpolate;
+ if (!InterpolateAttribute(interpolate, member->Type())) {
+ return false;
+ }
+ return true;
+ },
+ [&](const ast::StructMemberSizeAttribute*) {
+ if (!member->Type()->HasCreationFixedFootprint()) {
+ AddError(
+ "@size can only be applied to members where the member's type size "
+ "can be fully determined at shader creation time",
+ attr->source);
+ return false;
+ }
+ return true;
+ },
+ [&](Default) {
+ if (!attr->IsAnyOf<ast::BuiltinAttribute, //
+ ast::InternalAttribute, //
+ ast::InterpolateAttribute, //
+ ast::InvariantAttribute, //
+ ast::LocationAttribute, //
+ ast::StructMemberOffsetAttribute, //
+ ast::StructMemberAlignAttribute>()) {
+ if (attr->Is<ast::StrideAttribute>() &&
+ IsValidationDisabled(member->Declaration()->attributes,
+ ast::DisabledValidation::kIgnoreStrideAttribute)) {
+ return true;
+ }
+ AddError("attribute is not valid for structure members", attr->source);
+ return false;
+ }
+ return true;
+ });
+ if (!ok) {
return false;
}
-
- if (auto* invariant = attr->As<ast::InvariantAttribute>()) {
- invariant_attribute = invariant;
- } else if (auto* location = attr->As<ast::LocationAttribute>()) {
- has_location = true;
- TINT_ASSERT(Resolver, member->Location().has_value());
- if (!LocationAttribute(location, member->Location().value(), member->Type(),
- locations, stage, member->Declaration()->source)) {
- return false;
- }
- } else if (auto* builtin = attr->As<ast::BuiltinAttribute>()) {
- if (!BuiltinAttribute(builtin, member->Type(), stage,
- /* is_input */ false)) {
- return false;
- }
- if (builtin->builtin == ast::BuiltinValue::kPosition) {
- has_position = true;
- }
- } else if (auto* interpolate = attr->As<ast::InterpolateAttribute>()) {
- interpolate_attribute = interpolate;
- if (!InterpolateAttribute(interpolate, member->Type())) {
- return false;
- }
- }
}
if (invariant_attribute && !has_position) {
diff --git a/src/tint/sem/abstract_numeric.cc b/src/tint/sem/abstract_numeric.cc
index 39615cd..eea9abe 100644
--- a/src/tint/sem/abstract_numeric.cc
+++ b/src/tint/sem/abstract_numeric.cc
@@ -18,7 +18,12 @@
namespace tint::sem {
-AbstractNumeric::AbstractNumeric() : Base(TypeFlags{Flag::kConstructable}) {}
+AbstractNumeric::AbstractNumeric()
+ : Base(TypeFlags{
+ Flag::kConstructable,
+ Flag::kCreationFixedFootprint,
+ Flag::kFixedFootprint,
+ }) {}
AbstractNumeric::AbstractNumeric(AbstractNumeric&&) = default;
AbstractNumeric::~AbstractNumeric() = default;
diff --git a/src/tint/sem/array.cc b/src/tint/sem/array.cc
index ed95e62..1cd6515 100644
--- a/src/tint/sem/array.cc
+++ b/src/tint/sem/array.cc
@@ -35,6 +35,15 @@
if (element->IsConstructible()) {
flags.Add(TypeFlag::kConstructable);
}
+ if (element->HasCreationFixedFootprint()) {
+ flags.Add(TypeFlag::kCreationFixedFootprint);
+ }
+ }
+ if (std::holds_alternative<ConstantArrayCount>(count) ||
+ std::holds_alternative<OverrideArrayCount>(count)) {
+ if (element->HasFixedFootprint()) {
+ flags.Add(TypeFlag::kFixedFootprint);
+ }
}
return flags;
}
diff --git a/src/tint/sem/array_test.cc b/src/tint/sem/array_test.cc
index e6efff2..51c1894 100644
--- a/src/tint/sem/array_test.cc
+++ b/src/tint/sem/array_test.cc
@@ -135,5 +135,25 @@
EXPECT_FALSE(runtime_sized->IsConstructible());
}
+TEST_F(ArrayTest, HasCreationFixedFootprint) {
+ auto* fixed_sized = create<Array>(create<U32>(), ConstantArrayCount{2u}, 4u, 8u, 32u, 16u);
+ auto* override_sized = create<Array>(create<U32>(), OverrideArrayCount{}, 4u, 8u, 32u, 16u);
+ auto* runtime_sized = create<Array>(create<U32>(), RuntimeArrayCount{}, 4u, 8u, 32u, 16u);
+
+ EXPECT_TRUE(fixed_sized->HasCreationFixedFootprint());
+ EXPECT_FALSE(override_sized->HasCreationFixedFootprint());
+ EXPECT_FALSE(runtime_sized->HasCreationFixedFootprint());
+}
+
+TEST_F(ArrayTest, HasFixedFootprint) {
+ auto* fixed_sized = create<Array>(create<U32>(), ConstantArrayCount{2u}, 4u, 8u, 32u, 16u);
+ auto* override_sized = create<Array>(create<U32>(), OverrideArrayCount{}, 4u, 8u, 32u, 16u);
+ auto* runtime_sized = create<Array>(create<U32>(), RuntimeArrayCount{}, 4u, 8u, 32u, 16u);
+
+ EXPECT_TRUE(fixed_sized->HasFixedFootprint());
+ EXPECT_TRUE(override_sized->HasFixedFootprint());
+ EXPECT_FALSE(runtime_sized->HasFixedFootprint());
+}
+
} // namespace
} // namespace tint::sem
diff --git a/src/tint/sem/atomic.cc b/src/tint/sem/atomic.cc
index 965ebe2..f8cf068 100644
--- a/src/tint/sem/atomic.cc
+++ b/src/tint/sem/atomic.cc
@@ -22,7 +22,12 @@
namespace tint::sem {
-Atomic::Atomic(const sem::Type* subtype) : Base(TypeFlags{}), subtype_(subtype) {
+Atomic::Atomic(const sem::Type* subtype)
+ : Base(TypeFlags{
+ Flag::kCreationFixedFootprint,
+ Flag::kFixedFootprint,
+ }),
+ subtype_(subtype) {
TINT_ASSERT(AST, !subtype->Is<Reference>());
}
diff --git a/src/tint/sem/bool.cc b/src/tint/sem/bool.cc
index 8d3d1bf..fea6679 100644
--- a/src/tint/sem/bool.cc
+++ b/src/tint/sem/bool.cc
@@ -20,7 +20,12 @@
namespace tint::sem {
-Bool::Bool() : Base(TypeFlags{Flag::kConstructable}) {}
+Bool::Bool()
+ : Base(TypeFlags{
+ Flag::kConstructable,
+ Flag::kCreationFixedFootprint,
+ Flag::kFixedFootprint,
+ }) {}
Bool::Bool(Bool&&) = default;
diff --git a/src/tint/sem/f16.cc b/src/tint/sem/f16.cc
index 0595821..158b826 100644
--- a/src/tint/sem/f16.cc
+++ b/src/tint/sem/f16.cc
@@ -21,7 +21,12 @@
namespace tint {
namespace sem {
-F16::F16() : Base(TypeFlags{Flag::kConstructable}) {}
+F16::F16()
+ : Base(TypeFlags{
+ Flag::kConstructable,
+ Flag::kCreationFixedFootprint,
+ Flag::kFixedFootprint,
+ }) {}
F16::F16(F16&&) = default;
diff --git a/src/tint/sem/f32.cc b/src/tint/sem/f32.cc
index c1f99eb..73c98e6 100644
--- a/src/tint/sem/f32.cc
+++ b/src/tint/sem/f32.cc
@@ -20,7 +20,12 @@
namespace tint::sem {
-F32::F32() : Base(TypeFlags{Flag::kConstructable}) {}
+F32::F32()
+ : Base(TypeFlags{
+ Flag::kConstructable,
+ Flag::kCreationFixedFootprint,
+ Flag::kFixedFootprint,
+ }) {}
F32::F32(F32&&) = default;
diff --git a/src/tint/sem/i32.cc b/src/tint/sem/i32.cc
index 8047c29..6b23155 100644
--- a/src/tint/sem/i32.cc
+++ b/src/tint/sem/i32.cc
@@ -20,7 +20,12 @@
namespace tint::sem {
-I32::I32() : Base(TypeFlags{Flag::kConstructable}) {}
+I32::I32()
+ : Base(TypeFlags{
+ Flag::kConstructable,
+ Flag::kCreationFixedFootprint,
+ Flag::kFixedFootprint,
+ }) {}
I32::I32(I32&&) = default;
diff --git a/src/tint/sem/matrix.cc b/src/tint/sem/matrix.cc
index 5a3eb09..1d0453d 100644
--- a/src/tint/sem/matrix.cc
+++ b/src/tint/sem/matrix.cc
@@ -23,7 +23,11 @@
namespace tint::sem {
Matrix::Matrix(const Vector* column_type, uint32_t columns)
- : Base(TypeFlags{Flag::kConstructable}),
+ : Base(TypeFlags{
+ Flag::kConstructable,
+ Flag::kCreationFixedFootprint,
+ Flag::kFixedFootprint,
+ }),
subtype_(column_type->type()),
column_type_(column_type),
rows_(column_type->Width()),
diff --git a/src/tint/sem/struct.cc b/src/tint/sem/struct.cc
index 4a0a3c1..d80a04a 100644
--- a/src/tint/sem/struct.cc
+++ b/src/tint/sem/struct.cc
@@ -30,11 +30,20 @@
namespace {
TypeFlags FlagsFrom(const StructMemberList& members) {
- TypeFlags flags{TypeFlag::kConstructable};
+ TypeFlags flags{
+ TypeFlag::kConstructable,
+ TypeFlag::kCreationFixedFootprint,
+ TypeFlag::kFixedFootprint,
+ };
for (auto* member : members) {
if (!member->Type()->IsConstructible()) {
flags.Remove(TypeFlag::kConstructable);
- break;
+ }
+ if (!member->Type()->HasFixedFootprint()) {
+ flags.Remove(TypeFlag::kFixedFootprint);
+ }
+ if (!member->Type()->HasCreationFixedFootprint()) {
+ flags.Remove(TypeFlag::kCreationFixedFootprint);
}
}
return flags;
diff --git a/src/tint/sem/struct_test.cc b/src/tint/sem/struct_test.cc
index 12ded84..a8b36ca 100644
--- a/src/tint/sem/struct_test.cc
+++ b/src/tint/sem/struct_test.cc
@@ -157,5 +157,71 @@
EXPECT_FALSE(sem_outer_runtime_sized_array->IsConstructible());
}
+TEST_F(StructTest, HasCreationFixedFootprint) {
+ auto* inner = //
+ Structure("Inner", utils::Vector{
+ Member("a", ty.i32()),
+ Member("b", ty.u32()),
+ Member("c", ty.f32()),
+ Member("d", ty.vec3<f32>()),
+ Member("e", ty.mat4x2<f32>()),
+ Member("f", ty.array<f32, 32>()),
+ });
+
+ auto* outer = Structure("Outer", utils::Vector{
+ Member("inner", ty.type_name("Inner")),
+ });
+
+ auto* outer_with_runtime_sized_array =
+ Structure("OuterRuntimeSizedArray", utils::Vector{
+ Member("inner", ty.type_name("Inner")),
+ Member("runtime_sized_array", ty.array<i32>()),
+ });
+
+ auto p = Build();
+ ASSERT_TRUE(p.IsValid()) << p.Diagnostics().str();
+
+ auto* sem_inner = p.Sem().Get(inner);
+ auto* sem_outer = p.Sem().Get(outer);
+ auto* sem_outer_with_runtime_sized_array = p.Sem().Get(outer_with_runtime_sized_array);
+
+ EXPECT_TRUE(sem_inner->HasCreationFixedFootprint());
+ EXPECT_TRUE(sem_outer->HasCreationFixedFootprint());
+ EXPECT_FALSE(sem_outer_with_runtime_sized_array->HasCreationFixedFootprint());
+}
+
+TEST_F(StructTest, HasFixedFootprint) {
+ auto* inner = //
+ Structure("Inner", utils::Vector{
+ Member("a", ty.i32()),
+ Member("b", ty.u32()),
+ Member("c", ty.f32()),
+ Member("d", ty.vec3<f32>()),
+ Member("e", ty.mat4x2<f32>()),
+ Member("f", ty.array<f32, 32>()),
+ });
+
+ auto* outer = Structure("Outer", utils::Vector{
+ Member("inner", ty.type_name("Inner")),
+ });
+
+ auto* outer_with_runtime_sized_array =
+ Structure("OuterRuntimeSizedArray", utils::Vector{
+ Member("inner", ty.type_name("Inner")),
+ Member("runtime_sized_array", ty.array<i32>()),
+ });
+
+ auto p = Build();
+ ASSERT_TRUE(p.IsValid()) << p.Diagnostics().str();
+
+ auto* sem_inner = p.Sem().Get(inner);
+ auto* sem_outer = p.Sem().Get(outer);
+ auto* sem_outer_with_runtime_sized_array = p.Sem().Get(outer_with_runtime_sized_array);
+
+ EXPECT_TRUE(sem_inner->HasFixedFootprint());
+ EXPECT_TRUE(sem_outer->HasFixedFootprint());
+ EXPECT_FALSE(sem_outer_with_runtime_sized_array->HasFixedFootprint());
+}
+
} // namespace
} // namespace tint::sem
diff --git a/src/tint/sem/type.cc b/src/tint/sem/type.cc
index 7191fc6..0596a37 100644
--- a/src/tint/sem/type.cc
+++ b/src/tint/sem/type.cc
@@ -34,7 +34,11 @@
namespace tint::sem {
-Type::Type(TypeFlags flags) : flags_(flags) {}
+Type::Type(TypeFlags flags) : flags_(flags) {
+ if (IsConstructible()) {
+ TINT_ASSERT(Semantic, HasCreationFixedFootprint());
+ }
+}
Type::Type(Type&&) = default;
diff --git a/src/tint/sem/type.h b/src/tint/sem/type.h
index 3b68946..38f5ba8 100644
--- a/src/tint/sem/type.h
+++ b/src/tint/sem/type.h
@@ -34,6 +34,12 @@
/// Type is constructable.
/// @see https://gpuweb.github.io/gpuweb/wgsl/#constructible-types
kConstructable,
+ /// Type has a creation-fixed footprint.
+ /// @see https://www.w3.org/TR/WGSL/#fixed-footprint-types
+ kCreationFixedFootprint,
+ /// Type has a fixed footprint.
+ /// @see https://www.w3.org/TR/WGSL/#fixed-footprint-types
+ kFixedFootprint,
};
/// An alias to utils::EnumSet<TypeFlag>
@@ -83,6 +89,16 @@
/// https://gpuweb.github.io/gpuweb/wgsl/#constructible-types
inline bool IsConstructible() const { return flags_.Contains(Flag::kConstructable); }
+ /// @returns true has a creation-fixed footprint.
+ /// @see https://www.w3.org/TR/WGSL/#fixed-footprint-types
+ inline bool HasCreationFixedFootprint() const {
+ return flags_.Contains(Flag::kCreationFixedFootprint);
+ }
+
+ /// @returns true has a fixed footprint.
+ /// @see https://www.w3.org/TR/WGSL/#fixed-footprint-types
+ inline bool HasFixedFootprint() const { return flags_.Contains(Flag::kFixedFootprint); }
+
/// @returns true if this type is a scalar
bool is_scalar() const;
/// @returns true if this type is a numeric scalar
diff --git a/src/tint/sem/u32.cc b/src/tint/sem/u32.cc
index 91fd7d7..a93ff4c 100644
--- a/src/tint/sem/u32.cc
+++ b/src/tint/sem/u32.cc
@@ -20,7 +20,12 @@
namespace tint::sem {
-U32::U32() : Base(TypeFlags{Flag::kConstructable}) {}
+U32::U32()
+ : Base(TypeFlags{
+ Flag::kConstructable,
+ Flag::kCreationFixedFootprint,
+ Flag::kFixedFootprint,
+ }) {}
U32::~U32() = default;
diff --git a/src/tint/sem/vector.cc b/src/tint/sem/vector.cc
index e6d3acf..32b7a34 100644
--- a/src/tint/sem/vector.cc
+++ b/src/tint/sem/vector.cc
@@ -22,7 +22,13 @@
namespace tint::sem {
Vector::Vector(Type const* subtype, uint32_t width)
- : Base(TypeFlags{Flag::kConstructable}), subtype_(subtype), width_(width) {
+ : Base(TypeFlags{
+ Flag::kConstructable,
+ Flag::kCreationFixedFootprint,
+ Flag::kFixedFootprint,
+ }),
+ subtype_(subtype),
+ width_(width) {
TINT_ASSERT(Semantic, width_ > 1);
TINT_ASSERT(Semantic, width_ < 5);
}
