Update based on code review

KhronosGroup · Jul 17, 2020 · ebb3182 · ebb3182
1 parent ca38381
commit ebb3182
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 13 deletions.
diff --git a/source/opt/debug_info_manager.cpp b/source/opt/debug_info_manager.cpp
@@ -483,9 +483,9 @@ Instruction* DebugInfoManager::AddDebugValueWithIndex(
   return added_dbg_value;
 }
 
-void DebugInfoManager::AddDebugValue(Instruction* scope_and_line,
-                                     uint32_t variable_id, uint32_t value_id,
-                                     Instruction* insert_pos) {
+void DebugInfoManager::AddDebugValueIfVarDeclIsVisible(
+    Instruction* scope_and_line, uint32_t variable_id, uint32_t value_id,
+    Instruction* insert_pos) {
   auto dbg_decl_itr = var_id_to_dbg_decl_.find(variable_id);
   if (dbg_decl_itr == var_id_to_dbg_decl_.end()) return;
 

diff --git a/source/opt/debug_info_manager.h b/source/opt/debug_info_manager.h
@@ -142,8 +142,9 @@ 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 AddDebugValue(Instruction* scope_and_line, uint32_t variable_id,
-                     uint32_t value_id, Instruction* insert_pos);
+  void AddDebugValueIfVarDeclIsVisible(Instruction* scope_and_line,
+                                       uint32_t variable_id, uint32_t value_id,
+                                       Instruction* insert_pos);
 
   // Generates a DebugValue instruction with |dbg_local_var_id|, |value_id|,
   // |expr_id|, |index_id| operands and inserts it before |insert_before|.

diff --git a/source/opt/local_single_store_elim_pass.cpp b/source/opt/local_single_store_elim_pass.cpp
@@ -146,7 +146,7 @@ bool LocalSingleStoreElimPass::ProcessVariable(Instruction* var_inst) {
         context()->get_type_mgr()->GetType(var_inst->type_id());
     const analysis::Type* store_type = var_type->AsPointer()->pointee_type();
     if (!(store_type->AsStruct() || store_type->AsArray())) {
-      context()->get_debug_info_mgr()->AddDebugValue(
+      context()->get_debug_info_mgr()->AddDebugValueIfVarDeclIsVisible(
           store_inst, var_id, store_inst->GetSingleWordInOperand(1),
           store_inst);
       context()->get_debug_info_mgr()->KillDebugDeclares(var_id);

diff --git a/source/opt/ssa_rewrite_pass.cpp b/source/opt/ssa_rewrite_pass.cpp
@@ -307,8 +307,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()->AddDebugValue(inst, var_id, val_id,
-                                                          inst);
+    pass_->context()->get_debug_info_mgr()->AddDebugValueIfVarDeclIsVisible(
+        inst, var_id, val_id, inst);
 
 #if SSA_REWRITE_DEBUGGING_LEVEL > 1
     std::cerr << "\tFound store '%" << var_id << " = %" << val_id << "': "
@@ -491,7 +491,7 @@ bool SSARewriter::ApplyReplacements() {
 
     // Add DebugValue for the new OpPhi instruction.
     insert_it->SetDebugScope(local_var->GetDebugScope());
-    pass_->context()->get_debug_info_mgr()->AddDebugValue(
+    pass_->context()->get_debug_info_mgr()->AddDebugValueIfVarDeclIsVisible(
         &*insert_it, phi_candidate->var_id(), phi_candidate->result_id(),
         &*insert_it);
 

diff --git a/test/opt/ir_context_test.cpp b/test/opt/ir_context_test.cpp
@@ -1070,11 +1070,13 @@ OpFunctionEnd)";
 
   // No DebugValue should be added because result id '26' is not used for
   // DebugDeclare.
-  ctx->get_debug_info_mgr()->AddDebugValue(dbg_decl, 26, 22, dbg_decl);
+  ctx->get_debug_info_mgr()->AddDebugValueIfVarDeclIsVisible(dbg_decl, 26, 22,
+                                                             dbg_decl);
   EXPECT_EQ(dbg_decl->NextNode()->opcode(), SpvOpReturn);
 
   // DebugValue should be added because result id '20' is used for DebugDeclare.
-  ctx->get_debug_info_mgr()->AddDebugValue(dbg_decl, 20, 22, dbg_decl);
+  ctx->get_debug_info_mgr()->AddDebugValueIfVarDeclIsVisible(dbg_decl, 20, 22,
+                                                             dbg_decl);
   EXPECT_EQ(dbg_decl->NextNode()->GetOpenCL100DebugOpcode(),
             OpenCLDebugInfo100DebugValue);
 
@@ -1087,13 +1089,15 @@ OpFunctionEnd)";
 
   // No DebugValue should be added because result id '20' is not used for
   // DebugDeclare.
-  ctx->get_debug_info_mgr()->AddDebugValue(dbg_decl, 20, 7, dbg_decl);
+  ctx->get_debug_info_mgr()->AddDebugValueIfVarDeclIsVisible(dbg_decl, 20, 7,
+                                                             dbg_decl);
   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()->AddDebugValue(dbg_decl, 26, 7, dbg_decl);
+  ctx->get_debug_info_mgr()->AddDebugValueIfVarDeclIsVisible(dbg_decl, 26, 7,
+                                                             dbg_decl);
   dbg_value = dbg_decl->NextNode();
   EXPECT_EQ(dbg_value->GetOpenCL100DebugOpcode(), OpenCLDebugInfo100DebugValue);
   EXPECT_EQ(dbg_value->GetSingleWordOperand(kDebugValueOperandValueIndex), 7);