diff --git a/source/opt/instruction.cpp b/source/opt/instruction.cpp index 2bfd491386..297679177d 100644 --- a/source/opt/instruction.cpp +++ b/source/opt/instruction.cpp @@ -270,9 +270,19 @@ Instruction* Instruction::GetBaseAddress() const { } bool Instruction::IsReadOnlyPointer() const { - if (context()->get_feature_mgr()->HasCapability(spv::Capability::Shader)) - return IsReadOnlyPointerShaders(); - else + if (context()->get_feature_mgr()->HasCapability(spv::Capability::Shader)) { + switch (IsReadOnlyPointerShaders()) { + case ReadOnlyShaderResult::kFalse: + return false; + case ReadOnlyShaderResult::kTrue: + return true; + case ReadOnlyShaderResult::kMayHaveAliasingBinding: + return IsSetBindingUniformlyReadOnly(); + default: + assert(false); + return false; + } + } else return IsReadOnlyPointerKernel(); } @@ -404,22 +414,58 @@ bool Instruction::IsVulkanStorageBuffer() const { spv::StorageClass storage_class = spv::StorageClass(GetSingleWordInOperand(kPointerTypeStorageClassIndex)); - if (storage_class == spv::StorageClass::Uniform) { - bool is_buffer_block = false; - context()->get_decoration_mgr()->ForEachDecoration( - base_type->result_id(), uint32_t(spv::Decoration::BufferBlock), - [&is_buffer_block](const Instruction&) { is_buffer_block = true; }); - return is_buffer_block; - } else if (storage_class == spv::StorageClass::StorageBuffer) { - bool is_block = false; - context()->get_decoration_mgr()->ForEachDecoration( - base_type->result_id(), uint32_t(spv::Decoration::Block), - [&is_block](const Instruction&) { is_block = true; }); - return is_block; + + 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 true; + } + if (storage_class == spv::StorageClass::StorageBuffer && + decoration_mgr->HasDecoration(base_result_id, spv::Decoration::Block)) { + return true; } return false; } +bool Instruction::IsVulkanStorageBufferNonWritable() const { + if (!IsVulkanStorageBuffer()) { + 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)); + } + assert(base_type->opcode() == spv::Op::OpTypeStruct); + + // Test if the number of NonWritable member decorations matches + // the number of members within the struct. + // We're assuming the decorations to have unique valid member id. + uint32_t nonwritable_members = 0; + context()->get_decoration_mgr()->ForEachDecoration( + base_type->result_id(), + static_cast(spv::Decoration::NonWritable), + [&nonwritable_members](const Instruction& decr) { + if (decr.opcode() == spv::Op::OpMemberDecorate) { + ++nonwritable_members; + } + }); + + return nonwritable_members == base_type->NumInOperands(); +} + bool Instruction::IsVulkanStorageBufferVariable() const { if (opcode() != spv::Op::OpVariable) { return false; @@ -468,14 +514,15 @@ bool Instruction::IsVulkanUniformBuffer() const { return is_block; } -bool Instruction::IsReadOnlyPointerShaders() const { +Instruction::ReadOnlyShaderResult Instruction::IsReadOnlyPointerShaders() + const { if (type_id() == 0) { - return false; + return ReadOnlyShaderResult::kFalse; } Instruction* type_def = context()->get_def_use_mgr()->GetDef(type_id()); if (type_def->opcode() != spv::Op::OpTypePointer) { - return false; + return ReadOnlyShaderResult::kFalse; } spv::StorageClass storage_class = spv::StorageClass( @@ -485,26 +532,31 @@ bool Instruction::IsReadOnlyPointerShaders() const { case spv::StorageClass::UniformConstant: if (!type_def->IsVulkanStorageImage() && !type_def->IsVulkanStorageTexelBuffer()) { - return true; + return ReadOnlyShaderResult::kTrue; } break; case spv::StorageClass::Uniform: + case spv::StorageClass::StorageBuffer: if (!type_def->IsVulkanStorageBuffer()) { - return true; + return ReadOnlyShaderResult::kTrue; + } + if (type_def->IsVulkanStorageBufferNonWritable()) { + return ReadOnlyShaderResult::kMayHaveAliasingBinding; } break; case spv::StorageClass::PushConstant: case spv::StorageClass::Input: - return true; + return ReadOnlyShaderResult::kTrue; default: 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; + if (context()->get_decoration_mgr()->HasDecoration( + result_id(), spv::Decoration::NonWritable)) { + return ReadOnlyShaderResult::kTrue; + } + + return ReadOnlyShaderResult::kFalse; } bool Instruction::IsReadOnlyPointerKernel() const { @@ -523,6 +575,55 @@ bool Instruction::IsReadOnlyPointerKernel() const { return storage_class == spv::StorageClass::UniformConstant; } +bool Instruction::ResolveSetAndBinding(uint32_t& set, uint32_t& binding) const { + bool found_set = false; + bool found_binding = false; + for (Instruction* decr_inst : + context()->get_decoration_mgr()->GetDecorationsFor(result_id(), false)) { + spv::Decoration decoration = + spv::Decoration(decr_inst->GetSingleWordInOperand(1u)); + if (decoration == spv::Decoration::DescriptorSet) { + set = decr_inst->GetSingleWordInOperand(2u); + found_set = true; + } + if (decoration == spv::Decoration::Binding) { + binding = decr_inst->GetSingleWordInOperand(2u); + found_binding = true; + } + } + return found_set && found_binding; +} + +bool Instruction::IsSetBindingUniformlyReadOnly() const { + uint32_t set, binding; + if (!ResolveSetAndBinding(set, binding)) { + assert(false && "Set and binding couldn't be resolved!"); + return false; + } + for (const auto& other_inst : context()->types_values()) { + if (other_inst.opcode() != spv::Op::OpVariable) { + continue; + } + if (other_inst.result_id() == result_id()) { + continue; + } + + uint32_t other_set = 0; + uint32_t other_binding = 0; + if (!other_inst.ResolveSetAndBinding(other_set, other_binding)) { + continue; + } + + if (other_set == set && other_binding == binding) { + if (other_inst.IsReadOnlyPointerShaders() == + ReadOnlyShaderResult::kFalse) { + return false; + } + } + } + return true; +} + void Instruction::UpdateLexicalScope(uint32_t scope) { dbg_scope_.SetLexicalScope(scope); for (auto& i : dbg_line_insts_) { diff --git a/source/opt/instruction.h b/source/opt/instruction.h index dad644dcc9..3f0de17f42 100644 --- a/source/opt/instruction.h +++ b/source/opt/instruction.h @@ -481,6 +481,10 @@ class Instruction : public utils::IntrusiveNodeBase { // storage buffer. bool IsVulkanStorageBuffer() const; + // Returns true if the instruction defines a pointer type that points to a + // buffer that is decorated with NonWritable. + bool IsVulkanStorageBufferNonWritable() 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; @@ -620,13 +624,26 @@ class Instruction : public utils::IntrusiveNodeBase { return 0; } + enum class ReadOnlyShaderResult { kFalse, kTrue, kMayHaveAliasingBinding }; + // Returns true if the instruction generates a read-only pointer, with the - // same caveats documented in the comment for IsReadOnlyPointer. The first - // version assumes the module is a shader module. The second assumes a - // kernel. - bool IsReadOnlyPointerShaders() const; + // same caveats documented in the comment for IsReadOnlyPointer. + // The first version assumes the module is a shader module, which may also + // return a hint that the result _may_ be readonly as long as it isn't aliased + // by something that is writable. The second assumes a kernel. + ReadOnlyShaderResult IsReadOnlyPointerShaders() const; bool IsReadOnlyPointerKernel() const; + // Resolves |DescriptorSet| and |Binding| decoration of this instruction. + // Returns true if both could be found. + bool ResolveSetAndBinding(uint32_t& set, uint32_t& binding) const; + + // Returns true if all _other_ instruction that have the same set + // and binding are read only. + // This function should only be called if IsReadOnlyPointerShaders() has + // already been verified to return true. + bool IsSetBindingUniformlyReadOnly() const; + // Returns true if the result of |inst| can be used as the base image for an // instruction that samples a image, reads an image, or writes to an image. bool IsValidBaseImage() const; diff --git a/test/opt/instruction_test.cpp b/test/opt/instruction_test.cpp index 7ea5850d8f..83b7536cc5 100644 --- a/test/opt/instruction_test.cpp +++ b/test/opt/instruction_test.cpp @@ -29,8 +29,8 @@ namespace spvtools { namespace opt { namespace { -using ::testing::Eq; using spvtest::MakeInstruction; +using ::testing::Eq; using DescriptorTypeTest = PassTest<::testing::Test>; using OpaqueTypeTest = PassTest<::testing::Test>; using GetBaseTest = PassTest<::testing::Test>; @@ -347,6 +347,7 @@ TEST_F(DescriptorTypeTest, StorageImage) { EXPECT_FALSE(type->IsVulkanStorageTexelBuffer()); EXPECT_FALSE(type->IsVulkanStorageBuffer()); EXPECT_FALSE(type->IsVulkanUniformBuffer()); + EXPECT_FALSE(type->IsVulkanStorageBufferNonWritable()); 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->IsVulkanStorageBufferNonWritable()); 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->IsVulkanStorageBufferNonWritable()); 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->IsVulkanStorageBufferNonWritable()); Instruction* variable = context->get_def_use_mgr()->GetDef(3); EXPECT_FALSE(variable->IsReadOnlyPointer()); @@ -478,6 +482,235 @@ TEST_F(DescriptorTypeTest, StorageBuffer) { EXPECT_FALSE(object_copy->IsReadOnlyPointer()); } +TEST_F(DescriptorTypeTest, NonWritableStorageBuffer) { + 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 + OpDecorate %3 DescriptorSet 0 + OpDecorate %3 Binding 0 + OpDecorate %4 BufferBlock + OpMemberDecorate %4 0 NonWritable + OpDecorate %5 DescriptorSet 0 + OpDecorate %5 Binding 1 + OpDecorate %6 BufferBlock + %7 = OpTypeVoid + %8 = OpTypeFunction %7 + %9 = OpTypeFloat 32 + %10 = OpTypeVector %9 4 + %11 = OpTypeRuntimeArray %10 + %4 = OpTypeStruct %11 + %6 = OpTypeStruct %11 + %12 = OpTypePointer Uniform %4 + %13 = OpTypePointer Uniform %6 + %3 = OpVariable %12 Uniform + %5 = OpVariable %13 Uniform + %2 = OpFunction %7 None %8 + %14 = OpLabel + %15 = OpCopyObject %11 %3 + OpReturn + OpFunctionEnd +)"; + + std::unique_ptr context = + BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text); + Instruction* type = context->get_def_use_mgr()->GetDef(12); + EXPECT_FALSE(type->IsVulkanStorageImage()); + EXPECT_FALSE(type->IsVulkanSampledImage()); + EXPECT_FALSE(type->IsVulkanStorageTexelBuffer()); + EXPECT_TRUE(type->IsVulkanStorageBuffer()); + EXPECT_FALSE(type->IsVulkanUniformBuffer()); + EXPECT_TRUE(type->IsVulkanStorageBufferNonWritable()); + + 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, NonWritableStorageBufferMultipleMembers) { + const std::string text = R"( + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint GLCompute %2 "main" + OpExecutionMode %2 LocalSize 1 1 1 + OpDecorate %3 Block + OpMemberDecorate %3 0 Offset 0 + OpDecorate %4 Binding 1 + OpDecorate %4 DescriptorSet 0 + OpDecorate %5 Block + OpMemberDecorate %5 0 NonWritable + OpMemberDecorate %5 0 Offset 0 + OpMemberDecorate %5 1 NonWritable + OpMemberDecorate %5 1 Offset 4 + OpDecorate %6 Binding 0 + OpDecorate %6 DescriptorSet 0 + %7 = OpTypeVoid + %8 = OpTypeFunction %7 + %9 = OpTypeInt 32 0 + %3 = OpTypeStruct %9 + %10 = OpTypePointer StorageBuffer %3 + %4 = OpVariable %10 StorageBuffer + %11 = OpTypeInt 32 1 + %12 = OpConstant %11 0 + %5 = OpTypeStruct %9 %9 + %13 = OpTypePointer StorageBuffer %5 + %6 = OpVariable %13 StorageBuffer + %14 = OpTypePointer StorageBuffer %9 + %2 = OpFunction %7 None %8 + %16 = OpLabel + %17 = OpAccessChain %14 %6 %12 + %18 = OpLoad %9 %17 + OpReturn + OpFunctionEnd +)"; + + std::unique_ptr context = + BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text); + Instruction* type = context->get_def_use_mgr()->GetDef(13); + EXPECT_TRUE(type->IsVulkanStorageBuffer()); + EXPECT_TRUE(type->IsVulkanStorageBufferNonWritable()); + Instruction* variable = context->get_def_use_mgr()->GetDef(6); + EXPECT_TRUE(variable->IsReadOnlyPointer()); +} + +TEST_F(DescriptorTypeTest, MixedNonWritableStorageBuffer) { + const std::string text = R"( + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint GLCompute %2 "main" + OpExecutionMode %2 LocalSize 1 1 1 + OpDecorate %3 Block + OpMemberDecorate %3 0 Offset 0 + OpDecorate %4 Binding 1 + OpDecorate %4 DescriptorSet 0 + OpDecorate %5 Block + OpMemberDecorate %5 0 NonWritable + OpMemberDecorate %5 0 Offset 0 + OpMemberDecorate %5 1 Offset 4 + OpDecorate %6 Binding 0 + OpDecorate %6 DescriptorSet 0 + %7 = OpTypeVoid + %8 = OpTypeFunction %7 + %9 = OpTypeInt 32 0 + %3 = OpTypeStruct %9 + %10 = OpTypePointer StorageBuffer %3 + %4 = OpVariable %10 StorageBuffer + %11 = OpTypeInt 32 1 + %12 = OpConstant %11 0 + %5 = OpTypeStruct %9 %9 + %13 = OpTypePointer StorageBuffer %5 + %6 = OpVariable %13 StorageBuffer + %14 = OpTypePointer StorageBuffer %9 + %2 = OpFunction %7 None %8 + %16 = OpLabel + %17 = OpAccessChain %14 %6 %12 + %18 = OpLoad %9 %17 + OpReturn + OpFunctionEnd +)"; + std::unique_ptr context = + BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text); + Instruction* type = context->get_def_use_mgr()->GetDef(13); + EXPECT_TRUE(type->IsVulkanStorageBuffer()); + EXPECT_FALSE(type->IsVulkanStorageBufferNonWritable()); + Instruction* variable = context->get_def_use_mgr()->GetDef(6); + EXPECT_FALSE(variable->IsReadOnlyPointer()); +} + +TEST_F(DescriptorTypeTest, AliasedNotWritableStorageBuffer) { + 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 + OpDecorate %3 DescriptorSet 0 + OpDecorate %3 Binding 0 + OpDecorate %4 BufferBlock + OpMemberDecorate %4 0 NonWritable + OpDecorate %5 DescriptorSet 0 + OpDecorate %5 Binding 0 + OpDecorate %6 BufferBlock + %7 = OpTypeVoid + %8 = OpTypeFunction %7 + %9 = OpTypeFloat 32 + %10 = OpTypeVector %9 4 + %11 = OpTypeRuntimeArray %10 + %4 = OpTypeStruct %11 + %6 = OpTypeStruct %11 + %12 = OpTypePointer Uniform %4 + %13 = OpTypePointer Uniform %6 + %3 = OpVariable %12 Uniform + %5 = OpVariable %13 Uniform + %2 = OpFunction %7 None %8 + %14 = OpLabel + %15 = OpCopyObject %11 %3 + OpReturn + OpFunctionEnd +)"; + + std::unique_ptr context = + BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text); + Instruction* variable_readonly = context->get_def_use_mgr()->GetDef(3); + EXPECT_FALSE(variable_readonly->IsReadOnlyPointer()); + Instruction* variable_writeonly = context->get_def_use_mgr()->GetDef(5); + EXPECT_FALSE(variable_writeonly->IsReadOnlyPointer()); +} + +TEST_F(DescriptorTypeTest, AliasedNonWritableStorageBufferMember) { + const std::string text = R"( + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint GLCompute %2 "main" + OpExecutionMode %2 LocalSize 1 1 1 + OpDecorate %3 Block + OpMemberDecorate %3 0 Offset 0 + OpMemberDecorate %3 0 NonWritable + OpDecorate %4 Binding 0 + OpDecorate %4 DescriptorSet 0 + OpDecorate %5 Block + OpMemberDecorate %5 0 NonWritable + OpMemberDecorate %5 0 Offset 0 + OpMemberDecorate %5 1 Offset 4 + OpDecorate %6 Binding 0 + OpDecorate %6 DescriptorSet 0 + %7 = OpTypeVoid + %8 = OpTypeFunction %7 + %9 = OpTypeInt 32 0 + %3 = OpTypeStruct %9 + %10 = OpTypePointer StorageBuffer %3 + %4 = OpVariable %10 StorageBuffer + %11 = OpTypeInt 32 1 + %12 = OpConstant %11 0 + %5 = OpTypeStruct %9 %9 + %13 = OpTypePointer StorageBuffer %5 + %6 = OpVariable %13 StorageBuffer + %14 = OpTypePointer StorageBuffer %9 + %2 = OpFunction %7 None %8 + %16 = OpLabel + %17 = OpAccessChain %14 %6 %12 + %18 = OpLoad %9 %17 + OpReturn + OpFunctionEnd +)"; + std::unique_ptr context = + BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text); + Instruction* type = context->get_def_use_mgr()->GetDef(10); + EXPECT_TRUE(type->IsVulkanStorageBuffer()); + EXPECT_TRUE(type->IsVulkanStorageBufferNonWritable()); + Instruction* variable = context->get_def_use_mgr()->GetDef(4); + EXPECT_FALSE(variable->IsReadOnlyPointer()); +} + TEST_F(DescriptorTypeTest, UniformBuffer) { const std::string text = R"( OpCapability Shader @@ -513,6 +746,7 @@ TEST_F(DescriptorTypeTest, UniformBuffer) { EXPECT_FALSE(type->IsVulkanStorageTexelBuffer()); EXPECT_FALSE(type->IsVulkanStorageBuffer()); EXPECT_TRUE(type->IsVulkanUniformBuffer()); + EXPECT_FALSE(type->IsVulkanStorageBufferNonWritable()); Instruction* variable = context->get_def_use_mgr()->GetDef(3); EXPECT_TRUE(variable->IsReadOnlyPointer());