diff options
author | sfricke-samsung <46493288+sfricke-samsung@users.noreply.github.com> | 2021-02-05 06:49:14 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-02-05 09:49:14 -0500 |
commit | f11f7434815838bbad349124767b258ce7df41f0 (patch) | |
tree | dcca96ece5545a535693aac148869814f554826d | |
parent | c91a25af13e112729be7272d58b0df25e772c3e0 (diff) | |
download | spirv-tools-f11f7434815838bbad349124767b258ce7df41f0.tar.gz |
spirv-val: Add Vulkan Invariant Decoration VUID (#4132)
-rw-r--r-- | source/val/validate_memory.cpp | 36 | ||||
-rw-r--r-- | source/val/validation_state.cpp | 2 | ||||
-rw-r--r-- | test/val/val_memory_test.cpp | 103 |
3 files changed, 133 insertions, 8 deletions
diff --git a/source/val/validate_memory.cpp b/source/val/validate_memory.cpp index edc2a0ce..2318c962 100644 --- a/source/val/validate_memory.cpp +++ b/source/val/validate_memory.cpp @@ -407,6 +407,10 @@ spv_result_t ValidateVariable(ValidationState_t& _, const Instruction* inst) { << "' is not a pointer type."; } + const auto type_index = 2; + const auto value_id = result_type->GetOperandAs<uint32_t>(type_index); + auto value_type = _.FindDef(value_id); + const auto initializer_index = 3; const auto storage_class_index = 2; if (initializer_index < inst->operands().size()) { @@ -423,7 +427,7 @@ spv_result_t ValidateVariable(ValidationState_t& _, const Instruction* inst) { << "OpVariable Initializer <id> '" << _.getIdName(initializer_id) << "' is not a constant or module-scope variable."; } - if (initializer->type_id() != result_type->GetOperandAs<uint32_t>(2u)) { + if (initializer->type_id() != value_id) { return _.diag(SPV_ERROR_INVALID_ID, inst) << "Initializer type must match the type pointed to by the Result " "Type"; @@ -440,9 +444,6 @@ spv_result_t ValidateVariable(ValidationState_t& _, const Instruction* inst) { storage_class != SpvStorageClassHitAttributeNV && storage_class != SpvStorageClassCallableDataNV && storage_class != SpvStorageClassIncomingCallableDataNV) { - const auto storage_index = 2; - const auto storage_id = result_type->GetOperandAs<uint32_t>(storage_index); - const auto storage = _.FindDef(storage_id); bool storage_input_or_output = storage_class == SpvStorageClassInput || storage_class == SpvStorageClassOutput; bool builtin = false; @@ -455,7 +456,7 @@ spv_result_t ValidateVariable(ValidationState_t& _, const Instruction* inst) { } } if (!(storage_input_or_output && builtin) && - ContainsInvalidBool(_, storage, storage_input_or_output)) { + ContainsInvalidBool(_, value_type, storage_input_or_output)) { return _.diag(SPV_ERROR_INVALID_ID, inst) << "If OpTypeBool is stored in conjunction with OpVariable, it " << "can only be used with non-externally visible shader Storage " @@ -576,6 +577,28 @@ spv_result_t ValidateVariable(ValidationState_t& _, const Instruction* inst) { "of this type"; } } + + // Check for invalid use of Invariant + if (storage_class != SpvStorageClassInput && + storage_class != SpvStorageClassOutput) { + if (_.HasDecoration(inst->id(), SpvDecorationInvariant)) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << _.VkErrorID(4677) + << "Variable decorated with Invariant must only be identified " + "with the Input or Output storage class in Vulkan " + "environment."; + } + // Need to check if only the members in a struct are decorated + if (value_type && value_type->opcode() == SpvOpTypeStruct) { + if (_.HasDecoration(value_id, SpvDecorationInvariant)) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << _.VkErrorID(4677) + << "Variable struct member decorated with Invariant must only " + "be identified with the Input or Output storage class in " + "Vulkan environment."; + } + } + } } // Vulkan Appendix A: Check that if contains initializer, then @@ -640,9 +663,6 @@ spv_result_t ValidateVariable(ValidationState_t& _, const Instruction* inst) { } // Vulkan specific validation rules for OpTypeRuntimeArray - const auto type_index = 2; - const auto value_id = result_type->GetOperandAs<uint32_t>(type_index); - auto value_type = _.FindDef(value_id); if (spvIsVulkanEnv(_.context()->target_env)) { // OpTypeRuntimeArray should only ever be in a container like OpTypeStruct, // so should never appear as a bare variable. diff --git a/source/val/validation_state.cpp b/source/val/validation_state.cpp index 13323a39..10c0fcdf 100644 --- a/source/val/validation_state.cpp +++ b/source/val/validation_state.cpp @@ -1710,6 +1710,8 @@ std::string ValidationState_t::VkErrorID(uint32_t id, return VUID_WRAP(VUID-StandaloneSpirv-GLSLShared-04669); case 4675: return VUID_WRAP(VUID-StandaloneSpirv-FPRoundingMode-04675); + case 4677: + return VUID_WRAP(VUID-StandaloneSpirv-Invariant-04677); case 4683: return VUID_WRAP(VUID-StandaloneSpirv-LocalSize-04683); case 4685: diff --git a/test/val/val_memory_test.cpp b/test/val/val_memory_test.cpp index 7d66dcd4..958476cb 100644 --- a/test/val/val_memory_test.cpp +++ b/test/val/val_memory_test.cpp @@ -4042,6 +4042,109 @@ OpFunctionEnd "typed as OpTypeStruct, or an array of this type")); } +TEST_F(ValidateMemory, VulkanInvariantOutputSuccess) { + const std::string spirv = R"( +OpCapability Shader +OpMemoryModel Logical GLSL450 +OpEntryPoint Vertex %main "main" +OpDecorate %var Location 0 +OpDecorate %var Invariant +%void = OpTypeVoid +%f32 = OpTypeFloat 32 +%ptr_output = OpTypePointer Output %f32 +%var = OpVariable %ptr_output Output +%void_fn = OpTypeFunction %void +%main = OpFunction %void None %void_fn +%entry = OpLabel +OpReturn +OpFunctionEnd +)"; + + CompileSuccessfully(spirv, SPV_ENV_VULKAN_1_0); + EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_0)); +} + +TEST_F(ValidateMemory, VulkanInvariantInputStructSuccess) { + const std::string spirv = R"( +OpCapability Shader +OpMemoryModel Logical GLSL450 +OpEntryPoint Fragment %main "main" +OpExecutionMode %main OriginUpperLeft +OpDecorate %var Location 0 +OpMemberDecorate %struct 1 Invariant +%void = OpTypeVoid +%f32 = OpTypeFloat 32 +%struct = OpTypeStruct %f32 %f32 +%ptr_input = OpTypePointer Input %struct +%var = OpVariable %ptr_input Input +%void_fn = OpTypeFunction %void +%main = OpFunction %void None %void_fn +%entry = OpLabel +OpReturn +OpFunctionEnd +)"; + + CompileSuccessfully(spirv, SPV_ENV_VULKAN_1_0); + EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_0)); +} + +TEST_F(ValidateMemory, VulkanInvariantWrongStorageClass) { + const std::string spirv = R"( +OpCapability Shader +OpMemoryModel Logical GLSL450 +OpEntryPoint Vertex %main "main" +OpDecorate %var Invariant +%void = OpTypeVoid +%f32 = OpTypeFloat 32 +%ptr_private = OpTypePointer Private %f32 +%var = OpVariable %ptr_private Private +%void_fn = OpTypeFunction %void +%main = OpFunction %void None %void_fn +%entry = OpLabel +OpReturn +OpFunctionEnd +)"; + + CompileSuccessfully(spirv, SPV_ENV_VULKAN_1_0); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_0)); + EXPECT_THAT(getDiagnosticString(), + AnyVUID("VUID-StandaloneSpirv-Invariant-04677")); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr( + "Variable decorated with Invariant must only be identified with the " + "Input or Output storage class in Vulkan environment.")); +} + +TEST_F(ValidateMemory, VulkanInvariantMemberWrongStorageClass) { + const std::string spirv = R"( +OpCapability Shader +OpMemoryModel Logical GLSL450 +OpEntryPoint Fragment %main "main" +OpExecutionMode %main OriginUpperLeft +OpMemberDecorate %struct 1 Invariant +%void = OpTypeVoid +%f32 = OpTypeFloat 32 +%struct = OpTypeStruct %f32 %f32 +%ptr_private = OpTypePointer Private %struct +%var = OpVariable %ptr_private Private +%void_fn = OpTypeFunction %void +%main = OpFunction %void None %void_fn +%entry = OpLabel +OpReturn +OpFunctionEnd +)"; + + CompileSuccessfully(spirv, SPV_ENV_VULKAN_1_0); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_0)); + EXPECT_THAT(getDiagnosticString(), + AnyVUID("VUID-StandaloneSpirv-Invariant-04677")); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("Variable struct member decorated with Invariant must " + "only be identified with the Input or Output storage " + "class in Vulkan environment.")); +} + TEST_F(ValidateMemory, PhysicalStorageBufferPtrEqual) { const std::string spirv = R"( OpCapability Shader |