From 40f5bf59c6acb4754a0bffd3c53a715732883a12 Mon Sep 17 00:00:00 2001 From: Cassandra Beckley Date: Mon, 5 Dec 2022 20:31:01 +0000 Subject: [PATCH] Revert "spirv-val: Multiple interface var with same SC (#4969)" (#5009) This reverts commit 996d4c021f7112356b305b7172fd722d02eefdb5. This commit is likely good, but it causes a failure in DXC tests. Will un-revert once we fix the issue in DXC. --- source/val/validate_interfaces.cpp | 61 -------------------- source/val/validation_state.cpp | 8 --- test/val/val_decoration_test.cpp | 42 -------------- test/val/val_ray_tracing_test.cpp | 89 ------------------------------ 4 files changed, 200 deletions(-) diff --git a/source/val/validate_interfaces.cpp b/source/val/validate_interfaces.cpp index 54152da7f..00a5999bd 100644 --- a/source/val/validate_interfaces.cpp +++ b/source/val/validate_interfaces.cpp @@ -537,64 +537,6 @@ spv_result_t ValidateLocations(ValidationState_t& _, return SPV_SUCCESS; } -spv_result_t ValidateStorageClass(ValidationState_t& _, - const Instruction* entry_point) { - bool has_push_constant = false; - bool has_ray_payload = false; - bool has_hit_attribute = false; - bool has_callable_data = false; - for (uint32_t i = 3; i < entry_point->operands().size(); ++i) { - auto interface_id = entry_point->GetOperandAs(i); - auto interface_var = _.FindDef(interface_id); - auto storage_class = interface_var->GetOperandAs(2); - switch (storage_class) { - case spv::StorageClass::PushConstant: { - if (has_push_constant) { - return _.diag(SPV_ERROR_INVALID_DATA, entry_point) - << _.VkErrorID(6673) - << "Entry-point has more than one variable with the " - "PushConstant storage class in the interface"; - } - has_push_constant = true; - break; - } - case spv::StorageClass::IncomingRayPayloadKHR: { - if (has_ray_payload) { - return _.diag(SPV_ERROR_INVALID_DATA, entry_point) - << _.VkErrorID(4700) - << "Entry-point has more than one variable with the " - "IncomingRayPayloadKHR storage class in the interface"; - } - has_ray_payload = true; - break; - } - case spv::StorageClass::HitAttributeKHR: { - if (has_hit_attribute) { - return _.diag(SPV_ERROR_INVALID_DATA, entry_point) - << _.VkErrorID(4702) - << "Entry-point has more than one variable with the " - "HitAttributeKHR storage class in the interface"; - } - has_hit_attribute = true; - break; - } - case spv::StorageClass::IncomingCallableDataKHR: { - if (has_callable_data) { - return _.diag(SPV_ERROR_INVALID_DATA, entry_point) - << _.VkErrorID(4706) - << "Entry-point has more than one variable with the " - "IncomingCallableDataKHR storage class in the interface"; - } - has_callable_data = true; - break; - } - default: - break; - } - } - return SPV_SUCCESS; -} - } // namespace spv_result_t ValidateInterfaces(ValidationState_t& _) { @@ -613,9 +555,6 @@ spv_result_t ValidateInterfaces(ValidationState_t& _) { if (auto error = ValidateLocations(_, &inst)) { return error; } - if (auto error = ValidateStorageClass(_, &inst)) { - return error; - } } if (inst.opcode() == spv::Op::OpTypeVoid) break; } diff --git a/source/val/validation_state.cpp b/source/val/validation_state.cpp index 78ffda10b..c95eec366 100644 --- a/source/val/validation_state.cpp +++ b/source/val/validation_state.cpp @@ -2077,20 +2077,14 @@ std::string ValidationState_t::VkErrorID(uint32_t id, return VUID_WRAP(VUID-StandaloneSpirv-RayPayloadKHR-04698); case 4699: return VUID_WRAP(VUID-StandaloneSpirv-IncomingRayPayloadKHR-04699); - case 4700: - return VUID_WRAP(VUID-StandaloneSpirv-IncomingRayPayloadKHR-04700); case 4701: return VUID_WRAP(VUID-StandaloneSpirv-HitAttributeKHR-04701); - case 4702: - return VUID_WRAP(VUID-StandaloneSpirv-HitAttributeKHR-04702); case 4703: return VUID_WRAP(VUID-StandaloneSpirv-HitAttributeKHR-04703); case 4704: return VUID_WRAP(VUID-StandaloneSpirv-CallableDataKHR-04704); case 4705: return VUID_WRAP(VUID-StandaloneSpirv-IncomingCallableDataKHR-04705); - case 4706: - return VUID_WRAP(VUID-StandaloneSpirv-IncomingCallableDataKHR-04706); case 7119: return VUID_WRAP(VUID-StandaloneSpirv-ShaderRecordBufferKHR-07119); case 4708: @@ -2149,8 +2143,6 @@ std::string ValidationState_t::VkErrorID(uint32_t id, return VUID_WRAP(VUID-StandaloneSpirv-OpTypeSampledImage-06671); case 6672: return VUID_WRAP(VUID-StandaloneSpirv-Location-06672); - case 6673: - return VUID_WRAP(VUID-StandaloneSpirv-OpVariable-06673); case 6674: return VUID_WRAP(VUID-StandaloneSpirv-OpEntryPoint-06674); case 6675: diff --git a/test/val/val_decoration_test.cpp b/test/val/val_decoration_test.cpp index dae6c2672..ff62f4b0c 100644 --- a/test/val/val_decoration_test.cpp +++ b/test/val/val_decoration_test.cpp @@ -3210,48 +3210,6 @@ TEST_F(ValidateDecorations, "statically used per shader entry point.")); } -TEST_F(ValidateDecorations, - VulkanMultiplePushConstantsSingleEntryPointInterfaceBad) { - std::string spirv = R"( - OpCapability Shader - OpMemoryModel Logical GLSL450 - OpEntryPoint Vertex %func1 "func1" %pc1 %pc2 - OpDecorate %struct Block - OpMemberDecorate %struct 0 Offset 0 - %void = OpTypeVoid - %voidfn = OpTypeFunction %void - %float = OpTypeFloat 32 - %int = OpTypeInt 32 0 - %int_0 = OpConstant %int 0 - %struct = OpTypeStruct %float - %ptr = OpTypePointer PushConstant %struct -%ptr_float = OpTypePointer PushConstant %float - %pc1 = OpVariable %ptr PushConstant - %pc2 = OpVariable %ptr PushConstant - %func1 = OpFunction %void None %voidfn - %label1 = OpLabel - %access1 = OpAccessChain %ptr_float %pc1 %int_0 - %load1 = OpLoad %float %access1 - OpReturn - OpFunctionEnd - %func2 = OpFunction %void None %voidfn - %label2 = OpLabel - %access2 = OpAccessChain %ptr_float %pc2 %int_0 - %load2 = OpLoad %float %access2 - OpReturn - OpFunctionEnd -)"; - - CompileSuccessfully(spirv, SPV_ENV_VULKAN_1_2); - EXPECT_EQ(SPV_ERROR_INVALID_DATA, - ValidateAndRetrieveValidationState(SPV_ENV_VULKAN_1_2)); - EXPECT_THAT(getDiagnosticString(), - AnyVUID("VUID-StandaloneSpirv-OpVariable-06673")); - EXPECT_THAT(getDiagnosticString(), - HasSubstr("Entry-point has more than one variable with the " - "PushConstant storage class in the interface")); -} - TEST_F(ValidateDecorations, VulkanUniformMissingDescriptorSetBad) { std::string spirv = R"( OpCapability Shader diff --git a/test/val/val_ray_tracing_test.cpp b/test/val/val_ray_tracing_test.cpp index 60f2f8911..58b9356ce 100644 --- a/test/val/val_ray_tracing_test.cpp +++ b/test/val/val_ray_tracing_test.cpp @@ -578,95 +578,6 @@ OpTraceRayKHR %as %uint_1 %uint_1 %uint_1 %uint_1 %uint_1 %v3composite %float_0 "IncomingRayPayloadKHR")); } -TEST_F(ValidateRayTracing, InterfaceIncomingRayPayload) { - const std::string body = R"( -OpCapability RayTracingKHR -OpExtension "SPV_KHR_ray_tracing" -OpMemoryModel Logical GLSL450 -OpEntryPoint CallableKHR %main "main" %inData1 %inData2 -OpName %main "main" -%void = OpTypeVoid -%func = OpTypeFunction %void -%int = OpTypeInt 32 1 -%inData_ptr = OpTypePointer IncomingRayPayloadKHR %int -%inData1 = OpVariable %inData_ptr IncomingRayPayloadKHR -%inData2 = OpVariable %inData_ptr IncomingRayPayloadKHR -%main = OpFunction %void None %func -%label = OpLabel -OpReturn -OpFunctionEnd -)"; - - CompileSuccessfully(body.c_str(), SPV_ENV_VULKAN_1_2); - EXPECT_EQ(SPV_ERROR_INVALID_DATA, - ValidateAndRetrieveValidationState(SPV_ENV_VULKAN_1_2)); - EXPECT_THAT(getDiagnosticString(), - AnyVUID("VUID-StandaloneSpirv-IncomingRayPayloadKHR-04700")); - EXPECT_THAT( - getDiagnosticString(), - HasSubstr("Entry-point has more than one variable with the " - "IncomingRayPayloadKHR storage class in the interface")); -} - -TEST_F(ValidateRayTracing, InterfaceHitAttribute) { - const std::string body = R"( -OpCapability RayTracingKHR -OpExtension "SPV_KHR_ray_tracing" -OpMemoryModel Logical GLSL450 -OpEntryPoint CallableKHR %main "main" %inData1 %inData2 -OpName %main "main" -%void = OpTypeVoid -%func = OpTypeFunction %void -%int = OpTypeInt 32 1 -%inData_ptr = OpTypePointer HitAttributeKHR %int -%inData1 = OpVariable %inData_ptr HitAttributeKHR -%inData2 = OpVariable %inData_ptr HitAttributeKHR -%main = OpFunction %void None %func -%label = OpLabel -OpReturn -OpFunctionEnd -)"; - - CompileSuccessfully(body.c_str(), SPV_ENV_VULKAN_1_2); - EXPECT_EQ(SPV_ERROR_INVALID_DATA, - ValidateAndRetrieveValidationState(SPV_ENV_VULKAN_1_2)); - EXPECT_THAT(getDiagnosticString(), - AnyVUID("VUID-StandaloneSpirv-HitAttributeKHR-04702")); - EXPECT_THAT(getDiagnosticString(), - HasSubstr("Entry-point has more than one variable with the " - "HitAttributeKHR storage class in the interface")); -} - -TEST_F(ValidateRayTracing, InterfaceIncomingCallableData) { - const std::string body = R"( -OpCapability RayTracingKHR -OpExtension "SPV_KHR_ray_tracing" -OpMemoryModel Logical GLSL450 -OpEntryPoint CallableKHR %main "main" %inData1 %inData2 -OpName %main "main" -%void = OpTypeVoid -%func = OpTypeFunction %void -%int = OpTypeInt 32 1 -%inData_ptr = OpTypePointer IncomingCallableDataKHR %int -%inData1 = OpVariable %inData_ptr IncomingCallableDataKHR -%inData2 = OpVariable %inData_ptr IncomingCallableDataKHR -%main = OpFunction %void None %func -%label = OpLabel -OpReturn -OpFunctionEnd -)"; - - CompileSuccessfully(body.c_str(), SPV_ENV_VULKAN_1_2); - EXPECT_EQ(SPV_ERROR_INVALID_DATA, - ValidateAndRetrieveValidationState(SPV_ENV_VULKAN_1_2)); - EXPECT_THAT(getDiagnosticString(), - AnyVUID("VUID-StandaloneSpirv-IncomingCallableDataKHR-04706")); - EXPECT_THAT( - getDiagnosticString(), - HasSubstr("Entry-point has more than one variable with the " - "IncomingCallableDataKHR storage class in the interface")); -} - } // namespace } // namespace val } // namespace spvtools