[DebugInfo] Avoid dropping location info across block boundaries

LiveDebugValues propagates variable locations between blocks by creating
new DBG_VALUE insts in the successors, then interpreting them when it
passes back through the block at a later time. However, this flushes out
any extra information about the location that LiveDebugValues holds: for
example, connections between variable locations such as discussed in
D65368. And as reported in PR42772 this causes us to lose track of the
fact that a spill-location is actually a spill, not a register location.

This patch fixes that by deferring the creation of propagated DBG_VALUEs
until after propagation has completed: instead location propagation occurs
only by sharing location ID numbers between blocks. 

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

llvm-svn: 369508

1 files changed, 105 insertions, 57 deletions

diff --git a/llvm/lib/CodeGen/LiveDebugValues.cpp b/llvm/lib/CodeGen/LiveDebugValues.cpp
index 74e641be060..033394fb0f5 100644
--- a/llvm/lib/CodeGen/LiveDebugValues.cpp
+++ b/llvm/lib/CodeGen/LiveDebugValues.cpp
@@ -7,14 +7,23 @@
 //===----------------------------------------------------------------------===//
 ///
 /// This pass implements a data flow analysis that propagates debug location
-/// information by inserting additional DBG_VALUE instructions into the machine
-/// instruction stream. The pass internally builds debug location liveness
-/// ranges to determine the points where additional DBG_VALUEs need to be
-/// inserted.
+/// information by inserting additional DBG_VALUE insts into the machine
+/// instruction stream. Before running, each DBG_VALUE inst corresponds to a
+/// source assignment of a variable. Afterwards, a DBG_VALUE inst specifies a
+/// variable location for the current basic block (see SourceLevelDebugging.rst).
 ///
 /// This is a separate pass from DbgValueHistoryCalculator to facilitate
 /// testing and improve modularity.
 ///
+/// Each variable location is represented by a VarLoc object that identifies the
+/// source variable, its current machine-location, and the DBG_VALUE inst that
+/// specifies the location. Each VarLoc is indexed in the (function-scope)
+/// VarLocMap, giving each VarLoc a unique index. Rather than operate directly
+/// on machine locations, the dataflow analysis in this pass identifies
+/// locations by their index in the VarLocMap, meaning all the variable
+/// locations in a block can be described by a sparse vector of VarLocMap
+/// indexes.
+///
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/DenseMap.h"
@@ -320,6 +329,14 @@ private:
       Vars.insert({Var, VarLocID});
     }
 
+    /// Insert a set of ranges.
+    void insertFromLocSet(const VarLocSet &ToLoad, const VarLocMap &Map) {
+      for (unsigned Id : ToLoad) {
+        const VarLoc &Var = Map[Id];
+        insert(Id, Var.Var);
+      }
+    }
+
     /// Empty the set.
     void clear() {
       VarLocs.clear();
@@ -361,10 +378,10 @@ private:
   void transferRegisterDef(MachineInstr &MI, OpenRangesSet &OpenRanges,
                            VarLocMap &VarLocIDs, TransferMap &Transfers,
                            DebugParamMap &DebugEntryVals);
-  bool transferTerminatorInst(MachineInstr &MI, OpenRangesSet &OpenRanges,
-                              VarLocInMBB &OutLocs, const VarLocMap &VarLocIDs);
+  bool transferTerminator(MachineBasicBlock *MBB, OpenRangesSet &OpenRanges,
+                          VarLocInMBB &OutLocs, const VarLocMap &VarLocIDs);
 
-  bool process(MachineInstr &MI, OpenRangesSet &OpenRanges,
+  void process(MachineInstr &MI, OpenRangesSet &OpenRanges,
                VarLocInMBB &OutLocs, VarLocMap &VarLocIDs,
                TransferMap &Transfers, DebugParamMap &DebugEntryVals,
                bool transferChanges, OverlapMap &OverlapFragments,
@@ -376,7 +393,12 @@ private:
   bool join(MachineBasicBlock &MBB, VarLocInMBB &OutLocs, VarLocInMBB &InLocs,
             const VarLocMap &VarLocIDs,
             SmallPtrSet<const MachineBasicBlock *, 16> &Visited,
-            SmallPtrSetImpl<const MachineBasicBlock *> &ArtificialBlocks);
+            SmallPtrSetImpl<const MachineBasicBlock *> &ArtificialBlocks,
+            VarLocInMBB &PendingInLocs);
+
+  /// Create DBG_VALUE insts for inlocs that have been propagated but
+  /// had their instruction creation deferred.
+  void flushPendingLocs(VarLocInMBB &PendingInLocs, VarLocMap &VarLocIDs);
 
   bool ExtendRanges(MachineFunction &MF);
 
@@ -904,14 +926,11 @@ void LiveDebugValues::transferRegisterCopy(MachineInstr &MI,
 }
 
 /// Terminate all open ranges at the end of the current basic block.
-bool LiveDebugValues::transferTerminatorInst(MachineInstr &MI,
-                                             OpenRangesSet &OpenRanges,
-                                             VarLocInMBB &OutLocs,
-                                             const VarLocMap &VarLocIDs) {
+bool LiveDebugValues::transferTerminator(MachineBasicBlock *CurMBB,
+                                         OpenRangesSet &OpenRanges,
+                                         VarLocInMBB &OutLocs,
+                                         const VarLocMap &VarLocIDs) {
   bool Changed = false;
-  const MachineBasicBlock *CurMBB = MI.getParent();
-  if (!(MI.isTerminator() || (&MI == &CurMBB->back())))
-    return false;
 
   if (OpenRanges.empty())
     return false;
@@ -993,13 +1012,12 @@ void LiveDebugValues::accumulateFragmentMap(MachineInstr &MI,
 }
 
 /// This routine creates OpenRanges and OutLocs.
-bool LiveDebugValues::process(MachineInstr &MI, OpenRangesSet &OpenRanges,
+void LiveDebugValues::process(MachineInstr &MI, OpenRangesSet &OpenRanges,
                               VarLocInMBB &OutLocs, VarLocMap &VarLocIDs,
                               TransferMap &Transfers, DebugParamMap &DebugEntryVals,
                               bool transferChanges,
                               OverlapMap &OverlapFragments,
                               VarToFragments &SeenFragments) {
-  bool Changed = false;
   transferDebugValue(MI, OpenRanges, VarLocIDs);
   transferRegisterDef(MI, OpenRanges, VarLocIDs, Transfers,
                       DebugEntryVals);
@@ -1011,8 +1029,6 @@ bool LiveDebugValues::process(MachineInstr &MI, OpenRangesSet &OpenRanges,
     if (MI.isDebugValue())
       accumulateFragmentMap(MI, SeenFragments, OverlapFragments);
   }
-  Changed = transferTerminatorInst(MI, OpenRanges, OutLocs, VarLocIDs);
-  return Changed;
 }
 
 /// This routine joins the analysis results of all incoming edges in @MBB by
@@ -1022,7 +1038,8 @@ bool LiveDebugValues::join(
     MachineBasicBlock &MBB, VarLocInMBB &OutLocs, VarLocInMBB &InLocs,
     const VarLocMap &VarLocIDs,
     SmallPtrSet<const MachineBasicBlock *, 16> &Visited,
-    SmallPtrSetImpl<const MachineBasicBlock *> &ArtificialBlocks) {
+    SmallPtrSetImpl<const MachineBasicBlock *> &ArtificialBlocks,
+    VarLocInMBB &PendingInLocs) {
   LLVM_DEBUG(dbgs() << "join MBB: " << MBB.getNumber() << "\n");
   bool Changed = false;
 
@@ -1088,44 +1105,15 @@ bool LiveDebugValues::join(
     return false;
 
   VarLocSet &ILS = InLocs[&MBB];
+  VarLocSet &Pending = PendingInLocs[&MBB];
 
-  // Insert DBG_VALUE instructions, if not already inserted.
+  // New locations will have DBG_VALUE insts inserted at the start of the
+  // block, after location propagation has finished. Record the insertions
+  // that we need to perform in the Pending set.
   VarLocSet Diff = InLocsT;
   Diff.intersectWithComplement(ILS);
   for (auto ID : Diff) {
-    // This VarLoc is not found in InLocs i.e. it is not yet inserted. So, a
-    // new range is started for the var from the mbb's beginning by inserting
-    // a new DBG_VALUE. process() will end this range however appropriate.
-    const VarLoc &DiffIt = VarLocIDs[ID];
-    const MachineInstr *DebugInstr = &DiffIt.MI;
-    MachineInstr *MI = nullptr;
-    if (DiffIt.isConstant()) {
-      MachineOperand MO(DebugInstr->getOperand(0));
-      MI = BuildMI(MBB, MBB.instr_begin(), DebugInstr->getDebugLoc(),
-                   DebugInstr->getDesc(), false, MO,
-                   DebugInstr->getDebugVariable(),
-                   DebugInstr->getDebugExpression());
-    } else {
-      auto *DebugExpr = DebugInstr->getDebugExpression();
-      Register Reg = DebugInstr->getOperand(0).getReg();
-      bool IsIndirect = DebugInstr->isIndirectDebugValue();
-
-      if (DiffIt.Kind == VarLoc::SpillLocKind) {
-        // This is is a spilt location; DebugInstr refers to the unspilt
-        // location. We need to rebuild the spilt location expression and
-        // point the DBG_VALUE at the frame register.
-        DebugExpr = DIExpression::prepend(DebugInstr->getDebugExpression(),
-                                          DIExpression::ApplyOffset,
-                                          DiffIt.Loc.SpillLocation.SpillOffset);
-        Reg = TRI->getFrameRegister(*DebugInstr->getMF());
-        IsIndirect = true;
-      }
-
-      MI = BuildMI(MBB, MBB.instr_begin(), DebugInstr->getDebugLoc(),
-                   DebugInstr->getDesc(), IsIndirect, Reg,
-                   DebugInstr->getDebugVariable(), DebugExpr);
-    }
-    LLVM_DEBUG(dbgs() << "Inserted: "; MI->dump(););
+    Pending.set(ID);
     ILS.set(ID);
     ++NumInserted;
     Changed = true;
@@ -1133,6 +1121,53 @@ bool LiveDebugValues::join(
   return Changed;
 }
 
+void LiveDebugValues::flushPendingLocs(VarLocInMBB &PendingInLocs,
+                                       VarLocMap &VarLocIDs) {
+  // PendingInLocs records all locations propagated into blocks, which have
+  // not had DBG_VALUE insts created. Go through and create those insts now.
+  for (auto &Iter : PendingInLocs) {
+    // Map is keyed on a constant pointer, unwrap it so we can insert insts.
+    auto &MBB = const_cast<MachineBasicBlock &>(*Iter.first);
+    VarLocSet &Pending = Iter.second;
+
+    for (unsigned ID : Pending) {
+      // The ID location is live-in to MBB -- work out what kind of machine
+      // location it is and create a DBG_VALUE.
+      const VarLoc &DiffIt = VarLocIDs[ID];
+      const MachineInstr *DebugInstr = &DiffIt.MI;
+      MachineInstr *MI = nullptr;
+
+      if (DiffIt.isConstant()) {
+        MachineOperand MO(DebugInstr->getOperand(0));
+        MI = BuildMI(MBB, MBB.instr_begin(), DebugInstr->getDebugLoc(),
+                     DebugInstr->getDesc(), false, MO,
+                     DebugInstr->getDebugVariable(),
+                     DebugInstr->getDebugExpression());
+      } else {
+        auto *DebugExpr = DebugInstr->getDebugExpression();
+        Register Reg = DebugInstr->getOperand(0).getReg();
+        bool IsIndirect = DebugInstr->isIndirectDebugValue();
+
+        if (DiffIt.Kind == VarLoc::SpillLocKind) {
+          // This is is a spilt location; DebugInstr refers to the unspilt
+          // location. We need to rebuild the spilt location expression and
+          // point the DBG_VALUE at the frame register.
+          DebugExpr = DIExpression::prepend(
+              DebugInstr->getDebugExpression(), DIExpression::ApplyOffset,
+              DiffIt.Loc.SpillLocation.SpillOffset);
+          Reg = TRI->getFrameRegister(*DebugInstr->getMF());
+          IsIndirect = true;
+        }
+
+        MI = BuildMI(MBB, MBB.instr_begin(), DebugInstr->getDebugLoc(),
+                     DebugInstr->getDesc(), IsIndirect, Reg,
+                     DebugInstr->getDebugVariable(), DebugExpr);
+      }
+      LLVM_DEBUG(dbgs() << "Inserted: "; MI->dump(););
+    }
+  }
+}
+
 /// Calculate the liveness information for the given machine function and
 /// extend ranges across basic blocks.
 bool LiveDebugValues::ExtendRanges(MachineFunction &MF) {
@@ -1149,6 +1184,9 @@ bool LiveDebugValues::ExtendRanges(MachineFunction &MF) {
   VarLocInMBB OutLocs;        // Ranges that exist beyond bb.
   VarLocInMBB InLocs;         // Ranges that are incoming after joining.
   TransferMap Transfers;      // DBG_VALUEs associated with spills.
+  VarLocInMBB PendingInLocs;  // Ranges that are incoming after joining, but
+                              // that we have deferred creating DBG_VALUE insts
+                              // for immediately.
 
   VarToFragments SeenFragments;
 
@@ -1214,6 +1252,7 @@ bool LiveDebugValues::ExtendRanges(MachineFunction &MF) {
       process(MI, OpenRanges, OutLocs, VarLocIDs, Transfers, DebugEntryVals,
               dontTransferChanges, OverlapFragments, SeenFragments);
     }
+    transferTerminator(&MBB, OpenRanges, OutLocs, VarLocIDs);
     // Add any entry DBG_VALUE instructions necessitated by parameter
     // clobbering.
     for (auto &TR : Transfers) {
@@ -1221,6 +1260,9 @@ bool LiveDebugValues::ExtendRanges(MachineFunction &MF) {
                      TR.DebugInst);
     }
     Transfers.clear();
+
+    // Initialize pending inlocs.
+    PendingInLocs[&MBB] = VarLocSet();
   }
 
   auto hasNonArtificialLocation = [](const MachineInstr &MI) -> bool {
@@ -1257,8 +1299,8 @@ bool LiveDebugValues::ExtendRanges(MachineFunction &MF) {
     while (!Worklist.empty()) {
       MachineBasicBlock *MBB = OrderToBB[Worklist.top()];
       Worklist.pop();
-      MBBJoined =
-          join(*MBB, OutLocs, InLocs, VarLocIDs, Visited, ArtificialBlocks);
+      MBBJoined = join(*MBB, OutLocs, InLocs, VarLocIDs, Visited,
+                       ArtificialBlocks, PendingInLocs);
       Visited.insert(MBB);
       if (MBBJoined) {
         MBBJoined = false;
@@ -1266,11 +1308,13 @@ bool LiveDebugValues::ExtendRanges(MachineFunction &MF) {
         // Now that we have started to extend ranges across BBs we need to
         // examine spill instructions to see whether they spill registers that
         // correspond to user variables.
+        // First load any pending inlocs.
+        OpenRanges.insertFromLocSet(PendingInLocs[MBB], VarLocIDs);
         for (auto &MI : *MBB)
-          OLChanged |=
               process(MI, OpenRanges, OutLocs, VarLocIDs, Transfers,
                       DebugEntryVals, transferChanges, OverlapFragments,
                       SeenFragments);
+        OLChanged |= transferTerminator(MBB, OpenRanges, OutLocs, VarLocIDs);
 
         // Add any DBG_VALUE instructions necessitated by spills.
         for (auto &TR : Transfers)
@@ -1298,6 +1342,10 @@ bool LiveDebugValues::ExtendRanges(MachineFunction &MF) {
     assert(Pending.empty() && "Pending should be empty");
   }
 
+  // Deferred inlocs will not have had any DBG_VALUE insts created; do
+  // that now.
+  flushPendingLocs(PendingInLocs, VarLocIDs);
+
   LLVM_DEBUG(printVarLocInMBB(MF, OutLocs, VarLocIDs, "Final OutLocs", dbgs()));
   LLVM_DEBUG(printVarLocInMBB(MF, InLocs, VarLocIDs, "Final InLocs", dbgs()));
   return Changed;