[DebugInstrRef] Parse debug instruction-references from/to MIR
authorJeremy Morse <jeremy.morse@sony.com>
Wed, 14 Oct 2020 09:47:44 +0000 (10:47 +0100)
committerJeremy Morse <jeremy.morse@sony.com>
Wed, 14 Oct 2020 09:57:09 +0000 (10:57 +0100)
This patch defines the 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. The instruction number is stored directly in a MachineInstr.

Actually referring to an instruction comes in a later patch, but is done
using one of these instruction numbers.

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.

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

12 files changed:
llvm/include/llvm/CodeGen/MachineFunction.h
llvm/include/llvm/CodeGen/MachineInstr.h
llvm/lib/CodeGen/MIRParser/MILexer.cpp
llvm/lib/CodeGen/MIRParser/MILexer.h
llvm/lib/CodeGen/MIRParser/MIParser.cpp
llvm/lib/CodeGen/MIRParser/MIRParser.cpp
llvm/lib/CodeGen/MIRPrinter.cpp
llvm/lib/CodeGen/MachineFunction.cpp
llvm/lib/CodeGen/MachineVerifier.cpp
llvm/test/DebugInfo/MIR/InstrRef/instr-ref-roundtrip.mir [new file with mode: 0644]
llvm/test/DebugInfo/MIR/InstrRef/no-duplicates.mir [new file with mode: 0644]
llvm/test/DebugInfo/MIR/InstrRef/no-metainstrs.mir [new file with mode: 0644]

index c2c03b4..1539a24 100644 (file)
@@ -436,6 +436,10 @@ public:
   /// 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);
index 957ec21..582f826 100644 (file)
@@ -456,6 +456,10 @@ public:
   /// 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
index 2ee3ee0..706cf76 100644 (file)
@@ -217,6 +217,7 @@ static MIToken::TokenKind getIdentifierKind(StringRef Identifier) {
       .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)
index ef16da9..fa418cd 100644 (file)
@@ -74,6 +74,7 @@ struct MIToken {
     kw_exact,
     kw_nofpexcept,
     kw_debug_location,
+    kw_debug_instr_number,
     kw_cfi_same_value,
     kw_cfi_offset,
     kw_cfi_rel_offset,
index ded31cd..5ba896b 100644 (file)
@@ -984,6 +984,7 @@ bool MIParser::parse(MachineInstr *&MI) {
          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;
@@ -1014,6 +1015,19 @@ bool MIParser::parse(MachineInstr *&MI) {
     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();
@@ -1070,6 +1084,8 @@ bool MIParser::parse(MachineInstr *&MI) {
     MI->setHeapAllocMarker(MF, HeapAllocMarker);
   if (!MemOperands.empty())
     MI->setMemRefs(MF, MemOperands);
+  if (InstrNum)
+    MI->setDebugInstrNum(InstrNum);
   return false;
 }
 
index 030c3d3..2250c09 100644 (file)
@@ -161,6 +161,9 @@ private:
                                        SMRange SourceRange);
 
   void computeFunctionProperties(MachineFunction &MF);
+
+  void setupDebugValueTracking(MachineFunction &MF,
+    PerFunctionMIParsingState &PFS, const yaml::MachineFunction &YamlMF);
 };
 
 } // end namespace llvm
@@ -402,6 +405,18 @@ bool MIRParserImpl::initializeCallSiteInfo(
   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) {
@@ -510,6 +525,8 @@ MIRParserImpl::initializeMachineFunction(const yaml::MachineFunction &YamlMF,
   if (initializeCallSiteInfo(PFS, YamlMF))
     return false;
 
+  setupDebugValueTracking(MF, PFS, YamlMF);
+
   MF.getSubtarget().mirFileLoaded(MF);
 
   MF.verify();
index dde0dc4..420a346 100644 (file)
@@ -770,6 +770,13 @@ void MIPrinter::print(const MachineInstr &MI) {
     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)
index 56c1309..f9fc775 100644 (file)
@@ -943,6 +943,10 @@ void MachineFunction::moveCallSiteInfo(const MachineInstr *Old,
   CallSitesInfo[New] = CSInfo;
 }
 
+void MachineFunction::setDebugInstrNumberingCount(unsigned Num) {
+  DebugInstrNumberingCount = Num;
+}
+
 /// \}
 
 //===----------------------------------------------------------------------===//
index 31c6ab8..5c22673 100644 (file)
@@ -1556,6 +1556,11 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
     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())
@@ -2519,6 +2524,21 @@ void MachineVerifier::visitMachineFunctionAfter() {
   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() {
diff --git a/llvm/test/DebugInfo/MIR/InstrRef/instr-ref-roundtrip.mir b/llvm/test/DebugInfo/MIR/InstrRef/instr-ref-roundtrip.mir
new file mode 100644 (file)
index 0000000..2389a95
--- /dev/null
@@ -0,0 +1,16 @@
+# 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
+...
diff --git a/llvm/test/DebugInfo/MIR/InstrRef/no-duplicates.mir b/llvm/test/DebugInfo/MIR/InstrRef/no-duplicates.mir
new file mode 100644 (file)
index 0000000..da8da44
--- /dev/null
@@ -0,0 +1,38 @@
+# 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
+...
diff --git a/llvm/test/DebugInfo/MIR/InstrRef/no-metainstrs.mir b/llvm/test/DebugInfo/MIR/InstrRef/no-metainstrs.mir
new file mode 100644 (file)
index 0000000..6fb94f9
--- /dev/null
@@ -0,0 +1,39 @@
+# 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
+...