Infrastructure changes to support DW_OP_piece in LLVM IR.

[aprantl]

Summary

Here are the infrastructure changes to support DW_OP_piece in LLVM IR split out from D2680.

[aprantl]

And apparently I failed to attach the accompanying text:

sorry for the long delay, but I was out sick at the beginning of the week, and then when I was rebasing the patch, I saw an opportunity to clean it up a little. Anyhow, here is the promised infrastructure-only patch. It still touches a lot of files, but most of the changes are very straight-forward.

docs/SourceLevelDebugging.rst -- documentation of piece complex exprs
include/llvm/IR/DIBuilder.h -- add createVariablePiece()
lib/IR/DIBuilder.cpp -- add createVariablePiece()
include/llvm/IR/DebugInfo.h -- support for Variable pieces
lib/IR/DebugInfo.cpp -- misc. pieces support functions
include/llvm/CodeGen/AsmPrinter.h -- move EmitDwarfRegOpPiece into AsmPrinter
lib/CodeGen/AsmPrinter/AsmPrinter.cpp -- prettyprinting of pieces
lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp -- emit DWARF for pieces
lib/CodeGen/AsmPrinter/DebugLocEntry.h -- Support for more than 1 Value/Entry.
lib/CodeGen/AsmPrinter/DwarfDebug.cpp -- Range handling of DW_op_pieces
lib/CodeGen/AsmPrinter/DwarfDebug.h -- move setMInsn to .cpp
lib/CodeGen/AsmPrinter/DwarfUnit.cpp -- emit DW_op_pieces inside of a DW_AT_location
test/DebugInfo/X86/pieces-1.ll -- testcase!

• adrian

[echristo]

I'm thinking about some of the rest of it, but have you thought about having the complex expression required and just be another metadata node that would be null if there's no complex expression there?

[echristo]

Some inline comments and a couple of global comments:

Can you do the AP/TM refactorings separate?

it looks like the DebugLocEntry rewrite can largely be done independently of the DW_OP_piece infrastructure so it'd be great if you could do that separately - it'd make the parts that are just for OP_piece very obvious and should be a "no functional change" patch.

These two splits are preapproved for committing separate. I think, other than the inline comments and the request to split things up, the format of the IR is the only thing I've got a concern about here.

Thanks for all the work!

include/llvm/CodeGen/AsmPrinter.h

436

Cut and paste.

443

Can we use the complex address stuff to encompass register-indirect addressing there so we don't have to pass the bool?

lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp

222

Text here explaining the why of the assert.

229

Odd formatting plus some text, i.e. "DW_OP_piece wants byte sized offsets".

230

We do this in so many places throughout llvm that it's, unfortunately, probably not a useful comment.

348

EmitDwarfReg perhaps instead since we've got a 0 offset?

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

1375

Formatting.

lib/IR/DebugInfo.cpp

959

This needs a comment of how and why it's used.

[aprantl]

This is the remaining patch, with the DebugLocEntry and AsmPrinter refactorings split out.
Related:

• http://reviews.llvm.org/D3373 , which makes DIVariable complex address expressions separate MDNodes.
• LLVM r207368, r207369, and r207372

lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp

348

No, we specifically want the sub/super-register magic here. I clarified the doxygen comment for EmitDwarfRegOpPiece() in 207372 to make this more obvious.

[echristo]

Hi Adrian,

Some inline comments. On first start it's looking pretty good. Thanks!

-eric

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

110

Perhaps a different name here since it's not just doing the set. Perhaps updateMachineInstr?

1174

P1 && P1?

1175

Mmm... implicit precedence knowledge required. Can you fix this up?

1210

Is it possible to break this function up a bit? Realize it's an unrelated refactor, but it would help a lot.

1286

Removed a nullptr initialization?

1304

nullptr?

1311

weird formatting?

1326

Comment please :)

1514

I'm not sure I understand the AtFnBegin stuff, could you explain it a bit more?

1594

Comment update.

2144

Probably just emitLocPieces.

2151

Clever. How about some text? :)

test/DebugInfo/X86/pieces-1.ll

2

Not llc -O0? Or do we need things to be optimized by llc?

31

Duplicate label adding?

35

If you're going to leave this in then my personal preference would be to use -mtriple above. Is it possible this works for just "any x86 with dwarf" though?

[echristo]

Ah. Makes sense. Thanks.

-eric

[aprantl]

Besides addressing the comments from the previous revision there are tow changes:

• Emit "empty" pieces (DW_OP_piece without a location) for unavailable pieces. This is actually mandated by the DWARF spec, but only mentioned en passant in the examples.
• Simplify and improve the range computation/coalescing that used to be in DwarfDebug::collectVariableInfo() and split it out to a new DwarfDebug::buildLocationList(). The new code is shorter, easier to reason about, and will result in easier-to-digest location lists, because it won't emit overlapping ranges any more. See also the comment above buildLocationList().

[aprantl]

Rebased on Aleksei's collectVariableDebugInfo refactoring.

[aprantl]

Rebased on Alexey's recent debug value history refactoring changes.

[aprantl]

Rebased patch on today's ToT.

[aprantl]

Rebased on Aleksei's huge improvement to DbgValueHistoryCalculator.cpp

[aprantl]

Rebased on 209832.

[echristo]

Looks like a good start, some cleanups, comments and questions inline and then we can continue the review.

Thanks!

-eric

docs/SourceLevelDebugging.rst

592

