[llvm] 63cc251 - [DebugInfo][InstrRef][4/4] Support DBG_INSTR_REF through all backend passes

Jeremy Morse via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 8 08:42:34 PDT 2021


Author: Jeremy Morse
Date: 2021-07-08T16:42:24+01:00
New Revision: 63cc251eb949b7879919d505dbe375b1cd038781

URL: https://github.com/llvm/llvm-project/commit/63cc251eb949b7879919d505dbe375b1cd038781
DIFF: https://github.com/llvm/llvm-project/commit/63cc251eb949b7879919d505dbe375b1cd038781.diff

LOG: [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

Added: 
    llvm/test/DebugInfo/MIR/InstrRef/x86-drop-compare-inst.mir

Modified: 
    llvm/lib/CodeGen/LiveDebugVariables.cpp
    llvm/lib/CodeGen/LiveIntervals.cpp
    llvm/lib/CodeGen/MachineSink.cpp
    llvm/lib/CodeGen/RegisterCoalescer.cpp
    llvm/lib/CodeGen/RegisterPressure.cpp
    llvm/lib/CodeGen/ScheduleDAGInstrs.cpp
    llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
    llvm/lib/Target/X86/X86InstrInfo.cpp
    llvm/test/DebugInfo/MIR/InstrRef/phi-coalesce-subreg.mir
    llvm/test/DebugInfo/MIR/InstrRef/phi-coalescing.mir
    llvm/test/DebugInfo/MIR/InstrRef/phi-regallocd-to-stack.mir
    llvm/test/DebugInfo/MIR/InstrRef/phi-through-regalloc.mir

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/LiveDebugVariables.cpp b/llvm/lib/CodeGen/LiveDebugVariables.cpp
index 0244f452d646d..54058a547928a 100644
--- a/llvm/lib/CodeGen/LiveDebugVariables.cpp
+++ b/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 
diff erent 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,7 +1262,7 @@ void LDVImpl::computeIntervals() {
   }
 }
 
-bool LDVImpl::runOnMachineFunction(MachineFunction &mf) {
+bool LDVImpl::runOnMachineFunction(MachineFunction &mf, bool InstrRef) {
   clear();
   MF = &mf;
   LIS = &pass.getAnalysis<LiveIntervals>();
@@ -1246,7 +1270,7 @@ bool LDVImpl::runOnMachineFunction(MachineFunction &mf) {
   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);
+      }
     }
   }
 

diff  --git a/llvm/lib/CodeGen/LiveIntervals.cpp b/llvm/lib/CodeGen/LiveIntervals.cpp
index c038ba9430121..6075f64b2f270 100644
--- a/llvm/lib/CodeGen/LiveIntervals.cpp
+++ b/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);

diff  --git a/llvm/lib/CodeGen/MachineSink.cpp b/llvm/lib/CodeGen/MachineSink.cpp
index 41fa3a22c9ce3..ec98394dca79d 100644
--- a/llvm/lib/CodeGen/MachineSink.cpp
+++ b/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);

diff  --git a/llvm/lib/CodeGen/RegisterCoalescer.cpp b/llvm/lib/CodeGen/RegisterCoalescer.cpp
index 0248115dd991e..3b75e0e4a20a0 100644
--- a/llvm/lib/CodeGen/RegisterCoalescer.cpp
+++ b/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;
     });

diff  --git a/llvm/lib/CodeGen/RegisterPressure.cpp b/llvm/lib/CodeGen/RegisterPressure.cpp
index fae1ecfeb738e..62a459fca611b 100644
--- a/llvm/lib/CodeGen/RegisterPressure.cpp
+++ b/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());

diff  --git a/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp b/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp
index 62948fff86147..2a4b0476b8b71 100644
--- a/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp
+++ b/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;
     }

