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.
diff --git a/source/opt/debug_info_manager.cpp b/source/opt/debug_info_manager.cpp
index 98d98d4..5ec0efb 100644
--- a/source/opt/debug_info_manager.cpp
+++ b/source/opt/debug_info_manager.cpp
@@ -562,10 +562,10 @@
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 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 7dfb7c7..abb7b9a 100644
--- a/source/opt/debug_info_manager.h
+++ b/source/opt/debug_info_manager.h
@@ -145,13 +145,10 @@
// 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 2efb1e2..81648c7 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::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 23cd641..22d8110 100644
--- a/source/opt/ssa_rewrite_pass.cpp
+++ b/source/opt/ssa_rewrite_pass.cpp
@@ -67,7 +67,6 @@
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 @@
}
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 @@
return 0;
}
-bool SSARewriter::ApplyReplacements(Function* fp) {
+bool SSARewriter::ApplyReplacements() {
bool modified = false;
#if SSA_REWRITE_DEBUGGING_LEVEL > 2
@@ -508,20 +506,9 @@
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 @@
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 @@
}
}
-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 @@
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 44cdd36..2470f85 100644
--- a/source/opt/ssa_rewrite_pass.h
+++ b/source/opt/ssa_rewrite_pass.h
@@ -181,7 +181,7 @@
// 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 @@
// 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 @@
// 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 {
