[RFC] Rewrite the way we generate debug locations for variables.

[samsonov]

Summary

Completely change the way we turn DBG_VALUE machine instructions
into location information for local variables. This is inspired by discussion
in r128327 review thread.

We used to terminate location range for each variable at the end of the basic
block with DBG_VALUE for it. This is incorrect, e.g. the register used to hold
the variable location may be never clobbered in the entire function, so one
DBG_VALUE is enough to define the location of a variable in the whole function.

Now we actually analyze the control flow: calculate the presumed location of
a variable at the end and the beginning of each basic block. For example,
if the location of a variable is the same at the end of all predecessors of
basic block B, we may assume that we know the location of a variable in B,
and avoid terminating the location range too early.

I think it's pretty much all we can do w/o actually changing the CodeGen for
DBG_VALUE instructions.

If you agree with the direction of this patch, I should probably test it
extensively. There are many concerns:
0) Correctness. LLVM/Clang regression tests don't really check anything -

we should run it on gdb test-suite or smth. comprehensive.

1. Debug info size. We should verify that .debug_loc section wouldn't explode.
2. Compilation time - in the worst case we smth. like O(NumVars * NumInstructions), which can be a lot for large functions.

[samsonov]

clang-formatize the new code

[dblaikie]

Quick gut-reaction: that class is big enough that it deserves its own file.
(I realize you're just looking for a sanity check of the approach - and
I'll try to express some opinion on that too, though I don't think I'm
fully qualified to be the arbiter for this increased (though probably
justified) complexity - I don't have nearly enough knowledge about the
backend, location information, and other ways to approach this problem -
I'll try to ask around)

• David

[dblaikie]

I also /really/ wish we could get this info from register allocation or
something rather than recomputing it. CC'ing lhames here in case he's got
ideas. Will pester him via other channels as well.

[samsonov]

I don't think we're doing the work of register allocator here, though this may look similar :) Instead of tracking the "liveness" of registers, we're calculating the "liveness" of DBG_VALUE instructions, which are tied to user variables, and only occasionally - to registers.

[aprantl]

Hi Alexey,

thanks for tackling this problem!

as for quick gut reactions:
Having some doxygen-style comments for each of the member functions of DbgValueHistoryCalculator would make reviewing this patch a lot easier.

As for testing, in addition to the gdb test suite, there is also the lldb test suite, and if you’re looking for a place to put in more end-to-end debug-info tests, there is the llvm debuginfo-tests repository which runs on both lldb and gdb buildbots. (There are only a handful of tests in there right now, but it doesn’t have to stay that way.) On the llvm side this will need at least a test exercising llc | llvm-dwarfdump.

This is a really naive question, but it will help me understand your decisions better: would it be possible to change codegen to insert a DBG_VALUE into every MC block that post-dominates the basic block that contained the original dbg.declare/value intrinsic?

cheers,
adrian

[samsonov]

Hi Adrian,

Sorry for the lack of doxygen-comments for functions, I'll add them with the next version of the patch.

This is a really naive question, but it will help me understand your decisions better: would it be possible to change codegen to insert a DBG_VALUE into every MC block that post-dominates the basic block that contained the original dbg.declare/value intrinsic?

This might make things a bit easier, but:
a) this would bloat the MachineFunction, and IIRC David mentioned that some people complain about the number of DBG_VALUE calls already.
b) This might be confusing. E.g.

BB1:
  DBG_VALUE: var "x" is in %rax
  clobber %rax
  jmp BB2
BB2:
  // we probably shouldn't insert DBG_VALUE for "x" here, it's inaccessible.

or

BB1:
  DBG_VALUE: var "x" is in %rax
  jmp BB3
BB2:
  DBG_VALUE: var "x" is in %rbx
  jmp BB3
BB3:
  // what DBG_VALUE for "x" should we insert here?

We probably can add a pass to CodeGen that would perform similar (to this patch) kind of analysis and add additional DBG_VALUE calls, making debug info generation more straightforward. Here, instead, I try to not make any assumptions about the CodeGen output and do the best I can, w/o modifying the code.

[probinson]

I really really really wish I had time to review this now.
For the moment some kibitzing based on prior experience.

-----Original Message-----
From: [e-mail] [mailto:llvm-commits-
[e-mail]] On Behalf Of Alexey Samsonov
Sent: Thursday, January 30, 2014 8:20 AM
To: [e-mail]; [e-mail]
Cc: [e-mail]
Subject: [PATCH] [RFC] Rewrite the way we generate debug locations for
variables.

Hi dblaikie,

Completely change the way we turn DBG_VALUE machine instructions
into location information for local variables. This is inspired by
discussion
in r128327 review thread.

We used to terminate location range for each variable at the end of the
basic
block with DBG_VALUE for it. This is incorrect, e.g. the register used
to hold
the variable location may be never clobbered in the entire function, so
one
DBG_VALUE is enough to define the location of a variable in the whole
function.

Terminating at that point is the safe thing to do. Working out
when it's safe not to terminate is the tricky part, yeah.

Now we actually analyze the control flow: calculate the presumed
location of
a variable at the end and the beginning of each basic block. For
example,
if the location of a variable is the same at the end of all predecessors
of
basic block B, we may assume that we know the location of a variable in
B,

Sure.

and avoid terminating the location range too early.

Whoa. Looking at predecessors can tell you where something is on
block entry, but that's not the same as being able to not terminate
a range. Predecessors are a control-flow thing, ranges are a
physical-layout thing. Maybe you account for this in your patch,
like I said I haven't looked at it yet, but it's a trap for the
unwary.

--paulr

I think it's pretty much all we can do w/o actually changing the CodeGen
for
DBG_VALUE instructions.

If you agree with the direction of this patch, I should probably test it
extensively. There are many concerns:
0) Correctness. LLVM/Clang regression tests don't really check anything

• we should run it on gdb test-suite or smth. comprehensive.
• Debug info size. We should verify that .debug_loc section wouldn't

explode.

2. Compilation time - in the worst case we smth. like O(NumVars * NumInstructions), which can be a lot for large functions.

http://llvm-reviews.chandlerc.com/D2658

Files:

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

[probinson]

-----Original Message-----
From: [e-mail] [mailto:llvm-commits-
[e-mail]] On Behalf Of Adrian Prantl
Sent: Thursday, January 30, 2014 12:41 PM
To: [e-mail]
Cc: [e-mail]
Subject: Re: [PATCH] [RFC] Rewrite the way we generate debug locations
for variables.

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

 static bool isDbgValueInDefinedReg(const MachineInstr *MI) {
   assert(MI->isDebugValue() && "Invalid DBG_VALUE machine instruction!");
   return MI->getNumOperands() == 3 && MI->getOperand(0).isReg() &&
          MI->getOperand(0).getReg() &&
          (MI->getOperand(1).isImm() ||
           (MI->getOperand(1).isReg() && MI->getOperand(1).getReg() == 0U));
 }
 
+static bool isDbgValueNonTruncating(const MachineInstr *MI) {
+  assert(MI->isDebugValue() && "Invalid DBG_VALUE machine instruction!");
+  return MI->getNumOperands() > 1 && MI->getOperand(0).isReg() &&
+         !MI->getOperand(0).getReg();
+}
+
 // Get .debug_loc entry for the instruction range starting at MI.
 static DotDebugLocEntry getDebugLocEntry(AsmPrinter *Asm,
                                          const MCSymbol *FLabel,
                                          const MCSymbol *SLabel,
                                          const MachineInstr *MI) {
   const MDNode *Var = MI->getOperand(MI->getNumOperands() - 1).getMetadata();
 
   assert(MI->getNumOperands() == 3);
for (SmallVectorImpl<const MDNode *>::const_iterator
     for (SmallVectorImpl<const MachineInstr *>::const_iterator
              HI = History.begin(),
              HE = History.end();
          HI != HE; ++HI) {
       const MachineInstr *Begin = *HI;
       assert(Begin->isDebugValue() && "Invalid History entry");
 
       // Check if DBG_VALUE is truncating a range.
-      if (Begin->getNumOperands() > 1 && Begin->getOperand(0).isReg() &&
-          !Begin->getOperand(0).getReg())
+      if (isDbgValueNonTruncating(Begin))
         continue;
 
       // Compute the range for a register location.
       const MCSymbol *FLabel = getLabelBeforeInsn(Begin);
       const MCSymbol *SLabel = 0;
 
       if (HI + 1 == HE)
         // If Begin is the last instruction in History then its value is valid
for (SmallVectorImpl<InsnRange>::const_iterator RI = Ranges.begin(),
       assert(RI->first && "InsnRange does not have first instruction!");
       assert(RI->second && "InsnRange does not have second instruction!");
       requestLabelBeforeInsn(RI->first);
       requestLabelAfterInsn(RI->second);
     }
   }
 }
 
+static const MDNode *getVarFromDbgValue(const MachineInstr *MI) {
+  assert(MI->isDebugValue());
+  return MI->getOperand(MI->getNumOperands() - 1).getMetadata();
+}
+
+namespace {
+
+class DbgValueHistoryCalculator {
+  const MachineFunction *MF;
+  const TargetRegisterInfo *TRI;
+
+  // We need to enumerate MBB in the order they are emitted into an object file.
+  unsigned NumMBBs;
+  DenseMap<const MachineBasicBlock *, unsigned> MBBToIndex;
+  std::vector<const MachineBasicBlock *> OrderedMBBs;
+
+  typedef SmallVectorImpl<const MachineInstr *> MachineInstrVec;
+  /// Matches DwarfDebug::DbgValueHistoryMap.
+  typedef DenseMap<const MDNode *, SmallVector<const MachineInstr *, 4> >
+  DbgValueHistoryMap;
+  typedef SmallVectorImpl<const MachineBasicBlock *> MachineBasicBlockVec;
+  typedef DenseMap<const MDNode *, SmallVector<const MachineBasicBlock *, 4> >
+  VarToMBBVecMap;
+  typedef DenseMap<const MachineBasicBlock *, const MachineInstr *>
+  MBBToDbgValueMap;
+
+  // The following fields are used to calculate history for each variable.
+  /// Variable for which we calculate history.
+  const MDNode *CurVar;
+  /// Number of MBB predecessors, for which we already know the at-end location.
+  DenseMap<const MachineBasicBlock *, unsigned> MBBPredsCalculated;
+  /// DBG_VALUE specifying the variable location at the beginning of MBB.
+  /// Null if it is ambiguous or unknown.
+  MBBToDbgValueMap DbgValueAtMBBStart;
+  /// DBG_VALUE specifying the variable location at the end of MBB.
+  /// Null if it is ambiguous or unknown.
+  MBBToDbgValueMap DbgValueAtMBBEnd;
+  /// MBBs for which we already know at-beginning and at-end variable location.
+  SmallPtrSet<const MachineBasicBlock *, 8> HandledMBBs;
+  /// Priority queue, specifying which MBB should be handled next (MBBs are
+  /// ordered in the same way they are emitted into an object file).
+  std::set<std::pair<unsigned, const MachineBasicBlock *> > MBBQueue;
+  /// Instructions which are part of variable location history, for each MBB.
+  std::vector<SmallVector<const MachineInstr *, 4> > HistoryForMBB;
+
+public:
+  DbgValueHistoryCalculator(const MachineFunction *MF,
+                            const TargetRegisterInfo *TRI)
+      : MF(MF), TRI(TRI) {
+    NumMBBs = 0;
+    for (MachineFunction::const_iterator I = MF->begin(), E = MF->end(); I != E;
+         ++I) {
+      const MachineBasicBlock *MBB = I;
+      OrderedMBBs.push_back(MBB);
+      MBBToIndex[MBB] = NumMBBs++;
+    }
+  }
+
+  void calculateHistory(DbgValueHistoryMap &Result) {
+    VarToMBBVecMap MBBWithDbgValue;
+    // For each variable, collect MBBs with at least one DBG_VALUE instruction.
+    for (MachineFunction::const_iterator I = MF->begin(), E = MF->end(); I != E;
+         ++I) {
+      const MachineBasicBlock *MBB = I;
+      for (MachineBasicBlock::const_iterator II = I->begin(), IE = I->end();
+           II != IE; ++II) {
+        const MachineInstr *MI = II;
+        if (!MI->isDebugValue())
+          continue;
+        const MDNode *Var = getVarFromDbgValue(MI);
+        MachineBasicBlockVec &MBBForVar = MBBWithDbgValue[Var];
+        if (!MBBForVar.empty() && MBBForVar.back() == MBB)
+          continue;
+        MBBForVar.push_back(MBB);
+      }
+    }
+
+    // Build a history for each user variable.
+    for (VarToMBBVecMap::const_iterator I = MBBWithDbgValue.begin(),
+                                        E = MBBWithDbgValue.end();
+         I != E; ++I) {
+      const MDNode *Var = I->first;
+      calculateHistoryForVariable(Var, I->second, Result[Var]);
+    }
+  }
+
+private:
+  void calculateHistoryForVariable(const MDNode *Var,
+                                   const MachineBasicBlockVec &MBBForVar,
+                                   MachineInstrVec &Result) {
+    prepareToCalculateHistoryForVar(Var);
+    // Step 1. Iterate over all MBB with at least one DBG_VALUE in them.
+    for (MachineBasicBlockVec::const_iterator I = MBBForVar.begin(),
+                                              E = MBBForVar.end();
+         I != E; ++I) {
+      calculateHistoryForMBB(*I);
+    }
+    // Step 2. Iterate over all MBB for which we now know var location at
+    // the beginning of MBB.
+    while (!MBBQueue.empty()) {
+      const MachineBasicBlock *MBB = MBBQueue.begin()->second;
+      MBBQueue.erase(MBBQueue.begin());
+      propagateThroughMBB(MBB, DbgValueAtMBBStart[MBB]);
+      HandledMBBs.insert(MBB);
+    }
+    // Step 3. Iterate over all MBB with unknown var location at the beginning
+    // of MBB.
+    for (MachineFunction::const_iterator I = MF->begin(), E = MF->end(); I != E;
+         ++I) {
+      const MachineBasicBlock *MBB = I;
+      if (MBB != MF->begin() && HandledMBBs.count(MBB) == 0)
+        propagateThroughMBB(MBB, 0);
+    }
+    // Step 4. Merge histories for all MBBs into a single vector.
+    mergeHistoryForVar(Result);
+  }
+
+  void prepareToCalculateHistoryForVar(const MDNode *Var) {
+    CurVar = Var;
+    MBBPredsCalculated.clear();
+    DbgValueAtMBBStart.clear();
+    DbgValueAtMBBEnd.clear();
+    HandledMBBs.clear();
+    MBBQueue.clear();
+    std::vector<SmallVector<const MachineInstr *, 4> > NewHistoryForMBB(
+        NumMBBs);
+    HistoryForMBB.swap(NewHistoryForMBB);
+  }
+
+  void calculateHistoryForMBB(const MachineBasicBlock *MBB) {
+    unsigned MBBIndex = MBBToIndex[MBB];
+    const MachineInstr *CurDbgValue = 0;
+    unsigned CurRegister = -1U;
+    for (MachineBasicBlock::const_iterator I = MBB->begin(), E = MBB->end();
+         I != E; ++I) {
+      const MachineInstr *MI = I;
+      if (MI->isDebugValue()) {
+        if (CurVar == getVarFromDbgValue(MI)) {
+          CurDbgValue = MI;
+          CurRegister = getDefinedRegFromDbgValue(MI);
+          HistoryForMBB[MBBIndex].push_back(MI);
+        }
+      } else if (instructionClobbersRegister(MI, CurRegister)) {
+        CurDbgValue = 0;
+        CurRegister = -1U;
+        HistoryForMBB[MBBIndex].push_back(MI);
+      }
+    }
+    propagateDbgValueToSuccessors(MBB, CurDbgValue);
+  }
+
+  unsigned getDefinedRegFromDbgValue(const MachineInstr *MI) const {
+    return (MI != 0 && isDbgValueInDefinedReg(MI)) ? MI->getOperand(0).getReg()
+                                                   : -1U;
+  }
+
+  bool instructionClobbersRegister(const MachineInstr *MI, unsigned Reg) const {
+    if (Reg == -1U)
+      return false;
+    for (MachineInstr::const_mop_iterator MOI = MI->operands_begin(),
+                                          MOE = MI->operands_end();
+         MOI != MOE; ++MOI) {
+      if (!MOI->isReg() || !MOI->isDef() || !MOI->getReg())
+        continue;
+      for (MCRegAliasIterator AI(MOI->getReg(), TRI, true); AI.isValid();
+           ++AI) {
+        if (*AI == Reg)
+          return true;
+      }
+    }
+    return false;
+  }
+
+  void propagateDbgValueToSuccessors(const MachineBasicBlock *MBB,
+                                     const MachineInstr *DbgValueAtEnd) {
+    DbgValueAtMBBEnd[MBB] = DbgValueAtEnd;
+    // If var location is unknown at the end of MBB, don't propagate anything.
+    if (DbgValueAtEnd == 0)
+      return;
+    for (MachineBasicBlock::const_succ_iterator I = MBB->succ_begin(),
+                                                E = MBB->succ_end();
+         I != E; ++I) {
+      const MachineBasicBlock *Succ = *I;
+      std::pair<MBBToDbgValueMap::iterator, bool> AtSuccStart =
+          DbgValueAtMBBStart.insert(std::make_pair(Succ, DbgValueAtEnd));
+      if (!AtSuccStart.second) {
+        // Verify that var locations at the beginning of Succ are identical for
+        // all control flow paths.
+        const MachineInstr *DbgValueAtSuccStart = AtSuccStart.first->second;
+        if (DbgValueAtSuccStart == 0 ||
+            !DbgValueAtSuccStart->isIdenticalTo(DbgValueAtEnd)) {
+          AtSuccStart.first->second = 0;
+          continue;
+        }
+      }
+      // If all predecessors of Succ provided the same var location at the
+      // beginning of Succ, we are ready to handle it.
+      if (++MBBPredsCalculated[Succ] == Succ->pred_size())
+        MBBQueue.insert(std::make_pair(MBBToIndex[Succ], Succ));
+    }
+  }
+
+  void propagateThroughMBB(const MachineBasicBlock *MBB,
+                           const MachineInstr *DbgValueAtStart) {
+    unsigned MBBIndex = MBBToIndex[MBB];
+    if (MBBIndex == 0)
+      return;
+    const MachineBasicBlock *PrevMBB = OrderedMBBs[MBBIndex - 1];
+
+    // We can't insert DBG_VALUE instructions at this point, so the only way we
+    // can
+    // have specific var location at the beginning of MBB is when at-beginning
+    // var location for MBB is the same as at-end var location for previous MBB.
+    // If it is different, or if at-beginning var location for MBB is unknown,
+    // we must clobber the at-end var location from previous MBB.
+    const MachineInstr *DbgValueAtPrevEnd = 0;
+    MBBToDbgValueMap::const_iterator AtPrevEnd = DbgValueAtMBBEnd.find(PrevMBB);
+    bool ShouldClobberHistory = false;
+    if (AtPrevEnd == DbgValueAtMBBEnd.end()) {
+      // If at-end var location for previous MBB is unknown, clobber history
+      // just in case.
+      ShouldClobberHistory = true;
+    } else {
+      DbgValueAtPrevEnd = AtPrevEnd->second;
+    }
+    if (DbgValueAtStart == 0) {
+      if (DbgValueAtPrevEnd != 0) {
+        // Assume that it's fine to continue the range for non register located
+        // variables.
+        // FIXME: Is this assumption really correct?
+        if (!isDbgValueInDefinedReg(DbgValueAtPrevEnd))
+          DbgValueAtStart = DbgValueAtPrevEnd;
+        else
+          ShouldClobberHistory = true;
+      }
+    } else {
+      // If we know at-start location for current MBB, clober history if it
+      // doesn't correspond to at-end location of previous MBB.
+      if (DbgValueAtPrevEnd == 0 ||
+          !DbgValueAtStart->isIdenticalTo(DbgValueAtPrevEnd))
+        ShouldClobberHistory = true;
+    }
+
+    const MachineInstr *CurDbgValue = DbgValueAtStart;
+    unsigned CurRegister = getDefinedRegFromDbgValue(CurDbgValue);
+    for (MachineBasicBlock::const_iterator I = MBB->begin(), E = MBB->end();
+         I != E; ++I) {
+      const MachineInstr *MI = I;
+      if (MI->isDebugValue()) {
+        // Don't do anything for this MBB - we've handled all the following
+        // instructions in calculateHistoryForMBB().
+        if (CurVar == getVarFromDbgValue(MI))
+          return;
+      } else if (ShouldClobberHistory ||
+                 instructionClobbersRegister(MI, CurRegister)) {
+        // dbg val is unknown from now till the rest of the block.
+        HistoryForMBB[MBBIndex].insert(HistoryForMBB[MBBIndex].begin(), MI);
+        CurDbgValue = 0;
+        CurRegister = -1U;
+        break;
+      }
+    }
+    propagateDbgValueToSuccessors(MBB, CurDbgValue);
+  }
+
+  void mergeHistoryForVar(MachineInstrVec &Result) {
+    Result.clear();
+    for (int i = 0, n = HistoryForMBB.size(); i < n; ++i) {
+      for (int ii = 0, nn = HistoryForMBB[i].size(); ii < nn; ++ii) {
+        const MachineInstr *MI = HistoryForMBB[i][ii];
+        // Saniry check: DBG_VALUE should point to the current var only.
+        assert(!MI->isDebugValue() || CurVar == getVarFromDbgValue(MI));
+        // Ignore non-truncating debug info entries.
+        if (MI->isDebugValue() && isDbgValueNonTruncating(MI))
+          continue;
+        if (Result.empty()) {
+          // Start history with the first dbg_value instruction.
+          if (MI->isDebugValue())
+            Result.push_back(MI);
+          continue;
+        }
+        // Coalesce two identical DBG_VALUE instructions.
+        if (MI->isDebugValue() && MI->isIdenticalTo(Result.back()))
+          continue;
+        // Don't add to history two consecutive range-breaking instructions.
+        if (!MI->isDebugValue() && !Result.back()->isDebugValue())
+          continue;
+        Result.push_back(MI);
+      }
+    }
+  }
+};
+
+} // namespace
+
 // Gather pre-function debug information.  Assumes being called immediately
 // after the function entry point has been emitted.
 void DwarfDebug::beginFunction(const MachineFunction *MF) {
   CurFn = MF;
 
   // If there's no debug info for the function we're not going to do anything.
   if (!MMI->hasDebugInfo())
     return;
void DwarfDebug::beginFunction(const MachineFunction *MF) {
   if (Asm->getObjFileLowering().getTextSection() != Asm->getCurrentSection())
     UsedNonDefaultText = true;
 
   // Emit a label for the function so that we have a beginning address.
   FunctionBeginSym = Asm->GetTempSymbol("func_begin", Asm->getFunctionNumber());
   // Assumes in correct section after the entry point.
   Asm->OutStreamer.EmitLabel(FunctionBeginSym);
 
-  const TargetRegisterInfo *TRI = Asm->TM.getRegisterInfo();
-  // LiveUserVar - Map physreg numbers to the MDNode they contain.
-  std::vector<const MDNode *> LiveUserVar(TRI->getNumRegs());
-
   for (MachineFunction::const_iterator I = MF->begin(), E = MF->end(); I != E;
        ++I) {
-    bool AtBlockEntry = true;
     for (MachineBasicBlock::const_iterator II = I->begin(), IE = I->end();
          II != IE; ++II) {
       const MachineInstr *MI = II;
-
       if (MI->isDebugValue()) {
         assert(MI->getNumOperands() > 1 && "Invalid machine instruction!");
-
-        // Keep track of user variables.
-        const MDNode *Var =
-            MI->getOperand(MI->getNumOperands() - 1).getMetadata();
-
-        // Variable is in a register, we need to check for clobbers.
-        if (isDbgValueInDefinedReg(MI))
-          LiveUserVar[MI->getOperand(0).getReg()] = Var;
-
-        // Check the history of this variable.
-        SmallVectorImpl<const MachineInstr *> &History = DbgValues[Var];
-        if (History.empty()) {
+        const MDNode *Var = getVarFromDbgValue(MI);
+        if (DbgValues.count(Var) == 0) {
+          // Create empty history entry for Var.
+          DbgValues[Var].clear();
           UserVariables.push_back(Var);
-          // The first mention of a function argument gets the FunctionBeginSym
-          // label, so arguments are visible when breaking at function entry.
-          DIVariable DV(Var);
-          if (DV.isVariable() && DV.getTag() == dwarf::DW_TAG_arg_variable &&
-              getDISubprogram(DV.getContext()).describes(MF->getFunction()))
-            LabelsBeforeInsn[MI] = FunctionBeginSym;
-        } else {
-          // We have seen this variable before. Try to coalesce DBG_VALUEs.
-          const MachineInstr *Prev = History.back();
-          if (Prev->isDebugValue()) {
-            // Coalesce identical entries at the end of History.
-            if (History.size() >= 2 &&
-                Prev->isIdenticalTo(History[History.size() - 2])) {
-              DEBUG(dbgs() << "Coalescing identical DBG_VALUE entries:\n"
-                           << "\t" << *Prev << "\t"
-                           << *History[History.size() - 2] << "\n");
-              History.pop_back();
-            }
-
-            // Terminate old register assignments that don't reach MI;
-            MachineFunction::const_iterator PrevMBB = Prev->getParent();
-            if (PrevMBB != I && (!AtBlockEntry || llvm::next(PrevMBB) != I) &&
-                isDbgValueInDefinedReg(Prev)) {
-              // Previous register assignment needs to terminate at the end of
-              // its basic block.
-              MachineBasicBlock::const_iterator LastMI =
-                  PrevMBB->getLastNonDebugInstr();
-              if (LastMI == PrevMBB->end()) {
-                // Drop DBG_VALUE for empty range.
-                DEBUG(dbgs() << "Dropping DBG_VALUE for empty range:\n"
-                             << "\t" << *Prev << "\n");
-                History.pop_back();
-              } else if (llvm::next(PrevMBB) != PrevMBB->getParent()->end())
-                // Terminate after LastMI.
-                History.push_back(LastMI);
-            }
-          }
-        }
-        History.push_back(MI);
       } else {
-        // Not a DBG_VALUE instruction.
-        if (!MI->isLabel())
-          AtBlockEntry = false;
-
         // First known non-DBG_VALUE and non-frame setup location marks
         // the beginning of the function body.
         if (!MI->getFlag(MachineInstr::FrameSetup) &&
             (PrologEndLoc.isUnknown() && !MI->getDebugLoc().isUnknown()))
           PrologEndLoc = MI->getDebugLoc();
-
-        // Check if the instruction clobbers any registers with debug vars.
-        for (MachineInstr::const_mop_iterator MOI = MI->operands_begin(),
-                                              MOE = MI->operands_end();
-             MOI != MOE; ++MOI) {
-          if (!MOI->isReg() || !MOI->isDef() || !MOI->getReg())
-            continue;
-          for (MCRegAliasIterator AI(MOI->getReg(), TRI, true); AI.isValid();
-               ++AI) {
-            unsigned Reg = *AI;
-            const MDNode *Var = LiveUserVar[Reg];
-            if (!Var)
-              continue;
-            // Reg is now clobbered.
-            LiveUserVar[Reg] = 0;
-
-            // Was MD last defined by a DBG_VALUE referring to Reg?
-            DbgValueHistoryMap::iterator HistI = DbgValues.find(Var);
-            if (HistI == DbgValues.end())
-              continue;
-            SmallVectorImpl<const MachineInstr *> &History = HistI->second;
-            if (History.empty())
-              continue;
-            const MachineInstr *Prev = History.back();
-            // Sanity-check: Register assignments are terminated at the end of
-            // their block.
-            if (!Prev->isDebugValue() || Prev->getParent() != MI->getParent())
-              continue;
-            // Is the variable still in Reg?
-            if (!isDbgValueInDefinedReg(Prev) ||
-                Prev->getOperand(0).getReg() != Reg)
-              continue;
-            // Var is clobbered. Make sure the next instruction gets a label.
-            History.push_back(MI);
-          }
-        }
       }
     }
   }
 
+  DbgValueHistoryCalculator HistCalc(MF, Asm->TM.getRegisterInfo());
+  HistCalc.calculateHistory(DbgValues);
+
   for (DbgValueHistoryMap::iterator I = DbgValues.begin(), E = DbgValues.end();
        I != E; ++I) {
-    SmallVectorImpl<const MachineInstr *> &History = I->second;
+    const SmallVectorImpl<const MachineInstr *> &History = I->second;
     if (History.empty())
       continue;
-
-    // Make sure the final register assignments are terminated.
-    const MachineInstr *Prev = History.back();
-    if (Prev->isDebugValue() && isDbgValueInDefinedReg(Prev)) {
-      const MachineBasicBlock *PrevMBB = Prev->getParent();
-      MachineBasicBlock::const_iterator LastMI =
-          PrevMBB->getLastNonDebugInstr();
-      if (LastMI == PrevMBB->end())
-        // Drop DBG_VALUE for empty range.
-        History.pop_back();
-      else if (PrevMBB != &PrevMBB->getParent()->back()) {
-        // Terminate after LastMI.
-        History.push_back(LastMI);
-      }
-    }
+    const MDNode *Var = I->first;
+    // The first mention of a function argument gets the FunctionBeginSym
+    // label, so arguments are visible when breaking at function entry.
+    DIVariable DV(Var);
+    if (DV.isVariable() && DV.getTag() == dwarf::DW_TAG_arg_variable &&
+        getDISubprogram(DV.getContext()).describes(MF->getFunction()))
+      LabelsBeforeInsn[History[0]] = FunctionBeginSym;
     // Request labels for the full history.
     for (unsigned i = 0, e = History.size(); i != e; ++i) {
       const MachineInstr *MI = History[i];
       if (MI->isDebugValue())
         requestLabelBeforeInsn(MI);
       else
         requestLabelAfterInsn(MI);
     }