[Debugfiy][WIP] Print the pass name next to the result

[gramanas]

Summary

CheckDebugify now prints the pass name right next to the result of the check.

Does not yet work with the new Pass Manager, hence the WIP.

[vsk]

Thanks for the patch. Comments inline --

tools/opt/Debugify.cpp

195

You'll need to update test/DebugInfo/debugify-each.ll to reflect this change.

258

Please use clang-format to format your diffs. See:

• https://clang.llvm.org/docs/ClangFormat.html
• https://github.com/llvm-mirror/clang/blob/master/tools/clang-format/clang-format-diff.py

tools/opt/PassPrinters.h

70

There isn't support for -debugify-each in the newPM yet, so there's no need to tackle that in this patch. I think we should leave newPM-related changes to a future patch.

70

I think 'NameOfWrappedPass' would be a more accurate/descriptive name here.

[gramanas]

• Adress review comments

[vsk]

LGTM. Thanks!

As a follow-up, would you mind fixing the situation with debugify output? Currently we're dumping a lot of information to stdout. Ideally we would only print the result of the pipeline to stdout, so as to not break piping opt's output to other LLVM tools. As a first step, I think all of debugify's warnings and messages should be sent to stderr, and then eventually to a separate file.

[gramanas]

Will do, it's allready in my TODO list!

test/DebugInfo/debugify-each.ll

 ; RUN: opt -debugify-each -O3 -S -o - < %s | FileCheck %s
 ; RUN: opt -debugify-each -instrprof -sroa -sccp -S -o - < %s | FileCheck %s
 
 define void @foo() {
   ret void
 }
 
 define void @bar() {
   ret void
 }
 
 ; Verify that the module & function (check-)debugify passes run at least twice.
 
-; CHECK-DAG: CheckModuleDebugify: PASS
-; CHECK-DAG: CheckFunctionDebugify: PASS
-; CHECK-DAG: CheckFunctionDebugify: PASS
-; CHECK-DAG: CheckFunctionDebugify: PASS
-; CHECK-DAG: CheckFunctionDebugify: PASS
+; CHECK-DAG: CheckModuleDebugify [{{.*}}]: PASS
+; CHECK-DAG: CheckFunctionDebugify [{{.*}}]: PASS
+; CHECK-DAG: CheckFunctionDebugify [{{.*}}]: PASS
+; CHECK-DAG: CheckFunctionDebugify [{{.*}}]: PASS
+; CHECK-DAG: CheckFunctionDebugify [{{.*}}]: PASS
 
-; CHECK-DAG: CheckModuleDebugify: PASS
-; CHECK-DAG: CheckFunctionDebugify: PASS
-; CHECK-DAG: CheckFunctionDebugify: PASS
-; CHECK-DAG: CheckFunctionDebugify: PASS
-; CHECK-DAG: CheckFunctionDebugify: PASS
+; CHECK-DAG: CheckModuleDebugify [{{.*}}]: PASS
+; CHECK-DAG: CheckFunctionDebugify [{{.*}}]: PASS
+; CHECK-DAG: CheckFunctionDebugify [{{.*}}]: PASS
+; CHECK-DAG: CheckFunctionDebugify [{{.*}}]: PASS
+; CHECK-DAG: CheckFunctionDebugify [{{.*}}]: PASS

test/DebugInfo/debugify.ll

 ; CHECK-FAIL: ERROR: Instruction with empty DebugLoc in function bar --   call void @foo()
 ; CHECK-FAIL: ERROR: Instruction with empty DebugLoc in function bar --   {{.*}} add i32 0, 1
 ; CHECK-FAIL: ERROR: Instruction with empty DebugLoc in function bar --   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: CheckModuleDebugify: FAIL
+; CHECK-FAIL: CheckModuleDebugify [{{.*}}]: FAIL
 
-; PASS: CheckModuleDebugify: PASS
+; PASS: CheckModuleDebugify [{{.*}}]: PASS

tools/opt/Debugify.cpp