s/addresses/locations

594

s/address/location

include/llvm/CodeGen/AsmPrinter.h

407

s/small//

lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp

255

This seems to be the only function you've changed the capitalization on?

lib/CodeGen/AsmPrinter/DebugLocEntry.h

26

This appears unused.

111

Description of the logic here?

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

1155

Having a hard time visualizing the && here, where will both of these be true?

1180

I'm not sure what's overlapping here.

[aprantl]

Addressed comments from previous round; mostly clarifications.

[aprantl]

lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp

255

Yes, I promoted it from being a static function to a method and adopted what appears to be the spelling convention in AsmPrinter (although it's not terribly consistent).

[echristo]

As a note to all I just added llvm-commits to the thread last week,
it'd been an ongoing review and llvm-commits had accidentally been
forgotten.

-eric

[aprantl]

Fix a merge error that slipped in while rebasing this patch.

[aprantl]

Added the definition of DIBuilder::createVariablePiece().

[echristo]

One comment and then nothing but nits in the inline stuff. It looks very good in general, however, tests? :)

I assume they were dropped somewhere so go ahead and add them and LGTM.

Thanks for all of your hard work here!

-eric

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

1240

Extra white space.

1242

"inaccessible"

1256

Extra whitespace.

2086

Weird formatting?

2092

Weird formatting?

lib/CodeGen/AsmPrinter/DwarfUnit.cpp

605

Comment with the format here that you're adding? Also probably const for the SizeOfByte.

[dblaikie]

Submitted in http://llvm.org/viewvc/llvm-project?rev=214576&view=rev

docs/SourceLevelDebugging.rst

 Name the source variable name.  Context and line indicate where the variable
 was defined.  Type descriptor defines the declared type of the variable.
 
+The ``OpPiece`` operator is used for (typically larger aggregate)
+variables that are fragmented across several addresses. It takes two
+i32 arguments, an offset and a size to describe which piece of the
+variable is at this address.
+
+
 .. _format_common_intrinsics:
 
 Debugger intrinsic functions

include/llvm/CodeGen/AsmPrinter.h

   /// Get the value for DW_AT_APPLE_isa. Zero if no isa encoding specified.
   virtual unsigned getISAEncoding() { return 0; }
 
+  /// Emit a dwarf register operation for describing
+  /// - a small value occupying only part of a register or
+  /// - a small register representing only part of a value.
+  void EmitDwarfOpPiece(ByteStreamer &Streamer, unsigned SizeInBits,
+                        unsigned OffsetInBits = 0) const;
+
+
   /// \brief Emit a partial DWARF register operation.
   /// \param MLoc             the register
   /// \param PieceSize        size and

                            unsigned PieceSize = 0,
                            unsigned PieceOffset = 0) const;
 
-  /// Emit dwarf register operation.
+  /// EmitDwarfRegOp - Emit dwarf register operation.
   /// \param Indirect   whether this is a register-indirect address
   virtual void EmitDwarfRegOp(ByteStreamer &BS, const MachineLocation &MLoc,
                               bool Indirect) const;

include/llvm/IR/DIBuilder.h

 
     public:
     explicit DIBuilder(Module &M);
-    enum ComplexAddrKind { OpPlus=1, OpDeref };
+    enum ComplexAddrKind { OpPlus=1, OpDeref, OpPiece };
     enum DebugEmissionKind { FullDebug=1, LineTablesOnly };
 
     /// finalize - Construct any deferred debug info descriptors.

                                      DITypeRef Ty, ArrayRef<Value *> Addr,
                                      unsigned ArgNo = 0);
 
+    /// createVariablePiece - Create a descriptor to describe one part
+    /// of aggregate variable that is fragmented across multiple Values.
+    ///
+    /// @param Variable      Variable that is partially represented by this.
+    /// @param OffsetInBytes Offset of the piece in bytes.
+    /// @param SizeInBytes   Size of the piece in bytes.
+    DIVariable createVariablePiece(DIVariable Variable,
+                                   unsigned OffsetInBytes,
+                                   unsigned SizeInBytes);
+
     /// createFunction - Create a new descriptor for the specified subprogram.
     /// See comments in DISubprogram for descriptions of these fields.
     /// @param Scope         Function scope.

include/llvm/IR/DebugInfo.h

   /// information for an inlined function arguments.
   bool isInlinedFnArgument(const Function *CurFn);
 
+  /// isVariablePiece - Return whether this is a piece of an aggregate
+  /// variable.
+  bool isVariablePiece() const;
+  /// getPieceOffset - Return the offset of this piece in bytes.
+  uint64_t getPieceOffset() const;
+  /// getPieceSize - Return the size of this piece in bytes.
+  uint64_t getPieceSize() const;
+
+  /// Return the size reported by the variable's type.
+  unsigned getSizeInBits(const DITypeIdentifierMap &Map);
+
   void printExtendedName(raw_ostream &OS) const;
 };
 

 /// cleanseInlinedVariable - Remove inlined scope from the variable.
 DIVariable cleanseInlinedVariable(MDNode *DV, LLVMContext &VMContext);
 
+/// getEntireVariable - Remove OpPiece exprs from the variable.
+DIVariable getEntireVariable(DIVariable DV);
+
 /// Construct DITypeIdentifierMap by going through retained types of each CU.
 DITypeIdentifierMap generateDITypeIdentifierMap(const NamedMDNode *CU_Nodes);
 