diff  --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index baef5e7c4a770..5827a5f7fd4c0 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -5503,6 +5503,35 @@ bool SelectionDAGBuilder::EmitFuncArgumentDbgValue(
   if (!Arg)
     return false;
 
+  MachineFunction &MF = DAG.getMachineFunction();
+  const TargetInstrInfo *TII = DAG.getSubtarget().getInstrInfo();
+
+  // Helper to create DBG_INSTR_REFs or DBG_VALUEs, depending on what kind
+  // we've been asked to pursue.
+  auto MakeVRegDbgValue = [&](Register Reg, DIExpression *FragExpr,
+                              bool Indirect) {
+    if (Reg.isVirtual() && TM.Options.ValueTrackingVariableLocations) {
+      // For VRegs, in instruction referencing mode, create a DBG_INSTR_REF
+      // pointing at the VReg, which will be patched up later.
+      auto &Inst = TII->get(TargetOpcode::DBG_INSTR_REF);
+      auto MIB = BuildMI(MF, DL, Inst);
+      MIB.addReg(Reg, RegState::Debug);
+      MIB.addImm(0);
+      MIB.addMetadata(Variable);
+      auto *NewDIExpr = FragExpr;
+      // We don't have an "Indirect" field in DBG_INSTR_REF, fold that into
+      // the DIExpression.
+      if (Indirect)
+        NewDIExpr = DIExpression::prepend(FragExpr, DIExpression::DerefBefore);
+      MIB.addMetadata(NewDIExpr);
+      return MIB;
+    } else {
+      // Create a completely standard DBG_VALUE.
+      auto &Inst = TII->get(TargetOpcode::DBG_VALUE);
+      return BuildMI(MF, DL, Inst, Indirect, Reg, Variable, FragExpr);
+    }
+  };
+
   if (!IsDbgDeclare) {
     // ArgDbgValues are hoisted to the beginning of the entry block. So we
     // should only emit as ArgDbgValue if the dbg.value intrinsic is found in
@@ -5568,9 +5597,6 @@ bool SelectionDAGBuilder::EmitFuncArgumentDbgValue(
     }
   }
 
-  MachineFunction &MF = DAG.getMachineFunction();
-  const TargetInstrInfo *TII = DAG.getSubtarget().getInstrInfo();
-
   bool IsIndirect = false;
   Optional<MachineOperand> Op;
   // Some arguments' frame index is recorded during argument lowering.
@@ -5640,9 +5666,9 @@ bool SelectionDAGBuilder::EmitFuncArgumentDbgValue(
           DAG.AddDbgValue(SDV, false);
           continue;
         }
-        FuncInfo.ArgDbgValues.push_back(
-          BuildMI(MF, DL, TII->get(TargetOpcode::DBG_VALUE), IsDbgDeclare,
-                  RegAndSize.first, Variable, *FragmentExpr));
+        MachineInstr *NewMI =
+            MakeVRegDbgValue(RegAndSize.first, *FragmentExpr, IsDbgDeclare);
+        FuncInfo.ArgDbgValues.push_back(NewMI);
       }
     };
 
@@ -5673,11 +5699,15 @@ bool SelectionDAGBuilder::EmitFuncArgumentDbgValue(
 
   assert(Variable->isValidLocationForIntrinsic(DL) &&
          "Expected inlined-at fields to agree");
-  IsIndirect = (Op->isReg()) ? IsIndirect : true;
-  FuncInfo.ArgDbgValues.push_back(
-      BuildMI(MF, DL, TII->get(TargetOpcode::DBG_VALUE), IsIndirect,
-              *Op, Variable, Expr));
+  MachineInstr *NewMI = nullptr;
+
+  if (Op->isReg())
+    NewMI = MakeVRegDbgValue(Op->getReg(), Expr, IsIndirect);
+  else
+    NewMI = BuildMI(MF, DL, TII->get(TargetOpcode::DBG_VALUE), true, *Op,
+                    Variable, Expr);
 
+  FuncInfo.ArgDbgValues.push_back(NewMI);
   return true;
 }
 

