[DebugInstrRef][2/9] Parse debug instruction-references from/to MIR

[jmorse]

Summary

This patch defines a MIR format for debug instruction references: it's an integer trailing an instruction, marked out by "debug-instr-number", much like how "debug-location" identifies the DebugLoc metadata of an instruction. I'm not wed to this name, or particularly to this syntax, but something is needed.

I've added a round-trip test and two verifier checks: that we don't label meta-instructions as generating values, and that there are no duplicates.

llvm/include/llvm/CodeGen/MachineFunction.h

public:
   using VariableDbgInfoMapTy = SmallVector<VariableDbgInfo, 4>;
   VariableDbgInfoMapTy VariableDbgInfos;
 
   /// A count of how many instructions in the function have had numbers
   /// assigned to them. Used for debug value tracking, to determine the
   /// next instruction number.
   unsigned DebugInstrNumberingCount = 0;
 
+  /// Set value of DebugInstrNumberingCount field. Avoid using this unless
+  /// you're deserializing this data.
+  void setDebugInstrNumberingCount(unsigned Num);
+
   MachineFunction(Function &F, const LLVMTargetMachine &Target,
                   const TargetSubtargetInfo &STI, unsigned FunctionNum,
                   MachineModuleInfo &MMI);
   MachineFunction(const MachineFunction &) = delete;
   MachineFunction &operator=(const MachineFunction &) = delete;
   ~MachineFunction();
 
   /// Reset the instance as if it was just created.

llvm/include/llvm/CodeGen/MachineInstr.h

public:
   /// Fetch the instruction number of this MachineInstr. If it does not have
   /// one already, a new and unique number will be assigned.
   unsigned getDebugInstrNum();
 
   /// Examine the instruction number of this MachineInstr. May be zero if
   /// it hasn't been assigned a number yet.
   unsigned peekDebugInstrNum() const { return DebugInstrNum; }
 
+  /// Set instruction number of this MachineInstr. Avoid using unless you're
+  /// deserializing this information.
+  void setDebugInstrNum(unsigned Num) { DebugInstrNum = Num; }
+
   /// Emit an error referring to the source location of this instruction.
   /// This should only be used for inline assembly that is somehow
   /// impossible to compile. Other errors should have been handled much
   /// earlier.
   ///
   /// If this method returns, the caller should try to recover from the error.
   void emitError(StringRef Msg) const;
 

llvm/lib/CodeGen/MIRParser/MILexer.h

