Resolver: Add validation for host-sharable / storage
Fixes a TODO
Bug: tint:60
Change-Id: Ica44d6dbff682374473cacec9d0515e6d3b02f4c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45245
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index faec6b1..f7b708c 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -468,7 +468,9 @@
intrinsic_table_test.cc
program_test.cc
resolver/decoration_validation_test.cc
+ resolver/host_sharable_validation_test.cc
resolver/intrinsic_test.cc
+ resolver/is_host_sharable_test.cc
resolver/is_storeable_test.cc
resolver/resolver_test_helper.cc
resolver/resolver_test_helper.h
diff --git a/src/resolver/host_sharable_validation_test.cc b/src/resolver/host_sharable_validation_test.cc
new file mode 100644
index 0000000..772e4db
--- /dev/null
+++ b/src/resolver/host_sharable_validation_test.cc
@@ -0,0 +1,153 @@
+// Copyright 2021 The Tint Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "src/resolver/resolver.h"
+
+#include "gmock/gmock.h"
+#include "src/resolver/resolver_test_helper.h"
+#include "src/semantic/struct.h"
+#include "src/type/access_control_type.h"
+
+namespace tint {
+namespace resolver {
+namespace {
+
+using ResolverHostSharableValidationTest = ResolverTest;
+
+TEST_F(ResolverHostSharableValidationTest, Bool) {
+ Global(Source{{56, 78}}, "g", ty.bool_(), ast::StorageClass::kStorage);
+
+ ASSERT_FALSE(r()->Resolve());
+
+ EXPECT_EQ(
+ r()->error(),
+ R"(56:78 error: Type 'bool' cannot be used in storage class 'storage' as it is non-host-sharable
+56:78 note: while instantiating variable g)");
+}
+
+TEST_F(ResolverHostSharableValidationTest, Pointer) {
+ Global(Source{{56, 78}}, "g", ty.pointer<i32>(ast::StorageClass::kNone),
+ ast::StorageClass::kStorage);
+
+ ASSERT_FALSE(r()->Resolve());
+
+ EXPECT_EQ(
+ r()->error(),
+ R"(56:78 error: Type 'ptr<i32>' cannot be used in storage class 'storage' as it is non-host-sharable
+56:78 note: while instantiating variable g)");
+}
+
+TEST_F(ResolverHostSharableValidationTest, BoolMember) {
+ auto* s = Structure("S", {Member(Source{{12, 34}}, "x", ty.bool_())});
+ Global(Source{{56, 78}}, "g", s, ast::StorageClass::kStorage);
+
+ ASSERT_FALSE(r()->Resolve());
+
+ EXPECT_EQ(
+ r()->error(),
+ R"(56:78 error: Type 'bool' cannot be used in storage class 'storage' as it is non-host-sharable
+12:34 note: while analysing structure member S.x
+56:78 note: while instantiating variable g)");
+}
+
+TEST_F(ResolverHostSharableValidationTest, BoolVectorMember) {
+ auto* s = Structure("S", {Member(Source{{12, 34}}, "x", ty.vec3<bool>())});
+ Global(Source{{56, 78}}, "g", s, ast::StorageClass::kStorage);
+
+ ASSERT_FALSE(r()->Resolve());
+
+ EXPECT_EQ(
+ r()->error(),
+ R"(56:78 error: Type 'vec3<bool>' cannot be used in storage class 'storage' as it is non-host-sharable
+12:34 note: while analysing structure member S.x
+56:78 note: while instantiating variable g)");
+}
+
+TEST_F(ResolverHostSharableValidationTest, Aliases) {
+ auto* a1 = ty.alias("a1", ty.bool_());
+ auto* s = Structure("S", {Member(Source{{12, 34}}, "x", a1)});
+ auto* a2 = ty.alias("a2", s);
+ Global(Source{{56, 78}}, "g", a2, ast::StorageClass::kStorage);
+
+ ASSERT_FALSE(r()->Resolve());
+
+ EXPECT_EQ(
+ r()->error(),
+ R"(56:78 error: Type 'bool' cannot be used in storage class 'storage' as it is non-host-sharable
+12:34 note: while analysing structure member S.x
+56:78 note: while instantiating variable g)");
+}
+
+TEST_F(ResolverHostSharableValidationTest, AccessControl) {
+ auto* s = Structure("S", {Member(Source{{12, 34}}, "x", ty.bool_())});
+ auto* a = create<type::AccessControl>(ast::AccessControl::kReadOnly, s);
+ Global(Source{{56, 78}}, "g", a, ast::StorageClass::kStorage);
+
+ ASSERT_FALSE(r()->Resolve());
+
+ EXPECT_EQ(
+ r()->error(),
+ R"(56:78 error: Type 'bool' cannot be used in storage class 'storage' as it is non-host-sharable
+12:34 note: while analysing structure member S.x
+56:78 note: while instantiating variable g)");
+}
+
+TEST_F(ResolverHostSharableValidationTest, NestedStructures) {
+ auto* i1 = Structure("I1", {Member(Source{{1, 2}}, "x", ty.bool_())});
+ auto* i2 = Structure("I2", {Member(Source{{3, 4}}, "y", i1)});
+ auto* i3 = Structure("I3", {Member(Source{{5, 6}}, "z", i2)});
+
+ auto* s = Structure("S", {Member(Source{{7, 8}}, "m", i3)});
+ Global(Source{{9, 10}}, "g", s, ast::StorageClass::kStorage);
+
+ ASSERT_FALSE(r()->Resolve());
+
+ EXPECT_EQ(
+ r()->error(),
+ R"(9:10 error: Type 'bool' cannot be used in storage class 'storage' as it is non-host-sharable
+1:2 note: while analysing structure member I1.x
+3:4 note: while analysing structure member I2.y
+5:6 note: while analysing structure member I3.z
+7:8 note: while analysing structure member S.m
+9:10 note: while instantiating variable g)");
+}
+
+TEST_F(ResolverHostSharableValidationTest, NoError) {
+ auto* i1 =
+ Structure("I1", {
+ Member(Source{{1, 1}}, "x1", ty.f32()),
+ Member(Source{{2, 1}}, "y1", ty.vec3<f32>()),
+ Member(Source{{3, 1}}, "z1", ty.array<i32, 4>()),
+ });
+ auto* i2 = Structure("I2", {
+ Member(Source{{4, 1}}, "x2", ty.mat2x2<f32>()),
+ Member(Source{{5, 1}}, "y2", i1),
+ Member(Source{{6, 1}}, "z2", ty.mat3x2<i32>()),
+ });
+ auto* i3 =
+ Structure("I3", {
+ Member(Source{{4, 1}}, "x3", ty.alias("a1", i1)),
+ Member(Source{{5, 1}}, "y3", i2),
+ Member(Source{{6, 1}}, "z3", ty.alias("a2", i2)),
+ });
+
+ auto* s = Structure("S", {Member(Source{{7, 8}}, "m", i3)});
+ Global(Source{{9, 10}}, "g", s, ast::StorageClass::kStorage);
+
+ ASSERT_TRUE(r()->Resolve()) << r()->error();
+}
+
+} // namespace
+} // namespace resolver
+} // namespace tint
diff --git a/src/resolver/is_host_sharable_test.cc b/src/resolver/is_host_sharable_test.cc
new file mode 100644
index 0000000..a8e641a
--- /dev/null
+++ b/src/resolver/is_host_sharable_test.cc
@@ -0,0 +1,150 @@
+// Copyright 2021 The Tint Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "src/resolver/resolver.h"
+
+#include "gmock/gmock.h"
+#include "src/resolver/resolver_test_helper.h"
+#include "src/type/access_control_type.h"
+
+namespace tint {
+namespace resolver {
+namespace {
+
+using ResolverIsHostSharable = ResolverTest;
+
+TEST_F(ResolverIsHostSharable, Void) {
+ EXPECT_FALSE(r()->IsHostSharable(ty.void_()));
+}
+
+TEST_F(ResolverIsHostSharable, Bool) {
+ EXPECT_FALSE(r()->IsHostSharable(ty.bool_()));
+}
+
+TEST_F(ResolverIsHostSharable, NumericScalar) {
+ EXPECT_TRUE(r()->IsHostSharable(ty.i32()));
+ EXPECT_TRUE(r()->IsHostSharable(ty.u32()));
+ EXPECT_TRUE(r()->IsHostSharable(ty.f32()));
+}
+
+TEST_F(ResolverIsHostSharable, NumericVector) {
+ EXPECT_TRUE(r()->IsHostSharable(ty.vec2<i32>()));
+ EXPECT_TRUE(r()->IsHostSharable(ty.vec3<i32>()));
+ EXPECT_TRUE(r()->IsHostSharable(ty.vec4<i32>()));
+ EXPECT_TRUE(r()->IsHostSharable(ty.vec2<u32>()));
+ EXPECT_TRUE(r()->IsHostSharable(ty.vec3<u32>()));
+ EXPECT_TRUE(r()->IsHostSharable(ty.vec4<u32>()));
+ EXPECT_TRUE(r()->IsHostSharable(ty.vec2<f32>()));
+ EXPECT_TRUE(r()->IsHostSharable(ty.vec3<f32>()));
+ EXPECT_TRUE(r()->IsHostSharable(ty.vec4<f32>()));
+}
+
+TEST_F(ResolverIsHostSharable, BoolVector) {
+ EXPECT_FALSE(r()->IsHostSharable(ty.vec2<bool>()));
+ EXPECT_FALSE(r()->IsHostSharable(ty.vec3<bool>()));
+ EXPECT_FALSE(r()->IsHostSharable(ty.vec4<bool>()));
+ EXPECT_FALSE(r()->IsHostSharable(ty.vec2<bool>()));
+ EXPECT_FALSE(r()->IsHostSharable(ty.vec3<bool>()));
+ EXPECT_FALSE(r()->IsHostSharable(ty.vec4<bool>()));
+ EXPECT_FALSE(r()->IsHostSharable(ty.vec2<bool>()));
+ EXPECT_FALSE(r()->IsHostSharable(ty.vec3<bool>()));
+ EXPECT_FALSE(r()->IsHostSharable(ty.vec4<bool>()));
+}
+
+TEST_F(ResolverIsHostSharable, Matrix) {
+ EXPECT_TRUE(r()->IsHostSharable(ty.mat2x2<f32>()));
+ EXPECT_TRUE(r()->IsHostSharable(ty.mat2x3<f32>()));
+ EXPECT_TRUE(r()->IsHostSharable(ty.mat2x4<f32>()));
+ EXPECT_TRUE(r()->IsHostSharable(ty.mat3x2<f32>()));
+ EXPECT_TRUE(r()->IsHostSharable(ty.mat3x3<f32>()));
+ EXPECT_TRUE(r()->IsHostSharable(ty.mat3x4<f32>()));
+ EXPECT_TRUE(r()->IsHostSharable(ty.mat4x2<f32>()));
+ EXPECT_TRUE(r()->IsHostSharable(ty.mat4x3<f32>()));
+ EXPECT_TRUE(r()->IsHostSharable(ty.mat4x4<f32>()));
+}
+
+TEST_F(ResolverIsHostSharable, Pointer) {
+ EXPECT_FALSE(
+ r()->IsHostSharable(ty.pointer<i32>(ast::StorageClass::kPrivate)));
+}
+
+TEST_F(ResolverIsHostSharable, AliasVoid) {
+ EXPECT_FALSE(r()->IsHostSharable(ty.alias("a", ty.void_())));
+}
+
+TEST_F(ResolverIsHostSharable, AliasI32) {
+ EXPECT_TRUE(r()->IsHostSharable(ty.alias("a", ty.i32())));
+}
+
+TEST_F(ResolverIsHostSharable, AccessControlVoid) {
+ EXPECT_FALSE(r()->IsHostSharable(
+ create<type::AccessControl>(ast::AccessControl::kReadOnly, ty.void_())));
+}
+
+TEST_F(ResolverIsHostSharable, AccessControlI32) {
+ EXPECT_TRUE(r()->IsHostSharable(
+ create<type::AccessControl>(ast::AccessControl::kReadOnly, ty.i32())));
+}
+
+TEST_F(ResolverIsHostSharable, ArraySizedOfHostSharable) {
+ EXPECT_TRUE(r()->IsHostSharable(ty.array(ty.i32(), 5)));
+}
+
+TEST_F(ResolverIsHostSharable, ArrayUnsizedOfHostSharable) {
+ EXPECT_TRUE(r()->IsHostSharable(ty.array<i32>()));
+}
+
+TEST_F(ResolverIsHostSharable, Struct_AllMembersHostSharable) {
+ EXPECT_TRUE(r()->IsHostSharable(Structure("S", {
+ Member("a", ty.i32()),
+ Member("b", ty.f32()),
+ })));
+}
+
+TEST_F(ResolverIsHostSharable, Struct_SomeMembersNonHostSharable) {
+ auto* ptr_ty = ty.pointer<i32>(ast::StorageClass::kPrivate);
+ EXPECT_FALSE(r()->IsHostSharable(Structure("S", {
+ Member("a", ty.i32()),
+ Member("b", ptr_ty),
+ })));
+}
+
+TEST_F(ResolverIsHostSharable, Struct_NestedHostSharable) {
+ auto* host_sharable = Structure("S", {
+ Member("a", ty.i32()),
+ Member("b", ty.f32()),
+ });
+ EXPECT_TRUE(r()->IsHostSharable(Structure("S", {
+ Member("a", ty.i32()),
+ Member("b", host_sharable),
+ })));
+}
+
+TEST_F(ResolverIsHostSharable, Struct_NestedNonHostSharable) {
+ auto* ptr_ty = ty.pointer<i32>(ast::StorageClass::kPrivate);
+ auto* non_host_sharable =
+ Structure("non_host_sharable", {
+ Member("a", ty.i32()),
+ Member("b", ptr_ty),
+ });
+ EXPECT_FALSE(
+ r()->IsHostSharable(Structure("S", {
+ Member("a", ty.i32()),
+ Member("b", non_host_sharable),
+ })));
+}
+
+} // namespace
+} // namespace resolver
+} // namespace tint
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index 90a4afc..4f4f797 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -131,6 +131,32 @@
return false;
}
+// https://gpuweb.github.io/gpuweb/wgsl.html#host-shareable-types
+bool Resolver::IsHostSharable(type::Type* type) {
+ type = type->UnwrapIfNeeded();
+ if (type->IsAnyOf<type::I32, type::U32, type::F32>()) {
+ return true;
+ }
+ if (auto* vec = type->As<type::Vector>()) {
+ return IsHostSharable(vec->type());
+ }
+ if (auto* mat = type->As<type::Matrix>()) {
+ return IsHostSharable(mat->type());
+ }
+ if (auto* arr = type->As<type::Array>()) {
+ return IsHostSharable(arr->type());
+ }
+ if (auto* str = type->As<type::Struct>()) {
+ for (auto* member : str->impl()->members()) {
+ if (!IsHostSharable(member->type())) {
+ return false;
+ }
+ }
+ return true;
+ }
+ return false;
+}
+
bool Resolver::ResolveInternal() {
for (auto* ty : builder_->Types()) {
if (auto* str = ty->As<type::Struct>()) {
@@ -157,7 +183,10 @@
}
if (!ApplyStorageClassUsageToType(var->declared_storage_class(),
- var->type())) {
+ var->type(), var->source())) {
+ diagnostics_.add_note("while instantiating variable " +
+ builder_->Symbols().NameFor(var->symbol()),
+ var->source());
return false;
}
}
@@ -1159,7 +1188,11 @@
}
}
- if (!ApplyStorageClassUsageToType(info->storage_class, var->type())) {
+ if (!ApplyStorageClassUsageToType(info->storage_class, var->type(),
+ var->source())) {
+ diagnostics_.add_note("while instantiating variable " +
+ builder_->Symbols().NameFor(var->symbol()),
+ var->source());
return false;
}
@@ -1559,7 +1592,8 @@
}
bool Resolver::ApplyStorageClassUsageToType(ast::StorageClass sc,
- type::Type* ty) {
+ type::Type* ty,
+ Source usage) {
ty = ty->UnwrapIfNeeded();
if (auto* str = ty->As<type::Struct>()) {
@@ -1572,25 +1606,29 @@
}
info->storage_class_usage.emplace(sc);
for (auto* member : str->impl()->members()) {
- // TODO(amaiorano): Determine the host-sharable types
- bool can_be_host_sharable = true;
- if (ast::IsHostSharable(sc) && !can_be_host_sharable) {
+ if (!ApplyStorageClassUsageToType(sc, member->type(), usage)) {
std::stringstream err;
- err << "Structure '" << str->FriendlyName(builder_->Symbols())
- << "' is used by storage class " << sc
- << " which contains a member of non-host-sharable type "
- << member->type()->FriendlyName(builder_->Symbols());
- diagnostics_.add_error(err.str(), member->source());
- return false;
- }
- if (!ApplyStorageClassUsageToType(sc, member->type())) {
+ err << "while analysing structure member "
+ << str->FriendlyName(builder_->Symbols()) << "."
+ << builder_->Symbols().NameFor(member->symbol());
+ diagnostics_.add_note(err.str(), member->source());
return false;
}
}
+ return true;
}
if (auto* arr = ty->As<type::Array>()) {
- return ApplyStorageClassUsageToType(sc, arr->type());
+ return ApplyStorageClassUsageToType(sc, arr->type(), usage);
+ }
+
+ if (ast::IsHostSharable(sc) && !IsHostSharable(ty)) {
+ std::stringstream err;
+ err << "Type '" << ty->FriendlyName(builder_->Symbols())
+ << "' cannot be used in storage class '" << sc
+ << "' as it is non-host-sharable";
+ diagnostics_.add_error(err.str(), usage);
+ return false;
}
return true;
diff --git a/src/resolver/resolver.h b/src/resolver/resolver.h
index 97e915a..d827e81 100644
--- a/src/resolver/resolver.h
+++ b/src/resolver/resolver.h
@@ -70,9 +70,13 @@
bool Resolve();
/// @param type the given type
- /// @returns true if the given type is storable.
+ /// @returns true if the given type is storable
static bool IsStorable(type::Type* type);
+ /// @param type the given type
+ /// @returns true if the given type is host-sharable
+ static bool IsHostSharable(type::Type* type);
+
private:
/// Structure holding semantic information about a variable.
/// Used to build the semantic::Variable nodes at the end of resolving.
@@ -227,8 +231,14 @@
/// Records the storage class usage for the given type, and any transient
/// dependencies of the type. Validates that the type can be used for the
/// given storage class, erroring if it cannot.
+ /// @param sc the storage class to apply to the type and transitent types
+ /// @param ty the type to apply the storage class on
+ /// @param usage the Source of the root variable declaration that uses the
+ /// given type and storage class. Used for generating sensible error messages.
/// @returns true on success, false on error
- bool ApplyStorageClassUsageToType(ast::StorageClass, type::Type*);
+ bool ApplyStorageClassUsageToType(ast::StorageClass sc,
+ type::Type* ty,
+ Source usage);
/// @param align the output default alignment in bytes for the type `ty`
/// @param size the output default size in bytes for the type `ty`
diff --git a/test/BUILD.gn b/test/BUILD.gn
index d595209..85bf3fc 100644
--- a/test/BUILD.gn
+++ b/test/BUILD.gn
@@ -170,7 +170,9 @@
"../src/program_builder_test.cc",
"../src/program_test.cc",
"../src/resolver/decoration_validation_test.cc",
+ "../src/resolver/host_sharable_validation_test.cc",
"../src/resolver/intrinsic_test.cc",
+ "../src/resolver/is_host_sharable_test.cc",
"../src/resolver/is_storeable_test.cc",
"../src/resolver/resolver_test.cc",
"../src/resolver/resolver_test_helper.cc",
