[Debugify] Introduce debugify-each and DebugifyFunctionPass

[tyb0807]

Summary

Hi,

As discussed in this thread on the mailing list http://lists.llvm.org/pipermail/llvm-dev/2018-April/122785.html, here's my attempt to implement a debugify-each mode and to create a DebugifyFunctionPass.

If you are ok with this approach, I will implement a DebugifyBasicBlockPass, a DebugifyLoopPass and a DebugifyRegionPass likewise.

Another TODO is to make debugify-each as well as Debugify passes other than DebugifyModulePass work with the new Pass Manager, but this is not necessary for the GSoC (-O1/2/3/s/z optimization pipelines are handled by the LegacyPassManager). But I am willing to tackle it if you do not have anything against.

This is my first major patch, so please forgive me if I've done anything wrong...

Thank you all for your help

[vsk]

Thanks for the patch. I have some comments inline.

Let's land debugify-each support for module and function passes before tackling other types of passes.

test/DebugInfo/debugify.ll

31

Could you move the tests for -debugify-each to a new file? This one is getting hard to follow.

31

In the new test for -debugify-each, we should test opt -O1 -debugify-each and check that at least one module pass and function pass are instrumented & checked.

tools/opt/Debugify.cpp

65

We should assert that the named metadata here has exactly 2 operands, as it's not immediately clear from the way the debugify function pass is invoked that this is the case.

191

These bitvectors should be passed as references to avoid copies ('const BitVector &').

207

Why is the explicit cast needed?

279

The extra parens are not needed here -- you can just write 'ShouldApply = !...'

312

You can move this to the initialization list as 'Strip(Strip)'.

357

Ditto.

tools/opt/opt.cpp

264

How about a more general name, like 'OptCustomPassManager'?

268

The DebugifyEach flag doesn't need to be passed in here.

282

Hm, this misses the 'Print Function IR' pass. There should be a less brittle way to do this. Could you add a 'bool isIRPrintingPass(Pass *P)' function to IRPrintingPasses.h? You can inspect the address of the pass ID to implement the predicate.

287

Please use fall through to fold bodies for the identical cases together.

[tyb0807]

Hello Vedant,

Thanks for your comments. I've updated the patch accordingly.

[vsk]

Thanks! At a high level this is looking good. I have more comments inline which should help with code reuse. Currently there is some extra duplicated logic for function passes, and this should be consolidated for easier maintenance.

tools/opt/Debugify.cpp

207

Why is this RAUW needed at all? Should StripDebugInfo handle this for you?

240

You shouldn't need to duplicate any logic for setting up these BitVectors, and you shouldn't need a separate printCheckDebugifyResults() function.

Please reuse the same logic for checking debugify metadata in module and function passes. E.g:

void checkDebugifyMetadata(Module &M, iterator_range<Module::iterator> Functions) {
  // Get the number of lines / vars from the module's named metadata, set up the bit vectors, etc.
  for (Function &F : Functions) {
    // Check for missing lines / vars.
  }
  // Print the results.
}

// In the FunctionPass helper:
checkDebugifyMetadata(M, make_range(F.getIterator(), std::next(F.getIterator()))); //< Just check "F".

// In the ModulePass helper:
checkDebugifyMetadata(M, M.functions());

If in the future we add a debugify wrapper for BasicBlock passes, we can simply add a 'BasicBlock *' parameter to 'checkDebugifyMetadata'.

301

Per my previous comments, I'd suggest making two changes here:

• Move all of the logic to a helper to make a future port to the new PM easier.
• Make applyDebugifyMetadata accept a range of functions to visit (iterator_range<Module::iterator>), so there is only one call to createCompileUnit, etc. in this file.

320

I don't think we should compute and store ShouldApply in the doInitialization() step, because it happens just once per pass. It should be done in the applyDebugifyMetadata function.

322

Please move all of the actual logic here into a helper function, to simplify a future port to the new pass manager.

tools/opt/PassPrinters.h

67

I think this is dead code.

tools/opt/opt.cpp

264

You can use an idiom here to reduce repetition and simplify the code:

using super = legacy::PassManager;

269

Please use an early-return for the cases (outside of the switch) where we don't insert extra passes. E.g:

bool WrapWithDebugify = DebugifyEach && !P->getAsImmutablePass() && !isIRPrintingPass(P);
if (!WrapWithDebugify) {
  super::add(P);
  return;
}

284

We can safely skip RegionPass and CGSCCPass, as they are just used for structurize-cfg, the old inliner, and 2-3 other passes. I don't think it's worth adding the complexity to support this, so we can leave it out of the TODO.

637–638

This conditional shouldn't be written out twice. Could you introduce 'bool AddOneTimeDebugifyPasses = EnableDebugify && !DebugifyEach'?

[vsk]

One last point: to aid debugging, it would help if you could dump the Module / Function IR when the check-debugify step fails. Currently, if a pass fails checking, opt simply moves on without printing any helpful resources.

[tyb0807]

Address review comments. Now we print out the module when check-debugify reports an error. I guess another important information should be the optimization pass that makes check-debugify fail, but I'm not sure how to get that information though.

P/S: Thanks Vedant, the code looks a lot more elegant now!

[vsk]

Thanks for working on this @tyb0807, it looks really close :). There are just a few more minor things I'd like addressed.

test/DebugInfo/debugify-each.ll

1 ↗

