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>

src/resolver/resolver.cc

@@ -529,6 +529,21 @@ bool Resolver::ValidateVariable(const ast::Variable* var) {
   }
 
   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 "

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 @@ static constexpr DimensionParams Dimension_cases[] = {
 
 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 @@ static constexpr FormatParams format_cases[] = {
 using StorageTextureFormatTest = ResolverTestWithParam<FormatParams>;
 TEST_P(StorageTextureFormatTest, All) {
   auto& params = GetParam();
-  // {
+  // [[group(0), binding(0)]]
-  // var a : texture_storage_1d<*>;
+  // var a : [[access(read)]] texture_storage_1d<*>;
-  // var b : texture_storage_2d<*>;
+  // [[group(0), binding(1)]]
-  // var c : texture_storage_2d_array<*>;
+  // var b : [[access(read)]] texture_storage_2d<*>;
-  // var d : texture_storage_3<*>;
+  // [[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),
+  auto st_a = ty.storage_texture(ast::TextureDimension::k1d, params.format);
-         ast::StorageClass::kUniformConstant, nullptr,
+  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),
+  auto st_b = ty.storage_texture(ast::TextureDimension::k2d, params.format);
-         ast::StorageClass::kUniformConstant, nullptr,
+  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::BindingDecoration>(0),
-             create<ast::GroupDecoration>(0),
+             create<ast::GroupDecoration>(1),
          });
 
-  Global("c",
+  auto st_c =
-         ty.storage_texture(ast::TextureDimension::k2dArray, params.format),
+      ty.storage_texture(ast::TextureDimension::k2dArray, params.format);
-         ast::StorageClass::kUniformConstant, nullptr,
+  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::BindingDecoration>(0),
-             create<ast::GroupDecoration>(0),
+             create<ast::GroupDecoration>(2),
          });
 
-  Global("d", ty.storage_texture(ast::TextureDimension::k3d, params.format),
+  auto st_d = ty.storage_texture(ast::TextureDimension::k3d, params.format);
-         ast::StorageClass::kUniformConstant, nullptr,
+  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::BindingDecoration>(0),
-             create<ast::GroupDecoration>(0),
+             create<ast::GroupDecoration>(3),
          });
 
   if (params.is_valid) {
@@ -695,6 +707,76 @@ TEST_P(StorageTextureFormatTest, All) {
 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

src/writer/spirv/builder_global_variable_test.cc

@@ -556,7 +556,6 @@ TEST_F(BuilderTest, GlobalVar_TextureStorageWriteOnly) {
 
   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 @@ TEST_F(BuilderTest, GlobalVar_TextureStorageWithDifferentAccess) {
   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);
 

src/writer/spirv/builder_type_test.cc

@@ -836,8 +836,9 @@ TEST_F(BuilderTest_Type, SampledTexture_Generate_CubeArray) {
 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_1d) {
 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_2d) {
 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_2dArray) {
 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 @@ TEST_F(BuilderTest_Type,
        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 @@ TEST_F(BuilderTest_Type,
        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 @@ TEST_F(BuilderTest_Type,
        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();