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.

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 {