[DebugInfo][RemoveDIs] Extend intrinsic-conversion in debugify (#80861)

jmorse · web-flow · commit a4531108da35 · 2024-02-06T18:30:20.000Z
A while back the entry/exit points of debugify were instrumented with
conversion functions to/from non-intrinsic-form debug-info. This is the
path of least resistance to incrementally converting parts of LLVM to
use the new format. However, it turns out that debugify registers
callbacks with the pass manager and can be fed non-intrinsic form
debug-info. Thus: this patch wraps each of the four major debugify
functions with the convertion utilities, and extends test coverage to a
test that exposes this problem.

(An alternative would be to put this code in the callback lambdas, but
then it would be fighting pass manager abstractions of what type the IR
has).

Handily debugify has been designed to record the /meaning/ of debug-info
rather than take pointers to intrinsics and the like, so the storage
mechanism for debug-info is transparent to it!
diff --git a/llvm/lib/Transforms/Utils/Debugify.cpp b/llvm/lib/Transforms/Utils/Debugify.cpp
@@ -87,6 +87,10 @@ bool llvm::applyDebugifyMetadata(
     return false;
   }
 
+  bool NewDebugMode = M.IsNewDbgInfoFormat;
+  if (NewDebugMode)
+    M.convertFromNewDbgValues();
+
   DIBuilder DIB(M);
   LLVMContext &Ctx = M.getContext();
   auto *Int32Ty = Type::getInt32Ty(Ctx);
@@ -210,6 +214,9 @@ bool llvm::applyDebugifyMetadata(
   if (!M.getModuleFlag(DIVersionKey))
     M.addModuleFlag(Module::Warning, DIVersionKey, DEBUG_METADATA_VERSION);
 
+  if (NewDebugMode)
+    M.convertToNewDbgValues();
+
   return true;
 }
 
@@ -304,6 +311,10 @@ bool llvm::collectDebugInfoMetadata(Module &M,
     return false;
   }
 
+  bool NewDebugMode = M.IsNewDbgInfoFormat;
+  if (NewDebugMode)
+    M.convertFromNewDbgValues();
+
   uint64_t FunctionsCnt = DebugInfoBeforePass.DIFunctions.size();
   // Visit each instruction.
   for (Function &F : Functions) {
@@ -368,6 +379,9 @@ bool llvm::collectDebugInfoMetadata(Module &M,
     }
   }
 
+  if (NewDebugMode)
+    M.convertToNewDbgValues();
+
   return true;
 }
 
@@ -547,6 +561,10 @@ bool llvm::checkDebugInfoMetadata(Module &M,
     return false;
   }
 
+  bool NewDebugMode = M.IsNewDbgInfoFormat;
+  if (NewDebugMode)
+    M.convertFromNewDbgValues();
+
   // Map the debug info holding DIs after a pass.
   DebugInfoPerPass DebugInfoAfterPass;
 
@@ -657,6 +675,9 @@ bool llvm::checkDebugInfoMetadata(Module &M,
   // the debugging information from the previous pass.
   DebugInfoBeforePass = DebugInfoAfterPass;
 
+  if (NewDebugMode)
+    M.convertToNewDbgValues();
+
   LLVM_DEBUG(dbgs() << "\n\n");
   return Result;
 }
@@ -714,6 +735,10 @@ bool checkDebugifyMetadata(Module &M,
     return false;
   }
 
