Skip to content

Commit

Permalink
spirv-val: Add missing NonSemantic.Shader.DebugInfo.100 (#5846)
Browse files Browse the repository at this point in the history 
* spirv-val: Cleanup DebugExtension tests

* spirv-val: Cleanup DebugExtension tests extensions

* spirv-val: Add missing NonSemantic.Shader.DebugInfo.100

* spirv-opt: Fix broken DebugLine in tests
  • Loading branch information
@spencer-lunarg
spencer-lunarg authored Nov 25, 2024
1 parent ea1d8cd commit f3c4a50
Show file tree
Hide file tree
Showing 6 changed files with 797 additions and 1,174 deletions.
2 changes: 1 addition & 1 deletion source/val/function.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ class Function {
Construct& FindConstructForEntryBlock(const BasicBlock* entry_block,
ConstructType t);

/// The result id of the OpLabel that defined this block
/// The result id of OpFunction
uint32_t id_;

/// The type of the function
Expand Down
123 changes: 96 additions & 27 deletions source/val/validate_extensions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3090,7 +3090,6 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
// validation.
case NonSemanticShaderDebugInfo100DebugInfoNone:
case NonSemanticShaderDebugInfo100DebugCompilationUnit:
case NonSemanticShaderDebugInfo100DebugTypeBasic:
case NonSemanticShaderDebugInfo100DebugTypePointer:
case NonSemanticShaderDebugInfo100DebugTypeQualifier:
case NonSemanticShaderDebugInfo100DebugTypeArray:
Expand All @@ -3116,7 +3115,6 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
case NonSemanticShaderDebugInfo100DebugInlinedAt:
case NonSemanticShaderDebugInfo100DebugLocalVariable:
case NonSemanticShaderDebugInfo100DebugInlinedVariable:
case NonSemanticShaderDebugInfo100DebugDeclare:
case NonSemanticShaderDebugInfo100DebugValue:
case NonSemanticShaderDebugInfo100DebugOperation:
case NonSemanticShaderDebugInfo100DebugExpression:
Expand All @@ -3125,6 +3123,24 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
case NonSemanticShaderDebugInfo100DebugImportedEntity:
case NonSemanticShaderDebugInfo100DebugSource:
break;

// These checks are for operands that are differnet in
// ShaderDebugInfo100
case NonSemanticShaderDebugInfo100DebugTypeBasic: {
CHECK_CONST_UINT_OPERAND("Flags", 8);
break;
}
case NonSemanticShaderDebugInfo100DebugDeclare: {
for (uint32_t word_index = 8; word_index < num_words; ++word_index) {
auto index_inst = _.FindDef(inst->word(word_index));
auto type_id = index_inst != nullptr ? index_inst->type_id() : 0;
if (type_id == 0 || !IsIntScalar(_, type_id, false, false))
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< ext_inst_name() << ": "
<< "expected index must be scalar integer";
}
break;
}
case NonSemanticShaderDebugInfo100DebugTypeMatrix: {
CHECK_DEBUG_OPERAND("Vector Type", CommonDebugInfoDebugTypeVector, 5);

Expand All @@ -3146,14 +3162,83 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
}
break;
}
// TODO: Add validation rules for remaining cases as well.
case NonSemanticShaderDebugInfo100DebugFunctionDefinition:
case NonSemanticShaderDebugInfo100DebugSourceContinued:
case NonSemanticShaderDebugInfo100DebugLine:
case NonSemanticShaderDebugInfo100DebugFunctionDefinition: {
CHECK_DEBUG_OPERAND("Function", CommonDebugInfoDebugFunction, 5);
CHECK_OPERAND("Definition", spv::Op::OpFunction, 6);
const auto* current_function = inst->function();
if (current_function->first_block()->id() != inst->block()->id()) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< ext_inst_name()
<< ": must be in the entry basic block of the function";
}

const uint32_t definition_id = inst->word(6);
if (definition_id != current_function->id()) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< ext_inst_name()
<< ": operand Definition must point to the OpFunction it is "
"inside";
}
break;
}
case NonSemanticShaderDebugInfo100DebugLine: {
CHECK_DEBUG_OPERAND("Source", CommonDebugInfoDebugSource, 5);
CHECK_CONST_UINT_OPERAND("Line Start", 6);
CHECK_CONST_UINT_OPERAND("Line End", 7);
CHECK_CONST_UINT_OPERAND("Column Start", 8);
CHECK_CONST_UINT_OPERAND("Column End", 9);

// above already validates if 32-bit and non-spec constant
// but want to use EvalInt32IfConst to be consistent with other Eval
// locations
bool is_int32 = false, is_const_int32 = false;
uint32_t line_start = 0;
uint32_t line_end = 0;
uint32_t column_start = 0;
uint32_t column_end = 0;
std::tie(is_int32, is_const_int32, line_start) =
_.EvalInt32IfConst(inst->word(6));
std::tie(is_int32, is_const_int32, line_end) =
_.EvalInt32IfConst(inst->word(7));
std::tie(is_int32, is_const_int32, column_start) =
_.EvalInt32IfConst(inst->word(8));
std::tie(is_int32, is_const_int32, column_end) =
_.EvalInt32IfConst(inst->word(9));
if (line_end < line_start) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< ext_inst_name() << ": operand Line End (" << line_end
<< ") is less than Line Start (" << line_start << ")";
} else if (column_end < column_start) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< ext_inst_name() << ": operand Column End (" << column_end
<< ") is less than Column Start (" << column_start << ")";
}
break;
}
case NonSemanticShaderDebugInfo100DebugSourceContinued: {
CHECK_OPERAND("Text", spv::Op::OpString, 5);
break;
}
case NonSemanticShaderDebugInfo100DebugBuildIdentifier: {
CHECK_OPERAND("Identifier", spv::Op::OpString, 5);
CHECK_CONST_UINT_OPERAND("Flags", 6);
break;
}
case NonSemanticShaderDebugInfo100DebugStoragePath: {
CHECK_OPERAND("Path", spv::Op::OpString, 5);
break;
}
case NonSemanticShaderDebugInfo100DebugEntryPoint: {
CHECK_DEBUG_OPERAND("Entry Point", CommonDebugInfoDebugFunction, 5);
CHECK_DEBUG_OPERAND("Compilation Unit",
CommonDebugInfoDebugCompilationUnit, 6);
CHECK_OPERAND("Compiler Signature", spv::Op::OpString, 7);
CHECK_OPERAND("Command-line Arguments", spv::Op::OpString, 8);
break;
}

// Has no additional checks
case NonSemanticShaderDebugInfo100DebugNoLine:
case NonSemanticShaderDebugInfo100DebugBuildIdentifier:
case NonSemanticShaderDebugInfo100DebugStoragePath:
case NonSemanticShaderDebugInfo100DebugEntryPoint:
break;
case NonSemanticShaderDebugInfo100InstructionsMax:
assert(0);
Expand Down Expand Up @@ -3455,9 +3540,7 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
}
case CommonDebugInfoDebugFunction: {
CHECK_OPERAND("Name", spv::Op::OpString, 5);
auto validate_type = ValidateOperandDebugType(_, "Type", inst, 6,
ext_inst_name, false);
if (validate_type != SPV_SUCCESS) return validate_type;
CHECK_DEBUG_OPERAND("Type", CommonDebugInfoDebugTypeFunction, 6);
CHECK_DEBUG_OPERAND("Source", CommonDebugInfoDebugSource, 7);
CHECK_CONST_UINT_OPERAND("Line", 8);
CHECK_CONST_UINT_OPERAND("Column", 9);
Expand Down Expand Up @@ -3492,9 +3575,7 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
}
case CommonDebugInfoDebugFunctionDeclaration: {
CHECK_OPERAND("Name", spv::Op::OpString, 5);
auto validate_type = ValidateOperandDebugType(_, "Type", inst, 6,
ext_inst_name, false);
if (validate_type != SPV_SUCCESS) return validate_type;
CHECK_DEBUG_OPERAND("Type", CommonDebugInfoDebugTypeFunction, 6);
CHECK_DEBUG_OPERAND("Source", CommonDebugInfoDebugSource, 7);
CHECK_CONST_UINT_OPERAND("Line", 8);
CHECK_CONST_UINT_OPERAND("Column", 9);
Expand Down Expand Up @@ -3556,18 +3637,6 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
}

CHECK_DEBUG_OPERAND("Expression", CommonDebugInfoDebugExpression, 7);

