Add check for access controls on Storage Textures
BUG=tint:736
Change-Id: I6baae6507597383dd58b0d86605cbd44d5488c8f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48842
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index a96c6b7..06bb7c9 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -529,6 +529,21 @@
}
if (auto* r = type->UnwrapAll()->As<sem::StorageTexture>()) {
+ auto* ac = type->As<sem::AccessControl>();
+ if (!ac) {
+ diagnostics_.add_error("Storage Textures must have access control.",
+ var->source());
+ return false;
+ }
+
+ if (ac->IsReadWrite()) {
+ diagnostics_.add_error(
+ "Storage Textures only support Read-Only and Write-Only access "
+ "control.",
+ var->source());
+ return false;
+ }
+
if (!IsValidStorageTextureDimension(r->dim())) {
diagnostics_.add_error(
"Cube dimensions for storage textures are not "
diff --git a/src/resolver/type_validation_test.cc b/src/resolver/type_validation_test.cc
index d9707aa..eacae8e 100644
--- a/src/resolver/type_validation_test.cc
+++ b/src/resolver/type_validation_test.cc
@@ -17,6 +17,7 @@
#include "src/ast/struct_block_decoration.h"
#include "src/resolver/resolver.h"
#include "src/resolver/resolver_test_helper.h"
+#include "src/sem/access_control_type.h"
#include "src/sem/multisampled_texture_type.h"
#include "src/sem/storage_texture_type.h"
@@ -587,9 +588,14 @@
using StorageTextureDimensionTest = ResolverTestWithParam<DimensionParams>;
TEST_P(StorageTextureDimensionTest, All) {
+ // [[group(0), binding(0)]]
+ // var a : [[access(read)]] texture_storage_*<ru32int>;
auto& params = GetParam();
- Global("a", ty.storage_texture(params.dim, ast::ImageFormat::kR32Uint),
- ast::StorageClass::kUniformConstant, nullptr,
+
+ auto st = ty.storage_texture(params.dim, ast::ImageFormat::kR32Uint);
+ auto ac = ty.access(ast::AccessControl::kReadOnly, st);
+
+ Global("a", ac, ast::StorageClass::kUniformConstant, nullptr,
ast::DecorationList{
create<ast::BindingDecoration>(0),
create<ast::GroupDecoration>(0),
@@ -650,40 +656,46 @@
using StorageTextureFormatTest = ResolverTestWithParam<FormatParams>;
TEST_P(StorageTextureFormatTest, All) {
auto& params = GetParam();
- // {
- // var a : texture_storage_1d<*>;
- // var b : texture_storage_2d<*>;
- // var c : texture_storage_2d_array<*>;
- // var d : texture_storage_3<*>;
- // }
+ // [[group(0), binding(0)]]
+ // var a : [[access(read)]] texture_storage_1d<*>;
+ // [[group(0), binding(1)]]
+ // var b : [[access(read)]] texture_storage_2d<*>;
+ // [[group(0), binding(2)]]
+ // var c : [[access(read)]] texture_storage_2d_array<*>;
+ // [[group(0), binding(3)]]
+ // var d : [[access(read)]] texture_storage_3d<*>;
- Global("a", ty.storage_texture(ast::TextureDimension::k1d, params.format),
- ast::StorageClass::kUniformConstant, nullptr,
+ auto st_a = ty.storage_texture(ast::TextureDimension::k1d, params.format);
+ auto ac_a = ty.access(ast::AccessControl::kReadOnly, st_a);
+ Global("a", ac_a, ast::StorageClass::kUniformConstant, nullptr,
ast::DecorationList{
create<ast::BindingDecoration>(0),
create<ast::GroupDecoration>(0),
});
- Global("b", ty.storage_texture(ast::TextureDimension::k2d, params.format),
- ast::StorageClass::kUniformConstant, nullptr,
+ auto st_b = ty.storage_texture(ast::TextureDimension::k2d, params.format);
+ auto ac_b = ty.access(ast::AccessControl::kReadOnly, st_b);
+ Global("b", ac_b, ast::StorageClass::kUniformConstant, nullptr,
ast::DecorationList{
- create<ast::BindingDecoration>(1),
- create<ast::GroupDecoration>(0),
+ create<ast::BindingDecoration>(0),
+ create<ast::GroupDecoration>(1),
});
- Global("c",
- ty.storage_texture(ast::TextureDimension::k2dArray, params.format),
- ast::StorageClass::kUniformConstant, nullptr,
+ auto st_c =
+ ty.storage_texture(ast::TextureDimension::k2dArray, params.format);
+ auto ac_c = ty.access(ast::AccessControl::kReadOnly, st_c);
+ Global("c", ac_c, ast::StorageClass::kUniformConstant, nullptr,
ast::DecorationList{
- create<ast::BindingDecoration>(2),
- create<ast::GroupDecoration>(0),
+ create<ast::BindingDecoration>(0),
+ create<ast::GroupDecoration>(2),
});
- Global("d", ty.storage_texture(ast::TextureDimension::k3d, params.format),
- ast::StorageClass::kUniformConstant, nullptr,
+ auto st_d = ty.storage_texture(ast::TextureDimension::k3d, params.format);
+ auto ac_d = ty.access(ast::AccessControl::kReadOnly, st_d);
+ Global("d", ac_d, ast::StorageClass::kUniformConstant, nullptr,
ast::DecorationList{
- create<ast::BindingDecoration>(3),
- create<ast::GroupDecoration>(0),
+ create<ast::BindingDecoration>(0),
+ create<ast::GroupDecoration>(3),
});
if (params.is_valid) {
@@ -695,6 +707,76 @@
INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest,
StorageTextureFormatTest,
testing::ValuesIn(format_cases));
+
+using StorageTextureAccessControlTest = ResolverTest;
+
+TEST_F(StorageTextureAccessControlTest, MissingAccessControl_Fail) {
+ // [[group(0), binding(0)]]
+ // var a : texture_storage_1d<ru32int>;
+
+ auto st = ty.storage_texture(ast::TextureDimension::k1d,
+ ast::ImageFormat::kR32Uint);
+
+ Global("a", st, ast::StorageClass::kUniformConstant, nullptr,
+ ast::DecorationList{
+ create<ast::BindingDecoration>(0),
+ create<ast::GroupDecoration>(0),
+ });
+
+ EXPECT_FALSE(r()->Resolve());
+}
+
+TEST_F(StorageTextureAccessControlTest, RWAccessControl_Fail) {
+ // [[group(0), binding(0)]]
+ // var a : [[access(readwrite)]] texture_storage_1d<ru32int>;
+
+ auto st = ty.storage_texture(ast::TextureDimension::k1d,
+ ast::ImageFormat::kR32Uint);
+ auto ac = ty.access(ast::AccessControl::kReadWrite, st);
+
+ Global("a", ac, ast::StorageClass::kUniformConstant, nullptr,
+ ast::DecorationList{
+ create<ast::BindingDecoration>(0),
+ create<ast::GroupDecoration>(0),
+ });
+
+ EXPECT_FALSE(r()->Resolve());
+}
+
+TEST_F(StorageTextureAccessControlTest, ReadOnlyAccessControl_Pass) {
+ // [[group(0), binding(0)]]
+ // var a : [[access(read)]] texture_storage_1d<ru32int>;
+
+ auto st = ty.storage_texture(ast::TextureDimension::k1d,
+ ast::ImageFormat::kR32Uint);
+ auto ac = ty.access(ast::AccessControl::kReadOnly, st);
+
+ Global("a", ac, ast::StorageClass::kUniformConstant, nullptr,
+ ast::DecorationList{
+ create<ast::BindingDecoration>(0),
+ create<ast::GroupDecoration>(0),
+ });
+
+ EXPECT_TRUE(r()->Resolve()) << r()->error();
+}
+
+TEST_F(StorageTextureAccessControlTest, WriteOnlyAccessControl_Pass) {
+ // [[group(0), binding(0)]]
+ // var a : [[access(write)]] texture_storage_1d<ru32int>;
+
+ auto st = ty.storage_texture(ast::TextureDimension::k1d,
+ ast::ImageFormat::kR32Uint);
+ auto ac = ty.access(ast::AccessControl::kWriteOnly, st);
+
+ Global("a", ac, ast::StorageClass::kUniformConstant, nullptr,
+ ast::DecorationList{
+ create<ast::BindingDecoration>(0),
+ create<ast::GroupDecoration>(0),
+ });
+
+ EXPECT_TRUE(r()->Resolve()) << r()->error();
+}
+
} // namespace StorageTextureTests
} // namespace
diff --git a/src/writer/spirv/builder_global_variable_test.cc b/src/writer/spirv/builder_global_variable_test.cc
index 91227e0..53b0b5a 100644
--- a/src/writer/spirv/builder_global_variable_test.cc
+++ b/src/writer/spirv/builder_global_variable_test.cc
@@ -556,7 +556,6 @@
auto type = ty.storage_texture(ast::TextureDimension::k2d,
ast::ImageFormat::kR32Uint);
- Global("test_var", type, ast::StorageClass::kInput);
auto ac = ty.access(ast::AccessControl::kWriteOnly, type);
@@ -584,8 +583,6 @@
auto st = ty.storage_texture(ast::TextureDimension::k2d,
ast::ImageFormat::kR32Uint);
- Global("test_var", st, ast::StorageClass::kInput);
-
auto type_a = ty.access(ast::AccessControl::kReadOnly, st);
auto* var_a = Global("a", type_a, ast::StorageClass::kUniformConstant);
diff --git a/src/writer/spirv/builder_type_test.cc b/src/writer/spirv/builder_type_test.cc
index 1cf783f..eb5fbb0 100644
--- a/src/writer/spirv/builder_type_test.cc
+++ b/src/writer/spirv/builder_type_test.cc
@@ -836,8 +836,9 @@
TEST_F(BuilderTest_Type, StorageTexture_Generate_1d) {
auto s = ty.storage_texture(ast::TextureDimension::k1d,
ast::ImageFormat::kR32Float);
+ auto ac = ty.access(ast::AccessControl::kReadOnly, s);
- Global("test_var", s, ast::StorageClass::kInput);
+ Global("test_var", ac, ast::StorageClass::kInput);
spirv::Builder& b = Build();
@@ -851,8 +852,9 @@
TEST_F(BuilderTest_Type, StorageTexture_Generate_2d) {
auto s = ty.storage_texture(ast::TextureDimension::k2d,
ast::ImageFormat::kR32Float);
+ auto ac = ty.access(ast::AccessControl::kReadOnly, s);
- Global("test_var", s, ast::StorageClass::kInput);
+ Global("test_var", ac, ast::StorageClass::kInput);
spirv::Builder& b = Build();
@@ -866,8 +868,9 @@
TEST_F(BuilderTest_Type, StorageTexture_Generate_2dArray) {
auto s = ty.storage_texture(ast::TextureDimension::k2dArray,
ast::ImageFormat::kR32Float);
+ auto ac = ty.access(ast::AccessControl::kReadOnly, s);
- Global("test_var", s, ast::StorageClass::kInput);
+ Global("test_var", ac, ast::StorageClass::kInput);
spirv::Builder& b = Build();
@@ -881,8 +884,9 @@
TEST_F(BuilderTest_Type, StorageTexture_Generate_3d) {
auto s = ty.storage_texture(ast::TextureDimension::k3d,
ast::ImageFormat::kR32Float);
+ auto ac = ty.access(ast::AccessControl::kReadOnly, s);
- Global("test_var", s, ast::StorageClass::kInput);
+ Global("test_var", ac, ast::StorageClass::kInput);
spirv::Builder& b = Build();
@@ -897,8 +901,9 @@
StorageTexture_Generate_SampledTypeFloat_Format_r32float) {
auto s = ty.storage_texture(ast::TextureDimension::k2d,
ast::ImageFormat::kR32Float);
+ auto ac = ty.access(ast::AccessControl::kReadOnly, s);
- Global("test_var", s, ast::StorageClass::kInput);
+ Global("test_var", ac, ast::StorageClass::kInput);
spirv::Builder& b = Build();
@@ -913,8 +918,9 @@
StorageTexture_Generate_SampledTypeSint_Format_r32sint) {
auto s = ty.storage_texture(ast::TextureDimension::k2d,
ast::ImageFormat::kR32Sint);
+ auto ac = ty.access(ast::AccessControl::kReadOnly, s);
- Global("test_var", s, ast::StorageClass::kInput);
+ Global("test_var", ac, ast::StorageClass::kInput);
spirv::Builder& b = Build();
@@ -929,8 +935,9 @@
StorageTexture_Generate_SampledTypeUint_Format_r32uint) {
auto s = ty.storage_texture(ast::TextureDimension::k2d,
ast::ImageFormat::kR32Uint);
+ auto ac = ty.access(ast::AccessControl::kReadOnly, s);
- Global("test_var", s, ast::StorageClass::kInput);
+ Global("test_var", ac, ast::StorageClass::kInput);
spirv::Builder& b = Build();
