encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

tint/resolver: Clean up variable validation 

Move the validation of usage and address space to helper.
Improve diagnostics.
Fix / clean up tests.

This is in preparation for tint:1553

Change-Id: I2cbc8b851ecf02f214341f8cba6bd52413c42911
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/111880
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
This commit is contained in:
Ben Clayton 2022-11-29 22:34:35 +00:00 committed by Dawn LUCI CQ
parent 67d52eb4f2
commit 34afd9b6c6
7 changed files with 267 additions and 268 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
src/tint/resolver/address_space_layout_validation_test.cc
View File

@ -52,7 +52,7 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest, StorageBuffer_UnalignedMember)
/* offset(5) align(1) size( 4) */ b : f32;
/* offset(9) align(1) size( 3) */ // -- implicit struct size padding --;
/* */ };
78:90 note: see declaration of variable)");
78:90 note: 'S' used in address space 'storage' here)");
}
TEST_F(ResolverAddressSpaceLayoutValidationTest, StorageBuffer_UnalignedMember_SuggestedFix) {
@ -116,7 +116,7 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest, UniformBuffer_UnalignedMember_S
/* align(4) size(4) */ struct Inner {
/* offset(0) align(4) size(4) */ scalar : i32;
/* */ };
78:90 note: see declaration of variable)");
78:90 note: 'Outer' used in address space 'uniform' here)");
}
TEST_F(ResolverAddressSpaceLayoutValidationTest,
@ -182,7 +182,7 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest, UniformBuffer_UnalignedMember_A
/* offset( 0) align(4) size( 4) */ scalar : f32;
/* offset( 4) align(4) size(160) */ inner : @stride(16) array<f32, 10>;
/* */ };
78:90 note: see declaration of variable)");
78:90 note: 'Outer' used in address space 'uniform' here)");
}
TEST_F(ResolverAddressSpaceLayoutValidationTest, UniformBuffer_UnalignedMember_Array_SuggestedFix) {
@ -253,7 +253,7 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest, UniformBuffer_MembersOffsetNotM
/* align(1) size(5) */ struct Inner {
/* offset(0) align(1) size(5) */ scalar : i32;
/* */ };
22:24 note: see declaration of variable)");
22:24 note: 'Outer' used in address space 'uniform' here)");
}
// See https://crbug.com/tint/1344
@ -308,7 +308,7 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest,
/* offset(12) align(1) size( 5) */ scalar : i32;
/* offset(17) align(1) size( 3) */ // -- implicit struct size padding --;
/* */ };
22:24 note: see declaration of variable)");
22:24 note: 'Outer' used in address space 'uniform' here)");
}
TEST_F(ResolverAddressSpaceLayoutValidationTest,
@ -418,7 +418,7 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest, UniformBuffer_InvalidArrayStrid
/* offset( 0) align(4) size(40) */ inner : array<f32, 10>;
/* offset(40) align(4) size( 4) */ scalar : i32;
/* */ };
78:90 note: see declaration of variable)");
78:90 note: 'Outer' used in address space 'uniform' here)");
}
TEST_F(ResolverAddressSpaceLayoutValidationTest, UniformBuffer_InvalidArrayStride_Vector) {
@ -453,7 +453,7 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest, UniformBuffer_InvalidArrayStrid
/* offset(80) align(4) size( 4) */ scalar : i32;
/* offset(84) align(1) size( 4) */ // -- implicit struct size padding --;
/* */ };
78:90 note: see declaration of variable)");
78:90 note: 'Outer' used in address space 'uniform' here)");
}
TEST_F(ResolverAddressSpaceLayoutValidationTest, UniformBuffer_InvalidArrayStride_Struct) {
@ -495,7 +495,7 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest, UniformBuffer_InvalidArrayStrid
/* offset( 0) align(4) size(80) */ inner : array<ArrayElem, 10>;
/* offset(80) align(4) size( 4) */ scalar : i32;
/* */ };
78:90 note: see declaration of variable)");
78:90 note: 'Outer' used in address space 'uniform' here)");
}
TEST_F(ResolverAddressSpaceLayoutValidationTest, UniformBuffer_InvalidArrayStride_TopLevelArray) {
@ -507,7 +507,7 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest, UniformBuffer_InvalidArrayStrid
ASSERT_FALSE(r()->Resolve());
EXPECT_EQ(
r()->error(),
R"(34:56 error: uniform storage requires that array elements be aligned to 16 bytes, but array element alignment is currently 4. Consider using a vector or struct as the element type instead.)");
R"(78:90 error: uniform storage requires that array elements be aligned to 16 bytes, but array element alignment is currently 4. Consider using a vector or struct as the element type instead.)");
}
TEST_F(ResolverAddressSpaceLayoutValidationTest, UniformBuffer_InvalidArrayStride_NestedArray) {
@ -534,7 +534,7 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest, UniformBuffer_InvalidArrayStrid
/* align(4) size(64) */ struct Outer {
/* offset( 0) align(4) size(64) */ inner : array<array<f32, 4>, 4>;
/* */ };
78:90 note: see declaration of variable)");
78:90 note: 'Outer' used in address space 'uniform' here)");
}
TEST_F(ResolverAddressSpaceLayoutValidationTest, UniformBuffer_InvalidArrayStride_SuggestedFix) {
@ -587,7 +587,7 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest, PushConstant_UnalignedMember) {
/* offset(5) align(1) size( 4) */ b : f32;
/* offset(9) align(1) size( 3) */ // -- implicit struct size padding --;
/* */ };
78:90 note: see declaration of variable)");
78:90 note: 'S' used in address space 'push_constant' here)");
}
TEST_F(ResolverAddressSpaceLayoutValidationTest, PushConstant_Aligned) {

3
src/tint/resolver/address_space_validation_test.cc
View File

@ -472,8 +472,7 @@ TEST_F(ResolverAddressSpaceValidationTest, PushConstantF16) {
ASSERT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"56:78 error: using f16 types in 'push_constant' address space is not "
"implemented yet");
"error: using f16 types in 'push_constant' address space is not implemented yet");
}
TEST_F(ResolverAddressSpaceValidationTest, PushConstantPointer) {

134
src/tint/resolver/atomics_validation_test.cc
View File

@ -40,7 +40,7 @@ TEST_F(ResolverAtomicValidationTest, AddressSpace_Storage) {
}
TEST_F(ResolverAtomicValidationTest, AddressSpace_Storage_Struct) {
auto* s = Structure("s", utils::Vector{Member("a", ty.atomic(Source{{12, 34}}, ty.i32()))});
auto* s = Structure("s", utils::Vector{Member(Source{{12, 34}}, "a", ty.atomic(ty.i32()))});
GlobalVar("g", ty.Of(s), ast::AddressSpace::kStorage, ast::Access::kReadWrite, Group(0_a),
Binding(0_a));
@ -55,31 +55,28 @@ TEST_F(ResolverAtomicValidationTest, InvalidType) {
}
TEST_F(ResolverAtomicValidationTest, InvalidAddressSpace_Simple) {
GlobalVar("a", ty.atomic(Source{{12, 34}}, ty.i32()), ast::AddressSpace::kPrivate);
GlobalVar(Source{{12, 34}}, "a", ty.atomic(ty.i32()), ast::AddressSpace::kPrivate);
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: atomic variables must have <storage> or <workgroup> "
"address space");
"12:34 error: atomic variables must have <storage> or <workgroup> address space");
}
TEST_F(ResolverAtomicValidationTest, InvalidAddressSpace_Array) {
GlobalVar("a", ty.atomic(Source{{12, 34}}, ty.i32()), ast::AddressSpace::kPrivate);
GlobalVar(Source{{12, 34}}, "a", ty.atomic(ty.i32()), ast::AddressSpace::kPrivate);
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: atomic variables must have <storage> or <workgroup> "
"address space");
"12:34 error: atomic variables must have <storage> or <workgroup> address space");
}
TEST_F(ResolverAtomicValidationTest, InvalidAddressSpace_Struct) {
auto* s = Structure("s", utils::Vector{Member("a", ty.atomic(Source{{12, 34}}, ty.i32()))});
GlobalVar("g", ty.Of(s), ast::AddressSpace::kPrivate);
auto* s = Structure("s", utils::Vector{Member("a", ty.atomic(ty.i32()))});
GlobalVar(Source{{56, 78}}, "g", ty.Of(s), ast::AddressSpace::kPrivate);
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"error: atomic variables must have <storage> or <workgroup> "
"address space\n"
"56:78 error: atomic variables must have <storage> or <workgroup> address space\n"
"note: atomic sub-type of 's' is declared here");
}
@ -91,12 +88,11 @@ TEST_F(ResolverAtomicValidationTest, InvalidAddressSpace_StructOfStruct) {
auto* Inner =
Structure("Inner", utils::Vector{Member("m", ty.atomic(Source{{12, 34}}, ty.i32()))});
auto* Outer = Structure("Outer", utils::Vector{Member("m", ty.Of(Inner))});
GlobalVar("g", ty.Of(Outer), ast::AddressSpace::kPrivate);
GlobalVar(Source{{56, 78}}, "g", ty.Of(Outer), ast::AddressSpace::kPrivate);
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"error: atomic variables must have <storage> or <workgroup> "
"address space\n"
"56:78 error: atomic variables must have <storage> or <workgroup> address space\n"
"note: atomic sub-type of 'Outer' is declared here");
}
@ -108,13 +104,12 @@ TEST_F(ResolverAtomicValidationTest, InvalidAddressSpace_StructOfStructOfArray)
auto* Inner =
Structure("Inner", utils::Vector{Member(Source{{12, 34}}, "m", ty.atomic(ty.i32()))});
auto* Outer = Structure("Outer", utils::Vector{Member("m", ty.Of(Inner))});
GlobalVar("g", ty.Of(Outer), ast::AddressSpace::kPrivate);
GlobalVar(Source{{56, 78}}, "g", ty.Of(Outer), ast::AddressSpace::kPrivate);
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"error: atomic variables must have <storage> or <workgroup> "
"address space\n"
"12:34 note: atomic sub-type of 'Outer' is declared here");
R"(56:78 error: atomic variables must have <storage> or <workgroup> address space
12:34 note: atomic sub-type of 'Outer' is declared here)");
}
TEST_F(ResolverAtomicValidationTest, InvalidAddressSpace_ArrayOfArray) {
@ -127,8 +122,7 @@ TEST_F(ResolverAtomicValidationTest, InvalidAddressSpace_ArrayOfArray) {
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"error: atomic variables must have <storage> or <workgroup> "
"address space");
"56:78 error: atomic variables must have <storage> or <workgroup> address space");
}
TEST_F(ResolverAtomicValidationTest, InvalidAddressSpace_ArrayOfStruct) {
@ -137,14 +131,13 @@ TEST_F(ResolverAtomicValidationTest, InvalidAddressSpace_ArrayOfStruct) {
// };
// var<private> v: array<S, 5u>;
auto* s = Structure("S", utils::Vector{Member("m", ty.atomic<u32>())});
auto* s = Structure("S", utils::Vector{Member(Source{{12, 34}}, "m", ty.atomic<u32>())});
GlobalVar(Source{{56, 78}}, "v", ty.array(ty.Of(s), 5_u), ast::AddressSpace::kPrivate);
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"error: atomic variables must have <storage> or <workgroup> "
"address space\n"
"note: atomic sub-type of 'array<S, 5>' is declared here");
R"(56:78 error: atomic variables must have <storage> or <workgroup> address space
12:34 note: atomic sub-type of 'array<S, 5>' is declared here)");
}
TEST_F(ResolverAtomicValidationTest, InvalidAddressSpace_ArrayOfStructOfArray) {
@ -154,16 +147,14 @@ TEST_F(ResolverAtomicValidationTest, InvalidAddressSpace_ArrayOfStructOfArray) {
// };
// var<private> v: array<S, 5u>;
auto* atomic_array =
Alias(Source{{12, 34}}, "AtomicArray", ty.atomic(Source{{12, 34}}, ty.i32()));
auto* s = Structure("S", utils::Vector{Member("m", ty.Of(atomic_array))});
auto* atomic_array = Alias("AtomicArray", ty.atomic(ty.i32()));
auto* s = Structure("S", utils::Vector{Member(Source{{12, 34}}, "m", ty.Of(atomic_array))});
GlobalVar(Source{{56, 78}}, "v", ty.array(ty.Of(s), 5_u), ast::AddressSpace::kPrivate);
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"error: atomic variables must have <storage> or <workgroup> "
"address space\n"
"note: atomic sub-type of 'array<S, 5>' is declared here");
R"(56:78 error: atomic variables must have <storage> or <workgroup> address space
12:34 note: atomic sub-type of 'array<S, 5>' is declared here)");
}
TEST_F(ResolverAtomicValidationTest, InvalidAddressSpace_Complex) {
@ -181,19 +172,18 @@ TEST_F(ResolverAtomicValidationTest, InvalidAddressSpace_Complex) {
// struct S0 { x: S1; };
// var<private> g : S0;
auto* atomic_array =
Alias(Source{{12, 34}}, "AtomicArray", ty.atomic(Source{{12, 34}}, ty.i32()));
auto* atomic_array = Alias("AtomicArray", ty.atomic(ty.i32()));
auto* array_i32_4 = ty.array(ty.i32(), 4_u);
auto* array_atomic_u32_8 = ty.array(ty.atomic(ty.u32()), 8_u);
auto* array_atomic_i32_4 = ty.array(ty.atomic(ty.i32()), 4_u);
auto* s6 = Structure("S6", utils::Vector{Member("x", array_i32_4)});
auto* s5 = Structure("S5", utils::Vector{Member("x", ty.Of(s6)), //
Member("y", ty.Of(atomic_array)), //
Member("z", array_atomic_u32_8)}); //
auto* s4 = Structure("S4", utils::Vector{Member("x", ty.Of(s6)), //
Member("y", ty.Of(s5)), //
Member("z", array_atomic_i32_4)}); //
auto* s5 = Structure("S5", utils::Vector{Member("x", ty.Of(s6)), //
Member(Source{{12, 34}}, "y", ty.Of(atomic_array)), //
Member("z", array_atomic_u32_8)}); //
auto* s4 = Structure("S4", utils::Vector{Member("x", ty.Of(s6)), //
Member("y", ty.Of(s5)), //
Member("z", array_atomic_i32_4)}); //
auto* s3 = Structure("S3", utils::Vector{Member("x", ty.Of(s4))});
auto* s2 = Structure("S2", utils::Vector{Member("x", ty.Of(s3))});
auto* s1 = Structure("S1", utils::Vector{Member("x", ty.Of(s2))});
@ -202,33 +192,32 @@ TEST_F(ResolverAtomicValidationTest, InvalidAddressSpace_Complex) {
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"error: atomic variables must have <storage> or <workgroup> "
"address space\n"
"note: atomic sub-type of 'S0' is declared here");
R"(56:78 error: atomic variables must have <storage> or <workgroup> address space
12:34 note: atomic sub-type of 'S0' is declared here)");
}
TEST_F(ResolverAtomicValidationTest, Struct_AccessMode_Read) {
auto* s = Structure("s", utils::Vector{Member("a", ty.atomic(Source{{12, 34}}, ty.i32()))});
auto* s = Structure("s", utils::Vector{Member(Source{{12, 34}}, "a", ty.atomic(ty.i32()))});
GlobalVar(Source{{56, 78}}, "g", ty.Of(s), ast::AddressSpace::kStorage, ast::Access::kRead,
Group(0_a), Binding(0_a));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"error: atomic variables in <storage> address space must have read_write "
"access mode\n"
"note: atomic sub-type of 's' is declared here");
EXPECT_EQ(
r()->error(),
R"(56:78 error: atomic variables in <storage> address space must have read_write access mode
12:34 note: atomic sub-type of 's' is declared here)");
}
TEST_F(ResolverAtomicValidationTest, InvalidAccessMode_Struct) {
auto* s = Structure("s", utils::Vector{Member("a", ty.atomic(Source{{12, 34}}, ty.i32()))});
auto* s = Structure("s", utils::Vector{Member(Source{{12, 34}}, "a", ty.atomic(ty.i32()))});
GlobalVar(Source{{56, 78}}, "g", ty.Of(s), ast::AddressSpace::kStorage, ast::Access::kRead,
Group(0_a), Binding(0_a));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"error: atomic variables in <storage> address space must have read_write "
"access mode\n"
"note: atomic sub-type of 's' is declared here");
EXPECT_EQ(
r()->error(),
R"(56:78 error: atomic variables in <storage> address space must have read_write access mode
12:34 note: atomic sub-type of 's' is declared here)");
}
TEST_F(ResolverAtomicValidationTest, InvalidAccessMode_StructOfStruct) {
@ -237,16 +226,16 @@ TEST_F(ResolverAtomicValidationTest, InvalidAccessMode_StructOfStruct) {
// var<storage, read> g : Outer;
auto* Inner =
Structure("Inner", utils::Vector{Member("m", ty.atomic(Source{{12, 34}}, ty.i32()))});
Structure("Inner", utils::Vector{Member(Source{{12, 34}}, "m", ty.atomic(ty.i32()))});
auto* Outer = Structure("Outer", utils::Vector{Member("m", ty.Of(Inner))});
GlobalVar(Source{{56, 78}}, "g", ty.Of(Outer), ast::AddressSpace::kStorage, ast::Access::kRead,
Group(0_a), Binding(0_a));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"error: atomic variables in <storage> address space must have read_write "
"access mode\n"
"note: atomic sub-type of 'Outer' is declared here");
EXPECT_EQ(
r()->error(),
R"(56:78 error: atomic variables in <storage> address space must have read_write access mode
12:34 note: atomic sub-type of 'Outer' is declared here)");
}
TEST_F(ResolverAtomicValidationTest, InvalidAccessMode_StructOfStructOfArray) {
@ -261,10 +250,10 @@ TEST_F(ResolverAtomicValidationTest, InvalidAccessMode_StructOfStructOfArray) {
Group(0_a), Binding(0_a));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"error: atomic variables in <storage> address space must have "
"read_write access mode\n"
"12:34 note: atomic sub-type of 'Outer' is declared here");
EXPECT_EQ(
r()->error(),
R"(56:78 error: atomic variables in <storage> address space must have read_write access mode
12:34 note: atomic sub-type of 'Outer' is declared here)");
}
TEST_F(ResolverAtomicValidationTest, InvalidAccessMode_Complex) {
@ -282,31 +271,30 @@ TEST_F(ResolverAtomicValidationTest, InvalidAccessMode_Complex) {
// struct S0 { x: S1; };
// var<storage, read> g : S0;
auto* atomic_array =
Alias(Source{{12, 34}}, "AtomicArray", ty.atomic(Source{{12, 34}}, ty.i32()));
auto* atomic_array = Alias("AtomicArray", ty.atomic(ty.i32()));
auto* array_i32_4 = ty.array(ty.i32(), 4_u);
auto* array_atomic_u32_8 = ty.array(ty.atomic(ty.u32()), 8_u);
auto* array_atomic_i32_4 = ty.array(ty.atomic(ty.i32()), 4_u);
auto* s6 = Structure("S6", utils::Vector{Member("x", array_i32_4)});
auto* s5 = Structure("S5", utils::Vector{Member("x", ty.Of(s6)), //
Member("y", ty.Of(atomic_array)), //
Member("z", array_atomic_u32_8)}); //
auto* s4 = Structure("S4", utils::Vector{Member("x", ty.Of(s6)), //
Member("y", ty.Of(s5)), //
Member("z", array_atomic_i32_4)}); //
auto* s5 = Structure("S5", utils::Vector{Member("x", ty.Of(s6)), //
Member(Source{{56, 78}}, "y", ty.Of(atomic_array)), //
Member("z", array_atomic_u32_8)}); //
auto* s4 = Structure("S4", utils::Vector{Member("x", ty.Of(s6)), //
Member("y", ty.Of(s5)), //
Member("z", array_atomic_i32_4)}); //
auto* s3 = Structure("S3", utils::Vector{Member("x", ty.Of(s4))});
auto* s2 = Structure("S2", utils::Vector{Member("x", ty.Of(s3))});
auto* s1 = Structure("S1", utils::Vector{Member("x", ty.Of(s2))});
auto* s0 = Structure("S0", utils::Vector{Member("x", ty.Of(s1))});
GlobalVar(Source{{56, 78}}, "g", ty.Of(s0), ast::AddressSpace::kStorage, ast::Access::kRead,
GlobalVar(Source{{12, 34}}, "g", ty.Of(s0), ast::AddressSpace::kStorage, ast::Access::kRead,
Group(0_a), Binding(0_a));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"error: atomic variables in <storage> address space must have "
"read_write access mode\n"
"note: atomic sub-type of 'S0' is declared here");
EXPECT_EQ(
r()->error(),
R"(12:34 error: atomic variables in <storage> address space must have read_write access mode
56:78 note: atomic sub-type of 'S0' is declared here)");
}
TEST_F(ResolverAtomicValidationTest, Local) {

16
src/tint/resolver/resolver.cc
View File

@ -103,7 +103,11 @@ Resolver::Resolver(ProgramBuilder* builder)
const_eval_(*builder),
intrinsic_table_(IntrinsicTable::Create(*builder)),
sem_(builder, dependencies_),
validator_(builder, sem_) {}
validator_(builder,
sem_,
enabled_extensions_,
atomic_composite_info_,
valid_type_storage_layouts_) {}
Resolver::~Resolver() = default;
@ -891,13 +895,7 @@ sem::GlobalVariable* Resolver::GlobalVariable(const ast::Variable* v) {
return nullptr;
}
if (!validator_.GlobalVariable(sem, override_ids_, atomic_composite_info_)) {
return nullptr;
}
// TODO(bclayton): Call this at the end of resolve on all uniform and storage
// referenced structs
if (!validator_.AddressSpaceLayout(sem, enabled_extensions_, valid_type_storage_layouts_)) {
if (!validator_.GlobalVariable(sem, override_ids_)) {
return nullptr;
}
@ -2300,7 +2298,7 @@ sem::Call* Resolver::BuiltinCall(const ast::CallExpression* expr,
current_function_->AddDirectCall(call);
}
if (!validator_.RequiredExtensionForBuiltinFunction(call, enabled_extensions_)) {
if (!validator_.RequiredExtensionForBuiltinFunction(call)) {
return nullptr;
}

3
src/tint/resolver/resolver.h
View File

@ -111,8 +111,6 @@ class Resolver {
const Validator* GetValidatorForTesting() const { return &validator_; }
private:
Validator::ValidTypeStorageLayouts valid_type_storage_layouts_;
/// Resolves the program, without creating final the semantic nodes.
/// @returns true on success, false on error
bool ResolveInternal();
@ -471,6 +469,7 @@ class Resolver {
sem::CompoundStatement* current_compound_statement_ = nullptr;
uint32_t current_scoping_depth_ = 0;
utils::UniqueVector<const sem::GlobalVariable*, 4>* resolved_overrides_ = nullptr;
utils::Hashset<TypeAndAddressSpace, 8> valid_type_storage_layouts_;
};
} // namespace tint::resolver

264
src/tint/resolver/validator.cc
View File

@ -153,8 +153,18 @@ void TraverseCallChain(diag::List& diagnostics,
} // namespace
Validator::Validator(ProgramBuilder* builder, SemHelper& sem)
: symbols_(builder->Symbols()), diagnostics_(builder->Diagnostics()), sem_(sem) {}
Validator::Validator(
ProgramBuilder* builder,
SemHelper& sem,
const ast::Extensions& enabled_extensions,
const utils::Hashmap<const sem::Type*, const Source*, 8>& atomic_composite_info,
utils::Hashset<TypeAndAddressSpace, 8>& valid_type_storage_layouts)
: symbols_(builder->Symbols()),
diagnostics_(builder->Diagnostics()),
sem_(sem),
enabled_extensions_(enabled_extensions),
atomic_composite_info_(atomic_composite_info),
valid_type_storage_layouts_(valid_type_storage_layouts) {}
Validator::~Validator() = default;
@ -360,8 +370,7 @@ bool Validator::VariableInitializer(const ast::Variable* v,
bool Validator::AddressSpaceLayout(const sem::Type* store_ty,
ast::AddressSpace address_space,
Source source,
ValidTypeStorageLayouts& layouts) const {
Source source) const {
// https://gpuweb.github.io/gpuweb/wgsl/#storage-class-layout-constraints
auto is_uniform_struct_or_array = [address_space](const sem::Type* ty) {
@ -386,8 +395,8 @@ bool Validator::AddressSpaceLayout(const sem::Type* store_ty,
return symbols_.NameFor(sm->Declaration()->symbol);
};
// Cache result of type + address space pair.
if (!layouts.emplace(store_ty, address_space).second) {
// Only validate the [type + address space] once
if (!valid_type_storage_layouts_.Add(TypeAndAddressSpace{store_ty, address_space})) {
return true;
}
@ -395,6 +404,12 @@ bool Validator::AddressSpaceLayout(const sem::Type* store_ty,
return true;
}
auto note_usage = [&] {
AddNote("'" + store_ty->FriendlyName(symbols_) + "' used in address space '" +
utils::ToString(address_space) + "' here",
source);
};
// Among three host-shareable address spaces, f16 is supported in "uniform" and
// "storage" address space, but not "push_constant" address space yet.
if (Is<sem::F16>(sem::Type::DeepestElementOf(store_ty)) &&
@ -409,10 +424,10 @@ bool Validator::AddressSpaceLayout(const sem::Type* store_ty,
uint32_t required_align = required_alignment_of(m->Type());
// Recurse into the member type.
if (!AddressSpaceLayout(m->Type(), address_space, m->Declaration()->type->source,
layouts)) {
if (!AddressSpaceLayout(m->Type(), address_space, m->Declaration()->type->source)) {
AddNote("see layout of struct:\n" + str->Layout(symbols_),
str->Declaration()->source);
note_usage();
return false;
}
@ -435,6 +450,7 @@ bool Validator::AddressSpaceLayout(const sem::Type* store_ty,
member_str->Declaration()->source);
}
note_usage();
return false;
}
@ -460,6 +476,7 @@ bool Validator::AddressSpaceLayout(const sem::Type* store_ty,
AddNote("and layout of previous member struct:\n" +
prev_member_str->Layout(symbols_),
prev_member_str->Declaration()->source);
note_usage();
return false;
}
}
@ -472,7 +489,7 @@ bool Validator::AddressSpaceLayout(const sem::Type* store_ty,
// TODO(crbug.com/tint/1388): Ideally we'd pass the source for nested element type here, but
// we can't easily get that from the semantic node. We should consider recursing through the
// AST type nodes instead.
if (!AddressSpaceLayout(arr->ElemType(), address_space, source, layouts)) {
if (!AddressSpaceLayout(arr->ElemType(), address_space, source)) {
return false;
}
@ -484,21 +501,16 @@ bool Validator::AddressSpaceLayout(const sem::Type* store_ty,
// shader author can resolve the issue.
std::string hint;
if (arr->ElemType()->is_scalar()) {
hint =
"Consider using a vector or struct as the element type "
"instead.";
hint = "Consider using a vector or struct as the element type instead.";
} else if (auto* vec = arr->ElemType()->As<sem::Vector>();
vec && vec->type()->Size() == 4) {
hint = "Consider using a vec4 instead.";
} else if (arr->ElemType()->Is<sem::Struct>()) {
hint =
"Consider using the @size attribute on the last struct "
"member.";
hint = "Consider using the @size attribute on the last struct member.";
} else {
hint =
"Consider wrapping the element type in a struct and using "
"the "
"@size attribute.";
"Consider wrapping the element type in a struct and using the @size "
"attribute.";
}
AddError(
"uniform storage requires that array elements be aligned to 16 "
@ -513,42 +525,6 @@ bool Validator::AddressSpaceLayout(const sem::Type* store_ty,
return true;
}
bool Validator::AddressSpaceLayout(const sem::Variable* var,
const ast::Extensions& enabled_extensions,
ValidTypeStorageLayouts& layouts) const {
if (var->AddressSpace() == ast::AddressSpace::kPushConstant &&
!enabled_extensions.Contains(ast::Extension::kChromiumExperimentalPushConstant) &&
IsValidationEnabled(var->Declaration()->attributes,
ast::DisabledValidation::kIgnoreAddressSpace)) {
AddError(
"use of variable address space 'push_constant' requires enabling extension "
"'chromium_experimental_push_constant'",
var->Declaration()->source);
return false;
}
if (auto* str = var->Type()->UnwrapRef()->As<sem::Struct>()) {
// Check the structure has a declaration. Builtins like modf() and frexp() return untypeable
// structures, and so they have no declaration. Just skip validation for these.
if (auto* str_decl = str->Declaration()) {
if (!AddressSpaceLayout(str, var->AddressSpace(), str_decl->source, layouts)) {
AddNote("see declaration of variable", var->Declaration()->source);
return false;
}
}
} else {
Source source = var->Declaration()->source;
if (var->Declaration()->type) {
source = var->Declaration()->type->source;
}
if (!AddressSpaceLayout(var->Type()->UnwrapRef(), var->AddressSpace(), source, layouts)) {
return false;
}
}
return true;
}
bool Validator::LocalVariable(const sem::Variable* local) const {
auto* decl = local->Declaration();
if (IsArrayWithOverrideCount(local->Type())) {
@ -581,8 +557,7 @@ bool Validator::LocalVariable(const sem::Variable* local) const {
bool Validator::GlobalVariable(
const sem::GlobalVariable* global,
const utils::Hashmap<OverrideId, const sem::Variable*, 8>& override_ids,
const utils::Hashmap<const sem::Type*, const Source*, 8>& atomic_composite_info) const {
const utils::Hashmap<OverrideId, const sem::Variable*, 8>& override_ids) const {
auto* decl = global->Declaration();
if (global->AddressSpace() != ast::AddressSpace::kWorkgroup &&
IsArrayWithOverrideCount(global->Type())) {
@ -592,7 +567,7 @@ bool Validator::GlobalVariable(
}
bool ok = Switch(
decl, //
[&](const ast::Var* var) {
[&](const ast::Var*) {
if (auto* init = global->Initializer();
init && init->Stage() > sem::EvaluationStage::kOverride) {
AddError("module-scope 'var' initializer must be a constant or override-expression",
@ -620,29 +595,6 @@ bool Validator::GlobalVariable(
}
}
// https://gpuweb.github.io/gpuweb/wgsl/#variable-declaration
// The access mode always has a default, and except for variables in the storage address
// space, must not be written.
if (var->declared_access != ast::Access::kUndefined) {
if (global->AddressSpace() == ast::AddressSpace::kStorage) {
// The access mode for the storage address space can only be 'read' or
// 'read_write'.
if (var->declared_access == ast::Access::kWrite) {
AddError("access mode 'write' is not valid for the 'storage' address space",
decl->source);
return false;
}
} else {
AddError("only variables in <storage> address space may declare an access mode",
decl->source);
return false;
}
}
if (!AtomicVariable(global, atomic_composite_info)) {
return false;
}
return Var(global);
},
[&](const ast::Override*) { return Override(global, override_ids); },
@ -698,65 +650,40 @@ bool Validator::GlobalVariable(
return true;
}
// https://gpuweb.github.io/gpuweb/wgsl/#atomic-types
// Atomic types may only be instantiated by variables in the workgroup storage class or by storage
// buffer variables with a read_write access mode.
bool Validator::AtomicVariable(
const sem::Variable* var,
const utils::Hashmap<const sem::Type*, const Source*, 8>& atomic_composite_info) const {
auto address_space = var->AddressSpace();
auto* decl = var->Declaration();
auto access = var->Access();
auto* type = var->Type()->UnwrapRef();
auto source = decl->type ? decl->type->source : decl->source;
if (type->Is<sem::Atomic>()) {
if (address_space != ast::AddressSpace::kWorkgroup &&
address_space != ast::AddressSpace::kStorage) {
AddError("atomic variables must have <storage> or <workgroup> address space", source);
return false;
}
} else if (type->IsAnyOf<sem::Struct, sem::Array>()) {
if (auto found = atomic_composite_info.Find(type)) {
if (address_space != ast::AddressSpace::kStorage &&
address_space != ast::AddressSpace::kWorkgroup) {
AddError("atomic variables must have <storage> or <workgroup> address space",
source);
AddNote("atomic sub-type of '" + sem_.TypeNameOf(type) + "' is declared here",
**found);
return false;
} else if (address_space == ast::AddressSpace::kStorage &&
access != ast::Access::kReadWrite) {
AddError(
"atomic variables in <storage> address space must have read_write "
"access mode",
source);
AddNote("atomic sub-type of '" + sem_.TypeNameOf(type) + "' is declared here",
**found);
return false;
}
}
}
return true;
}
bool Validator::Var(const sem::Variable* v) const {
auto* var = v->Declaration()->As<ast::Var>();
auto* storage_ty = v->Type()->UnwrapRef();
auto* store_ty = v->Type()->UnwrapRef();
if (!IsStorable(storage_ty)) {
AddError(sem_.TypeNameOf(storage_ty) + " cannot be used as the type of a var", var->source);
if (!IsStorable(store_ty)) {
AddError(sem_.TypeNameOf(store_ty) + " cannot be used as the type of a var", var->source);
return false;
}
if (storage_ty->is_handle() && var->declared_address_space != ast::AddressSpace::kNone) {
// https://gpuweb.github.io/gpuweb/wgsl/#module-scope-variables
// If the store type is a texture type or a sampler type, then the variable declaration must
// not have a address space attribute. The address space will always be handle.
AddError(
"variables of type '" + sem_.TypeNameOf(storage_ty) + "' must not have a address space",
var->source);
if (store_ty->is_handle()) {
if (var->declared_address_space != ast::AddressSpace::kNone) {
// https://gpuweb.github.io/gpuweb/wgsl/#module-scope-variables
// If the store type is a texture type or a sampler type, then the variable declaration
// must not have a address space attribute. The address space will always be handle.
AddError("variables of type '" + sem_.TypeNameOf(store_ty) +
"' must not have a address space",
var->source);
return false;
}
}
if (var->declared_access != ast::Access::kUndefined) {
// https://gpuweb.github.io/gpuweb/wgsl/#variable-declaration
// The access mode always has a default, and except for variables in the storage address
// space, must not be written.
if (var->declared_address_space != ast::AddressSpace::kStorage) {
AddError("only variables in <storage> address space may declare an access mode",
var->source);
return false;
}
}
if (!CheckTypeAccessAddressSpace(v->Type()->UnwrapRef(), v->Access(), v->AddressSpace(),
var->attributes, var->source)) {
return false;
}
@ -1642,9 +1569,7 @@ bool Validator::TextureBuiltinFunction(const sem::Call* call) const {
check_arg_is_constexpr(sem::ParameterUsage::kComponent, 0, 3);
}
bool Validator::RequiredExtensionForBuiltinFunction(
const sem::Call* call,
const ast::Extensions& enabled_extensions) const {
bool Validator::RequiredExtensionForBuiltinFunction(const sem::Call* call) const {
const auto* builtin = call->Target()->As<sem::Builtin>();
if (!builtin) {
return true;
@ -1655,7 +1580,7 @@ bool Validator::RequiredExtensionForBuiltinFunction(
return true;
}
if (!enabled_extensions.Contains(extension)) {
if (!enabled_extensions_.Contains(extension)) {
AddError("cannot call built-in function '" + std::string(builtin->str()) +
"' without extension " + utils::ToString(extension),
call->Declaration()->source);
@ -2451,4 +2376,69 @@ std::string Validator::VectorPretty(uint32_t size, const sem::Type* element_type
return vec_type.FriendlyName(symbols_);
}
bool Validator::CheckTypeAccessAddressSpace(
const sem::Type* store_ty,
ast::Access access,
ast::AddressSpace address_space,
const utils::VectorRef<const tint::ast::Attribute*> attributes,
const Source& source) const {
if (!AddressSpaceLayout(store_ty, address_space, source)) {
return false;
}
if (address_space == ast::AddressSpace::kPushConstant &&
!enabled_extensions_.Contains(ast::Extension::kChromiumExperimentalPushConstant) &&
IsValidationEnabled(attributes, ast::DisabledValidation::kIgnoreAddressSpace)) {
AddError(
"use of variable address space 'push_constant' requires enabling extension "
"'chromium_experimental_push_constant'",
source);
return false;
}
if (address_space == ast::AddressSpace::kStorage && access == ast::Access::kWrite) {
// The access mode for the storage address space can only be 'read' or
// 'read_write'.
AddError("access mode 'write' is not valid for the 'storage' address space", source);
return false;
}
auto atomic_error = [&]() -> const char* {
if (address_space != ast::AddressSpace::kStorage &&
address_space != ast::AddressSpace::kWorkgroup) {
return "atomic variables must have <storage> or <workgroup> address space";
}
if (address_space == ast::AddressSpace::kStorage && access != ast::Access::kReadWrite) {
return "atomic variables in <storage> address space must have read_write access "
"mode";
}
return nullptr;
};
auto check_sub_atomics = [&] {
if (auto atomic_use = atomic_composite_info_.Get(store_ty)) {
if (auto* err = atomic_error()) {
AddError(err, source);
AddNote("atomic sub-type of '" + sem_.TypeNameOf(store_ty) + "' is declared here",
**atomic_use);
return false;
}
}
return true;
};
return Switch(
store_ty, //
[&](const sem::Atomic*) {
if (auto* err = atomic_error()) {
AddError(err, source);
return false;
}
return true;
},
[&](const sem::Struct*) { return check_sub_atomics(); }, //
[&](const sem::Array*) { return check_sub_atomics(); }, //
[&](Default) { return true; });
}
} // namespace tint::resolver

93
src/tint/resolver/validator.h
View File

@ -24,6 +24,7 @@
#include "src/tint/resolver/sem_helper.h"
#include "src/tint/sem/evaluation_stage.h"
#include "src/tint/source.h"
#include "src/tint/utils/hash.h"
#include "src/tint/utils/hashmap.h"
#include "src/tint/utils/vector.h"
@ -66,19 +67,38 @@ class WhileStatement;
namespace tint::resolver {
/// TypeAndAddressSpace is a pair of type and address space
struct TypeAndAddressSpace {
/// The type
const sem::Type* type;
/// The address space
ast::AddressSpace address_space;
/// Equality operator
/// @param other the other TypeAndAddressSpace to compare this TypeAndAddressSpace to
/// @returns true if the type and address space of this TypeAndAddressSpace is equal to @p other
bool operator==(const TypeAndAddressSpace& other) const {
return type == other.type && address_space == other.address_space;
}
};
/// Validation logic for various ast nodes. The validations in general should
/// be shallow and depend on the resolver to call on children. The validations
/// also assume that sem changes have already been made. The validation checks
/// should not alter the AST or SEM trees.
class Validator {
public:
/// The valid type storage layouts typedef
using ValidTypeStorageLayouts = std::set<std::pair<const sem::Type*, ast::AddressSpace>>;
/// Constructor
/// @param builder the program builder
/// @param helper the SEM helper to validate with
Validator(ProgramBuilder* builder, SemHelper& helper);
/// @param enabled_extensions all the extensions declared in current module
/// @param atomic_composite_info atomic composite info of the module
/// @param valid_type_storage_layouts a set of validated type layouts by address space
Validator(ProgramBuilder* builder,
SemHelper& helper,
const ast::Extensions& enabled_extensions,
const utils::Hashmap<const sem::Type*, const Source*, 8>& atomic_composite_info,
utils::Hashset<TypeAndAddressSpace, 8>& valid_type_storage_layouts);
~Validator();
/// Adds the given error message to the diagnostics
@ -143,20 +163,12 @@ class Validator {
uint32_t el_size,
uint32_t el_align) const;
/// Validates an atomic
/// @param a the atomic ast node to validate
/// Validates an atomic type
/// @param a the atomic ast node
/// @param s the atomic sem node
/// @returns true on success, false otherwise.
bool Atomic(const ast::Atomic* a, const sem::Atomic* s) const;
/// Validates an atoic variable
/// @param var the variable to validate
/// @param atomic_composite_info store atomic information
/// @returns true on success, false otherwise.
bool AtomicVariable(
const sem::Variable* var,
const utils::Hashmap<const sem::Type*, const Source*, 8>& atomic_composite_info) const;
/// Validates an assignment
/// @param a the assignment statement
/// @param rhs_ty the type of the right hand side
@ -238,12 +250,10 @@ class Validator {
/// Validates a global variable
/// @param var the global variable to validate
/// @param override_id the set of override ids in the module
/// @param atomic_composite_info atomic composite info in the module
/// @returns true on success, false otherwise
bool GlobalVariable(
const sem::GlobalVariable* var,
const utils::Hashmap<OverrideId, const sem::Variable*, 8>& override_id,
const utils::Hashmap<const sem::Type*, const Source*, 8>& atomic_composite_info) const;
const utils::Hashmap<OverrideId, const sem::Variable*, 8>& override_id) const;
/// Validates a break-if statement
/// @param stmt the statement to validate
@ -423,10 +433,8 @@ class Validator {
/// Validates an optional builtin function and its required extension.
/// @param call the builtin call to validate
/// @param enabled_extensions all the extensions declared in current module
/// @returns true on success, false otherwise
bool RequiredExtensionForBuiltinFunction(const sem::Call* call,
const ast::Extensions& enabled_extensions) const;
bool RequiredExtensionForBuiltinFunction(const sem::Call* call) const;
/// Validates there are no duplicate attributes
/// @param attributes the list of attributes to validate
@ -437,21 +445,8 @@ class Validator {
/// @param type the type to validate
/// @param sc the address space
/// @param source the source of the type
/// @param layouts previously validated storage layouts
/// @returns true on success, false otherwise
bool AddressSpaceLayout(const sem::Type* type,
ast::AddressSpace sc,
Source source,
ValidTypeStorageLayouts& layouts) const;
/// Validates a address space layout
/// @param var the variable to validate
/// @param layouts previously validated storage layouts
/// @param enabled_extensions all the extensions declared in current module
/// @returns true on success, false otherwise.
bool AddressSpaceLayout(const sem::Variable* var,
const ast::Extensions& enabled_extensions,
ValidTypeStorageLayouts& layouts) const;
bool AddressSpaceLayout(const sem::Type* type, ast::AddressSpace sc, Source source) const;
/// @returns true if the attribute list contains a
/// ast::DisableValidationAttribute with the validation mode equal to
@ -497,11 +492,41 @@ class Validator {
/// @return pretty string representation
std::string VectorPretty(uint32_t size, const sem::Type* element_type) const;
/// Raises an error if combination of @p store_ty, @p access and @p address_space are not valid
/// for a `var` or `ptr` declaration.
/// @param store_ty the store type of the var or pointer
/// @param access the var or pointer access
/// @param address_space the var or pointer address space
/// @param source the source for the error
/// @returns true on success, false if an error was raised.
bool CheckTypeAccessAddressSpace(const sem::Type* store_ty,
ast::Access access,
ast::AddressSpace address_space,
const utils::VectorRef<const tint::ast::Attribute*> attributes,
const Source& source) const;
SymbolTable& symbols_;
diag::List& diagnostics_;
SemHelper& sem_;
const ast::Extensions& enabled_extensions_;
const utils::Hashmap<const sem::Type*, const Source*, 8>& atomic_composite_info_;
utils::Hashset<TypeAndAddressSpace, 8>& valid_type_storage_layouts_;
};
} // namespace tint::resolver
namespace std {
/// Custom std::hash specialization for tint::resolver::TypeAndAddressSpace.
template <>
class hash<tint::resolver::TypeAndAddressSpace> {
public:
/// @param tas the TypeAndAddressSpace
/// @return the hash value
inline std::size_t operator()(const tint::resolver::TypeAndAddressSpace& tas) const {
return tint::utils::Hash(tas.type, tas.address_space);
}
};
} // namespace std
#endif // SRC_TINT_RESOLVER_VALIDATOR_H_