From 0045b01ff9648eeef1781282f0454ae584f3ace7 Mon Sep 17 00:00:00 2001 From: Natalie Chouinard Date: Thu, 25 Jan 2024 14:05:04 -0500 Subject: [PATCH] opt: Add VulkanMemoryModelDeviceScope to trim (#5544) Add the VulkanMemoryModelDeviceScope capability to the capability trimming pass. According the the spec, "If the Vulkan memory model is declared and any instruction uses Device scope, the VulkanMemoryModelDeviceScope capability must be declared." Since this case, based on the type of an operand, is not covered by the JSON grammar, it is added explicitly. --- source/opt/ir_context.h | 6 ++ source/opt/trim_capabilities_pass.cpp | 11 ++++ source/opt/trim_capabilities_pass.h | 3 +- test/opt/trim_capabilities_pass_test.cpp | 76 ++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 1 deletion(-) diff --git a/source/opt/ir_context.h b/source/opt/ir_context.h index de3c41066..5685db809 100644 --- a/source/opt/ir_context.h +++ b/source/opt/ir_context.h @@ -229,6 +229,8 @@ class IRContext { inline void AddExtInstImport(std::unique_ptr&& e); // Set the memory model for this module. inline void SetMemoryModel(std::unique_ptr&& m); + // Get the memory model for this module. + inline const Instruction* GetMemoryModel() const; // Appends an entry point instruction to this module. inline void AddEntryPoint(std::unique_ptr&& e); // Appends an execution mode instruction to this module. @@ -1156,6 +1158,10 @@ void IRContext::SetMemoryModel(std::unique_ptr&& m) { module()->SetMemoryModel(std::move(m)); } +const Instruction* IRContext::GetMemoryModel() const { + return module()->GetMemoryModel(); +} + void IRContext::AddEntryPoint(std::unique_ptr&& e) { module()->AddEntryPoint(std::move(e)); } diff --git a/source/opt/trim_capabilities_pass.cpp b/source/opt/trim_capabilities_pass.cpp index 19f8569e0..24f9e4670 100644 --- a/source/opt/trim_capabilities_pass.cpp +++ b/source/opt/trim_capabilities_pass.cpp @@ -448,6 +448,17 @@ void TrimCapabilitiesPass::addInstructionRequirementsForOperand( return; } + // If the Vulkan memory model is declared and any instruction uses Device + // scope, the VulkanMemoryModelDeviceScope capability must be declared. This + // rule cannot be covered by the grammar, so must be checked explicitly. + if (operand.type == SPV_OPERAND_TYPE_SCOPE_ID) { + const Instruction* memory_model = context()->GetMemoryModel(); + if (memory_model && memory_model->GetSingleWordInOperand(1u) == + uint32_t(spv::MemoryModel::Vulkan)) { + capabilities->insert(spv::Capability::VulkanMemoryModelDeviceScope); + } + } + // case 1: Operand is a single value, can directly lookup. if (!spvOperandIsConcreteMask(operand.type)) { const spv_operand_desc_t* desc = {}; diff --git a/source/opt/trim_capabilities_pass.h b/source/opt/trim_capabilities_pass.h index 9f2373299..3a8460f3b 100644 --- a/source/opt/trim_capabilities_pass.h +++ b/source/opt/trim_capabilities_pass.h @@ -97,7 +97,8 @@ class TrimCapabilitiesPass : public Pass { spv::Capability::StorageInputOutput16, spv::Capability::StoragePushConstant16, spv::Capability::StorageUniform16, - spv::Capability::StorageUniformBufferBlock16 + spv::Capability::StorageUniformBufferBlock16, + spv::Capability::VulkanMemoryModelDeviceScope // clang-format on }; diff --git a/test/opt/trim_capabilities_pass_test.cpp b/test/opt/trim_capabilities_pass_test.cpp index c90afb4c6..d74ccdf2f 100644 --- a/test/opt/trim_capabilities_pass_test.cpp +++ b/test/opt/trim_capabilities_pass_test.cpp @@ -2584,6 +2584,82 @@ TEST_F(TrimCapabilitiesPassTest, UInt16_RemainsWhenUsed) { EXPECT_EQ(std::get<1>(result), Pass::Status::SuccessWithoutChange); } +TEST_F(TrimCapabilitiesPassTest, + VulkanMemoryModelDeviceScope_RemovedWhenUnused) { + const std::string kTest = R"( + OpCapability VulkanMemoryModelDeviceScope +; CHECK-NOT: OpCapability VulkanMemoryModelDeviceScope + OpCapability Shader + OpMemoryModel Logical GLSL450 + OpEntryPoint GLCompute %1 "main" + %void = OpTypeVoid + %3 = OpTypeFunction %void + %1 = OpFunction %void None %3 + %6 = OpLabel + OpReturn + OpFunctionEnd; + )"; + const auto result = + SinglePassRunAndMatch(kTest, /* skip_nop= */ false); + EXPECT_EQ(std::get<1>(result), Pass::Status::SuccessWithChange); +} + +TEST_F(TrimCapabilitiesPassTest, + VulkanMemoryModelDeviceScope_RemovedWhenUsedWithGLSL450) { + const std::string kTest = R"( + OpCapability VulkanMemoryModelDeviceScope +; CHECK-NOT: OpCapability VulkanMemoryModelDeviceScope + OpCapability Shader + OpCapability ShaderClockKHR + OpCapability Int64 + OpExtension "SPV_KHR_shader_clock" + OpMemoryModel Logical GLSL450 + OpEntryPoint GLCompute %main "main" + OpExecutionMode %main LocalSize 1 2 4 + %void = OpTypeVoid + %uint = OpTypeInt 32 0 + %ulong = OpTypeInt 64 0 + %uint_1 = OpConstant %uint 1 + %3 = OpTypeFunction %void + %main = OpFunction %void None %3 + %6 = OpLabel + %22 = OpReadClockKHR %ulong %uint_1 ; Device Scope + OpReturn + OpFunctionEnd + )"; + const auto result = + SinglePassRunAndMatch(kTest, /* skip_nop= */ false); + EXPECT_EQ(std::get<1>(result), Pass::Status::SuccessWithChange); +} + +TEST_F(TrimCapabilitiesPassTest, + VulkanMemoryModelDeviceScope_RemainsWhenUsedWithVulkan) { + const std::string kTest = R"( + OpCapability VulkanMemoryModelDeviceScope +; CHECK: OpCapability VulkanMemoryModelDeviceScope + OpCapability Shader + OpCapability ShaderClockKHR + OpCapability Int64 + OpExtension "SPV_KHR_shader_clock" + OpMemoryModel Logical Vulkan + OpEntryPoint GLCompute %main "main" + OpExecutionMode %main LocalSize 1 2 4 + %void = OpTypeVoid + %uint = OpTypeInt 32 0 + %ulong = OpTypeInt 64 0 + %uint_1 = OpConstant %uint 1 + %3 = OpTypeFunction %void + %main = OpFunction %void None %3 + %6 = OpLabel + %22 = OpReadClockKHR %ulong %uint_1 ; Device Scope + OpReturn + OpFunctionEnd + )"; + const auto result = + SinglePassRunAndMatch(kTest, /* skip_nop= */ false); + EXPECT_EQ(std::get<1>(result), Pass::Status::SuccessWithoutChange); +} + } // namespace } // namespace opt } // namespace spvtools