Skip to content

Commit c58415d

Browse files
spencer-lunargspencer-lunarg
authored
spirv-val: Add DebugGlobalVariable to spirv-val messages (KhronosGroup#6681)
next small step for KhronosGroup#6617 If the error is an `OpVariable` will look like ``` error: 46: Storage class must match result type storage class %3 = OpVariable %_ptr_Uniform__struct_13 StorageBuffer --> a.comp:2:0 | 2 | layout(set = 0, binding = 1, std430) buffer SSBO { | ``` Goal was to make the `PrintShaderDebugInfoSource` they could both share. View as a very small step forward
1 parent 5cdb96f commit c58415d

3 files changed

Lines changed: 208 additions & 12 deletions

File tree

‎source/val/validation_state.cpp‎

Lines changed: 70 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2180,15 +2180,27 @@ std::vector<uint32_t>& ValidationState_t::GetDebugSourceLineLength(
21802180
return it->second;
21812181
}
21822182

2183+
// Main entrypoint to using ShaderDebugInfo to get better error messages
21832184
std::string ValidationState_t::InspectShaderDebugInfo(const Instruction& inst) {
2184-
const uint32_t set_id = ShaderDebugInfoSet();
2185-
if (set_id == 0) {
2185+
if (ShaderDebugInfoSet() == 0) {
21862186
return ""; // no ShaderDebugInfo found
2187-
} else if (inst.function() == nullptr) {
2188-
// currently only checking for code in function blocks
2189-
return "";
21902187
}
21912188

2189+
std::ostringstream ss;
2190+
if (inst.function() != nullptr) {
2191+
InspectDebugLine(ss, inst);
2192+
} else if (inst.opcode() == spv::Op::OpVariable) {
2193+
// Know are global because not in any function
2194+
// TODO - test with OpUntypedVariable as well
2195+
InspectDebugGlobalVariable(ss, inst);
2196+
}
2197+
2198+
return ss.str();
2199+
}
2200+
2201+
void ValidationState_t::InspectDebugLine(std::ostringstream& ss,
2202+
const Instruction& inst) {
2203+
const uint32_t set_id = ShaderDebugInfoSet();
21922204
// Find the DebugLine that is preceding
21932205
const Instruction* debug_line_inst = nullptr;
21942206
size_t idx = &inst - &ordered_instructions()[0];
@@ -2207,13 +2219,13 @@ std::string ValidationState_t::InspectShaderDebugInfo(const Instruction& inst) {
22072219
}
22082220
}
22092221

2210-
if (!debug_line_inst) return "";
2222+
if (!debug_line_inst) return;
22112223

22122224
const Instruction* debug_source =
22132225
FindDef(debug_line_inst->GetOperandAs<uint32_t>(4));
22142226
if (!debug_source || debug_source->GetOperandAs<uint32_t>(3) !=
22152227
NonSemanticShaderDebugInfoDebugSource) {
2216-
return "";
2228+
return;
22172229
}
22182230

22192231
bool is_int32 = false, is_const_int32 = false;
@@ -2230,8 +2242,53 @@ std::string ValidationState_t::InspectShaderDebugInfo(const Instruction& inst) {
22302242
std::tie(is_int32, is_const_int32, column_end) =
22312243
EvalInt32IfConst(debug_line_inst->word(9));
22322244

2233-
std::ostringstream ss;
2245+
PrintShaderDebugInfoSource(ss, *debug_source, line_start, line_end,
2246+
column_start);
2247+
}
2248+
2249+
void ValidationState_t::InspectDebugGlobalVariable(
2250+
std::ostringstream& ss, const Instruction& variable_inst) {
2251+
const uint32_t set_id = ShaderDebugInfoSet();
2252+
2253+
const Instruction* debug_gloabl_var_inst = nullptr;
2254+
for (const auto& inst : ordered_instructions()) {
2255+
if (inst.opcode() == spv::Op::OpFunction) {
2256+
return; // validated to not be in a function block
2257+
}
2258+
2259+
if (inst.opcode() == spv::Op::OpExtInst &&
2260+
inst.GetOperandAs<uint32_t>(2) == set_id &&
2261+
inst.GetOperandAs<uint32_t>(3) ==
2262+
NonSemanticShaderDebugInfoDebugGlobalVariable &&
2263+
inst.GetOperandAs<uint32_t>(11) == variable_inst.id()) {
2264+
debug_gloabl_var_inst = &inst;
2265+
break;
2266+
}
2267+
}
2268+
if (!debug_gloabl_var_inst) return;
2269+
2270+
const Instruction* debug_source =
2271+
FindDef(debug_gloabl_var_inst->GetOperandAs<uint32_t>(6));
2272+
if (!debug_source || debug_source->GetOperandAs<uint32_t>(3) !=
2273+
NonSemanticShaderDebugInfoDebugSource) {
2274+
return;
2275+
}
2276+
2277+
bool is_int32 = false, is_const_int32 = false;
2278+
uint32_t line_start = 0;
2279+
uint32_t column_start = 0;
2280+
std::tie(is_int32, is_const_int32, line_start) =
2281+
EvalInt32IfConst(debug_gloabl_var_inst->word(8));
2282+
std::tie(is_int32, is_const_int32, column_start) =
2283+
EvalInt32IfConst(debug_gloabl_var_inst->word(9));
22342284

2285+
PrintShaderDebugInfoSource(ss, *debug_source, line_start, line_start,
2286+
column_start);
2287+
}
2288+
2289+
void ValidationState_t::PrintShaderDebugInfoSource(
2290+
std::ostringstream& ss, const Instruction& debug_source,
2291+
uint32_t line_start, uint32_t line_end, uint32_t column_start) {
22352292
// The left hand side line number, need to make sure if going from line number
22362293
// 99 to 100 that all lines have the same padding
22372294
const size_t vertical_line_padding = std::to_string(line_end).length();
@@ -2271,20 +2328,22 @@ std::string ValidationState_t::InspectShaderDebugInfo(const Instruction& inst) {
22712328
};
22722329

22732330
const Instruction* file_string =
2274-
FindDef(debug_source->GetOperandAs<uint32_t>(4));
2331+
FindDef(debug_source.GetOperandAs<uint32_t>(4));
22752332
ss << "\n --> " << file_string->GetOperandAs<std::string>(1) << ":"
22762333
<< line_start << ":" << column_start << '\n';
22772334

22782335
add_vertical_line(0);
22792336
ss << '\n';
22802337

22812338
const Instruction* source_string =
2282-
FindDef(debug_source->GetOperandAs<uint32_t>(5));
2339+
FindDef(debug_source.GetOperandAs<uint32_t>(5));
22832340
add_vertical_line(line_start);
22842341
stream_text(source_string);
22852342

2343+
const uint32_t set_id = ShaderDebugInfoSet();
2344+
22862345
// Look for any DebugSourceContinued
2287-
size_t src_idx = debug_source - &ordered_instructions()[0] + 1;
2346+
size_t src_idx = &debug_source - &ordered_instructions()[0] + 1;
22882347
for (; src_idx < ordered_instructions().size(); ++src_idx) {
22892348
const Instruction& continued_insn = ordered_instructions()[src_idx];
22902349
if (continued_insn.opcode() != spv::Op::OpExtInst ||
@@ -2304,7 +2363,6 @@ std::string ValidationState_t::InspectShaderDebugInfo(const Instruction& inst) {
23042363

23052364
add_vertical_line(0);
23062365
ss << "\n";
2307-
return ss.str();
23082366
}
23092367

23102368
bool ValidationState_t::IsValidStorageClass(

‎source/val/validation_state.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,6 +1007,13 @@ class ValidationState_t {
10071007
void RegisterShaderDebugInfo(uint32_t id) { shader_debug_info_set_id = id; }
10081008
uint32_t ShaderDebugInfoSet() const { return shader_debug_info_set_id; }
10091009
std::string InspectShaderDebugInfo(const Instruction& inst);
1010+
void InspectDebugLine(std::ostringstream& ss, const Instruction& inst);
1011+
void InspectDebugGlobalVariable(std::ostringstream& ss,
1012+
const Instruction& inst);
1013+
void PrintShaderDebugInfoSource(std::ostringstream& ss,
1014+
const Instruction& debug_source,
1015+
uint32_t line_start, uint32_t line_end,
1016+
uint32_t column_start);
10101017

10111018
private:
10121019
ValidationState_t(const ValidationState_t&);

‎test/val/val_shader_debug_info_test.cpp‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,137 @@ TEST_F(ValidateShaderDebugInfo, UnusedOpFunction) {
232232
"OpFunction Function Type <id> '3[%uint]' is not a function type."));
233233
}
234234

235+
TEST_F(ValidateShaderDebugInfo, VariableBasic) {
236+
const std::string str = R"(
237+
OpCapability Shader
238+
OpExtension "SPV_KHR_non_semantic_info"
239+
%1 = OpExtInstImport "NonSemantic.Shader.DebugInfo.100"
240+
OpMemoryModel Logical GLSL450
241+
OpEntryPoint GLCompute %main "main" %x
242+
OpExecutionMode %main LocalSize 1 1 1
243+
%2 = OpString "a.comp"
244+
%8 = OpString "uint"
245+
%16 = OpString "main"
246+
%19 = OpString "#version 450
247+
layout(set = 0, binding = 1, std430) buffer SSBO {
248+
vec4 data;
249+
} x;
250+
251+
void main() {
252+
x.data = vec4(0.0);
253+
}"
254+
%25 = OpString "float"
255+
%31 = OpString "data"
256+
%34 = OpString "SSBO"
257+
%40 = OpString "x"
258+
%46 = OpString "int"
259+
OpDecorate %SSBO Block
260+
OpMemberDecorate %SSBO 0 Offset 0
261+
OpDecorate %x Binding 1
262+
OpDecorate %x DescriptorSet 0
263+
%void = OpTypeVoid
264+
%5 = OpTypeFunction %void
265+
%uint = OpTypeInt 32 0
266+
%uint_32 = OpConstant %uint 32
267+
%uint_6 = OpConstant %uint 6
268+
%uint_0 = OpConstant %uint 0
269+
%9 = OpExtInst %void %1 DebugTypeBasic %8 %uint_32 %uint_6 %uint_0
270+
%uint_3 = OpConstant %uint 3
271+
%6 = OpExtInst %void %1 DebugTypeFunction %uint_3 %void
272+
%18 = OpExtInst %void %1 DebugSource %2 %19
273+
%uint_1 = OpConstant %uint 1
274+
%uint_4 = OpConstant %uint 4
275+
%uint_2 = OpConstant %uint 2
276+
%20 = OpExtInst %void %1 DebugCompilationUnit %uint_1 %uint_4 %18 %uint_2
277+
%17 = OpExtInst %void %1 DebugFunction %16 %6 %18 %uint_6 %uint_0 %20 %16 %uint_3 %uint_6
278+
%float = OpTypeFloat 32
279+
%26 = OpExtInst %void %1 DebugTypeBasic %25 %uint_32 %uint_3 %uint_0
280+
%v4float = OpTypeVector %float 4
281+
%28 = OpExtInst %void %1 DebugTypeVector %26 %uint_4
282+
%SSBO = OpTypeStruct %v4float
283+
%uint_10 = OpConstant %uint 10
284+
%30 = OpExtInst %void %1 DebugTypeMember %31 %28 %18 %uint_3 %uint_10 %uint_0 %uint_0 %uint_3
285+
%33 = OpExtInst %void %1 DebugTypeComposite %34 %uint_1 %18 %uint_2 %uint_0 %20 %34 %uint_0 %uint_3 %30
286+
%_ptr_StorageBuffer_SSBO = OpTypePointer Uniform %SSBO
287+
%uint_12 = OpConstant %uint 12
288+
%37 = OpExtInst %void %1 DebugTypePointer %33 %uint_12 %uint_0
289+
%x = OpVariable %_ptr_StorageBuffer_SSBO StorageBuffer
290+
%uint_8 = OpConstant %uint 8
291+
%39 = OpExtInst %void %1 DebugGlobalVariable %40 %33 %18 %uint_2 %uint_0 %20 %40 %x %uint_8
292+
%float_0 = OpConstant %float 0
293+
%50 = OpConstantComposite %v4float %float_0 %float_0 %float_0 %float_0
294+
%_ptr_StorageBuffer_v4float = OpTypePointer StorageBuffer %v4float
295+
%main = OpFunction %void None %5
296+
%15 = OpLabel
297+
%53 = OpAccessChain %_ptr_StorageBuffer_v4float %x %uint_0
298+
OpStore %53 %50
299+
OpReturn
300+
OpFunctionEnd
301+
)";
302+
CompileSuccessfully(str.c_str(), SPV_ENV_VULKAN_1_3);
303+
EXPECT_NE(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_3));
304+
EXPECT_THAT(getDiagnosticString(), HasSubstr(R"( --> a.comp:2:0
305+
|
306+
2 | layout(set = 0, binding = 1, std430) buffer SSBO {
307+
|)"));
308+
}
309+
310+
// Test if no DebugGlobalVariable is found, things still work
311+
TEST_F(ValidateShaderDebugInfo, NoDebugGlobalVariable) {
312+
const std::string str = R"(
313+
OpCapability Shader
314+
OpExtension "SPV_KHR_non_semantic_info"
315+
%1 = OpExtInstImport "NonSemantic.Shader.DebugInfo.100"
316+
OpMemoryModel Logical GLSL450
317+
OpEntryPoint GLCompute %main "main" %x
318+
OpExecutionMode %main LocalSize 1 1 1
319+
%2 = OpString "a.comp"
320+
%8 = OpString "uint"
321+
%19 = OpString "BAD"
322+
%25 = OpString "float"
323+
OpDecorate %SSBO Block
324+
OpMemberDecorate %SSBO 0 Offset 0
325+
OpDecorate %x Binding 1
326+
OpDecorate %x DescriptorSet 0
327+
%void = OpTypeVoid
328+
%5 = OpTypeFunction %void
329+
%uint = OpTypeInt 32 0
330+
%uint_32 = OpConstant %uint 32
331+
%uint_6 = OpConstant %uint 6
332+
%uint_0 = OpConstant %uint 0
333+
%9 = OpExtInst %void %1 DebugTypeBasic %8 %uint_32 %uint_6 %uint_0
334+
%uint_3 = OpConstant %uint 3
335+
%6 = OpExtInst %void %1 DebugTypeFunction %uint_3 %void
336+
%18 = OpExtInst %void %1 DebugSource %2 %19
337+
%uint_1 = OpConstant %uint 1
338+
%uint_4 = OpConstant %uint 4
339+
%uint_2 = OpConstant %uint 2
340+
%20 = OpExtInst %void %1 DebugCompilationUnit %uint_1 %uint_4 %18 %uint_2
341+
%float = OpTypeFloat 32
342+
%26 = OpExtInst %void %1 DebugTypeBasic %25 %uint_32 %uint_3 %uint_0
343+
%v4float = OpTypeVector %float 4
344+
%28 = OpExtInst %void %1 DebugTypeVector %26 %uint_4
345+
%SSBO = OpTypeStruct %v4float
346+
%uint_10 = OpConstant %uint 10
347+
%_ptr_StorageBuffer_SSBO = OpTypePointer Uniform %SSBO
348+
%uint_12 = OpConstant %uint 12
349+
%x = OpVariable %_ptr_StorageBuffer_SSBO StorageBuffer
350+
%uint_8 = OpConstant %uint 8
351+
%float_0 = OpConstant %float 0
352+
%50 = OpConstantComposite %v4float %float_0 %float_0 %float_0 %float_0
353+
%_ptr_StorageBuffer_v4float = OpTypePointer StorageBuffer %v4float
354+
%main = OpFunction %void None %5
355+
%15 = OpLabel
356+
%53 = OpAccessChain %_ptr_StorageBuffer_v4float %x %uint_0
357+
OpStore %53 %50
358+
OpReturn
359+
OpFunctionEnd
360+
)";
361+
CompileSuccessfully(str.c_str(), SPV_ENV_VULKAN_1_3);
362+
EXPECT_NE(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_3));
363+
EXPECT_THAT(getDiagnosticString(), Not(HasSubstr("a.comp")));
364+
}
365+
235366
} // namespace
236367
} // namespace val
237368
} // namespace spvtools

0 commit comments

Comments
 (0)