Skip to content

Commit fd3948e

Browse files
jaebaekjaebaek
authored
Add DebugValue for function param regardless of scope (KhronosGroup#3923)
`DebugInfoManager::AddDebugValueIfVarDeclIsVisible()` adds OpenCL.DebugInfo.100 DebugValue from DebugDeclare only when the DebugDeclare is visible to the give scope. It helps us correctly handle a reference variable e.g., { // scope #1. int foo = /* init */; { // scope #2. int& bar = foo; ... in the above code, we must not propagate DebugValue of `int& bar` for store instructions in the scope #1 because it is alive only in the scope #2. We have an exception: If the given DebugDeclare is used for a function parameter, `DebugInfoManager::AddDebugValueIfVarDeclIsVisible()` has to always add DebugValue instruction regardless of the scope. It is because the initializer (store instruction) for the function parameter can be out of the function parameter's scope (the function) in particular when the function was inlined. Without this change, the function parameter value information always disappears whenever we run the function inlining pass and `DebugInfoManager::AddDebugValueIfVarDeclIsVisible()`.
1 parent 663d050 commit fd3948e

File tree

3 files changed

+116
-12
lines changed

3 files changed

+116
-12
lines changed

‎source/opt/debug_info_manager.cpp‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ static const uint32_t kDebugLocalVariableOperandParentIndex = 9;
4040
static const uint32_t kExtInstInstructionInIdx = 1;
4141
static const uint32_t kDebugGlobalVariableOperandFlagsIndex = 12;
4242
static const uint32_t kDebugLocalVariableOperandFlagsIndex = 10;
43+
static const uint32_t kDebugLocalVariableOperandArgNumberIndex = 11;
4344

4445
namespace spvtools {
4546
namespace opt {
@@ -440,15 +441,27 @@ bool DebugInfoManager::IsAncestorOfScope(uint32_t scope, uint32_t ancestor) {
440441
return false;
441442
}
442443

443-
bool DebugInfoManager::IsDeclareVisibleToInstr(Instruction* dbg_declare,
444-
uint32_t instr_scope_id) {
445-
if (instr_scope_id == kNoDebugScope) return false;
446-
444+
Instruction* DebugInfoManager::GetDebugLocalVariableFromDeclare(
445+
Instruction* dbg_declare) {
446+
assert(dbg_declare);
447447
uint32_t dbg_local_var_id =
448448
dbg_declare->GetSingleWordOperand(kDebugDeclareOperandLocalVariableIndex);
449449
auto dbg_local_var_itr = id_to_dbg_inst_.find(dbg_local_var_id);
450450
assert(dbg_local_var_itr != id_to_dbg_inst_.end());
451-
uint32_t decl_scope_id = dbg_local_var_itr->second->GetSingleWordOperand(
451+
return dbg_local_var_itr->second;
452+
}
453+
454+
bool DebugInfoManager::IsFunctionParameter(Instruction* dbg_local_var) const {
455+
// If a DebugLocalVariable has ArgNumber operand, it is a function parameter.
456+
return dbg_local_var->NumOperands() >
457+
kDebugLocalVariableOperandArgNumberIndex;
458+
}
459+
460+
bool DebugInfoManager::IsLocalVariableVisibleToInstr(Instruction* dbg_local_var,
461+
uint32_t instr_scope_id) {
462+
if (instr_scope_id == kNoDebugScope) return false;
463+
464+
uint32_t decl_scope_id = dbg_local_var->GetSingleWordOperand(
452465
kDebugLocalVariableOperandParentIndex);
453466

454467
// If the scope of DebugDeclare is an ancestor scope of the instruction's
@@ -502,11 +515,20 @@ void DebugInfoManager::AddDebugValueIfVarDeclIsVisible(
502515

503516
uint32_t instr_scope_id = scope_and_line->GetDebugScope().GetLexicalScope();
504517
for (auto* dbg_decl_or_val : dbg_decl_itr->second) {
505-
if (!IsDeclareVisibleToInstr(dbg_decl_or_val, instr_scope_id)) continue;
518+
// If it declares a function parameter, the store instruction for the
519+
// function parameter can exist out of the function parameter's scope
520+
// because of the function inlining. We always add DebugValue for a
521+
// function parameter next to the DebugDeclare regardless of the scope.
522+
auto* dbg_local_var = GetDebugLocalVariableFromDeclare(dbg_decl_or_val);
523+
bool is_function_param = IsFunctionParameter(dbg_local_var);
524+
if (!is_function_param &&
525+
!IsLocalVariableVisibleToInstr(dbg_local_var, instr_scope_id))
526+
continue;
506527

507528
// Avoid inserting the new DebugValue between OpPhi or OpVariable
508529
// instructions.
509-
Instruction* insert_before = insert_pos->NextNode();
530+
Instruction* insert_before = is_function_param ? dbg_decl_or_val->NextNode()
531+
: insert_pos->NextNode();
510532
while (insert_before->opcode() == SpvOpPhi ||
511533
insert_before->opcode() == SpvOpVariable) {
512534
insert_before = insert_before->NextNode();
@@ -523,7 +545,8 @@ void DebugInfoManager::AddDebugValueIfVarDeclIsVisible(
523545
kDebugValueOperandLocalVariableIndex),
524546
value_id, 0, index_id, insert_before);
525547
assert(added_dbg_value != nullptr);
526-
added_dbg_value->UpdateDebugInfoFrom(scope_and_line);
548+
added_dbg_value->UpdateDebugInfoFrom(is_function_param ? dbg_decl_or_val
549+
: scope_and_line);
527550
AnalyzeDebugInst(added_dbg_value);
528551
}
529552
}

‎source/opt/debug_info_manager.h‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -215,10 +215,17 @@ class DebugInfoManager {
215215
// of |scope|.
216216
bool IsAncestorOfScope(uint32_t scope, uint32_t ancestor);
217217

218-
// Returns true if the declaration of a local variable |dbg_declare|
219-
// is visible in the scope of an instruction |instr_scope_id|.
220-
bool IsDeclareVisibleToInstr(Instruction* dbg_declare,
221-
uint32_t instr_scope_id);
218+
// Returns the DebugLocalVariable declared by |dbg_declare|.
219+
Instruction* GetDebugLocalVariableFromDeclare(Instruction* dbg_declare);
220+
221+
// Returns true if the DebugLocalVariable |dbg_local_var| is a function
222+
// parameter.
223+
bool IsFunctionParameter(Instruction* dbg_local_var) const;
224+
225+
// Returns true if the DebugLocalVariable |dbg_local_var| is visible
226+
// in the scope of an instruction |instr_scope_id|.
227+
bool IsLocalVariableVisibleToInstr(Instruction* dbg_local_var,
228+
uint32_t instr_scope_id);
222229

223230
// Returns the parent scope of the scope |child_scope|.
224231
uint32_t GetParentScope(uint32_t child_scope);

‎test/opt/local_single_store_elim_test.cpp‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,6 +1211,80 @@ TEST_F(LocalSingleStoreElimTest, DebugValueTest) {
12111211
SinglePassRunAndMatch<LocalSingleStoreElimPass>(text, false);
12121212
}
12131213

1214+
TEST_F(LocalSingleStoreElimTest, DebugDeclareForFunctionParameter) {
1215+
// We have to add DebugValue for DebugDeclare regardless of the scope
1216+
// if it declares a function parameter.
1217+
const std::string text = R"(
1218+
OpCapability Shader
1219+
%1 = OpExtInstImport "OpenCL.DebugInfo.100"
1220+
OpMemoryModel Logical GLSL450
1221+
OpEntryPoint GLCompute %main "main"
1222+
OpExecutionMode %main LocalSize 1 1 1
1223+
%3 = OpString "fn_call.hlsl"
1224+
%4 = OpString "int"
1225+
%5 = OpString ""
1226+
%6 = OpString "x"
1227+
%7 = OpString "A"
1228+
%8 = OpString "main"
1229+
OpName %type_StructuredBuffer_int "type.StructuredBuffer.int"
1230+
OpName %data "data"
1231+
OpName %main "main"
1232+
OpDecorate %data DescriptorSet 0
1233+
OpDecorate %data Binding 0
1234+
OpDecorate %_runtimearr_int ArrayStride 4
1235+
OpMemberDecorate %type_StructuredBuffer_int 0 Offset 0
1236+
OpMemberDecorate %type_StructuredBuffer_int 0 NonWritable
1237+
OpDecorate %type_StructuredBuffer_int BufferBlock
1238+
%int = OpTypeInt 32 1
1239+
%uint = OpTypeInt 32 0
1240+
%uint_0 = OpConstant %uint 0
1241+
%uint_32 = OpConstant %uint 32
1242+
%_runtimearr_int = OpTypeRuntimeArray %int
1243+
%type_StructuredBuffer_int = OpTypeStruct %_runtimearr_int
1244+
%_ptr_Uniform_type_StructuredBuffer_int = OpTypePointer Uniform %type_StructuredBuffer_int
1245+
%void = OpTypeVoid
1246+
%18 = OpTypeFunction %void
1247+
%_ptr_Function_int = OpTypePointer Function %int
1248+
%_ptr_Uniform_int = OpTypePointer Uniform %int
1249+
%data = OpVariable %_ptr_Uniform_type_StructuredBuffer_int Uniform
1250+
%21 = OpExtInst %void %1 DebugInfoNone
1251+
%22 = OpExtInst %void %1 DebugExpression
1252+
%23 = OpExtInst %void %1 DebugTypeBasic %4 %uint_32 Signed
1253+
%24 = OpExtInst %void %1 DebugTypeFunction FlagIsProtected|FlagIsPrivate %23 %23
1254+
%25 = OpExtInst %void %1 DebugSource %3
1255+
%26 = OpExtInst %void %1 DebugCompilationUnit 1 4 %25 HLSL
1256+
%27 = OpExtInst %void %1 DebugFunction %7 %24 %25 17 1 %26 %5 FlagIsProtected|FlagIsPrivate 18 %21
1257+
%28 = OpExtInst %void %1 DebugLocalVariable %6 %23 %25 17 11 %27 FlagIsLocal 1
1258+
%29 = OpExtInst %void %1 DebugTypeFunction FlagIsProtected|FlagIsPrivate %void
1259+
%30 = OpExtInst %void %1 DebugFunction %8 %29 %25 25 1 %26 %5 FlagIsProtected|FlagIsPrivate 25 %21
1260+
%31 = OpExtInst %void %1 DebugLexicalBlock %25 25 13 %30
1261+
%32 = OpExtInst %void %1 DebugInlinedAt 26 %31
1262+
;CHECK: [[fn_param:%\w+]] = OpExtInst %void [[ext:%\w+]] DebugLocalVariable
1263+
;CHECK: [[bb:%\w+]] = OpExtInst %void [[ext]] DebugLexicalBlock
1264+
;CHECK: [[inlined_at:%\w+]] = OpExtInst %void [[ext]] DebugInlinedAt 26 [[bb]]
1265+
%main = OpFunction %void None %18
1266+
%bb = OpLabel
1267+
%33 = OpExtInst %void %1 DebugScope %31
1268+
%34 = OpVariable %_ptr_Function_int Function
1269+
OpLine %3 26 15
1270+
%35 = OpAccessChain %_ptr_Uniform_int %data %uint_0 %uint_0
1271+
%36 = OpLoad %int %35
1272+
;CHECK: DebugScope [[bb]]
1273+
;CHECK: OpLine {{%\w+}} 26 15
1274+
;CHECK: OpStore {{%\w+}} [[val:%\w+]]
1275+
OpStore %34 %36
1276+
%37 = OpExtInst %void %1 DebugScope %27 %32
1277+
%38 = OpExtInst %void %1 DebugDeclare %28 %34 %22
1278+
;CHECK: DebugScope {{%\w+}} [[inlined_at:%\w+]]
1279+
;CHECK: DebugValue [[fn_param]] [[val]]
1280+
OpReturn
1281+
OpFunctionEnd
1282+
)";
1283+
1284+
SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
1285+
SinglePassRunAndMatch<LocalSingleStoreElimPass>(text, false);
1286+
}
1287+
12141288
// TODO(greg-lunarg): Add tests to verify handling of these cases:
12151289
//
12161290
// Other types

0 commit comments

Comments
 (0)