(On Diff #146298)

At a high level, we don't want to test the debugify logic in this file, because we already have a test for that. Instead, we want to check that -debugify-each is running after each pass.

Let's minimize this test:

• Please remove the checks for what the inserted debug info metadata looks like.
• We just need two functions, e.g "foo" and "bar", which both only contain "ret void".
• Add RUN lines for: "-debugify-each -O{1,2,3}", and "-enable-debugify -debugify-each -sroa -sccp". All of these separate test runs should share the exact same FileCheck "CHECK" commands. Specifically, we expect the module & function debugify passes to run at least twice. So you could write:

; Verify that the module & function (check-)debugify passes run at least twice.

; CHECK: CheckModuleDebugify: PASS
; CHECK: CheckFunctionDebugify: PASS
; CHECK: CheckFunctionDebugify: PASS

; CHECK: CheckModuleDebugify: PASS
; CHECK: CheckFunctionDebugify: PASS
; CHECK: CheckFunctionDebugify: PASS

These should be the only CHECK lines you need.

tools/opt/Debugify.cpp

44

Could you use a StringRef here to avoid copying the std::string?

129–130

Could you run clang-format? There are some whitespace-only changes which complicate the diff.

148

Ditto, please use StringRef.

159–160

We should only need special handling for "line 0", as that refers to a compiler-generated "ambiguous" location. I.e, if (DL && DL.getLine() != 0) { reset(...) }.

We don't need need any other range checks, because an assertion in BitVector should catch out-of-bounds accesses. This means we can omit the "if (DL)" which follows.

180–181

As this change doesn't change any behavior, let's keep the previous version to minimize the diff.

248

I still don't understand why we need to erase the declaration of llvm.dbg.value() from the module. Shouldn't StripDebugInfo() do this for us? If not, we should just fix StripDebugInfo() in a follow-up patch.

259

Please replace "to everything" with "to instructions within a single function".

262

Just a nit, but this could be easier to read if it's split up into logically smaller pieces, e.g:

Module &M = *F.getParent();
auto FuncIt = F.getIterator();
return apply...(M, make_range(FuncIt, std::next(FuncIt)), ...);

324

Same nitpick about simplifying this callsite.

tools/opt/opt.cpp

267

The implicit default constructor here calls the base constructor. You can just leave this line out -- the compiler does it for you.

[vsk]

test/DebugInfo/debugify-each.ll

1 ↗

(On Diff #146298)

Actually, s/-sroa/-instrprof -sroa/, because you'll want to run some ModulePass. It would also be fine to use CHECK-DAG here, to avoid a dependence on the pass order.

[vsk]

tools/opt/opt.cpp

298

You don't need to define this function either.

[tyb0807]

Addressed review comments.

I am thinking about making this work for BasicBlockPass and LoopPass. How can I make an empty range in LLVM (for these passes, function iterator should contain nothing)?

Does something like make_range(std::next(BB.getParent()->getIterator()), std::next(BB.getParent()->getIterator())) make sense?

Another question is, what if a Loop contains BasicBlocks from different Functions? I guess we can just pass an empty range for the function iterator parameter, and add another loop over a basic block iterator parameter in applyDebugifyMetadata and checkDebugifyMetadata.

Thanks again for your help Vedant.

[vsk]

In D46525#1096626, @tyb0807 wrote:

Addressed review comments.

I am thinking about making this work for BasicBlockPass and LoopPass.

Would you mind discussing this on IRC, or llvm-dev? Stepping back a bit, I think it'd make sense to prioritize finding & fixing bugs with -debugify-each before making it more granular. Once there's more experience using the testing tool, we may develop better ideas for improvements.

lib/IR/IRPrintingPasses.cpp

51 ↗

(On Diff #146430)

Why does this function need to be modified?

[tyb0807]

Sorry it was a local change, forgot to remove it before diffing. Please tell me if there's any way to handle local change while still being able to create patch properly.

So do you need anything else (even if it is not in debugify)? I am willing to help!

[vsk]

LGTM. If you don’t have commit access, let me know, and I can commit this for you on Monday.

In D46525#1096844, @tyb0807 wrote:

Sorry it was a local change, forgot to remove it before diffing. Please tell me if there's any way to handle local change while still being able to create patch properly.

No problem. If you use git, you can use “git -p” to selectively stage changes, commit them locally, and then “git stash” any remaining changes which shouldn’t be in the diff.

So do you need anything else (even if it is not in debugify)? I am willing to help!

Thanks again for your help. There’s certainly more to do. Some ideas:

• Extend the -verify-each mode in opt, so that it also is enabled after each module/function pass.
• Run opt -O1 with -debugify-each on input .ll files generated from clang, and see if you can find/fix debug info loss bugs.

Others in the community will have more ideas, so it’s worth asking around on IRC or LLVM-dev.

[tyb0807]

Thank you Vedant. I do not have commit access, please commit this for me.

tools/opt/Debugify.cpp

248

I guess it should do this, but actually it doesn't. Removing this here and submitting another patch to fix StripDebugInfo()

320

Oh well the reason for ShouldApply is that I don't want to print the error message for every function. I agree that this looks ugly but if you're ok with having the message printed multiple times for the same check pass then I'll remove this

test/DebugInfo/debugify.ll

 ; RUN: opt -debugify -S -o - < %s | FileCheck %s
 ; RUN: opt -passes=debugify -S -o - < %s | FileCheck %s
 
 ; RUN: opt -debugify -debugify -S -o - < %s 2>&1 | \
 ; RUN:   FileCheck %s -check-prefix=CHECK-REPEAT
 ; RUN: opt -passes=debugify,debugify -S -o - < %s 2>&1 | \
 ; RUN:   FileCheck %s -check-prefix=CHECK-REPEAT
 
 ; RUN: opt -debugify -check-debugify -S -o - < %s | \
-; RUN:   FileCheck %s -implicit-check-not="CheckDebugify: FAIL"
+; RUN:   FileCheck %s -implicit-check-not="CheckModuleDebugify: FAIL"
 ; RUN: opt -passes=debugify,check-debugify -S -o - < %s | \
-; RUN:   FileCheck %s -implicit-check-not="CheckDebugify: FAIL"
+; RUN:   FileCheck %s -implicit-check-not="CheckModuleDebugify: FAIL"
 ; RUN: opt -enable-debugify -passes=verify -S -o - < %s | \
-; RUN:   FileCheck %s -implicit-check-not="CheckDebugify: FAIL"
+; RUN:   FileCheck %s -implicit-check-not="CheckModuleDebugify: FAIL"
 
 ; RUN: opt -debugify -strip -check-debugify -S -o - < %s | \
 ; RUN:   FileCheck %s -check-prefix=CHECK-FAIL
 
 ; RUN: opt -enable-debugify -strip -S -o - < %s | \
 ; RUN:   FileCheck %s -check-prefix=CHECK-FAIL
 
 ; RUN: opt -enable-debugify -S -o - < %s | FileCheck %s -check-prefix=PASS
 
+; RUN: opt -debugify-each -debugify -S -o - < %s 2>&1 | \
+; RUN:   FileCheck %s -check-prefix=CHECK-EACH-REPEAT
+
+; RUN: opt -debugify-each -check-debugify -S -o - < %s 2>&1 | \
+; RUN:   FileCheck %s -check-prefix=CHECK-EACH-REPEAT-CHECK
+
+; RUN: opt -debugify-each -strip -S -o - < %s | \
+; RUN:   FileCheck %s -check-prefix=CHECK-EACH-FAIL
+
 ; CHECK-LABEL: define void @foo
 define void @foo() {
 ; CHECK: ret void, !dbg ![[RET1:.*]]
   ret void
 }
 
 ; CHECK-LABEL: define i32 @bar
 define i32 @bar() {

 ; CHECK-DAG: ![[TY32:.*]] = !DIBasicType(name: "ty32", size: 32, encoding: DW_ATE_unsigned)
 ; CHECK-DAG: !DILocalVariable(name: "1", scope: {{.*}}, file: {{.*}}, line: 3, type: ![[TY32]])
 
 ; --- Metadata counts
 ; CHECK-DAG: ![[NUM_INSTS]] = !{i32 4}
 ; CHECK-DAG: ![[NUM_VARS]] = !{i32 1}
 
 ; --- Repeat case
-; CHECK-REPEAT: Debugify: Skipping module with debug info
+; CHECK-REPEAT: ModuleDebugify: Skipping module with debug info
 
 ; --- Failure case
 ; CHECK-FAIL: ERROR: Instruction with empty DebugLoc --   ret void
 ; CHECK-FAIL: ERROR: Instruction with empty DebugLoc --   call void @foo()
 ; CHECK-FAIL: ERROR: Instruction with empty DebugLoc --   {{.*}} add i32 0, 1
 ; CHECK-FAIL: ERROR: Instruction with empty DebugLoc --   ret i32 0
 ; CHECK-FAIL: WARNING: Missing line 1
 ; CHECK-FAIL: WARNING: Missing line 2
 ; CHECK-FAIL: WARNING: Missing line 3
 ; CHECK-FAIL: WARNING: Missing line 4
 ; CHECK-FAIL: ERROR: Missing variable 1
-; CHECK-FAIL: CheckDebugify: FAIL
+; CHECK-FAIL: CheckModuleDebugify: FAIL
+
+; PASS: CheckModuleDebugify: PASS
 
-; PASS: CheckDebugify: PASS
+; --- Debugify-each repeat case
+; CHECK-EACH-REPEAT: ModuleDebugify: Skipping module with debug info
+; CHECK-EACH-REPEAT: CheckModuleDebugify: PASS
+; CHECK-EACH-REPEAT: CheckFunctionDebugify: PASS
+; CHECK-EACH-REPEAT: CheckFunctionDebugify: PASS
+; CHECK-EACH-REPEAT: CheckFunctionDebugify: PASS
+
+; --- Debugify-each repeat check case
+; CHECK-EACH-REPEAT-CHECK: CheckModuleDebugify: PASS
+; CHECK-EACH-REPEAT-CHECK: CheckModuleDebugify: PASS
+; CHECK-EACH-REPEAT-CHECK: CheckFunctionDebugify: PASS
+; CHECK-EACH-REPEAT-CHECK: CheckFunctionDebugify: PASS
+; CHECK-EACH-REPEAT-CHECK: CheckFunctionDebugify: PASS
+
+; --- Debugify-each failure case
+; CHECK-EACH-FAIL: ERROR: Instruction with empty DebugLoc --   ret void
+; CHECK-EACH-FAIL: ERROR: Instruction with empty DebugLoc --   call void @foo()
+; CHECK-EACH-FAIL: ERROR: Instruction with empty DebugLoc --   {{.*}} add i32 0, 1
+; CHECK-EACH-FAIL: ERROR: Instruction with empty DebugLoc --   ret i32 0
+; CHECK-EACH-FAIL: WARNING: Missing line 1
+; CHECK-EACH-FAIL: WARNING: Missing line 2
+; CHECK-EACH-FAIL: WARNING: Missing line 3
+; CHECK-EACH-FAIL: WARNING: Missing line 4
+; CHECK-EACH-FAIL: ERROR: Missing variable 1
+; CHECK-EACH-FAIL: CheckModuleDebugify: FAIL
+; CHECK-EACH-FAIL: CheckFunctionDebugify: PASS
+; CHECK-EACH-FAIL: CheckFunctionDebugify: PASS
+; CHECK-EACH-FAIL: CheckFunctionDebugify: PASS

test/Transforms/Mem2Reg/PromoteMemToRegister.ll

 ; Simple sanity check testcase.  Both alloca's should be eliminated.
 ; RUN: opt < %s -debugify -mem2reg -check-debugify -S | FileCheck %s
 
 ; CHECK-NOT: alloca
-; CHECK: CheckDebugify: PASS
+; CHECK: CheckModuleDebugify: PASS
 
 define double @testfunc(i32 %i, double %j) {
   %I = alloca i32   ; <i32*> [#uses=4]
   %J = alloca double    ; <double*> [#uses=2]
   store i32 %i, i32* %I
   store double %j, double* %J
   %t1 = load i32, i32* %I    ; <i32> [#uses=1]
   %t2 = add i32 %t1, 1    ; <i32> [#uses=1]
   store i32 %t2, i32* %I
   %t3 = load i32, i32* %I    ; <i32> [#uses=1]
   %t4 = sitofp i32 %t3 to double    ; <double> [#uses=1]
   %t5 = load double, double* %J    ; <double> [#uses=1]
   %t6 = fmul double %t4, %t5    ; <double> [#uses=1]
   ret double %t6
 }
 

tools/opt/Debugify.cpp

 using namespace llvm;
 
 namespace {
 
 bool isFunctionSkipped(Function &F) {
   return F.isDeclaration() || !F.hasExactDefinition();
 }
 
-bool applyDebugifyMetadata(Module &M) {
-  // Skip modules with debug info.
-  if (M.getNamedMetadata("llvm.dbg.cu")) {
-    errs() << "Debugify: Skipping module with debug info\n";
-    return false;
-  }
-
-  DIBuilder DIB(M);
-  LLVMContext &Ctx = M.getContext();
-
-  // Get a DIType which corresponds to Ty.
-  DenseMap<uint64_t, DIType *> TypeCache;
-  auto getCachedDIType = [&](Type *Ty) -> DIType * {
-    uint64_t Size =
-        Ty->isSized() ? M.getDataLayout().getTypeAllocSizeInBits(Ty) : 0;
-    DIType *&DTy = TypeCache[Size];
-    if (!DTy) {
-      std::string Name = "ty" + utostr(Size);
-      DTy = DIB.createBasicType(Name, Size, dwarf::DW_ATE_unsigned);
-    }
-    return DTy;
-  };
+DIType *getCachedDIType(uint64_t Size,
+                        DIBuilder &DIB,
+                        DenseMap<uint64_t, DIType *> &Cache) {
+  DIType *&DTy = Cache[Size];
+}
 
-  unsigned NextLine = 1;
-  unsigned NextVar = 1;
-  auto File = DIB.createFile(M.getName(), "/");
-  auto CU =
-      DIB.createCompileUnit(dwarf::DW_LANG_C, DIB.createFile(M.getName(), "/"),
-                            "debugify", /*isOptimized=*/true, "", 0);
+/// Track the number of distinct lines and variables.
+void insertLineAndVarCounters(unsigned NextLine,
+                            unsigned NextVar, 
+                            Module &M) {
+  LLVMContext &Ctx = M.getContext();
+  NamedMDNode *NMD = M.getOrInsertNamedMetadata("llvm.debugify");
+  auto *IntTy = Type::getInt32Ty(Ctx);
+  auto addDebugifyOperand = [&](unsigned N) {
+    NMD->addOperand(MDNode::get(
+        Ctx, ValueAsMetadata::getConstant(ConstantInt::get(IntTy, N))));
+  };
+  addDebugifyOperand(NextLine - 1); // Original number of lines.
+  addDebugifyOperand(NextVar - 1);  // Original number of variables.
+}
 
-  // Visit each instruction.
-  for (Function &F : M) {
-    if (isFunctionSkipped(F))
-      continue;
+bool applyFunctionDebugifyMetadata(Function &F,
+                                   DIBuilder &DIB,
+                                   DICompileUnit *CU,
+                                   unsigned &NextLine,
+                                   unsigned &NextVar,
+                                   DenseMap<uint64_t, DIType*> &Cache) {
+    return false;
+
+  Module *M = F.getParent();
+  LLVMContext &Ctx = M->getContext();
+  DIFile* File = CU->getFile();
 
-    auto SPType = DIB.createSubroutineType(DIB.getOrCreateTypeArray(None));
-    bool IsLocalToUnit = F.hasPrivateLinkage() || F.hasInternalLinkage();
-    auto SP =
-        DIB.createFunction(CU, F.getName(), F.getName(), File, NextLine, SPType,
-                           IsLocalToUnit, /*isDefinition=*/true, NextLine,
-                           DINode::FlagZero, /*isOptimized=*/true);
-    F.setSubprogram(SP);
-    for (BasicBlock &BB : F) {
-      // Attach debug locations.
-      for (Instruction &I : BB)
-        I.setDebugLoc(DILocation::get(Ctx, NextLine++, 1, SP));
+  // Visit each instruction.
 
-      // Attach debug values.
-      for (Instruction &I : BB) {
-        // Skip void-valued instructions.
-        if (I.getType()->isVoidTy())
-          continue;
 
-        // Skip the terminator instruction and any just-inserted intrinsics.
-        if (isa<TerminatorInst>(&I) || isa<DbgValueInst>(&I))
-          break;
 
-        std::string Name = utostr(NextVar++);
-        const DILocation *Loc = I.getDebugLoc().get();
-        auto LocalVar = DIB.createAutoVariable(SP, Name, File, Loc->getLine(),
-                                               getCachedDIType(I.getType()),
-                                               /*AlwaysPreserve=*/true);
-        DIB.insertDbgValueIntrinsic(&I, LocalVar, DIB.createExpression(), Loc,
-                                    BB.getTerminator());
-      }
-    }
-    DIB.finalizeSubprogram(SP);
+      Type *Ty = I.getType();
+      uint64_t Size = Ty->isSized() ? M->getDataLayout().getTypeAllocSizeInBits(Ty) : 0;
+                                             getCachedDIType(Size, DIB, Cache),
+  return true;
+}
+
+bool applyModuleDebugifyMetadata(Module &M) {
+  // Skip modules with debug info.
+  if (M.getNamedMetadata("llvm.dbg.cu")) {
+    errs() << "ModuleDebugify: Skipping module with debug info\n";
+    return false;
   }
+
+  DIBuilder DIB(M);
+
+  // Get a DIType which corresponds to Ty.
+  DenseMap<uint64_t, DIType *> TypeCache;
+
+  unsigned NextLine = 1;
+  unsigned NextVar = 1;
+  auto CU =
+      DIB.createCompileUnit(dwarf::DW_LANG_C, DIB.createFile(M.getName(), "/"),
+                            "debugify", /*isOptimized=*/true, "", 0);
+
+  // Visit each instruction.
+  for (Function &F : M)
+    applyFunctionDebugifyMetadata(F, DIB, CU, NextLine, NextVar, TypeCache); 
+
   DIB.finalize();
 
-  // Track the number of distinct lines and variables.
-  NamedMDNode *NMD = M.getOrInsertNamedMetadata("llvm.debugify");
-  auto *IntTy = Type::getInt32Ty(Ctx);
-  auto addDebugifyOperand = [&](unsigned N) {
-    NMD->addOperand(MDNode::get(
-        Ctx, ValueAsMetadata::getConstant(ConstantInt::get(IntTy, N))));
-  };
-  addDebugifyOperand(NextLine - 1); // Original number of lines.
-  addDebugifyOperand(NextVar - 1);  // Original number of variables.
+  insertLineAndVarCounters(NextLine, NextVar, M);
   return true;
 }
 
-void checkDebugifyMetadata(Module &M) {
-  // Skip modules without debugify metadata.
-  NamedMDNode *NMD = M.getNamedMetadata("llvm.debugify");
-  if (!NMD)
-    return;
+bool checkFunctionDebugifyMetadata(Function &F,
+                                   BitVector &MissingLines,
+                                   BitVector &MissingVars) {
+  if (isFunctionSkipped(F))
+    return false;
 
-  auto getDebugifyOperand = [&](unsigned Idx) -> unsigned {
-    return mdconst::extract<ConstantInt>(NMD->getOperand(Idx)->getOperand(0))
-        ->getZExtValue();
-  };
-  unsigned OriginalNumLines = getDebugifyOperand(0);
-  unsigned OriginalNumVars = getDebugifyOperand(1);
   bool HasErrors = false;
 
   // Find missing lines.
-  BitVector MissingLines{OriginalNumLines, true};
-  for (Function &F : M) {
-    if (isFunctionSkipped(F))
-      continue;
-
-    for (Instruction &I : instructions(F)) {
-      if (isa<DbgValueInst>(&I))
-        continue;
 
-      auto DL = I.getDebugLoc();
-      if (DL) {
-        MissingLines.reset(DL.getLine() - 1);
-        continue;
-      }
+    if (DL && MissingLines.size() > (DL.getLine() - 1)) {
 
+    if (DL)
+      outs() << "ERROR: Instruction with invalid DebugLoc -- ";
+    else
       outs() << "ERROR: Instruction with empty DebugLoc -- ";
-      I.print(outs());
-      outs() << "\n";
-      HasErrors = true;
-    }
-  }
-  for (unsigned Idx : MissingLines.set_bits())
-    outs() << "WARNING: Missing line " << Idx + 1 << "\n";
 
   // Find missing variables.
-  BitVector MissingVars{OriginalNumVars, true};
-  for (Function &F : M) {
-    if (isFunctionSkipped(F))
-      continue;
-
-    for (Instruction &I : instructions(F)) {
-      auto *DVI = dyn_cast<DbgValueInst>(&I);
-      if (!DVI)
-        continue;
 
-      unsigned Var = ~0U;
-      (void)to_integer(DVI->getVariable()->getName(), Var, 10);
-      assert(Var <= OriginalNumVars && "Unexpected name for DILocalVariable");
-      MissingVars.reset(Var - 1);
-    }
-  }
+    assert(Var <= MissingVars.size() && "Unexpected name for DILocalVariable");
+
+  return HasErrors;
+
+void printCheckDebugifyResults(std::string Conclusion,
+                               BitVector MissingLines,
+                               BitVector MissingVars) {
+  for (unsigned Idx : MissingLines.set_bits())
+    outs() << "WARNING: Missing line " << Idx + 1 << "\n";
+
   for (unsigned Idx : MissingVars.set_bits())
     outs() << "ERROR: Missing variable " << Idx + 1 << "\n";
+
+  outs() << Conclusion << '\n';
+}
+
+void stripDebugifyMetadata(Module &M) {
+  NamedMDNode *NMD = M.getNamedMetadata("llvm.debugify");
+  StripDebugInfo(M);
+  M.eraseNamedMetadata(NMD);
+  if (Function *F = M.getFunction("llvm.dbg.value")) {
+    F->replaceAllUsesWith(UndefValue::get((Type*)F->getType()));
+    F->eraseFromParent();
+  }
+}
+
+unsigned getDebugifyOperand(NamedMDNode *NMD, unsigned Idx) {
+  return mdconst::extract<ConstantInt>(NMD->getOperand(Idx)->getOperand(0))
+      ->getZExtValue();
+}
+
+void checkModuleDebugifyMetadata(Module &M) {
+  // Skip modules without debugify metadata.
+  NamedMDNode *NMD = M.getNamedMetadata("llvm.debugify");
+  if (!NMD) {
+    errs() << "CheckModuleDebugify: "
+           << "Skipping module without debugify metadata\n";
+    return;
+  }
+
+  unsigned OriginalNumLines = getDebugifyOperand(NMD, 0);
+  unsigned OriginalNumVars = getDebugifyOperand(NMD, 1);
+  bool HasErrors = false;
+
+  BitVector MissingLines{OriginalNumLines, true};
+  BitVector MissingVars{OriginalNumVars, true};
+  for (Function &F : M)
+    HasErrors |= checkFunctionDebugifyMetadata(F, MissingLines, MissingVars);
+
   HasErrors |= MissingVars.count() > 0;
 
-  outs() << "CheckDebugify: " << (HasErrors ? "FAIL" : "PASS") << "\n";
+  std::string Conclusion =
+      "CheckModuleDebugify: " + (HasErrors ? std::string("FAIL")
+                                           : std::string("PASS"));
+  printCheckDebugifyResults(Conclusion, MissingLines, MissingVars);
+}
+
+/// ModulePass for attaching synthetic debug info to everything, used with the
+/// legacy module pass manager.
+struct DebugifyModulePass : public ModulePass {
+  bool runOnModule(Module &M) override { return applyModuleDebugifyMetadata(M); }
+
+  DebugifyModulePass() : ModulePass(ID) {}
+
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    AU.setPreservesAll();
+  }
+
+  static char ID; // Pass identification.
+};
+
+/// FunctionPass for attaching synthetic debug info to everything, used with the
+/// legacy module pass manager.
+struct DebugifyFunctionPass : public FunctionPass {
+  bool runOnFunction(Function &F) override {
+    if (!ShouldApply)
+      return false;
+
+    Module *M = F.getParent();
+    unsigned NextLine = 1;
+    unsigned NextVar = 1;
+    DIBuilder DIB(*M);
+    DICompileUnit *CU = DIB.createCompileUnit(dwarf::DW_LANG_C,
+                                  DIB.createFile(M->getName(), "/"),
+                                  "debugify", /*isOptimized=*/true, "", 0);
+
+    bool Changed = applyFunctionDebugifyMetadata(F, DIB, CU, NextLine, NextVar, TypeCache);
+
+    insertLineAndVarCounters(NextLine, NextVar, *M);
+    return Changed;
   }
 
-/// Attach synthetic debug info to everything.
-struct DebugifyPass : public ModulePass {
-  bool runOnModule(Module &M) override { return applyDebugifyMetadata(M); }
+  bool doInitialization(Module &M) override {
+    ShouldApply = (M.getNamedMetadata("llvm.dbg.cu") == nullptr);
+    if (!ShouldApply)
+      errs() << "FunctionDebugify: Skipping module with debug info\n";
+    return false;
+  }
 
-  DebugifyPass() : ModulePass(ID) {}
+  DebugifyFunctionPass() : FunctionPass(ID) {}
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.setPreservesAll();
   }
 
   static char ID; // Pass identification.
+
+private:
+  bool ShouldApply;
+  DenseMap<uint64_t, DIType *> TypeCache;
 };
 
-/// Check debug info inserted by -debugify for completeness.
-struct CheckDebugifyPass : public ModulePass {
+/// ModulePass for checking debug info inserted by -debugify, used with the
+/// legacy module pass manager.
+struct CheckDebugifyModulePass : public ModulePass {
   bool runOnModule(Module &M) override {
-    checkDebugifyMetadata(M);
+    checkModuleDebugifyMetadata(M);
+    if (Strip) {
+      stripDebugifyMetadata(M);
+      return true;
+    }
+
+    return false;
+  }
+
+  CheckDebugifyModulePass(bool Strip = false) : ModulePass(ID) {
+    this->Strip = Strip;
+  }
+
+  static char ID; // Pass identification.
+
+private:
+  bool Strip;
+};
+
+/// FunctionPass for checking debug info inserted by -debugify-function, used
+/// with the legacy module pass manager.
+struct CheckDebugifyFunctionPass : public FunctionPass {
+  bool runOnFunction(Function &F) override {
+    Module *M = F.getParent();
+    NamedMDNode *NMD = M->getNamedMetadata("llvm.debugify");
+    if (!NMD) {
+      errs() << "CheckFunctionDebugify: "
+             << "Skipping module without debugify metadata\n";
+      return false;
+    }
+
+    unsigned OriginalNumLines = getDebugifyOperand(NMD, 0);
+    unsigned OriginalNumVars = getDebugifyOperand(NMD, 1);
+
+    BitVector MissingLines{OriginalNumLines, true};
+    BitVector MissingVars{OriginalNumVars, true};
+
+    bool HasErrors = checkFunctionDebugifyMetadata(F,
+                                                   MissingLines,
+                                                   MissingVars);
+    HasErrors |= MissingVars.count() > 0;
+
+    std::string Conclusion =
+        "CheckFunctionDebugify: " + (HasErrors ? std::string("FAIL")
+                                               : std::string("PASS"));
+    printCheckDebugifyResults(Conclusion, MissingLines, MissingVars);
+
+    if (Strip) {
+      stripDebugifyMetadata(*M);
+      return true;
+    }
+
     return false;
   }
 
-  CheckDebugifyPass() : ModulePass(ID) {}
+  CheckDebugifyFunctionPass(bool Strip = false) : FunctionPass(ID) {
+    this->Strip = Strip;
+  }
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.setPreservesAll();
   }
 
   static char ID; // Pass identification.
+
+private:
+  bool Strip;
 };
 
 } // end anonymous namespace
 
-ModulePass *createDebugifyPass() { return new DebugifyPass(); }
+ModulePass *createDebugifyModulePass() {
+  return new DebugifyModulePass();
+}
+
+FunctionPass *createDebugifyFunctionPass() {
+  return new DebugifyFunctionPass();
+}
 
 PreservedAnalyses NewPMDebugifyPass::run(Module &M, ModuleAnalysisManager &) {
-  applyDebugifyMetadata(M);
+  applyModuleDebugifyMetadata(M);
   return PreservedAnalyses::all();
 }
 
-ModulePass *createCheckDebugifyPass() { return new CheckDebugifyPass(); }
+ModulePass *createCheckDebugifyModulePass(bool Strip) {
+  return new CheckDebugifyModulePass(Strip);
+}
+
+FunctionPass *createCheckDebugifyFunctionPass(bool Strip) {
+  return new CheckDebugifyFunctionPass(Strip);
+}
 
 PreservedAnalyses NewPMCheckDebugifyPass::run(Module &M,
                                               ModuleAnalysisManager &) {
-  checkDebugifyMetadata(M);
+  checkModuleDebugifyMetadata(M);
   return PreservedAnalyses::all();
 }
 
-char DebugifyPass::ID = 0;
-static RegisterPass<DebugifyPass> X("debugify",
+char DebugifyModulePass::ID = 0;
+static RegisterPass<DebugifyModulePass> DM("debugify",
                                     "Attach debug info to everything");
 
-char CheckDebugifyPass::ID = 0;
-static RegisterPass<CheckDebugifyPass> Y("check-debugify",
+char CheckDebugifyModulePass::ID = 0;
+static RegisterPass<CheckDebugifyModulePass> CDM("check-debugify",
                                          "Check debug info from -debugify");
+
+char DebugifyFunctionPass::ID = 0;
+static RegisterPass<DebugifyFunctionPass> DF("debugify-function",
+                                    "Attach debug info to a function");
+
+char CheckDebugifyFunctionPass::ID = 0;
+static RegisterPass<CheckDebugifyFunctionPass> CDF("check-debugify-function",
+                                         "Check debug info from -debugify-function");

tools/opt/PassPrinters.h

 RegionPass *createRegionPassPrinter(const PassInfo *PI, raw_ostream &out,
                                     bool Quiet);
 
 BasicBlockPass *createBasicBlockPassPrinter(const PassInfo *PI,
                                             raw_ostream &out, bool Quiet);
 
 } // end namespace llvm
 
-llvm::ModulePass *createDebugifyPass();
+llvm::ModulePass *createDebugifyModulePass();
+llvm::FunctionPass *createDebugifyFunctionPass();
 
 struct NewPMDebugifyPass : public llvm::PassInfoMixin<NewPMDebugifyPass> {
   llvm::PreservedAnalyses run(llvm::Module &M, llvm::ModuleAnalysisManager &AM);
 };
 
-llvm::ModulePass *createCheckDebugifyPass();
+llvm::ModulePass *createCheckDebugifyModulePass(bool Strip = false);
+llvm::FunctionPass *createCheckDebugifyFunctionPass(bool Strip = false);
 
 struct NewPMCheckDebugifyPass
     : public llvm::PassInfoMixin<NewPMCheckDebugifyPass> {
   llvm::PreservedAnalyses run(llvm::Module &M, llvm::ModuleAnalysisManager &AM);
 };
 
+llvm::ModulePass *createStripDbgInfoPass();
+
 #endif // LLVM_TOOLS_OPT_PASSPRINTERS_H

tools/opt/opt.cpp

 static cl::opt<bool>
 AnalyzeOnly("analyze", cl::desc("Only perform analysis, no optimization"));
 
 static cl::opt<bool> EnableDebugify(
     "enable-debugify",
     cl::desc(
         "Start the pipeline with debugify and end it with check-debugify"));
 
+static cl::opt<bool> DebugifyEach(
+    "debugify-each",
+    cl::desc(
+        "Start each pass with debugify and end it with check-debugify"));
+
 static cl::opt<bool>
 PrintBreakpoints("print-breakpoints-for-testing",
                  cl::desc("Print select breakpoints location for testing"));
 
 static cl::opt<std::string> ClDataLayout("data-layout",
                                          cl::desc("data layout string to use"),
                                          cl::value_desc("layout-string"),
                                          cl::init(""));
static cl::opt<unsigned> PassRemarksHotnessThreshold(
     cl::desc("Minimum profile count required for an optimization remark to be output"),
     cl::Hidden);
 
 static cl::opt<std::string>
     RemarksFilename("pass-remarks-output",
                     cl::desc("YAML output filename for pass remarks"),
                     cl::value_desc("filename"));
 
+class DebugifyEachPassManager : public legacy::PassManager {
+  bool DebugifyEach;
+
+public:
+  DebugifyEachPassManager(bool DebugifyEach) : legacy::PassManager(),
+                                               DebugifyEach(DebugifyEach) {}
+
+  void add(Pass *P) override {
+    if (!DebugifyEach) {
+      PassManager::add(P);
+      return;
+    }
+
+    // Do not debugify ImmutablePass
+    if (P->getAsImmutablePass()) {
+      PassManager::add(P);
+    }
+    // Do not debugify PrinterPass
+    else if (P->getPassName() == "Print Module IR") {
+      PassManager::add(P);
+    } else {
+      PassKind Kind = P->getPassKind();
+      switch (Kind) {
+        case PT_BasicBlock:
+          PassManager::add(P);
+          break;
+        case PT_Region:
+          PassManager::add(P);
+          break;
+        case PT_Loop:
+          PassManager::add(P);
+          break;
+        case PT_Function:
+          PassManager::add(createDebugifyFunctionPass());
+          PassManager::add(P);
+          PassManager::add(createCheckDebugifyFunctionPass(true));
+          break;
+        case PT_CallGraphSCC:
+          PassManager::add(P);
+          break;
+        case PT_Module:
+          PassManager::add(createDebugifyModulePass());
+          PassManager::add(P);
+          PassManager::add(createCheckDebugifyModulePass(true));
+          break;
+        case PT_PassManager:
+          PassManager::add(P);
+          break;
+        default:
+          break;
+      }
+    }
+  }
+
+  bool run(Module &M) {
+    return PassManager::run(M);
+  }
+};
+    
 static inline void addPass(legacy::PassManagerBase &PM, Pass *P) {
   // Add the pass to the pass manager...
   PM.add(P);
 
   // If we are verifying all of the intermediate steps, add the verifier...
   if (VerifyEach)
     PM.add(createVerifierPass());
 }
return runPassPipeline(argv[0], *M, TM.get(), Out.get(), ThinLinkOut.get(),
                            PreserveBitcodeUseListOrder, EmitSummaryIndex,
                            EmitModuleHash, EnableDebugify)
                ? 0
                : 1;
   }
 
   // Create a PassManager to hold and optimize the collection of passes we are
   // about to build.
-  //
-  legacy::PassManager Passes;
+  DebugifyEachPassManager Passes(DebugifyEach);
 
   // Add an appropriate TargetLibraryInfo pass for the module's triple.
   TargetLibraryInfoImpl TLII(ModuleTriple);
 
   // The -disable-simplify-libcalls flag actually disables all builtin optzns.
   if (DisableSimplifyLibCalls)
     TLII.disableAllFunctions();
   Passes.add(new TargetLibraryInfoWrapperPass(TLII));
 
   // Add internal analysis passes from the target machine.
   Passes.add(createTargetTransformInfoWrapperPass(TM ? TM->getTargetIRAnalysis()
                                                      : TargetIRAnalysis()));
 
-  if (EnableDebugify)
-    Passes.add(createDebugifyPass());
+  if (EnableDebugify && !DebugifyEach)
+    Passes.add(createDebugifyModulePass());
 
   std::unique_ptr<legacy::FunctionPassManager> FPasses;
   if (OptLevelO0 || OptLevelO1 || OptLevelO2 || OptLevelOs || OptLevelOz ||
       OptLevelO3) {
     FPasses.reset(new legacy::FunctionPassManager(M.get()));
     FPasses->add(createTargetTransformInfoWrapperPass(
         TM ? TM->getTargetIRAnalysis() : TargetIRAnalysis()));
   }
for (Function &F : *M)
       FPasses->run(F);
     FPasses->doFinalization();
   }
 
   // Check that the module is well formed on completion of optimization
   if (!NoVerify && !VerifyEach)
     Passes.add(createVerifierPass());
 
-  if (EnableDebugify)
-    Passes.add(createCheckDebugifyPass());
+  if (EnableDebugify && !DebugifyEach)
+    Passes.add(createCheckDebugifyModulePass(false));
 
   // In run twice mode, we want to make sure the output is bit-by-bit
   // equivalent if we run the pass manager again, so setup two buffers and
   // a stream to write to them. Note that llc does something similar and it
   // may be worth to abstract this out in the future.
   SmallVector<char, 0> Buffer;
   SmallVector<char, 0> FirstRunBuffer;
   std::unique_ptr<raw_svector_ostream> BOS;