if (vulkanDebugInfo) {
for (uint32_t word_index = 8; word_index < num_words;
++word_index) {
auto index_inst = _.FindDef(inst->word(word_index));
auto type_id = index_inst != nullptr ? index_inst->type_id() : 0;
if (type_id == 0 || !IsIntScalar(_, type_id, false, false))
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< ext_inst_name() << ": "
<< "expected index must be scalar integer";
}
}
break;
}
case CommonDebugInfoDebugExpression: {
Expand Down
4 changes: 2 additions & 2 deletions source/val/validate_layout.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ spv_result_t ModuleScopedInstructions(ValidationState_t& _,

if (local_debug_info) {
if (_.in_function_body() == false) {
// DebugScope, DebugNoScope, DebugDeclare, DebugValue must
// appear in a function body.
// TODO - Print the actual name of the instruction as this list is
// not complete (see ext_inst_name in ValidateExtInst() for example)
return _.diag(SPV_ERROR_INVALID_LAYOUT, inst)
<< "DebugScope, DebugNoScope, DebugDeclare, DebugValue "
<< "of debug info extension must appear in a function "
Expand Down
6 changes: 3 additions & 3 deletions test/opt/inline_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3749,13 +3749,13 @@ float4 main(float4 color : COLOR) : SV_TARGET {
%color = OpFunctionParameter %_ptr_Function_v4float
%bb_entry = OpLabel
%140 = OpExtInst %void %1 DebugFunctionDefinition %22 %src_main
%141 = OpExtInst %void %1 DebugLine %5 %uint_1 %uint_1 %uint_1 %uint_1
%141 = OpExtInst %void %1 DebugLine %15 %uint_1 %uint_1 %uint_1 %uint_1
%34 = OpExtInst %void %1 DebugScope %22
%36 = OpExtInst %void %1 DebugDeclare %25 %color %13
%38 = OpExtInst %void %1 DebugScope %26
%142 = OpExtInst %void %1 DebugLine %5 %uint_2 %uint_2 %uint_10 %uint_10
%142 = OpExtInst %void %1 DebugLine %15 %uint_2 %uint_2 %uint_10 %uint_10
%39 = OpLoad %v4float %color
%143 = OpExtInst %void %1 DebugLine %5 %uint_2 %uint_2 %uint_3 %uint_3
%143 = OpExtInst %void %1 DebugLine %15 %uint_2 %uint_2 %uint_3 %uint_3
OpReturnValue %39
OpFunctionEnd
)";
Expand Down
18 changes: 9 additions & 9 deletions test/opt/loop_optimizations/unroll_simple.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -510,36 +510,36 @@ OpBranch %24
%24 = OpLabel
%35 = OpPhi %8 %10 %23 %34 %26
%s1 = OpExtInst %6 %ext DebugScope %dbg_main
%d10 = OpExtInst %6 %ext DebugLine %file_name %uint_1 %uint_1 %uint_0 %uint_0
%d10 = OpExtInst %6 %ext DebugLine %src %uint_1 %uint_1 %uint_0 %uint_0
%value0 = OpExtInst %6 %ext DebugValue %dbg_f %35 %null_expr
OpLoopMerge %25 %26 Unroll
OpBranch %27
%27 = OpLabel
%s2 = OpExtInst %6 %ext DebugScope %dbg_main
%d1 = OpExtInst %6 %ext DebugLine %file_name %uint_1 %uint_1 %uint_1 %uint_1
%d1 = OpExtInst %6 %ext DebugLine %src %uint_1 %uint_1 %uint_1 %uint_1
%29 = OpSLessThan %12 %35 %11
%d2 = OpExtInst %6 %ext DebugLine %file_name %uint_2 %uint_2 %uint_0 %uint_0
%d2 = OpExtInst %6 %ext DebugLine %src %uint_2 %uint_2 %uint_0 %uint_0
OpBranchConditional %29 %30 %25
%30 = OpLabel
%s3 = OpExtInst %6 %ext DebugScope %bb
%decl0 = OpExtInst %6 %ext DebugDeclare %dbg_f %5 %null_expr
%decl1 = OpExtInst %6 %ext DebugValue %dbg_i %5 %deref_expr
%d3 = OpExtInst %6 %ext DebugLine %file_name %uint_3 %uint_3 %uint_0 %uint_0
%d3 = OpExtInst %6 %ext DebugLine %src %uint_3 %uint_3 %uint_0 %uint_0
%32 = OpAccessChain %19 %5 %35
%d4 = OpExtInst %6 %ext DebugLine %file_name %uint_4 %uint_4 %uint_0 %uint_0
%d4 = OpExtInst %6 %ext DebugLine %src %uint_4 %uint_4 %uint_0 %uint_0
OpStore %32 %18
%d5 = OpExtInst %6 %ext DebugLine %file_name %uint_5 %uint_5 %uint_0 %uint_0
%d5 = OpExtInst %6 %ext DebugLine %src %uint_5 %uint_5 %uint_0 %uint_0
OpBranch %26
%26 = OpLabel
%s4 = OpExtInst %6 %ext DebugScope %dbg_main
%d6 = OpExtInst %6 %ext DebugLine %file_name %uint_6 %uint_6 %uint_0 %uint_0
%d6 = OpExtInst %6 %ext DebugLine %src %uint_6 %uint_6 %uint_0 %uint_0
%34 = OpIAdd %8 %35 %20
%value1 = OpExtInst %6 %ext DebugValue %dbg_f %34 %null_expr
%d7 = OpExtInst %6 %ext DebugLine %file_name %uint_7 %uint_7 %uint_0 %uint_0
%d7 = OpExtInst %6 %ext DebugLine %src %uint_7 %uint_7 %uint_0 %uint_0
OpBranch %24
%25 = OpLabel
%s5 = OpExtInst %6 %ext DebugScope %dbg_main
%d8 = OpExtInst %6 %ext DebugLine %file_name %uint_8 %uint_8 %uint_0 %uint_0
%d8 = OpExtInst %6 %ext DebugLine %src %uint_8 %uint_8 %uint_0 %uint_0
OpReturn
OpFunctionEnd)";

Expand Down
Loading

0 comments on commit f3c4a50

Please sign in to comment.