[InstrRef][FastISel] Emit DBG_INSTR_REF from fast-isel when using instruction-referencing

[jmorse]

Summary

If you attach attribute((optnone)) to a function when using optimisations, that function will use fast-isel instead of the usual SelectionDAG method. This is a problem for instruction referencing, because it means DBG_VALUEs of virtual registers will be created, triggering some safety assertions in LiveDebugVariables. Those assertions exist to detect exactly this scenario, where an unexpected piece of code is generating virtual register references in instruction referencing mode.

Fix this by transforming the DBG_VALUEs created by fast-isel into half-formed DBG_INSTR_REFs, after which they get patched up in finalizeDebugInstrRefs. The test modified adds a fast-isel mode to the instruction referencing isel test.

[jmorse]

Uh, actually the dbg.declare scenario isn't covered, I'll revise a test for that in...

[jmorse]

Add test for dbg.declares. Prepend a deref on the DIExpression of DBG_INSTR_REFs that correspond to dbg.declares.

[StephenTozer]

The change looks simple enough, and the test coverage is good: LGTM.

llvm/lib/CodeGen/SelectionDAG/FastISel.cpp

if (!Op && !Address->use_empty() && isa<Instruction>(Address) &&
       Op = MachineOperand::CreateReg(FuncInfo.InitializeRegForValue(Address),
                                      false);
 
     if (Op) {
       assert(DI->getVariable()->isValidLocationForIntrinsic(DbgLoc) &&
              "Expected inlined-at fields to agree");
       // A dbg.declare describes the address of a source variable, so lower it
       // into an indirect DBG_VALUE.
+      auto Builder =
           BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
-              TII.get(TargetOpcode::DBG_VALUE), /*IsIndirect*/ true,
-              *Op, DI->getVariable(), DI->getExpression());
+                  TII.get(TargetOpcode::DBG_VALUE), /*IsIndirect*/ true, *Op,
+                  DI->getVariable(), DI->getExpression());
+
+      // If using instruction referencing, mutate this into a DBG_INSTR_REF,
+      // to be later patched up by finalizeDebugInstrRefs.
+      if (TM.Options.ValueTrackingVariableLocations && Op->isReg()) {
+        Builder->setDesc(TII.get(TargetOpcode::DBG_INSTR_REF));
+        Builder->getOperand(1).ChangeToImmediate(0);
+      }
     } else {
       // We can't yet handle anything else here because it would require
       // generating code, thus altering codegen because of debug info.
       LLVM_DEBUG(dbgs() << "Dropping debug info for " << *DI
                         << " (no materialized reg for address)\n");
     }
     return true;
   }
if (!V || isa<UndefValue>(V) || DI->hasArgList()) {
       BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, II)
           .addFPImm(CF)
           .addImm(0U)
           .addMetadata(DI->getVariable())
           .addMetadata(DI->getExpression());
     } else if (Register Reg = lookUpRegForValue(V)) {
       // FIXME: This does not handle register-indirect values at offset 0.
       bool IsIndirect = false;
+      auto Builder =
           BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, II, IsIndirect, Reg,
                   DI->getVariable(), DI->getExpression());