bool applyDebugifyMetadata(Module &M,
   addDebugifyOperand(NextVar - 1);  // Original number of variables.
   assert(NMD->getNumOperands() == 2 &&
          "llvm.debugify should have exactly 2 operands!");
   return true;
 }
 
 bool checkDebugifyMetadata(Module &M,
                            iterator_range<Module::iterator> Functions,
+                           StringRef NameOfWrappedPass,
                            StringRef Banner,
                            bool Strip) {
   // Skip modules without debugify metadata.
   NamedMDNode *NMD = M.getNamedMetadata("llvm.debugify");
   if (!NMD) {
     errs() << Banner << "Skipping module without debugify metadata\n";
     return false;
   }
bool checkDebugifyMetadata(Module &M,
   // Print the results.
   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";
   HasErrors |= MissingVars.count() > 0;
 
-  outs() << Banner << (HasErrors ? "FAIL" : "PASS") << '\n';
+  outs() << Banner << " [" << NameOfWrappedPass << "]: "
+         << (HasErrors ? "FAIL" : "PASS") << '\n';
   if (HasErrors) {
     outs() << "Module IR Dump\n";
     M.print(outs(), nullptr, false);
   }
 
   // Strip the Debugify Metadata if required.
   if (Strip) {
     StripDebugInfo(M);
struct DebugifyFunctionPass : public FunctionPass {
 
   static char ID; // Pass identification.
 };
 
 /// ModulePass for checking debug info inserted by -debugify, used with the
 /// legacy module pass manager.
 struct CheckDebugifyModulePass : public ModulePass {
   bool runOnModule(Module &M) override {
-    return checkDebugifyMetadata(M, M.functions(), "CheckModuleDebugify: ",
-                                 Strip);
+    return checkDebugifyMetadata(M, M.functions(), NameOfWrappedPass,
+                                 "CheckModuleDebugify", Strip);
   }
 
-  CheckDebugifyModulePass(bool Strip = false) : ModulePass(ID), Strip(Strip) {}
+  CheckDebugifyModulePass(bool Strip = false, StringRef NameOfWrappedPass = "")
+      : ModulePass(ID), Strip(Strip), NameOfWrappedPass(NameOfWrappedPass) {}
 
   static char ID; // Pass identification.
 
 private:
   bool Strip;
+  StringRef NameOfWrappedPass;
 };
 
 /// 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();
     auto FuncIt = F.getIterator();
     return checkDebugifyMetadata(M, make_range(FuncIt, std::next(FuncIt)),
-                     "CheckFunctionDebugify: ", Strip);
+                                 NameOfWrappedPass, "CheckFunctionDebugify", Strip);
   }
 
-  CheckDebugifyFunctionPass(bool Strip = false) : FunctionPass(ID), Strip(Strip) {}
+  CheckDebugifyFunctionPass(bool Strip = false, StringRef NameOfWrappedPass = "")
+      : FunctionPass(ID), Strip(Strip), NameOfWrappedPass(NameOfWrappedPass) {}
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.setPreservesAll();
   }
 
   static char ID; // Pass identification.
 
 private:
   bool Strip;
+  StringRef NameOfWrappedPass;
 };
 
 } // end anonymous namespace
 
 ModulePass *createDebugifyModulePass() {
   return new DebugifyModulePass();
 }
 
 FunctionPass *createDebugifyFunctionPass() {
   return new DebugifyFunctionPass();
 }
 
 PreservedAnalyses NewPMDebugifyPass::run(Module &M, ModuleAnalysisManager &) {
   applyDebugifyMetadata(M, M.functions(), "ModuleDebugify: ");
   return PreservedAnalyses::all();
 }
 
-ModulePass *createCheckDebugifyModulePass(bool Strip) {
-  return new CheckDebugifyModulePass(Strip);
+ModulePass *createCheckDebugifyModulePass(bool Strip, StringRef NameOfWrappedPass) {
+  return new CheckDebugifyModulePass(Strip, NameOfWrappedPass);
 }
 
-FunctionPass *createCheckDebugifyFunctionPass(bool Strip) {
-  return new CheckDebugifyFunctionPass(Strip);
+FunctionPass *createCheckDebugifyFunctionPass(bool Strip, StringRef NameOfWrappedPass) {
+  return new CheckDebugifyFunctionPass(Strip, NameOfWrappedPass);
 }
 
 PreservedAnalyses NewPMCheckDebugifyPass::run(Module &M,
                                               ModuleAnalysisManager &) {
-  checkDebugifyMetadata(M, M.functions(), "CheckModuleDebugify: ", false);
+  checkDebugifyMetadata(M, M.functions(), "", "CheckModuleDebugify", false);
   return PreservedAnalyses::all();
 }
 
 char DebugifyModulePass::ID = 0;
 static RegisterPass<DebugifyModulePass> DM("debugify",
                                     "Attach debug info to everything");
 
 char CheckDebugifyModulePass::ID = 0;

tools/opt/PassPrinters.h

 
 llvm::ModulePass *createDebugifyModulePass();
 llvm::FunctionPass *createDebugifyFunctionPass();
 
 struct NewPMDebugifyPass : public llvm::PassInfoMixin<NewPMDebugifyPass> {
   llvm::PreservedAnalyses run(llvm::Module &M, llvm::ModuleAnalysisManager &AM);
 };
 
-llvm::ModulePass *createCheckDebugifyModulePass(bool Strip = false);
-llvm::FunctionPass *createCheckDebugifyFunctionPass(bool Strip = false);
+llvm::ModulePass *
+createCheckDebugifyModulePass(bool Strip = false,
+                              llvm::StringRef NameOfWrappedPass = "");
+
+llvm::FunctionPass *
+createCheckDebugifyFunctionPass(bool Strip = false,
+                                llvm::StringRef NameOfWrappedPass = "");
 
 struct NewPMCheckDebugifyPass
     : public llvm::PassInfoMixin<NewPMCheckDebugifyPass> {
   llvm::PreservedAnalyses run(llvm::Module &M, llvm::ModuleAnalysisManager &AM);
 };
 
 #endif // LLVM_TOOLS_OPT_PASSPRINTERS_H

tools/opt/opt.cpp

if (!WrapWithDebugify) {
       return;
     }
     PassKind Kind = P->getPassKind();
     // TODO: Implement Debugify for BasicBlockPass, LoopPass.
     switch (Kind) {
       case PT_Function:
         super::add(createDebugifyFunctionPass());
         super::add(P);
-        super::add(createCheckDebugifyFunctionPass(true));
+        super::add(createCheckDebugifyFunctionPass(true, P->getPassName()));
         break;
       case PT_Module:
         super::add(createDebugifyModulePass());
         super::add(P);
-        super::add(createCheckDebugifyModulePass(true));
+        super::add(createCheckDebugifyModulePass(true, P->getPassName()));
         break;
       default:
         super::add(P);
         break;
     }
   }
 };