[Debugify] Strip added metadata in the -debugify-each pipeline

Summary:
Share logic to strip debugify metadata between the IR and MIR level
debugify passes. This makes it simpler to hunt for bugs by diffing IR
with vs. without -debugify-each turned on.

As a drive-by, fix an issue causing CallGraphNodes to become invalid
when a dead llvm.dbg.value prototype is deleted.

Reviewers: dsanders, aprantl

Subscribers: hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D77915

llvm/include/llvm/IR/IntrinsicInst.h

@@ -63,20 +63,26 @@ namespace llvm {
     }
   };
 
+  /// Check if \p ID corresponds to a debug info intrinsic.
+  static inline bool isDbgInfoIntrinsic(Intrinsic::ID ID) {
+    switch (ID) {
+    case Intrinsic::dbg_declare:
+    case Intrinsic::dbg_value:
+    case Intrinsic::dbg_addr:
+    case Intrinsic::dbg_label:
+      return true;
+    default:
+      return false;
+    }
+  }
+
   /// This is the common base class for debug info intrinsics.
   class DbgInfoIntrinsic : public IntrinsicInst {
   public:
     /// \name Casting methods
     /// @{
     static bool classof(const IntrinsicInst *I) {
-      switch (I->getIntrinsicID()) {
-      case Intrinsic::dbg_declare:
-      case Intrinsic::dbg_value:
-      case Intrinsic::dbg_addr:
-      case Intrinsic::dbg_label:
-        return true;
-      default: return false;
-      }
+      return isDbgInfoIntrinsic(I->getIntrinsicID());
     }
     static bool classof(const Value *V) {
       return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));

llvm/include/llvm/Transforms/Utils/Debugify.h

@@ -31,6 +31,12 @@ class DIBuilder;
 bool applyDebugifyMetadata(
     Module &M, iterator_range<Module::iterator> Functions, StringRef Banner,
     std::function<bool(DIBuilder &, Function &)> ApplyToMF);
+
+/// Strip out all of the metadata and debug info inserted by debugify. If no
+/// llvm.debugify module-level named metadata is present, this is a no-op.
+/// Returns true if any change was made.
+bool stripDebugifyMetadata(Module &M);
+
 } // namespace llvm
 
 llvm::ModulePass *createDebugifyModulePass();

llvm/lib/Analysis/CallGraph.cpp

@@ -11,6 +11,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/IR/Function.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/PassManager.h"
@@ -31,9 +32,10 @@ using namespace llvm;
 CallGraph::CallGraph(Module &M)
     : M(M), ExternalCallingNode(getOrInsertFunction(nullptr)),
       CallsExternalNode(std::make_unique<CallGraphNode>(nullptr)) {
-  // Add every function to the call graph.
+  // Add every interesting function to the call graph.
   for (Function &F : M)
-    addToCallGraph(&F);
+    if (!isDbgInfoIntrinsic(F.getIntrinsicID()))
+      addToCallGraph(&F);
 }
 
 CallGraph::CallGraph(CallGraph &&Arg)

llvm/lib/CodeGen/MachineStripDebug.cpp

@@ -16,6 +16,7 @@
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Transforms/Utils/Debugify.h"
 
 #define DEBUG_TYPE "mir-strip-debug"
 
@@ -73,33 +74,7 @@ struct StripDebugMachineModule : public ModulePass {
       }
     }
 
-    Changed |= StripDebugInfo(M);
-
-    NamedMDNode *NMD = M.getNamedMetadata("llvm.debugify");
-    if (NMD) {
-      NMD->eraseFromParent();
-      Changed |= true;
-    }
-
-    NMD = M.getModuleFlagsMetadata();
-    if (NMD) {
-      // There must be an easier way to remove an operand from a NamedMDNode.
-      SmallVector<MDNode *, 4> Flags;
-      for (MDNode *Flag : NMD->operands())
-        Flags.push_back(Flag);
-      NMD->clearOperands();
-      for (MDNode *Flag : Flags) {
-        MDString *Key = dyn_cast_or_null<MDString>(Flag->getOperand(1));
-        if (Key->getString() == "Debug Info Version") {
-          Changed |= true;
-          continue;
-        }
-        NMD->addOperand(Flag);
-      }
-      // If we left it empty we might as well remove it.
-      if (NMD->getNumOperands() == 0)
-        NMD->eraseFromParent();
-    }
+    Changed |= stripDebugifyMetadata(M);
 
     return Changed;
   }