+
+      // If using instruction referencing, mutate this into a DBG_INSTR_REF,
+      // to be later patched up by finalizeDebugInstrRefs.
+      if (TM.Options.ValueTrackingVariableLocations) {
+        Builder->setDesc(TII.get(TargetOpcode::DBG_INSTR_REF));
+        Builder->getOperand(1).ChangeToImmediate(0);
+      }
     } else {
       // We don't know how to handle other cases, so we drop.
       LLVM_DEBUG(dbgs() << "Dropping debug info for " << *DI << "\n");
     }
     return true;
   }
   case Intrinsic::dbg_label: {
     const DbgLabelInst *DI = cast<DbgLabelInst>(II);

llvm/test/DebugInfo/X86/instr-ref-selectiondag.ll

 ; RUN: llc %s -mtriple=x86_64-unknown-unknown -o - -stop-before=finalize-isel \
 ; RUN:   | FileCheck %s --check-prefix=NORMAL \
 ; RUN:     --implicit-check-not=debug-instr-number \
 ; RUN:     --implicit-check-not=DBG_INSTR_REF
 ; RUN: llc %s -mtriple=x86_64-unknown-unknown -o - -stop-before=finalize-isel \
 ; RUN:     -experimental-debug-variable-locations -verify-machineinstrs \
 ; RUN:   | FileCheck %s --check-prefix=INSTRREF \
 ; RUN:     --implicit-check-not=DBG_VALUE
+; RUN: llc %s -mtriple=x86_64-unknown-unknown -o - -stop-before=finalize-isel \
+; RUN:     -experimental-debug-variable-locations -verify-machineinstrs \
+; RUN:     -fast-isel \
+; RUN:   | FileCheck %s --check-prefix=FASTISEL-INSTRREF \
+; RUN:     --implicit-check-not=DBG_VALUE
+
 
 ; Test that SelectionDAG produces DBG_VALUEs normally, but DBG_INSTR_REFs when
 ; asked.
 
 ; NORMAL-LABEL: name: foo
 
 ; NORMAL:      %[[REG0:[0-9]+]]:gr32 = ADD32rr
 ; NORMAL-NEXT: DBG_VALUE %[[REG0]]
 ; NORMAL-NEXT: %[[REG1:[0-9]+]]:gr32 = ADD32rr
 ; NORMAL-NEXT: DBG_VALUE %[[REG1]]
 
 ; Note that I'm baking in an assumption of one-based ordering here. We could
 ; capture and check for the instruction numbers, we'd rely on machine verifier
 ; ensuring there were no duplicates.
 
 ; INSTRREF-LABEL: name: foo
 
 ; INSTRREF:      ADD32rr
 ; INSTRREF-SAME: debug-instr-number 1
 ; INSTRREF-NEXT: DBG_INSTR_REF 1, 0
 ; INSTRREF-NEXT: ADD32rr
 ; INSTRREF-SAME: debug-instr-number 2
 ; INSTRREF-NEXT: DBG_INSTR_REF 2, 0
 
+; Test that fast-isel will produce DBG_INSTR_REFs too.
+
+; FASTISEL-INSTRREF-LABEL: name: foo
+
+; FASTISEL-INSTRREF:      ADD32rr
+; FASTISEL-INSTRREF-SAME: debug-instr-number 1
+; FASTISEL-INSTRREF-NEXT: DBG_INSTR_REF 1, 0
+; FASTISEL-INSTRREF-NEXT: ADD32rr
+; FASTISEL-INSTRREF-SAME: debug-instr-number 2
+; FASTISEL-INSTRREF-NEXT: DBG_INSTR_REF 2, 0
+
+
+
 @glob32 = global i32 0
 @glob16 = global i16 0
 @glob8 = global i8 0
 
 declare void @llvm.dbg.value(metadata, metadata, metadata)
 
 define i32 @foo(i32 %bar, i32 %baz, i32 %qux) !dbg !7 {
 entry:

 
 ;; Don't test the location of these instr-refs, only that the three non-argument
 ;; dbg.values become DBG_INSTR_REFs. We previously checked that these numbers
 ;; get substituted, with appropriate subregister qualifiers.
 ; INSTRREF:      DBG_INSTR_REF 2, 0
 ; INSTRREF:      DBG_INSTR_REF 4, 0
 ; INSTRREF:      DBG_INSTR_REF 6, 0
 
+;; In fast-isel, we get four DBG_INSTR_REFs (compared to three and one
+;; DBG_VALUE with normal isel). We get additional substitutions as a result:
+
+; FASTISEL-INSTRREF:      debugValueSubstitutions:
+; FASTISEL-INSTRREF-NEXT: - { srcinst: 3, srcop: 0, dstinst: 2, dstop: 0, subreg: 6 }
+; FASTISEL-INSTRREF-NEXT: - { srcinst: 5, srcop: 0, dstinst: 4, dstop: 0, subreg: 6 }
+; FASTISEL-INSTRREF-NEXT: - { srcinst: 6, srcop: 0, dstinst: 5, dstop: 0, subreg: 4 }
+; FASTISEL-INSTRREF-NEXT  - { srcinst: 8, srcop: 0, dstinst: 7, dstop: 0, subreg: 6 }
+; FASTISEL-INSTRREF-NEXT  - { srcinst: 9, srcop: 0, dstinst: 8, dstop: 0, subreg: 4 }
+; FASTISEL-INSTRREF-NEXT  - { srcinst: 10, srcop: 0, dstinst: 9, dstop: 0, subreg: 1 }
+
+;; Those substitutions are anchored against these DBG_PHIs:
+
+; FASTISEL-INSTRREF:      DBG_PHI $rdi, 7
+; FASTISEL-INSTRREF-NEXT: DBG_PHI $rdi, 4
+; FASTISEL-INSTRREF-NEXT: DBG_PHI $rdi, 2
+; FASTISEL-INSTRREF-NEXT: DBG_PHI $rdi, 1
+
+; FASTISEL-INSTRREF:      DBG_INSTR_REF 1, 0
+; FASTISEL-INSTRREF:      DBG_INSTR_REF 3, 0
+; FASTISEL-INSTRREF:      DBG_INSTR_REF 6, 0
+; FASTISEL-INSTRREF:      DBG_INSTR_REF 10, 0
+
 define i32 @bar(i64 %bar) !dbg !20 {
 entry:
   call void @llvm.dbg.value(metadata i64 %bar, metadata !21, metadata !DIExpression()), !dbg !22
   %0 = trunc i64 %bar to i32, !dbg !22
   call void @llvm.dbg.value(metadata i32 %0, metadata !21, metadata !DIExpression()), !dbg !22
   store i32 %0, i32 *@glob32, !dbg !22
   %1 = trunc i32 %0 to i16, !dbg !22
   call void @llvm.dbg.value(metadata i16 %1, metadata !21, metadata !DIExpression()), !dbg !22

 ; NORMAL-LABEL: bb.2.shoes:
 ; NORMAL:      %1:gr16 = COPY %0.sub_16bit
 
 ; INSTRREF-LABEL: name: baz
 
 ; INSTRREF:      debugValueSubstitutions:
 ; INSTRREF-NEXT:  - { srcinst: 2, srcop: 0, dstinst: 1, dstop: 6, subreg: 4 }
 
-; INSTRREF:      CALL64pcrel32 target-flags(x86-plt) @xyzzy, {{.*}} debug-instr-number 1
+; INSTRREF:      CALL64pcrel32 target-flags(x86-plt) @xyzzy, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax, debug-instr-number 1
 ; INSTRREF:      DBG_INSTR_REF 2, 0
 
+;; Fast-isel produces the same arrangement, a DBG_INSTR_REF pointing back to
+;; the call instruction. However: the operand numbers are different (6 for
+;; normal isel, 4 for fast-isel). This isn't because of debug-info differences,
+;; it's because normal isel implicit-defs the stack registers, and fast-isel
+;; does not. The meaning is the same.
+
+; FASTISEL-INSTRREF-LABEL: name: baz
+
+; FASTISEL-INSTRREF:      debugValueSubstitutions:
+; FASTISEL-INSTRREF-NEXT:  - { srcinst: 2, srcop: 0, dstinst: 1, dstop: 4, subreg: 4 }
+
+; FASTISEL-INSTRREF:      CALL64pcrel32 target-flags(x86-plt) @xyzzy, csr_64, implicit $rsp, implicit $ssp, implicit-def $rax, debug-instr-number 1
+; FASTISEL-INSTRREF:      DBG_INSTR_REF 2, 0
+
 declare i64 @xyzzy()
 
 define i32 @baz() !dbg !30 {
 entry:
   %foo = call i64 @xyzzy(), !dbg !32
   br label %shoes
 
 slippers: