opt: Pass DebugDeclare scope to DebugValue (KhronosGroup#6178)

Author: SteveUrquhart

source/opt/debug_info_manager.cpp

@@ -558,11 +558,11 @@ bool DebugInfoManager::IsDeclareVisibleToInstr(Instruction* dbg_declare,
   return false;
 }
 
-bool DebugInfoManager::AddDebugValueForVariable(Instruction* scope_and_line,
+bool DebugInfoManager::AddDebugValueForVariable(Instruction* line,
                                                 uint32_t variable_id,
                                                 uint32_t value_id,
                                                 Instruction* insert_pos) {
-  assert(scope_and_line != nullptr);
+  assert(line != nullptr);
 
   auto dbg_decl_itr = var_id_to_dbg_decl_.find(variable_id);
   if (dbg_decl_itr == var_id_to_dbg_decl_.end()) return false;
@@ -577,14 +577,15 @@ bool DebugInfoManager::AddDebugValueForVariable(Instruction* scope_and_line,
       insert_before = insert_before->NextNode();
     }
     modified |= AddDebugValueForDecl(dbg_decl_or_val, value_id, insert_before,
-                                     scope_and_line) != nullptr;
+                                     line) != nullptr;
   }
   return modified;
 }
 
-Instruction* DebugInfoManager::AddDebugValueForDecl(
-    Instruction* dbg_decl, uint32_t value_id, Instruction* insert_before,
-    Instruction* scope_and_line) {
+Instruction* DebugInfoManager::AddDebugValueForDecl(Instruction* dbg_decl,
+                                                    uint32_t value_id,
+                                                    Instruction* insert_before,
+                                                    Instruction* line) {
   if (dbg_decl == nullptr || !IsDebugDeclare(dbg_decl)) return nullptr;
 
   std::unique_ptr<Instruction> dbg_val(dbg_decl->Clone(context()));
@@ -593,7 +594,7 @@ Instruction* DebugInfoManager::AddDebugValueForDecl(
   dbg_val->SetOperand(kDebugDeclareOperandVariableIndex, {value_id});
   dbg_val->SetOperand(kDebugValueOperandExpressionIndex,
                       {GetEmptyDebugExpression()->result_id()});
-  dbg_val->UpdateDebugInfoFrom(scope_and_line);
+  dbg_val->UpdateDebugInfoFrom(dbg_decl, line);
 
   auto* added_dbg_val = insert_before->InsertBefore(std::move(dbg_val));
   AnalyzeDebugInst(added_dbg_val);

source/opt/debug_info_manager.h

@@ -143,22 +143,21 @@ class DebugInfoManager {
   bool KillDebugDeclares(uint32_t variable_id);
 
   // 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|.
+  // variable that is in the scope of |line| and whose memory is |variable_id|
+  // and inserts it after the instruction |insert_pos|.
   // 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);
+  bool AddDebugValueForVariable(Instruction* 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
-  // |scope_and_line|, or no scope and line information if |scope_and_line|
-  // is nullptr. The new DebugValue has the same operands as DebugDeclare
-  // but it uses |value_id| for the value. Returns the created DebugValue,
+  // |insert_before|. The new DebugValue has the same line as |line} and the
+  // same scope as |dbg_decl|. The new DebugValue has the same operands as
+  // DebugDeclare but it uses |value_id| for the value. Returns the created
+  // DebugValue,
   // or nullptr if fails to create one.
   Instruction* AddDebugValueForDecl(Instruction* dbg_decl, uint32_t value_id,
                                     Instruction* insert_before,
-                                    Instruction* scope_and_line);
+                                    Instruction* line);
 
   // Erases |instr| from data structures of this class.
   void ClearDebugInfo(Instruction* instr);

source/opt/instruction.cpp

@@ -546,11 +546,13 @@ void Instruction::ClearDbgLineInsts() {
   clear_dbg_line_insts();
 }
 
-void Instruction::UpdateDebugInfoFrom(const Instruction* from) {
+void Instruction::UpdateDebugInfoFrom(const Instruction* from,
+                                      const Instruction* line) {
   if (from == nullptr) return;
   ClearDbgLineInsts();
-  if (!from->dbg_line_insts().empty())
-    AddDebugLine(&from->dbg_line_insts().back());
+  const Instruction* fromLine = line != nullptr ? line : from;
+  if (!fromLine->dbg_line_insts().empty())
+    AddDebugLine(&fromLine->dbg_line_insts().back());
   SetDebugScope(from->GetDebugScope());
   if (!IsLineInst() &&
       context()->AreAnalysesValid(IRContext::kAnalysisDebugInfo)) {

source/opt/instruction.h

@@ -338,7 +338,8 @@ class Instruction : public utils::IntrusiveNodeBase<Instruction> {
   // Updates lexical scope of DebugScope and OpLine.
   void UpdateLexicalScope(uint32_t scope);
   // Updates OpLine and DebugScope based on the information of |from|.
-  void UpdateDebugInfoFrom(const Instruction* from);
+  void UpdateDebugInfoFrom(const Instruction* from,
+                           const Instruction* line = nullptr);
   // Remove the |index|-th operand
   void RemoveOperand(uint32_t index) {
     operands_.erase(operands_.begin() + index);

source/opt/scalar_replacement_pass.cpp

@@ -186,7 +186,7 @@ bool ScalarReplacementPass::ReplaceWholeDebugDeclare(
     Instruction* added_dbg_value =
         context()->get_debug_info_mgr()->AddDebugValueForDecl(
             dbg_decl, /*value_id=*/var->result_id(),
-            /*insert_before=*/insert_before, /*scope_and_line=*/dbg_decl);
+            /*insert_before=*/insert_before, /*line=*/dbg_decl);
 
     if (added_dbg_value == nullptr) return false;
     added_dbg_value->AddOperand(

test/opt/debug_info_manager_test.cpp

@@ -767,8 +767,6 @@ void main(float in_var_color : COLOR) {
         %101 = OpExtInst %void %1 DebugScope %22
                OpLine %5 13 7
                OpStore %100 %31
-               OpNoLine
-        %102 = OpExtInst %void %1 DebugNoScope
          %36 = OpExtInst %void %1 DebugDeclare %25 %100 %13
                OpReturn
                OpFunctionEnd

test/opt/local_ssa_elim_test.cpp

@@ -2978,7 +2978,9 @@ TEST_F(LocalSSAElimTest, DebugValueForReferenceVariableInBB) {
 
 ; CHECK:      OpExtInst %void [[ext]] DebugScope [[dbg_main]]
 ; CHECK:      OpStore %f %float_0
+; CHECK-NEXT: OpExtInst %void [[ext]] DebugScope [[dbg_bb]]
 ; CHECK-NEXT: OpExtInst %void [[ext]] DebugValue [[dbg_x]] %float_0
+; CHECK-NEXT: OpExtInst %void [[ext]] DebugScope [[dbg_main]]
 ; CHECK-NEXT: OpExtInst %void [[ext]] DebugValue [[dbg_f]] %float_0
 ; CHECK-NEXT: OpStore %i %int_0
 ; CHECK-NEXT: OpExtInst %void [[ext]] DebugValue [[dbg_i]] %int_0
@@ -5434,6 +5436,7 @@ float4 main([[vk::location(0)]] float2 inUV : TEXCOORD0) : SV_TARGET
        %1614 = OpLabel
 ;CHECK:      %1614 = OpLabel
 ;CHECK-NEXT: [[phi:%\w+]] = OpPhi 
+;CHECK-NEXT: {{%\w+}} = OpExtInst %void {{%\w+}} DebugScope %179
 ;CHECK-NEXT: {{%\w+}} = OpExtInst %void {{%\w+}} DebugValue %233
        %2335 = OpExtInst %void %2 DebugScope %179
        %1795 = OpExtInst %void %2 DebugLine %64 %uint_149 %uint_149 %uint_16 %uint_16