Reapply "[RemoveDIs] Print non-intrinsic debug info in textual IR output (#79281)"

This reapplication changes debug intrinsic declaration removal to only take
place when printing final IR, so that the processing format of the Module
does not affect the output.

This reverts commit d128448.

Author: SLTozer

llvm/include/llvm/IR/Module.h

@@ -218,6 +218,11 @@ class LLVM_EXTERNAL_VISIBILITY Module {
   /// \ref BasicBlock.
   bool IsNewDbgInfoFormat;
 
+  /// Used when printing this module in the new debug info format; removes all
+  /// declarations of debug intrinsics that are replaced by non-intrinsic
+  /// records in the new format.
+  void removeDebugIntrinsicDeclarations();
+
   /// \see BasicBlock::convertToNewDbgValues.
   void convertToNewDbgValues() {
     for (auto &F : *this) {
@@ -234,6 +239,13 @@ class LLVM_EXTERNAL_VISIBILITY Module {
     IsNewDbgInfoFormat = false;
   }
 
+  void setIsNewDbgInfoFormat(bool UseNewFormat) {
+    if (UseNewFormat && !IsNewDbgInfoFormat)
+      convertToNewDbgValues();
+    else if (!UseNewFormat && IsNewDbgInfoFormat)
+      convertFromNewDbgValues();
+  }
+
   /// The Module constructor. Note that there is no default constructor. You
   /// must provide a name for the module upon construction.
   explicit Module(StringRef ModuleID, LLVMContext& C);

llvm/include/llvm/IR/PrintPasses.h

@@ -78,6 +78,25 @@ std::string doSystemDiff(StringRef Before, StringRef After,
                          StringRef OldLineFormat, StringRef NewLineFormat,
                          StringRef UnchangedLineFormat);
 
+/// Used to temporarily set the debug info format of a function, module, or
+/// basic block for the duration of this object's lifetime, after which the
+/// prior state will be restored.
+template <typename T> class ScopedDbgInfoFormatSetter {
+  T &Obj;
+  bool OldState;
+
+public:
+  ScopedDbgInfoFormatSetter(T &Obj, bool NewState)
+      : Obj(Obj), OldState(Obj.IsNewDbgInfoFormat) {
+    Obj.setIsNewDbgInfoFormat(NewState);
+  }
+  ~ScopedDbgInfoFormatSetter() { Obj.setIsNewDbgInfoFormat(OldState); }
+};
+
+template <typename T>
+ScopedDbgInfoFormatSetter(T &Obj, bool NewState)
+    -> ScopedDbgInfoFormatSetter<T>;
+
 } // namespace llvm
 
 #endif // LLVM_IR_PRINTPASSES_H

llvm/lib/IR/AsmWriter.cpp

@@ -861,7 +861,7 @@ class SlotTracker : public AbstractSlotTrackerStorage {
   /// Add all of the metadata from an instruction.
   void processInstructionMetadata(const Instruction &I);
 
-  /// Add all of the metadata from an instruction.
+  /// Add all of the metadata from a DbgRecord.
   void processDbgRecordMetadata(const DbgRecord &DPV);
 };
 
@@ -1140,6 +1140,9 @@ void SlotTracker::processFunctionMetadata(const Function &F) {
 
 void SlotTracker::processDbgRecordMetadata(const DbgRecord &DR) {
   if (const DPValue *DPV = dyn_cast<const DPValue>(&DR)) {
+    // Process metadata used by DbgRecords; we only specifically care about the
+    // DILocalVariable, DILocation, and DIAssignID fields, as the Value and
+    // Expression fields should only be printed inline and so do not use a slot.
     CreateMetadataSlot(DPV->getVariable());
     if (DPV->isDbgAssign())
       CreateMetadataSlot(DPV->getAssignID());
@@ -2703,6 +2706,7 @@ class AssemblyWriter {
   void printDPValue(const DPValue &DPI);
   void printDPLabel(const DPLabel &DPL);
   void printDbgRecord(const DbgRecord &DPI);
+  void printDbgRecordLine(const DbgRecord &DPI);
 
   void printUseListOrder(const Value *V, const std::vector<unsigned> &Shuffle);
   void printUseLists(const Function *F);
@@ -3885,9 +3889,6 @@ void AssemblyWriter::printTypeIdentities() {
 
 /// printFunction - Print all aspects of a function.
 void AssemblyWriter::printFunction(const Function *F) {
-  bool ConvertBack = F->IsNewDbgInfoFormat;
-  if (ConvertBack)
-    const_cast<Function *>(F)->convertFromNewDbgValues();
   if (AnnotationWriter) AnnotationWriter->emitFunctionAnnot(F, Out);
 
   if (F->isMaterializable())
@@ -4030,8 +4031,6 @@ void AssemblyWriter::printFunction(const Function *F) {
     Out << "}\n";
   }
 
-  if (ConvertBack)
-    const_cast<Function *>(F)->convertToNewDbgValues();
   Machine.purgeFunction();
 }
 
@@ -4098,6 +4097,8 @@ void AssemblyWriter::printBasicBlock(const BasicBlock *BB) {
 
   // Output all of the instructions in the basic block...
   for (const Instruction &I : *BB) {
+    for (const DbgRecord &DR : I.getDbgValueRange())
+      printDbgRecordLine(DR);
     printInstructionLine(I);
   }
 
@@ -4611,12 +4612,10 @@ void AssemblyWriter::printDbgRecord(const DbgRecord &DR) {
     llvm_unreachable("Unexpected DbgRecord kind");
 }
 
-void AssemblyWriter::printDPValue(const DPValue &Value) {
-  // There's no formal representation of a DPValue -- print purely as a
-  // debugging aid.
-  Out << "  DPValue ";
-
-  switch (Value.getType()) {
+void AssemblyWriter::printDPValue(const DPValue &DPV) {
+  auto WriterCtx = getContext();
+  Out << "#dbg_";
+  switch (DPV.getType()) {
   case DPValue::LocationType::Value:
     Out << "value";
     break;
@@ -4629,35 +4628,39 @@ void AssemblyWriter::printDPValue(const DPValue &Value) {
   default:
     llvm_unreachable("Tried to print a DPValue with an invalid LocationType!");
   }
-  Out << " { ";
-  auto WriterCtx = getContext();
-  WriteAsOperandInternal(Out, Value.getRawLocation(), WriterCtx, true);
+  Out << "(";
+  WriteAsOperandInternal(Out, DPV.getRawLocation(), WriterCtx, true);
   Out << ", ";
-  WriteAsOperandInternal(Out, Value.getVariable(), WriterCtx, true);
+  WriteAsOperandInternal(Out, DPV.getVariable(), WriterCtx, true);
   Out << ", ";
-  WriteAsOperandInternal(Out, Value.getExpression(), WriterCtx, true);
+  WriteAsOperandInternal(Out, DPV.getExpression(), WriterCtx, true);
   Out << ", ";
-  if (Value.isDbgAssign()) {
-    WriteAsOperandInternal(Out, Value.getAssignID(), WriterCtx, true);
+  if (DPV.isDbgAssign()) {
+    WriteAsOperandInternal(Out, DPV.getAssignID(), WriterCtx, true);
     Out << ", ";
-    WriteAsOperandInternal(Out, Value.getRawAddress(), WriterCtx, true);
+    WriteAsOperandInternal(Out, DPV.getRawAddress(), WriterCtx, true);
     Out << ", ";
-    WriteAsOperandInternal(Out, Value.getAddressExpression(), WriterCtx, true);
+    WriteAsOperandInternal(Out, DPV.getAddressExpression(), WriterCtx, true);
     Out << ", ";
   }
-  WriteAsOperandInternal(Out, Value.getDebugLoc().get(), WriterCtx, true);
-  Out << " marker @" << Value.getMarker();
-  Out << " }";
+  WriteAsOperandInternal(Out, DPV.getDebugLoc().getAsMDNode(), WriterCtx, true);
+  Out << ")";
+}
+
+/// printDbgRecordLine - Print a DbgRecord with indentation and a newline
+/// character.
+void AssemblyWriter::printDbgRecordLine(const DbgRecord &DR) {
+  // Print lengthier indentation to bring out-of-line with instructions.
+  Out << "    ";
+  printDbgRecord(DR);
+  Out << '\n';
 }
 
 void AssemblyWriter::printDPLabel(const DPLabel &Label) {
-  // There's no formal representation of a DPLabel -- print purely as
-  // a debugging aid.
-  Out << "  DPLabel { ";
   auto WriterCtx = getContext();
+  Out << "#dbg_label(";
   WriteAsOperandInternal(Out, Label.getLabel(), WriterCtx, true);
-  Out << " marker @" << Label.getMarker();
-  Out << " }";
+  Out << ")";
 }
 
 void AssemblyWriter::printMetadataAttachments(
@@ -4805,19 +4808,11 @@ void BasicBlock::print(raw_ostream &ROS, AssemblyAnnotationWriter *AAW,
 
 void Module::print(raw_ostream &ROS, AssemblyAnnotationWriter *AAW,
                    bool ShouldPreserveUseListOrder, bool IsForDebug) const {
-  // RemoveDIs: always print with debug-info in intrinsic format.
-  bool ConvertAfter = IsNewDbgInfoFormat;
-  if (IsNewDbgInfoFormat)
-    const_cast<Module *>(this)->convertFromNewDbgValues();
-
   SlotTracker SlotTable(this);
   formatted_raw_ostream OS(ROS);
   AssemblyWriter W(OS, SlotTable, this, AAW, IsForDebug,
                    ShouldPreserveUseListOrder);
   W.printModule(this);
-
-  if (ConvertAfter)
-    const_cast<Module *>(this)->convertToNewDbgValues();
 }
 
 void NamedMDNode::print(raw_ostream &ROS, bool IsForDebug) const {
@@ -4908,8 +4903,6 @@ void DPValue::print(raw_ostream &ROS, bool IsForDebug) const {
 
 void DPMarker::print(raw_ostream &ROS, ModuleSlotTracker &MST,
                      bool IsForDebug) const {
-  // There's no formal representation of a DPMarker -- print purely as a
-  // debugging aid.
   formatted_raw_ostream OS(ROS);
   SlotTracker EmptySlotTable(static_cast<const Module *>(nullptr));
   SlotTracker &SlotTable =
@@ -4931,8 +4924,6 @@ void DPLabel::print(raw_ostream &ROS, bool IsForDebug) const {
 
 void DPValue::print(raw_ostream &ROS, ModuleSlotTracker &MST,
                     bool IsForDebug) const {
-  // There's no formal representation of a DPValue -- print purely as a
-  // debugging aid.
   formatted_raw_ostream OS(ROS);
   SlotTracker EmptySlotTable(static_cast<const Module *>(nullptr));
   SlotTracker &SlotTable =
@@ -4950,8 +4941,6 @@ void DPValue::print(raw_ostream &ROS, ModuleSlotTracker &MST,
 
 void DPLabel::print(raw_ostream &ROS, ModuleSlotTracker &MST,
                     bool IsForDebug) const {
-  // There's no formal representation of a DbgLabelRecord -- print purely as
-  // a debugging aid.
   formatted_raw_ostream OS(ROS);
   SlotTracker EmptySlotTable(static_cast<const Module *>(nullptr));
   SlotTracker &SlotTable =

llvm/lib/IR/BasicBlock.cpp

@@ -61,10 +61,6 @@ DPMarker *BasicBlock::createMarker(InstListType::iterator It) {
 }
 
 void BasicBlock::convertToNewDbgValues() {
-  // Is the command line option set?
-  if (!UseNewDbgInfoFormat)
-    return;
-
   IsNewDbgInfoFormat = true;
 
   // Iterate over all instructions in the instruction list, collecting dbg.value

llvm/lib/IR/IRPrintingPasses.cpp

@@ -23,6 +23,11 @@
 
 using namespace llvm;
 
+cl::opt<bool> WriteNewDbgInfoFormat(
+    "write-experimental-debuginfo",
+    cl::desc("Write debug info in the new non-intrinsic format"),
+    cl::init(false));
+
 namespace {
 
 class PrintModulePassWrapper : public ModulePass {
@@ -39,11 +44,14 @@ class PrintModulePassWrapper : public ModulePass {
         ShouldPreserveUseListOrder(ShouldPreserveUseListOrder) {}
 
   bool runOnModule(Module &M) override {
-    // RemoveDIs: there's no textual representation of the DPValue debug-info,
-    // convert to dbg.values before writing out.
-    bool IsNewDbgInfoFormat = M.IsNewDbgInfoFormat;
-    if (IsNewDbgInfoFormat)
-      M.convertFromNewDbgValues();
+    // RemoveDIs: Regardless of the format we've processed this module in, use
+    // `WriteNewDbgInfoFormat` to determine which format we use to write it.
+    ScopedDbgInfoFormatSetter FormatSetter(M, WriteNewDbgInfoFormat);
+    // Remove intrinsic declarations when printing in the new format.
+    // TODO: Move this into Module::setIsNewDbgInfoFormat when we're ready to
+    // update test output.
+    if (WriteNewDbgInfoFormat)
+      M.removeDebugIntrinsicDeclarations();
 
     if (llvm::isFunctionInPrintList("*")) {
       if (!Banner.empty())
@@ -62,9 +70,6 @@ class PrintModulePassWrapper : public ModulePass {
       }
     }
 
-    if (IsNewDbgInfoFormat)
-      M.convertToNewDbgValues();
-
     return false;
   }
 
@@ -87,11 +92,9 @@ class PrintFunctionPassWrapper : public FunctionPass {
 
   // This pass just prints a banner followed by the function as it's processed.
   bool runOnFunction(Function &F) override {
-    // RemoveDIs: there's no textual representation of the DPValue debug-info,
-    // convert to dbg.values before writing out.
-    bool IsNewDbgInfoFormat = F.IsNewDbgInfoFormat;
-    if (IsNewDbgInfoFormat)
-      F.convertFromNewDbgValues();
+    // RemoveDIs: Regardless of the format we've processed this function in, use
+    // `WriteNewDbgInfoFormat` to determine which format we use to write it.
+    ScopedDbgInfoFormatSetter FormatSetter(F, WriteNewDbgInfoFormat);
 
     if (isFunctionInPrintList(F.getName())) {
       if (forcePrintModuleIR())
@@ -101,9 +104,6 @@ class PrintFunctionPassWrapper : public FunctionPass {
         OS << Banner << '\n' << static_cast<Value &>(F);
     }
 
-    if (IsNewDbgInfoFormat)
-      F.convertToNewDbgValues();
-
     return false;
   }
 

llvm/lib/IR/Module.cpp

@@ -85,6 +85,28 @@ Module::~Module() {
   IFuncList.clear();
 }
 
+void Module::removeDebugIntrinsicDeclarations() {
+  auto *DeclareIntrinsicFn =
+      Intrinsic::getDeclaration(this, Intrinsic::dbg_declare);
+  assert((!isMaterialized() || DeclareIntrinsicFn->hasZeroLiveUses()) &&
+         "Debug declare intrinsic should have had uses removed.");
+  DeclareIntrinsicFn->eraseFromParent();
+  auto *ValueIntrinsicFn =
+      Intrinsic::getDeclaration(this, Intrinsic::dbg_value);
+  assert((!isMaterialized() || ValueIntrinsicFn->hasZeroLiveUses()) &&
+         "Debug value intrinsic should have had uses removed.");
+  ValueIntrinsicFn->eraseFromParent();
+  auto *AssignIntrinsicFn =
+      Intrinsic::getDeclaration(this, Intrinsic::dbg_assign);
+  assert((!isMaterialized() || AssignIntrinsicFn->hasZeroLiveUses()) &&
+         "Debug assign intrinsic should have had uses removed.");
+  AssignIntrinsicFn->eraseFromParent();
+  auto *LabelntrinsicFn = Intrinsic::getDeclaration(this, Intrinsic::dbg_label);
+  assert((!isMaterialized() || LabelntrinsicFn->hasZeroLiveUses()) &&
+         "Debug label intrinsic should have had uses removed.");
+  LabelntrinsicFn->eraseFromParent();
+}
+
 std::unique_ptr<RandomNumberGenerator>
 Module::createRNG(const StringRef Name) const {
   SmallString<32> Salt(Name);

llvm/lib/IRPrinter/IRPrintingPasses.cpp

@@ -22,6 +22,8 @@
 
 using namespace llvm;
 
+extern cl::opt<bool> WriteNewDbgInfoFormat;
+
 PrintModulePass::PrintModulePass() : OS(dbgs()) {}
 PrintModulePass::PrintModulePass(raw_ostream &OS, const std::string &Banner,
                                  bool ShouldPreserveUseListOrder,
@@ -31,11 +33,14 @@ PrintModulePass::PrintModulePass(raw_ostream &OS, const std::string &Banner,
       EmitSummaryIndex(EmitSummaryIndex) {}
 
 PreservedAnalyses PrintModulePass::run(Module &M, ModuleAnalysisManager &AM) {
-  // RemoveDIs: there's no textual representation of the DPValue debug-info,
-  // convert to dbg.values before writing out.
-  bool ShouldConvert = M.IsNewDbgInfoFormat;
-  if (ShouldConvert)
-    M.convertFromNewDbgValues();
+  // RemoveDIs: Regardless of the format we've processed this module in, use
+  // `WriteNewDbgInfoFormat` to determine which format we use to write it.
+  ScopedDbgInfoFormatSetter FormatSetter(M, WriteNewDbgInfoFormat);
+  // Remove intrinsic declarations when printing in the new format.
+  // TODO: Move this into Module::setIsNewDbgInfoFormat when we're ready to
+  // update test output.
+  if (WriteNewDbgInfoFormat)
+    M.removeDebugIntrinsicDeclarations();
 
   if (llvm::isFunctionInPrintList("*")) {
     if (!Banner.empty())
@@ -63,9 +68,6 @@ PreservedAnalyses PrintModulePass::run(Module &M, ModuleAnalysisManager &AM) {
     Index->print(OS);
   }
 
-  if (ShouldConvert)
-    M.convertToNewDbgValues();
-
   return PreservedAnalyses::all();
 }
 
@@ -75,11 +77,9 @@ PrintFunctionPass::PrintFunctionPass(raw_ostream &OS, const std::string &Banner)
 
 PreservedAnalyses PrintFunctionPass::run(Function &F,
                                          FunctionAnalysisManager &) {
-  // RemoveDIs: there's no textual representation of the DPValue debug-info,
-  // convert to dbg.values before writing out.
-  bool ShouldConvert = F.IsNewDbgInfoFormat;
-  if (ShouldConvert)
-    F.convertFromNewDbgValues();
+  // RemoveDIs: Regardless of the format we've processed this function in, use
+  // `WriteNewDbgInfoFormat` to determine which format we use to write it.
+  ScopedDbgInfoFormatSetter FormatSetter(F, WriteNewDbgInfoFormat);
 
   if (isFunctionInPrintList(F.getName())) {
     if (forcePrintModuleIR())
@@ -88,8 +88,5 @@ PreservedAnalyses PrintFunctionPass::run(Function &F,
       OS << Banner << '\n' << static_cast<Value &>(F);
   }
 
-  if (ShouldConvert)
-    F.convertToNewDbgValues();
-
   return PreservedAnalyses::all();
 }