[DebugInfo] Improve handling of clobbered fragments

[dstenb]

Summary

Currently the DbgValueHistorymap only keeps track of clobbered registers
for the last debug value that it has encountered. This could lead to
preceding register-described debug values living on longer in the
location lists than they should. See PR40283 for an example. This
patch does not introduce tracking of multiple registers, but changes
the DbgValueHistoryMap structure to allow for that in a follow-up
patch. This patch is not NFC, as it at least fixes two bugs in
DwarfDebug (both are covered in the new clobbered-fragments.mir test):

• If a debug value was clobbered (its End pointer set), the value would still be added to OpenRanges, meaning that the succeeding location list entries could potentially contain stale values.

• If a debug value was clobbered, and there were non-overlapping fragments that were still live after the clobbering, DwarfDebug would not create a location list entry starting directly after the clobbering instruction. This meant that the location list could have a gap until the next debug value for the variable was encountered.

Before this patch, the history map was represented by <Begin, End>
pairs, where a new pair was created for each new debug value. When
dealing with partially overlapping register-described debug values, such
as in the following example:

DBG_VALUE $reg2, $noreg, !1, !DIExpression(DW_OP_LLVM_fragment, 32, 32)
[...]
DBG_VALUE $reg3, $noreg, !1, !DIExpression(DW_OP_LLVM_fragment, 64, 32)
[...]
$reg2 = insn1
[...]
$reg3 = insn2