diff  --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp
index bf7a0bd980357..74e0dff568cd3 100644
--- a/llvm/lib/Target/X86/X86InstrInfo.cpp
+++ b/llvm/lib/Target/X86/X86InstrInfo.cpp
@@ -4182,6 +4182,8 @@ bool X86InstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
     }
     CmpInstr.setDesc(get(NewOpcode));
     CmpInstr.RemoveOperand(0);
+    // Mutating this instruction invalidates any debug data associated with it.
+    CmpInstr.setDebugInstrNum(0);
     // Fall through to optimize Cmp if Cmp is CMPrr or CMPri.
     if (NewOpcode == X86::CMP64rm || NewOpcode == X86::CMP32rm ||
         NewOpcode == X86::CMP16rm || NewOpcode == X86::CMP8rm)

diff  --git a/llvm/test/DebugInfo/MIR/InstrRef/phi-coalesce-subreg.mir b/llvm/test/DebugInfo/MIR/InstrRef/phi-coalesce-subreg.mir
index 035dd321bbd55..b18f57f4d91ab 100644
--- a/llvm/test/DebugInfo/MIR/InstrRef/phi-coalesce-subreg.mir
+++ b/llvm/test/DebugInfo/MIR/InstrRef/phi-coalesce-subreg.mir
@@ -106,8 +106,6 @@ body:             |
     %3:gr32 = COPY $edi
     %6:gr16 = COPY %4.sub_16bit
     %5:gr16 = COPY %3.sub_16bit
-    DBG_VALUE %5, $noreg, !12, !DIExpression(), debug-location !13
-    DBG_VALUE %6, $noreg, !14, !DIExpression(), debug-location !13
     ADJCALLSTACKDOWN64 0, 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp, debug-location !13
     %15:gr32 = MOVSX32rr16 %5, debug-location !13
     $edi = COPY %15, debug-location !13
@@ -116,7 +114,6 @@ body:             |
     %14:gr32 = MOVSX32rr16 %5, debug-location !13
     %13:gr32 = ADD32ri8 killed %14, 12, implicit-def $eflags, debug-location !13
     %11:gr16 = COPY killed %13.sub_16bit, debug-location !13
-    DBG_VALUE %11, $noreg, !12, !DIExpression(), debug-location !13
     ADJCALLSTACKDOWN64 0, 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp, debug-location !13
     %9:gr32 = MOVSX32rr16 %11, debug-location !13
     $edi = COPY %9, debug-location !13
@@ -135,14 +132,12 @@ body:             |
     %20:gr32 = MOVSX32rr16 %11, debug-location !13
     %19:gr32 = ADD32ri8 killed %20, 1, implicit-def $eflags, debug-location !13
     %17:gr16 = COPY killed %19.sub_16bit, debug-location !13
-    DBG_VALUE %17, $noreg, !12, !DIExpression(), debug-location !13
 
-  ; Verify that vreg 17 is coalesced into gr32.
+  ; Verify that vreg 17 is coalesced into a gr32, and not copied any further.
   ; DOESCOALESCE:       %{{[0-9]+}}:gr32 = ADD32ri8
-  ; DOESCOALESCE-NEXT:  DBG_VALUE %{{[0-9]+}}.sub_16bit,
+  ; DOESCOALESCE-NOT:   COPY
   ; Verify those registers land in $bx
   ; CHECK:              renamable $ebp = ADD32ri8
-  ; CHECK-NEXT:         DBG_VALUE $bp
 
   ; DOESCOALESCE-LABEL: bb.2.if.end:
   ; CHECK-LABEL:        bb.2.if.end:
@@ -154,7 +149,6 @@ body:             |
     %30:gr32 = MOVSX32rr16 killed %2, debug-location !13
     %29:gr32 = ADD32rr killed %30, killed %31, implicit-def $eflags, debug-location !13
     %26:gr16 = COPY killed %29.sub_16bit, debug-location !13
-    DBG_VALUE %26, $noreg, !12, !DIExpression(), debug-location !13
     ADJCALLSTACKDOWN64 0, 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp, debug-location !13
     %24:gr32 = MOVSX32rr16 %26, debug-location !13
     $edi = COPY %24, debug-location !13

