From ad81a81ba18b4482f4af3f427ded3054b9d451b9 Mon Sep 17 00:00:00 2001 From: Alister Chowdhury Date: Sat, 15 Nov 2025 06:59:54 +0000 Subject: [PATCH 1/9] 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 --- source/opt/instruction.cpp | 52 +++++++++++++++++++++++++++++++---- source/opt/instruction.h | 4 +++ test/opt/instruction_test.cpp | 50 +++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 5 deletions(-) diff --git a/source/opt/instruction.cpp b/source/opt/instruction.cpp index 2bfd491386..02be1cf59d 100644 --- a/source/opt/instruction.cpp +++ b/source/opt/instruction.cpp @@ -420,6 +420,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; @@ -492,6 +534,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: @@ -500,11 +545,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 { diff --git a/source/opt/instruction.h b/source/opt/instruction.h index dad644dcc9..f1fa6e8eff 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 + // 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; diff --git a/test/opt/instruction_test.cpp b/test/opt/instruction_test.cpp index 7ea5850d8f..126043ad73 100644 --- a/test/opt/instruction_test.cpp +++ b/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 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()); From 5d31c2afdb7be5873e24040283e14aa9b45e0e1b Mon Sep 17 00:00:00 2001 From: Alister Chowdhury Date: Fri, 28 Nov 2025 05:06:27 +0000 Subject: [PATCH 2/9] Moved the more modern logic to IsVulkanStorageBuffer, which is then called by IsVulkanReadOnlyStorageBuffer --- source/opt/instruction.cpp | 54 +++++++++++++------------------------- 1 file changed, 18 insertions(+), 36 deletions(-) diff --git a/source/opt/instruction.cpp b/source/opt/instruction.cpp index 02be1cf59d..d58b80eeae 100644 --- a/source/opt/instruction.cpp +++ b/source/opt/instruction.cpp @@ -404,24 +404,29 @@ 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::IsVulkanReadOnlyStorageBuffer() const { - if (opcode() != spv::Op::OpTypePointer) { + if (!IsVulkanStorageBuffer()) { return false; } Instruction* base_type = @@ -433,31 +438,8 @@ bool Instruction::IsVulkanReadOnlyStorageBuffer() const { 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); } From eda5be0c417f886a8ab72214a69ad83938a9669a Mon Sep 17 00:00:00 2001 From: Alister Chowdhury Date: Sat, 29 Nov 2025 15:44:50 +0000 Subject: [PATCH 3/9] Attempting to respect binding aliasing, which includes: * Renaming `IsVulkanReadOnlyStorageBuffer` to `IsVulkanStorageBufferNonWriteable` Mainly to not accidentally mislead someone who comes across it. * Changed `IsReadOnlyPointerShaders` to return an enum, that can provide a hint that we may need to check other variables. * Added `IsSetBindingUniformlyReadOnly` which is basically for said hint, to make sure nothing that isn't read-only is bound to the same address. --- source/opt/instruction.cpp | 92 +++++++++++++++++++++++++++++------ source/opt/instruction.h | 23 ++++++--- test/opt/instruction_test.cpp | 90 +++++++++++++++++++++++++--------- 3 files changed, 163 insertions(+), 42 deletions(-) diff --git a/source/opt/instruction.cpp b/source/opt/instruction.cpp index d58b80eeae..bea20a6024 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(); } @@ -425,7 +435,7 @@ bool Instruction::IsVulkanStorageBuffer() const { return false; } -bool Instruction::IsVulkanReadOnlyStorageBuffer() const { +bool Instruction::IsVulkanStorageBufferNonWriteable() const { if (!IsVulkanStorageBuffer()) { return false; } @@ -492,14 +502,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( @@ -509,26 +520,30 @@ 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: if (!type_def->IsVulkanStorageBuffer()) { - return true; + return ReadOnlyShaderResult::kTrue; } - if (type_def->IsVulkanReadOnlyStorageBuffer()) { - return true; + if (type_def->IsVulkanStorageBufferNonWriteable()) { + return ReadOnlyShaderResult::kMayHaveAliasingBinding; } break; case spv::StorageClass::PushConstant: case spv::StorageClass::Input: - return true; + return ReadOnlyShaderResult::kTrue; default: break; } - return context()->get_decoration_mgr()->HasDecoration( - result_id(), spv::Decoration::NonWritable); + if (context()->get_decoration_mgr()->HasDecoration( + result_id(), spv::Decoration::NonWritable)) { + return ReadOnlyShaderResult::kTrue; + } + + return ReadOnlyShaderResult::kFalse; } bool Instruction::IsReadOnlyPointerKernel() const { @@ -547,6 +562,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 f1fa6e8eff..b743b0adcb 100644 --- a/source/opt/instruction.h +++ b/source/opt/instruction.h @@ -482,8 +482,8 @@ class Instruction : public utils::IntrusiveNodeBase { bool IsVulkanStorageBuffer() const; // Returns true if the instruction defines a pointer type that points to a - // read-only buffer. - bool IsVulkanReadOnlyStorageBuffer() const; + // buffer that is decorated with NonWriteable. + bool IsVulkanStorageBufferNonWriteable() 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. @@ -624,13 +624,24 @@ 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. + 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 126043ad73..99d4c343d8 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,7 +347,7 @@ TEST_F(DescriptorTypeTest, StorageImage) { EXPECT_FALSE(type->IsVulkanStorageTexelBuffer()); EXPECT_FALSE(type->IsVulkanStorageBuffer()); EXPECT_FALSE(type->IsVulkanUniformBuffer()); - EXPECT_FALSE(type->IsVulkanReadOnlyStorageBuffer()); + EXPECT_FALSE(type->IsVulkanStorageBufferNonWriteable()); Instruction* variable = context->get_def_use_mgr()->GetDef(3); EXPECT_FALSE(variable->IsReadOnlyPointer()); @@ -388,7 +388,7 @@ TEST_F(DescriptorTypeTest, SampledImage) { EXPECT_FALSE(type->IsVulkanStorageTexelBuffer()); EXPECT_FALSE(type->IsVulkanStorageBuffer()); EXPECT_FALSE(type->IsVulkanUniformBuffer()); - EXPECT_FALSE(type->IsVulkanReadOnlyStorageBuffer()); + EXPECT_FALSE(type->IsVulkanStorageBufferNonWriteable()); Instruction* variable = context->get_def_use_mgr()->GetDef(3); EXPECT_TRUE(variable->IsReadOnlyPointer()); @@ -429,7 +429,7 @@ TEST_F(DescriptorTypeTest, StorageTexelBuffer) { EXPECT_TRUE(type->IsVulkanStorageTexelBuffer()); EXPECT_FALSE(type->IsVulkanStorageBuffer()); EXPECT_FALSE(type->IsVulkanUniformBuffer()); - EXPECT_FALSE(type->IsVulkanReadOnlyStorageBuffer()); + EXPECT_FALSE(type->IsVulkanStorageBufferNonWriteable()); Instruction* variable = context->get_def_use_mgr()->GetDef(3); EXPECT_FALSE(variable->IsReadOnlyPointer()); @@ -473,7 +473,7 @@ TEST_F(DescriptorTypeTest, StorageBuffer) { EXPECT_FALSE(type->IsVulkanStorageTexelBuffer()); EXPECT_TRUE(type->IsVulkanStorageBuffer()); EXPECT_FALSE(type->IsVulkanUniformBuffer()); - EXPECT_FALSE(type->IsVulkanReadOnlyStorageBuffer()); + EXPECT_FALSE(type->IsVulkanStorageBufferNonWriteable()); Instruction* variable = context->get_def_use_mgr()->GetDef(3); EXPECT_FALSE(variable->IsReadOnlyPointer()); @@ -490,35 +490,40 @@ TEST_F(DescriptorTypeTest, ReadOnlyStorageBuffer) { 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 + OpDecorate %4 BufferBlock + OpDecorate %4 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(10); + 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->IsVulkanReadOnlyStorageBuffer()); + EXPECT_TRUE(type->IsVulkanStorageBufferNonWriteable()); Instruction* variable = context->get_def_use_mgr()->GetDef(3); EXPECT_TRUE(variable->IsReadOnlyPointer()); @@ -527,6 +532,47 @@ TEST_F(DescriptorTypeTest, ReadOnlyStorageBuffer) { EXPECT_FALSE(object_copy->IsReadOnlyPointer()); } +TEST_F(DescriptorTypeTest, AliasedNotReadOnlyStorageBuffer) { + 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 + OpDecorate %4 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_readonly->IsReadOnlyPointer()); +} + TEST_F(DescriptorTypeTest, UniformBuffer) { const std::string text = R"( OpCapability Shader @@ -562,7 +608,7 @@ TEST_F(DescriptorTypeTest, UniformBuffer) { EXPECT_FALSE(type->IsVulkanStorageTexelBuffer()); EXPECT_FALSE(type->IsVulkanStorageBuffer()); EXPECT_TRUE(type->IsVulkanUniformBuffer()); - EXPECT_FALSE(type->IsVulkanReadOnlyStorageBuffer()); + EXPECT_FALSE(type->IsVulkanStorageBufferNonWriteable()); Instruction* variable = context->get_def_use_mgr()->GetDef(3); EXPECT_TRUE(variable->IsReadOnlyPointer()); From dfdd0667f7c4f9dbc283047c9351c91e5f718b2a Mon Sep 17 00:00:00 2001 From: Alister Chowdhury Date: Sat, 29 Nov 2025 15:51:00 +0000 Subject: [PATCH 4/9] Spelling --- source/opt/instruction.cpp | 4 ++-- source/opt/instruction.h | 4 ++-- test/opt/instruction_test.cpp | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/source/opt/instruction.cpp b/source/opt/instruction.cpp index bea20a6024..02fd1f1e0c 100644 --- a/source/opt/instruction.cpp +++ b/source/opt/instruction.cpp @@ -435,7 +435,7 @@ bool Instruction::IsVulkanStorageBuffer() const { return false; } -bool Instruction::IsVulkanStorageBufferNonWriteable() const { +bool Instruction::IsVulkanStorageBufferNonWritable() const { if (!IsVulkanStorageBuffer()) { return false; } @@ -527,7 +527,7 @@ Instruction::ReadOnlyShaderResult Instruction::IsReadOnlyPointerShaders() if (!type_def->IsVulkanStorageBuffer()) { return ReadOnlyShaderResult::kTrue; } - if (type_def->IsVulkanStorageBufferNonWriteable()) { + if (type_def->IsVulkanStorageBufferNonWritable()) { return ReadOnlyShaderResult::kMayHaveAliasingBinding; } break; diff --git a/source/opt/instruction.h b/source/opt/instruction.h index b743b0adcb..93d11a8fd0 100644 --- a/source/opt/instruction.h +++ b/source/opt/instruction.h @@ -482,8 +482,8 @@ class Instruction : public utils::IntrusiveNodeBase { bool IsVulkanStorageBuffer() const; // Returns true if the instruction defines a pointer type that points to a - // buffer that is decorated with NonWriteable. - bool IsVulkanStorageBufferNonWriteable() const; + // 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. diff --git a/test/opt/instruction_test.cpp b/test/opt/instruction_test.cpp index 99d4c343d8..973de14072 100644 --- a/test/opt/instruction_test.cpp +++ b/test/opt/instruction_test.cpp @@ -347,7 +347,7 @@ TEST_F(DescriptorTypeTest, StorageImage) { EXPECT_FALSE(type->IsVulkanStorageTexelBuffer()); EXPECT_FALSE(type->IsVulkanStorageBuffer()); EXPECT_FALSE(type->IsVulkanUniformBuffer()); - EXPECT_FALSE(type->IsVulkanStorageBufferNonWriteable()); + EXPECT_FALSE(type->IsVulkanStorageBufferNonWritable()); Instruction* variable = context->get_def_use_mgr()->GetDef(3); EXPECT_FALSE(variable->IsReadOnlyPointer()); @@ -388,7 +388,7 @@ TEST_F(DescriptorTypeTest, SampledImage) { EXPECT_FALSE(type->IsVulkanStorageTexelBuffer()); EXPECT_FALSE(type->IsVulkanStorageBuffer()); EXPECT_FALSE(type->IsVulkanUniformBuffer()); - EXPECT_FALSE(type->IsVulkanStorageBufferNonWriteable()); + EXPECT_FALSE(type->IsVulkanStorageBufferNonWritable()); Instruction* variable = context->get_def_use_mgr()->GetDef(3); EXPECT_TRUE(variable->IsReadOnlyPointer()); @@ -429,7 +429,7 @@ TEST_F(DescriptorTypeTest, StorageTexelBuffer) { EXPECT_TRUE(type->IsVulkanStorageTexelBuffer()); EXPECT_FALSE(type->IsVulkanStorageBuffer()); EXPECT_FALSE(type->IsVulkanUniformBuffer()); - EXPECT_FALSE(type->IsVulkanStorageBufferNonWriteable()); + EXPECT_FALSE(type->IsVulkanStorageBufferNonWritable()); Instruction* variable = context->get_def_use_mgr()->GetDef(3); EXPECT_FALSE(variable->IsReadOnlyPointer()); @@ -473,7 +473,7 @@ TEST_F(DescriptorTypeTest, StorageBuffer) { EXPECT_FALSE(type->IsVulkanStorageTexelBuffer()); EXPECT_TRUE(type->IsVulkanStorageBuffer()); EXPECT_FALSE(type->IsVulkanUniformBuffer()); - EXPECT_FALSE(type->IsVulkanStorageBufferNonWriteable()); + EXPECT_FALSE(type->IsVulkanStorageBufferNonWritable()); Instruction* variable = context->get_def_use_mgr()->GetDef(3); EXPECT_FALSE(variable->IsReadOnlyPointer()); @@ -523,7 +523,7 @@ TEST_F(DescriptorTypeTest, ReadOnlyStorageBuffer) { EXPECT_FALSE(type->IsVulkanStorageTexelBuffer()); EXPECT_TRUE(type->IsVulkanStorageBuffer()); EXPECT_FALSE(type->IsVulkanUniformBuffer()); - EXPECT_TRUE(type->IsVulkanStorageBufferNonWriteable()); + EXPECT_TRUE(type->IsVulkanStorageBufferNonWritable()); Instruction* variable = context->get_def_use_mgr()->GetDef(3); EXPECT_TRUE(variable->IsReadOnlyPointer()); @@ -608,7 +608,7 @@ TEST_F(DescriptorTypeTest, UniformBuffer) { EXPECT_FALSE(type->IsVulkanStorageTexelBuffer()); EXPECT_FALSE(type->IsVulkanStorageBuffer()); EXPECT_TRUE(type->IsVulkanUniformBuffer()); - EXPECT_FALSE(type->IsVulkanStorageBufferNonWriteable()); + EXPECT_FALSE(type->IsVulkanStorageBufferNonWritable()); Instruction* variable = context->get_def_use_mgr()->GetDef(3); EXPECT_TRUE(variable->IsReadOnlyPointer()); From 081ca36843788bc949965ec941c1573adc475a9e Mon Sep 17 00:00:00 2001 From: Alister Chowdhury Date: Sat, 29 Nov 2025 16:20:25 +0000 Subject: [PATCH 5/9] writeonly test fix --- test/opt/instruction_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/opt/instruction_test.cpp b/test/opt/instruction_test.cpp index 973de14072..ef2b122091 100644 --- a/test/opt/instruction_test.cpp +++ b/test/opt/instruction_test.cpp @@ -570,7 +570,7 @@ TEST_F(DescriptorTypeTest, AliasedNotReadOnlyStorageBuffer) { 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_readonly->IsReadOnlyPointer()); + EXPECT_FALSE(variable_writeonly->IsReadOnlyPointer()); } TEST_F(DescriptorTypeTest, UniformBuffer) { From 135185f7148b38a88509fe4524f7262ec53471bf Mon Sep 17 00:00:00 2001 From: Alister Chowdhury Date: Sat, 29 Nov 2025 16:32:09 +0000 Subject: [PATCH 6/9] const ref foreach --- source/opt/instruction.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/opt/instruction.cpp b/source/opt/instruction.cpp index 02fd1f1e0c..36904ad2cb 100644 --- a/source/opt/instruction.cpp +++ b/source/opt/instruction.cpp @@ -587,7 +587,7 @@ bool Instruction::IsSetBindingUniformlyReadOnly() const { assert(false && "Set and binding couldn't be resolved!"); return false; } - for (const auto other_inst : context()->types_values()) { + for (const auto& other_inst : context()->types_values()) { if (other_inst.opcode() != spv::Op::OpVariable) { continue; } From 97bd5df9dee095ec911559870f75046e89de6ef4 Mon Sep 17 00:00:00 2001 From: Alister Chowdhury Date: Sat, 13 Dec 2025 06:48:10 +0000 Subject: [PATCH 7/9] Review updates * Changed IsVulkanStorageBufferNonWritable to test members, for NonWritable. * Specified IsSetBindingUniformlyReadOnly should be called after IsReadOnlyPointerShaders returns true. * Updated tests to use OpMemberDecorate. * Updated tests to include testing for having a NonWritable member. --- source/opt/instruction.cpp | 31 +++++++- source/opt/instruction.h | 2 + test/opt/instruction_test.cpp | 146 +++++++++++++++++++++++++++++++++- 3 files changed, 172 insertions(+), 7 deletions(-) diff --git a/source/opt/instruction.cpp b/source/opt/instruction.cpp index 36904ad2cb..04fc609ba7 100644 --- a/source/opt/instruction.cpp +++ b/source/opt/instruction.cpp @@ -448,10 +448,34 @@ bool Instruction::IsVulkanStorageBufferNonWritable() const { base_type = context()->get_def_use_mgr()->GetDef( base_type->GetSingleWordInOperand(0)); } + assert(base_type->opcode() == spv::Op::OpTypeStruct); + + // Check that all individual members of the struct are NonWritable. analysis::DecorationManager* decoration_mgr = context()->get_decoration_mgr(); - uint32_t base_result_id = base_type->result_id(); - return decoration_mgr->HasDecoration(base_result_id, - spv::Decoration::NonWritable); + + if (base_type->NumInOperands() <= 64) { + uint64_t writeable_mask = (1llu << (base_type->NumInOperands())) - 1; + decoration_mgr->ForEachDecoration( + base_type->result_id(), + static_cast(spv::Decoration::NonWritable), + [&writeable_mask](const Instruction& decr) { + if (decr.opcode() == spv::Op::OpMemberDecorate) { + writeable_mask &= ~(1llu << decr.GetSingleWordInOperand(1)); + } + }); + return !writeable_mask; + } + + std::vector nonwritable_members; + 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.push_back(decr.GetSingleWordInOperand(1)); + } + }); + return nonwritable_members.size() == base_type->NumInOperands(); } bool Instruction::IsVulkanStorageBufferVariable() const { @@ -524,6 +548,7 @@ Instruction::ReadOnlyShaderResult Instruction::IsReadOnlyPointerShaders() } break; case spv::StorageClass::Uniform: + case spv::StorageClass::StorageBuffer: if (!type_def->IsVulkanStorageBuffer()) { return ReadOnlyShaderResult::kTrue; } diff --git a/source/opt/instruction.h b/source/opt/instruction.h index 93d11a8fd0..b79eaf0208 100644 --- a/source/opt/instruction.h +++ b/source/opt/instruction.h @@ -640,6 +640,8 @@ class Instruction : public utils::IntrusiveNodeBase { // 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 diff --git a/test/opt/instruction_test.cpp b/test/opt/instruction_test.cpp index ef2b122091..83b7536cc5 100644 --- a/test/opt/instruction_test.cpp +++ b/test/opt/instruction_test.cpp @@ -482,7 +482,7 @@ TEST_F(DescriptorTypeTest, StorageBuffer) { EXPECT_FALSE(object_copy->IsReadOnlyPointer()); } -TEST_F(DescriptorTypeTest, ReadOnlyStorageBuffer) { +TEST_F(DescriptorTypeTest, NonWritableStorageBuffer) { const std::string text = R"( OpCapability Shader %1 = OpExtInstImport "GLSL.std.450" @@ -493,7 +493,7 @@ TEST_F(DescriptorTypeTest, ReadOnlyStorageBuffer) { OpDecorate %3 DescriptorSet 0 OpDecorate %3 Binding 0 OpDecorate %4 BufferBlock - OpDecorate %4 NonWritable + OpMemberDecorate %4 0 NonWritable OpDecorate %5 DescriptorSet 0 OpDecorate %5 Binding 1 OpDecorate %6 BufferBlock @@ -532,7 +532,99 @@ TEST_F(DescriptorTypeTest, ReadOnlyStorageBuffer) { EXPECT_FALSE(object_copy->IsReadOnlyPointer()); } -TEST_F(DescriptorTypeTest, AliasedNotReadOnlyStorageBuffer) { +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" @@ -543,7 +635,7 @@ TEST_F(DescriptorTypeTest, AliasedNotReadOnlyStorageBuffer) { OpDecorate %3 DescriptorSet 0 OpDecorate %3 Binding 0 OpDecorate %4 BufferBlock - OpDecorate %4 NonWritable + OpMemberDecorate %4 0 NonWritable OpDecorate %5 DescriptorSet 0 OpDecorate %5 Binding 0 OpDecorate %6 BufferBlock @@ -573,6 +665,52 @@ TEST_F(DescriptorTypeTest, AliasedNotReadOnlyStorageBuffer) { 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 From e5b75524fdd6b388c260f384b112345bb34cb6ca Mon Sep 17 00:00:00 2001 From: Alister Chowdhury Date: Sat, 13 Dec 2025 18:42:43 +0000 Subject: [PATCH 8/9] Changed nonwritable logic to just count, rather than use a bitfield or vector --- source/opt/instruction.cpp | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/source/opt/instruction.cpp b/source/opt/instruction.cpp index 04fc609ba7..297679177d 100644 --- a/source/opt/instruction.cpp +++ b/source/opt/instruction.cpp @@ -450,32 +450,20 @@ bool Instruction::IsVulkanStorageBufferNonWritable() const { } assert(base_type->opcode() == spv::Op::OpTypeStruct); - // Check that all individual members of the struct are NonWritable. - analysis::DecorationManager* decoration_mgr = context()->get_decoration_mgr(); - - if (base_type->NumInOperands() <= 64) { - uint64_t writeable_mask = (1llu << (base_type->NumInOperands())) - 1; - decoration_mgr->ForEachDecoration( - base_type->result_id(), - static_cast(spv::Decoration::NonWritable), - [&writeable_mask](const Instruction& decr) { - if (decr.opcode() == spv::Op::OpMemberDecorate) { - writeable_mask &= ~(1llu << decr.GetSingleWordInOperand(1)); - } - }); - return !writeable_mask; - } - - std::vector nonwritable_members; - decoration_mgr->ForEachDecoration( + // 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.push_back(decr.GetSingleWordInOperand(1)); + ++nonwritable_members; } }); - return nonwritable_members.size() == base_type->NumInOperands(); + + return nonwritable_members == base_type->NumInOperands(); } bool Instruction::IsVulkanStorageBufferVariable() const { From 2125eb5a0ad60ec3d3e4189016b6618333f55540 Mon Sep 17 00:00:00 2001 From: Steven Perron Date: Fri, 27 Mar 2026 13:37:47 -0400 Subject: [PATCH 9/9] Format code --- source/opt/instruction.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/opt/instruction.h b/source/opt/instruction.h index b79eaf0208..3f0de17f42 100644 --- a/source/opt/instruction.h +++ b/source/opt/instruction.h @@ -640,8 +640,8 @@ class Instruction : public utils::IntrusiveNodeBase { // 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. + // 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