lib/CodeGen/AsmPrinter/AsmPrinter.cpp

     if (!Name.empty())
       OS << Name << ":";
   }
-  OS << V.getName() << " <- ";
+  OS << V.getName();
+  if (V.isVariablePiece())
+    OS << " [piece offset=" << V.getPieceOffset()
+       << " size="<<V.getPieceSize()<<"]";
+  OS << " <- ";
 
   // The second operand is only an offset if it's an immediate.
   bool Deref = MI->getOperand(0).isReg() && MI->getOperand(1).isImm();

lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp

     Streamer.EmitInt8(dwarf::DW_OP_deref, "DW_OP_deref");
 }
 
-/// Emit a dwarf register operation for describing
-/// - a small value occupying only part of a register or
-/// - a small register representing only part of a value.
-static void emitDwarfOpPiece(ByteStreamer &Streamer, unsigned SizeInBits,
-                             unsigned OffsetInBits) {
+void AsmPrinter::EmitDwarfOpPiece(ByteStreamer &Streamer, unsigned SizeInBits,
+                                  unsigned OffsetInBits) const {
   assert(SizeInBits > 0 && "zero-sized piece");
   unsigned SizeOfByte = 8;
   if (OffsetInBits > 0 || SizeInBits % SizeOfByte) {

   // If this is a valid register number, emit it.
   if (Reg >= 0) {
     emitDwarfRegOp(Streamer, Reg);
-    emitDwarfOpPiece(Streamer, PieceSizeInBits, PieceOffsetInBits);
+    EmitDwarfOpPiece(Streamer, PieceSizeInBits, PieceOffsetInBits);
     return;
   }
 

       OutStreamer.AddComment("super-register");
       emitDwarfRegOp(Streamer, Reg);
       if (PieceOffsetInBits == Offset) {
-        emitDwarfOpPiece(Streamer, Size, Offset);
+        EmitDwarfOpPiece(Streamer, Size, Offset);
       } else {
         // If this is part of a variable in a sub-register at a
         // non-zero offset, we need to manually shift the value into
         // place, since the DW_OP_piece describes the part of the
         // variable, not the position of the subregister.
-        emitDwarfOpPiece(Streamer, Size, PieceOffsetInBits);
+        EmitDwarfOpPiece(Streamer, Size, PieceOffsetInBits);
         if (Offset)
           emitDwarfOpShr(Streamer, Offset);
       }

     if (Reg >= 0 && Intersection.any()) {
       OutStreamer.AddComment("sub-register");
       emitDwarfRegOp(Streamer, Reg);
-      emitDwarfOpPiece(Streamer, Size, Offset == CurPos ? 0 : Offset);
+      EmitDwarfOpPiece(Streamer, Size, Offset == CurPos ? 0 : Offset);
       CurPos = Offset + Size;
 
       // Mark it as emitted.

lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.h

   // 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:
   typedef std::pair<const MachineInstr *, const MachineInstr *> InstrRange;
   typedef SmallVector<InstrRange, 4> InstrRanges;
   typedef MapVector<const MDNode *, InstrRanges> InstrRangesMap;
+private:
   InstrRangesMap VarInstrRanges;
 
 public:

lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp

 #include "llvm/ADT/SmallVector.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/IR/DebugInfo.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Target/TargetRegisterInfo.h"
 #include <algorithm>

                                          const MachineInstr &MI) {
   // Instruction range should start with a DBG_VALUE instruction for the
   // variable.
-  assert(MI.isDebugValue() && MI.getDebugVariable() == Var);
+  assert(MI.isDebugValue() && getEntireVariable(MI.getDebugVariable()) == Var);
   auto &Ranges = VarInstrRanges[Var];
   if (!Ranges.empty() && Ranges.back().second == nullptr &&
       Ranges.back().first->isIdenticalTo(&MI)) {

       }
 
       assert(MI.getNumOperands() > 1 && "Invalid DBG_VALUE instruction!");
-      const MDNode *Var = MI.getDebugVariable();
+      // 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.
+      DIVariable Var = getEntireVariable(MI.getDebugVariable());
 
       if (unsigned PrevReg = Result.getRegisterForVar(Var))
         dropRegDescribedVar(RegVars, PrevReg, Var);

lib/CodeGen/AsmPrinter/DebugLocEntry.h

 #ifndef CODEGEN_ASMPRINTER_DEBUGLOCENTRY_H__
 #define CODEGEN_ASMPRINTER_DEBUGLOCENTRY_H__
 #include "llvm/IR/Constants.h"
+#include "llvm/IR/DebugInfo.h"
 #include "llvm/MC/MachineLocation.h"
 #include "llvm/MC/MCSymbol.h"
 

   // Begin and end symbols for the address range that this location is valid.
   const MCSymbol *Begin;
   const MCSymbol *End;
+  const Instruction *EndOfRange;
 
 public:
   /// A single location or constant.

   /// share the same Loc/Constant and if Next immediately follows this
   /// Entry.
   bool Merge(const DebugLocEntry &Next) {
+    if (Begin == Next.Begin && Values.size() > 0 && Next.Values.size() > 0) {
+      DIVariable Var(Values[0].Variable);
+      DIVariable NextVar(Next.Values[0].Variable);
+      if (Var.getName() == NextVar.getName() &&
+          Var.isVariablePiece() && NextVar.isVariablePiece()) {
+        Values.append(Next.Values.begin(), Next.Values.end());
+        End = Next.End;
+        return true;
+      }
+    }
     if ((End == Next.Begin && Values == Next.Values)) {
       End = Next.End;
       return true;
     }
     return false;
   }
+
   const MCSymbol *getBeginSym() const { return Begin; }
   const MCSymbol *getEndSym() const { return End; }
+  const Instruction *getEndOfRange() const { return EndOfRange; }
   const DwarfCompileUnit *getCU() const { return Unit; }
   const ArrayRef<Value> getValues() const { return Values; }
-  void addValue(Value Val) { Values.push_back(Val); }
+  void addValue(Value Val) {
+    assert(DIVariable(Val.Variable).isVariablePiece() &&
+           "multi-value DebugLocEntries must be pieces");
+    Values.push_back(Val);
+  }
 };
 
 }

lib/CodeGen/AsmPrinter/DwarfDebug.h

   StringRef getName() const { return Var.getName(); }
   DbgVariable *getAbstractVariable() const { return AbsVar; }
   const MachineInstr *getMInsn() const { return MInsn; }
-  void setMInsn(const MachineInstr *M) { MInsn = M; }
+  /// Set MInsn and extract the DIVariable from it.
+  void updateFromMachineInstr(const MachineInstr *M);
   int getFrameIndex() const { return FrameIndex; }
   void setFrameIndex(int FI) { FrameIndex = FI; }
   // Translate tag to proper Dwarf tag.

   /// \brief Populate LexicalScope entries with variables' info.
   void collectVariableInfo(SmallPtrSet<const MDNode *, 16> &ProcessedVars);
 
+  /// \brief Build the location list for all DBG_VALUEs in the
+  /// function that describe the same variable.
+  void buildLocationList(SmallVectorImpl<DebugLocEntry> &DebugLoc,
+                         const DbgValueHistoryMap::InstrRanges &Ranges,
+                         DwarfCompileUnit *TheCU);
+
   /// \brief Collect variable information from the side table maintained
   /// by MMI.
   void collectVariableInfoFromMMITable(SmallPtrSet<const MDNode *, 16> &P);

   /// \brief Emit an entry for the debug loc section. This can be used to
   /// handle an entry that's going to be emitted into the debug loc section.
   void emitDebugLocEntry(ByteStreamer &Streamer, const DebugLocEntry &Entry);
+  /// \brief emit a single value for the debug loc section.
+  void emitDebugLocValue(ByteStreamer &Streamer,
+                         const DebugLocEntry::Value &Value);
+  /// Emits an optimal (=sorted) sequence of DW_OP_pieces.
+  void emitLocPieces(ByteStreamer &Streamer,
+                     const DITypeIdentifierMap &Map,
+                     ArrayRef<DebugLocEntry::Value> Values);
 
   /// Emit the location for a debug loc entry, including the size header.
   void emitDebugLocEntryLocation(const DebugLocEntry &Entry);

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

 
 //===----------------------------------------------------------------------===//
 
+void DbgVariable::updateFromMachineInstr(const MachineInstr *M) {
+  MInsn = M;
+  // Update Var from MInsn. The DBG_VALUE may describe a specific
+  // piece of the variable in its complex expression.
+  assert(Var.getName() == MInsn->getDebugVariable().getName()
+         && "variable and variable in DBG_VALUE do not match");
+  Var = MInsn->getDebugVariable();
+}
+
 /// resolve - Look in the DwarfDebug map for the MDNode that
 /// corresponds to the reference.
 template <typename T> T DbgVariable::resolve(DIRef<T> Ref) const {

   llvm_unreachable("Unexpected 3 operand DBG_VALUE instruction!");
 }
 
+/// Determine whether two variable pieces overlap.
+static bool piecesOverlap(DIVariable P1, DIVariable P2) {
+  if (!P1.isVariablePiece() || !P2.isVariablePiece())
+    return true;
+  return (P1.getPieceOffset() < (P2.getPieceOffset() + P2.getPieceSize())) &&
+         (P2.getPieceOffset() < (P1.getPieceOffset() + P1.getPieceSize()));
+}
+
+/// Build the location list for all DBG_VALUEs in the function that
+/// describe the same variable.  If the ranges of several independent
+/// pieces of the same variable overlap partially, split them up and
+/// combine the ranges. The resulting DebugLocEntries are will have
+/// strict monotonically increasing begin addresses and will never
+/// overlap.
+//
+// Input:
+//
+//   Ranges History [var, loc, piece ofs size]
+// 0 |      [x, reg0, piece 0, 32]
+// 1 | |    [x, reg1, piece 32, 32] <- IsPieceOfPrevEntry
+// 2 | |     ...
+// 3   |    [clobber reg0]
+// 4        [x, mem, piece 0, 64] <- overlapping piece for 0 and 1.
+//
+// Output:
+//
+// [0-1]    [x, reg0, piece 0, 32]
+// [1-3]    [x, reg0, piece 0, 32, reg1, piece 32, 32]
+// [3-4]    [x, reg1, piece 32, 32]
+// [4- ]    [x, mem,  piece 0, 64] <- overlapping piece for 0 and 1.
+void DwarfDebug::
+buildLocationList(SmallVectorImpl<DebugLocEntry> &DebugLoc,
+                  const DbgValueHistoryMap::InstrRanges &Ranges,
+                  DwarfCompileUnit *TheCU) {
+  typedef std::pair<DIVariable, DebugLocEntry::Value> Range;
+  SmallVector<Range, 4> OpenRanges;
+
+  for (auto I = Ranges.begin(), E = Ranges.end(); I != E; ++I) {
+    const MachineInstr *Begin = I->first;
+    const MachineInstr *End = I->second;
+    assert(Begin->isDebugValue() && "Invalid History entry");
+
+
+    // Check if a variable is unaccessible in this range.
+    if (!Begin->isDebugValue() ||
+        (Begin->getNumOperands() > 1 && Begin->getOperand(0).isReg() &&
+         !Begin->getOperand(0).getReg())) {
+      OpenRanges.clear();
+      continue;
+    }
+
+    // If this piece overlaps with any open ranges, truncate them.
+    DIVariable DIVar = Begin->getDebugVariable();
+    auto Last = std::remove_if(OpenRanges.begin(), OpenRanges.end(), [&](Range R){
+        return piecesOverlap(DIVar, R.first);
+      });
+    OpenRanges.erase(Last, OpenRanges.end());
+
+
+    const MCSymbol *StartLabel = getLabelBeforeInsn(Begin);
+    assert(StartLabel && "Forgot label before DBG_VALUE starting a range!");
+
+    const MCSymbol *EndLabel;
+    if (End != nullptr)
+      EndLabel = getLabelAfterInsn(End);
+    else if (std::next(I) == Ranges.end())
+      EndLabel = FunctionEndSym;
+    else
+      EndLabel = getLabelBeforeInsn(std::next(I)->first);
+    assert(EndLabel && "Forgot label after instruction ending a range!");
+
+    DEBUG(dbgs() << "DotDebugLoc Pair:\n"
+                 << "\t" << *Begin << "\t" << *End << "\n");
+
+    auto Value = getDebugLocValue(Begin);
+    DebugLocEntry Loc(StartLabel, EndLabel, Value, TheCU);
+    if (DebugLoc.empty() || !DebugLoc.back().Merge(Loc)) {
+      // Add all values from still valid non-overlapping pieces.
+      for (auto Range : OpenRanges)
+        Loc.addValue(Range.second);
+      DebugLoc.push_back(std::move(Loc));
+    }
+    // Add this value to the list of open ranges.
+    if (DIVar.isVariablePiece())
+      OpenRanges.push_back({DIVar, Value});
+
+    DEBUG(dbgs() << "Values:\n";
+          for (auto Value : DebugLoc.back().getValues())
+            Value.getVariable()->dump();
+          dbgs() << "-----\n");
+  }
+}
+
+
 // Find variables for each lexical scope.
 void
 DwarfDebug::collectVariableInfo(SmallPtrSet<const MDNode *, 16> &Processed) {

     if (!Scope)
       continue;
 
-    Processed.insert(DV);
+    Processed.insert(getEntireVariable(DV));
     const MachineInstr *MInsn = Ranges.front().first;
     assert(MInsn->isDebugValue() && "History must begin with debug value");
     DbgVariable *AbsVar = findAbstractVariable(DV, MInsn->getDebugLoc());

 
     // Check if the first DBG_VALUE is valid for the rest of the function.
     if (Ranges.size() == 1 && Ranges.front().second == nullptr) {
-      RegVar->setMInsn(MInsn);
+      RegVar->updateFromMachineInstr(MInsn);
       continue;
     }
 

     DebugLocList &LocList = DotDebugLocEntries.back();
     LocList.Label =
         Asm->GetTempSymbol("debug_loc", DotDebugLocEntries.size() - 1);
-    SmallVector<DebugLocEntry, 4> &DebugLoc = LocList.List;
-    for (auto I = Ranges.begin(), E = Ranges.end(); I != E; ++I) {
-      const MachineInstr *Begin = I->first;
-      const MachineInstr *End = I->second;
-      assert(Begin->isDebugValue() && "Invalid History entry");
-
-      // Check if a variable is unaccessible in this range.
-      if (Begin->getNumOperands() > 1 && Begin->getOperand(0).isReg() &&
-          !Begin->getOperand(0).getReg())
-        continue;
 
-      const MCSymbol *StartLabel = getLabelBeforeInsn(Begin);
-      assert(StartLabel && "Forgot label before DBG_VALUE starting a range!");
-
-      const MCSymbol *EndLabel;
-      if (End != nullptr)
-        EndLabel = getLabelAfterInsn(End);
-      else if (std::next(I) == Ranges.end())
-        EndLabel = FunctionEndSym;
-      else
-        EndLabel = getLabelBeforeInsn(std::next(I)->first);
-      assert(EndLabel && "Forgot label after instruction ending a range!");
-
-      DEBUG(dbgs() << "DotDebugLoc Pair:\n"
-                   << "\t" << *Begin << "\t" << *End << "\n");
-      DebugLocEntry Loc(StartLabel, EndLabel, getDebugLocValue(Begin), TheCU);
-      if (DebugLoc.empty() || !DebugLoc.back().Merge(Loc))
-        DebugLoc.push_back(std::move(Loc));
-    }
+    // Build the location list for this variable.
+    buildLocationList(LocList.List, Ranges, TheCU);
   }
 
   // Collect info for variables that were optimized out.

 
     // The first mention of a function argument gets the FunctionBeginSym
     // label, so arguments are visible when breaking at function entry.
-    DIVariable DV(I.first);
+    DIVariable DV(Ranges.front().first->getDebugVariable());
     if (DV.isVariable() && DV.getTag() == dwarf::DW_TAG_arg_variable &&
-        getDISubprogram(DV.getContext()).describes(MF->getFunction()))
-      LabelsBeforeInsn[Ranges.front().first] = FunctionBeginSym;
+        getDISubprogram(DV.getContext()).describes(MF->getFunction())) {
+      if (!DV.isVariablePiece())
+        LabelsBeforeInsn[Ranges.front().first] = FunctionBeginSym;
+      else {
+        // Mark all non-overlapping initial pieces.
+        for (auto I = Ranges.begin(); I != Ranges.end(); ++I) {
+          DIVariable Piece = I->first->getDebugVariable();
+          if (std::all_of(Ranges.begin(), I,
+                          [&](DbgValueHistoryMap::InstrRange Pred){
+                return !piecesOverlap(Piece, Pred.first->getDebugVariable());
+              }))
+            LabelsBeforeInsn[I->first] = FunctionBeginSym;
+          else
+            break;
+        }
+      }
+    }
 
     for (const auto &Range : Ranges) {
       requestLabelBeforeInsn(Range.first);

   Holder.emitStrings(Asm->getObjFileLowering().getDwarfStrSection());
 }
 
+/// Emits an optimal (=sorted) sequence of DW_OP_pieces.
+void DwarfDebug::emitLocPieces(ByteStreamer &Streamer,
+                               const DITypeIdentifierMap &Map,
+                               ArrayRef<DebugLocEntry::Value> Values) {
+  typedef DebugLocEntry::Value Piece;
+  SmallVector<Piece, 4> Pieces(Values.begin(), Values.end());
+  assert(std::all_of(Pieces.begin(), Pieces.end(), [](Piece &P) {
+        return DIVariable(P.getVariable()).isVariablePiece();
+      }) && "all values are expected to be pieces");
+
+  // Sort the pieces so they can be emitted using DW_OP_piece.
+  std::sort(Pieces.begin(), Pieces.end(), [](const Piece &A, const Piece &B) {
+      DIVariable VarA(A.getVariable());
+      DIVariable VarB(B.getVariable());
+      return VarA.getPieceOffset() < VarB.getPieceOffset();
+    });
+  // Remove any duplicate entries by dropping all but the first.
+  Pieces.erase(std::unique(Pieces.begin(), Pieces.end(),
+                           [] (const Piece &A,const Piece &B){
+                             return A.getVariable() == B.getVariable();
+                           }), Pieces.end());
+
+  unsigned Offset = 0;
+  for (auto Piece : Pieces) {
+    DIVariable Var(Piece.getVariable());
+    unsigned PieceOffset = Var.getPieceOffset();
+    unsigned PieceSize = Var.getPieceSize();
+    assert(Offset <= PieceOffset && "overlapping pieces in DebugLocEntry");
+    if (Offset < PieceOffset) {
+      // The DWARF spec seriously mandates pieces with no locations for gaps.
+      Asm->EmitDwarfOpPiece(Streamer, (PieceOffset-Offset)*8);
+      Offset += PieceOffset-Offset;
+    }
+
+    Offset += PieceSize;
+
+    // Assuming 8 bits per byte.
+    assert(!Var.isIndirect() && "indirect address for piece");
+#ifndef NDEBUG
+    unsigned VarSize = Var.getSizeInBits(Map);
+    assert(PieceSize+PieceOffset <= VarSize/8
+     && "piece is larger than or outside of variable");
+    assert(PieceSize*8 != VarSize
+     && "piece covers entire variable");
+#endif
+    if (Piece.isLocation() && Piece.getLoc().isReg())
+      Asm->EmitDwarfRegOpPiece(Streamer,
+             Piece.getLoc(),
+             PieceSize*8);
+    else {
+      emitDebugLocValue(Streamer, Piece);
+      Asm->EmitDwarfOpPiece(Streamer, PieceSize*8);
+    }
+  }
+}
+
+
 void DwarfDebug::emitDebugLocEntry(ByteStreamer &Streamer,
                                    const DebugLocEntry &Entry) {
-  assert(Entry.getValues().size() == 1 &&
-         "multi-value entries are not supported yet.");
   const DebugLocEntry::Value Value = Entry.getValues()[0];
   DIVariable DV(Value.getVariable());
+  if (DV.isVariablePiece())
+    // Emit all pieces that belong to the same variable and range.
+    return emitLocPieces(Streamer, TypeIdentifierMap, Entry.getValues());
+
+  assert(Entry.getValues().size() == 1 && "only pieces may have >1 value");
+  emitDebugLocValue(Streamer, Value);
+}
+
+void DwarfDebug::emitDebugLocValue(ByteStreamer &Streamer,
+                                   const DebugLocEntry::Value &Value) {
+  DIVariable DV(Value.getVariable());
+  // Regular entry.
   if (Value.isInt()) {
     DIBasicType BTy(resolve(DV.getType()));
     if (BTy.Verify() && (BTy.getEncoding() == dwarf::DW_ATE_signed ||

         } else if (Element == DIBuilder::OpDeref) {
           if (!Loc.isReg())
             Streamer.EmitInt8(dwarf::DW_OP_deref, "DW_OP_deref");
+        } else if (Element == DIBuilder::OpPiece) {
+          i += 3;
+          // handled in emitDebugLocEntry.
         } else
           llvm_unreachable("unknown Opcode found in complex address");
       }

lib/CodeGen/AsmPrinter/DwarfUnit.cpp

     } else if (Element == DIBuilder::OpDeref) {
       if (!Location.isReg())
         addUInt(*Loc, dwarf::DW_FORM_data1, dwarf::DW_OP_deref);
+    } else if (Element == DIBuilder::OpPiece) {
+      addUInt(*Loc, dwarf::DW_FORM_data1, dwarf::DW_OP_bit_piece);
+      addUInt(*Loc, dwarf::DW_FORM_udata, DV.getAddrElement(++i));
+      addUInt(*Loc, dwarf::DW_FORM_udata, DV.getAddrElement(++i));
     } else
       llvm_unreachable("unknown DIBuilder Opcode");
   }

lib/IR/DIBuilder.cpp

   Elts.push_back(Constant::getNullValue(Type::getInt32Ty(VMContext)));
   Elts.push_back(Constant::getNullValue(Type::getInt32Ty(VMContext)));
   Elts.append(Addr.begin(), Addr.end());
+  return DIVariable(MDNode::get(VMContext, Elts));
+}
 