the history map would then contain the entries [<DV1, insn1>, [<DV2, insn2>].
This would leave it up to the users of the map to be aware of
the relative order of the instructions, which e.g. could make
DwarfDebug::buildLocationList() needlessly complex. Instead, this patch
makes the history map structure monotonically increasing by dropping the
End pointer, and replacing that with explicit clobbering entries in the
vector. Each debug value has an "end index", which if set, points to the
entry in the vector that ends the debug value. The ending entry can
either be an overlapping debug value, or an instruction which clobbers
the register that the debug value is described by. The ending entry's
instruction can thus either be excluded or included in the debug value's
range. If the end index is not set, the debug value that the entry
introduces is valid until the end of the function.

Changes to test cases:

• DebugInfo/X86/pieces-3.ll: The range of the first DBG_VALUE, which describes that the fragment (0, 64) is located in RDI, was incorrectly ended by the clobbering of RAX, which the second (non-overlapping) DBG_VALUE was described by. With this patch we get a second entry that only describes RDI after that clobbering.
• DebugInfo/ARM/partial-subreg.ll: This test seems to indiciate a bug in LiveDebugValues that is caused by it not being aware of fragments. I have added some comments in the test case about that. Also, before this patch DwarfDebug would incorrectly include a register-described debug value from a preceding block in a location list entry.

[dstenb]

This patch depends on four preceding refactoring commits. I avoided adding reviewers to two of them (I should perhaps have done the same for the other two) to avoid spamming all of you (who probably deals with enough revisions as it is), but they are up for review.

[aprantl]

What's the performance impact of this change? Specifically, I'd be interested in the wall clock time difference for building an RelWithDebInfo+asan build of clang.

include/llvm/CodeGen/DbgEntityHistoryCalculator.h

25–42

///

43–44

This should be the second paragraph of the class doxygen comment above (i.e., the non-brief part).

45–50

/// etc..

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

111

Do we really need the properties of a std::map here and in the line above?
So far every time I tried using a std::map in AsmPrinter a std::vector + std::sort turned out to be faster and to use less memory.

152

/// and \p Var

[aprantl]

(Asan created really large basic blocks and is therefore a great stress test for dbg value handling)

[dstenb]

In D59941#1447940, @aprantl wrote:

What's the performance impact of this change? Specifically, I'd be interested in the wall clock time difference for building an RelWithDebInfo+asan build of clang.

I applied the whole patch series (including the follow-up patch, D59942), and ran through five compilations with respectively without the patches using the following Cmake configuration:

cmake -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo -DLLVM_USE_SANITIZER=Address -DLLVM_TARGETS_TO_BUILD='X86' -DLLVM_PARALLEL_LINK_JOBS=4 -DLLVM_USE_LINKER=gold

I ran the compilations on an 8-thread i7-8650U machine with 32 GBs of RAM.

This gave the following measurements:

Master:
real time: 37m34s, 37m32s, 37m31s, 37m30s, 37m28s
average real time: 37m31s

Patched:
real time: 37m40s, 37m38s, 41m43s, 37m34s, 37m37s
average real time: 38m26s

As seen, the wall clock for four of the five patched runs is increased by a few seconds compared to master. The run that increases by four minutes I suspect is simply due to some scheduled work during the night.

[dstenb]

Doxygenize comments.

[dstenb]

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

111

There is at least be no need for iterators to be valid after insertion/deletion, nor is the iteration order important for determinism, which I guess is the two of the main benefits of std::map compared to LLVM's own map implementations. I tried switching both to DenseMap, but I did at least not see any effect on the build time when building the RelWithDebInfo+asan clang binary, but I'm not sure what conclusions I should draw from that.

I will look into switching both to sorted vectors.

[aprantl]

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

111

... or sorted llvm::SmallVectors.

[dblaikie]

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

111

Usually the issue for this tradeoff (map/set (be it standard or LLVM's custom data structures) versus sorted vector (be it std::vector or SmallVector)) is whether or not there's a separate "gather" and "lookup" phase.

If lookups are required while the data structure is also growing, then it's probably inefficient to keep resorting the data structure to keep those lookups efficient.

if there's a defined point where all building is done (& no querying - so you probably know/can guarantee you aren't inserting duplicates without having to test), a single sort and then purely lookup from that point on.

Is that the case here?

[dstenb]

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

111

I don't think this is the case here. There is not really separate "gather" and "lookup" phases for the maps, and instead the insertion and querying is interleaved. Both maps may grow/shrink when encountering new debug values or clobbering instructions throughout the iteration of the machine function, so there is not really a defined point where we know we are done building the maps.

include/llvm/CodeGen/DbgEntityHistoryCalculator.h

 //===- llvm/CodeGen/DbgEntityHistoryCalculator.h ----------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CODEGEN_DBGVALUEHISTORYCALCULATOR_H
 #define LLVM_CODEGEN_DBGVALUEHISTORYCALCULATOR_H
 
 #include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include <utility>
 
 namespace llvm {
 
 class DILocalVariable;
 class MachineFunction;
 class MachineInstr;
 class TargetRegisterInfo;
 
-// For each user variable, keep a list of instruction ranges where this variable
-// is accessible. The variables are listed in order of appearance.
+/// For each user variable, keep a list of instruction ranges where this
+/// variable is accessible. The variables are listed in order of appearance.
+///
+/// There exist two types of history map entries:
+///
+/// * Debug value entry:
+///
+///   A new debug value becomes live. If the entry's \p EndIndex is \p NoEntry,
+///   the value is valid until the end of the function. For other values, the
+///   index points to the entry in the entry vector that ends this debug
+///   value. The ending entry can either be an overlapping debug value, or
+///   an instruction that clobbers the value.
+///
+/// * Clobbering entry:
+///
+///   This entry's instruction clobbers one or more preceding
+///   register-described debug values that have their end index
+///   set to this entry's position in the entry vector.
 class DbgValueHistoryMap {
-  // Each instruction range starts with a DBG_VALUE instruction, specifying the
-  // location of a variable, which is assumed to be valid until the end of the
-  // range. If end is not specified, location is valid until the start
-  // instruction of the next instruction range, or until the end of the
-  // function.
 public:
-  class Entry {
-    const MachineInstr *Begin;
-    const MachineInstr *End;
+  /// Index in the entry vector.
+  typedef size_t EntryIndex;
+
+  /// Special value to indicate that an entry is valid until the end of the
+  /// function.
+  static const EntryIndex NoEntry = std::numeric_limits<EntryIndex>::max();
 
+  class Entry {
   public:
-    Entry(const MachineInstr *Begin) : Begin(Begin), End(nullptr) {}
+    enum EntryKind { DbgValue, Clobber };
+
+    Entry(const MachineInstr *Instr, EntryKind Kind)
+        : Instr(Instr, Kind), EndIndex(NoEntry) {}
 
-    const MachineInstr *getBegin() const { return Begin; }
-    const MachineInstr *getEnd() const { return End; }
+    const MachineInstr *getInstr() const { return Instr.getPointer(); }
+    EntryIndex getEndIndex() const { return EndIndex; }
+    EntryKind getEntryKind() const { return Instr.getInt(); }
 
-    void endEntry(const MachineInstr &End);
+    bool isClobber() const { return getEntryKind() == Clobber; }
+    bool isDbgValue() const { return getEntryKind() == DbgValue; }
+    bool isEnded() const { return EndIndex != NoEntry; }
+
+    void endEntry(EntryIndex EndIndex);
+
+  private:
+    PointerIntPair<const MachineInstr *, 1, EntryKind> Instr;
+    EntryIndex EndIndex;
   };
   using Entries = SmallVector<Entry, 4>;
   using InlinedEntity = std::pair<const DINode *, const DILocation *>;
   using EntriesMap = MapVector<InlinedEntity, Entries>;
 
 private:
   EntriesMap VarEntries;
 
 public:
-  void startEntry(InlinedEntity Var, const MachineInstr &MI);
-  void endEntry(InlinedEntity Var, const MachineInstr &MI);
+  bool startDbgValue(InlinedEntity Var, const MachineInstr &MI,
+                     EntryIndex &NewIndex);
+  EntryIndex startClobber(InlinedEntity Var, const MachineInstr &MI);
 
   // Returns register currently describing @Var. If @Var is currently
   // unaccessible or is not described by a register, returns 0.
   unsigned getRegisterForVar(InlinedEntity Var) const;
 
+  Entry &getEntry(InlinedEntity Var, EntryIndex Index) {
+    auto &Entries = VarEntries[Var];
+    return Entries[Index];
+  }
+
   bool empty() const { return VarEntries.empty(); }
   void clear() { VarEntries.clear(); }
   EntriesMap::const_iterator begin() const { return VarEntries.begin(); }
   EntriesMap::const_iterator end() const { return VarEntries.end(); }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
   LLVM_DUMP_METHOD void dump() const;
 #endif

lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

 
 void CodeViewDebug::calculateRanges(
     LocalVariable &Var, const DbgValueHistoryMap::Entries &Entries) {
   const TargetRegisterInfo *TRI = Asm->MF->getSubtarget().getRegisterInfo();
 
   // Calculate the definition ranges.
   for (auto I = Entries.begin(), E = Entries.end(); I != E; ++I) {
     const auto &Entry = *I;
-    const MachineInstr *DVInst = Entry.getBegin();
+    if (!Entry.isDbgValue())
+      continue;
+    const MachineInstr *DVInst = Entry.getInstr();
     assert(DVInst->isDebugValue() && "Invalid History entry");
     // FIXME: Find a way to represent constant variables, since they are
     // relatively common.
     Optional<DbgVariableLocation> Location =
         DbgVariableLocation::extractFromMachineInstruction(*DVInst);
     if (!Location)
       continue;
 
if (Location->Register == 0 || Location->LoadChain.size() > 1)
 
       if (Var.DefRanges.empty() ||
           Var.DefRanges.back().isDifferentLocation(DR)) {
         Var.DefRanges.emplace_back(std::move(DR));
       }
     }
 
     // Compute the label range.
-    const MCSymbol *Begin = getLabelBeforeInsn(Entry.getBegin());
-    const MCSymbol *End = getLabelAfterInsn(Entry.getEnd());
-    if (!End) {
-      // This range is valid until the next overlapping bitpiece. In the
-      // common case, ranges will not be bitpieces, so they will overlap.
-      auto J = std::next(I);
-      const DIExpression *DIExpr = DVInst->getDebugExpression();
-      while (J != E &&
-             !DIExpr->fragmentsOverlap(J->getBegin()->getDebugExpression()))
-        ++J;
-      if (J != E)
-        End = getLabelBeforeInsn(J->getBegin());
-      else
-        End = Asm->getFunctionEnd();
-    }
+    const MCSymbol *Begin = getLabelBeforeInsn(Entry.getInstr());
+    const MCSymbol *End;
+    if (Entry.getEndIndex() != DbgValueHistoryMap::NoEntry) {
+      auto &EndingEntry = Entries[Entry.getEndIndex()];
+      End = EndingEntry.isDbgValue()
+                ? getLabelBeforeInsn(EndingEntry.getInstr())
+                : getLabelAfterInsn(EndingEntry.getInstr());
+    } else
 
     // If the last range end is our begin, just extend the last range.
     // Otherwise make a new range.
     SmallVectorImpl<std::pair<const MCSymbol *, const MCSymbol *>> &R =
         Var.DefRanges.back().Ranges;
     if (!R.empty() && R.back().second == Begin)
       R.back().second = End;
     else

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

 //===- llvm/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp -------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/DbgEntityHistoryCalculator.h"
 #include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineInstr.h"
 #include "llvm/CodeGen/MachineOperand.h"
 #include "llvm/CodeGen/TargetLowering.h"
 #include "llvm/CodeGen/TargetRegisterInfo.h"
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DebugLoc.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
 #include <map>
 #include <utility>
 
 using namespace llvm;
 
 #define DEBUG_TYPE "dwarfdebug"
 
+namespace {
+using EntryIndex = DbgValueHistoryMap::EntryIndex;
+}
+
 // If @MI is a DBG_VALUE with debug value described by a
 // defined register, returns the number of this register.
 // In the other case, returns 0.
 static unsigned isDescribedByReg(const MachineInstr &MI) {
   assert(MI.isDebugValue());
   assert(MI.getNumOperands() == 4);
   // If location of variable is described using a register (directly or
   // indirectly), this register is always a first operand.
   return MI.getOperand(0).isReg() ? MI.getOperand(0).getReg() : 0;
 }
 
-void DbgValueHistoryMap::startEntry(InlinedEntity Var, const MachineInstr &MI) {
+bool DbgValueHistoryMap::startDbgValue(InlinedEntity Var,
+                                       const MachineInstr &MI,
+                                       EntryIndex &NewIndex) {
   // Instruction range should start with a DBG_VALUE instruction for the
   // variable.
   assert(MI.isDebugValue() && "not a DBG_VALUE");
   auto &Entries = VarEntries[Var];
-  if (!Entries.empty() && Entries.back().getEnd() == nullptr &&
-      Entries.back().getBegin()->isIdenticalTo(MI)) {
+  if (!Entries.empty() && Entries.back().isDbgValue() &&
+      !Entries.back().isEnded() &&
+      Entries.back().getInstr()->isIdenticalTo(MI)) {
     LLVM_DEBUG(dbgs() << "Coalescing identical DBG_VALUE entries:\n"
-                      << "\t" << Entries.back().getBegin() << "\t" << MI
+                      << "\t" << Entries.back().getInstr() << "\t" << MI
                       << "\n");
-    return;
+    return false;
   }
-  Entries.emplace_back(&MI);
+  Entries.emplace_back(&MI, Entry::DbgValue);
+  NewIndex = Entries.size() - 1;
+  return true;
 }
 
-void DbgValueHistoryMap::endEntry(InlinedEntity Var, const MachineInstr &MI) {
+EntryIndex DbgValueHistoryMap::startClobber(InlinedEntity Var,
+                                            const MachineInstr &MI) {
   auto &Entries = VarEntries[Var];
-  assert(!Entries.empty() && "No range exists for variable!");
-  Entries.back().endEntry(MI);
+  Entries.emplace_back(&MI, Entry::Clobber);
+  return Entries.size() - 1;
 }
 
-void DbgValueHistoryMap::Entry::endEntry(const MachineInstr &MI) {
+void DbgValueHistoryMap::Entry::endEntry(EntryIndex Index) {
   // For now, instruction ranges are not allowed to cross basic block
   // boundaries.
-  assert(Begin->getParent() == MI.getParent());
-  assert(End == nullptr && "Range is already closed!");
-  End = &MI;
+  assert(isDbgValue() && "Setting end index for non-debug value");
+  assert(EndIndex == NoEntry && "End index has already been set");
+  EndIndex = Index;
 }
 
 unsigned DbgValueHistoryMap::getRegisterForVar(InlinedEntity Var) const {
   const auto &I = VarEntries.find(Var);
   if (I == VarEntries.end())
     return 0;
   const auto &Entries = I->second;
-  if (Entries.empty() || Entries.back().getEnd() != nullptr)
+  if (Entries.empty() || Entries.back().isEnded())
+    return 0;
+  if (Entries.back().isClobber())
     return 0;
-  return isDescribedByReg(*Entries.back().getBegin());
+  return isDescribedByReg(*Entries.back().getInstr());
 }
 
 void DbgLabelInstrMap::addInstr(InlinedEntity Label, const MachineInstr &MI) {
   assert(MI.isDebugLabel() && "not a DBG_LABEL");
   LabelInstr[Label] = &MI;
 }
 
 namespace {
 
 // Maps physreg numbers to the variables they describe.
 using InlinedEntity = DbgValueHistoryMap::InlinedEntity;
 using RegDescribedVarsMap = std::map<unsigned, SmallVector<InlinedEntity, 1>>;
 
+// Keeps track of the debug value entries that are currently live for each
+// inlined entity. As the history map entries are stored in a SmallVector, they
+// may be moved at insertion of new entries, so store indices rather than
+// pointers.
+using DbgValueEntriesMap = std::map<InlinedEntity, SmallSet<EntryIndex, 1>>;
+
 } // end anonymous namespace
 
 // Claim that @Var is not described by @RegNo anymore.
 static void dropRegDescribedVar(RegDescribedVarsMap &RegVars, unsigned RegNo,
                                 InlinedEntity Var) {
   const auto &I = RegVars.find(RegNo);
   assert(RegNo != 0U && I != RegVars.end());
   auto &VarSet = I->second;

 static void addRegDescribedVar(RegDescribedVarsMap &RegVars, unsigned RegNo,
                                InlinedEntity Var) {
   assert(RegNo != 0U);
   auto &VarSet = RegVars[RegNo];
   assert(!is_contained(VarSet, Var));
   VarSet.push_back(Var);
 }
 
+static void clobberRegEntries(InlinedEntity Var, unsigned RegNo,
+                              const MachineInstr &ClobberingInstr,
+                              DbgValueEntriesMap &LiveEntries,
+                              DbgValueHistoryMap &HistMap) {
+  EntryIndex ClobberIndex = HistMap.startClobber(Var, ClobberingInstr);
+
+  // TODO: Close all preceding live entries that are clobbered by this
+  // instruction.
+  EntryIndex ValueIndex = ClobberIndex - 1;
+  auto &ValueEntry = HistMap.getEntry(Var, ValueIndex);
+  ValueEntry.endEntry(ClobberIndex);
+  LiveEntries[Var].erase(ValueIndex);
+}
+
+/// Add a new debug value for \p Var. Closes all overlapping debug values.
+static void handleNewDebugValue(InlinedEntity Var, const MachineInstr &DV,
+                                RegDescribedVarsMap &RegVars,
+                                DbgValueEntriesMap &LiveEntries,
+                                DbgValueHistoryMap &HistMap) {
+  // TODO: We should track all registers which this variable is currently
+  // described by.
+
+  if (unsigned PrevReg = HistMap.getRegisterForVar(Var))
+    dropRegDescribedVar(RegVars, PrevReg, Var);
+
+  EntryIndex NewIndex;
+  if (HistMap.startDbgValue(Var, DV, NewIndex)) {
+    // If we have created a new debug value entry, close all preceding
+    // live entries that overlap.
+    SmallVector<EntryIndex, 4> IndicesToErase;
+    const DIExpression *DIExpr = DV.getDebugExpression();
+    for (auto Index : LiveEntries[Var]) {
+      auto &Entry = HistMap.getEntry(Var, Index);
+      assert(Entry.isDbgValue() && "Not a DBG_VALUE in LiveEntries");
+      const MachineInstr &DV = *Entry.getInstr();
+      if (DIExpr->fragmentsOverlap(DV.getDebugExpression())) {
+        IndicesToErase.push_back(Index);
+        Entry.endEntry(NewIndex);
+      }
+    }
+    // Drop all entries that have ended, and mark the new entry as live.
+    for (auto Index : IndicesToErase)
+      LiveEntries[Var].erase(Index);
+    LiveEntries[Var].insert(NewIndex);
+  }
+
+  if (unsigned NewReg = isDescribedByReg(DV))
+    addRegDescribedVar(RegVars, NewReg, Var);
+}
+
 // Terminate the location range for variables described by register at
 // @I by inserting @ClobberingInstr to their history.
 static void clobberRegisterUses(RegDescribedVarsMap &RegVars,
                                 RegDescribedVarsMap::iterator I,
                                 DbgValueHistoryMap &HistMap,
+                                DbgValueEntriesMap &LiveEntries,
                                 const MachineInstr &ClobberingInstr) {
   // Iterate over all variables described by this register and add this
   // instruction to their history, clobbering it.
   for (const auto &Var : I->second)
-    HistMap.endEntry(Var, ClobberingInstr);
+    clobberRegEntries(Var, I->first, ClobberingInstr, LiveEntries, HistMap);
   RegVars.erase(I);
 }
 
 // Terminate the location range for variables described by register
 // @RegNo by inserting @ClobberingInstr to their history.
 static void clobberRegisterUses(RegDescribedVarsMap &RegVars, unsigned RegNo,
                                 DbgValueHistoryMap &HistMap,
+                                DbgValueEntriesMap &LiveEntries,
                                 const MachineInstr &ClobberingInstr) {
   const auto &I = RegVars.find(RegNo);
   if (I == RegVars.end())
     return;
-  clobberRegisterUses(RegVars, I, HistMap, ClobberingInstr);
+  clobberRegisterUses(RegVars, I, HistMap, LiveEntries, ClobberingInstr);
 }
 
 // Returns the first instruction in @MBB which corresponds to
 // the function epilogue, or nullptr if @MBB doesn't contain an epilogue.
 static const MachineInstr *getFirstEpilogueInst(const MachineBasicBlock &MBB) {
   auto LastMI = MBB.getLastNonDebugInstr();
   if (LastMI == MBB.end() || !LastMI->isReturn())
     return nullptr;
void llvm::calculateDbgEntityHistory(const MachineFunction *MF,
                                      DbgValueHistoryMap &DbgValues,
                                      DbgLabelInstrMap &DbgLabels) {
   BitVector ChangingRegs(TRI->getNumRegs());
   collectChangingRegs(MF, TRI, ChangingRegs);
 
   const TargetLowering *TLI = MF->getSubtarget().getTargetLowering();
   unsigned SP = TLI->getStackPointerRegisterToSaveRestore();
   RegDescribedVarsMap RegVars;
+  DbgValueEntriesMap LiveEntries;
   for (const auto &MBB : *MF) {
     for (const auto &MI : MBB) {
       if (!MI.isDebugInstr()) {
         // Not a DBG_VALUE instruction. It may clobber registers which describe
         // some variables.
         for (const MachineOperand &MO : MI.operands()) {
           if (MO.isReg() && MO.isDef() && MO.getReg()) {
             // Ignore call instructions that claim to clobber SP. The AArch64
             // backend does this for aggregate function arguments.
             if (MI.isCall() && MO.getReg() == SP)
               continue;
             // If this is a virtual register, only clobber it since it doesn't
             // have aliases.
             if (TRI->isVirtualRegister(MO.getReg()))
-              clobberRegisterUses(RegVars, MO.getReg(), DbgValues, MI);
+              clobberRegisterUses(RegVars, MO.getReg(), DbgValues, LiveEntries,
+                                  MI);
             // If this is a register def operand, it may end a debug value
             // range.
             else {
               for (MCRegAliasIterator AI(MO.getReg(), TRI, true); AI.isValid();
                    ++AI)
                 if (ChangingRegs.test(*AI))
-                  clobberRegisterUses(RegVars, *AI, DbgValues, MI);
+                  clobberRegisterUses(RegVars, *AI, DbgValues, LiveEntries, MI);
             }
           } else if (MO.isRegMask()) {
             // If this is a register mask operand, clobber all debug values in
             // non-CSRs.
             for (unsigned I : ChangingRegs.set_bits()) {
               // Don't consider SP to be clobbered by register masks.
               if (unsigned(I) != SP && TRI->isPhysicalRegister(I) &&
                   MO.clobbersPhysReg(I)) {
-                clobberRegisterUses(RegVars, I, DbgValues, MI);
+                clobberRegisterUses(RegVars, I, DbgValues, LiveEntries, MI);
               }
             }
           }
         }
         continue;
       }
 
       if (MI.isDebugValue()) {
         assert(MI.getNumOperands() > 1 && "Invalid DBG_VALUE instruction!");
         // Use the base variable (without any DW_OP_piece expressions)
         // as index into History. The full variables including the
         // piece expressions are attached to the MI.
         const DILocalVariable *RawVar = MI.getDebugVariable();
         assert(RawVar->isValidLocationForIntrinsic(MI.getDebugLoc()) &&
                "Expected inlined-at fields to agree");
         InlinedEntity Var(RawVar, MI.getDebugLoc()->getInlinedAt());
 
-        if (unsigned PrevReg = DbgValues.getRegisterForVar(Var))
-          dropRegDescribedVar(RegVars, PrevReg, Var);
-
-        DbgValues.startEntry(Var, MI);
-
-        if (unsigned NewReg = isDescribedByReg(MI))
-          addRegDescribedVar(RegVars, NewReg, Var);
+        handleNewDebugValue(Var, MI, RegVars, LiveEntries, DbgValues);
       } else if (MI.isDebugLabel()) {
         assert(MI.getNumOperands() == 1 && "Invalid DBG_LABEL instruction!");
         const DILabel *RawLabel = MI.getDebugLabel();
         assert(RawLabel->isValidLocationForIntrinsic(MI.getDebugLoc()) &&
             "Expected inlined-at fields to agree");
         // When collecting debug information for labels, there is no MCSymbol
         // generated for it. So, we keep MachineInstr in DbgLabels in order
         // to query MCSymbol afterward.
         InlinedEntity L(RawLabel, MI.getDebugLoc()->getInlinedAt());
         DbgLabels.addInstr(L, MI);
       }
     }
 
     // Make sure locations for register-described variables are valid only
     // until the end of the basic block (unless it's the last basic block, in
     // which case let their liveness run off to the end of the function).
     if (!MBB.empty() && &MBB != &MF->back()) {
       for (auto I = RegVars.begin(), E = RegVars.end(); I != E;) {
         auto CurElem = I++; // CurElem can be erased below.
         if (TRI->isVirtualRegister(CurElem->first) ||
             ChangingRegs.test(CurElem->first))
-          clobberRegisterUses(RegVars, CurElem, DbgValues, MBB.back());
+          clobberRegisterUses(RegVars, CurElem, DbgValues, LiveEntries,
+                              MBB.back());
       }
     }
   }
 }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 LLVM_DUMP_METHOD void DbgValueHistoryMap::dump() const {
   dbgs() << "DbgValueHistoryMap:\n";
for (const auto &VarRangePair : *this) {
     if (Location)
       dbgs() << Location->getFilename() << ":" << Location->getLine() << ":"
              << Location->getColumn();
     else
       dbgs() << "<unknown location>";
 
     dbgs() << " --\n";
 
-    for (const auto &Entry : Entries) {
-      dbgs() << "   Begin: " << *Entry.getBegin();
-      if (Entry.getEnd())
-        dbgs() << "   End  : " << *Entry.getEnd();
+    for (const auto &E : enumerate(Entries)) {
+      const auto &Entry = E.value();
+      dbgs() << "  Entry[" << E.index() << "]: ";
+      if (Entry.isDbgValue())
+        dbgs() << "Debug value\n";
+      else
+        dbgs() << "Clobber\n";
+      dbgs() << "   Instr: " << *Entry.getInstr();
+      if (Entry.isDbgValue()) {
+        if (Entry.getEndIndex() == NoEntry)
+          dbgs() << "   - Valid until end of function\n";
+        else
+          dbgs() << "   - Closed by Entry[" << Entry.getEndIndex() << "]\n";
+      }
       dbgs() << "\n";
     }
   }
 }
 #endif

lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp

for (const auto &I : DbgValues) {
     //
     // We do not change the label for values that are described by registers,
     // as that could place them above their defining instructions. We should
     // ideally not change the labels for constant debug values either, since
     // doing that violates the ranges that are calculated in the history map.
     // However, we currently do not emit debug values for constant arguments
     // directly at the start of the function, so this code is still useful.
     const DILocalVariable *DIVar =
-        Entries.front().getBegin()->getDebugVariable();
+        Entries.front().getInstr()->getDebugVariable();
     if (DIVar->isParameter() &&
         getDISubprogram(DIVar->getScope())->describes(&MF->getFunction())) {
-      if (!IsDescribedByReg(Entries.front().getBegin()))
-        LabelsBeforeInsn[Entries.front().getBegin()] = Asm->getFunctionBegin();
-      if (Entries.front().getBegin()->getDebugExpression()->isFragment()) {
+      if (!IsDescribedByReg(Entries.front().getInstr()))
+        LabelsBeforeInsn[Entries.front().getInstr()] = Asm->getFunctionBegin();
+      if (Entries.front().getInstr()->getDebugExpression()->isFragment()) {
         // Mark all non-overlapping initial fragments.
         for (auto I = Entries.begin(); I != Entries.end(); ++I) {
-          const DIExpression *Fragment = I->getBegin()->getDebugExpression();
+          if (!I->isDbgValue())
+            continue;
+          const DIExpression *Fragment = I->getInstr()->getDebugExpression();
           if (std::any_of(Entries.begin(), I,
                           [&](DbgValueHistoryMap::Entry Pred) {
-                            return Fragment->fragmentsOverlap(
-                                Pred.getBegin()->getDebugExpression());
+                            return Pred.isDbgValue() &&
+                                   Fragment->fragmentsOverlap(
+                                       Pred.getInstr()->getDebugExpression());
                           }))
             break;
-          if (!IsDescribedByReg(I->getBegin()))
-            LabelsBeforeInsn[I->getBegin()] = Asm->getFunctionBegin();
+          if (!IsDescribedByReg(I->getInstr()))
+            LabelsBeforeInsn[I->getInstr()] = Asm->getFunctionBegin();
         }
       }
     }
 
     for (const auto &Entry : Entries) {
-      requestLabelBeforeInsn(Entry.getBegin());
-      if (Entry.getEnd())
-        requestLabelAfterInsn(Entry.getEnd());
+      if (Entry.isDbgValue())
+        requestLabelBeforeInsn(Entry.getInstr());
+      else
+        requestLabelAfterInsn(Entry.getInstr());
     }
   }
 
   // Ensure there is a symbol before DBG_LABEL.
   for (const auto &I : DbgLabels) {
     const MachineInstr *MI = I.second;
     requestLabelBeforeInsn(MI);
   }

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

if (MI->getOperand(0).isFPImm())
     return DebugLocEntry::Value(Expr, MI->getOperand(0).getFPImm());
   if (MI->getOperand(0).isCImm())
     return DebugLocEntry::Value(Expr, MI->getOperand(0).getCImm());
 
   llvm_unreachable("Unexpected 4-operand DBG_VALUE instruction!");
 }
 
 /// Build the location list for all DBG_VALUEs in the function that
-/// describe the same variable.  If the ranges of several independent
-/// fragments of the same variable overlap partially, split them up and
-/// combine the ranges. The resulting DebugLocEntries are will have
+/// describe the same variable. The resulting DebugLocEntries will have
 /// strict monotonically increasing begin addresses and will never
 /// overlap.
 //
+// See the definition of DbgValueHistoryMap::Entry for an explanation of the
+// different kinds of history map entries. One thing to be aware of is that if
+// a debug value is ended by another entry (rather than being valid until the
+// end of the function), that entry's instruction may or may not be included in
+// the range, depending on if the entry is a clobbering entry (it has an
+// instruction that clobbers one or more preceding locations), or if it is an
+// (overlapping) debug value entry. This distinction can be seen in the example
+// below. The first debug value is ended by the clobbering entry 2, and the
+// second and third debug values are ended by the overlapping debug value entry
+// 4.
+//
 // Input:
 //
-//   Ranges History [var, loc, fragment ofs size]
-// 0 |      [x, (reg0, fragment 0, 32)]
-// 1 | |    [x, (reg1, fragment 32, 32)] <- IsFragmentOfPrevEntry
-// 2 | |    ...
-// 3   |    [clobber reg0]
-// 4        [x, (mem, fragment 0, 64)] <- overlapping with both previous fragments of
-//                                     x.
+//   History map entries [type, end index, mi]
+//
+// 0 |      [DbgValue, 2, DBG_VALUE $reg0, [...] (fragment 0, 32)]
+// 1 | |    [DbgValue, 4, DBG_VALUE $reg1, [...] (fragment 32, 32)]
+// 2 | |    [Clobber, $reg0 = [...], -, -]
+// 3   | |  [DbgValue, 4, DBG_VALUE 123, [...] (fragment 64, 32)]
+// 4        [DbgValue, ~0, DBG_VALUE @g, [...] (fragment 0, 96)]
 //
-// Output:
+// Output [start, end) [Value...]:
 //
-// [0-1]    [x, (reg0, fragment  0, 32)]
-// [1-3]    [x, (reg0, fragment  0, 32), (reg1, fragment 32, 32)]
-// [3-4]    [x, (reg1, fragment 32, 32)]
-// [4- ]    [x, (mem,  fragment  0, 64)]
+// [0-1)    [(reg0, fragment 0, 32)]
+// [1-3)    [(reg0, fragment 0, 32), (reg1, fragment 32, 32)]
+// [3-4)    [(reg1, fragment 32, 32), (123, fragment 64, 32)]
+// [4-)     [(@g, fragment 0, 96)]
 void DwarfDebug::buildLocationList(SmallVectorImpl<DebugLocEntry> &DebugLoc,
                                    const DbgValueHistoryMap::Entries &Entries) {
-  SmallVector<DebugLocEntry::Value, 4> OpenRanges;
+  using OpenRange =
+      std::pair<DbgValueHistoryMap::EntryIndex, DebugLocEntry::Value>;
+  SmallVector<OpenRange, 4> OpenRanges;
 
-  for (auto EI = Entries.begin(), EE = Entries.end(); EI != EE; ++EI) {
-    const MachineInstr *Begin = EI->getBegin();
-    const MachineInstr *End = EI->getEnd();
-    assert(Begin->isDebugValue() && "Invalid History entry");
+  for (auto EB = Entries.begin(), EI = EB, EE = Entries.end(); EI != EE; ++EI) {
+    const MachineInstr *Instr = EI->getInstr();
 
+    if (EI->isDbgValue()) {
       // Check if a variable is inaccessible in this range.
-    if (Begin->getNumOperands() > 1 &&
-        Begin->getOperand(0).isReg() && !Begin->getOperand(0).getReg()) {
+      // TODO: This should only truncate open ranges that are overlapping.
+      if (Instr->getNumOperands() > 1 &&
+          Instr->getOperand(0).isReg() && !Instr->getOperand(0).getReg()) {
         OpenRanges.clear();
         continue;
       }
+    }
 
-    // If this debug value overlaps with any open ranges, truncate them.
-    const DIExpression *DIExpr = Begin->getDebugExpression();
-    auto Last = remove_if(OpenRanges, [&](DebugLocEntry::Value R) {
-      return DIExpr->fragmentsOverlap(R.getExpression());
-    });
+    // Remove all values that are no longer live.
+    size_t Index = std::distance(EB, EI);
+    auto Last =
+        remove_if(OpenRanges, [&](OpenRange &R) { return R.first <= Index; });
     OpenRanges.erase(Last, OpenRanges.end());
 
-    const MCSymbol *StartLabel = getLabelBeforeInsn(Begin);
-    assert(StartLabel && "Forgot label before DBG_VALUE starting a range!");
+    // If we are dealing with a clobbering entry, this iteration will result in
+    // a location list entry starting after the clobbering instruction.
+    const MCSymbol *StartLabel =
+        EI->isClobber() ? getLabelAfterInsn(Instr) : getLabelBeforeInsn(Instr);
+    assert(StartLabel &&
+           "Forgot label before/after instruction starting a range!");
 
     const MCSymbol *EndLabel;
-    if (End != nullptr)
-      EndLabel = getLabelAfterInsn(End);
-    else if (std::next(EI) == Entries.end())
+    if (std::next(EI) == Entries.end())
       EndLabel = Asm->getFunctionEnd();
+    else if (std::next(EI)->isClobber())
+      EndLabel = getLabelAfterInsn(std::next(EI)->getInstr());
     else
-      EndLabel = getLabelBeforeInsn(std::next(EI)->getBegin());
+      EndLabel = getLabelBeforeInsn(std::next(EI)->getInstr());
     assert(EndLabel && "Forgot label after instruction ending a range!");
 
-    LLVM_DEBUG(dbgs() << "DotDebugLoc: " << *Begin << "\n");
+    if (EI->isDbgValue())
+      LLVM_DEBUG(dbgs() << "DotDebugLoc: " << *Instr << "\n");
 
-    auto Value = getDebugLocValue(Begin);
-    OpenRanges.push_back(Value);
+    // If this history map entry has a debug value, add that to the list of
+    // open ranges.
+    if (EI->isDbgValue()) {
+      auto Value = getDebugLocValue(Instr);
+      OpenRanges.emplace_back(EI->getEndIndex(), Value);
+    }
+
+    // Location list entries with empty location descriptions are redundant
+    // information in DWARF, so do not emit those.
+    if (OpenRanges.empty())
+      continue;
 
     // Omit entries with empty ranges as they do not have any effect in DWARF.
     if (StartLabel == EndLabel) {
       LLVM_DEBUG(dbgs() << "Omitting location list entry with empty range.\n");
       continue;
     }
 
-    DebugLoc.emplace_back(StartLabel, EndLabel, OpenRanges);
+    SmallVector<DebugLocEntry::Value, 4> Values;
+    for (auto &R : OpenRanges)
+      Values.push_back(R.second);
+    DebugLoc.emplace_back(StartLabel, EndLabel, Values);
 
     // Attempt to coalesce the ranges of two otherwise identical
     // DebugLocEntries.
     auto CurEntry = DebugLoc.rbegin();
     LLVM_DEBUG({
       dbgs() << CurEntry->getValues().size() << " Values:\n";
       for (auto &Value : CurEntry->getValues())
         Value.dump();
for (const auto &I : DbgValues) {
     // If variable scope is not found then skip this variable.
     if (!Scope)
       continue;
 
     Processed.insert(IV);
     DbgVariable *RegVar = cast<DbgVariable>(createConcreteEntity(TheCU,
                                             *Scope, LocalVar, IV.second));
 
-    const MachineInstr *MInsn = HistoryMapEntries.front().getBegin();
+    const MachineInstr *MInsn = HistoryMapEntries.front().getInstr();
     assert(MInsn->isDebugValue() && "History must begin with debug value");
 
     // Check if there is a single DBG_VALUE, valid throughout the var's scope.
-    if (HistoryMapEntries.size() == 1 &&
-        validThroughout(LScopes, MInsn, HistoryMapEntries.front().getEnd())) {
+    // If the history map contains a single debug value, there may be an
+    // additional entry which clobbers the debug value.
+    bool SingleValueWithClobber =
+        HistoryMapEntries.size() == 2 && HistoryMapEntries[1].isClobber();
+    if (HistoryMapEntries.size() == 1 || SingleValueWithClobber) {
+      const auto *End =
+          SingleValueWithClobber ? HistoryMapEntries[1].getInstr() : nullptr;
+      if (validThroughout(LScopes, MInsn, End)) {
         RegVar->initializeDbgValue(MInsn);
         continue;
       }
+    }
+
     // Do not emit location lists if .debug_loc secton is disabled.
     if (!useLocSection())
       continue;
 
     // Handle multiple DBG_VALUE instructions describing one variable.
     DebugLocStream::ListBuilder List(DebugLocs, TheCU, *Asm, *RegVar, *MInsn);
 
     // Build the location list for this variable.

test/DebugInfo/ARM/partial-subreg.ll

 ; RUN: llc %s -filetype=obj -o - | llvm-dwarfdump -v - | FileCheck %s
 ; This tests a fragment that partially covers subregister compositions.
 ;
 ; Our fragment is 96 bits long and lies in a 128-bit register, which
 ; in turn has to be composed out of its two 64-bit subregisters.
 
 ; CHECK: .debug_info
 ; CHECK: DW_TAG_subprogram
 ; CHECK:   DW_AT_name {{.*}}"subscript.get"
 ; CHECK:  DW_TAG_formal_parameter
 ; CHECK-NEXT: DW_AT_location [DW_FORM_sec_offset] ({{.*}}
 ; CHECK-NEXT:  [0x{{.*}}, 0x{{.*}}): DW_OP_regx D16, DW_OP_piece 0x8, DW_OP_regx D17, DW_OP_piece 0x4
-; CHECK-NEXT:  [0x{{.*}}, 0x{{.*}}): DW_OP_regx D16, DW_OP_piece 0x8, DW_OP_regx D17, DW_OP_piece 0x4, DW_OP_regx D16, DW_OP_piece 0x8, DW_OP_regx D17, DW_OP_piece 0x4
+; CHECK-NEXT:  [0x{{.*}}, 0x{{.*}}): DW_OP_regx D16, DW_OP_piece 0x8, DW_OP_regx D17, DW_OP_piece 0x4
+
+; FIXME: The second location list entry should not be emitted.
+;
+; The input to LiveDebugValues is:
+;
+; bb.0.entry:
+;   [...]
+;   Bcc %bb.2, 1, killed $cpsr, debug-location !10; simd.swift:5900:12
+; bb.1:
+;   [...]
+;   DBG_VALUE $q8, $noreg, !"self", !DIExpression(DW_OP_LLVM_fragment, 0, 96)
+;   B %bb.3
+; bb.2.select.false:
+;   [...]
+;   DBG_VALUE $q8, $noreg, !"self", !DIExpression(DW_OP_LLVM_fragment, 0, 96)
+; bb.3.select.end:
+;   [...]
+;
+; The two DBG_VALUEs in the blocks describe different fragments of the
+; variable. However, LiveDebugValues is not aware of fragments, so it will
+; incorrectly insert a copy of the first DBG_VALUE in bb.3.select.end, since
+; the debug values in its predecessor blocks are described by the same
+; register.
 
 source_filename = "simd.ll"
 target datalayout = "e-m:o-p:32:32-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
 target triple = "armv7-apple-ios7.0"
 
 ; Function Attrs: nounwind readnone
 declare void @llvm.dbg.value(metadata, metadata, metadata) #0
 

test/DebugInfo/MIR/X86/clobbered-fragments.mir

+# RUN: llc -O1 -start-after=livedebugvalues -o - %s | FileCheck %s
+
+# Reproducer based on the following C file:
+#
+# int global1, global2, global3;
+#
+# extern void ext1(int);
+# extern void ext2(int, int);
+# extern void ext3(int, int, int);
+#
+# int test1() {
+#   int local[3] = {global1, 123, 456};
+#   ext2(local[0], local[1]);
+#   return local[1];
+# }
+#
+# Compiled using -O1 -g.
+
+--- |
+  target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+  target triple = "x86_64-unknown-linux-gnu"
+
+  @global1 = common global i32 0, align 4
+  @global2 = common global i32 0, align 4
+  @global3 = common global i32 0, align 4
+
+  ; Function Attrs: nounwind uwtable
+  define i32 @test1() #0 !dbg !8 {
+  entry:
+    %0 = load i32, i32* @global1, align 4, !dbg !16
+    call void @llvm.dbg.value(metadata i32 %0, metadata !12, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32)), !dbg !16
+    call void @llvm.dbg.value(metadata i32 123, metadata !12, metadata !DIExpression(DW_OP_LLVM_fragment, 32, 32)), !dbg !16
+    call void @llvm.dbg.value(metadata i32 456, metadata !12, metadata !DIExpression(DW_OP_LLVM_fragment, 64, 32)), !dbg !16
+    tail call void @ext2(i32 %0, i32 123), !dbg !16
+    ret i32 123, !dbg !17
+  }
+
+  declare void @ext2(i32, i32)
+
+  ; Function Attrs: nounwind readnone speculatable
+  declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+  attributes #0 = { nounwind uwtable }
+  attributes #1 = { nounwind readnone speculatable }
+
+  !llvm.dbg.cu = !{!1}
+  !llvm.module.flags = !{!5, !6}
+  !llvm.ident = !{!7}
+
+  !1 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2, producer: "clang version 9.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !3, globals: !3, nameTableKind: None)
+  !2 = !DIFile(filename: "foo.c", directory: "/")
+  !3 = !{}
+  !4 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !5 = !{i32 2, !"Dwarf Version", i32 4}
+  !6 = !{i32 2, !"Debug Info Version", i32 3}
+  !7 = !{!"clang version 9.0.0"}
+  !8 = distinct !DISubprogram(name: "test1", scope: !2, file: !2, line: 7, type: !9, scopeLine: 7, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !1, retainedNodes: !11)
+  !9 = !DISubroutineType(types: !10)
+  !10 = !{!4}
+  !11 = !{!12}
+  !12 = !DILocalVariable(name: "local", scope: !8, file: !2, line: 8, type: !13)
+  !13 = !DICompositeType(tag: DW_TAG_array_type, baseType: !4, size: 96, elements: !14)
+  !14 = !{!15}
+  !15 = !DISubrange(count: 3)
+  !16 = !DILocation(line: 8, scope: !8)
+  !17 = !DILocation(line: 9, scope: !8)
+
+...
+---
+name:            test1
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    frame-setup PUSH64r undef $rax, implicit-def $rsp, implicit $rsp
+    renamable $edi = MOV32rm $rip, 1, $noreg, @global1, $noreg, debug-location !16 :: (dereferenceable load 4 from @global1)
+    DBG_VALUE 456, $noreg, !12, !DIExpression(DW_OP_LLVM_fragment, 64, 32), debug-location !16
+    DBG_VALUE 123, $noreg, !12, !DIExpression(DW_OP_LLVM_fragment, 32, 32), debug-location !16
+    DBG_VALUE $edi, $noreg, !12, !DIExpression(DW_OP_LLVM_fragment, 0, 32), debug-location !16
+    $esi = MOV32ri 123, debug-location !16
+    CALL64pcrel32 @ext2, csr_64, implicit $rsp, implicit $ssp, implicit $edi, implicit killed $esi, implicit-def $rsp, implicit-def $ssp, debug-location !16
+    $eax = MOV32ri 123, debug-location !17
+    $rcx = frame-destroy POP64r implicit-def $rsp, implicit $rsp, debug-location !17
+    RETQ killed $eax, debug-location !17
+
+...
+
+# CHECK: .Ltmp1
+# CHECK-NEXT: #DEBUG_VALUE: test1:local <- [DW_OP_LLVM_fragment 64 32] 456
+# CHECK-NEXT: #DEBUG_VALUE: test1:local <- [DW_OP_LLVM_fragment 32 32] 123
+# CHECK-NEXT: #DEBUG_VALUE: test1:local <- [DW_OP_LLVM_fragment 0 32] $edi
+# CHECK:         callq   ext2
+# CHECK-NEXT: .Ltmp2:
+
+#### Location list for test1:local.
+
+# Verify that a location list entry, which does not contain the fragment that
+# is described by the clobbered $edi, is emitted directly after the call to
+# ext2().
+
+# CHECK: .Ldebug_loc0:
+# CHECK-NEXT: .quad   .Ltmp1-.Lfunc_begin0
+# CHECK-NEXT: .quad   .Ltmp2-.Lfunc_begin0
+# CHECK-NEXT: .short  14     # Loc expr size
+# CHECK-NEXT: .byte   85     # super-register DW_OP_reg5
+# CHECK-NEXT: .byte   147    # DW_OP_piece
+# CHECK-NEXT: .byte   4      # 4
+# CHECK-NEXT: .byte   16     # DW_OP_constu
+# CHECK-NEXT: .byte   123    # 123
+# CHECK-NEXT: .byte   159    # DW_OP_stack_value
+# CHECK-NEXT: .byte   147    # DW_OP_piece
+# CHECK-NEXT: .byte   4      # 4
+# CHECK-NEXT: .byte   16     # DW_OP_constu
+# CHECK-NEXT: .byte   200    # 456
+# CHECK-NEXT: .byte   3      #
+# CHECK-NEXT: .byte   159    # DW_OP_stack_value
+# CHECK-NEXT: .byte   147    # DW_OP_piece
+# CHECK-NEXT: .byte   4      # 4
+# CHECK-NEXT: .quad   .Ltmp2-.Lfunc_begin0
+# CHECK-NEXT: .quad   .Lfunc_end0-.Lfunc_begin0
+# CHECK-NEXT: .short  13     # Loc expr size
+# CHECK-NEXT: .byte   147    # DW_OP_piece
+# CHECK-NEXT: .byte   4      # 4
+# CHECK-NEXT: .byte   16     # DW_OP_constu
+# CHECK-NEXT: .byte   123    # 123
+# CHECK-NEXT: .byte   159    # DW_OP_stack_value
+# CHECK-NEXT: .byte   147    # DW_OP_piece
+# CHECK-NEXT: .byte   4      # 4
+# CHECK-NEXT: .byte   16     # DW_OP_constu
+# CHECK-NEXT: .byte   200    # 456
+# CHECK-NEXT: .byte   3      #
+# CHECK-NEXT: .byte   159    # DW_OP_stack_value
+# CHECK-NEXT: .byte   147    # DW_OP_piece
+# CHECK-NEXT: .byte   4      # 4
+# CHECK-NEXT: .quad   0
+# CHECK-NEXT: .quad   0
+

test/DebugInfo/X86/pieces-3.ll

 ;    int foo(Outer outer) {
 ;      Inner i1 = outer.inner[1];
 ;      return i1.a;
 ;    }
 ;
 ; CHECK: DW_TAG_formal_parameter [3]
 ; CHECK-NEXT:   DW_AT_location [DW_FORM_data4]        (
 ; CHECK-NEXT:     [0x0000000000000000, 0x0000000000000007): DW_OP_reg5 RDI, DW_OP_piece 0x8, DW_OP_piece 0x4, DW_OP_reg4 RSI, DW_OP_piece 0x4
+; CHECK-NEXT:     [0x0000000000000007, 0x0000000000000009): DW_OP_reg5 RDI, DW_OP_piece 0x8
 ; CHECK-NEXT:   DW_AT_name {{.*}}"outer"
 ; CHECK: DW_TAG_variable
 ; CHECK-NEXT:   DW_AT_name {{.*}}"i1"
 ; CHECK-NOT:    DW_AT_location
 
 ; ModuleID = '/Volumes/Data/llvm/test/DebugInfo/X86/sroasplit-2.ll'
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.9.0"