Add MIR-level debugify with only locations support for now

[dsanders]

Summary

Re-used the IR-level debugify for the most part. The MIR-level code then
adds locations to the MachineInstrs afterwards based on the LLVM-IR debug
info.

It's worth mentioning that the resulting locations make little sense as
the range of line numbers used in a Function at the MIR level exceeds that
of the equivelent IR level function. As such, MachineInstrs can appear to
originate from outside the subprogram scope (and from other subprogram
scopes). However, it doesn't seem worth worrying about as the source is
imaginary anyway.

There's a few high level goals this pass works towards:

• We should be able to debugify our .ll/.mir in the lit tests without changing the checks and still pass them. I.e. Debug info should not change codegen. Combining this with a strip-debug pass should enable this. The main issue I ran into without the strip-debug pass was instructions with MMO's and checks on both the instruction and the MMO as the debug-location is between them. I currently have a simple hack in the MIRPrinter to resolve that but the more general solution is a proper strip-debug pass.
• We should be able to test that GlobalISel does not lose debug info. I recently found that the legalizer can be unexpectedly lossy in seemingly simple cases (e.g. expanding one instr into many). I have a verifier (will be posted separately) that can be integrated with passes that use the observer interface and will catch location loss (it does not verify correctness, just that there's zero lossage). It is a little conservative as the line-0 locations that arise from conflicts do not track the conflicting locations but it can still catch a fair bit.

Depends on D77439, D77438

[aprantl]

Cool! Vedant should probably look this over, since he implemented the IR pass.

[vsk]

Awesome. Agreed that a proper -strip-debug pass is sorely needed (one that strips out the intrinsics/meta-instructions, dead prototypes, and unused metadata).

This looks good overall but needs a test.

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

22

Could you please add doxygen for the parameters?

llvm/lib/CodeGen/MachineDebugify.cpp

32

Is MF.dump() needed?

46

Yes, this is fine.

[dsanders]

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

22

Sure

llvm/lib/CodeGen/MachineDebugify.cpp

32

Oops. I meant to remove that. It's how I confirmed I had the boilerplate working before I started adding the code.

[dsanders]

Add doxygen comment
Remove MF.dump()

[vsk]

I realize this functionality will be exercised in D77576, but still think it'd be worthwhile to add a standalone test for this.

[dsanders]

Add test case

[vsk]

Thanks, lgtm!

Later on, I think we'll want to insert DBG_VALUE insts as well, but this looks very useful as-is.

[dsanders]

In D77446#1967893, @vsk wrote:

Thanks, lgtm!

Later on, I think we'll want to insert DBG_VALUE insts as well, but this looks very useful as-is.

Yep, I'll need to find all my DBG_VALUE-affects-CodeGen bugs :-)

[djtodoro]

Thanks for this work! I've missed this, since it was not marked as 'debug-info' project.

I think we should document this at https://llvm.org/docs/SourceLevelDebugging.html#testing-debug-info-preservation-in-optimizations.

[djtodoro]

In D77446#2031004, @djtodoro wrote:

Thanks for this work! I've missed this, since it was not marked as 'debug-info' project.

I think we should document this at https://llvm.org/docs/SourceLevelDebugging.html#testing-debug-info-preservation-in-optimizations.

The D80052 will address this comment.

[dsanders]

In D77446#2043308, @djtodoro wrote:

In D77446#2031004, @djtodoro wrote:

Thanks for this work! I've missed this, since it was not marked as 'debug-info' project.

I think we should document this at https://llvm.org/docs/SourceLevelDebugging.html#testing-debug-info-preservation-in-optimizations.

The D80052 will address this comment.

Good point. There's also the LostDebugLocObserver for detecting location loss during backend passes which I've mentioned on that review. If Vedant adds a section for it I can fill in some detail on how to use it afterwards.

Thanks

[ychen]

Any reason this is a module pass instead of a machine pass?

llvm/include/llvm/CodeGen/Passes.h

/// MachineDominanaceFrontier - This pass is a machine dominators analysis pass.
   FunctionPass *createCFGuardLongjmpPass();
 
   /// Create Hardware Loop pass. \see HardwareLoops.cpp
   FunctionPass *createHardwareLoopsPass();
 
   /// Create IR Type Promotion pass. \see TypePromotion.cpp
   FunctionPass *createTypePromotionPass();
 
+  /// Creates MIR Debugify pass. \see MachineDebugify.cpp
+  ModulePass *createDebugifyMachineModulePass();
 } // End llvm namespace
 
 #endif