+  bool NewDebugMode = M.IsNewDbgInfoFormat;
+  if (NewDebugMode)
+    M.convertFromNewDbgValues();
+
   auto getDebugifyOperand = [&](unsigned Idx) -> unsigned {
     return mdconst::extract<ConstantInt>(NMD->getOperand(Idx)->getOperand(0))
         ->getZExtValue();
@@ -791,24 +816,22 @@ bool checkDebugifyMetadata(Module &M,
   dbg() << ": " << (HasErrors ? "FAIL" : "PASS") << '\n';
 
   // Strip debugify metadata if required.
+  bool Ret = false;
   if (Strip)
-    return stripDebugifyMetadata(M);
+    Ret = stripDebugifyMetadata(M);
+
+  if (NewDebugMode)
+    M.convertToNewDbgValues();
 
-  return false;
+  return Ret;
 }
 
 /// ModulePass for attaching synthetic debug info to everything, used with the
 /// legacy module pass manager.
 struct DebugifyModulePass : public ModulePass {
   bool runOnModule(Module &M) override {
-    bool NewDebugMode = M.IsNewDbgInfoFormat;
-    if (NewDebugMode)
-      M.convertFromNewDbgValues();
-
-    bool Result = applyDebugify(M, Mode, DebugInfoBeforePass, NameOfWrappedPass);
-
-    if (NewDebugMode)
-      M.convertToNewDbgValues();
+    bool Result =
+        applyDebugify(M, Mode, DebugInfoBeforePass, NameOfWrappedPass);
     return Result;
   }
 
@@ -834,14 +857,8 @@ struct DebugifyModulePass : public ModulePass {
 /// single function, used with the legacy module pass manager.
 struct DebugifyFunctionPass : public FunctionPass {
   bool runOnFunction(Function &F) override {
-    bool NewDebugMode = F.IsNewDbgInfoFormat;
-    if (NewDebugMode)
-      F.convertFromNewDbgValues();
-
-    bool Result = applyDebugify(F, Mode, DebugInfoBeforePass, NameOfWrappedPass);
-
-    if (NewDebugMode)
-      F.convertToNewDbgValues();
+    bool Result =
+        applyDebugify(F, Mode, DebugInfoBeforePass, NameOfWrappedPass);
     return Result;
   }
 
@@ -868,10 +885,6 @@ struct DebugifyFunctionPass : public FunctionPass {
 /// legacy module pass manager.
 struct CheckDebugifyModulePass : public ModulePass {
   bool runOnModule(Module &M) override {
-    bool NewDebugMode = M.IsNewDbgInfoFormat;
-    if (NewDebugMode)
-      M.convertFromNewDbgValues();
-
     bool Result;
     if (Mode == DebugifyMode::SyntheticDebugInfo)
       Result = checkDebugifyMetadata(M, M.functions(), NameOfWrappedPass,
@@ -882,9 +895,6 @@ struct CheckDebugifyModulePass : public ModulePass {
         "CheckModuleDebugify (original debuginfo)", NameOfWrappedPass,
         OrigDIVerifyBugsReportFilePath);
 
-    if (NewDebugMode)
-      M.convertToNewDbgValues();
-
     return Result;
   }
 
@@ -918,10 +928,6 @@ struct CheckDebugifyModulePass : public ModulePass {
 /// with the legacy module pass manager.
 struct CheckDebugifyFunctionPass : public FunctionPass {
   bool runOnFunction(Function &F) override {
-    bool NewDebugMode = F.IsNewDbgInfoFormat;
-    if (NewDebugMode)
-      F.convertFromNewDbgValues();
-
     Module &M = *F.getParent();
     auto FuncIt = F.getIterator();
     bool Result;
@@ -935,8 +941,6 @@ struct CheckDebugifyFunctionPass : public FunctionPass {
         "CheckFunctionDebugify (original debuginfo)", NameOfWrappedPass,
         OrigDIVerifyBugsReportFilePath);
 
-    if (NewDebugMode)
-      F.convertToNewDbgValues();
     return Result;
   }
 
@@ -1009,10 +1013,6 @@ createDebugifyFunctionPass(enum DebugifyMode Mode,
 }
 
 PreservedAnalyses NewPMDebugifyPass::run(Module &M, ModuleAnalysisManager &) {
-  bool NewDebugMode = M.IsNewDbgInfoFormat;
-  if (NewDebugMode)
-    M.convertFromNewDbgValues();
-
   if (Mode == DebugifyMode::SyntheticDebugInfo)
     applyDebugifyMetadata(M, M.functions(),
                           "ModuleDebugify: ", /*ApplyToMF*/ nullptr);
@@ -1021,9 +1021,6 @@ PreservedAnalyses NewPMDebugifyPass::run(Module &M, ModuleAnalysisManager &) {
                              "ModuleDebugify (original debuginfo)",
                               NameOfWrappedPass);
 
-  if (NewDebugMode)
-      M.convertToNewDbgValues();
-
   PreservedAnalyses PA;
   PA.preserveSet<CFGAnalyses>();
   return PA;
diff --git a/llvm/test/Transforms/Util/Debugify/loc-only-original-mode.ll b/llvm/test/Transforms/Util/Debugify/loc-only-original-mode.ll
@@ -14,6 +14,11 @@
 ; RUN:     -verify-each-debuginfo-preserve \
 ; RUN:     -debugify-func-limit=2 -S 2>&1 | FileCheck %s --check-prefix=CHECK-DROP
 
+;; Add some runlines that use RemoveDIs non-intrinsic debug-info, to check that
+;; variable preservation checking works.
+; RUN: opt < %s -passes=deadargelim --try-experimental-debuginfo-iterators \
+; RUN:     -verify-each-debuginfo-preserve \
+; RUN:     -debugify-level=location+variables -S 2>&1 | FileCheck %s --check-prefix=CHECK-DROP
 
 ; CHECK-NOT: drops dbg.value()/dbg.declare()
 ; CHECK-DROP: drops dbg.value()/dbg.declare()
@@ -22,21 +27,21 @@ target triple = "x86_64-unknown-linux-gnu"
 
 define dso_local i32 @fn2(i32 %l, i32 %k) !dbg !7 {
 entry:
-  call void @llvm.dbg.value(metadata i32 %l, metadata !12, metadata !DIExpression()), !dbg !15
-  call void @llvm.dbg.value(metadata i32 %k, metadata !13, metadata !DIExpression()), !dbg !15
+  tail call void @llvm.dbg.value(metadata i32 %l, metadata !12, metadata !DIExpression()), !dbg !15
+  tail call void @llvm.dbg.value(metadata i32 %k, metadata !13, metadata !DIExpression()), !dbg !15
   %call = call i32 (...) @fn3(), !dbg !16
-  call void @llvm.dbg.value(metadata i32 %call, metadata !14, metadata !DIExpression()), !dbg !15
+  tail call void @llvm.dbg.value(metadata i32 %call, metadata !14, metadata !DIExpression()), !dbg !15
   ret i32 %call, !dbg !17
 }
 
 declare !dbg !18 dso_local i32 @fn3(...)
 
 define dso_local i32 @fn(i32 %x, i32 %y) !dbg !22 {
 entry:
-  call void @llvm.dbg.value(metadata i32 %x, metadata !24, metadata !DIExpression()), !dbg !27
-  call void @llvm.dbg.value(metadata i32 %y, metadata !25, metadata !DIExpression()), !dbg !27
+  tail call void @llvm.dbg.value(metadata i32 %x, metadata !24, metadata !DIExpression()), !dbg !27
+  tail call void @llvm.dbg.value(metadata i32 %y, metadata !25, metadata !DIExpression()), !dbg !27
   %call = call i32 @fn2(i32 %x, i32 %y), !dbg !27
-  call void @llvm.dbg.value(metadata i32 %call, metadata !26, metadata !DIExpression()), !dbg !27
+  tail call void @llvm.dbg.value(metadata i32 %call, metadata !26, metadata !DIExpression()), !dbg !27
   %add = add nsw i32 %call, %x, !dbg !27
   %add1 = add nsw i32 %add, %y, !dbg !27
   ret i32 %add1, !dbg !27
