[LiveDebugValues][InstrRef][2/2] Emit entry value variable locations

This patch adds support to the instruction-referencing LiveDebugValues
implementation for emitting entry values. The instruction referencing
implementations tracking by value rather than location means that we can
get around two of the issues with VarLocs. DBG_VALUE instructions that
re-assign the same value to a variable are no longer a problem, because we
can "see through" to the value being assigned. We also don't need to do
anything special during the dataflow stages: the "variable value problem"
doesn't need to know whether a value is available most of the time, and the
times it deoes need to know are always when entry values need to be
terminated.

The patch modifies the "TransferTracker" class, adding methods to identify
when a variable ias an entry value candidate, and when a machine value is
an entry value. recoverAsEntryValue tests these two things and emits an
entry-value expression if they're true. It's used when we clobber or
otherwise lose a value and can't find a replacement location for the value
it contained.

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

lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

@@ -161,6 +161,7 @@
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachineInstr.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
+#include "llvm/CodeGen/MachineInstrBundle.h"
 #include "llvm/CodeGen/MachineMemOperand.h"
 #include "llvm/CodeGen/MachineOperand.h"
 #include "llvm/CodeGen/PseudoSourceValue.h"
@@ -185,6 +186,7 @@
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/TypeSize.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Target/TargetMachine.h"
 #include "llvm/Transforms/Utils/SSAUpdaterImpl.h"
 #include <algorithm>
 #include <cassert>
@@ -960,18 +962,20 @@ public:
 class TransferTracker {
 public:
   const TargetInstrInfo *TII;
+  const TargetLowering *TLI;
   /// This machine location tracker is assumed to always contain the up-to-date
   /// value mapping for all machine locations. TransferTracker only reads
   /// information from it. (XXX make it const?)
   MLocTracker *MTracker;
   MachineFunction &MF;
+  bool ShouldEmitDebugEntryValues;
 
   /// Record of all changes in variable locations at a block position. Awkwardly
   /// we allow inserting either before or after the point: MBB != nullptr
   /// indicates it's before, otherwise after.
   struct Transfer {
-    MachineBasicBlock::iterator Pos; /// Position to insert DBG_VALUes
-    MachineBasicBlock *MBB;          /// non-null if we should insert after.
+    MachineBasicBlock::instr_iterator Pos; /// Position to insert DBG_VALUes
+    MachineBasicBlock *MBB; /// non-null if we should insert after.
     SmallVector<MachineInstr *, 4> Insts; /// Vector of DBG_VALUEs to insert.
   };
 
@@ -1028,9 +1032,13 @@ public:
 
   TransferTracker(const TargetInstrInfo *TII, MLocTracker *MTracker,
                   MachineFunction &MF, const TargetRegisterInfo &TRI,
-                  const BitVector &CalleeSavedRegs)
+                  const BitVector &CalleeSavedRegs, const TargetPassConfig &TPC)
       : TII(TII), MTracker(MTracker), MF(MF), TRI(TRI),
-        CalleeSavedRegs(CalleeSavedRegs) {}
+        CalleeSavedRegs(CalleeSavedRegs) {
+    TLI = MF.getSubtarget().getTargetLowering();
+    auto &TM = TPC.getTM<TargetMachine>();
+    ShouldEmitDebugEntryValues = TM.Options.ShouldEmitDebugEntryValues();
+  }
 
   /// Load object with live-in variable values. \p mlocs contains the live-in
   /// values in each machine location, while \p vlocs the live-in variable
@@ -1098,6 +1106,8 @@ public:
         // use-before-def to be resolved as we step through the block.
         if (Num.getBlock() == (unsigned)MBB.getNumber() && !Num.isPHI())
           addUseBeforeDef(Var.first, Var.second.Properties, Num);
+        else
+          recoverAsEntryValue(Var.first, Var.second.Properties, Num);
         continue;
       }
 
