[DebugInfo][InstrRef][4/4] Support DBG_INSTR_REF through all backend …

…passes

This is a cleanup patch -- we're now able to support all flavours of
variable location in instruction referencing mode. This patch updates
various tests for debug instructions to be broader: numerous code paths
try to ignore debug isntructions, and they now have to ignore the
additional DBG_PHI and DBG_INSTR_REFs that we can generate.

A small amount of rework happens for LiveDebugVariables: as we don't need
to track live intervals through regalloc any more, we can get away with
unlinking debug instructions before regalloc, then re-inserting them after.
Note that this isn't (yet) true of DBG_VALUE_LISTs, they still have to go
through live interval tracking.

In SelectionDAG, add a helper lambda that emits half-formed DBG_INSTR_REFs
for arguments in instr-ref mode, DBG_VALUE otherwise. This is one of the
final locations where DBG_VALUEs are emitted for vreg arguments.

X86InstrInfo now un-sets the debug instr number on SUB instructions that
get mutated into CMP instructions. As the instruction no longer computes a
subtraction, we can't use it for variable locations.

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

llvm/lib/CodeGen/LiveDebugVariables.cpp

@@ -38,9 +38,11 @@
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineOperand.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
+#include "llvm/CodeGen/Passes.h"
 #include "llvm/CodeGen/SlotIndexes.h"
 #include "llvm/CodeGen/TargetInstrInfo.h"
 #include "llvm/CodeGen/TargetOpcodes.h"
+#include "llvm/CodeGen/TargetPassConfig.h"
 #include "llvm/CodeGen/TargetRegisterInfo.h"
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
 #include "llvm/CodeGen/VirtRegMap.h"
@@ -56,6 +58,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Target/TargetMachine.h"
 #include <algorithm>
 #include <cassert>
 #include <iterator>
