From df4198e50eaa705298167d3c0b3cb01ffd28a5ff Mon Sep 17 00:00:00 2001 From: Jaebaek Seo Date: Tue, 27 Oct 2020 15:10:08 -0400 Subject: [PATCH] Add DebugValue for DebugDecl invisible to value assignment (#3973) For some cases, we have DebugDecl invisible to a value assignment, but the value assignment information is important i.e., debugger cannot inspect the variable without the information. For example, a parameter of an inlined function must have its value assignment i.e., argument passing out of its function scope. If we simply remove DebugDecl because it is invisible to the argument passing, we cannot inspec the variable. This PR - Adds DebugValue for DebugDecl invisible to a value assignment. We use the value of the variable in the basic block that contains DebugDecl, which is found by ssa-rewrite. If the value instruction does not dominate DebugDecl, we use the value of the variable in the immediate dominator of the basic block. - Checks the visibility of DebugDecl for Phi value assignment based on the all value operands of the Phi. Since Phi just references multiple values from multiple basic blocks, scopes of value operands must be regarded as the scope of the Phi. --- source/opt/debug_info_manager.cpp | 92 ++++++--- source/opt/debug_info_manager.h | 27 ++- source/opt/instruction.cpp | 2 +- source/opt/local_single_store_elim_pass.cpp | 4 +- source/opt/ssa_rewrite_pass.cpp | 80 +++++++- source/opt/ssa_rewrite_pass.h | 13 ++ test/opt/ir_context_test.cpp | 8 +- test/opt/local_single_store_elim_test.cpp | 74 ------- test/opt/local_ssa_elim_test.cpp | 211 +++++++++++++++++++- 9 files changed, 374 insertions(+), 137 deletions(-) diff --git a/source/opt/debug_info_manager.cpp b/source/opt/debug_info_manager.cpp index 4941d648e..48285bda6 100644 --- a/source/opt/debug_info_manager.cpp +++ b/source/opt/debug_info_manager.cpp @@ -40,7 +40,6 @@ static const uint32_t kDebugLocalVariableOperandParentIndex = 9; 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 { @@ -441,32 +440,40 @@ bool DebugInfoManager::IsAncestorOfScope(uint32_t scope, uint32_t ancestor) { return false; } -Instruction* DebugInfoManager::GetDebugLocalVariableFromDeclare( - Instruction* dbg_declare) { - assert(dbg_declare); +bool DebugInfoManager::IsDeclareVisibleToInstr(Instruction* dbg_declare, + Instruction* scope) { + assert(dbg_declare != nullptr); + assert(scope != nullptr); + + std::vector scope_ids; + if (scope->opcode() == SpvOpPhi) { + scope_ids.push_back(scope->GetDebugScope().GetLexicalScope()); + for (uint32_t i = 0; i < scope->NumInOperands(); i += 2) { + auto* value = context()->get_def_use_mgr()->GetDef( + scope->GetSingleWordInOperand(i)); + if (value != nullptr) + scope_ids.push_back(value->GetDebugScope().GetLexicalScope()); + } + } else { + scope_ids.push_back(scope->GetDebugScope().GetLexicalScope()); + } + 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()); - 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( + uint32_t decl_scope_id = dbg_local_var_itr->second->GetSingleWordOperand( kDebugLocalVariableOperandParentIndex); // If the scope of DebugDeclare is an ancestor scope of the instruction's // scope, the local variable is visible to the instruction. - return IsAncestorOfScope(instr_scope_id, decl_scope_id); + for (uint32_t scope_id : scope_ids) { + if (scope_id != kNoDebugScope && + IsAncestorOfScope(scope_id, decl_scope_id)) { + return true; + } + } + return false; } Instruction* DebugInfoManager::AddDebugValueWithIndex( @@ -509,26 +516,21 @@ Instruction* DebugInfoManager::AddDebugValueWithIndex( void DebugInfoManager::AddDebugValueIfVarDeclIsVisible( Instruction* scope_and_line, uint32_t variable_id, uint32_t value_id, - Instruction* insert_pos) { + Instruction* insert_pos, + std::unordered_set* invisible_decls) { auto dbg_decl_itr = var_id_to_dbg_decl_.find(variable_id); if (dbg_decl_itr == var_id_to_dbg_decl_.end()) return; - uint32_t instr_scope_id = scope_and_line->GetDebugScope().GetLexicalScope(); for (auto* dbg_decl_or_val : dbg_decl_itr->second) { - // 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)) + if (scope_and_line && + !IsDeclareVisibleToInstr(dbg_decl_or_val, scope_and_line)) { + if (invisible_decls) invisible_decls->insert(dbg_decl_or_val); continue; + } // Avoid inserting the new DebugValue between OpPhi or OpVariable // instructions. - Instruction* insert_before = is_function_param ? dbg_decl_or_val->NextNode() - : insert_pos->NextNode(); + Instruction* insert_before = insert_pos->NextNode(); while (insert_before->opcode() == SpvOpPhi || insert_before->opcode() == SpvOpVariable) { insert_before = insert_before->NextNode(); @@ -545,12 +547,36 @@ void DebugInfoManager::AddDebugValueIfVarDeclIsVisible( kDebugValueOperandLocalVariableIndex), value_id, 0, index_id, insert_before); assert(added_dbg_value != nullptr); - added_dbg_value->UpdateDebugInfoFrom(is_function_param ? dbg_decl_or_val - : scope_and_line); + added_dbg_value->UpdateDebugInfoFrom(scope_and_line ? scope_and_line + : dbg_decl_or_val); AnalyzeDebugInst(added_dbg_value); } } +bool DebugInfoManager::AddDebugValueForDecl(Instruction* dbg_decl, + uint32_t value_id) { + if (dbg_decl == nullptr || !IsDebugDeclare(dbg_decl)) return false; + + std::unique_ptr dbg_val(dbg_decl->Clone(context())); + dbg_val->SetResultId(context()->TakeNextId()); + dbg_val->SetInOperand(kExtInstInstructionInIdx, + {OpenCLDebugInfo100DebugValue}); + dbg_val->SetOperand(kDebugDeclareOperandVariableIndex, {value_id}); + dbg_val->SetOperand(kDebugValueOperandExpressionIndex, + {GetEmptyDebugExpression()->result_id()}); + + auto* added_dbg_val = dbg_decl->InsertBefore(std::move(dbg_val)); + AnalyzeDebugInst(added_dbg_val); + if (context()->AreAnalysesValid(IRContext::Analysis::kAnalysisDefUse)) + context()->get_def_use_mgr()->AnalyzeInstDefUse(added_dbg_val); + if (context()->AreAnalysesValid( + IRContext::Analysis::kAnalysisInstrToBlockMapping)) { + auto insert_blk = context()->get_instr_block(dbg_decl); + context()->set_instr_block(added_dbg_val, insert_blk); + } + return true; +} + uint32_t DebugInfoManager::GetVariableIdOfDebugValueUsedForDeclare( Instruction* inst) { if (inst->GetOpenCL100DebugOpcode() != OpenCLDebugInfo100DebugValue) return 0; diff --git a/source/opt/debug_info_manager.h b/source/opt/debug_info_manager.h index bf398b467..edb11b73f 100644 --- a/source/opt/debug_info_manager.h +++ b/source/opt/debug_info_manager.h @@ -144,9 +144,11 @@ class DebugInfoManager { // Generates a DebugValue instruction with value |value_id| for every local // variable that is in the scope of |scope_and_line| and whose memory is // |variable_id| and inserts it after the instruction |insert_pos|. - void AddDebugValueIfVarDeclIsVisible(Instruction* scope_and_line, - uint32_t variable_id, uint32_t value_id, - Instruction* insert_pos); + // |invisible_decls| returns DebugDeclares invisible to |scope_and_line|. + void AddDebugValueIfVarDeclIsVisible( + Instruction* scope_and_line, uint32_t variable_id, uint32_t value_id, + Instruction* insert_pos, + std::unordered_set* invisible_decls); // Generates a DebugValue instruction with |dbg_local_var_id|, |value_id|, // |expr_id|, |index_id| operands and inserts it before |insert_before|. @@ -155,6 +157,11 @@ class DebugInfoManager { uint32_t index_id, Instruction* insert_before); + // Adds DebugValue for DebugDeclare |dbg_decl|. The new DebugValue has the + // same line, scope, and operands but it uses |value_id| for value. Returns + // weather it succeeds or not. + bool AddDebugValueForDecl(Instruction* dbg_decl, uint32_t value_id); + // Erases |instr| from data structures of this class. void ClearDebugInfo(Instruction* instr); @@ -215,17 +222,9 @@ class DebugInfoManager { // of |scope|. bool IsAncestorOfScope(uint32_t scope, uint32_t ancestor); - // 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 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, Instruction* scope); // Returns the parent scope of the scope |child_scope|. uint32_t GetParentScope(uint32_t child_scope); diff --git a/source/opt/instruction.cpp b/source/opt/instruction.cpp index 961826139..1054a2039 100644 --- a/source/opt/instruction.cpp +++ b/source/opt/instruction.cpp @@ -527,7 +527,7 @@ void Instruction::UpdateDebugInfoFrom(const Instruction* from) { if (from == nullptr) return; clear_dbg_line_insts(); if (!from->dbg_line_insts().empty()) - dbg_line_insts().push_back(from->dbg_line_insts()[0]); + dbg_line_insts().push_back(from->dbg_line_insts().back()); SetDebugScope(from->GetDebugScope()); if (!IsDebugLineInst(opcode()) && context()->AreAnalysesValid(IRContext::kAnalysisDebugInfo)) { diff --git a/source/opt/local_single_store_elim_pass.cpp b/source/opt/local_single_store_elim_pass.cpp index c3479749e..f1b8177bc 100644 --- a/source/opt/local_single_store_elim_pass.cpp +++ b/source/opt/local_single_store_elim_pass.cpp @@ -149,8 +149,8 @@ bool LocalSingleStoreElimPass::ProcessVariable(Instruction* var_inst) { const analysis::Type* store_type = var_type->AsPointer()->pointee_type(); if (!(store_type->AsStruct() || store_type->AsArray())) { context()->get_debug_info_mgr()->AddDebugValueIfVarDeclIsVisible( - store_inst, var_id, store_inst->GetSingleWordInOperand(1), - store_inst); + nullptr, var_id, store_inst->GetSingleWordInOperand(1), store_inst, + nullptr); context()->get_debug_info_mgr()->KillDebugDeclares(var_id); } } diff --git a/source/opt/ssa_rewrite_pass.cpp b/source/opt/ssa_rewrite_pass.cpp index 76f1781aa..5a5688706 100644 --- a/source/opt/ssa_rewrite_pass.cpp +++ b/source/opt/ssa_rewrite_pass.cpp @@ -66,6 +66,7 @@ namespace opt { namespace { const uint32_t kStoreValIdInIdx = 1; const uint32_t kVariableInitIdInIdx = 1; +const uint32_t kDebugDeclareOperandVariableIdx = 5; } // namespace std::string SSARewriter::PhiCandidate::PrettyPrint(const CFG* cfg) const { @@ -241,8 +242,8 @@ uint32_t SSARewriter::AddPhiOperands(PhiCandidate* phi_candidate) { return repl_id; } -uint32_t SSARewriter::GetReachingDef(uint32_t var_id, BasicBlock* bb) { - // If |var_id| has a definition in |bb|, return it. +uint32_t SSARewriter::GetValueAtBlock(uint32_t var_id, BasicBlock* bb) { + assert(bb != nullptr); const auto& bb_it = defs_at_block_.find(bb); if (bb_it != defs_at_block_.end()) { const auto& current_defs = bb_it->second; @@ -251,9 +252,15 @@ uint32_t SSARewriter::GetReachingDef(uint32_t var_id, BasicBlock* bb) { return var_it->second; } } + return 0; +} + +uint32_t SSARewriter::GetReachingDef(uint32_t var_id, BasicBlock* bb) { + // If |var_id| has a definition in |bb|, return it. + uint32_t val_id = GetValueAtBlock(var_id, bb); + if (val_id != 0) return val_id; // Otherwise, look up the value for |var_id| in |bb|'s predecessors. - uint32_t val_id = 0; auto& predecessors = pass_->cfg()->preds(bb->id()); if (predecessors.size() == 1) { // If |bb| has exactly one predecessor, we look for |var_id|'s definition @@ -308,7 +315,7 @@ void SSARewriter::ProcessStore(Instruction* inst, BasicBlock* bb) { if (pass_->IsTargetVar(var_id)) { WriteVariable(var_id, bb, val_id); pass_->context()->get_debug_info_mgr()->AddDebugValueIfVarDeclIsVisible( - inst, var_id, val_id, inst); + inst, var_id, val_id, inst, &decls_invisible_to_value_assignment_); #if SSA_REWRITE_DEBUGGING_LEVEL > 1 std::cerr << "\tFound store '%" << var_id << " = %" << val_id << "': " @@ -439,8 +446,6 @@ bool SSARewriter::ApplyReplacements() { // Add Phi instructions from completed Phi candidates. std::vector generated_phis; - // Add DebugValue instructions for Phi instructions. - std::vector dbg_values_for_phis; for (const PhiCandidate* phi_candidate : phis_to_generate_) { #if SSA_REWRITE_DEBUGGING_LEVEL > 2 std::cerr << "Phi candidate: " << phi_candidate->PrettyPrint(pass_->cfg()) @@ -493,7 +498,7 @@ bool SSARewriter::ApplyReplacements() { insert_it->SetDebugScope(local_var->GetDebugScope()); pass_->context()->get_debug_info_mgr()->AddDebugValueIfVarDeclIsVisible( &*insert_it, phi_candidate->var_id(), phi_candidate->result_id(), - &*insert_it); + &*insert_it, &decls_invisible_to_value_assignment_); modified = true; } @@ -581,6 +586,61 @@ void SSARewriter::FinalizePhiCandidates() { } } +Pass::Status SSARewriter::AddDebugValuesForInvisibleDebugDecls(Function* fp) { + // For the cases the value assignment is invisible to DebugDeclare e.g., + // the argument passing for an inlined function. + // + // Before inlining foo(int x): + // a = 3; + // foo(3); + // After inlining: + // a = 3; // we want to specify "DebugValue: %x = %int_3" + // foo and x disappeared! + // + // We want to specify the value for the variable using |defs_at_block_[bb]|, + // where |bb| is the basic block contains the decl. + DominatorAnalysis* dom_tree = pass_->context()->GetDominatorAnalysis(fp); + Pass::Status status = Pass::Status::SuccessWithoutChange; + for (auto* decl : decls_invisible_to_value_assignment_) { + uint32_t var_id = + decl->GetSingleWordOperand(kDebugDeclareOperandVariableIdx); + auto* var = pass_->get_def_use_mgr()->GetDef(var_id); + if (var->opcode() == SpvOpFunctionParameter) continue; + + auto* bb = pass_->context()->get_instr_block(decl); + uint32_t value_id = GetValueAtBlock(var_id, bb); + Instruction* value = nullptr; + if (value_id) value = pass_->get_def_use_mgr()->GetDef(value_id); + + // If |value| is defined before the function body, it dominates |decl|. + // If |value| dominates |decl|, we can set it as DebugValue. + if (value && (pass_->context()->get_instr_block(value) == nullptr || + dom_tree->Dominates(value, decl))) { + if (!pass_->context()->get_debug_info_mgr()->AddDebugValueForDecl( + decl, value->result_id())) { + return Pass::Status::Failure; + } + } else { + // If |value| in the same basic block does not dominate |decl|, we can + // assign the value in the immediate dominator. + value_id = GetValueAtBlock(var_id, dom_tree->ImmediateDominator(bb)); + if (value_id && + !pass_->context()->get_debug_info_mgr()->AddDebugValueForDecl( + decl, value_id)) { + return Pass::Status::Failure; + } + } + + // DebugDeclares of target variables will be removed by + // SSARewritePass::Process(). + if (!pass_->IsTargetVar(var_id)) { + pass_->context()->get_debug_info_mgr()->KillDebugDeclares(var_id); + } + status = Pass::Status::SuccessWithChange; + } + return status; +} + Pass::Status SSARewriter::RewriteFunctionIntoSSA(Function* fp) { #if SSA_REWRITE_DEBUGGING_LEVEL > 0 std::cerr << "Function before SSA rewrite:\n" @@ -610,6 +670,12 @@ Pass::Status SSARewriter::RewriteFunctionIntoSSA(Function* fp) { // Finally, apply all the replacements in the IR. bool modified = ApplyReplacements(); + auto status = AddDebugValuesForInvisibleDebugDecls(fp); + if (status == Pass::Status::SuccessWithChange || + status == Pass::Status::Failure) { + return status; + } + #if SSA_REWRITE_DEBUGGING_LEVEL > 0 std::cerr << "\n\n\nFunction after SSA rewrite:\n" << fp->PrettyPrint(0) << "\n"; diff --git a/source/opt/ssa_rewrite_pass.h b/source/opt/ssa_rewrite_pass.h index bbbfebbf1..1f4cd24ca 100644 --- a/source/opt/ssa_rewrite_pass.h +++ b/source/opt/ssa_rewrite_pass.h @@ -192,6 +192,10 @@ class SSARewriter { } } + // Returns the value of |var_id| at |bb| if |defs_at_block_| contains it. + // Otherwise, returns 0. + uint32_t GetValueAtBlock(uint32_t var_id, BasicBlock* bb); + // Processes the store operation |inst| in basic block |bb|. This extracts // the variable ID being stored into, determines whether the variable is an // SSA-target variable, and, if it is, it stores its value in the @@ -249,6 +253,11 @@ class SSARewriter { // candidates. void FinalizePhiCandidates(); + // Adds DebugValues for DebugDeclares in + // |decls_invisible_to_value_assignment_|. Returns whether the function was + // modified or not, and whether or not the conversion was successful. + Pass::Status AddDebugValuesForInvisibleDebugDecls(Function* fp); + // Prints the table of Phi candidates to std::cerr. void PrintPhiCandidates() const; @@ -286,6 +295,10 @@ class SSARewriter { // Memory pass requesting the SSA rewriter. MemPass* pass_; + + // Set of DebugDeclare instructions that are not added as DebugValue because + // they are invisible to the store or phi instructions. + std::unordered_set decls_invisible_to_value_assignment_; }; class SSARewritePass : public MemPass { diff --git a/test/opt/ir_context_test.cpp b/test/opt/ir_context_test.cpp index ff8be97f5..b6866d019 100644 --- a/test/opt/ir_context_test.cpp +++ b/test/opt/ir_context_test.cpp @@ -1136,12 +1136,12 @@ OpFunctionEnd)"; // No DebugValue should be added because result id '26' is not used for // DebugDeclare. ctx->get_debug_info_mgr()->AddDebugValueIfVarDeclIsVisible(dbg_decl, 26, 22, - dbg_decl); + dbg_decl, nullptr); EXPECT_EQ(dbg_decl->NextNode()->opcode(), SpvOpReturn); // DebugValue should be added because result id '20' is used for DebugDeclare. ctx->get_debug_info_mgr()->AddDebugValueIfVarDeclIsVisible(dbg_decl, 20, 22, - dbg_decl); + dbg_decl, nullptr); EXPECT_EQ(dbg_decl->NextNode()->GetOpenCL100DebugOpcode(), OpenCLDebugInfo100DebugValue); @@ -1155,14 +1155,14 @@ OpFunctionEnd)"; // No DebugValue should be added because result id '20' is not used for // DebugDeclare. ctx->get_debug_info_mgr()->AddDebugValueIfVarDeclIsVisible(dbg_decl, 20, 7, - dbg_decl); + dbg_decl, nullptr); Instruction* dbg_value = dbg_decl->NextNode(); EXPECT_EQ(dbg_value->GetOpenCL100DebugOpcode(), OpenCLDebugInfo100DebugValue); EXPECT_EQ(dbg_value->GetSingleWordOperand(kDebugValueOperandValueIndex), 22); // DebugValue should be added because result id '26' is used for DebugDeclare. ctx->get_debug_info_mgr()->AddDebugValueIfVarDeclIsVisible(dbg_decl, 26, 7, - dbg_decl); + dbg_decl, nullptr); dbg_value = dbg_decl->NextNode(); EXPECT_EQ(dbg_value->GetOpenCL100DebugOpcode(), OpenCLDebugInfo100DebugValue); EXPECT_EQ(dbg_value->GetSingleWordOperand(kDebugValueOperandValueIndex), 7); diff --git a/test/opt/local_single_store_elim_test.cpp b/test/opt/local_single_store_elim_test.cpp index f1b98212c..ebc89a630 100644 --- a/test/opt/local_single_store_elim_test.cpp +++ b/test/opt/local_single_store_elim_test.cpp @@ -1211,80 +1211,6 @@ TEST_F(LocalSingleStoreElimTest, DebugValueTest) { SinglePassRunAndMatch(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(text, false); -} - // TODO(greg-lunarg): Add tests to verify handling of these cases: // // Other types diff --git a/test/opt/local_ssa_elim_test.cpp b/test/opt/local_ssa_elim_test.cpp index 3d3c4791d..2ecd238b7 100644 --- a/test/opt/local_ssa_elim_test.cpp +++ b/test/opt/local_ssa_elim_test.cpp @@ -2165,6 +2165,212 @@ OpFunctionEnd SinglePassRunAndMatch(text, true); } +TEST_F(LocalSSAElimTest, AddDebugValueForFunctionParameterWithPhi) { + // Test the distribution of DebugValue for a parameter of an inlined function + // and the visibility of Phi instruction. The ssa-rewrite pass must add + // DebugValue for the value assignment of function argument even when it is an + // inlined function. It has to check the visibility Phi through all its value + // operands. See the DebugValue for "int i" of "foo()" in the following code. + // + // struct VS_OUTPUT { + // float4 pos : SV_POSITION; + // float4 color : COLOR; + // }; + // + // float4 foo(int i, float4 pos) { + // while (i < pos.x) { + // pos = pos.x + i; + // ++i; + // } + // return pos; + // } + // + // VS_OUTPUT main(float4 pos : POSITION, + // float4 color : COLOR) { + // VS_OUTPUT vout; + // vout.pos = foo(4, pos); + // vout.color = color; + // return vout; + // } + const std::string text = R"( + OpCapability Shader + %1 = OpExtInstImport "OpenCL.DebugInfo.100" + OpMemoryModel Logical GLSL450 + OpEntryPoint Vertex %main "main" %in_var_POSITION %in_var_COLOR %gl_Position %out_var_COLOR + %7 = OpString "vertex.hlsl" + %8 = OpString "float" + %9 = OpString "VS_OUTPUT" + %10 = OpString "color" + %11 = OpString "pos" + %12 = OpString "int" + %13 = OpString "foo" + %14 = OpString "" + %15 = OpString "i" + %16 = OpString "main" + %17 = OpString "vout" + OpName %in_var_POSITION "in.var.POSITION" + OpName %in_var_COLOR "in.var.COLOR" + OpName %out_var_COLOR "out.var.COLOR" + OpName %main "main" + OpName %param_var_pos "param.var.pos" + OpName %param_var_color "param.var.color" + OpName %VS_OUTPUT "VS_OUTPUT" + OpMemberName %VS_OUTPUT 0 "pos" + OpMemberName %VS_OUTPUT 1 "color" + OpDecorate %gl_Position BuiltIn Position + OpDecorate %in_var_POSITION Location 0 + OpDecorate %in_var_COLOR Location 1 + OpDecorate %out_var_COLOR Location 0 + %int = OpTypeInt 32 1 + %int_4 = OpConstant %int 4 + %int_0 = OpConstant %int 0 + %int_1 = OpConstant %int 1 + %uint = OpTypeInt 32 0 + %uint_32 = OpConstant %uint 32 + %float = OpTypeFloat 32 + %v4float = OpTypeVector %float 4 +%_ptr_Input_v4float = OpTypePointer Input %v4float +%_ptr_Output_v4float = OpTypePointer Output %v4float + %void = OpTypeVoid + %uint_256 = OpConstant %uint 256 + %uint_128 = OpConstant %uint 128 + %uint_0 = OpConstant %uint 0 + %50 = OpTypeFunction %void +%_ptr_Function_v4float = OpTypePointer Function %v4float + %VS_OUTPUT = OpTypeStruct %v4float %v4float +%_ptr_Function_int = OpTypePointer Function %int +%_ptr_Function_float = OpTypePointer Function %float + %bool = OpTypeBool +%in_var_POSITION = OpVariable %_ptr_Input_v4float Input +%in_var_COLOR = OpVariable %_ptr_Input_v4float Input +%gl_Position = OpVariable %_ptr_Output_v4float Output +%out_var_COLOR = OpVariable %_ptr_Output_v4float Output + %156 = OpExtInst %void %1 DebugInfoNone + %77 = OpExtInst %void %1 DebugExpression + %58 = OpExtInst %void %1 DebugTypeBasic %8 %uint_32 Float + %59 = OpExtInst %void %1 DebugTypeVector %58 4 + %60 = OpExtInst %void %1 DebugSource %7 + %61 = OpExtInst %void %1 DebugCompilationUnit 1 4 %60 HLSL + %62 = OpExtInst %void %1 DebugTypeComposite %9 Structure %60 1 8 %61 %9 %uint_256 FlagIsProtected|FlagIsPrivate %63 %64 + %64 = OpExtInst %void %1 DebugTypeMember %10 %59 %60 3 10 %62 %uint_128 %uint_128 FlagIsProtected|FlagIsPrivate + %63 = OpExtInst %void %1 DebugTypeMember %11 %59 %60 2 10 %62 %uint_0 %uint_128 FlagIsProtected|FlagIsPrivate + %65 = OpExtInst %void %1 DebugTypeBasic %12 %uint_32 Signed + %66 = OpExtInst %void %1 DebugTypeFunction FlagIsProtected|FlagIsPrivate %59 %65 %59 + %67 = OpExtInst %void %1 DebugFunction %13 %66 %60 6 1 %61 %14 FlagIsProtected|FlagIsPrivate 6 %156 + %68 = OpExtInst %void %1 DebugLexicalBlock %60 6 31 %67 + %69 = OpExtInst %void %1 DebugLexicalBlock %60 7 21 %68 + %70 = OpExtInst %void %1 DebugLocalVariable %11 %59 %60 6 26 %67 FlagIsLocal 2 + +; CHECK: [[i_name:%\w+]] = OpString "i" +; CHECK: [[null_expr:%\w+]] = OpExtInst %void [[ext:%\w+]] DebugExpression +; CHECK: [[dbg_i:%\w+]] = OpExtInst %void [[ext]] DebugLocalVariable [[i_name]] {{%\w+}} {{%\w+}} 6 16 {{%\w+}} FlagIsLocal 1 + %71 = OpExtInst %void %1 DebugLocalVariable %15 %65 %60 6 16 %67 FlagIsLocal 1 + %72 = OpExtInst %void %1 DebugTypeFunction FlagIsProtected|FlagIsPrivate %62 %59 %59 + %73 = OpExtInst %void %1 DebugFunction %16 %72 %60 14 1 %61 %14 FlagIsProtected|FlagIsPrivate 15 %156 + %74 = OpExtInst %void %1 DebugLexicalBlock %60 15 38 %73 + %75 = OpExtInst %void %1 DebugLocalVariable %17 %62 %60 16 13 %74 FlagIsLocal + %76 = OpExtInst %void %1 DebugLocalVariable %10 %59 %60 15 23 %73 FlagIsLocal 2 + %78 = OpExtInst %void %1 DebugLocalVariable %11 %59 %60 14 23 %73 FlagIsLocal 1 + %155 = OpExtInst %void %1 DebugInlinedAt 17 %74 + %main = OpFunction %void None %50 + %79 = OpLabel + %168 = OpExtInst %void %1 DebugScope %74 + +; CHECK: [[i:%\w+]] = OpVariable %_ptr_Function_int Function + %120 = OpVariable %_ptr_Function_int Function + %121 = OpVariable %_ptr_Function_v4float Function + %169 = OpExtInst %void %1 DebugNoScope +%param_var_pos = OpVariable %_ptr_Function_v4float Function +%param_var_color = OpVariable %_ptr_Function_v4float Function + %80 = OpLoad %v4float %in_var_POSITION + OpStore %param_var_pos %80 + %81 = OpLoad %v4float %in_var_COLOR + OpStore %param_var_color %81 + %170 = OpExtInst %void %1 DebugScope %73 + %124 = OpExtInst %void %1 DebugDeclare %78 %param_var_pos %77 + %125 = OpExtInst %void %1 DebugDeclare %76 %param_var_color %77 + %171 = OpExtInst %void %1 DebugScope %74 + OpLine %7 17 18 + +; CHECK: OpStore {{%\w+}} %int_4 +; CHECK: DebugValue [[dbg_i]] %int_4 [[null_expr]] + OpStore %120 %int_4 + OpStore %121 %80 + %172 = OpExtInst %void %1 DebugScope %67 %155 + %135 = OpExtInst %void %1 DebugDeclare %71 %120 %77 + %136 = OpExtInst %void %1 DebugDeclare %70 %121 %77 + %173 = OpExtInst %void %1 DebugScope %68 %155 + OpLine %7 7 3 + OpBranch %137 + %174 = OpExtInst %void %1 DebugNoScope + %137 = OpLabel + +; CHECK: [[phi:%\w+]] = OpPhi %int %int_4 +; CHECK: DebugValue [[dbg_i]] [[phi]] [[null_expr]] + %175 = OpExtInst %void %1 DebugScope %68 %155 + OpLine %7 7 10 + %138 = OpLoad %int %120 + %139 = OpConvertSToF %float %138 + OpLine %7 7 14 + %140 = OpAccessChain %_ptr_Function_float %121 %int_0 + %141 = OpLoad %float %140 + OpLine %7 7 12 + %142 = OpFOrdLessThan %bool %139 %141 + OpLine %7 7 3 + %176 = OpExtInst %void %1 DebugNoScope + OpLoopMerge %153 %152 None + OpBranchConditional %142 %143 %153 + %177 = OpExtInst %void %1 DebugNoScope + %143 = OpLabel + %178 = OpExtInst %void %1 DebugScope %69 %155 + OpLine %7 8 11 + %144 = OpAccessChain %_ptr_Function_float %121 %int_0 + %145 = OpLoad %float %144 + OpLine %7 8 19 + %146 = OpLoad %int %120 + %147 = OpConvertSToF %float %146 + OpLine %7 8 17 + %148 = OpFAdd %float %145 %147 + OpLine %7 8 11 + %149 = OpCompositeConstruct %v4float %148 %148 %148 %148 + OpLine %7 8 5 + OpStore %121 %149 + OpLine %7 9 5 + %151 = OpIAdd %int %146 %int_1 + OpLine %7 9 7 + +; CHECK: OpStore [[i]] [[value:%\w+]] +; CHECK: DebugValue [[dbg_i]] [[value]] [[null_expr]] + OpStore %120 %151 + %179 = OpExtInst %void %1 DebugScope %68 %155 + OpLine %7 10 3 + OpBranch %152 + %180 = OpExtInst %void %1 DebugNoScope + %152 = OpLabel + %181 = OpExtInst %void %1 DebugScope %68 %155 + OpBranch %137 + %182 = OpExtInst %void %1 DebugNoScope + %153 = OpLabel + %183 = OpExtInst %void %1 DebugScope %68 %155 + OpLine %7 11 10 + %154 = OpLoad %v4float %121 + %184 = OpExtInst %void %1 DebugScope %74 + %167 = OpExtInst %void %1 DebugValue %75 %154 %77 %int_0 + %166 = OpExtInst %void %1 DebugValue %75 %81 %77 %int_1 + OpLine %7 19 10 + %165 = OpCompositeConstruct %VS_OUTPUT %154 %81 + %185 = OpExtInst %void %1 DebugNoScope + %83 = OpCompositeExtract %v4float %165 0 + OpStore %gl_Position %83 + %84 = OpCompositeExtract %v4float %165 1 + OpStore %out_var_COLOR %84 + OpReturn + OpFunctionEnd +)"; + + SinglePassRunAndMatch(text, true); +} + TEST_F(LocalSSAElimTest, PartiallyKillDebugDeclare) { // For a reference variable e.g., int i in the following example, // we do not propagate DebugValue for a store or phi instruction @@ -2486,8 +2692,9 @@ TEST_F(LocalSSAElimTest, DebugValueForReferenceVariableInBB) { ; CHECK: OpExtInst %void [[ext]] DebugScope [[dbg_main]] ; CHECK: [[phi0:%\w+]] = OpPhi %float %float_0 ; CHECK: [[phi1:%\w+]] = OpPhi %int %int_0 -; CHECK-NEXT: OpExtInst %void [[ext]] DebugValue [[dbg_f]] [[phi0]] -; CHECK-NEXT: OpExtInst %void [[ext]] DebugValue [[dbg_i]] [[phi1]] +; CHECK-DAG: OpExtInst %void [[ext]] DebugValue [[dbg_f]] [[phi0]] +; CHECK-DAG: OpExtInst %void [[ext]] DebugValue [[dbg_x]] [[phi0]] +; CHECK-DAG: OpExtInst %void [[ext]] DebugValue [[dbg_i]] [[phi1]] ; CHECK: OpLoopMerge [[loop_merge:%\w+]] [[loop_cont:%\w+]] None ; CHECK-NEXT: OpBranch [[loop_body:%\w+]]