@@ -1153,10 +1163,73 @@ public:
 
   /// Helper to move created DBG_VALUEs into Transfers collection.
   void flushDbgValues(MachineBasicBlock::iterator Pos, MachineBasicBlock *MBB) {
-    if (PendingDbgValues.size() > 0) {
-      Transfers.push_back({Pos, MBB, PendingDbgValues});
-      PendingDbgValues.clear();
-    }
+    if (PendingDbgValues.size() == 0)
+      return;
+
+    // Pick out the instruction start position.
+    MachineBasicBlock::instr_iterator BundleStart;
+    if (MBB && Pos == MBB->begin())
+      BundleStart = MBB->instr_begin();
+    else
+      BundleStart = getBundleStart(Pos->getIterator());
+
+    Transfers.push_back({BundleStart, MBB, PendingDbgValues});
+    PendingDbgValues.clear();
+  }
+
+  bool isEntryValueVariable(const DebugVariable &Var,
+                            const DIExpression *Expr) const {
+    if (!Var.getVariable()->isParameter())
+      return false;
+
+    if (Var.getInlinedAt())
+      return false;
+
+    if (Expr->getNumElements() > 0)
+      return false;
+
+    return true;
+  }
+
+  bool isEntryValueValue(const ValueIDNum &Val) const {
+    // Must be in entry block (block number zero), and be a PHI / live-in value.
+    if (Val.getBlock() || !Val.isPHI())
+      return false;
+
+    // Entry values must enter in a register.
+    if (MTracker->isSpill(Val.getLoc()))
+      return false;
+
+    Register SP = TLI->getStackPointerRegisterToSaveRestore();
+    Register FP = TRI.getFrameRegister(MF);
+    Register Reg = MTracker->LocIdxToLocID[Val.getLoc()];
+    return Reg != SP && Reg != FP;
+  }
+
+  bool recoverAsEntryValue(const DebugVariable &Var, DbgValueProperties &Prop,
+                           const ValueIDNum &Num) {
+    // Is this variable location a candidate to be an entry value. First,
+    // should we be trying this at all?
+    if (!ShouldEmitDebugEntryValues)
+      return false;
+
+    // Is the variable appropriate for entry values (i.e., is a parameter).
+    if (!isEntryValueVariable(Var, Prop.DIExpr))
+      return false;
+
+    // Is the value assigned to this variable still the entry value?
+    if (!isEntryValueValue(Num))
+      return false;
+
+    // Emit a variable location using an entry value expression.
+    DIExpression *NewExpr =
+        DIExpression::prepend(Prop.DIExpr, DIExpression::EntryValue);
+    Register Reg = MTracker->LocIdxToLocID[Num.getLoc()];
+    MachineOperand MO = MachineOperand::CreateReg(Reg, false);
+    MO.setIsDebug(true);
+
+    PendingDbgValues.push_back(emitMOLoc(MO, Var, {NewExpr, Prop.Indirect}));
+    return true;
   }
 
   /// Change a variable value after encountering a DBG_VALUE inside a block.
@@ -1248,8 +1321,15 @@ public:
 
     // If there is no location, and we weren't asked to make the variable
     // explicitly undef, then stop here.
-    if (!NewLoc && !MakeUndef)
+    if (!NewLoc && !MakeUndef) {
+      // Try and recover a few more locations with entry values.
+      for (auto &Var : ActiveMLocIt->second) {
+        auto &Prop = ActiveVLocs.find(Var)->second.Properties;
+        recoverAsEntryValue(Var, Prop, OldValue);
+      }
+      flushDbgValues(Pos, nullptr);
       return;
+    }
 
     // Examine all the variables based on this location.
     DenseSet<DebugVariable> NewMLocs;
@@ -1603,7 +1683,8 @@ private:
   /// that we can compare explictly against VarLocBasedImpl.
   void emitLocations(MachineFunction &MF, LiveInsT SavedLiveIns,
                      ValueIDNum **MOutLocs, ValueIDNum **MInLocs,
-                     DenseMap<DebugVariable, unsigned> &AllVarsNumbering);
+                     DenseMap<DebugVariable, unsigned> &AllVarsNumbering,
+                     const TargetPassConfig &TPC);
 
   /// Boilerplate computation of some initial sets, artifical blocks and
   /// RPOT block ordering.