llvm/lib/Transforms/Utils/Debugify.cpp

@@ -176,6 +176,52 @@ bool llvm::applyDebugifyMetadata(
   return true;
 }
 
+bool llvm::stripDebugifyMetadata(Module &M) {
+  bool Changed = false;
+
+  // Remove the llvm.debugify module-level named metadata.
+  NamedMDNode *DebugifyMD = M.getNamedMetadata("llvm.debugify");
+  if (DebugifyMD) {
+    M.eraseNamedMetadata(DebugifyMD);
+    Changed = true;
+  }
+
+  // Strip out all debug intrinsics and supporting metadata (subprograms, types,
+  // variables, etc).
+  Changed |= StripDebugInfo(M);
+
+  // Strip out the dead dbg.value prototype.
+  Function *DbgValF = M.getFunction("llvm.dbg.value");
+  if (DbgValF) {
+    assert(DbgValF->isDeclaration() && DbgValF->use_empty() &&
+           "Not all debug info stripped?");
+    DbgValF->eraseFromParent();
+    Changed = true;
+  }
+
+  // Strip out the module-level Debug Info Version metadata.
+  // FIXME: There must be an easier way to remove an operand from a NamedMDNode.
+  NamedMDNode *NMD = M.getModuleFlagsMetadata();
+  assert(NMD && "debugify metadata present without Debug Info Version set?");
+  SmallVector<MDNode *, 4> Flags;
+  for (MDNode *Flag : NMD->operands())
+    Flags.push_back(Flag);
+  NMD->clearOperands();
+  for (MDNode *Flag : Flags) {
+    MDString *Key = dyn_cast_or_null<MDString>(Flag->getOperand(1));
+    if (Key->getString() == "Debug Info Version") {
+      Changed = true;
+      continue;
+    }
+    NMD->addOperand(Flag);
+  }
+  // If we left it empty we might as well remove it.
+  if (NMD->getNumOperands() == 0)
+    NMD->eraseFromParent();
+
+  return Changed;
+}
+
 namespace {
 /// Return true if a mis-sized diagnostic is issued for \p DVI.
 bool diagnoseMisSizedDbgValue(Module &M, DbgValueInst *DVI) {
@@ -305,12 +351,9 @@ bool checkDebugifyMetadata(Module &M,
     dbg() << " [" << NameOfWrappedPass << "]";
   dbg() << ": " << (HasErrors ? "FAIL" : "PASS") << '\n';
 
-  // Strip the Debugify Metadata if required.
-  if (Strip) {
-    StripDebugInfo(M);
-    M.eraseNamedMetadata(NMD);
-    return true;
-  }
+  // Strip debugify metadata if required.
+  if (Strip)
+    return stripDebugifyMetadata(M);
 
   return false;
 }

llvm/test/DebugInfo/debugify-each.ll

@@ -18,19 +18,13 @@
 
 ; Check that stripped textual IR compares equal before and after applying
 ; debugify.
-; RUN: opt -O1 < %s -S -o - | \
-; RUN:   opt -strip -strip-dead-prototypes -strip-named-metadata -S -o %t.before
-; RUN: opt -O1 -debugify-each < %s -S -o - | \
-; RUN:   opt -strip -strip-dead-prototypes -strip-named-metadata -S -o %t.after
+; RUN: opt -O1 < %s -S -o %t.before
+; RUN: opt -O1 -debugify-each < %s -S -o %t.after
 ; RUN: diff %t.before %t.after
 
 ; Check that stripped IR compares equal before and after applying debugify.
-; RUN: opt -O1 < %s | \
-; RUN:   opt -strip -strip-dead-prototypes -strip-named-metadata | \
-; RUN:   llvm-dis -o %t.before
-; RUN: opt -O1 -debugify-each < %s | \
-; RUN:   opt -strip -strip-dead-prototypes -strip-named-metadata | \
-; RUN:   llvm-dis -o %t.after
+; RUN: opt -O1 < %s | llvm-dis -o %t.before
+; RUN: opt -O1 -debugify-each < %s | llvm-dis -o %t.after
 ; RUN: diff %t.before %t.after
 
 define void @foo(i32 %arg) {