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.
diff --git a/source/opt/debug_info_manager.cpp b/source/opt/debug_info_manager.cpp
index 4941d64..48285bd 100644
--- a/source/opt/debug_info_manager.cpp
+++ b/source/opt/debug_info_manager.cpp
@@ -40,7 +40,6 @@
 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 @@
   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<uint32_t> 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 @@
 
 void DebugInfoManager::AddDebugValueIfVarDeclIsVisible(
     Instruction* scope_and_line, uint32_t variable_id, uint32_t value_id,
-    Instruction* insert_pos) {
+    Instruction* insert_pos,
+    std::unordered_set<Instruction*>* 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 @@
                                    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<Instruction> 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 bf398b4..edb11b7 100644
--- a/source/opt/debug_info_manager.h
+++ b/source/opt/debug_info_manager.h
@@ -144,9 +144,11 @@
   // 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<Instruction*>* 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 @@
                                       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 @@
   // 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 9618261..1054a20 100644
--- a/source/opt/instruction.cpp
+++ b/source/opt/instruction.cpp
@@ -527,7 +527,7 @@
   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 c347974..f1b8177 100644
--- a/source/opt/local_single_store_elim_pass.cpp
+++ b/source/opt/local_single_store_elim_pass.cpp
@@ -149,8 +149,8 @@
     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 76f1781..5a56887 100644
--- a/source/opt/ssa_rewrite_pass.cpp
+++ b/source/opt/ssa_rewrite_pass.cpp
@@ -66,6 +66,7 @@
 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 @@
   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 @@
       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 @@
   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 @@
 
   // Add Phi instructions from completed Phi candidates.
   std::vector<Instruction*> generated_phis;
-  // Add DebugValue instructions for Phi instructions.
-  std::vector<Instruction*> 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 @@
     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 @@
   }
 }
 
+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 @@
   // 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 bbbfebb..1f4cd24 100644
--- a/source/opt/ssa_rewrite_pass.h
+++ b/source/opt/ssa_rewrite_pass.h
@@ -192,6 +192,10 @@
     }
   }
 
+  // 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 @@
   // 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 @@
 
   // 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 {
diff --git a/test/opt/ir_context_test.cpp b/test/opt/ir_context_test.cpp
index ff8be97..b6866d0 100644
--- a/test/opt/ir_context_test.cpp
+++ b/test/opt/ir_context_test.cpp
@@ -1136,12 +1136,12 @@
   // 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 @@
   // 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 f1b9821..ebc89a6 100644
--- a/test/opt/local_single_store_elim_test.cpp
+++ b/test/opt/local_single_store_elim_test.cpp
@@ -1211,80 +1211,6 @@
   SinglePassRunAndMatch<LocalSingleStoreElimPass>(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<LocalSingleStoreElimPass>(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 3d3c479..2ecd238 100644
--- a/test/opt/local_ssa_elim_test.cpp
+++ b/test/opt/local_ssa_elim_test.cpp
@@ -2165,6 +2165,212 @@
   SinglePassRunAndMatch<SSARewritePass>(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<SSARewritePass>(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 @@
 ; 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+]]