llvm/include/llvm/InitializePasses.h

 void initializeCrossDSOCFIPass(PassRegistry&);
 void initializeDAEPass(PassRegistry&);
 void initializeDAHPass(PassRegistry&);
 void initializeDCELegacyPassPass(PassRegistry&);
 void initializeDSELegacyPassPass(PassRegistry&);
 void initializeDataFlowSanitizerPass(PassRegistry&);
 void initializeDeadInstEliminationPass(PassRegistry&);
 void initializeDeadMachineInstructionElimPass(PassRegistry&);
+void initializeDebugifyMachineModulePass(PassRegistry &);
 void initializeDelinearizationPass(PassRegistry&);
 void initializeDemandedBitsWrapperPassPass(PassRegistry&);
 void initializeDependenceAnalysisPass(PassRegistry&);
 void initializeDependenceAnalysisWrapperPassPass(PassRegistry&);
 void initializeDetectDeadLanesPass(PassRegistry&);
 void initializeDivRemPairsLegacyPassPass(PassRegistry&);
 void initializeDomOnlyPrinterPass(PassRegistry&);
 void initializeDomOnlyViewerPass(PassRegistry&);

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

 
 #ifndef LLVM_TRANSFORM_UTILS_DEBUGIFY_H
 #define LLVM_TRANSFORM_UTILS_DEBUGIFY_H
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/IR/PassManager.h"
 
+namespace llvm {
+class DIBuilder;
+
+/// Add synthesized debug information to a module.
+///
+/// \param M The module to add debug information to.
+/// \param Functions A range of functions to add debug information to.
+/// \param Banner A prefix string to add to debug/error messages.
+/// \param ApplyToMF A call back that will add debug information to the
+///                  MachineFunction for a Function. If nullptr, then the
+///                  MachineFunction (if any) will not be modified.
+bool applyDebugifyMetadata(
+    Module &M, iterator_range<Module::iterator> Functions, StringRef Banner,
+    std::function<bool(DIBuilder &, Function &)> ApplyToMF);
+} // namespace llvm
+
 llvm::ModulePass *createDebugifyModulePass();
 llvm::FunctionPass *createDebugifyFunctionPass();
 
 struct NewPMDebugifyPass : public llvm::PassInfoMixin<NewPMDebugifyPass> {
   llvm::PreservedAnalyses run(llvm::Module &M, llvm::ModuleAnalysisManager &AM);
 };
 
 /// Track how much `debugify` information has been lost.

llvm/lib/CodeGen/CMakeLists.txt

add_llvm_component_library(LLVMCodeGen
   LowerEmuTLS.cpp
   MachineBasicBlock.cpp
   MachineBlockFrequencyInfo.cpp
   MachineBlockPlacement.cpp
   MachineBranchProbabilityInfo.cpp
   MachineCombiner.cpp
   MachineCopyPropagation.cpp
   MachineCSE.cpp
+  MachineDebugify.cpp
   MachineDominanceFrontier.cpp
   MachineDominators.cpp
   MachineFrameInfo.cpp
   MachineFunction.cpp
   MachineFunctionPass.cpp
   MachineFunctionPrinterPass.cpp
   MachineInstrBundle.cpp
   MachineInstr.cpp

llvm/lib/CodeGen/CodeGen.cpp

void llvm::initializeCodeGen(PassRegistry &Registry) {
   initializeAtomicExpandPass(Registry);
   initializeBBSectionsPreparePass(Registry);
   initializeBranchFolderPassPass(Registry);
   initializeBranchRelaxationPass(Registry);
   initializeCFGuardLongjmpPass(Registry);
   initializeCFIInstrInserterPass(Registry);
   initializeCodeGenPreparePass(Registry);
   initializeDeadMachineInstructionElimPass(Registry);
+  initializeDebugifyMachineModulePass(Registry);
   initializeDetectDeadLanesPass(Registry);
   initializeDwarfEHPreparePass(Registry);
   initializeEarlyIfConverterPass(Registry);
   initializeEarlyIfPredicatorPass(Registry);
   initializeEarlyMachineLICMPass(Registry);
   initializeEarlyTailDuplicatePass(Registry);
   initializeExpandMemCmpPassPass(Registry);
   initializeExpandPostRAPass(Registry);

llvm/lib/CodeGen/MachineDebugify.cpp

+//===- MachineDebugify.cpp - Attach synthetic debug info to everything ----===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file This pass attaches synthetic debug info to everything. It can be used
+/// to create targeted tests for debug info preservation.
+///
+/// This isn't intended to have feature parity with Debugify.
+//===----------------------------------------------------------------------===//
+
+#include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/CodeGen/MachineModuleInfo.h"
+#include "llvm/CodeGen/Passes.h"
+#include "llvm/IR/DIBuilder.h"
+#include "llvm/IR/DebugInfo.h"
+#include "llvm/InitializePasses.h"
+#include "llvm/Transforms/Utils/Debugify.h"
+
+#define DEBUG_TYPE "mir-debugify"
+
+using namespace llvm;
+
+namespace {
+bool applyDebugifyMetadataToMachineFunction(MachineModuleInfo &MMI,
+                                            DIBuilder &DIB, Function &F) {
+  MachineFunction &MF = MMI.getOrCreateMachineFunction(F);
+
+  DISubprogram *SP = F.getSubprogram();
+  assert(SP && "IR Debugify just created it?");
+
+  LLVMContext &Ctx = F.getParent()->getContext();
+  unsigned NextLine = SP->getLine();
+
+  for (MachineBasicBlock &MBB : MF) {
+    for (MachineInstr &MI : MBB) {
+      // This will likely emit line numbers beyond the end of the imagined
+      // source function and into subsequent ones. We don't do anything about
+      // that as it doesn't really matter to the compiler where the line is in
+      // the imaginary source code.
+      MI.setDebugLoc(DILocation::get(Ctx, NextLine++, 1, SP));
+    }
+  }
+
+  return true;
+}
+
+/// ModulePass for attaching synthetic debug info to everything, used with the
+/// legacy module pass manager.
+struct DebugifyMachineModule : public ModulePass {
+  bool runOnModule(Module &M) override {
+    MachineModuleInfo &MMI =
+        getAnalysis<MachineModuleInfoWrapperPass>().getMMI();
+    return applyDebugifyMetadata(
+        M, M.functions(),
+        "ModuleDebugify: ", [&](DIBuilder &DIB, Function &F) -> bool {
+          return applyDebugifyMetadataToMachineFunction(MMI, DIB, F);
+        });
+  }
+
+  DebugifyMachineModule() : ModulePass(ID) {}
+
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    AU.addRequired<MachineModuleInfoWrapperPass>();
+    AU.addPreserved<MachineModuleInfoWrapperPass>();
+  }
+
+  static char ID; // Pass identification.
+};
+char DebugifyMachineModule::ID = 0;
+
+} // end anonymous namespace
+
+INITIALIZE_PASS_BEGIN(DebugifyMachineModule, DEBUG_TYPE,
+                      "Machine Debugify Module", false, false)
+INITIALIZE_PASS_END(DebugifyMachineModule, DEBUG_TYPE,
+                    "Machine Debugify Module", false, false)
+
+ModulePass *createDebugifyMachineModulePass() {
+  return new DebugifyMachineModule();
+}

llvm/lib/Transforms/Utils/Debugify.cpp

 /// like (but are in fact not) terminators.
 Instruction *findTerminatingInstruction(BasicBlock &BB) {
   if (auto *I = BB.getTerminatingMustTailCall())
     return I;
   if (auto *I = BB.getTerminatingDeoptimizeCall())
     return I;
   return BB.getTerminator();
 }
+} // end anonymous namespace
 
-bool applyDebugifyMetadata(Module &M,
-                           iterator_range<Module::iterator> Functions,
-                           StringRef Banner) {
+bool llvm::applyDebugifyMetadata(
+    Module &M, iterator_range<Module::iterator> Functions, StringRef Banner,
+    std::function<bool(DIBuilder &DIB, Function &F)> ApplyToMF) {
   // Skip modules with debug info.
   if (M.getNamedMetadata("llvm.dbg.cu")) {
     dbg() << Banner << "Skipping module with debug info\n";
     return false;
   }
 
   DIBuilder DIB(M);
   LLVMContext &Ctx = M.getContext();
for (BasicBlock &BB : F) {
         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,
                                     InsertBefore);
       }
     }
+    if (ApplyToMF)
+      ApplyToMF(DIB, F);
     DIB.finalizeSubprogram(SP);
   }
   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.
   assert(NMD->getNumOperands() == 2 &&
          "llvm.debugify should have exactly 2 operands!");
 
   // Claim that this synthetic debug info is valid.
   StringRef DIVersionKey = "Debug Info Version";
   if (!M.getModuleFlag(DIVersionKey))
     M.addModuleFlag(Module::Warning, DIVersionKey, DEBUG_METADATA_VERSION);
 
   return true;
 }
 
