spirv-val: Multiple interface var with same SC (KhronosGroup#5528)

spencer-lunarg · web-flow · commit 20ad38c18d3f · 2024-02-13T15:55:43.000-05:00
diff --git a/source/val/validate_interfaces.cpp b/source/val/validate_interfaces.cpp
@@ -587,6 +587,64 @@ 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<uint32_t>(i);
+    auto interface_var = _.FindDef(interface_id);
+    auto storage_class = interface_var->GetOperandAs<spv::StorageClass>(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& _) {
@@ -605,6 +663,9 @@ 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
@@ -2173,14 +2173,20 @@ 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:
@@ -2239,6 +2245,8 @@ 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
@@ -3209,6 +3209,48 @@ 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
@@ -578,6 +578,95 @@ 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
