From d49841ca9f6842ba93d810e361b4896dec9fea98 Mon Sep 17 00:00:00 2001
From: Corentin Wallez <cwallez@chromium.org>
Date: Wed, 25 Nov 2020 13:59:27 +0000
Subject: [PATCH] writer/spirv: Emit NonRead/Writable decoration of storage
 texture vars

Fixes: tint:366
Change-Id: I03e1312841d5b86d1192382bf4cdf0ad1bcfc43d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/33921
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: David Neto <dneto@google.com>
---
 src/writer/spirv/builder.cc                   | 20 +++++++++-
 .../spirv/builder_global_variable_test.cc     | 40 +++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc
index 0d18d84fb1..7b74ecbb60 100644
--- a/src/writer/spirv/builder.cc
+++ b/src/writer/spirv/builder.cc
@@ -710,7 +710,25 @@ bool Builder::GenerateGlobalVariable(ast::Variable* var) {
                      Operand::Int(ConvertStorageClass(sc))};
   if (var->has_constructor()) {
     ops.push_back(Operand::Int(init_id));
-  } else if (!type->IsTexture() && !type->IsSampler()) {
+  } else if (type->IsTexture()) {
+    // Decorate storage texture variables with NonRead/Writeable if needed.
+    if (type->AsTexture()->IsStorage()) {
+      switch (type->AsTexture()->AsStorage()->access()) {
+        case ast::AccessControl::kWriteOnly:
+          push_annot(
+              spv::Op::OpDecorate,
+              {Operand::Int(var_id), Operand::Int(SpvDecorationNonReadable)});
+          break;
+        case ast::AccessControl::kReadOnly:
+          push_annot(
+              spv::Op::OpDecorate,
+              {Operand::Int(var_id), Operand::Int(SpvDecorationNonWritable)});
+          break;
+        case ast::AccessControl::kReadWrite:
+          break;
+      }
+    }
+  } else if (!type->IsSampler()) {
     // Certain cases require us to generate a constructor value.
     //
     // 1- ConstantId's must be attached to the OpConstant, if we have a
diff --git a/src/writer/spirv/builder_global_variable_test.cc b/src/writer/spirv/builder_global_variable_test.cc
index 97ed8ffb69..9cfbd0f013 100644
--- a/src/writer/spirv/builder_global_variable_test.cc
+++ b/src/writer/spirv/builder_global_variable_test.cc
@@ -620,6 +620,46 @@ OpName %5 "tint_63"
 )");
 }
 
+TEST_F(BuilderTest, GlobalVar_TextureStorageReadOnly) {
+  // var<uniform_constant> a : texture_storage_ro_2d<r32uint>;
+  ast::type::StorageTextureType type(ast::type::TextureDimension::k2d,
+                                     ast::AccessControl::kReadOnly,
+                                     ast::type::ImageFormat::kR32Uint);
+  ASSERT_TRUE(td.DetermineStorageTextureSubtype(&type)) << td.error();
+
+  ast::Variable var_a("a", ast::StorageClass::kUniformConstant, &type);
+
+  EXPECT_TRUE(b.GenerateGlobalVariable(&var_a)) << b.error();
+
+  EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %1 NonWritable
+)");
+  EXPECT_EQ(DumpInstructions(b.types()), R"(%4 = OpTypeInt 32 0
+%3 = OpTypeImage %4 2D 0 0 0 2 R32ui
+%2 = OpTypePointer UniformConstant %3
+%1 = OpVariable %2 UniformConstant
+)");
+}
+
+TEST_F(BuilderTest, GlobalVar_TextureStorageWriteOnly) {
+  // var<uniform_constant> a : texture_storage_wo_2d<r32uint>;
+  ast::type::StorageTextureType type(ast::type::TextureDimension::k2d,
+                                     ast::AccessControl::kWriteOnly,
+                                     ast::type::ImageFormat::kR32Uint);
+  ASSERT_TRUE(td.DetermineStorageTextureSubtype(&type)) << td.error();
+
+  ast::Variable var_a("a", ast::StorageClass::kUniformConstant, &type);
+
+  EXPECT_TRUE(b.GenerateGlobalVariable(&var_a)) << b.error();
+
+  EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %1 NonReadable
+)");
+  EXPECT_EQ(DumpInstructions(b.types()), R"(%4 = OpTypeVoid
+%3 = OpTypeImage %4 2D 0 0 0 2 R32ui
+%2 = OpTypePointer UniformConstant %3
+%1 = OpVariable %2 UniformConstant
+)");
+}
+
 }  // namespace
 }  // namespace spirv
 }  // namespace writer