diff  --git a/llvm/test/DebugInfo/MIR/InstrRef/phi-coalescing.mir b/llvm/test/DebugInfo/MIR/InstrRef/phi-coalescing.mir
index ae4b68b8c2654..c37804f6036c1 100644
--- a/llvm/test/DebugInfo/MIR/InstrRef/phi-coalescing.mir
+++ b/llvm/test/DebugInfo/MIR/InstrRef/phi-coalescing.mir
@@ -14,7 +14,7 @@
 #
 # Original C code, the PHI is of the value of 'bar' after the control flow.
 # Compiled at -O0, applied -mem2reg, llc -O0, then manually added the PHI
-# instruction label.
+# instruction label. Additional variable locations removed.
 #
 #    void ext(long);
 #    long getlong(void);
@@ -115,14 +115,11 @@ body:             |
     %3:gr64 = COPY $rdi
     %4:gr64 = COPY killed %3
     %6:gr64 = COPY killed %5
-    DBG_VALUE %4, $noreg, !12, !DIExpression(), debug-location !13
-    DBG_VALUE %6, $noreg, !14, !DIExpression(), debug-location !13
     ADJCALLSTACKDOWN64 0, 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp, debug-location !13
     $rdi = COPY %4, debug-location !13
     CALL64pcrel32 @ext, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, debug-location !13
     ADJCALLSTACKUP64 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp, debug-location !13
     %9:gr64 = ADD64ri32 %4, 12, implicit-def $eflags, debug-location !13
-    DBG_VALUE %9, $noreg, !12, !DIExpression(), debug-location !13
     ADJCALLSTACKDOWN64 0, 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp, debug-location !13
     $rdi = COPY %9, debug-location !13
     CALL64pcrel32 @ext, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, debug-location !13
@@ -138,7 +135,6 @@ body:             |
   ; CHECK-LABEL:        bb.1.if.then:
   bb.1.if.then:
     %10:gr64 = ADD64ri32 %9, 1, implicit-def $eflags, debug-location !13
-    DBG_VALUE %10, $noreg, !12, !DIExpression(), debug-location !13
 
   ; Verify that the vreg is 
diff erent immediately after register coalescing.
   ; DOESCOALESCE-NOT:   %10:gr64 ADD64ri32
@@ -153,7 +149,6 @@ body:             |
   ; CHECK:              DBG_PHI $rbx, 1
     DBG_INSTR_REF 1, 0, !12, !DIExpression(), debug-location !13
     %14:gr64 = ADD64rr killed %2, %6, implicit-def $eflags, debug-location !13
-    DBG_VALUE %14, $noreg, !12, !DIExpression(), debug-location !13
     ADJCALLSTACKDOWN64 0, 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp, debug-location !13
     $rdi = COPY %14, debug-location !13
     CALL64pcrel32 @ext, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, debug-location !13

diff  --git a/llvm/test/DebugInfo/MIR/InstrRef/phi-regallocd-to-stack.mir b/llvm/test/DebugInfo/MIR/InstrRef/phi-regallocd-to-stack.mir
index 09593b1c9a91c..b40f2ae9cb6d0 100644
--- a/llvm/test/DebugInfo/MIR/InstrRef/phi-regallocd-to-stack.mir
+++ b/llvm/test/DebugInfo/MIR/InstrRef/phi-regallocd-to-stack.mir
@@ -77,10 +77,7 @@ body:             |
     DBG_VALUE $edi, $noreg, !11, !DIExpression(), debug-location !12
     DBG_VALUE $esi, $noreg, !13, !DIExpression(), debug-location !12
     %2:gr32 = COPY killed $esi
-    DBG_VALUE %2, $noreg, !13, !DIExpression(), debug-location !12
     %1:gr32 = COPY killed $edi