+/// createVariablePiece - Create a descriptor to describe one part
+/// of aggregate variable that is fragmented across multiple Values.
+DIVariable DIBuilder::createVariablePiece(DIVariable Variable,
+                                          unsigned OffsetInBytes,
+                                          unsigned SizeInBytes) {
+  assert(SizeInBytes > 0 && "zero-size piece");
+  SmallVector<Value *, 12> Elts;
+  for (unsigned i = 0; i < Variable->getNumOperands(); ++i)
+    Elts.push_back(Variable->getOperand(i));
+
+  Elts.push_back(ConstantInt::get(Type::getInt32Ty(VMContext), OpPiece));
+  Elts.push_back(ConstantInt::get(Type::getInt32Ty(VMContext), OffsetInBytes));
+  Elts.push_back(ConstantInt::get(Type::getInt32Ty(VMContext), SizeInBytes));
   return DIVariable(MDNode::get(VMContext, Elts));
 }
 

lib/IR/DebugInfo.cpp

 //
 //===----------------------------------------------------------------------===//
 
-#include "llvm/IR/DebugInfo.h"
 #include "LLVMContextImpl.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/Constants.h"
+#include "llvm/IR/DIBuilder.h"
+#include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"

 /// getInlinedAt - If this variable is inlined then return inline location.
 MDNode *DIVariable::getInlinedAt() const { return getNodeField(DbgNode, 7); }
 