@@ -535,10 +538,6 @@ class LDVImpl {
   LiveIntervals *LIS;
   const TargetRegisterInfo *TRI;
 
-  using StashedInstrRef =
-      std::tuple<unsigned, unsigned, const DILocalVariable *,
-                 const DIExpression *, DebugLoc>;
-
   /// Position and VReg of a PHI instruction during register allocation.
   struct PHIValPos {
     SlotIndex SI;    /// Slot where this PHI occurs.
@@ -553,7 +552,17 @@ class LDVImpl {
   /// at different positions.
   DenseMap<Register, std::vector<unsigned>> RegToPHIIdx;
 
-  std::map<SlotIndex, std::vector<StashedInstrRef>> StashedInstrReferences;
+  /// Record for any debug instructions unlinked from their blocks during
+  /// regalloc. Stores the instr and it's location, so that they can be
+  /// re-inserted after regalloc is over.
+  struct InstrPos {
+    MachineInstr *MI;       ///< Debug instruction, unlinked from it's block.
+    SlotIndex Idx;          ///< Slot position where MI should be re-inserted.
+    MachineBasicBlock *MBB; ///< Block that MI was in.
+  };
+
+  /// Collection of stored debug instructions, preserved until after regalloc.
+  SmallVector<InstrPos, 32> StashedDebugInstrs;
 
   /// Whether emitDebugValues is called.
   bool EmitDone = false;
@@ -591,15 +600,18 @@ class LDVImpl {
   /// \returns True if the DBG_VALUE instruction should be deleted.
   bool handleDebugValue(MachineInstr &MI, SlotIndex Idx);
 
-  /// Track a DBG_INSTR_REF. This needs to be removed from the MachineFunction
-  /// during regalloc -- but there's no need to maintain live ranges, as we
-  /// refer to a value rather than a location.
+  /// Track variable location debug instructions while using the instruction
+  /// referencing implementation. Such debug instructions do not need to be
+  /// updated during regalloc because they identify instructions rather than
+  /// register locations. However, they needs to be removed from the
+  /// MachineFunction during regalloc, then re-inserted later, to avoid
+  /// disrupting the allocator.
   ///
-  /// \param MI DBG_INSTR_REF instruction
+  /// \param MI Any DBG_VALUE / DBG_INSTR_REF / DBG_PHI instruction
   /// \param Idx Last valid SlotIndex before instruction
   ///
-  /// \returns True if the DBG_VALUE instruction should be deleted.
-  bool handleDebugInstrRef(MachineInstr &MI, SlotIndex Idx);
+  /// \returns Iterator to continue processing from after unlinking.
+  MachineBasicBlock::iterator handleDebugInstr(MachineInstr &MI, SlotIndex Idx);
 
   /// Add DBG_LABEL instruction to UserLabel.
   ///
@@ -613,9 +625,11 @@ class LDVImpl {
   /// for each instruction.
   ///
   /// \param mf MachineFunction to be scanned.
+  /// \param InstrRef Whether to operate in instruction referencing mode. If
+  ///        true, most of LiveDebugVariables doesn't run.
   ///
   /// \returns True if any debug values were found.
-  bool collectDebugValues(MachineFunction &mf);
+  bool collectDebugValues(MachineFunction &mf, bool InstrRef);
 
   /// Compute the live intervals of all user values after collecting all
   /// their def points.
@@ -624,14 +638,14 @@ class LDVImpl {
 public:
   LDVImpl(LiveDebugVariables *ps) : pass(*ps) {}
 
-  bool runOnMachineFunction(MachineFunction &mf);
+  bool runOnMachineFunction(MachineFunction &mf, bool InstrRef);
 
   /// Release all memory.
   void clear() {
     MF = nullptr;
     PHIValToPos.clear();
     RegToPHIIdx.clear();
-    StashedInstrReferences.clear();
+    StashedDebugInstrs.clear();
     userValues.clear();
     userLabels.clear();
     virtRegToEqClass.clear();
@@ -864,17 +878,21 @@ bool LDVImpl::handleDebugValue(MachineInstr &MI, SlotIndex Idx) {
   return true;
 }
 
-bool LDVImpl::handleDebugInstrRef(MachineInstr &MI, SlotIndex Idx) {
-  assert(MI.isDebugRef());
-  unsigned InstrNum = MI.getOperand(0).getImm();
-  unsigned OperandNum = MI.getOperand(1).getImm();
-  auto *Var = MI.getDebugVariable();
-  auto *Expr = MI.getDebugExpression();
-  auto &DL = MI.getDebugLoc();
-  StashedInstrRef Stashed =
-      std::make_tuple(InstrNum, OperandNum, Var, Expr, DL);
-  StashedInstrReferences[Idx].push_back(Stashed);
-  return true;
+MachineBasicBlock::iterator LDVImpl::handleDebugInstr(MachineInstr &MI,
+                                                      SlotIndex Idx) {
+  assert(MI.isDebugValue() || MI.isDebugRef() || MI.isDebugPHI());
+
+  // In instruction referencing mode, there should be no DBG_VALUE instructions
+  // that refer to virtual registers. They might still refer to constants.
+  if (MI.isDebugValue())
+    assert(!MI.getOperand(0).isReg() || !MI.getOperand(0).getReg().isVirtual());
+
+  // Unlink the instruction, store it in the debug instructions collection.
+  auto NextInst = std::next(MI.getIterator());
+  auto *MBB = MI.getParent();
+  MI.removeFromParent();
+  StashedDebugInstrs.push_back({&MI, Idx, MBB});
+  return NextInst;
 }
 
 bool LDVImpl::handleDebugLabel(MachineInstr &MI, SlotIndex Idx) {
@@ -900,7 +918,7 @@ bool LDVImpl::handleDebugLabel(MachineInstr &MI, SlotIndex Idx) {
   return true;
 }
 
-bool LDVImpl::collectDebugValues(MachineFunction &mf) {
+bool LDVImpl::collectDebugValues(MachineFunction &mf, bool InstrRef) {
   bool Changed = false;
   for (MachineBasicBlock &MBB : mf) {
     for (MachineBasicBlock::iterator MBBI = MBB.begin(), MBBE = MBB.end();
@@ -919,11 +937,17 @@ bool LDVImpl::collectDebugValues(MachineFunction &mf) {
               : LIS->getInstructionIndex(*std::prev(MBBI)).getRegSlot();
       // Handle consecutive debug instructions with the same slot index.
       do {
-        // Only handle DBG_VALUE in handleDebugValue(). Skip all other
-        // kinds of debug instructions.
-        if ((MBBI->isDebugValue() && handleDebugValue(*MBBI, Idx)) ||
-            (MBBI->isDebugRef() && handleDebugInstrRef(*MBBI, Idx)) ||
-            (MBBI->isDebugLabel() && handleDebugLabel(*MBBI, Idx))) {
+        // In instruction referencing mode, pass each instr to handleDebugInstr
+        // to be unlinked. Ignore DBG_VALUE_LISTs -- they refer to vregs, and
+        // need to go through the normal live interval splitting process.
+        if (InstrRef && (MBBI->isNonListDebugValue() || MBBI->isDebugPHI() ||
+                         MBBI->isDebugRef())) {
+          MBBI = handleDebugInstr(*MBBI, Idx);
+          Changed = true;
+        // In normal debug mode, use the dedicated DBG_VALUE / DBG_LABEL handler
+        // to track things through register allocation, and erase the instr.
+        } else if ((MBBI->isDebugValue() && handleDebugValue(*MBBI, Idx)) ||
+                   (MBBI->isDebugLabel() && handleDebugLabel(*MBBI, Idx))) {
           MBBI = MBB.erase(MBBI);
           Changed = true;
         } else
@@ -1238,15 +1262,15 @@ void LDVImpl::computeIntervals() {
   }
 }
 
-bool LDVImpl::runOnMachineFunction(MachineFunction &mf) {
+bool LDVImpl::runOnMachineFunction(MachineFunction &mf, bool InstrRef) {
   clear();
   MF = &mf;
   LIS = &pass.getAnalysis<LiveIntervals>();
   TRI = mf.getSubtarget().getRegisterInfo();
   LLVM_DEBUG(dbgs() << "********** COMPUTING LIVE DEBUG VARIABLES: "
                     << mf.getName() << " **********\n");
 
-  bool Changed = collectDebugValues(mf);
+  bool Changed = collectDebugValues(mf, InstrRef);
   computeIntervals();
   LLVM_DEBUG(print(dbgs()));
 
@@ -1287,9 +1311,19 @@ bool LiveDebugVariables::runOnMachineFunction(MachineFunction &mf) {
     removeDebugInstrs(mf);
     return false;
   }
+
+  // Have we been asked to track variable locations using instruction
+  // referencing?
+  bool InstrRef = false;
+  auto *TPC = getAnalysisIfAvailable<TargetPassConfig>();
+  if (TPC) {
+    auto &TM = TPC->getTM<TargetMachine>();
+    InstrRef = TM.Options.ValueTrackingVariableLocations;
+  }
+
   if (!pImpl)
     pImpl = new LDVImpl(this);
-  return static_cast<LDVImpl*>(pImpl)->runOnMachineFunction(mf);
+  return static_cast<LDVImpl *>(pImpl)->runOnMachineFunction(mf, InstrRef);
 }
 
 void LiveDebugVariables::releaseMemory() {
@@ -1850,22 +1884,45 @@ void LDVImpl::emitDebugValues(VirtRegMap *VRM) {
 
   LLVM_DEBUG(dbgs() << "********** EMITTING INSTR REFERENCES **********\n");
 
-  // Re-insert any DBG_INSTR_REFs back in the position they were. Ordering
-  // is preserved by vector.
-  const MCInstrDesc &RefII = TII->get(TargetOpcode::DBG_INSTR_REF);
-  for (auto &P : StashedInstrReferences) {
-    const SlotIndex &Idx = P.first;
-    auto *MBB = Slots->getMBBFromIndex(Idx);
-    MachineBasicBlock::iterator insertPos =
-        findInsertLocation(MBB, Idx, *LIS, BBSkipInstsMap);
-    for (auto &Stashed : P.second) {
-      auto MIB = BuildMI(*MF, std::get<4>(Stashed), RefII);
-      MIB.addImm(std::get<0>(Stashed));
-      MIB.addImm(std::get<1>(Stashed));
-      MIB.addMetadata(std::get<2>(Stashed));
-      MIB.addMetadata(std::get<3>(Stashed));
-      MachineInstr *New = MIB;
-      MBB->insert(insertPos, New);
+  // Re-insert any debug instrs back in the position they were. Ordering
+  // is preserved by vector. We must re-insert in the same order to ensure that
+  // debug instructions don't swap, which could re-order assignments.
+  for (auto &P : StashedDebugInstrs) {
+    SlotIndex Idx = P.Idx;
+
+    // Start block index: find the first non-debug instr in the block, and
+    // insert before it.
+    if (Idx == Slots->getMBBStartIdx(P.MBB)) {
+      MachineBasicBlock::iterator InsertPos =
+          findInsertLocation(P.MBB, Idx, *LIS, BBSkipInstsMap);
+      P.MBB->insert(InsertPos, P.MI);
+      continue;
+    }
+
+    if (MachineInstr *Pos = Slots->getInstructionFromIndex(Idx)) {
+      // Insert at the end of any debug instructions.
+      auto PostDebug = std::next(Pos->getIterator());
+      PostDebug = skipDebugInstructionsForward(PostDebug, P.MBB->instr_end());
+      P.MBB->insert(PostDebug, P.MI);
+    } else {
+      // Insert position disappeared; walk forwards through slots until we
+      // find a new one.
+      SlotIndex End = Slots->getMBBEndIdx(P.MBB);
+      for (; Idx < End; Idx = Slots->getNextNonNullIndex(Idx)) {
+        Pos = Slots->getInstructionFromIndex(Idx);
+        if (Pos) {
+          P.MBB->insert(Pos->getIterator(), P.MI);
+          break;
+        }
+      }
+
+      // We have reached the end of the block and didn't find anywhere to
+      // insert! It's not safe to discard any debug instructions; place them
+      // in front of the first terminator, or in front of end().
+      if (Idx >= End) {
+        auto TermIt = P.MBB->getFirstTerminator();
+        P.MBB->insert(TermIt, P.MI);
+      }
     }
   }
 

llvm/lib/CodeGen/LiveIntervals.cpp

@@ -475,7 +475,7 @@ bool LiveIntervals::shrinkToUses(LiveInterval *li,
   // Visit all instructions reading li->reg().
   Register Reg = li->reg();
   for (MachineInstr &UseMI : MRI->reg_instructions(Reg)) {
-    if (UseMI.isDebugValue() || !UseMI.readsVirtualRegister(Reg))
+    if (UseMI.isDebugInstr() || !UseMI.readsVirtualRegister(Reg))
       continue;
     SlotIndex Idx = getInstructionIndex(UseMI).getRegSlot();
     LiveQueryResult LRQ = li->Query(Idx);

llvm/lib/CodeGen/MachineSink.cpp

@@ -718,7 +718,7 @@ MachineSinking::getBBRegisterPressure(MachineBasicBlock &MBB) {
                                    MIE = MBB.instr_begin();
        MII != MIE; --MII) {
     MachineInstr &MI = *std::prev(MII);
-    if (MI.isDebugValue() || MI.isDebugLabel() || MI.isPseudoProbe())
+    if (MI.isDebugInstr() || MI.isPseudoProbe())
       continue;
     RegisterOperands RegOpers;
     RegOpers.collect(MI, *TRI, *MRI, false, false);

llvm/lib/CodeGen/RegisterCoalescer.cpp

@@ -928,7 +928,7 @@ RegisterCoalescer::removeCopyByCommutingDef(const CoalescerPair &CP,
     if (UseMO.isUndef())
       continue;
     MachineInstr *UseMI = UseMO.getParent();
-    if (UseMI->isDebugValue()) {
+    if (UseMI->isDebugInstr()) {
       // FIXME These don't have an instruction index.  Not clear we have enough
       // info to decide whether to do this replacement or not.  For now do it.
       UseMO.setReg(NewReg);
@@ -1561,7 +1561,7 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
          UI != MRI->use_end();) {
       MachineOperand &UseMO = *UI++;
       MachineInstr *UseMI = UseMO.getParent();
-      if (UseMI->isDebugValue()) {
+      if (UseMI->isDebugInstr()) {
         if (Register::isPhysicalRegister(DstReg))
           UseMO.substPhysReg(DstReg, *TRI);
         else
@@ -1742,7 +1742,7 @@ void RegisterCoalescer::updateRegDefsUses(Register SrcReg, Register DstReg,
       if (SubReg == 0 || MO.isUndef())
         continue;
       MachineInstr &MI = *MO.getParent();
-      if (MI.isDebugValue())
+      if (MI.isDebugInstr())
         continue;
       SlotIndex UseIdx = LIS->getInstructionIndex(MI).getRegSlot(true);
       addUndefFlag(*DstInt, UseIdx, MO, SubReg);
@@ -1769,7 +1769,7 @@ void RegisterCoalescer::updateRegDefsUses(Register SrcReg, Register DstReg,
 
     // If SrcReg wasn't read, it may still be the case that DstReg is live-in
     // because SrcReg is a sub-register.
-    if (DstInt && !Reads && SubIdx && !UseMI->isDebugValue())
+    if (DstInt && !Reads && SubIdx && !UseMI->isDebugInstr())
       Reads = DstInt->liveAt(LIS->getInstructionIndex(*UseMI));
 
     // Replace SrcReg with DstReg in all UseMI operands.
@@ -1799,7 +1799,7 @@ void RegisterCoalescer::updateRegDefsUses(Register SrcReg, Register DstReg,
             // unused lanes. This may happen with rematerialization.
             DstInt->createSubRange(Allocator, UnusedLanes);
           }
-          SlotIndex MIIdx = UseMI->isDebugValue()
+          SlotIndex MIIdx = UseMI->isDebugInstr()
             ? LIS->getSlotIndexes()->getIndexBefore(*UseMI)
             : LIS->getInstructionIndex(*UseMI);
           SlotIndex UseIdx = MIIdx.getRegSlot(true);
@@ -1815,7 +1815,7 @@ void RegisterCoalescer::updateRegDefsUses(Register SrcReg, Register DstReg,
 
     LLVM_DEBUG({
       dbgs() << "\t\tupdated: ";
-      if (!UseMI->isDebugValue())
+      if (!UseMI->isDebugInstr())
         dbgs() << LIS->getInstructionIndex(*UseMI) << "\t";
       dbgs() << *UseMI;
     });

llvm/lib/CodeGen/RegisterPressure.cpp

@@ -873,7 +873,7 @@ void RegPressureTracker::recedeSkipDebugValues() {
 
 void RegPressureTracker::recede(SmallVectorImpl<RegisterMaskPair> *LiveUses) {
   recedeSkipDebugValues();
-  if (CurrPos->isDebugValue() || CurrPos->isPseudoProbe()) {
+  if (CurrPos->isDebugInstr() || CurrPos->isPseudoProbe()) {
     // It's possible to only have debug_value and pseudo probe instructions and
     // hit the start of the block.
     assert(CurrPos == MBB->begin());

llvm/lib/CodeGen/ScheduleDAGInstrs.cpp

@@ -807,7 +807,7 @@ void ScheduleDAGInstrs::buildSchedGraph(AAResults *AA,
       DbgMI = nullptr;
     }
 
-    if (MI.isDebugValue() || MI.isDebugRef()) {
+    if (MI.isDebugValue() || MI.isDebugRef() || MI.isDebugPHI()) {
       DbgMI = &MI;
       continue;
     }