spirv-val: Add Patch Decoration check (KhronosGroup#6219)

Author: spencer-lunarg

source/val/validate_builtins.cpp

@@ -2692,6 +2692,13 @@ spv_result_t BuiltInsValidator::ValidateTessLevelOuterAtDefinition(
             })) {
       return error;
     }
+
+    if (!_.HasDecoration(inst.id(), spv::Decoration::Patch)) {
+      return _.diag(SPV_ERROR_INVALID_DATA, &inst)
+             << _.VkErrorID(10880)
+             << "BuiltIn TessLevelOuter variable needs to also have a Patch "
+                "decoration.";
+    }
   }
 
   // Seed at reference checks with this built-in.
@@ -2706,13 +2713,20 @@ spv_result_t BuiltInsValidator::ValidateTessLevelInnerAtDefinition(
             [this, &inst](const std::string& message) -> spv_result_t {
               return _.diag(SPV_ERROR_INVALID_DATA, &inst)
                      << _.VkErrorID(4397)
-                     << "According to the Vulkan spec BuiltIn TessLevelOuter "
+                     << "According to the Vulkan spec BuiltIn TessLevelInner "
                         "variable needs to be a 2-component 32-bit float "
                         "array. "
                      << message;
             })) {
       return error;
     }
+
+    if (!_.HasDecoration(inst.id(), spv::Decoration::Patch)) {
+      return _.diag(SPV_ERROR_INVALID_DATA, &inst)
+             << _.VkErrorID(10880)
+             << "BuiltIn TessLevelInner variable needs to also have a Patch "
+                "decoration.";
+    }
   }
 
   // Seed at reference checks with this built-in.

source/val/validation_state.cpp

@@ -2698,6 +2698,8 @@ std::string ValidationState_t::VkErrorID(uint32_t id,
     case 10824:
       // This use to be a standalone, but maintenance9 will set allow_vulkan_32_bit_bitwise now
       return VUID_WRAP(VUID-RuntimeSpirv-None-10824);
+    case 10880:
+      return VUID_WRAP(VUID-StandaloneSpirv-TessLevelInner-10880);
     default:
       return "";  // unknown id
   }

test/val/val_builtins_test.cpp

@@ -99,6 +99,10 @@ CodeGenerator GetInMainCodeGenerator(const char* const built_in,
   generator.before_types_ += built_in;
   generator.before_types_ += "\n";
 
+  if (strncmp(built_in, "TessLevel", 9) == 0) {
+    generator.before_types_ += "OpMemberDecorate %built_in_type 0 Patch\n";
+  }
+
   std::ostringstream after_types;
 
   after_types << "%built_in_type = OpTypeStruct " << data_type << "\n";
@@ -258,6 +262,10 @@ CodeGenerator GetInFunctionCodeGenerator(const char* const built_in,
   generator.before_types_ += built_in;
   generator.before_types_ += "\n";
 
+  if (strncmp(built_in, "TessLevel", 9) == 0) {
+    generator.before_types_ += "OpMemberDecorate %built_in_type 0 Patch\n";
+  }
+
   std::ostringstream after_types;
   after_types << "%built_in_type = OpTypeStruct " << data_type << "\n";
   if (InitializerRequired(storage_class)) {
@@ -395,6 +403,11 @@ CodeGenerator GetVariableCodeGenerator(const char* const built_in,
   generator.before_types_ = "OpDecorate %built_in_var BuiltIn ";
   generator.before_types_ += built_in;
   generator.before_types_ += "\n";
+
+  if (strncmp(built_in, "TessLevel", 9) == 0) {
+    generator.before_types_ += "OpDecorate %built_in_var Patch\n";
+  }
+
   if ((0 == std::strcmp(storage_class, "Input")) &&
       (0 == std::strcmp(execution_model, "Fragment"))) {
     // ensure any needed input types that might require Flat
@@ -6550,6 +6563,50 @@ TEST_F(ValidateBuiltIns, BadVulkanBuiltinPrimitiveIdFragmentWithRayTracing) {
               AnyVUID("VUID-PrimitiveId-PrimitiveId-04333"));
 }
 
+TEST_F(ValidateBuiltIns, TessellationMissingPatch) {
+  const std::string spirv = R"(
+               OpCapability Tessellation
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint TessellationControl %main "main" %gl_TessLevelInner %gl_TessLevelOuter
+               OpExecutionMode %main OutputVertices 3
+               OpDecorate %gl_TessLevelInner BuiltIn TessLevelInner
+               OpDecorate %gl_TessLevelOuter BuiltIn TessLevelOuter
+               OpDecorate %gl_TessLevelOuter Patch
+       %void = OpTypeVoid
+          %4 = OpTypeFunction %void
+      %float = OpTypeFloat 32
+       %uint = OpTypeInt 32 0
+     %uint_2 = OpConstant %uint 2
+%_arr_float_uint_2 = OpTypeArray %float %uint_2
+%_ptr_Output__arr_float_uint_2 = OpTypePointer Output %_arr_float_uint_2
+%gl_TessLevelInner = OpVariable %_ptr_Output__arr_float_uint_2 Output
+        %int = OpTypeInt 32 1
+      %int_0 = OpConstant %int 0
+    %float_1 = OpConstant %float 1
+%_ptr_Output_float = OpTypePointer Output %float
+     %uint_4 = OpConstant %uint 4
+%_arr_float_uint_4 = OpTypeArray %float %uint_4
+%_ptr_Output__arr_float_uint_4 = OpTypePointer Output %_arr_float_uint_4
+%gl_TessLevelOuter = OpVariable %_ptr_Output__arr_float_uint_4 Output
+       %main = OpFunction %void None %4
+          %6 = OpLabel
+         %17 = OpAccessChain %_ptr_Output_float %gl_TessLevelInner %int_0
+               OpStore %17 %float_1
+         %22 = OpAccessChain %_ptr_Output_float %gl_TessLevelOuter %int_0
+               OpStore %22 %float_1
+               OpReturn
+               OpFunctionEnd
+)";
+
+  CompileSuccessfully(spirv, SPV_ENV_VULKAN_1_0);
+  EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_VULKAN_1_0));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("BuiltIn TessLevelInner variable needs to also have a "
+                        "Patch decoration"));
+  EXPECT_THAT(getDiagnosticString(),
+              AnyVUID("VUID-StandaloneSpirv-TessLevelInner-10880"));
+}
+
 }  // namespace
 }  // namespace val
 }  // namespace spvtools