diff options
author | Greg Fischer <greg@lunarg.com> | 2022-09-21 00:27:23 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-09-21 00:27:23 +0300 |
commit | 11d0d162279b7cd19f82dac3934149dd97a3a42c (patch) | |
tree | 7b13944cab05f0010113b8f7e0c369cf961e9fd7 /source | |
parent | 91c29a197f20e58dfc436e1d9f28bef5a9ba9c4c (diff) |
Cleanup code for 272e4b3d0 (#4934)
Removed now unused DebugDeclare visibility logic for generating
DebugValue.
Also eliminated the phi sort introduced in 272e4b3. This should have
been removed in the first commit.
Diffstat (limited to 'source')
-rw-r--r-- | source/opt/debug_info_manager.cpp | 13 | ||||
-rw-r--r-- | source/opt/debug_info_manager.h | 11 | ||||
-rw-r--r-- | source/opt/local_single_store_elim_pass.cpp | 24 | ||||
-rw-r--r-- | source/opt/ssa_rewrite_pass.cpp | 91 | ||||
-rw-r--r-- | source/opt/ssa_rewrite_pass.h | 11 |
5 files changed, 19 insertions, 131 deletions
diff --git a/source/opt/debug_info_manager.cpp b/source/opt/debug_info_manager.cpp index 98d98d4ff..5ec0efb84 100644 --- a/source/opt/debug_info_manager.cpp +++ b/source/opt/debug_info_manager.cpp @@ -562,10 +562,10 @@ bool DebugInfoManager::IsDeclareVisibleToInstr(Instruction* dbg_declare, return false; } -bool DebugInfoManager::AddDebugValueIfVarDeclIsVisible( - Instruction* scope_and_line, uint32_t variable_id, uint32_t value_id, - Instruction* insert_pos, std::unordered_set<Instruction*>* invisible_decls, - bool force) { +bool DebugInfoManager::AddDebugValueForVariable(Instruction* scope_and_line, + uint32_t variable_id, + uint32_t value_id, + Instruction* insert_pos) { assert(scope_and_line != nullptr); auto dbg_decl_itr = var_id_to_dbg_decl_.find(variable_id); @@ -573,11 +573,6 @@ bool DebugInfoManager::AddDebugValueIfVarDeclIsVisible( bool modified = false; for (auto* dbg_decl_or_val : dbg_decl_itr->second) { - if (!IsDeclareVisibleToInstr(dbg_decl_or_val, scope_and_line) && !force) { - if (invisible_decls) invisible_decls->insert(dbg_decl_or_val); - continue; - } - // Avoid inserting the new DebugValue between OpPhi or OpVariable // instructions. Instruction* insert_before = insert_pos->NextNode(); diff --git a/source/opt/debug_info_manager.h b/source/opt/debug_info_manager.h index 7dfb7c7f9..abb7b9a08 100644 --- a/source/opt/debug_info_manager.h +++ b/source/opt/debug_info_manager.h @@ -145,13 +145,10 @@ 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|. - // Returns whether a DebugValue is added or not. |invisible_decls| returns - // DebugDeclares invisible to |scope_and_line|. Assume visible if |force| - // is true. - bool AddDebugValueIfVarDeclIsVisible( - Instruction* scope_and_line, uint32_t variable_id, uint32_t value_id, - Instruction* insert_pos, - std::unordered_set<Instruction*>* invisible_decls, bool force = false); + // Returns whether a DebugValue is added or not. + bool AddDebugValueForVariable(Instruction* scope_and_line, + uint32_t variable_id, uint32_t value_id, + Instruction* insert_pos); // Creates a DebugValue for DebugDeclare |dbg_decl| and inserts it before // |insert_before|. The new DebugValue has the same line and scope as diff --git a/source/opt/local_single_store_elim_pass.cpp b/source/opt/local_single_store_elim_pass.cpp index 2efb1e2c8..81648c7ba 100644 --- a/source/opt/local_single_store_elim_pass.cpp +++ b/source/opt/local_single_store_elim_pass.cpp @@ -175,29 +175,9 @@ bool LocalSingleStoreElimPass::ProcessVariable(Instruction* var_inst) { bool LocalSingleStoreElimPass::RewriteDebugDeclares(Instruction* store_inst, uint32_t var_id) { - std::unordered_set<Instruction*> invisible_decls; uint32_t value_id = store_inst->GetSingleWordInOperand(1); - bool modified = - context()->get_debug_info_mgr()->AddDebugValueIfVarDeclIsVisible( - store_inst, var_id, value_id, store_inst, &invisible_decls, true); - - // For cases like the argument passing for an inlined function, the value - // assignment is out of DebugDeclare's scope, but we have to preserve the - // value assignment information using DebugValue. Generally, we need - // ssa-rewrite analysis to decide a proper value assignment but at this point - // we confirm that |var_id| has a single store. We can safely add DebugValue. - if (!invisible_decls.empty()) { - BasicBlock* store_block = context()->get_instr_block(store_inst); - DominatorAnalysis* dominator_analysis = - context()->GetDominatorAnalysis(store_block->GetParent()); - for (auto* decl : invisible_decls) { - if (dominator_analysis->Dominates(store_inst, decl)) { - context()->get_debug_info_mgr()->AddDebugValueForDecl(decl, value_id, - decl, store_inst); - modified = true; - } - } - } + bool modified = context()->get_debug_info_mgr()->AddDebugValueForVariable( + store_inst, var_id, value_id, store_inst); modified |= context()->get_debug_info_mgr()->KillDebugDeclares(var_id); return modified; } diff --git a/source/opt/ssa_rewrite_pass.cpp b/source/opt/ssa_rewrite_pass.cpp index 23cd64135..22d811044 100644 --- a/source/opt/ssa_rewrite_pass.cpp +++ b/source/opt/ssa_rewrite_pass.cpp @@ -67,7 +67,6 @@ 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 { @@ -315,9 +314,8 @@ 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, &decls_invisible_to_value_assignment_, - true); + pass_->context()->get_debug_info_mgr()->AddDebugValueForVariable( + inst, var_id, val_id, inst); #if SSA_REWRITE_DEBUGGING_LEVEL > 1 std::cerr << "\tFound store '%" << var_id << " = %" << val_id << "': " @@ -498,7 +496,7 @@ uint32_t SSARewriter::GetPhiArgument(const PhiCandidate* phi_candidate, return 0; } -bool SSARewriter::ApplyReplacements(Function* fp) { +bool SSARewriter::ApplyReplacements() { bool modified = false; #if SSA_REWRITE_DEBUGGING_LEVEL > 2 @@ -508,20 +506,9 @@ bool SSARewriter::ApplyReplacements(Function* fp) { std::cerr << "\n\n"; #endif - // Sort phi candiates by reverse postorder. Operand of a Phi may be - // a phi itself so make sure all operand phis are generated first. - std::vector<const PhiCandidate*> ordered_phis_to_generate; - pass_->context()->cfg()->ForEachBlockInReversePostOrder( - &*fp->begin(), [&ordered_phis_to_generate, this](BasicBlock* bb) { - for (const PhiCandidate* phi_candidate : phis_to_generate_) { - if (phi_candidate->bb() == bb) - ordered_phis_to_generate.push_back(phi_candidate); - } - }); - // Add Phi instructions from completed Phi candidates. std::vector<Instruction*> generated_phis; - for (const PhiCandidate* phi_candidate : ordered_phis_to_generate) { + for (const PhiCandidate* phi_candidate : phis_to_generate_) { #if SSA_REWRITE_DEBUGGING_LEVEL > 2 std::cerr << "Phi candidate: " << phi_candidate->PrettyPrint(pass_->cfg()) << "\n"; @@ -569,11 +556,11 @@ bool SSARewriter::ApplyReplacements(Function* fp) { phi_candidate->var_id(), phi_candidate->result_id(), {SpvDecorationRelaxedPrecision}); - // Add DebugValue for the new OpPhi instruction. Assume OpPhi is visible. + // Add DebugValue for the new OpPhi instruction. insert_it->SetDebugScope(local_var->GetDebugScope()); - pass_->context()->get_debug_info_mgr()->AddDebugValueIfVarDeclIsVisible( + pass_->context()->get_debug_info_mgr()->AddDebugValueForVariable( &*insert_it, phi_candidate->var_id(), phi_candidate->result_id(), - &*insert_it, &decls_invisible_to_value_assignment_, true); + &*insert_it); modified = true; } @@ -662,62 +649,6 @@ 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; - // foo and x disappeared but we want to specify "DebugValue: %x = %int_3". - // - // 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(), decl, value) == nullptr) { - 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) value = pass_->get_def_use_mgr()->GetDef(value_id); - if (value_id && - pass_->context()->get_debug_info_mgr()->AddDebugValueForDecl( - decl, value_id, decl, value) == nullptr) { - 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" @@ -745,13 +676,7 @@ Pass::Status SSARewriter::RewriteFunctionIntoSSA(Function* fp) { FinalizePhiCandidates(); // Finally, apply all the replacements in the IR. - bool modified = ApplyReplacements(fp); - - auto status = AddDebugValuesForInvisibleDebugDecls(fp); - if (status == Pass::Status::SuccessWithChange || - status == Pass::Status::Failure) { - return status; - } + bool modified = ApplyReplacements(); #if SSA_REWRITE_DEBUGGING_LEVEL > 0 std::cerr << "\n\n\nFunction after SSA rewrite:\n" diff --git a/source/opt/ssa_rewrite_pass.h b/source/opt/ssa_rewrite_pass.h index 44cdd360a..2470f85f6 100644 --- a/source/opt/ssa_rewrite_pass.h +++ b/source/opt/ssa_rewrite_pass.h @@ -181,7 +181,7 @@ class SSARewriter { // Applies all the SSA replacement decisions. This replaces loads/stores to // SSA target variables with their corresponding SSA IDs, and inserts Phi // instructions for them. - bool ApplyReplacements(Function* fp); + bool ApplyReplacements(); // Registers a definition for variable |var_id| in basic block |bb| with // value |val_id|. @@ -253,11 +253,6 @@ 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; @@ -295,10 +290,6 @@ 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<Instruction*> decls_invisible_to_value_assignment_; }; class SSARewritePass : public MemPass { |