OpenCL.DebugInfo.100 DebugTypeArray with variable size (#3549)

The updated OpenCL.DebugInfo.100 spec says that we can use
variable for "Component Count" operand of DebugTypeArray i.e.,
DebugLocalVariable or DebugGlobalVariable. This commit updates spirv-val
based on the spec update.

source/val/validate_extensions.cpp

@@ -636,6 +636,14 @@ spv_result_t ValidateClspvReflectionInstruction(ValidationState_t& _,
   return SPV_SUCCESS;
 }
 
+bool IsConstIntScalarTypeWith32Or64Bits(ValidationState_t& _,
+                                        Instruction* instr) {
+  if (instr->opcode() != SpvOpConstant) return false;
+  if (!_.IsIntScalarType(instr->type_id())) return false;
+  uint32_t size_in_bits = _.GetBitWidth(instr->type_id());
+  return size_in_bits == 32 || size_in_bits == 64;
+}
+
 }  // anonymous namespace
 
 spv_result_t ValidateExtension(ValidationState_t& _, const Instruction* inst) {
@@ -2682,13 +2690,49 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
             ValidateOperandDebugType(_, "Base Type", inst, 5, ext_inst_name);
         if (validate_base_type != SPV_SUCCESS) return validate_base_type;
         for (uint32_t i = 6; i < num_words; ++i) {
-          CHECK_OPERAND("Component Count", SpvOpConstant, i);
+          bool invalid = false;
           auto* component_count = _.FindDef(inst->word(i));
-          if (!_.IsIntScalarType(component_count->type_id()) ||
+          if (IsConstIntScalarTypeWith32Or64Bits(_, component_count)) {
-              !component_count->word(3)) {
+            // TODO: We need a spec discussion for the bindless array.
+            if (!component_count->word(3)) {
+              invalid = true;
+            }
+          } else if (component_count->words().size() > 6 &&
+                     (OpenCLDebugInfo100Instructions(component_count->word(
+                          4)) == OpenCLDebugInfo100DebugLocalVariable ||
+                      OpenCLDebugInfo100Instructions(component_count->word(
+                          4)) == OpenCLDebugInfo100DebugGlobalVariable)) {
+            auto* component_count_type = _.FindDef(component_count->word(6));
+            if (component_count_type->words().size() > 7) {
+              if (OpenCLDebugInfo100Instructions(component_count_type->word(
+                      4)) != OpenCLDebugInfo100DebugTypeBasic ||
+                  OpenCLDebugInfo100DebugBaseTypeAttributeEncoding(
+                      component_count_type->word(7)) !=
+                      OpenCLDebugInfo100Unsigned) {
+                invalid = true;
+              } else {
+                // DebugTypeBasic for DebugLocalVariable/DebugGlobalVariable
+                // must have Unsigned encoding and 32 or 64 as its size in bits.
+                Instruction* size_in_bits =
+                    _.FindDef(component_count_type->word(6));
+                if (!_.IsIntScalarType(size_in_bits->type_id()) ||
+                    (size_in_bits->word(3) != 32 &&
+                     size_in_bits->word(3) != 64)) {
+                  invalid = true;
+                }
+              }
+            } else {
+              invalid = true;
+            }
+          } else {
+            invalid = true;
+          }
+          if (invalid) {
             return _.diag(SPV_ERROR_INVALID_DATA, inst)
-                   << ext_inst_name() << ": Component Count must be positive "
+                   << ext_inst_name() << ": Component Count must be "
-                   << "integer";
+                   << "OpConstant with a 32- or 64-bits integer scalar type or "
+                   << "DebugGlobalVariable or DebugLocalVariable with a 32- or "
+                   << "64-bits unsigned integer scalar type";
           }
         }
         break;
@@ -2830,11 +2874,6 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
             ValidateOperandLexicalScope(_, "Parent", inst, 10, ext_inst_name);
         if (validate_parent != SPV_SUCCESS) return validate_parent;
         CHECK_OPERAND("Linkage Name", SpvOpString, 11);
-        // TODO: The current OpenCL.100.DebugInfo spec says "Function
-        // is an OpFunction which is described by this instruction.".
-        // However, the function definition can be opted-out e.g.,
-        // inlining. We assume that Function operand can be a
-        // DebugInfoNone, but we must discuss it and update the spec.
         if (!DoesDebugInfoOperandMatchExpectation(
                 _,
                 [](OpenCLDebugInfo100Instructions dbg_inst) {
@@ -2870,9 +2909,6 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
         break;
       }
       case OpenCLDebugInfo100DebugScope: {
-        // TODO(https://gitlab.khronos.org/spirv/SPIR-V/issues/533): We are
-        // still in spec discussion about what must be "Scope" operand of
-        // DebugScope. Update this code if the conclusion is different.
         auto validate_scope =
             ValidateOperandLexicalScope(_, "Scope", inst, 5, ext_inst_name);
         if (validate_scope != SPV_SUCCESS) return validate_scope;
@@ -2896,11 +2932,6 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
       case OpenCLDebugInfo100DebugDeclare: {
         CHECK_DEBUG_OPERAND("Local Variable",
                             OpenCLDebugInfo100DebugLocalVariable, 5);
-
-        // TODO: We must discuss DebugDeclare.Variable of
-        // OpenCL.100.DebugInfo. Currently, it says "Variable must be an id of
-        // OpVariable instruction which defines the local variable.", but we
-        // want to allow OpFunctionParameter as well.
         auto* operand = _.FindDef(inst->word(6));
         if (operand->opcode() != SpvOpVariable &&
             operand->opcode() != SpvOpFunctionParameter) {

test/val/val_ext_inst_test.cpp

@@ -1344,6 +1344,40 @@ TEST_F(ValidateOpenCL100DebugInfo, DebugTypeArray) {
   ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());
 }
 
+TEST_F(ValidateOpenCL100DebugInfo, DebugTypeArrayWithVariableSize) {
+  const std::string src = R"(
+%src = OpString "simple.hlsl"
+%code = OpString "main() {}"
+%float_name = OpString "float"
+%int_name = OpString "int"
+%main_name = OpString "main"
+%foo_name = OpString "foo"
+)";
+
+  const std::string size_const = R"(
+%int_32 = OpConstant %u32 32
+)";
+
+  const std::string dbg_inst_header = R"(
+%dbg_src = OpExtInst %void %DbgExt DebugSource %src %code
+%comp_unit = OpExtInst %void %DbgExt DebugCompilationUnit 2 4 %dbg_src HLSL
+%float_info = OpExtInst %void %DbgExt DebugTypeBasic %float_name %int_32 Float
+%uint_info = OpExtInst %void %DbgExt DebugTypeBasic %int_name %int_32 Unsigned
+%main_type_info = OpExtInst %void %DbgExt DebugTypeFunction FlagIsPublic %void
+%main_info = OpExtInst %void %DbgExt DebugFunction %main_name %main_type_info %dbg_src 1 1 %comp_unit %main_name FlagIsPublic 1 %main
+%foo_info = OpExtInst %void %DbgExt DebugLocalVariable %foo_name %uint_info %dbg_src 1 1 %main_info FlagIsLocal
+%float_arr_info = OpExtInst %void %DbgExt DebugTypeArray %float_info %foo_info
+)";
+
+  const std::string extension = R"(
+%DbgExt = OpExtInstImport "OpenCL.DebugInfo.100"
+)";
+
+  CompileSuccessfully(GenerateShaderCodeForDebugInfo(
+      src, size_const, dbg_inst_header, "", extension, "Vertex"));
+  ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());
+}
+
 TEST_F(ValidateOpenCL100DebugInfo, DebugTypeArrayFailBaseType) {
   const std::string src = R"(
 %src = OpString "simple.hlsl"
@@ -1400,8 +1434,10 @@ TEST_F(ValidateOpenCL100DebugInfo, DebugTypeArrayFailComponentCount) {
       src, size_const, dbg_inst_header, "", extension, "Vertex"));
   ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
   EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("expected operand Component Count must be a result id "
+              HasSubstr("Component Count must be OpConstant with a 32- or "
-                        "of OpConstant"));
+                        "64-bits integer scalar type or DebugGlobalVariable or "
+                        "DebugLocalVariable with a 32- or 64-bits unsigned "
+                        "integer scalar type"));
 }
 
 TEST_F(ValidateOpenCL100DebugInfo, DebugTypeArrayFailComponentCountFloat) {
@@ -1430,7 +1466,10 @@ TEST_F(ValidateOpenCL100DebugInfo, DebugTypeArrayFailComponentCountFloat) {
       src, size_const, dbg_inst_header, "", extension, "Vertex"));
   ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
   EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("Component Count must be positive integer"));
