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.
diff --git a/source/val/validate_extensions.cpp b/source/val/validate_extensions.cpp
index e22e121..0094c0a 100644
--- a/source/val/validate_extensions.cpp
+++ b/source/val/validate_extensions.cpp
@@ -636,6 +636,14 @@
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 @@
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()) ||
- !component_count->word(3)) {
+ if (IsConstIntScalarTypeWith32Or64Bits(_, component_count)) {
+ // 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 "
- << "integer";
+ << ext_inst_name() << ": 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";
}
}
break;
@@ -2830,11 +2874,6 @@
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 @@
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 @@
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) {
diff --git a/test/val/val_ext_inst_test.cpp b/test/val/val_ext_inst_test.cpp
index 1ad69cf..73e8473 100644
--- a/test/val/val_ext_inst_test.cpp
+++ b/test/val/val_ext_inst_test.cpp
@@ -1344,6 +1344,40 @@
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 @@
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 "
- "of OpConstant"));
+ 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, DebugTypeArrayFailComponentCountFloat) {
@@ -1430,7 +1466,10 @@
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 @@
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) {