@@ -3302,8 +3383,9 @@ void InstrRefBasedLDV::dump_mloc_transfer(
 
 void InstrRefBasedLDV::emitLocations(
     MachineFunction &MF, LiveInsT SavedLiveIns, ValueIDNum **MOutLocs,
-    ValueIDNum **MInLocs, DenseMap<DebugVariable, unsigned> &AllVarsNumbering) {
-  TTracker = new TransferTracker(TII, MTracker, MF, *TRI, CalleeSavedRegs);
+    ValueIDNum **MInLocs, DenseMap<DebugVariable, unsigned> &AllVarsNumbering,
+    const TargetPassConfig &TPC) {
+  TTracker = new TransferTracker(TII, MTracker, MF, *TRI, CalleeSavedRegs, TPC);
   unsigned NumLocs = MTracker->getNumLocs();
 
   // For each block, load in the machine value locations and variable value
@@ -3351,9 +3433,14 @@ void InstrRefBasedLDV::emitLocations(
         MBB.insert(P.Pos, MI);
       }
     } else {
+      // Terminators, like tail calls, can clobber things. Don't try and place
+      // transfers after them.
+      if (P.Pos->isTerminator())
+        continue;
+
       MachineBasicBlock &MBB = *P.Pos->getParent();
       for (auto *MI : P.Insts) {
-        MBB.insertAfter(P.Pos, MI);
+        MBB.insertAfterBundle(P.Pos, MI);
       }
     }
   }