enum TokenKind {
     kw_contract,
     kw_afn,
     kw_reassoc,
     kw_nuw,
     kw_nsw,
     kw_exact,
     kw_nofpexcept,
     kw_debug_location,
+    kw_debug_instr_number,
     kw_cfi_same_value,
     kw_cfi_offset,
     kw_cfi_rel_offset,
     kw_cfi_def_cfa_register,
     kw_cfi_def_cfa_offset,
     kw_cfi_adjust_cfa_offset,
     kw_cfi_escape,
     kw_cfi_def_cfa,

llvm/lib/CodeGen/MIRParser/MILexer.cpp

return StringSwitch<MIToken::TokenKind>(Identifier)
       .Case("contract", MIToken::kw_contract)
       .Case("afn", MIToken::kw_afn)
       .Case("reassoc", MIToken::kw_reassoc)
       .Case("nuw", MIToken::kw_nuw)
       .Case("nsw", MIToken::kw_nsw)
       .Case("exact", MIToken::kw_exact)
       .Case("nofpexcept", MIToken::kw_nofpexcept)
       .Case("debug-location", MIToken::kw_debug_location)
+      .Case("debug-instr-number", MIToken::kw_debug_instr_number)
       .Case("same_value", MIToken::kw_cfi_same_value)
       .Case("offset", MIToken::kw_cfi_offset)
       .Case("rel_offset", MIToken::kw_cfi_rel_offset)
       .Case("def_cfa_register", MIToken::kw_cfi_def_cfa_register)
       .Case("def_cfa_offset", MIToken::kw_cfi_def_cfa_offset)
       .Case("adjust_cfa_offset", MIToken::kw_cfi_adjust_cfa_offset)
       .Case("escape", MIToken::kw_cfi_escape)
       .Case("def_cfa", MIToken::kw_cfi_def_cfa)

llvm/lib/CodeGen/MIRParser/MIParser.cpp

bool MIParser::parse(MachineInstr *&MI) {
   if (Token.isError() || parseInstruction(OpCode, Flags))
     return true;
 
   // Parse the remaining machine operands.
   while (!Token.isNewlineOrEOF() && Token.isNot(MIToken::kw_pre_instr_symbol) &&
          Token.isNot(MIToken::kw_post_instr_symbol) &&
          Token.isNot(MIToken::kw_heap_alloc_marker) &&
          Token.isNot(MIToken::kw_debug_location) &&
+         Token.isNot(MIToken::kw_debug_instr_number) &&
          Token.isNot(MIToken::coloncolon) && Token.isNot(MIToken::lbrace)) {
     auto Loc = Token.location();
     Optional<unsigned> TiedDefIdx;
     if (parseMachineOperandAndTargetFlags(OpCode, Operands.size(), MO, TiedDefIdx))
       return true;
     if (OpCode == TargetOpcode::DBG_VALUE && MO.isReg())
       MO.setIsDebug();
     Operands.push_back(
bool MIParser::parse(MachineInstr *&MI) {
   if (Token.is(MIToken::kw_post_instr_symbol))
     if (parsePreOrPostInstrSymbol(PostInstrSymbol))
       return true;
   MDNode *HeapAllocMarker = nullptr;
   if (Token.is(MIToken::kw_heap_alloc_marker))
     if (parseHeapAllocMarker(HeapAllocMarker))
       return true;
 
+  unsigned InstrNum = 0;
+  if (Token.is(MIToken::kw_debug_instr_number)) {
+    lex();
+    if (Token.isNot(MIToken::IntegerLiteral))
+      return error("expected an integer literal after 'debug-instr-number'");
+    if (getUnsigned(InstrNum))
+      return true;
+    lex();
+    // Lex past trailing comma if present.
+    if (Token.is(MIToken::comma))
+      lex();
+  }
+
   DebugLoc DebugLocation;
   if (Token.is(MIToken::kw_debug_location)) {
     lex();
     MDNode *Node = nullptr;
     if (Token.is(MIToken::exclaim)) {
       if (parseMDNode(Node))
         return true;
     } else if (Token.is(MIToken::md_dilocation)) {
bool MIParser::parse(MachineInstr *&MI) {
   if (PreInstrSymbol)
     MI->setPreInstrSymbol(MF, PreInstrSymbol);
   if (PostInstrSymbol)
     MI->setPostInstrSymbol(MF, PostInstrSymbol);
   if (HeapAllocMarker)
     MI->setHeapAllocMarker(MF, HeapAllocMarker);
   if (!MemOperands.empty())
     MI->setMemRefs(MF, MemOperands);
+  if (InstrNum)
+    MI->setDebugInstrNum(InstrNum);
   return false;
 }
 
 bool MIParser::parseStandaloneMBB(MachineBasicBlock *&MBB) {
   lex();
   if (Token.isNot(MIToken::MachineBasicBlock))
     return error("expected a machine basic block reference");
   if (parseMBBReference(MBB))

llvm/lib/CodeGen/MIRParser/MIRParser.cpp

SMDiagnostic diagFromMIStringDiag(const SMDiagnostic &Error,
                                     SMRange SourceRange);
 
   /// Return a MIR diagnostic converted from a diagnostic located in a YAML
   /// block scalar string.
   SMDiagnostic diagFromBlockStringDiag(const SMDiagnostic &Error,
                                        SMRange SourceRange);
 
   void computeFunctionProperties(MachineFunction &MF);
+
+  void setupDebugValueTracking(MachineFunction &MF,
+    PerFunctionMIParsingState &PFS, const yaml::MachineFunction &YamlMF);
 };
 
 } // end namespace llvm
 
 static void handleYAMLDiag(const SMDiagnostic &Diag, void *Context) {
   reinterpret_cast<MIRParserImpl *>(Context)->reportDiagnostic(Diag);
 }
 
if (TM.Options.EmitCallSiteInfo)
       MF.addCallArgsForwardingRegs(&*CallI, std::move(CSInfo));
   }
 
   if (YamlMF.CallSitesInfo.size() && !TM.Options.EmitCallSiteInfo)
     return error(Twine("Call site info provided but not used"));
   return false;
 }
 
+void MIRParserImpl::setupDebugValueTracking(MachineFunction &MF,
+    PerFunctionMIParsingState &PFS, const yaml::MachineFunction &YamlMF) {
+  // For now, we only compute the value of the "next instruction number"
+  // field.
+  unsigned MaxInstrNum = 0;
+  for (auto &MBB : MF)
+    for (auto &MI : MBB)
+      MaxInstrNum = std::max((unsigned)MI.peekDebugInstrNum(), MaxInstrNum);
+  MF.setDebugInstrNumberingCount(MaxInstrNum);
+}
+
+
 bool
 MIRParserImpl::initializeMachineFunction(const yaml::MachineFunction &YamlMF,
                                          MachineFunction &MF) {
   // TODO: Recreate the machine function.
   if (Target) {
     // Avoid clearing state if we're using the same subtarget again.
     Target->setTarget(MF.getSubtarget());
   } else {
MIRParserImpl::initializeMachineFunction(const yaml::MachineFunction &YamlMF,
   MachineRegisterInfo &MRI = MF.getRegInfo();
   MRI.freezeReservedRegs(MF);
 
   computeFunctionProperties(MF);
 
   if (initializeCallSiteInfo(PFS, YamlMF))
     return false;
 
+  setupDebugValueTracking(MF, PFS, YamlMF);
+
   MF.getSubtarget().mirFileLoaded(MF);
 
   MF.verify();
   return false;
 }
 
 bool MIRParserImpl::parseRegisterInfo(PerFunctionMIParsingState &PFS,
                                       const yaml::MachineFunction &YamlMF) {

llvm/lib/CodeGen/MIRPrinter.cpp

void MIPrinter::print(const MachineInstr &MI) {
   if (MDNode *HeapAllocMarker = MI.getHeapAllocMarker()) {
     if (NeedComma)
       OS << ',';
     OS << " heap-alloc-marker ";
     HeapAllocMarker->printAsOperand(OS, MST);
     NeedComma = true;
   }
 
+  if (auto Num = MI.peekDebugInstrNum()) {
+    if (NeedComma)
+      OS << ',';
+    OS << " debug-instr-number " << Num;
+    NeedComma = true;
+  }
+
   if (PrintLocations) {
     if (const DebugLoc &DL = MI.getDebugLoc()) {
       if (NeedComma)
         OS << ',';
       OS << " debug-location ";
       DL->printAsOperand(OS, MST);
     }
   }

llvm/lib/CodeGen/MachineFunction.cpp

void MachineFunction::moveCallSiteInfo(const MachineInstr *Old,
   if (CSIt == CallSitesInfo.end())
     return;
 
   CallSiteInfo CSInfo = std::move(CSIt->second);
   CallSitesInfo.erase(CSIt);
   CallSitesInfo[New] = CSInfo;
 }
 
+void MachineFunction::setDebugInstrNumberingCount(unsigned Num) {
+  DebugInstrNumberingCount = Num;
+}
+
 /// \}
 
 //===----------------------------------------------------------------------===//
 //  MachineJumpTableInfo implementation
 //===----------------------------------------------------------------------===//
 
 /// Return the size of each entry in the jump table.
 unsigned MachineJumpTableInfo::getEntrySize(const DataLayout &TD) const {

llvm/lib/CodeGen/MachineVerifier.cpp

void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
 
   // A fully-formed DBG_VALUE must have a location. Ignore partially formed
   // DBG_VALUEs: these are convenient to use in tests, but should never get
   // generated.
   if (MI->isDebugValue() && MI->getNumOperands() == 4)
     if (!MI->getDebugLoc())
       report("Missing DebugLoc for debug instruction", MI);
 
+  // Meta instructions should never be the subject of debug value tracking,
+  // they don't create a value in the output program at all.
+  if (MI->isMetaInstruction() && MI->peekDebugInstrNum())
+    report("Metadata instruction should not have a value tracking number", MI);
+
   // Check the MachineMemOperands for basic consistency.
   for (MachineMemOperand *Op : MI->memoperands()) {
     if (Op->isLoad() && !MI->mayLoad())
       report("Missing mayLoad flag", MI);
     if (Op->isStore() && !MI->mayStore())
       report("Missing mayStore flag", MI);
   }
 
for (const auto &MBB : *MF)
                    << printMBBReference(*Pred) << "\n";
           }
         }
       }
 
   for (auto CSInfo : MF->getCallSitesInfo())
     if (!CSInfo.first->isCall())
       report("Call site info referencing instruction that is not call", MF);
+
+  // If there's debug-info, check that we don't have any duplicate value
+  // tracking numbers.
+  if (MF->getFunction().getSubprogram()) {
+    DenseSet<unsigned> SeenNumbers;
+    for (auto &MBB : *MF) {
+      for (auto &MI : MBB) {
+        if (auto Num = MI.peekDebugInstrNum()) {
+          auto Result = SeenNumbers.insert((unsigned)Num);
+          if (!Result.second)
+            report("Instruction has a duplicated value tracking number", &MI);
+        }
+      }
+    }
+  }
 }
 
 void MachineVerifier::verifyLiveVariables() {
   assert(LiveVars && "Don't call verifyLiveVariables without LiveVars");
   for (unsigned i = 0, e = MRI->getNumVirtRegs(); i != e; ++i) {
     unsigned Reg = Register::index2VirtReg(i);
     LiveVariables::VarInfo &VI = LiveVars->getVarInfo(Reg);
     for (const auto &MBB : *MF) {

llvm/test/DebugInfo/MIR/InstrRef/instr-ref-roundtrip.mir

+# RUN: llc %s -march=x86-64 -run-pass=machineverifier -o - -experimental-debug-variable-locations | FileCheck %s
+#
+# CHECK: MOV64rr $rdi, debug-instr-number 1
+---
+name: test
+tracksRegLiveness: true
+liveins:
+  - { reg: '$rdi', virtual-reg: '' }
+body:  |
+  bb.0:
+  liveins: $rdi, $rax
+    $rbp = MOV64rr $rdi, debug-instr-number 1
+    dead $rcx = MOV64ri 0
+    CMP64ri8 renamable $rax, 1, implicit-def $eflags
+    RETQ $rax
+...

llvm/test/DebugInfo/MIR/InstrRef/no-duplicates.mir

+# RUN: not --crash llc %s -march=x86-64 -run-pass=machineverifier -o - 2>&1 | FileCheck %s
+#
+# CHECK: Instruction has a duplicated value tracking number
+--- |
+  define i32 @test(i32 %bar) local_unnamed_addr !dbg !7 {
+  entry:
+    ret i32 0, !dbg !12
+  }
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!3, !4, !5, !6}
+  !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
+  !1 = !DIFile(filename: "foo.cpp", directory: ".")
+  !2 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !3 = !{i32 2, !"Dwarf Version", i32 4}
+  !4 = !{i32 2, !"Debug Info Version", i32 3}
+  !5 = !{i32 1, !"wchar_size", i32 2}
+  !6 = !{i32 7, !"PIC Level", i32 2}
+  !7 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: !1, file: !1, line: 6, type: !8, scopeLine: 6, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !10)
+  !8 = !DISubroutineType(types: !9)
+  !9 = !{!2, !2}
+  !10 = !{!11}
+  !11 = !DILocalVariable(name: "baz", scope: !7, file: !1, line: 7, type: !2)
+  !12 = !DILocation(line: 10, scope: !7)
+...
+---
+name: test
+tracksRegLiveness: true
+liveins:
+  - { reg: '$rdi', virtual-reg: '' }
+body:  |
+  bb.0:
+  liveins: $rdi, $rax
+    $rbp = MOV64rr $rdi, debug-instr-number 1, debug-location !12
+    dead $rcx = MOV64ri 0, debug-instr-number 1, debug-location !12
+    CMP64ri8 renamable $rax, 1, implicit-def $eflags
+    RETQ $rax
+...

llvm/test/DebugInfo/MIR/InstrRef/no-metainstrs.mir

+# RUN: not --crash llc %s -march=x86-64 -run-pass=machineverifier -o - 2>&1 | FileCheck %s
+#
+# CHECK: Metadata instruction should not have a value tracking number
+--- |
+  define i32 @test(i32 %bar) local_unnamed_addr !dbg !7 {
+  entry:
+    ret i32 0, !dbg !12
+  }
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!3, !4, !5, !6}
+  !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
+  !1 = !DIFile(filename: "foo.cpp", directory: ".")
+  !2 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !3 = !{i32 2, !"Dwarf Version", i32 4}
+  !4 = !{i32 2, !"Debug Info Version", i32 3}
+  !5 = !{i32 1, !"wchar_size", i32 2}
+  !6 = !{i32 7, !"PIC Level", i32 2}
+  !7 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: !1, file: !1, line: 6, type: !8, scopeLine: 6, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !10)
+  !8 = !DISubroutineType(types: !9)
+  !9 = !{!2, !2}
+  !10 = !{!11}
+  !11 = !DILocalVariable(name: "baz", scope: !7, file: !1, line: 7, type: !2)
+  !12 = !DILocation(line: 10, scope: !7)
+...
+---
+name: test
+tracksRegLiveness: true
+liveins:
+  - { reg: '$rdi', virtual-reg: '' }
+body:  |
+  bb.0:
+  liveins: $rdi, $rax
+    $rbp = MOV64rr $rdi, debug-instr-number 1, debug-location !12
+    $ebp = KILL killed $rbp, debug-instr-number 2, debug-location !12
+    dead $rcx = MOV64ri 0
+    CMP64ri8 renamable $rax, 1, implicit-def $eflags
+    RETQ $rax
+...