+namespace {
 /// Return true if a mis-sized diagnostic is issued for \p DVI.
 bool diagnoseMisSizedDbgValue(Module &M, DbgValueInst *DVI) {
   // The size of a dbg.value's value operand should match the size of the
   // variable it corresponds to.
   //
   // TODO: This, along with a check for non-null value operands, should be
   // promoted to verifier failures.
   Value *V = DVI->getValue();
bool checkDebugifyMetadata(Module &M,
 
   return false;
 }
 
 /// 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 applyDebugifyMetadata(M, M.functions(), "ModuleDebugify: ");
+    return applyDebugifyMetadata(M, M.functions(),
+                                 "ModuleDebugify: ", /*ApplyToMF*/ nullptr);
   }
 
   DebugifyModulePass() : ModulePass(ID) {}
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.setPreservesAll();
   }
 
   static char ID; // Pass identification.
 };
 
 /// FunctionPass for attaching synthetic debug info to instructions within a
 /// single function, used with the legacy module pass manager.
 struct DebugifyFunctionPass : public FunctionPass {
   bool runOnFunction(Function &F) override {
     Module &M = *F.getParent();
     auto FuncIt = F.getIterator();
     return applyDebugifyMetadata(M, make_range(FuncIt, std::next(FuncIt)),
-                                 "FunctionDebugify: ");
+                                 "FunctionDebugify: ", /*ApplyToMF*/ nullptr);
   }
 
   DebugifyFunctionPass() : FunctionPass(ID) {}
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.setPreservesAll();
   }
 

 
 ModulePass *createDebugifyModulePass() { return new DebugifyModulePass(); }
 
 FunctionPass *createDebugifyFunctionPass() {
   return new DebugifyFunctionPass();
 }
 
 PreservedAnalyses NewPMDebugifyPass::run(Module &M, ModuleAnalysisManager &) {
-  applyDebugifyMetadata(M, M.functions(), "ModuleDebugify: ");
+  applyDebugifyMetadata(M, M.functions(),
+                        "ModuleDebugify: ", /*ApplyToMF*/ nullptr);
   return PreservedAnalyses::all();
 }
 
 ModulePass *createCheckDebugifyModulePass(bool Strip,
                                           StringRef NameOfWrappedPass,
                                           DebugifyStatsMap *StatsMap) {
   return new CheckDebugifyModulePass(Strip, NameOfWrappedPass, StatsMap);
 }

llvm/test/CodeGen/Generic/MIRDebugify/locations.mir

+# RUN: llc -run-pass=mir-debugify -o - %s | FileCheck --check-prefixes=ALL,VALUE %s
+# RUN: llc -run-pass=mir-debugify -debugify-level=locations -o - %s | FileCheck --check-prefixes=ALL --implicit-check-not=dbg.value %s
+--- |
+  ; ModuleID = 'loc-only.ll'
+  source_filename = "loc-only.ll"
+  
+  ; ALL-LABEL: @test
+  define i32 @test(i32 %a, i32 %b) {
+    %add = add i32 %a, 2
+  ; ALL-NEXT:  %add = add i32 %a, 2, !dbg [[L1:![0-9]+]]
+  ; VALUE-NEXT: call void @llvm.dbg.value(metadata i32 %add, metadata [[add:![0-9]+]], metadata !DIExpression()), !dbg [[L1]]
+    %sub = sub i32 %add, %b
+  ; ALL-NEXT: %sub = sub i32 %add, %b, !dbg [[L2:![0-9]+]]
+  ; VALUE-NEXT: call void @llvm.dbg.value(metadata i32 %sub, metadata [[sub:![0-9]+]], metadata !DIExpression()), !dbg [[L2]]
+  ; ALL-NEXT: ret i32 %sub, !dbg [[L3:![0-9]+]]
+    ret i32 %sub
+  }
+
+...
+---
+name:            test
+body:             |
+  bb.1 (%ir-block.0):
+    %0:_(s32) = IMPLICIT_DEF
+    %1:_(s32) = IMPLICIT_DEF
+    %2:_(s32) = G_CONSTANT i32 2
+    %3:_(s32) = G_ADD %0, %2
+    %4:_(s32) = G_SUB %3, %1
+    ; There's no attempt to have the locations make sense as it's an imaginary
+    ; source file anyway. These first three coincide with IR-level information
+    ; and therefore use metadata references.
+    ; ALL: %0:_(s32) = IMPLICIT_DEF debug-location [[L1]]
+    ; ALL: %1:_(s32) = IMPLICIT_DEF debug-location [[L2]]
+    ; ALL: %2:_(s32) = G_CONSTANT i32 2, debug-location [[L3]]
+    ; ALL: %3:_(s32) = G_ADD %0, %2, debug-location !DILocation(line: 4, column: 1, scope: !6)
+    ; ALL: %4:_(s32) = G_SUB %3, %1, debug-location !DILocation(line: 5, column: 1, scope: !6)
+...