@@ -3520,7 +3607,7 @@ bool InstrRefBasedLDV::ExtendRanges(MachineFunction &MF,
   // Using the computed value locations and variable values for each block,
   // create the DBG_VALUE instructions representing the extended variable
   // locations.
-  emitLocations(MF, SavedLiveIns, MOutLocs, MInLocs, AllVarsNumbering);
+  emitLocations(MF, SavedLiveIns, MOutLocs, MInLocs, AllVarsNumbering, *TPC);
 
   for (int Idx = 0; Idx < MaxNumBlocks; ++Idx) {
     delete[] MOutLocs[Idx];

test/DebugInfo/ARM/entry-value-multi-byte-expr.ll

@@ -1,4 +1,5 @@
 ; RUN: llc -debug-entry-values -filetype=asm -o - %s | FileCheck %s
+; RUN: llc -force-instr-ref-livedebugvalues=1 -debug-entry-values -filetype=asm -o - %s | FileCheck %s
 
 ; Verify that the size operands of the DW_OP_GNU_entry_value operations are
 ; correct for the multi-byte DW_OP_regx expressions.

test/DebugInfo/MIR/Hexagon/live-debug-values-bundled-entry-values.mir

@@ -1,6 +1,7 @@
 # We do not support the call site info for the target now, so we use the experimental option (-emit-call-site-info -debug-entry-values).
 
 # RUN: llc -emit-call-site-info -debug-entry-values -run-pass=livedebugvalues -o - %s | FileCheck %s
+# RUN: llc -emit-call-site-info -debug-entry-values -force-instr-ref-livedebugvalues=1 -run-pass=livedebugvalues -o - %s | FileCheck %s
 
 # Verify that the entry values for the input parameters are inserted after the
 # bundles which contains the registers' clobbering instructions (the calls to

test/DebugInfo/MIR/X86/dbginfo-entryvals.mir

@@ -1,4 +1,5 @@
 # RUN: llc -run-pass=livedebugvalues -verify-machineinstrs -march=x86-64 -o - %s | FileCheck %s
+# RUN: llc -force-instr-ref-livedebugvalues=1 -run-pass=livedebugvalues -verify-machineinstrs -march=x86-64 -o - %s | FileCheck %s
 #
 #extern void fn2(int);
 #

test/DebugInfo/MIR/X86/debug-entry-value-operation.mir

@@ -1,9 +1,15 @@
 # RUN: llc -start-before=livedebugvalues -mtriple=x86_64-apple-darwin -o %t %s -filetype=obj
 # RUN: llvm-dwarfdump %t | FileCheck %s
 
+# RUN: llc -force-instr-ref-livedebugvalues=1 -start-before=livedebugvalues -mtriple=x86_64-apple-darwin -o %t %s -filetype=obj
+# RUN: llvm-dwarfdump %t | FileCheck %s
+
 # RUN: llc -start-before=livedebugvalues -debugger-tune=sce -mtriple=x86_64-sce-ps4 -o %t1 %s -filetype=obj
 # RUN: llvm-dwarfdump %t1 | FileCheck %s -check-prefix=SCE
 
+# RUN: llc -force-instr-ref-livedebugvalues=1 -start-before=livedebugvalues -debugger-tune=sce -mtriple=x86_64-sce-ps4 -o %t1 %s -filetype=obj
+# RUN: llvm-dwarfdump %t1 | FileCheck %s -check-prefix=SCE
+
 ## Based on:
 ## int global;
 ## int foo(int p, int q, int r) {

test/DebugInfo/MIR/X86/entry-value-of-modified-param.mir

@@ -1,4 +1,5 @@
 # RUN: llc -run-pass=livedebugvalues -march=x86-64 -o - %s | FileCheck %s
+# RUN: llc -force-instr-ref-livedebugvalues=1 -run-pass=livedebugvalues -march=x86-64 -o - %s | FileCheck %s
 #
 #extern void fn1 (int, int, int);
 #

test/DebugInfo/MIR/X86/entry-values-diamond-bbs.mir

@@ -1,4 +1,5 @@
 # RUN: llc -run-pass=livedebugvalues -march=x86-64 -o - %s | FileCheck %s
+# RUN: llc -force-instr-ref-livedebugvalues=1 -run-pass=livedebugvalues -march=x86-64 -o - %s | FileCheck %s
 #
 # The test case was artificially adjusted, in order to make proper diamond basic
 # block structure relevant to the debug entry values propagation.

test/DebugInfo/MIR/X86/kill-entry-value-after-diamond-bbs.mir

@@ -1,4 +1,5 @@
-# RUN: llc -debug-entry-values -run-pass=livedebugvalues -march=x86-64 -o - %s | FileCheck %s
+# RUN: llc -debug-entry-values -run-pass=livedebugvalues -march=x86-64 -o - %s | FileCheck %s --check-prefixes=CHECK,VARLOCLDV
+# RUN: llc -debug-entry-values -force-instr-ref-livedebugvalues=1 -run-pass=livedebugvalues -march=x86-64 -o - %s | FileCheck %s --check-prefixes=CHECK,INSTRREFLDV
 #
 # The test case was artificially adjusted, in order to make proper diamond basic
 # block structure relevant to the debug entry values clobbering.
@@ -21,7 +22,12 @@
 # CHECK-NEXT: $ebp = MOV32ri 2
 # CHECK-NEXT: DBG_VALUE $esi, $noreg, ![[ARG_B]], !DIExpression(DW_OP_LLVM_entry_value, 1)
 # CHECK: bb.3.if.end
-# CHECK-NOT: DBG_VALUE $esi, $noreg, ![[ARG_B]], !DIExpression(DW_OP_LLVM_entry_value, 1)
+# VARLOCLDV-NOT: DBG_VALUE $esi, $noreg, ![[ARG_B]], !DIExpression(DW_OP_LLVM_entry_value, 1)
+# INSTRREFLDV: DBG_VALUE $esi, $noreg, ![[ARG_B]], !DIExpression(DW_OP_LLVM_entry_value, 1)
+#
+## Final two lines: VarLoc LiveDebugValues cannot determine that the DBG_VALUEs
+## down either path of the diamond set the variable to be its original value,
+## wheras instruction referencing LiveDebugValues can.
 #
 --- |
   ; ModuleID = 'test.c'

test/DebugInfo/MIR/X86/live-debug-values-entry-transfer.mir

@@ -1,4 +1,5 @@
 # RUN: llc %s -o - -run-pass=livedebugvalues -mtriple=x86_64-unknown-unknown | FileCheck %s
+# RUN: llc %s -o - -force-instr-ref-livedebugvalues=1 -run-pass=livedebugvalues -mtriple=x86_64-unknown-unknown | FileCheck %s
 #
 # In this lightly modified test case, the transfer in the entry block from
 # geti32's return value in $eax to the non-volatile $ebx should be recognized,

test/DebugInfo/MIR/X86/livedebugvalues_load_in_loop.mir

@@ -10,7 +10,6 @@
   ; In a register:
 
   ; CHECK-LABEL: bb.0.entry:
-  ; CHECK:       DBG_VALUE $rdi, $noreg, !16, !DIExpression()
   ; CHECK:       DBG_VALUE $rbp, $noreg, !16, !DIExpression()
   ; CHECK-LABEL: bb.1.bb1:
   ; CHECK:       DBG_VALUE $rbp, $noreg, !16, !DIExpression()
@@ -80,8 +79,8 @@ body:  |
   bb.0.entry:
   liveins: $rdi
     successors: %bb.1, %bb.2
-    DBG_VALUE $rdi, $noreg, !16, !DIExpression(), debug-location !17
     $rbp = MOV64rr $rdi, debug-location !17
+    DBG_VALUE $rbp, $noreg, !16, !DIExpression(), debug-location !17
     dead $rcx = MOV64ri 0, debug-location !17
     CALL64pcrel32 @bees, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp, implicit-def $rax, debug-location !17
     CMP64ri8 renamable $rax, 1, implicit-def $eflags, debug-location !17

test/DebugInfo/MIR/X86/multiple-param-dbg-value-entry.mir

@@ -1,4 +1,5 @@
 # RUN: llc -debug-entry-values -run-pass=livedebugvalues -march=x86-64 -o - %s| FileCheck %s
+# RUN: llc -force-instr-ref-livedebugvalues=1 -debug-entry-values -run-pass=livedebugvalues -march=x86-64 -o - %s| FileCheck %s
 ## Test case:
 ## int global;
 ## int foo(int p, int q, int r) {
@@ -10,8 +11,10 @@
 ##  Verify that DW_OP_LLVM_entry_values are generated for parameters with multiple
 ##  DBG_VALUEs at entry block.
 # CHECK: DBG_VALUE $edi, $noreg, !{{.*}}, !DIExpression(DW_OP_LLVM_entry_value, 1), debug-location {{.*}}
-# CHECK: DBG_VALUE $esi, $noreg, !{{.*}}, !DIExpression(DW_OP_LLVM_entry_value, 1), debug-location {{.*}}
-# CHECK: DBG_VALUE $edx, $noreg, !{{.*}}, !DIExpression(DW_OP_LLVM_entry_value, 1), debug-location {{.*}}
+# CHECK: INLINEASM
+# CHECK-DAG: DBG_VALUE $esi, $noreg, !{{.*}}, !DIExpression(DW_OP_LLVM_entry_value, 1), debug-location {{.*}}
+# CHECK-DAG: DBG_VALUE $edx, $noreg, !{{.*}}, !DIExpression(DW_OP_LLVM_entry_value, 1), debug-location {{.*}}
+# CHECK $eax = MOV32ri 123
 
 --- |
   ; ModuleID = 'multiple-param-dbg-value-entry.ll'

test/DebugInfo/MIR/X86/propagate-entry-value-cross-bbs.mir

@@ -1,4 +1,5 @@
 # RUN: llc -run-pass=livedebugvalues -march=x86-64 -o - %s | FileCheck %s
+# RUN: llc -force-instr-ref-livedebugvalues=1 -run-pass=livedebugvalues -march=x86-64 -o - %s | FileCheck %s
 #
 #extern void fn1 (int, int, int);
 #__attribute__((noinline))

test/DebugInfo/Mips/dw_op_entry_value_32bit.ll

@@ -1,7 +1,9 @@
 ;; Test mips32:
 ; RUN: llc -emit-call-site-info -stop-after=livedebugvalues -mtriple=mips-linux-gnu -o - %s | FileCheck %s
+; RUN: llc -force-instr-ref-livedebugvalues=1 -emit-call-site-info -stop-after=livedebugvalues -mtriple=mips-linux-gnu -o - %s | FileCheck %s
 ;; Test mipsel:
 ; RUN: llc -emit-call-site-info -stop-after=livedebugvalues -mtriple=mipsel-linux-gnu -o - %s | FileCheck %s --check-prefix=CHECKel
+; RUN: llc -force-instr-ref-livedebugvalues=1 -emit-call-site-info -stop-after=livedebugvalues -mtriple=mipsel-linux-gnu -o - %s | FileCheck %s --check-prefix=CHECKel
 
 ;; Built from source:
 ;; extern long fn1(long,long,long);

test/DebugInfo/Mips/dw_op_entry_value_64bit.ll

@@ -1,7 +1,9 @@
 ;; Test mips64:
 ; RUN: llc -emit-call-site-info -stop-after=livedebugvalues -mtriple=mips64-linux-gnu -o - %s | FileCheck %s --check-prefix=CHECK64
+; RUN: llc -force-instr-ref-livedebugvalues=1 -emit-call-site-info -stop-after=livedebugvalues -mtriple=mips64-linux-gnu -o - %s | FileCheck %s --check-prefix=CHECK64
 ;; Test mips64el:
 ; RUN: llc -emit-call-site-info -stop-after=livedebugvalues -mtriple=mips64el-linux-gnu -o - %s | FileCheck %s --check-prefix=CHECK64el
+; RUN: llc -force-instr-ref-livedebugvalues=1 -emit-call-site-info -stop-after=livedebugvalues -mtriple=mips64el-linux-gnu -o - %s | FileCheck %s --check-prefix=CHECK64el
 
 ;; Built from source:
 ;; extern long fn1(long,long,long);

test/DebugInfo/Sparc/entry-value-complex-reg-expr.ll

@@ -1,4 +1,5 @@
 ; RUN: llc -debug-entry-values -filetype=asm -o - %s | FileCheck %s
+; RUN: llc -force-instr-ref-livedebugvalues=1 -debug-entry-values -filetype=asm -o - %s | FileCheck %s
 
 ; The q0 register does not have a DWARF register number, and is instead emitted
 ; as a composite location description with two sub-registers. Previously we

test/DebugInfo/X86/dbg-value-regmask-clobber.ll

@@ -1,5 +1,7 @@
 ; RUN: llc < %s | FileCheck %s --check-prefix=ASM
+; RUN: llc -force-instr-ref-livedebugvalues=1 < %s | FileCheck %s --check-prefix=ASM
 ; RUN: llc < %s -filetype=obj | llvm-dwarfdump - | FileCheck %s --check-prefix=DWARF
+; RUN: llc -force-instr-ref-livedebugvalues=1 < %s -filetype=obj | llvm-dwarfdump - | FileCheck %s --check-prefix=DWARF
 
 ; Values in registers should be clobbered by calls, which use a regmask instead
 ; of individual register def operands.

test/DebugInfo/X86/no-entry-values-with-O0.ll

@@ -2,6 +2,10 @@
 ; RUN:  | llvm-dwarfdump - | FileCheck --implicit-check-not=DW_OP_entry_value %s
 ; RUN: llc -O0  -dwarf-version=5 -debugger-tune=gdb -march=x86-64 -filetype=obj < %s \
 ; RUN:  | llvm-dwarfdump - | FileCheck --implicit-check-not=DW_OP_entry_value %s
+; RUN: llc -force-instr-ref-livedebugvalues=1 -O0  -dwarf-version=5 -debugger-tune=lldb -march=x86-64 -filetype=obj < %s \
+; RUN:  | llvm-dwarfdump - | FileCheck --implicit-check-not=DW_OP_entry_value %s
+; RUN: llc -force-instr-ref-livedebugvalues=1 -O0  -dwarf-version=5 -debugger-tune=gdb -march=x86-64 -filetype=obj < %s \
+; RUN:  | llvm-dwarfdump - | FileCheck --implicit-check-not=DW_OP_entry_value %s
 
 ; The call-site-params are created iff corresponding DISubprogram contains
 ; the AllCallsDescribed DIFlag.