-    DBG_VALUE %1, $noreg, !11, !DIExpression(), debug-location !12
-    DBG_VALUE 0, $noreg, !14, !DIExpression(), debug-location !12
     ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp, debug-location !15
     %3:gr32 = MOV32r0 implicit-def dead $eflags
     $edi = COPY killed %3, debug-location !15
@@ -107,7 +104,6 @@ body:             |
     JMP_1 %bb.1, debug-location !17
 
   bb.1:
-    DBG_VALUE %2, $noreg, !14, !DIExpression(), debug-location !12
     %30:gr32 = MOV32ri 0
     %31:gr32 = MOV32ri 1
     %32:gr32 = MOV32ri 2

diff  --git a/llvm/test/DebugInfo/MIR/InstrRef/phi-through-regalloc.mir b/llvm/test/DebugInfo/MIR/InstrRef/phi-through-regalloc.mir
index fbb5db605a7c1..0e56a79c0930a 100644
--- a/llvm/test/DebugInfo/MIR/InstrRef/phi-through-regalloc.mir
+++ b/llvm/test/DebugInfo/MIR/InstrRef/phi-through-regalloc.mir
@@ -103,9 +103,7 @@ body:             |
     DBG_VALUE $edi, $noreg, !11, !DIExpression(), debug-location !12
     DBG_VALUE $esi, $noreg, !13, !DIExpression(), debug-location !12
     %2:gr32 = COPY killed $esi
-    DBG_VALUE %2, $noreg, !13, !DIExpression(), debug-location !12
     %1:gr32 = COPY killed $edi
-    DBG_VALUE %1, $noreg, !11, !DIExpression(), debug-location !12
     DBG_VALUE 0, $noreg, !14, !DIExpression(), debug-location !12
     ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp, debug-location !15
     %3:gr32 = MOV32r0 implicit-def dead $eflags
@@ -118,7 +116,6 @@ body:             |
     JMP_1 %bb.1, debug-location !17
 
   bb.1.if.else:
-    DBG_VALUE %2, $noreg, !14, !DIExpression(), debug-location !12
 
     ; CHECK-LABEL: bb.2.if.end:
   bb.2.if.end:

diff  --git a/llvm/test/DebugInfo/MIR/InstrRef/x86-drop-compare-inst.mir b/llvm/test/DebugInfo/MIR/InstrRef/x86-drop-compare-inst.mir
new file mode 100644
index 0000000000000..0a56d8a615b8c
--- /dev/null
+++ b/llvm/test/DebugInfo/MIR/InstrRef/x86-drop-compare-inst.mir
@@ -0,0 +1,95 @@
+# RUN: llc %s -o - -experimental-debug-variable-locations -run-pass=peephole-opt | FileCheck %s
+#
+# X86 initially produces subtract operations to perform comparisons, and then
+# downgrades them into cmp instructions if nothing uses the result. It does this
+# by calling setDesc on the instruction, mutating it from one sort to another,
+# which makes any debug instruction numbers attached to the number invalid.
+# This test tests that the relevant instruction number is dropped.
+#
+# CHECK-NOT: debug-instr-number
+# CHECK:     CMP32rm
+# CHECK-NOT: debug-instr-number
+#
+--- |
+  ; ModuleID = '/fast/fs/build34llvm4/reduced.ll'
+  target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+  target triple = "x86_64-unknown-linux-gnu"
+  
+  %"class.std::vector.534" = type { %"struct.std::_Vector_base.535" }
+  %"struct.std::_Vector_base.535" = type { %"struct.std::_Vector_base<unsigned char, std::allocator<unsigned char>>::_Vector_impl" }
+  %"struct.std::_Vector_base<unsigned char, std::allocator<unsigned char>>::_Vector_impl" = type { i8*, i8*, i8* }
+  
+  ; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
+  declare void @llvm.dbg.declare(metadata, metadata, metadata) #0
+  
+  define hidden fastcc void @soup() unnamed_addr !dbg !3 {
+  _ZN4llvm11raw_ostreamlsEPKc.exit2752:
+    %0 = load %"class.std::vector.534"*, %"class.std::vector.534"** undef, align 8, !dbg !7
+    %1 = load i8*, i8** undef, align 8, !dbg !7
+    %_M_start.i2756 = getelementptr inbounds %"class.std::vector.534", %"class.std::vector.534"* %0, i64 undef, i32 0, i32 0, i32 0, !dbg !7
+    %2 = load i8*, i8** %_M_start.i2756, align 8, !dbg !7
+    %sub.ptr.lhs.cast.i2757 = ptrtoint i8* %1 to i64, !dbg !7
+    %sub.ptr.rhs.cast.i2758 = ptrtoint i8* %2 to i64, !dbg !7
+    %sub.ptr.sub.i2759 = sub i64 %sub.ptr.lhs.cast.i2757, %sub.ptr.rhs.cast.i2758, !dbg !7
+    %conv373 = trunc i64 %sub.ptr.sub.i2759 to i32, !dbg !7
+    call void @llvm.dbg.value(metadata i32 %conv373, metadata !8, metadata !DIExpression()), !dbg !7
+    %cmp375.not2842 = icmp eq i32 %conv373, 0, !dbg !7
+    br i1 %cmp375.not2842, label %for.cond.cleanup376, label %for.body377, !dbg !7
+  
+  for.cond.cleanup376:                              ; preds = %_ZN4llvm11raw_ostreamlsEPKc.exit2752
+    ret void
+  
+  for.body377:                                      ; preds = %_ZN4llvm11raw_ostreamlsEPKc.exit2752
+    ret void
+  }
+  
+  ; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
+  declare void @llvm.dbg.value(metadata, metadata, metadata) #0
+  
+  attributes #0 = { nofree nosync nounwind readnone speculatable willreturn }
+  
+  !llvm.module.flags = !{!0}
+  !llvm.dbg.cu = !{!1}
+  
+  !0 = !{i32 2, !"Debug Info Version", i32 3}
+  !1 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !2, producer: "beards", isOptimized: true, runtimeVersion: 4, emissionKind: FullDebug)
+  !2 = !DIFile(filename: "bees.cpp", directory: "")
+  !3 = distinct !DISubprogram(name: "nope", scope: !2, file: !2, line: 1, type: !4, spFlags: DISPFlagDefinition, unit: !1)
+  !4 = !DISubroutineType(types: !5)
+  !5 = !{!6}
+  !6 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !7 = !DILocation(line: 1, scope: !3)
+  !8 = !DILocalVariable(name: "flannel", scope: !3)
+
+...
+---
+name:            soup
+alignment:       16
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: gr64 }
+  - { id: 1, class: gr64 }
+  - { id: 2, class: gr32 }
+  - { id: 3, class: gr32 }
+frameInfo:
+  maxAlignment:    1
+machineFunctionInfo: {}
+body:             |
+  bb.0._ZN4llvm11raw_ostreamlsEPKc.exit2752:
+    successors: %bb.1(0x30000000), %bb.2(0x50000000)
+  
+    %1:gr64 = IMPLICIT_DEF
+    %0:gr64 = MOV64rm killed %1, 1, $noreg, 0, $noreg, debug-location !7 :: (load (s64) from `i8** undef`)
+    %2:gr32 = COPY %0.sub_32bit, debug-location !7
+    %3:gr32 = SUB32rm %2, %0, 1, $noreg, 0, $noreg, implicit-def $eflags, debug-instr-number 1, debug-location !7 :: (load (s32) from %ir._M_start.i2756, align 8)
+    DBG_INSTR_REF 1, 0, !8, !DIExpression(), debug-location !7
+    JCC_1 %bb.2, 5, implicit $eflags, debug-location !7
+    JMP_1 %bb.1, debug-location !7
+  
+  bb.1.for.cond.cleanup376:
+    RET 0
+  
+  bb.2.for.body377:
+    RET 0
+
+...


        


More information about the llvm-commits mailing list