+bool DIVariable::isVariablePiece() const {
+  return hasComplexAddress() && getAddrElement(0) == DIBuilder::OpPiece;
+}
+
+uint64_t DIVariable::getPieceOffset() const {
+  assert(isVariablePiece());
+  return getAddrElement(1);
+}
+
+uint64_t DIVariable::getPieceSize() const {
+  assert(isVariablePiece());
+  return getAddrElement(2);
+}
+
+/// Return the size reported by the variable's type.
+unsigned DIVariable::getSizeInBits(const DITypeIdentifierMap &Map) {
+  DIType Ty = getType().resolve(Map);
+  // Follow derived types until we reach a type that
+  // reports back a size.
+  while (Ty.isDerivedType() && !Ty.isCompositeType()) {
+    DIDerivedType DT(&*Ty);
+    Ty = DT.getTypeDerivedFrom().resolve(Map);
+  }
+  assert(Ty.getSizeInBits() && "type with size 0");
+  return Ty.getSizeInBits();
+}
+
+
+
+
 //===----------------------------------------------------------------------===//
 // Predicates
 //===----------------------------------------------------------------------===//

   return DIVariable(MDNode::get(VMContext, Elts));
 }
 
+
+/// getEntireVariable - Remove OpPiece exprs from the variable.
+DIVariable llvm::getEntireVariable(DIVariable DV) {
+  if (!DV.isVariablePiece())
+    return DV;
+
+  SmallVector<Value *, 8> Elts;
+  for (unsigned i = 0; i < 8; ++i)
+    Elts.push_back(DV->getOperand(i));
+
+  return DIVariable(MDNode::get(DV->getContext(), Elts));
+}
+
+
 /// getDISubprogram - Find subprogram that is enclosing this scope.
 DISubprogram llvm::getDISubprogram(const MDNode *Scope) {
   DIDescriptor D(Scope);

     OS << " [" << Res << ']';
 
   OS << " [line " << getLineNumber() << ']';
+
+  if (isVariablePiece())
+    OS << " [piece, size " << getPieceSize()
+       << ", offset " << getPieceOffset() << ']';
 }
 
 void DIObjCProperty::printInternal(raw_ostream &OS) const {

test/DebugInfo/X86/pieces-1.ll

+; RUN: llc -O0 %s -filetype=obj -o %t.o
+; RUN: llvm-dwarfdump -debug-dump=loc %t.o | FileCheck --check-prefix=CHECK-DWARF %s
+;
+; rdar://problem/15928306
+;
+; Test that we can emit debug info for aggregate values that are split
+; up across multiple registers by SROA.
+;
+;    // Compile with -O1.
+;    typedef struct { long int a; int b;} S;
+;
+;    int foo(S s) {
+;            return s.b;
+;    }
+;
+;
+; CHECK-DWARF: .debug_loc contents:
+;
+
+; 0x0000000000000000 - 0x0000000000000006: rdi, piece 0x00000008, rsi, piece 0x00000004
+; CHECK-DWARF:            Beginning address offset: 0x0000000000000000
+; CHECK-DWARF:               Ending address offset: [[LTMP3:.*]]
+; CHECK-DWARF:                Location description: 55 93 08 54 93 04
+; 0x0000000000000006 - 0x0000000000000008: rbp-8, piece 0x00000008, rax, piece 0x00000004 )
+; CHECK-DWARF:            Beginning address offset: [[LTMP3]]
+; CHECK-DWARF:               Ending address offset: [[END:.*]]
+; CHECK-DWARF:                Location description: 76 78 93 08 54 93 04
+
+; Function Attrs: nounwind ssp uwtable
+define i32 @foo(i64 %s.coerce0, i32 %s.coerce1) #0 {
+entry:
+  call void @llvm.dbg.value(metadata !{i64 %s.coerce0}, i64 0, metadata !20), !dbg !21
+  call void @llvm.dbg.value(metadata !{i32 %s.coerce1}, i64 0, metadata !22), !dbg !21
+  ret i32 %s.coerce1, !dbg !23
+}
+
+; Function Attrs: nounwind readnone
+declare void @llvm.dbg.declare(metadata, metadata) #1
+
+; Function Attrs: nounwind readnone
+declare void @llvm.dbg.value(metadata, i64, metadata) #1
+
+attributes #0 = { nounwind ssp uwtable "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" }
+attributes #1 = { nounwind readnone }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!17, !18}
+!llvm.ident = !{!19}
+
+!0 = metadata !{i32 786449, metadata !1, i32 12, metadata !"clang version 3.5 ", i1 true, metadata !"", i32 0, metadata !2, metadata !2, metadata !3, metadata !2, metadata !2, metadata !""}
+!1 = metadata !{metadata !"pieces.c", metadata !""}
+!2 = metadata !{}
+!3 = metadata !{metadata !4}
+!4 = metadata !{i32 786478, metadata !1, metadata !5, metadata !"foo", metadata !"foo", metadata !"", i32 3, metadata !6, i1 false, i1 true, i32 0, i32 0, null, i32 256, i1 true, i32 (i64, i32)* @foo, null, null, metadata !15, i32 3} ; [ DW_TAG_subprogram ] [line 3] [def] [foo]
+!5 = metadata !{i32 786473, metadata !1}          ; [ DW_TAG_file_type ] [/pieces.c]
+!6 = metadata !{i32 786453, i32 0, null, metadata !"", i32 0, i64 0, i64 0, i64 0, i32 0, null, metadata !7, i32 0, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]
+!7 = metadata !{metadata !8, metadata !9}
+!8 = metadata !{i32 786468, null, null, metadata !"int", i32 0, i64 32, i64 32, i64 0, i32 0, i32 5} ; [ DW_TAG_base_type ] [int] [line 0, size 32, align 32, offset 0, enc DW_ATE_signed]
+!9 = metadata !{i32 786454, metadata !1, null, metadata !"S", i32 1, i64 0, i64 0, i64 0, i32 0, metadata !10} ; [ DW_TAG_typedef ] [S] [line 1, size 0, align 0, offset 0] [from ]
+!10 = metadata !{i32 786451, metadata !1, null, metadata !"", i32 1, i64 128, i64 64, i32 0, i32 0, null, metadata !11, i32 0, null, null, null} ; [ DW_TAG_structure_type ] [line 1, size 128, align 64, offset 0] [def] [from ]
+!11 = metadata !{metadata !12, metadata !14}
+!12 = metadata !{i32 786445, metadata !1, metadata !10, metadata !"a", i32 1, i64 64, i64 64, i64 0, i32 0, metadata !13} ; [ DW_TAG_member ] [a] [line 1, size 64, align 64, offset 0] [from long int]
+!13 = metadata !{i32 786468, null, null, metadata !"long int", i32 0, i64 64, i64 64, i64 0, i32 0, i32 5} ; [ DW_TAG_base_type ] [long int] [line 0, size 64, align 64, offset 0, enc DW_ATE_signed]
+!14 = metadata !{i32 786445, metadata !1, metadata !10, metadata !"b", i32 1, i64 32, i64 32, i64 64, i32 0, metadata !8} ; [ DW_TAG_member ] [b] [line 1, size 32, align 32, offset 64] [from int]
+!15 = metadata !{metadata !16}
+!16 = metadata !{i32 786689, metadata !4, metadata !"s", metadata !5, i32 16777219, metadata !9, i32 0, i32 0} ; [ DW_TAG_arg_variable ] [s] [line 3]
+!17 = metadata !{i32 2, metadata !"Dwarf Version", i32 4}
+!18 = metadata !{i32 1, metadata !"Debug Info Version", i32 1}
+!19 = metadata !{metadata !"clang version 3.5 "}
+!20 = metadata !{i32 786689, metadata !4, metadata !"s", metadata !5, i32 16777219, metadata !9, i32 0, i32 0, i64 3, i64 0, i64 8} ; [ DW_TAG_arg_variable ] [s] [line 3] [piece, size 8, offset 0]
+!21 = metadata !{i32 3, i32 0, metadata !4, null}
+!22 = metadata !{i32 786689, metadata !4, metadata !"s", metadata !5, i32 16777219, metadata !9, i32 0, i32 0, i64 3, i64 8, i64 4} ; [ DW_TAG_arg_variable ] [s] [line 3] [piece, size 4, offset 8]
+!23 = metadata !{i32 4, i32 0, metadata !4, null}
+!25 = metadata !{}