Tint: Add vertex validation for storage buffers and textures
Add validation checks for use of storage buffers and textures with an
access mode other than read in a vertex shader stage.
Relevant spec change: https://github.com/gpuweb/gpuweb/pull/3947
This fixes the currently failing CTS tests:
webgpu:shader,validation,decl,var:shader_stage:stage="vertex";kind="handle_rw"
webgpu:shader,validation,decl,var:shader_stage:stage="vertex";kind="handle_wo"
webgpu:shader,validation,decl,var:shader_stage:stage="vertex";kind="storage_rw"
This change depends on many fixes to existing tests tracked in crbug.com/344846829
Change-Id: Ie51f8efeb5cd7e1c2097413e0bf02d15ce87ec02
Bug: 344846829
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/190781
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Natalie Chouinard <chouinard@google.com>
diff --git a/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp b/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp
index 46594be..923fec7 100644
--- a/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp
@@ -1203,9 +1203,9 @@
struct Dst {
data : array<u32, 100>
}
- @group(0) @binding(0) var<storage, read_write> dst : Dst;
+ @group(0) @binding(0) var<storage, read> dst : Dst;
@vertex fn main(@builtin(vertex_index) VertexIndex : u32) -> @builtin(position) vec4f {
- dst.data[VertexIndex] = 0x1234u;
+ var val = dst.data[VertexIndex];
return vec4f();
})");
@@ -1213,7 +1213,7 @@
descriptor.layout = nullptr;
descriptor.vertex.module = vsModuleWithStorageBuffer;
descriptor.cFragment.module = fsModule;
- ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor));
+ device.CreateRenderPipeline(&descriptor);
}
// Tests that only strip primitive topologies allow an index format
diff --git a/src/tint/lang/wgsl/resolver/validation_test.cc b/src/tint/lang/wgsl/resolver/validation_test.cc
index c0b1f87..c43efd0 100644
--- a/src/tint/lang/wgsl/resolver/validation_test.cc
+++ b/src/tint/lang/wgsl/resolver/validation_test.cc
@@ -133,6 +133,57 @@
9:10 note: called by entry point 'f0')");
}
+TEST_F(ResolverValidationTest, RWStorageBufferUsedInVertexStage) {
+ // @group(0) @binding(0) var<storage, read_write> v : vec4<f32>;
+ // @vertex
+ // fn main() -> @builtin(position) vec4f {
+ // return v;
+ // }
+
+ GlobalVar(Source{{1, 2}}, "v", ty.vec4<f32>(), core::AddressSpace::kStorage,
+ core::Access::kReadWrite, Binding(0_a), Group(0_a));
+ Func("main", tint::Empty, ty.vec4<f32>(),
+ Vector{
+ Return(Expr(Source{{3, 4}}, "v")),
+ },
+ Vector{
+ Stage(ast::PipelineStage::kVertex),
+ },
+ Vector{
+ Builtin(core::BuiltinValue::kPosition),
+ });
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(3:4 error: var with 'storage' address space and 'read_write' access mode cannot be used by vertex pipeline stage
+1:2 note: variable is declared here)");
+}
+
+TEST_F(ResolverValidationTest, RWStorageTextureUsedInVertexStage) {
+ auto type = ty.storage_texture(core::type::TextureDimension::k2d, core::TexelFormat::kR32Uint,
+ core::Access::kReadWrite);
+ GlobalVar(Source{{1, 2}}, "v", type, Binding(0_a), Group(0_a));
+ GlobalVar("res", ty.vec4<f32>(), core::AddressSpace::kPrivate);
+ Func("main", tint::Empty, ty.vec4<f32>(),
+ Vector{
+ Assign(Phony(), Expr(Source{{3, 4}}, "v")),
+ Return(Expr(Source{{3, 4}}, "res")),
+ },
+ Vector{
+ Stage(ast::PipelineStage::kVertex),
+ },
+ Vector{
+ Builtin(core::BuiltinValue::kPosition),
+ });
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(3:4 error: storage texture with 'read_write' access mode cannot be used by vertex pipeline stage
+1:2 note: variable is declared here)");
+}
+
TEST_F(ResolverValidationDeathTest, UnhandledStmt) {
EXPECT_DEATH_IF_SUPPORTED(
{
diff --git a/src/tint/lang/wgsl/resolver/validator.cc b/src/tint/lang/wgsl/resolver/validator.cc
index cfd005d..c5def01 100644
--- a/src/tint/lang/wgsl/resolver/validator.cc
+++ b/src/tint/lang/wgsl/resolver/validator.cc
@@ -2164,6 +2164,15 @@
}
bool Validator::PipelineStages(VectorRef<sem::Function*> entry_points) const {
+ auto var_source = [&](const sem::Function* func, const sem::GlobalVariable* var) {
+ for (auto* user : var->Users()) {
+ if (user->Stmt() && func == user->Stmt()->Function()) {
+ return user->Declaration()->source;
+ }
+ }
+ return Source();
+ };
+
auto backtrace = [&](const sem::Function* func, const sem::Function* entry_point) {
if (func != entry_point) {
TraverseCallChain(entry_point, func, [&](const sem::Function* f) {
@@ -2178,30 +2187,40 @@
};
auto check_var_uses = [&](const sem::Function* func, const sem::Function* entry_point) {
- auto err = [&](ast::PipelineStage stage, const sem::GlobalVariable* var) {
- Source source;
- for (auto* user : var->Users()) {
- if (func == user->Stmt()->Function()) {
- source = user->Declaration()->source;
- break;
- }
- }
- AddError(source) << "var with " << style::Enum(var->AddressSpace())
- << " address space cannot be used by " << stage << " pipeline stage";
+ auto var_decl_note = [&](const sem::GlobalVariable* var) {
AddNote(var->Declaration()->source) << "variable is declared here";
backtrace(func, entry_point);
- return false;
};
auto stage = entry_point->Declaration()->PipelineStage();
for (auto* var : func->DirectlyReferencedGlobals()) {
- if (stage != ast::PipelineStage::kCompute &&
- var->AddressSpace() == core::AddressSpace::kWorkgroup) {
- return err(stage, var);
+ Source source = var_source(func, var);
+ if ((stage != ast::PipelineStage::kCompute &&
+ var->AddressSpace() == core::AddressSpace::kWorkgroup) ||
+ (stage != ast::PipelineStage::kFragment &&
+ var->AddressSpace() == core::AddressSpace::kPixelLocal)) {
+ AddError(source) << "var with " << style::Enum(var->AddressSpace())
+ << " address space cannot be used by " << stage
+ << " pipeline stage";
+ var_decl_note(var);
+ return false;
}
- if (stage != ast::PipelineStage::kFragment &&
- var->AddressSpace() == core::AddressSpace::kPixelLocal) {
- return err(stage, var);
+ if (stage == ast::PipelineStage::kVertex &&
+ var->AddressSpace() == core::AddressSpace::kStorage &&
+ var->Access() != core::Access::kRead) {
+ AddError(source) << "var with " << style::Enum(var->AddressSpace())
+ << " address space and " << style::Enum(var->Access())
+ << " access mode cannot be used by " << stage << " pipeline stage";
+ var_decl_note(var);
+ return false;
+ }
+ auto* storage = var->Type()->UnwrapRef()->As<core::type::StorageTexture>();
+ if (stage == ast::PipelineStage::kVertex && storage &&
+ storage->access() != core::Access::kRead) {
+ AddError(source) << "storage texture with " << style::Enum(storage->access())
+ << " access mode cannot be used by " << stage << " pipeline stage";
+ var_decl_note(var);
+ return false;
}
}
return true;
