Add removing references to debug instructions when removing them (#2923)

Fixes #2921

source/opt/ir_context.h

@@ -187,9 +187,6 @@ class IRContext {
   inline IteratorRange<Module::inst_iterator> debugs3();
   inline IteratorRange<Module::const_inst_iterator> debugs3() const;
 
-  // Clears all debug instructions (excluding OpLine & OpNoLine).
-  inline void debug_clear();
-
   // Add |capability| to the module, if it is not already enabled.
   inline void AddCapability(SpvCapability capability);
 
@@ -928,8 +925,6 @@ IteratorRange<Module::const_inst_iterator> IRContext::debugs3() const {
   return ((const Module*)module_.get())->debugs3();
 }
 
-void IRContext::debug_clear() { module_->debug_clear(); }
-
 void IRContext::AddCapability(SpvCapability capability) {
   if (!get_feature_mgr()->HasCapability(capability)) {
     std::unique_ptr<Instruction> capability_inst(new Instruction(

source/opt/module.h

@@ -192,22 +192,6 @@ class Module {
   inline IteratorRange<inst_iterator> execution_modes();
   inline IteratorRange<const_inst_iterator> execution_modes() const;
 
-  // Clears all debug instructions (excluding OpLine & OpNoLine).
-  void debug_clear() {
-    debug1_clear();
-    debug2_clear();
-    debug3_clear();
-  }
-
-  // Clears all debug 1 instructions (excluding OpLine & OpNoLine).
-  void debug1_clear() { debugs1_.clear(); }
-
-  // Clears all debug 2 instructions (excluding OpLine & OpNoLine).
-  void debug2_clear() { debugs2_.clear(); }
-
-  // Clears all debug 3 instructions (excluding OpLine & OpNoLine).
-  void debug3_clear() { debugs3_.clear(); }
-
   // Iterators for annotation instructions contained in this module.
   inline inst_iterator annotation_begin();
   inline inst_iterator annotation_end();

source/opt/strip_debug_info_pass.cpp

@@ -22,7 +22,23 @@ Pass::Status StripDebugInfoPass::Process() {
   bool modified = !context()->debugs1().empty() ||
                   !context()->debugs2().empty() ||
                   !context()->debugs3().empty();
-  context()->debug_clear();
+
+  std::vector<Instruction*> to_kill;
+  for (auto& dbg : context()->debugs1()) to_kill.push_back(&dbg);
+  for (auto& dbg : context()->debugs2()) to_kill.push_back(&dbg);
+  for (auto& dbg : context()->debugs3()) to_kill.push_back(&dbg);
+
+  // OpName must come first, since they may refer to other debug instructions.
+  // If they are after the instructions that refer to, then they will be killed
+  // when that instruction is killed, which will lead to a double kill.
+  std::sort(to_kill.begin(), to_kill.end(),
+            [](Instruction* lhs, Instruction* rhs) -> bool {
+              if (lhs->opcode() == SpvOpName && rhs->opcode() != SpvOpName)
+                return true;
+              return false;
+            });
+
+  for (auto* inst : to_kill) context()->KillInst(inst);
 
   context()->module()->ForEachInst([&modified](Instruction* inst) {
     modified |= !inst->dbg_line_insts().empty();

test/opt/strip_debug_info_test.cpp

@@ -74,6 +74,82 @@ TEST_F(StripLineDebugInfoTest, LineNoLine) {
                                             /* skip_nop = */ false);
 }
 
+using StripDebugStringTest = PassTest<::testing::Test>;
+
+TEST_F(StripDebugStringTest, OpDecorateRemoved) {
+  std::vector<const char*> input{
+      // clang-format off
+                     "OpCapability Shader",
+                "%1 = OpExtInstImport \"GLSL.std.450\"",
+                     "OpMemoryModel Logical GLSL450",
+                     "OpEntryPoint Vertex %2 \"main\"",
+                "%3 = OpString \"minimal.vert\"",
+                     "OpDecorate %3 Location 1337",
+             "%void = OpTypeVoid",
+                "%5 = OpTypeFunction %void",
+                "%2 = OpFunction %void None %5",
+                "%6 = OpLabel",
+                     "OpReturn",
+                     "OpFunctionEnd",
+      // clang-format on
+  };
+  std::vector<const char*> output{
+      // clang-format off
+                     "OpCapability Shader",
+                "%1 = OpExtInstImport \"GLSL.std.450\"",
+                     "OpMemoryModel Logical GLSL450",
+                     "OpEntryPoint Vertex %2 \"main\"",
+             "%void = OpTypeVoid",
+                "%5 = OpTypeFunction %void",
+                "%2 = OpFunction %void None %5",
+                "%6 = OpLabel",
+                     "OpReturn",
+                     "OpFunctionEnd",
+      // clang-format on
+  };
+  SinglePassRunAndCheck<StripDebugInfoPass>(JoinAllInsts(input),
+                                            JoinAllInsts(output),
+                                            /* skip_nop = */ false,
+                                            /* do_validation */ true);
+}
+
+TEST_F(StripDebugStringTest, OpNameRemoved) {
+  std::vector<const char*> input{
+      // clang-format off
+                     "OpCapability Shader",
+                "%1 = OpExtInstImport \"GLSL.std.450\"",
+                     "OpMemoryModel Logical GLSL450",
+                     "OpEntryPoint Vertex %2 \"main\"",
+                "%3 = OpString \"minimal.vert\"",
+                     "OpName %3 \"bob\"",
+             "%void = OpTypeVoid",
+                "%5 = OpTypeFunction %void",
+                "%2 = OpFunction %void None %5",
+                "%6 = OpLabel",
+                     "OpReturn",
+                     "OpFunctionEnd",
+      // clang-format on
+  };
+  std::vector<const char*> output{
+      // clang-format off
+                     "OpCapability Shader",
+                "%1 = OpExtInstImport \"GLSL.std.450\"",
+                     "OpMemoryModel Logical GLSL450",
+                     "OpEntryPoint Vertex %2 \"main\"",
+             "%void = OpTypeVoid",
+                "%5 = OpTypeFunction %void",
+                "%2 = OpFunction %void None %5",
+                "%6 = OpLabel",
+                     "OpReturn",
+                     "OpFunctionEnd",
+      // clang-format on
+  };
+  SinglePassRunAndCheck<StripDebugInfoPass>(JoinAllInsts(input),
+                                            JoinAllInsts(output),
+                                            /* skip_nop = */ false,
+                                            /* do_validation */ true);
+}
+
 using StripDebugInfoTest = PassTest<::testing::TestWithParam<const char*>>;
 
 TEST_P(StripDebugInfoTest, Kind) {