Adding deduplication of NonWritable OpLoads

The current logic would only mark a load as read-only if either the
OpVariable had NonWritable or a Uniform was not a storage buffer.

This adds `IsVulkanReadOnlyStorageBuffer`, which does mostly the same
stuff as `IsVulkanStorageBuffer`, with the addition of testing for
NonWritable.

When used as part of `IsReadOnlyLoad` for assigning value numbers,
prevents it from being forced a unique value.

Fixes #6412

Author: alister-chowdhury

source/opt/instruction.cpp

@@ -419,6 +419,48 @@ bool Instruction::IsVulkanStorageBuffer() const {
   return false;
 }
 
+bool Instruction::IsVulkanReadOnlyStorageBuffer() const {
+  if (opcode() != spv::Op::OpTypePointer) {
+    return false;
+  }
+  Instruction* base_type =
+      context()->get_def_use_mgr()->GetDef(GetSingleWordInOperand(1));
+
+  // Unpack the optional layer of arraying.
+  if (base_type->opcode() == spv::Op::OpTypeArray ||
+      base_type->opcode() == spv::Op::OpTypeRuntimeArray) {
+    base_type = context()->get_def_use_mgr()->GetDef(
+        base_type->GetSingleWordInOperand(0));
+  }
+
+  if (base_type->opcode() != spv::Op::OpTypeStruct) {
+    return false;
+  }
+
+  spv::StorageClass storage_class =
+      spv::StorageClass(GetSingleWordInOperand(kPointerTypeStorageClassIndex));
+
+  if (storage_class != spv::StorageClass::Uniform &&
+      storage_class != spv::StorageClass::StorageBuffer) {
+    return false;
+  }
+
+  analysis::DecorationManager* decoration_mgr = context()->get_decoration_mgr();
+  uint32_t base_result_id = base_type->result_id();
+
+  if (storage_class == spv::StorageClass::Uniform &&
+      !decoration_mgr->HasDecoration(base_result_id,
+                                     spv::Decoration::BufferBlock)) {
+    return false;
+  }
+  if (storage_class == spv::StorageClass::StorageBuffer &&
+      !decoration_mgr->HasDecoration(base_result_id, spv::Decoration::Block)) {
+    return false;
+  }
+  return decoration_mgr->HasDecoration(base_result_id,
+                                       spv::Decoration::NonWritable);
+}
+
 bool Instruction::IsVulkanStorageBufferVariable() const {
   if (opcode() != spv::Op::OpVariable) {
     return false;
@@ -491,6 +533,9 @@ bool Instruction::IsReadOnlyPointerShaders() const {
       if (!type_def->IsVulkanStorageBuffer()) {
         return true;
       }
+      if (type_def->IsVulkanReadOnlyStorageBuffer()) {
+        return true;
+      }
       break;
     case spv::StorageClass::PushConstant:
     case spv::StorageClass::Input:
@@ -499,11 +544,8 @@ bool Instruction::IsReadOnlyPointerShaders() const {
       break;
   }
 
-  bool is_nonwritable = false;
-  context()->get_decoration_mgr()->ForEachDecoration(
-      result_id(), uint32_t(spv::Decoration::NonWritable),
-      [&is_nonwritable](const Instruction&) { is_nonwritable = true; });
-  return is_nonwritable;
+  return context()->get_decoration_mgr()->HasDecoration(
+      result_id(), spv::Decoration::NonWritable);
 }
 
 bool Instruction::IsReadOnlyPointerKernel() const {

source/opt/instruction.h

@@ -481,6 +481,10 @@ class Instruction : public utils::IntrusiveNodeBase<Instruction> {
   // storage buffer.
   bool IsVulkanStorageBuffer() const;
 
+  // Returns true if the instruction defines a pointer type that points to a
+  // read-only buffer.
+  bool IsVulkanReadOnlyStorageBuffer() const;
+
   // Returns true if the instruction defines a variable in StorageBuffer or
   // Uniform storage class with a pointer type that points to a storage buffer.
   bool IsVulkanStorageBufferVariable() const;

test/opt/instruction_test.cpp

@@ -347,6 +347,7 @@ TEST_F(DescriptorTypeTest, StorageImage) {
   EXPECT_FALSE(type->IsVulkanStorageTexelBuffer());
   EXPECT_FALSE(type->IsVulkanStorageBuffer());
   EXPECT_FALSE(type->IsVulkanUniformBuffer());
+  EXPECT_FALSE(type->IsVulkanReadOnlyStorageBuffer());
 
   Instruction* variable = context->get_def_use_mgr()->GetDef(3);
   EXPECT_FALSE(variable->IsReadOnlyPointer());
@@ -387,6 +388,7 @@ TEST_F(DescriptorTypeTest, SampledImage) {
   EXPECT_FALSE(type->IsVulkanStorageTexelBuffer());
   EXPECT_FALSE(type->IsVulkanStorageBuffer());
   EXPECT_FALSE(type->IsVulkanUniformBuffer());
+  EXPECT_FALSE(type->IsVulkanReadOnlyStorageBuffer());
 
   Instruction* variable = context->get_def_use_mgr()->GetDef(3);
   EXPECT_TRUE(variable->IsReadOnlyPointer());
@@ -427,6 +429,7 @@ TEST_F(DescriptorTypeTest, StorageTexelBuffer) {
   EXPECT_TRUE(type->IsVulkanStorageTexelBuffer());
   EXPECT_FALSE(type->IsVulkanStorageBuffer());
   EXPECT_FALSE(type->IsVulkanUniformBuffer());
+  EXPECT_FALSE(type->IsVulkanReadOnlyStorageBuffer());
 
   Instruction* variable = context->get_def_use_mgr()->GetDef(3);
   EXPECT_FALSE(variable->IsReadOnlyPointer());
@@ -470,6 +473,7 @@ TEST_F(DescriptorTypeTest, StorageBuffer) {
   EXPECT_FALSE(type->IsVulkanStorageTexelBuffer());
   EXPECT_TRUE(type->IsVulkanStorageBuffer());
   EXPECT_FALSE(type->IsVulkanUniformBuffer());
+  EXPECT_FALSE(type->IsVulkanReadOnlyStorageBuffer());
 
   Instruction* variable = context->get_def_use_mgr()->GetDef(3);
   EXPECT_FALSE(variable->IsReadOnlyPointer());
@@ -478,6 +482,51 @@ TEST_F(DescriptorTypeTest, StorageBuffer) {
   EXPECT_FALSE(object_copy->IsReadOnlyPointer());
 }
 
+TEST_F(DescriptorTypeTest, ReadOnlyStorageBuffer) {
+  const std::string text = R"(
+               OpCapability Shader
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Fragment %2 "main"
+               OpExecutionMode %2 OriginUpperLeft
+               OpSource GLSL 430
+               OpName %3 "myStorageImage"
+               OpDecorate %3 DescriptorSet 0
+               OpDecorate %3 Binding 0
+               OpDecorate %9 BufferBlock
+               OpDecorate %9 NonWritable
+          %4 = OpTypeVoid
+          %5 = OpTypeFunction %4
+          %6 = OpTypeFloat 32
+          %7 = OpTypeVector %6 4
+          %8 = OpTypeRuntimeArray %7
+          %9 = OpTypeStruct %8
+         %10 = OpTypePointer Uniform %9
+          %3 = OpVariable %10 Uniform
+          %2 = OpFunction %4 None %5
+         %11 = OpLabel
+         %12 = OpCopyObject %8 %3
+               OpReturn
+               OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text);
+  Instruction* type = context->get_def_use_mgr()->GetDef(10);
+  EXPECT_FALSE(type->IsVulkanStorageImage());
+  EXPECT_FALSE(type->IsVulkanSampledImage());
+  EXPECT_FALSE(type->IsVulkanStorageTexelBuffer());
+  EXPECT_TRUE(type->IsVulkanStorageBuffer());
+  EXPECT_FALSE(type->IsVulkanUniformBuffer());
+  EXPECT_TRUE(type->IsVulkanReadOnlyStorageBuffer());
+
+  Instruction* variable = context->get_def_use_mgr()->GetDef(3);
+  EXPECT_TRUE(variable->IsReadOnlyPointer());
+
+  Instruction* object_copy = context->get_def_use_mgr()->GetDef(12);
+  EXPECT_FALSE(object_copy->IsReadOnlyPointer());
+}
+
 TEST_F(DescriptorTypeTest, UniformBuffer) {
   const std::string text = R"(
                OpCapability Shader
@@ -513,6 +562,7 @@ TEST_F(DescriptorTypeTest, UniformBuffer) {
   EXPECT_FALSE(type->IsVulkanStorageTexelBuffer());
   EXPECT_FALSE(type->IsVulkanStorageBuffer());
   EXPECT_TRUE(type->IsVulkanUniformBuffer());
+  EXPECT_FALSE(type->IsVulkanReadOnlyStorageBuffer());
 
   Instruction* variable = context->get_def_use_mgr()->GetDef(3);
   EXPECT_TRUE(variable->IsReadOnlyPointer());