+              HasSubstr("Component Count must be OpConstant with a 32- or "
+                        "64-bits integer scalar type or DebugGlobalVariable or "
+                        "DebugLocalVariable with a 32- or 64-bits unsigned "
+                        "integer scalar type"));
 }
 
 TEST_F(ValidateOpenCL100DebugInfo, DebugTypeArrayFailComponentCountZero) {
@@ -1459,7 +1498,47 @@ TEST_F(ValidateOpenCL100DebugInfo, DebugTypeArrayFailComponentCountZero) {
       src, size_const, dbg_inst_header, "", extension, "Vertex"));
   ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
   EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("Component Count must be positive integer"));
+              HasSubstr("Component Count must be OpConstant with a 32- or "
+                        "64-bits integer scalar type or DebugGlobalVariable or "
+                        "DebugLocalVariable with a 32- or 64-bits unsigned "
+                        "integer scalar type"));
+}
+
+TEST_F(ValidateOpenCL100DebugInfo, DebugTypeArrayFailVariableSizeTypeFloat) {
+  const std::string src = R"(
+%src = OpString "simple.hlsl"
+%code = OpString "main() {}"
+%float_name = OpString "float"
+%main_name = OpString "main"
+%foo_name = OpString "foo"
+)";
+
+  const std::string size_const = R"(
+%int_32 = OpConstant %u32 32
+)";
+
+  const std::string dbg_inst_header = R"(
+%dbg_src = OpExtInst %void %DbgExt DebugSource %src %code
+%comp_unit = OpExtInst %void %DbgExt DebugCompilationUnit 2 4 %dbg_src HLSL
+%float_info = OpExtInst %void %DbgExt DebugTypeBasic %float_name %int_32 Float
+%main_type_info = OpExtInst %void %DbgExt DebugTypeFunction FlagIsPublic %void
+%main_info = OpExtInst %void %DbgExt DebugFunction %main_name %main_type_info %dbg_src 1 1 %comp_unit %main_name FlagIsPublic 1 %main
+%foo_info = OpExtInst %void %DbgExt DebugLocalVariable %foo_name %float_info %dbg_src 1 1 %main_info FlagIsLocal
+%float_arr_info = OpExtInst %void %DbgExt DebugTypeArray %float_info %foo_info
+)";
+
+  const std::string extension = R"(
+%DbgExt = OpExtInstImport "OpenCL.DebugInfo.100"
+)";
+
+  CompileSuccessfully(GenerateShaderCodeForDebugInfo(
+      src, size_const, dbg_inst_header, "", extension, "Vertex"));
+  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Component Count must be OpConstant with a 32- or "
+                        "64-bits integer scalar type or DebugGlobalVariable or "
+                        "DebugLocalVariable with a 32- or 64-bits unsigned "
+                        "integer scalar type"));
 }
 
 TEST_F(ValidateOpenCL100DebugInfo, DebugTypeVector) {