Add DebugValue for function param regardless of scope (#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()`.diff --git a/source/opt/debug_info_manager.cpp b/source/opt/debug_info_manager.cpp
index 346134d..4941d64 100644
--- a/source/opt/debug_info_manager.cpp
+++ b/source/opt/debug_info_manager.cpp
@@ -40,6 +40,7 @@
static const uint32_t kExtInstInstructionInIdx = 1;
static const uint32_t kDebugGlobalVariableOperandFlagsIndex = 12;
static const uint32_t kDebugLocalVariableOperandFlagsIndex = 10;
+static const uint32_t kDebugLocalVariableOperandArgNumberIndex = 11;
namespace spvtools {
namespace opt {
@@ -440,15 +441,27 @@
return false;
}
-bool DebugInfoManager::IsDeclareVisibleToInstr(Instruction* dbg_declare,
- uint32_t instr_scope_id) {
- if (instr_scope_id == kNoDebugScope) return false;
-
+Instruction* DebugInfoManager::GetDebugLocalVariableFromDeclare(
+ Instruction* dbg_declare) {
+ assert(dbg_declare);
uint32_t dbg_local_var_id =
dbg_declare->GetSingleWordOperand(kDebugDeclareOperandLocalVariableIndex);
auto dbg_local_var_itr = id_to_dbg_inst_.find(dbg_local_var_id);
assert(dbg_local_var_itr != id_to_dbg_inst_.end());
- uint32_t decl_scope_id = dbg_local_var_itr->second->GetSingleWordOperand(
+ return dbg_local_var_itr->second;
+}
+
+bool DebugInfoManager::IsFunctionParameter(Instruction* dbg_local_var) const {
+ // If a DebugLocalVariable has ArgNumber operand, it is a function parameter.
+ return dbg_local_var->NumOperands() >
+ kDebugLocalVariableOperandArgNumberIndex;
+}
+
+bool DebugInfoManager::IsLocalVariableVisibleToInstr(Instruction* dbg_local_var,
+ uint32_t instr_scope_id) {
+ if (instr_scope_id == kNoDebugScope) return false;
+
+ uint32_t decl_scope_id = dbg_local_var->GetSingleWordOperand(
kDebugLocalVariableOperandParentIndex);
// If the scope of DebugDeclare is an ancestor scope of the instruction's
@@ -502,11 +515,20 @@
uint32_t instr_scope_id = scope_and_line->GetDebugScope().GetLexicalScope();
for (auto* dbg_decl_or_val : dbg_decl_itr->second) {
- if (!IsDeclareVisibleToInstr(dbg_decl_or_val, instr_scope_id)) continue;
+ // If it declares a function parameter, the store instruction for the
+ // function parameter can exist out of the function parameter's scope
+ // because of the function inlining. We always add DebugValue for a
+ // function parameter next to the DebugDeclare regardless of the scope.
+ auto* dbg_local_var = GetDebugLocalVariableFromDeclare(dbg_decl_or_val);
+ bool is_function_param = IsFunctionParameter(dbg_local_var);
+ if (!is_function_param &&
+ !IsLocalVariableVisibleToInstr(dbg_local_var, instr_scope_id))
+ continue;
// Avoid inserting the new DebugValue between OpPhi or OpVariable
// instructions.
- Instruction* insert_before = insert_pos->NextNode();
+ Instruction* insert_before = is_function_param ? dbg_decl_or_val->NextNode()
+ : insert_pos->NextNode();
while (insert_before->opcode() == SpvOpPhi ||
insert_before->opcode() == SpvOpVariable) {
insert_before = insert_before->NextNode();
@@ -523,7 +545,8 @@
kDebugValueOperandLocalVariableIndex),
value_id, 0, index_id, insert_before);
assert(added_dbg_value != nullptr);
- added_dbg_value->UpdateDebugInfoFrom(scope_and_line);
+ added_dbg_value->UpdateDebugInfoFrom(is_function_param ? dbg_decl_or_val
+ : scope_and_line);
AnalyzeDebugInst(added_dbg_value);
}
}
diff --git a/source/opt/debug_info_manager.h b/source/opt/debug_info_manager.h
index 6e7373b..bf398b4 100644
--- a/source/opt/debug_info_manager.h
+++ b/source/opt/debug_info_manager.h
@@ -215,10 +215,17 @@
// of |scope|.
bool IsAncestorOfScope(uint32_t scope, uint32_t ancestor);
- // Returns true if the declaration of a local variable |dbg_declare|
- // is visible in the scope of an instruction |instr_scope_id|.
- bool IsDeclareVisibleToInstr(Instruction* dbg_declare,
- uint32_t instr_scope_id);
+ // Returns the DebugLocalVariable declared by |dbg_declare|.
+ Instruction* GetDebugLocalVariableFromDeclare(Instruction* dbg_declare);
+
+ // Returns true if the DebugLocalVariable |dbg_local_var| is a function
+ // parameter.
+ bool IsFunctionParameter(Instruction* dbg_local_var) const;
+
+ // Returns true if the DebugLocalVariable |dbg_local_var| is visible
+ // in the scope of an instruction |instr_scope_id|.
+ bool IsLocalVariableVisibleToInstr(Instruction* dbg_local_var,
+ uint32_t instr_scope_id);
// Returns the parent scope of the scope |child_scope|.
uint32_t GetParentScope(uint32_t child_scope);
diff --git a/test/opt/local_single_store_elim_test.cpp b/test/opt/local_single_store_elim_test.cpp
index ebc89a6..f1b9821 100644
--- a/test/opt/local_single_store_elim_test.cpp
+++ b/test/opt/local_single_store_elim_test.cpp
@@ -1211,6 +1211,80 @@
SinglePassRunAndMatch<LocalSingleStoreElimPass>(text, false);
}
+TEST_F(LocalSingleStoreElimTest, DebugDeclareForFunctionParameter) {
+ // We have to add DebugValue for DebugDeclare regardless of the scope
+ // if it declares a function parameter.
+ const std::string text = R"(
+ OpCapability Shader
+ %1 = OpExtInstImport "OpenCL.DebugInfo.100"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint GLCompute %main "main"
+ OpExecutionMode %main LocalSize 1 1 1
+ %3 = OpString "fn_call.hlsl"
+ %4 = OpString "int"
+ %5 = OpString ""
+ %6 = OpString "x"
+ %7 = OpString "A"
+ %8 = OpString "main"
+ OpName %type_StructuredBuffer_int "type.StructuredBuffer.int"
+ OpName %data "data"
+ OpName %main "main"
+ OpDecorate %data DescriptorSet 0
+ OpDecorate %data Binding 0
+ OpDecorate %_runtimearr_int ArrayStride 4
+ OpMemberDecorate %type_StructuredBuffer_int 0 Offset 0
+ OpMemberDecorate %type_StructuredBuffer_int 0 NonWritable
+ OpDecorate %type_StructuredBuffer_int BufferBlock
+ %int = OpTypeInt 32 1
+ %uint = OpTypeInt 32 0
+ %uint_0 = OpConstant %uint 0
+ %uint_32 = OpConstant %uint 32
+%_runtimearr_int = OpTypeRuntimeArray %int
+%type_StructuredBuffer_int = OpTypeStruct %_runtimearr_int
+%_ptr_Uniform_type_StructuredBuffer_int = OpTypePointer Uniform %type_StructuredBuffer_int
+ %void = OpTypeVoid
+ %18 = OpTypeFunction %void
+%_ptr_Function_int = OpTypePointer Function %int
+%_ptr_Uniform_int = OpTypePointer Uniform %int
+ %data = OpVariable %_ptr_Uniform_type_StructuredBuffer_int Uniform
+ %21 = OpExtInst %void %1 DebugInfoNone
+ %22 = OpExtInst %void %1 DebugExpression
+ %23 = OpExtInst %void %1 DebugTypeBasic %4 %uint_32 Signed
+ %24 = OpExtInst %void %1 DebugTypeFunction FlagIsProtected|FlagIsPrivate %23 %23
+ %25 = OpExtInst %void %1 DebugSource %3
+ %26 = OpExtInst %void %1 DebugCompilationUnit 1 4 %25 HLSL
+ %27 = OpExtInst %void %1 DebugFunction %7 %24 %25 17 1 %26 %5 FlagIsProtected|FlagIsPrivate 18 %21
+ %28 = OpExtInst %void %1 DebugLocalVariable %6 %23 %25 17 11 %27 FlagIsLocal 1
+ %29 = OpExtInst %void %1 DebugTypeFunction FlagIsProtected|FlagIsPrivate %void
+ %30 = OpExtInst %void %1 DebugFunction %8 %29 %25 25 1 %26 %5 FlagIsProtected|FlagIsPrivate 25 %21
+ %31 = OpExtInst %void %1 DebugLexicalBlock %25 25 13 %30
+ %32 = OpExtInst %void %1 DebugInlinedAt 26 %31
+;CHECK: [[fn_param:%\w+]] = OpExtInst %void [[ext:%\w+]] DebugLocalVariable
+;CHECK: [[bb:%\w+]] = OpExtInst %void [[ext]] DebugLexicalBlock
+;CHECK: [[inlined_at:%\w+]] = OpExtInst %void [[ext]] DebugInlinedAt 26 [[bb]]
+ %main = OpFunction %void None %18
+ %bb = OpLabel
+ %33 = OpExtInst %void %1 DebugScope %31
+ %34 = OpVariable %_ptr_Function_int Function
+ OpLine %3 26 15
+ %35 = OpAccessChain %_ptr_Uniform_int %data %uint_0 %uint_0
+ %36 = OpLoad %int %35
+;CHECK: DebugScope [[bb]]
+;CHECK: OpLine {{%\w+}} 26 15
+;CHECK: OpStore {{%\w+}} [[val:%\w+]]
+ OpStore %34 %36
+ %37 = OpExtInst %void %1 DebugScope %27 %32
+ %38 = OpExtInst %void %1 DebugDeclare %28 %34 %22
+;CHECK: DebugScope {{%\w+}} [[inlined_at:%\w+]]
+;CHECK: DebugValue [[fn_param]] [[val]]
+ OpReturn
+ OpFunctionEnd
+ )";
+
+ SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
+ SinglePassRunAndMatch<LocalSingleStoreElimPass>(text, false);
+}
+
// TODO(greg-lunarg): Add tests to verify handling of these cases:
//
// Other types
