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

Jeremy Morse via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 14 02:58:06 PDT 2020


Author: Jeremy Morse
Date: 2020-10-14T10:57:09+01:00
New Revision: 2c5f3d54c5ee4efdf63736c23a3a7b448a308996

URL: https://github.com/llvm/llvm-project/commit/2c5f3d54c5ee4efdf63736c23a3a7b448a308996
DIFF: https://github.com/llvm/llvm-project/commit/2c5f3d54c5ee4efdf63736c23a3a7b448a308996.diff

LOG: [DebugInstrRef] Parse debug instruction-references from/to MIR

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

Added: 
    llvm/test/DebugInfo/MIR/InstrRef/instr-ref-roundtrip.mir
    llvm/test/DebugInfo/MIR/InstrRef/no-duplicates.mir
    llvm/test/DebugInfo/MIR/InstrRef/no-metainstrs.mir

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/CodeGen/MachineFunction.h b/llvm/include/llvm/CodeGen/MachineFunction.h
index c2c03b4cdbd7..1539a247dbe4 100644
--- a/llvm/include/llvm/CodeGen/MachineFunction.h
+++ b/llvm/include/llvm/CodeGen/MachineFunction.h
@@ -436,6 +436,10 @@ class MachineFunction {
   /// 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);

diff  --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h
index 957ec2124e0a..582f82646d7f 100644
--- a/llvm/include/llvm/CodeGen/MachineInstr.h
+++ b/llvm/include/llvm/CodeGen/MachineInstr.h
@@ -456,6 +456,10 @@ class MachineInstr
   /// 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

diff  --git a/llvm/lib/CodeGen/MIRParser/MILexer.cpp b/llvm/lib/CodeGen/MIRParser/MILexer.cpp
index 2ee3ee0f00f6..706cf767b63b 100644
--- a/llvm/lib/CodeGen/MIRParser/MILexer.cpp
+++ b/llvm/lib/CodeGen/MIRParser/MILexer.cpp
@@ -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)

diff  --git a/llvm/lib/CodeGen/MIRParser/MILexer.h b/llvm/lib/CodeGen/MIRParser/MILexer.h
index ef16da94d21b..fa418cdc8682 100644
--- a/llvm/lib/CodeGen/MIRParser/MILexer.h
+++ b/llvm/lib/CodeGen/MIRParser/MILexer.h
@@ -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,

diff  --git a/llvm/lib/CodeGen/MIRParser/MIParser.cpp b/llvm/lib/CodeGen/MIRParser/MIParser.cpp
index ded31cd08fb5..5ba896b7037c 100644
--- a/llvm/lib/CodeGen/MIRParser/MIParser.cpp
+++ b/llvm/lib/CodeGen/MIRParser/MIParser.cpp
@@ -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;
 }
 

diff  --git a/llvm/lib/CodeGen/MIRParser/MIRParser.cpp b/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
index 030c3d3e23ab..2250c0977ee1 100644
--- a/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
+++ b/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
@@ -161,6 +161,9 @@ class MIRParserImpl {
                                        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();

diff  --git a/llvm/lib/CodeGen/MIRPrinter.cpp b/llvm/lib/CodeGen/MIRPrinter.cpp
index dde0dc456c05..420a346cf67a 100644
--- a/llvm/lib/CodeGen/MIRPrinter.cpp
+++ b/llvm/lib/CodeGen/MIRPrinter.cpp
@@ -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)

diff  --git a/llvm/lib/CodeGen/MachineFunction.cpp b/llvm/lib/CodeGen/MachineFunction.cpp
index 56c13096f745..f9fc775f83bd 100644
--- a/llvm/lib/CodeGen/MachineFunction.cpp
+++ b/llvm/lib/CodeGen/MachineFunction.cpp
@@ -943,6 +943,10 @@ void MachineFunction::moveCallSiteInfo(const MachineInstr *Old,
   CallSitesInfo[New] = CSInfo;
 }
 
+void MachineFunction::setDebugInstrNumberingCount(unsigned Num) {
+  DebugInstrNumberingCount = Num;
+}
+
 /// \}
 
 //===----------------------------------------------------------------------===//

diff  --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 31c6ab8747f2..5c22673d5e1e 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -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
index 000000000000..2389a957cbc3
--- /dev/null
+++ b/llvm/test/DebugInfo/MIR/InstrRef/instr-ref-roundtrip.mir
@@ -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
index 000000000000..da8da44a597c
--- /dev/null
+++ b/llvm/test/DebugInfo/MIR/InstrRef/no-duplicates.mir
@@ -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
index 000000000000..6fb94f97202d
--- /dev/null
+++ b/llvm/test/DebugInfo/MIR/InstrRef/no-metainstrs.mir
@@ -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
+...


        


More information about the llvm-commits mailing list