[DebugInfo][RemoveDIs] Implement behind-the-scenes debug-info maintenance in splice / moveBefore / insertBefore APIs

[jmorse]

Summary

This is the "central" patch to the removing-debug-intrinsics project: it changes the instruction movement APIs (insert, move, splice) to interpret the "Head" bits we're attaching to BasicBlock::iterators, and updates debug-info records in the background to preserve the ordering of debug-info (which is in DPValue objects instead of dbg.values). The cost is the complexity of this patch, plus memory. The benefit is that LLVM developers can cease thinking about whether they're moving debug-info or not, because it'll happen behind the scenes.

To get an idea of what's going on, immediately skip to looking at the comment in BasicBlock::spliceDebugInfo. In dbg.value form, variable location information gets moved around automatically because it's an instruction, and it always stays in the right order. However, once converted into DPValue objects, we have to do this manually. The presence of the "head" bits on iterators indicates whether or not a transfer of instructions is intended to include the leading debug-info or not (in one scenario: the trailing debug-info). BasicBlock::splice now interprets those and moves DPValues around as appropriate. The range of transfers that can be described in splice() is explicitly enumerated in the unit test added, and there's a worked example in each scenario for clarity.

The rest of this patch applies the same reasoning in a variety of scenarios. When moveBefore (and it's siblings) are used to move instructions around, the caller has to indicate whether they intend for debug-info to move too (is it a "Preserving" call or not), and then the "Head" bits used to determine where debug-info moves to. Similar reasoning is needed for insertBefore.

Some utility methods are added to Instruction to allow examining whether an instruction has debug-info attached to it or not.

[Orlando]

SGTM so far, just working my way through the unittests now.

llvm/include/llvm/IR/Instruction.h

75

nit: from_here -> FromHere

87–88

Is it worth specifying in the comment the behaviour if there are multiple / none attached?

197

Perhaps moveBeforeImpl as it feels more idiomatic?

206

IMO worth adding comments describing the difference between moveBefore and moveBeforePreserving.

llvm/lib/IR/BasicBlock.cpp

835–836

up to and including or excluding Last?

841–842

I'm not sure I understand this final option - why would we need to move : them up ahead so far? Ah, is "those" in

or should those go before "+" DPValues?

referring to =?

879

Very helpful comment and diagrams!

llvm/lib/IR/Instruction.cpp

85

I think (It doesn't so far) can be removed.

115–116

I'm a bit confused about what this comment is saying, please can you explain further (here)?

245–246

nit: Any reason to use trailing return type here (same return type in function above doesn't)?

252

Hoist the assert above the if block since Parent is dereferenced there? IMO a vague string would be useful too (or comment above the assert).

llvm/unittests/IR/BasicBlockDbgInfoTest.cpp

212

EXPECT_TRUE(BeginIt.getHeadBit()); here for completeness?

258

nit: comma -> full stop? Otherwise gives the impression of an unfinished comment.

[Orlando]

Unittests LGTM. Comprehensive coverage yet easy to read, nice.

llvm/unittests/IR/BasicBlockDbgInfoTest.cpp

1091

ASSERT/CHECK_TRUE(Entry.empty()) for completeness? YMMV on this one.

[jmorse]

(Addressing comments, ticket boxes etc).

llvm/include/llvm/IR/Instruction.h

87–88

IMO no, as the DPValue argument should fairly obviously be the DPValue that's to be dropped -- it's not "drop one random" or "drop the first DPValue" after all.

197

SGTM

206

(Inserting a comment about the earlier moveBeforePreserving, and referring to it from here.

llvm/lib/IR/BasicBlock.cpp

841–842

Good spot, "those" is ==== here, I'll make that explicit.

llvm/lib/IR/Instruction.cpp

115–116

My recollection is that there are no scenarios where DPValues can change what instruction they're attached to as a result of insertAfter, for example:

call dbg.value
%foo = add
call dbg.value
%bar = sub
call dbg.value

Inserting after %foo or after %bar doesn't cause an instruction be inserted inbetween dbg.values and the next instruction. All the dbg.values continue to "happen" at the next instruction, and that's preserved using DPValues too.

Of course, if the insert position of an instruction is a dbg.value intrinsic then this is no longer true. Not allowing people to do that is one of the design goals though: doing that is always a bug.

245–246

I think there was a reason once, but it's since shifted X_X

[jmorse]

Some other changes that have filtered in:

• CodeGenPrepare::placeDbgValues deliberately removes dbg.value intrinsics, so I've removed the assertion in Instruction::eraseFromParent. We're in a much stronger position wrt. binary-identicalness so I'm not worried about this.
• There are some fixes for TrailingDPValues now being stored in LLVMContextImpl, but they're all mechanical.
• Some things have been renamed / reworded as suggested.
• One of the comments in the big-comment in spliceDebugInfo was wrong, "Or if we didn't want to insert at the head of Dest"... but the example had the "head" bit set to true. I've flipped this to the correct way, but figured I'd highlight this as being deliberate rather than an accident. If we don't want to insert before dbg.values on an iterator, the "head" bit should be false.

[Orlando]

Still LGTM

llvm/lib/IR/Instruction.cpp

115–116

That makes sense, though I still find the code comment confusing. If there's anything you can do to make it clearer that might help. YMMV

[jmorse]

llvm/lib/IR/Instruction.cpp

115–116

Ah yeah, I was going to update it, whoops. How about instead:

"There cannot be any debug-info records in the gap between this instruction, and the one we're inserting right now, therefore no DPValue maintenance is required".

?

[Orlando]

llvm/lib/IR/Instruction.cpp

115–116

IMO "There cannot be any debug-info records in the gap between this instruction and the one we're inserting" feels slightly misleading as there might be debug-info records, it's just that everything still ends up in the correct order without intervention IIUC.

How about something like:
"Inserting 'this' after 'InsertPos', 'this' comes before any debug-info records attached to the next instruction, so no debug-info updates are necessary."

I'm not 100% happy with that either. I feel I'm splitting hairs here though, so if you're not a fan of this iteration I'm happy for you to go with your suggestion.

llvm/include/llvm/IR/BasicBlock.h

 #define LLVM_IR_BASICBLOCK_H
 
 #include "llvm-c/Types.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/ADT/ilist.h"
 #include "llvm/ADT/ilist_node.h"
 #include "llvm/ADT/iterator.h"
 #include "llvm/ADT/iterator_range.h"
+#include "llvm/IR/DebugProgramInstruction.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/DebugProgramInstruction.h"
 #include "llvm/IR/SymbolTableListTraits.h"
 #include "llvm/IR/Value.h"
 #include <cassert>
 #include <cstddef>
 #include <iterator>
 
public:
   DPMarker *getTrailingDPValues();
 
   /// Delete any trailing DPValues at the end of this block, see
   /// \ref setTrailingDPValues.
   void deleteTrailingDPValues();
 
   void dumpDbgValues() const;
 
+  /// Return the DPMarker for the position given by \p It, so that DPValues can
+  /// be inserted there. This will either be nullptr if not present, a DPMarker,
+  /// or TrailingDPValues if It is end().
+  DPMarker *getMarker(InstListType::iterator It);
+
+  /// Return the DPMarker for the position that comes after \p I. \see
+  /// BasicBlock::getMarker, this can be nullptr, a DPMarker, or
+  /// TrailingDPValues if there is no next instruction.
+  DPMarker *getNextMarker(Instruction *I);
+
+  /// Insert a DPValue into a block at the position given by \p I.
+  void insertDPValueAfter(DPValue *DPV, Instruction *I);
+
+  /// Insert a DPValue into a block at the position given by \p Here.
+  void insertDPValueBefore(DPValue *DPV, InstListType::iterator Here);
+
+  /// Eject any debug-info trailing at the end of a block. DPValues can
+  /// transiently be located "off the end" of a block if the blocks terminator
+  /// is temporarily removed. Once a terminator is re-inserted this method will
+  /// move such DPValues back to the right place (ahead of the terminator).
+  void flushTerminatorDbgValues();
+
 private:
   void setParent(Function *parent);
 
   /// Constructor.
   ///
   /// If the function parameter is specified, the basic block is automatically
   /// inserted at either the end of the function (if InsertBefore is null), or
   /// before the specified basic block.
public:
   friend BasicBlock::iterator Instruction::eraseFromParent();
   friend BasicBlock::iterator Instruction::insertInto(BasicBlock *BB,
                                                       BasicBlock::iterator It);
   friend class llvm::SymbolTableListTraits<llvm::Instruction,
                                            ilist_iterator_bits<true>>;
   friend class llvm::ilist_node_with_parent<llvm::Instruction, llvm::BasicBlock,
                                             ilist_iterator_bits<true>>;
 
+  // Friendly methods that need to access us for the maintenence of
+  // debug-info attachments.
+  friend void Instruction::insertBefore(BasicBlock::iterator InsertPos);
+  friend void Instruction::insertAfter(Instruction *InsertPos);
+  friend void Instruction::insertBefore(BasicBlock &BB,
+                                        InstListType::iterator InsertPos);
+  friend void Instruction::moveBeforeImpl(BasicBlock &BB,
+                                          InstListType::iterator I,
+                                          bool Preserve);
+  friend iterator_range<DPValue::self_iterator> Instruction::cloneDebugInfoFrom(
+      const Instruction *From, std::optional<DPValue::self_iterator> FromHere,
+      bool InsertAtHead);
+
   /// Creates a new BasicBlock.
   ///
   /// If the Parent parameter is specified, the basic block is automatically
   /// inserted at either the end of the function (if InsertBefore is 0), or
   /// before the specified basic block.
   static BasicBlock *Create(LLVMContext &Context, const Twine &Name = "",
                             Function *Parent = nullptr,
                             BasicBlock *InsertBefore = nullptr) {
private:
   InstListType &getInstList() { return InstList; }
 
   /// Returns a pointer to a member of the instruction list.
   /// This is private on purpose, just like `getInstList()`.
   static InstListType BasicBlock::*getSublistAccess(Instruction *) {
     return &BasicBlock::InstList;
   }
 
+  /// Dedicated function for splicing debug-info: when we have an empty
+  /// splice (i.e. zero instructions), the caller may still intend any
+  /// debug-info in between the two "positions" to be spliced.
+  void spliceDebugInfoEmptyBlock(BasicBlock::iterator ToIt, BasicBlock *FromBB,
+                                 BasicBlock::iterator FromBeginIt,
+                                 BasicBlock::iterator FromEndIt);
+
+  /// Perform any debug-info specific maintenence for the given splice
+  /// activity. In the DPValue debug-info representation, debug-info is not
+  /// in instructions, and so it does not automatically move from one block
+  /// to another.
+  void spliceDebugInfo(BasicBlock::iterator ToIt, BasicBlock *FromBB,
+                       BasicBlock::iterator FromBeginIt,
+                       BasicBlock::iterator FromEndIt);
+
 public:
   /// Returns a pointer to the symbol table if one exists.
   ValueSymbolTable *getValueSymbolTable();
 
   /// Methods for support type inquiry through isa, cast, and dyn_cast.
   static bool classof(const Value *V) {
     return V->getValueID() == Value::BasicBlockVal;
   }

llvm/include/llvm/IR/Instruction.h

private:
   mutable unsigned Order = 0;
 
 public:
   /// Optional marker recording the position for debugging information that
   /// takes effect immediately before this instruction. Null unless there is
   /// debugging information present.
   DPMarker *DbgMarker = nullptr;
 
+  /// Clone any debug-info attached to \p From onto this instruction. Used to
+  /// copy debugging information from one block to another, when copying entire
+  /// blocks. \see DebugProgramInstruction.h , because the ordering of DPValues
+  /// is still important, fine grain control of which instructions are moved and
+  /// where they go is necessary.
+  /// \p From The instruction to clone debug-info from.
+  /// \p from_here Optional iterator to limit DPValues cloned to be a range from
+  ///    from_here to end().
+  /// \p InsertAtHead Whether the cloned DPValues should be placed at the end
+  ///    or the beginning of existing DPValues attached to this.
+  /// \returns A range over the newly cloned DPValues.
+  iterator_range<simple_ilist<DPValue>::iterator> cloneDebugInfoFrom(
+      const Instruction *From,
+      std::optional<simple_ilist<DPValue>::iterator> FromHere = std::nullopt,
+      bool InsertAtHead = false);
+
+  /// Return a range over the DPValues attached to this instruction.
+  iterator_range<simple_ilist<DPValue>::iterator> getDbgValueRange() const;
+
+  /// Returns true if any DPValues are attached to this instruction.
+  bool hasDbgValues() const;
+
+  /// Erase any DPValues attached to this instruction.
+  void dropDbgValues();
+
+  /// Erase a single DPValue \p I that is attached to this instruction.
+  void dropOneDbgValue(DPValue *I);
+
+  /// Handle the debug-info implications of this instruction being removed. Any
+  /// attached DPValues need to "fall" down onto the next instruction.
+  void handleMarkerRemoval();
+
 protected:
   // The 15 first bits of `Value::SubclassData` are available for subclasses of
   // `Instruction` to use.
   using OpaqueField = Bitfield::Element<uint16_t, 0, 15>;
 
   // Template alias so that all Instruction storing alignment use the same
   // definiton.
   // Valid alignments are powers of two from 2^0 to 2^MaxAlignmentExponent =
public:
   /// This method unlinks 'this' from the containing basic block and deletes it.
   ///
   /// \returns an iterator pointing to the element after the erased one
   InstListType::iterator eraseFromParent();
 
   /// Insert an unlinked instruction into a basic block immediately before
   /// the specified instruction.
   void insertBefore(Instruction *InsertPos);
-  void insertBefore(InstListType::iterator InsertPos) {
-    insertBefore(&*InsertPos);
-  }
+  void insertBefore(InstListType::iterator InsertPos);
 
   /// Insert an unlinked instruction into a basic block immediately after the
   /// specified instruction.
   void insertAfter(Instruction *InsertPos);
 
   /// Inserts an unlinked instruction into \p ParentBB at position \p It and
   /// returns the iterator of the inserted instruction.
   InstListType::iterator insertInto(BasicBlock *ParentBB,
                                     InstListType::iterator It);
 
-  void insertBefore(BasicBlock &BB, InstListType::iterator InsertPos) {
-    insertInto(&BB, InsertPos);
-  }
+  void insertBefore(BasicBlock &BB, InstListType::iterator InsertPos);
 
   /// Unlink this instruction from its current basic block and insert it into
   /// the basic block that MovePos lives in, right before MovePos.
   void moveBefore(Instruction *MovePos);
 
   /// Perform a \ref moveBefore operation, while signalling that the caller
   /// intends to preserve the original ordering of instructions. This implicitly
   /// means that any adjacent debug-info should move with this instruction.
   /// This method is currently a no-op placeholder, but it will become meaningful
   /// when the "RemoveDIs" project is enabled.
-  void moveBeforePreserving(Instruction *MovePos) {
-    moveBefore(MovePos);
-  }
+  void moveBeforePreserving(Instruction *MovePos);
 
+private:
+  /// RemoveDIs project: all other moves implemented with this method,
+  /// centralising debug-info updates into one place.
+  void moveBeforeImpl(BasicBlock &BB, InstListType::iterator I, bool Preserve);
+
+public:
   /// Unlink this instruction and insert into BB before I.
   ///
   /// \pre I is a valid iterator into BB.
   void moveBefore(BasicBlock &BB, InstListType::iterator I);
 
   /// (See other overload for moveBeforePreserving).
-  void moveBeforePreserving(BasicBlock &BB, InstListType::iterator I) {
-    moveBefore(BB, I);
-  }
+  void moveBeforePreserving(BasicBlock &BB, InstListType::iterator I);
 
   /// Unlink this instruction from its current basic block and insert it into
   /// the basic block that MovePos lives in, right after MovePos.
   void moveAfter(Instruction *MovePos);
 
   /// See \ref moveBeforePreserving .
-  void moveAfterPreserving(Instruction *MovePos) {
-    moveAfter(MovePos);
-  }
+  void moveAfterPreserving(Instruction *MovePos);
 
   /// Given an instruction Other in the same basic block as this instruction,
   /// return true if this instruction comes before Other. In this worst case,
   /// this takes linear time in the number of instructions in the block. The
   /// results are cached, so in common cases when the block remains unmodified,
   /// it takes constant time.
   bool comesBefore(const Instruction *Other) const;
 

llvm/lib/IR/BasicBlock.cpp

BasicBlock *BasicBlock::splitBasicBlockBefore(iterator I, const Twine &BBName) {
   }
   // Add a branch instruction from  "New" to "this" Block.
   BranchInst *BI = BranchInst::Create(this, New);
   BI->setDebugLoc(Loc);
 
   return New;
 }
 
-void BasicBlock::splice(BasicBlock::iterator ToIt, BasicBlock *FromBB,
-                        BasicBlock::iterator FromBeginIt,
-                        BasicBlock::iterator FromEndIt) {
-#ifdef EXPENSIVE_CHECKS
-  // Check that FromBeginIt is befor FromEndIt.
-  auto FromBBEnd = FromBB->end();
-  for (auto It = FromBeginIt; It != FromEndIt; ++It)
-    assert(It != FromBBEnd && "FromBeginIt not before FromEndIt!");
-#endif // EXPENSIVE_CHECKS
-  getInstList().splice(ToIt, FromBB->getInstList(), FromBeginIt, FromEndIt);
-}
-
 BasicBlock::iterator BasicBlock::erase(BasicBlock::iterator FromIt,
                                        BasicBlock::iterator ToIt) {
   return InstList.erase(FromIt, ToIt);
 }
 
 void BasicBlock::replacePhiUsesWith(BasicBlock *Old, BasicBlock *New) {
   // N.B. This might not be a complete BasicBlock, so don't assume
   // that it ends with a non-phi instruction.
void BasicBlock::renumberInstructions() {
   // Set the bit to indicate that the instruction order valid and cached.
   BasicBlockBits Bits = getBasicBlockBits();
   Bits.InstrOrderValid = true;
   setBasicBlockBits(Bits);
 
   NumInstrRenumberings++;
 }
 
+void BasicBlock::flushTerminatorDbgValues() {
+  // If we erase the terminator in a block, any DPValues will sink and "fall
+  // off the end", existing after any terminator that gets inserted. With
+  // dbg.value intrinsics we would just insert the terminator at end() and
+  // the dbg.values would come before the terminator. With DPValues, we must
+  // do this manually.
+  // To get out of this unfortunate form, whenever we insert a terminator,
+  // check whether there's anything trailing at the end and move those DPValues
+  // in front of the terminator.
+
+  // Do nothing if we're not in new debug-info format.
+  if (!IsNewDbgInfoFormat)
+    return;
+
+  // If there's no terminator, there's nothing to do.
+  Instruction *Term = getTerminator();
+  if (!Term)
+    return;
+
+  // Are there any dangling DPValues?
+  DPMarker *TrailingDPValues = getTrailingDPValues();
+  if (!TrailingDPValues)
+    return;
+
+  // Transfer DPValues from the trailing position onto the terminator.
+  Term->DbgMarker->absorbDebugValues(*TrailingDPValues, false);
+  deleteTrailingDPValues();
+}
+
+void BasicBlock::spliceDebugInfoEmptyBlock(BasicBlock::iterator Dest,
+                                           BasicBlock *Src,
+                                           BasicBlock::iterator First,
+                                           BasicBlock::iterator Last) {
+  // Imagine the folowing:
+  //
+  //   bb1:
+  //     dbg.value(...
+  //     ret i32 0
+  //
+  // If an optimisation pass attempts to splice the contents of the block from
+  // BB1->begin() to BB1->getTerminator(), then the dbg.value will be
+  // transferred to the destination.
+  // However, in the "new" DPValue format for debug-info, that range is empty:
+  // begin() returns an iterator to the terminator, as there will only be a
+  // single instruction in the block. We must piece together from the bits set
+  // in the iterators whether there was the intention to transfer any debug
+  // info.
+
+  // If we're not in "new" debug-info format, do nothing.
+  if (!IsNewDbgInfoFormat)
+    return;
+
+  assert(First == Last);
+  bool InsertAtHead = Dest.getHeadBit();
+  bool ReadFromHead = First.getHeadBit();
+
+  // If the source block is completely empty, including no terminator, then
+  // transfer any trailing DPValues that are still hanging around. This can
+  // occur when a block is optimised away and the terminator has been moved
+  // somewhere else.
+  if (Src->empty()) {
+    assert(Dest != end() &&
+           "Transferring trailing DPValues to another trailing position");
+    DPMarker *SrcTrailingDPValues = Src->getTrailingDPValues();
+    if (!SrcTrailingDPValues)
+      return;
+
+    DPMarker *M = Dest->DbgMarker;
+    M->absorbDebugValues(*SrcTrailingDPValues, InsertAtHead);
+    Src->deleteTrailingDPValues();
+    return;
+  }
+
+  // There are instructions in this block; if the First iterator was
+  // with begin() / getFirstInsertionPt() then the caller intended debug-info
+  // at the start of the block to be transferred.
+  if (!Src->empty() && First == Src->begin() && ReadFromHead)
+    Dest->DbgMarker->absorbDebugValues(*First->DbgMarker, InsertAtHead);
+
+  return;
+}
+
+void BasicBlock::spliceDebugInfo(BasicBlock::iterator Dest, BasicBlock *Src,
+                                 BasicBlock::iterator First,
+                                 BasicBlock::iterator Last) {
+  // Find out where to _place_ these dbg.values; if InsertAtHead is specified,
+  // this will be at the start of Dest's debug value range, otherwise this is
+  // just Dest's marker.
+  bool InsertAtHead = Dest.getHeadBit();
+  bool ReadFromHead = First.getHeadBit();
+  // Use this flag to signal the abnormal case, where we don't want to copy the
+  // DPValues ahead of the "Last" position.
+  bool ReadFromTail = !Last.getTailBit();
+
+  /*
+    Here's an illustration of what we're about to do. We have two blocks, this
+    and Src, and two segments of list. Each instruction is marked by a capital
+    while potential DPValue debug-info is marked out by "-" characters and a few
+    other special characters (+:=) where I want to highlight what's going on.
+
+                                                 Dest
+                                                   |
+     this-block:    A----A----A                ====A----A----A----A---A---A
+      Src-block                ++++B---B---B---B:::C
+                                   |               |
+                                  First           Last
+
+    The splice method is going to take all the instructions from First up to
+    (but not including) Last and insert them in _front_ of Dest, forming one
+    long list. All the DPValues attached to instructions _between_ First and
+    Last need no maintenence. However, we have to do special things with the
+    DPValues marked with the +:= characters. We only have three positions:
+    should the "+" DPValues be transferred, and if so to where? Do we move the
+    ":" DPValues? Would they go in front of the "=" DPValues, or should the "="
+    DPValues go before "+" DPValues?
+
+    We're told which way it should be by the bits carried in the iterators. The
+    "Head" bit indicates whether the specified position is supposed to be at the
+    front of the attached DPValues (true) or not (false). The Tail bit is true
+    on the other end of a range: is the range intended to include DPValues up to
+    the end (false) or not (true).
+
+    FIXME: the tail bit doesn't need to be distinct from the head bit, we could
+    combine them.
+
+    Here are some examples of different configurations:
+
+      Dest.Head = true, First.Head = true, Last.Tail = false
+
+      this-block:    A----A----A++++B---B---B---B:::====A----A----A----A---A---A
+                                    |                   |
+                                  First                Dest
+
+    Wheras if we didn't want to read from the Src list,
+
+      Dest.Head = true, First.Head = false, Last.Tail = false
+
+      this-block:    A----A----AB---B---B---B:::====A----A----A----A---A---A
+                                |                   |
+                              First                Dest
+
+    Or if we didn't want to insert at the head of Dest:
+
+      Dest.Head = false, First.Head = false, Last.Tail = false
+
+      this-block:    A----A----A====B---B---B---B:::A----A----A----A---A---A
+                                    |               |
+                                  First            Dest
+
+    Tests for these various configurations can be found in the unit test file
+    BasicBlockDbgInfoTest.cpp.
+
+   */
+
+  // Detach the marker at Dest -- this lets us move the "====" DPValues around.
+  DPMarker *DestMarker = nullptr;
+  if (Dest != end()) {
+    DestMarker = getMarker(Dest);
+    DestMarker->removeFromParent();
+    createMarker(&*Dest);
+  }
+
+  // If we're moving the tail range of DPValues (":::"), absorb them into the
+  // front of the DPValues at Dest.
+  if (ReadFromTail && Src->getMarker(Last)) {
+    DPMarker *OntoDest = getMarker(Dest);
+    DPMarker *FromLast = Src->getMarker(Last);
+    OntoDest->absorbDebugValues(*FromLast, true);
+  }
+
+  // If we're _not_ reading from the head of First, i.e. the "++++" DPValues,
+  // move their markers onto Last. They remain in the Src block. No action
+  // needed.
+  if (!ReadFromHead) {
+    DPMarker *OntoLast = Src->getMarker(Last);
+    DPMarker *FromFirst = Src->getMarker(First);
+    OntoLast->absorbDebugValues(*FromFirst,
+                                true); // Always insert at head of it.
+  }
+
+  // Finally, do something with the "====" DPValues we detached.
+  if (DestMarker) {
+    if (InsertAtHead) {
+      // Insert them at the end of the DPValues at Dest. The "::::" DPValues
+      // might be in front of them.
+      DPMarker *NewDestMarker = getMarker(Dest);
+      NewDestMarker->absorbDebugValues(*DestMarker, false);
+    } else {
+      // Insert them right at the start of the range we moved, ahead of First
+      // and the "++++" DPValues.
+      DPMarker *FirstMarker = getMarker(First);
+      FirstMarker->absorbDebugValues(*DestMarker, true);
+    }
+    DestMarker->eraseFromParent();
+  } else if (Dest == end() && !InsertAtHead) {
+    // In the rare circumstance where we insert at end(), and we did not
+    // generate the iterator with begin() / getFirstInsertionPt(), it means
+    // any trailing debug-info at the end of the block would "normally" have
+    // been pushed in front of "First". Move it there now.
+    DPMarker *FirstMarker = getMarker(First);
+    DPMarker *TrailingDPValues = getTrailingDPValues();
+    if (TrailingDPValues) {
+      FirstMarker->absorbDebugValues(*TrailingDPValues, true);
+      deleteTrailingDPValues();
+    }
+  }
+}
+
+void BasicBlock::splice(iterator Dest, BasicBlock *Src, iterator First,
+                        iterator Last) {
+  assert(Src->IsNewDbgInfoFormat == IsNewDbgInfoFormat);
+
+#ifdef EXPENSIVE_CHECKS
+  // Check that First is before Last.
+  auto FromBBEnd = Src->end();
+  for (auto It = First; It != Last; ++It)
+    assert(It != FromBBEnd && "FromBeginIt not before FromEndIt!");
+#endif // EXPENSIVE_CHECKS
+
+  // Lots of horrible special casing for empty transfers: the dbg.values between
+  // two positions could be spliced in dbg.value mode.
+  if (First == Last) {
+    spliceDebugInfoEmptyBlock(Dest, Src, First, Last);
+    return;
+  }
+
+  // Handle non-instr debug-info specific juggling.
+  if (IsNewDbgInfoFormat)
+    spliceDebugInfo(Dest, Src, First, Last);
+
+  // And move the instructions.
+  getInstList().splice(Dest, Src->getInstList(), First, Last);
+
+  flushTerminatorDbgValues();
+}
+
+void BasicBlock::insertDPValueAfter(DPValue *DPV, Instruction *I) {
+  assert(IsNewDbgInfoFormat);
+  assert(I->getParent() == this);
+
+  iterator NextIt = std::next(I->getIterator());
+  DPMarker *NextMarker =
+      (NextIt == end()) ? getTrailingDPValues() : NextIt->DbgMarker;
+  NextMarker->insertDPValue(DPV, true);
+}
+
+void BasicBlock::insertDPValueBefore(DPValue *DPV,
+                                     InstListType::iterator Where) {
+  // We should never directly insert at the end of the block, new DPValues
+  // shouldn't be generated at times when there's no terminator.
+  assert(Where != end());
+  assert(Where->getParent() == this);
+  bool InsertAtHead = Where.getHeadBit();
+  Where->DbgMarker->insertDPValue(DPV, InsertAtHead);
+}
+
+DPMarker *BasicBlock::getNextMarker(Instruction *I) {
+  return getMarker(std::next(I->getIterator()));
+}
+
+DPMarker *BasicBlock::getMarker(InstListType::iterator It) {
+  if (It == end()) {
+    DPMarker *DPM = getTrailingDPValues();
+    return DPM;
+  }
+  return It->DbgMarker;
+}
+
 #ifndef NDEBUG
 /// In asserts builds, this checks the numbering. In non-asserts builds, it
 /// is defined as a no-op inline function in BasicBlock.h.
 void BasicBlock::validateInstrOrdering() const {
   if (!isInstrOrderValid())
     return;
   const Instruction *Prev = nullptr;
   for (const Instruction &I : *this) {

llvm/lib/IR/DebugProgramInstruction.cpp

if (StoredDPValues.empty()) {
     eraseFromParent();
     Owner->DbgMarker = nullptr;
     return;
   }
 
   // The attached DPValues need to be preserved; attach them to the next
   // instruction. If there isn't a next instruction, put them on the
   // "trailing" list.
-  // (This logic gets refactored in a future patch, needed to break some
-  //  dependencies here).
-  BasicBlock::iterator NextInst = std::next(Owner->getIterator());
-  DPMarker *NextMarker;
-  if (NextInst == Owner->getParent()->end()) {
+  DPMarker *NextMarker = Owner->getParent()->getNextMarker(Owner);
+  if (NextMarker == nullptr) {
     NextMarker = new DPMarker();
     Owner->getParent()->setTrailingDPValues(NextMarker);
-  } else {
-    NextMarker = NextInst->DbgMarker;
   }
   NextMarker->absorbDebugValues(*this, true);
 
   eraseFromParent();
 }
 
 void DPMarker::removeFromParent() {
   MarkedInstr->DbgMarker = nullptr;

llvm/lib/IR/Instruction.cpp

const Module *Instruction::getModule() const {
   return getParent()->getModule();
 }
 
 const Function *Instruction::getFunction() const {
   return getParent()->getParent();
 }
 
 void Instruction::removeFromParent() {
+  // Perform any debug-info maintenence required.
+  handleMarkerRemoval();
+
   getParent()->getInstList().remove(getIterator());
 }
 
+void Instruction::handleMarkerRemoval() {
+  if (!Parent->IsNewDbgInfoFormat || !DbgMarker)
+    return;
+
+  DbgMarker->removeMarker();
+}
+
 BasicBlock::iterator Instruction::eraseFromParent() {
+  handleMarkerRemoval();
   return getParent()->getInstList().erase(getIterator());
 }
 
+void Instruction::insertBefore(Instruction *InsertPos) {
+  insertBefore(InsertPos->getIterator());
+}
+
 /// Insert an unlinked instruction into a basic block immediately before the
 /// specified instruction.
-void Instruction::insertBefore(Instruction *InsertPos) {
-  insertInto(InsertPos->getParent(), InsertPos->getIterator());
+void Instruction::insertBefore(BasicBlock::iterator InsertPos) {
+  insertBefore(*InsertPos->getParent(), InsertPos);
 }
 
 /// Insert an unlinked instruction into a basic block immediately after the
 /// specified instruction.
 void Instruction::insertAfter(Instruction *InsertPos) {
-  insertInto(InsertPos->getParent(), std::next(InsertPos->getIterator()));
+  BasicBlock *DestParent = InsertPos->getParent();
+
+  DestParent->getInstList().insertAfter(InsertPos->getIterator(), this);
+
+  // No need to manually update DPValues: if we insert after an instruction
+  // position, then we can never have any DPValues on "this".
+  if (DestParent->IsNewDbgInfoFormat)
+    DestParent->createMarker(this);
 }
 
 BasicBlock::iterator Instruction::insertInto(BasicBlock *ParentBB,
                                              BasicBlock::iterator It) {
   assert(getParent() == nullptr && "Expected detached instruction");
   assert((It == ParentBB->end() || It->getParent() == ParentBB) &&
          "It not in ParentBB");
-  return ParentBB->getInstList().insert(It, this);
+  insertBefore(*ParentBB, It);
+  return getIterator();
+}
+
+extern cl::opt<bool> UseNewDbgInfoFormat;
+
+void Instruction::insertBefore(BasicBlock &BB,
+                               InstListType::iterator InsertPos) {
+  assert(!DbgMarker);
+
+  BB.getInstList().insert(InsertPos, this);
+
+  if (!BB.IsNewDbgInfoFormat)
+    return;
+
+  BB.createMarker(this);
+
+  // We've inserted "this": if InsertAtHead is set then it comes before any
+  // DPValues attached to InsertPos. But if it's not set, then any DPValues
+  // should now come before "this".
+  bool InsertAtHead = InsertPos.getHeadBit();
+  if (!InsertAtHead) {
+    DPMarker *SrcMarker = BB.getMarker(InsertPos);
+    DbgMarker->absorbDebugValues(*SrcMarker, false);
+  }
+
+  // If we're inserting a terminator, check if we need to flush out
+  // TrailingDPValues.
+  if (isTerminator())
+    getParent()->flushTerminatorDbgValues();
 }
 
 /// Unlink this instruction from its current basic block and insert it into the
 /// basic block that MovePos lives in, right before MovePos.
 void Instruction::moveBefore(Instruction *MovePos) {
-  moveBefore(*MovePos->getParent(), MovePos->getIterator());
+  moveBeforeImpl(*MovePos->getParent(), MovePos->getIterator(), false);
+}
+
+void Instruction::moveBeforePreserving(Instruction *MovePos) {
+  moveBeforeImpl(*MovePos->getParent(), MovePos->getIterator(), true);
 }
 
 void Instruction::moveAfter(Instruction *MovePos) {
-  moveBefore(*MovePos->getParent(), ++MovePos->getIterator());
+  auto NextIt = std::next(MovePos->getIterator());
+  // We want this instruction to be moved to before NextIt in the instruction
+  // list, but before NextIt's debug value range.
+  NextIt.setHeadBit(true);
+  moveBeforeImpl(*MovePos->getParent(), NextIt, false);
+}
+
+void Instruction::moveAfterPreserving(Instruction *MovePos) {
+  auto NextIt = std::next(MovePos->getIterator());
+  // We want this instruction and its debug range to be moved to before NextIt
+  // in the instruction list, but before NextIt's debug value range.
+  NextIt.setHeadBit(true);
+  moveBeforeImpl(*MovePos->getParent(), NextIt, true);
 }
 
 void Instruction::moveBefore(BasicBlock &BB, InstListType::iterator I) {
+  moveBeforeImpl(BB, I, false);
+}
+
+void Instruction::moveBeforePreserving(BasicBlock &BB,
+                                       InstListType::iterator I) {
+  moveBeforeImpl(BB, I, true);
+}
+
+void Instruction::moveBeforeImpl(BasicBlock &BB, InstListType::iterator I,
+                              bool Preserve) {
   assert(I == BB.end() || I->getParent() == &BB);
-  BB.splice(I, getParent(), getIterator());
+  bool InsertAtHead = I.getHeadBit();
+
+  // If we've been given the "Preserve" flag, then just move the DPValues with
+  // the instruction, no more special handling needed.
+  if (BB.IsNewDbgInfoFormat && DbgMarker && !Preserve) {
+    if (I != this->getIterator()) {
+      // "this" is definitely moving; detach any existing DPValues.
+      handleMarkerRemoval();
+    }
+  }
+
+  // Move this single instruction. Use the list splice method directly, not
+  // the block splicer, which will do more debug-info things.
+  BB.getInstList().splice(I, getParent()->getInstList(), getIterator());
+
+  if (BB.IsNewDbgInfoFormat && !Preserve) {
+    if (!DbgMarker)
+      BB.createMarker(this);
+    DPMarker *NextMarker = getParent()->getNextMarker(this);
+
+    // If we're inserting at point I, and not in front of the DPValues attached
+    // there, then we should absorb the DPValues attached to I.
+    if (!InsertAtHead)
+      DbgMarker->absorbDebugValues(*NextMarker, false);
+  }
+
+  if (isTerminator())
+    getParent()->flushTerminatorDbgValues();
+}
+
+iterator_range<DPValue::self_iterator>
+Instruction::cloneDebugInfoFrom(const Instruction *From,
+                                std::optional<DPValue::self_iterator> FromHere,
+                                bool InsertAtHead) {
+  if (!From->DbgMarker)
+    return DPMarker::getEmptyDPValueRange();
+
+  assert(getParent()->IsNewDbgInfoFormat);
+  assert(getParent()->IsNewDbgInfoFormat ==
+         From->getParent()->IsNewDbgInfoFormat);
+
+  if (!DbgMarker)
+    getParent()->createMarker(this);
+
+  return DbgMarker->cloneDebugInfoFrom(From->DbgMarker, FromHere, InsertAtHead);
+}
+
+iterator_range<DPValue::self_iterator>
+Instruction::getDbgValueRange() const {
+  BasicBlock *Parent = const_cast<BasicBlock *>(getParent());
+  assert(Parent && "Instruction must be inserted to have DPValues");
+  if (!DbgMarker)
+    return DPMarker::getEmptyDPValueRange();
+
+  return DbgMarker->getDbgValueRange();
+}
+
+bool Instruction::hasDbgValues() const { return !getDbgValueRange().empty(); }
+
+void Instruction::dropDbgValues() {
+  if (DbgMarker)
+    DbgMarker->dropDPValues();
+}
+
+void Instruction::dropOneDbgValue(DPValue *DPV) {
+  DbgMarker->dropOneDPValue(DPV);
 }
 
 bool Instruction::comesBefore(const Instruction *Other) const {
   assert(Parent && Other->Parent &&
          "instructions without BB parents have no order");
   assert(Parent == Other->Parent && "cross-BB instruction order comparison");
   if (!Parent->isInstrOrderValid())
     Parent->renumberInstructions();

llvm/unittests/IR/BasicBlockDbgInfoTest.cpp

+//===- llvm/unittest/IR/BasicBlockTest.cpp - BasicBlock unit tests --------===//
+//
+// 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/IR/BasicBlock.h"
+#include "llvm/IR/DebugInfo.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/AsmParser/Parser.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/Instruction.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Module.h"
+#include "llvm/IR/NoFolder.h"
+#include "llvm/IR/Verifier.h"
+#include "llvm/Support/SourceMgr.h"
+#include "gmock/gmock-matchers.h"
+#include "gtest/gtest.h"
+#include <memory>
+
+using namespace llvm;
+
+extern cl::opt<bool> UseNewDbgInfoFormat;
+
+// None of these tests are meaningful or do anything if we do not have the
+// experimental "head" bit compiled into ilist_iterator (aka
+// ilist_iterator_w_bits), thus there's no point compiling these tests in.
+#ifdef EXPERIMENTAL_DEBUGINFO_ITERATORS
+
+static std::unique_ptr<Module> parseIR(LLVMContext &C, const char *IR) {
+  SMDiagnostic Err;
+  std::unique_ptr<Module> Mod = parseAssemblyString(IR, Err, C);
+  if (!Mod)
+    Err.print("BasicBlockDbgInfoTest", errs());
+  return Mod;
+}
+
+namespace {
+
+TEST(BasicBlockDbgInfoTest, MarkerOperations) {
+  LLVMContext C;
+  UseNewDbgInfoFormat = true;
+
+  std::unique_ptr<Module> M = parseIR(C, R"(
+    define i16 @f(i16 %a) !dbg !6 {
+      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+      %b = add i16 %a, 1, !dbg !11
+      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+      ret i16 0, !dbg !11
+    }
+    declare void @llvm.dbg.value(metadata, metadata, metadata) #0
+    attributes #0 = { nounwind readnone speculatable willreturn }
+
+    !llvm.dbg.cu = !{!0}
+    !llvm.module.flags = !{!5}
+
+    !0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+    !1 = !DIFile(filename: "t.ll", directory: "/")
+    !2 = !{}
+    !5 = !{i32 2, !"Debug Info Version", i32 3}
+    !6 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
+    !7 = !DISubroutineType(types: !2)
+    !8 = !{!9}
+    !9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10)
+    !10 = !DIBasicType(name: "ty16", size: 16, encoding: DW_ATE_unsigned)
+    !11 = !DILocation(line: 1, column: 1, scope: !6)
+)");
+
+  // Fetch the entry block,
+  BasicBlock &BB = M->getFunction("f")->getEntryBlock();
+  // Convert the module to "new" form debug-info.
+  M->convertToNewDbgValues();
+  EXPECT_EQ(BB.size(), 2u);
+
+  // Fetch out our two markers,
+  Instruction *Instr1 = &*BB.begin();
+  Instruction *Instr2 = Instr1->getNextNode();
+  DPMarker *Marker1 = Instr1->DbgMarker;
+  DPMarker *Marker2 = Instr2->DbgMarker;
+  // There's no TrailingDPValues marker allocated yet.
+  DPMarker *EndMarker = nullptr;
+
+  // Check that the "getMarker" utilities operate as expected.
+  EXPECT_EQ(BB.getMarker(Instr1->getIterator()), Marker1);
+  EXPECT_EQ(BB.getMarker(Instr2->getIterator()), Marker2);
+  EXPECT_EQ(BB.getNextMarker(Instr1), Marker2);
+  EXPECT_EQ(BB.getNextMarker(Instr2), EndMarker); // Is nullptr.
+
+  // There should be two DPValues,
+  EXPECT_EQ(Marker1->StoredDPValues.size(), 1u);
+  EXPECT_EQ(Marker2->StoredDPValues.size(), 1u);
+
+  // Unlink them and try to re-insert them through the basic block.
+  DPValue *DPV1 = &*Marker1->StoredDPValues.begin();
+  DPValue *DPV2 = &*Marker2->StoredDPValues.begin();
+  DPV1->removeFromParent();
+  DPV2->removeFromParent();
+  EXPECT_TRUE(Marker1->StoredDPValues.empty());
+  EXPECT_TRUE(Marker2->StoredDPValues.empty());
+
+  // This should appear in Marker1.
+  BB.insertDPValueBefore(DPV1, BB.begin());
+  EXPECT_EQ(Marker1->StoredDPValues.size(), 1u);
+  EXPECT_EQ(DPV1, &*Marker1->StoredDPValues.begin());
+
+  // This should attach to Marker2.
+  BB.insertDPValueAfter(DPV2, &*BB.begin());
+  EXPECT_EQ(Marker2->StoredDPValues.size(), 1u);
+  EXPECT_EQ(DPV2, &*Marker2->StoredDPValues.begin());
+
+  // Now, how about removing instructions? That should cause any DPValues to
+  // "fall down".
+  Instr1->removeFromParent();
+  Marker1 = nullptr;
+  // DPValues should now be in Marker2.
+  EXPECT_EQ(BB.size(), 1u);
+  EXPECT_EQ(Marker2->StoredDPValues.size(), 2u);
+  // They should also be in the correct order.
+  SmallVector<DPValue *, 2> DPVs;
+  for (DPValue &DPV : Marker2->getDbgValueRange())
+    DPVs.push_back(&DPV);
+  EXPECT_EQ(DPVs[0], DPV1);
+  EXPECT_EQ(DPVs[1], DPV2);
+
+  // If we remove the end instruction, the DPValues should fall down into
+  // the trailing marker.
+  EXPECT_EQ(BB.getTrailingDPValues(), nullptr);
+  Instr2->removeFromParent();
+  EXPECT_TRUE(BB.empty());
+  EndMarker = BB.getTrailingDPValues();;
+  ASSERT_NE(EndMarker, nullptr);
+  EXPECT_EQ(EndMarker->StoredDPValues.size(), 2u);
+  // Again, these should arrive in the correct order.
+
+  DPVs.clear();
+  for (DPValue &DPV : EndMarker->getDbgValueRange())
+    DPVs.push_back(&DPV);
+  EXPECT_EQ(DPVs[0], DPV1);
+  EXPECT_EQ(DPVs[1], DPV2);
+
+  // Inserting a normal instruction at the beginning: shouldn't dislodge the
+  // DPValues. It's intended to not go at the start.
+  Instr1->insertBefore(BB, BB.begin());
+  EXPECT_EQ(EndMarker->StoredDPValues.size(), 2u);
+  Instr1->removeFromParent();
+
+  // Inserting at end(): should dislodge the DPValues, if they were dbg.values
+  // then they would sit "above" the new instruction.
+  Instr1->insertBefore(BB, BB.end());
+  EXPECT_EQ(Instr1->DbgMarker->StoredDPValues.size(), 2u);
+  // However we won't de-allocate the trailing marker until a terminator is
+  // inserted.
+  EXPECT_EQ(EndMarker->StoredDPValues.size(), 0u);
+  EXPECT_EQ(BB.getTrailingDPValues(), EndMarker);
+
+  // Remove Instr1: now the DPValues will fall down again,
+  Instr1->removeFromParent();
+  EndMarker = BB.getTrailingDPValues();;
+  EXPECT_EQ(EndMarker->StoredDPValues.size(), 2u);
+
+  // Inserting a terminator, however it's intended, should dislodge the
+  // trailing DPValues, as it's the clear intention of the caller that this be
+  // the final instr in the block, and DPValues aren't allowed to live off the
+  // end forever.
+  Instr2->insertBefore(BB, BB.begin());
+  EXPECT_EQ(Instr2->DbgMarker->StoredDPValues.size(), 2u);
+  EXPECT_EQ(BB.getTrailingDPValues(), nullptr);
+
+  // Teardown,
+  Instr1->insertBefore(BB, BB.begin());
+
+  UseNewDbgInfoFormat = false;
+}
+
+TEST(BasicBlockDbgInfoTest, HeadBitOperations) {
+  LLVMContext C;
+  UseNewDbgInfoFormat = true;
+
+  std::unique_ptr<Module> M = parseIR(C, R"(
+    define i16 @f(i16 %a) !dbg !6 {
+      %b = add i16 %a, 1, !dbg !11
+      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+      %c = add i16 %a, 1, !dbg !11
+      %d = add i16 %a, 1, !dbg !11
+      ret i16 0, !dbg !11
+    }
+    declare void @llvm.dbg.value(metadata, metadata, metadata) #0
+    attributes #0 = { nounwind readnone speculatable willreturn }
+
+    !llvm.dbg.cu = !{!0}
+    !llvm.module.flags = !{!5}
+
+    !0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+    !1 = !DIFile(filename: "t.ll", directory: "/")
+    !2 = !{}
+    !5 = !{i32 2, !"Debug Info Version", i32 3}
+    !6 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
+    !7 = !DISubroutineType(types: !2)
+    !8 = !{!9}
+    !9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10)
+    !10 = !DIBasicType(name: "ty16", size: 16, encoding: DW_ATE_unsigned)
+    !11 = !DILocation(line: 1, column: 1, scope: !6)
+)");
+
+  // Test that the movement of debug-data when using moveBefore etc and
+  // insertBefore etc are governed by the "head" bit of iterators.
+  BasicBlock &BB = M->getFunction("f")->getEntryBlock();
+  // Convert the module to "new" form debug-info.
+  M->convertToNewDbgValues();
+
+  // Test that the head bit behaves as expected: it should be set when the
+  // code wants the _start_ of the block, but not otherwise.
+  EXPECT_TRUE(BB.getFirstInsertionPt().getHeadBit());
+  BasicBlock::iterator BeginIt = BB.begin();
+  EXPECT_TRUE(BeginIt.getHeadBit());
+  // If you launder the instruction pointer through dereferencing and then
+  // get the iterator again with getIterator, the head bit is lost. This is
+  // deliberate: if you're calling getIterator, then you're requesting an
+  // iterator for the position of _this_ instruction, not "the start of this
+  // block".
+  BasicBlock::iterator BeginIt2 = BeginIt->getIterator();
+  EXPECT_FALSE(BeginIt2.getHeadBit());
+
+  // Fetch some instruction pointers.
+  Instruction *BInst = &*BeginIt;
+  Instruction *CInst = BInst->getNextNode();
+  Instruction *DInst = CInst->getNextNode();
+  // CInst should have debug-info.
+  ASSERT_TRUE(CInst->DbgMarker);
+  EXPECT_FALSE(CInst->DbgMarker->StoredDPValues.empty());
+
+  // If we move "c" to the start of the block, just normally, then the DPValues
+  // should fall down to "d".
+  CInst->moveBefore(BB, BeginIt2);
+  EXPECT_TRUE(!CInst->DbgMarker || CInst->DbgMarker->StoredDPValues.empty());
+  ASSERT_TRUE(DInst->DbgMarker);
+  EXPECT_FALSE(DInst->DbgMarker->StoredDPValues.empty());
+
+  // Wheras if we move D to the start of the block with moveBeforePreserving,
+  // the DPValues should move with it.
+  DInst->moveBeforePreserving(BB, BB.begin());
+  EXPECT_FALSE(DInst->DbgMarker->StoredDPValues.empty());
+  EXPECT_EQ(&*BB.begin(), DInst);
+
+  // Similarly, moveAfterPreserving "D" to "C" should move DPValues with "D".
+  DInst->moveAfterPreserving(CInst);
+  EXPECT_FALSE(DInst->DbgMarker->StoredDPValues.empty());
+
+  // (move back to the start...)
+  DInst->moveBeforePreserving(BB, BB.begin());
+
+  // Current order of insts: "D -> C -> B -> Ret". DPValues on "D".
+  // If we move "C" to the beginning of the block, it should go before the
+  // DPValues. They'll stay on "D".
+  CInst->moveBefore(BB, BB.begin());
+  EXPECT_TRUE(!CInst->DbgMarker || CInst->DbgMarker->StoredDPValues.empty());
+  EXPECT_FALSE(DInst->DbgMarker->StoredDPValues.empty());
+  EXPECT_EQ(&*BB.begin(), CInst);
+  EXPECT_EQ(CInst->getNextNode(), DInst);
+
+  // Move back.
+  CInst->moveBefore(BInst);
+  EXPECT_EQ(&*BB.begin(), DInst);
+
+  // Current order of insts: "D -> C -> B -> Ret". DPValues on "D".
+  // Now move CInst to the position of DInst, but using getIterator instead of
+  // BasicBlock::begin. This signals that we want the "C" instruction to be
+  // immediately before "D", with any DPValues on "D" now moving to "C".
+  // It's the equivalent of moving an instruction to the position between a
+  // run of dbg.values and the next instruction.
+  CInst->moveBefore(BB, DInst->getIterator());
+  // CInst gains the DPValues.
+  EXPECT_TRUE(!DInst->DbgMarker || DInst->DbgMarker->StoredDPValues.empty());
+  EXPECT_FALSE(CInst->DbgMarker->StoredDPValues.empty());
+  EXPECT_EQ(&*BB.begin(), CInst);
+
+  UseNewDbgInfoFormat = false;
+}
+
+TEST(BasicBlockDbgInfoTest, InstrDbgAccess) {
+  LLVMContext C;
+  UseNewDbgInfoFormat = true;
+
+  std::unique_ptr<Module> M = parseIR(C, R"(
+    define i16 @f(i16 %a) !dbg !6 {
+      %b = add i16 %a, 1, !dbg !11
+      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+      %c = add i16 %a, 1, !dbg !11
+      %d = add i16 %a, 1, !dbg !11
+      ret i16 0, !dbg !11
+    }
+    declare void @llvm.dbg.value(metadata, metadata, metadata) #0
+    attributes #0 = { nounwind readnone speculatable willreturn }
+
+    !llvm.dbg.cu = !{!0}
+    !llvm.module.flags = !{!5}
+
+    !0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+    !1 = !DIFile(filename: "t.ll", directory: "/")
+    !2 = !{}
+    !5 = !{i32 2, !"Debug Info Version", i32 3}
+    !6 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
+    !7 = !DISubroutineType(types: !2)
+    !8 = !{!9}
+    !9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10)
+    !10 = !DIBasicType(name: "ty16", size: 16, encoding: DW_ATE_unsigned)
+    !11 = !DILocation(line: 1, column: 1, scope: !6)
+)");
+
+  // Check that DPValues can be accessed from Instructions without digging
+  // into the depths of DPMarkers.
+  BasicBlock &BB = M->getFunction("f")->getEntryBlock();
+  // Convert the module to "new" form debug-info.
+  M->convertToNewDbgValues();
+
+  Instruction *BInst = &*BB.begin();
+  Instruction *CInst = BInst->getNextNode();
+  Instruction *DInst = CInst->getNextNode();
+
+  ASSERT_TRUE(BInst->DbgMarker);
+  ASSERT_TRUE(CInst->DbgMarker);
+  ASSERT_EQ(CInst->DbgMarker->StoredDPValues.size(), 1u);
+  DPValue *DPV1 = &*CInst->DbgMarker->StoredDPValues.begin();
+  ASSERT_TRUE(DPV1);
+  EXPECT_EQ(BInst->DbgMarker->StoredDPValues.size(), 0u);
+
+  // Clone DPValues from one inst to another. Other arguments to clone are
+  // tested in DPMarker test.
+  auto Range1 = BInst->cloneDebugInfoFrom(CInst);
+  EXPECT_EQ(BInst->DbgMarker->StoredDPValues.size(), 1u);
+  DPValue *DPV2 = &*BInst->DbgMarker->StoredDPValues.begin();
+  EXPECT_EQ(std::distance(Range1.begin(), Range1.end()), 1u);
+  EXPECT_EQ(&*Range1.begin(), DPV2);
+  EXPECT_NE(DPV1, DPV2);
+
+  // We should be able to get a range over exactly the same information.
+  auto Range2 = BInst->getDbgValueRange();
+  EXPECT_EQ(Range1.begin(), Range2.begin());
+  EXPECT_EQ(Range1.end(), Range2.end());
+
+  // We should be able to query if there are DPValues,
+  EXPECT_TRUE(BInst->hasDbgValues());
+  EXPECT_TRUE(CInst->hasDbgValues());
+  EXPECT_FALSE(DInst->hasDbgValues());
+
+  // Dropping should be easy,
+  BInst->dropDbgValues();
+  EXPECT_FALSE(BInst->hasDbgValues());
+  EXPECT_EQ(BInst->DbgMarker->StoredDPValues.size(), 0u);
+
+  // And we should be able to drop individual DPValues.
+  CInst->dropOneDbgValue(DPV1);
+  EXPECT_FALSE(CInst->hasDbgValues());
+  EXPECT_EQ(CInst->DbgMarker->StoredDPValues.size(), 0u);
+
+  UseNewDbgInfoFormat = false;
+}
+
+/* Let's recall the big illustration from BasicBlock::spliceDebugInfo:
+
+                                               Dest
+                                                 |
+   this-block:    A----A----A                ====A----A----A----A---A---A
+    Src-block                ++++B---B---B---B:::C
+                                 |               |
+                                First           Last
+
+  in all it's glory. Depending on the bit-configurations for the iterator head
+  / tail bits on the three named iterators, there are eight ways for a splice to
+  occur. To save the amount of thinking needed to pack this into one unit test,
+  just test the same IR eight times with difference splices. The IR shall be
+  thus:
+
+    define i16 @f(i16 %a) !dbg !6 {
+    entry:
+      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+      %b = add i16 %a, 1, !dbg !11
+      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+      br label %exit, !dbg !11
+
+    exit:
+      call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+      %c = add i16 %b, 1, !dbg !11
+      ret i16 0, !dbg !11
+    }
+
+  The iterators will be:
+    Dest: exit block, "c" instruction.
+    First: entry block, "b" instruction.
+    Last: entry block, branch instruction.
+
+  The numbered configurations will be:
+
+       |    Dest-Head   |   First-Head   |   Last-tail
+   ----+----------------+----------------+------------
+    0  |      false     |     false      |     false
+    1  |      true      |     false      |     false
+    2  |      false     |     true       |     false
+    3  |      true      |     true       |     false
+    4  |      false     |     false      |     true
+    5  |      true      |     false      |     true
+    6  |      false     |     true       |     true
+    7  |      true      |     true       |     true
+
+  Each numbered test scenario will also have a short explanation indicating what
+  this bit configuration represents.
+*/
+
+static const std::string SpliceTestIR = R"(
+    define i16 @f(i16 %a) !dbg !6 {
+      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+      %b = add i16 %a, 1, !dbg !11
+      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+      br label %exit, !dbg !11
+
+    exit:
+      call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+      %c = add i16 %b, 1, !dbg !11
+      ret i16 0, !dbg !11
+    }
+    declare void @llvm.dbg.value(metadata, metadata, metadata) #0
+    attributes #0 = { nounwind readnone speculatable willreturn }
+
+    !llvm.dbg.cu = !{!0}
+    !llvm.module.flags = !{!5}
+
+    !0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+    !1 = !DIFile(filename: "t.ll", directory: "/")
+    !2 = !{}
+    !5 = !{i32 2, !"Debug Info Version", i32 3}
+    !6 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
+    !7 = !DISubroutineType(types: !2)
+    !8 = !{!9}
+    !9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10)
+    !10 = !DIBasicType(name: "ty16", size: 16, encoding: DW_ATE_unsigned)
+    !11 = !DILocation(line: 1, column: 1, scope: !6)
+)";
+
+class DbgSpliceTest : public ::testing::Test {
+protected:
+  LLVMContext C;
+  std::unique_ptr<Module> M;
+  BasicBlock *BBEntry, *BBExit;
+  BasicBlock::iterator Dest, First, Last;
+  Instruction *BInst, *Branch, *CInst;
+  DPValue *DPVA, *DPVB, *DPVConst;
+
+  void SetUp() override {
+    UseNewDbgInfoFormat = true;
+    M = parseIR(C, SpliceTestIR.c_str());
+    M->convertToNewDbgValues();
+
+    BBEntry = &M->getFunction("f")->getEntryBlock();
+    BBExit = BBEntry->getNextNode();
+
+    Dest = BBExit->begin();
+    First = BBEntry->begin();
+    Last = BBEntry->getTerminator()->getIterator();
+    BInst = &*First;
+    Branch = &*Last;
+    CInst = &*Dest;
+
+    DPVA = &*BInst->DbgMarker->StoredDPValues.begin();
+    DPVB = &*Branch->DbgMarker->StoredDPValues.begin();
+    DPVConst = &*CInst->DbgMarker->StoredDPValues.begin();
+  }
+
+  void TearDown() override { UseNewDbgInfoFormat = false; }
+
+  bool InstContainsDPValue(Instruction *I, DPValue *DPV) {
+    for (DPValue &D : I->getDbgValueRange()) {
+      if (&D == DPV) {
+        // Confirm too that the links between the records are correct.
+        EXPECT_EQ(DPV->Marker, I->DbgMarker);
+        EXPECT_EQ(I->DbgMarker->MarkedInstr, I);
+        return true;
+      }
+    }
+    return false;
+  }
+
+  bool CheckDPVOrder(Instruction *I, SmallVector<DPValue *> CheckVals) {
+    SmallVector<DPValue *> Vals;
+    for (DPValue &D : I->getDbgValueRange())
+      Vals.push_back(&D);
+
+    EXPECT_EQ(Vals.size(), CheckVals.size());
+    if (Vals.size() != CheckVals.size())
+      return false;
+
+    for (unsigned int I = 0; I < Vals.size(); ++I) {
+      EXPECT_EQ(Vals[I], CheckVals[I]);
+      // Provide another expectation failure to let us localise what goes wrong,
+      // by returning a flag to the caller.
+      if (Vals[I] != CheckVals[I])
+        return false;
+    }
+    return true;
+  }
+};
+
+TEST_F(DbgSpliceTest, DbgSpliceTest0) {
+  Dest.setHeadBit(false);
+  First.setHeadBit(false);
+  Last.setTailBit(false);
+
+  /*
+        define i16 @f(i16 %a) !dbg !6 {
+BBEntry entry:
+DPVA      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+First     %b = add i16 %a, 1, !dbg !11
+DPVB      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+Last      br label %exit, !dbg !11
+
+BBExit  exit:
+DPVConst  call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+Dest      %c = add i16 %b, 1, !dbg !11
+          ret i16 0, !dbg !11
+        }
+
+    Splice from First, not including leading dbg.value, to Last, including the
+    trailing dbg.value. Place at Dest, between the constant dbg.value and %c.
+    %b, and the following dbg.value, should move, to:
+
+        define i16 @f(i16 %a) !dbg !6 {
+BBEntry entry:
+DPVA      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+Last      br label %exit, !dbg !11
+
+BBExit  exit:
+DPVConst  call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+First     %b = add i16 %a, 1, !dbg !11
+DPVB      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+Dest      %c = add i16 %b, 1, !dbg !11
+          ret i16 0, !dbg !11
+        }
+
+
+  */
+  BBExit->splice(Dest, BBEntry, First, Last);
+  EXPECT_EQ(BInst->getParent(), BBExit);
+  EXPECT_EQ(CInst->getParent(), BBExit);
+  EXPECT_EQ(Branch->getParent(), BBEntry);
+
+  // DPVB: should be on Dest, in exit block.
+  EXPECT_TRUE(InstContainsDPValue(CInst, DPVB));
+
+  // DPVA, should have "fallen" onto the branch, remained in entry block.
+  EXPECT_TRUE(InstContainsDPValue(Branch, DPVA));
+
+  // DPVConst should be on the moved %b instruction.
+  EXPECT_TRUE(InstContainsDPValue(BInst, DPVConst));
+}
+
+TEST_F(DbgSpliceTest, DbgSpliceTest1) {
+  Dest.setHeadBit(true);
+  First.setHeadBit(false);
+  Last.setTailBit(false);
+
+  /*
+        define i16 @f(i16 %a) !dbg !6 {
+BBEntry entry:
+DPVA      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+First     %b = add i16 %a, 1, !dbg !11
+DPVB      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+Last      br label %exit, !dbg !11
+
+BBExit  exit:
+DPVConst  call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+Dest      %c = add i16 %b, 1, !dbg !11
+          ret i16 0, !dbg !11
+        }
+
+    Splice from First, not including leading dbg.value, to Last, including the
+    trailing dbg.value. Place at the head of Dest, i.e. at the very start of
+    BBExit, before any debug-info there. Becomes:
+
+        define i16 @f(i16 %a) !dbg !6 {
+BBEntry entry:
+DPVA      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+Last      br label %exit, !dbg !11
+
+BBExit  exit:
+First     %b = add i16 %a, 1, !dbg !11
+DPVB      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+DPVConst  call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+Dest      %c = add i16 %b, 1, !dbg !11
+          ret i16 0, !dbg !11
+        }
+
+
+  */
+  BBExit->splice(Dest, BBEntry, First, Last);
+  EXPECT_EQ(BInst->getParent(), BBExit);
+  EXPECT_EQ(CInst->getParent(), BBExit);
+  EXPECT_EQ(Branch->getParent(), BBEntry);
+
+  // DPVB: should be on CInst, in exit block.
+  EXPECT_TRUE(InstContainsDPValue(CInst, DPVB));
+
+  // DPVA, should have "fallen" onto the branch, remained in entry block.
+  EXPECT_TRUE(InstContainsDPValue(Branch, DPVA));
+
+  // DPVConst should be behind / after the moved instructions, remain on CInst.
+  EXPECT_TRUE(InstContainsDPValue(CInst, DPVConst));
+
+  // Order of DPVB and DPVConst should be thus:
+  EXPECT_TRUE(CheckDPVOrder(CInst, {DPVB, DPVConst}));
+}
+
+TEST_F(DbgSpliceTest, DbgSpliceTest2) {
+  Dest.setHeadBit(false);
+  First.setHeadBit(true);
+  Last.setTailBit(false);
+
+  /*
+        define i16 @f(i16 %a) !dbg !6 {
+BBEntry entry:
+DPVA      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+First     %b = add i16 %a, 1, !dbg !11
+DPVB      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+Last      br label %exit, !dbg !11
+
+BBExit  exit:
+DPVConst  call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+Dest      %c = add i16 %b, 1, !dbg !11
+          ret i16 0, !dbg !11
+        }
+
+    Splice from head of First, which includes the leading dbg.value, to Last,
+    including the trailing dbg.value. Place in front of Dest, but after any
+    debug-info there. Becomes:
+
+        define i16 @f(i16 %a) !dbg !6 {
+BBEntry entry:
+Last      br label %exit, !dbg !11
+
+BBExit  exit:
+DPVConst  call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+DPVA      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+First     %b = add i16 %a, 1, !dbg !11
+DPVB      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+Dest      %c = add i16 %b, 1, !dbg !11
+          ret i16 0, !dbg !11
+        }
+
+
+  */
+  BBExit->splice(Dest, BBEntry, First, Last);
+  EXPECT_EQ(BInst->getParent(), BBExit);
+  EXPECT_EQ(CInst->getParent(), BBExit);
+
+  // DPVB: should be on CInst, in exit block.
+  EXPECT_TRUE(InstContainsDPValue(CInst, DPVB));
+
+  // DPVA, should have transferred with the spliced instructions, remains on
+  // the "b" inst.
+  EXPECT_TRUE(InstContainsDPValue(BInst, DPVA));
+
+  // DPVConst should be ahead of the moved instructions, ahead of BInst.
+  EXPECT_TRUE(InstContainsDPValue(BInst, DPVConst));
+
+  // Order of DPVA and DPVConst should be thus:
+  EXPECT_TRUE(CheckDPVOrder(BInst, {DPVConst, DPVA}));
+}
+
+TEST_F(DbgSpliceTest, DbgSpliceTest3) {
+  Dest.setHeadBit(true);
+  First.setHeadBit(true);
+  Last.setTailBit(false);
+
+  /*
+        define i16 @f(i16 %a) !dbg !6 {
+BBEntry entry:
+DPVA      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+First     %b = add i16 %a, 1, !dbg !11
+DPVB      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+Last      br label %exit, !dbg !11
+
+BBExit  exit:
+DPVConst  call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+Dest      %c = add i16 %b, 1, !dbg !11
+          ret i16 0, !dbg !11
+        }
+
+    Splice from head of First, which includes the leading dbg.value, to Last,
+    including the trailing dbg.value. Place at head of Dest, before any
+    debug-info there. Becomes:
+
+        define i16 @f(i16 %a) !dbg !6 {
+BBEntry entry:
+Last      br label %exit, !dbg !11
+
+BBExit  exit:
+DPVA      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+First     %b = add i16 %a, 1, !dbg !11
+DPVB      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+DPVConst  call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+Dest      %c = add i16 %b, 1, !dbg !11
+          ret i16 0, !dbg !11
+        }
+
+  */
+  BBExit->splice(Dest, BBEntry, First, Last);
+  EXPECT_EQ(BInst->getParent(), BBExit);
+  EXPECT_EQ(CInst->getParent(), BBExit);
+
+  // DPVB: should be on CInst, in exit block.
+  EXPECT_TRUE(InstContainsDPValue(CInst, DPVB));
+
+  // DPVA, should have transferred with the spliced instructions, remains on
+  // the "b" inst.
+  EXPECT_TRUE(InstContainsDPValue(BInst, DPVA));
+
+  // DPVConst should be behind the moved instructions, ahead of CInst.
+  EXPECT_TRUE(InstContainsDPValue(CInst, DPVConst));
+
+  // Order of DPVB and DPVConst should be thus:
+  EXPECT_TRUE(CheckDPVOrder(CInst, {DPVB, DPVConst}));
+}
+
+TEST_F(DbgSpliceTest, DbgSpliceTest4) {
+  Dest.setHeadBit(false);
+  First.setHeadBit(false);
+  Last.setTailBit(true);
+
+  /*
+        define i16 @f(i16 %a) !dbg !6 {
+BBEntry entry:
+DPVA      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+First     %b = add i16 %a, 1, !dbg !11
+DPVB      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+Last      br label %exit, !dbg !11
+
+BBExit  exit:
+DPVConst  call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+Dest      %c = add i16 %b, 1, !dbg !11
+          ret i16 0, !dbg !11
+        }
+
+    Splice from First, not including the leading dbg.value, to Last, but NOT
+    including the trailing dbg.value because the tail bit is set. Place at Dest,
+    after any debug-info there. Becomes:
+
+        define i16 @f(i16 %a) !dbg !6 {
+BBEntry entry:
+DPVA      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+DPVB      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+Last      br label %exit, !dbg !11
+
+BBExit  exit:
+DPVConst  call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+First     %b = add i16 %a, 1, !dbg !11
+Dest      %c = add i16 %b, 1, !dbg !11
+          ret i16 0, !dbg !11
+        }
+
+  */
+  BBExit->splice(Dest, BBEntry, First, Last);
+  EXPECT_EQ(BInst->getParent(), BBExit);
+  EXPECT_EQ(CInst->getParent(), BBExit);
+  EXPECT_EQ(Branch->getParent(), BBEntry);
+
+  // DPVB: should be on Branch as before, remain in entry block.
+  EXPECT_TRUE(InstContainsDPValue(Branch, DPVB));
+
+  // DPVA, should have remained in entry block, falls onto Branch inst.
+  EXPECT_TRUE(InstContainsDPValue(Branch, DPVA));
+
+  // DPVConst should be ahead of the moved instructions, BInst.
+  EXPECT_TRUE(InstContainsDPValue(BInst, DPVConst));
+
+  // Order of DPVA and DPVA should be thus:
+  EXPECT_TRUE(CheckDPVOrder(Branch, {DPVA, DPVB}));
+}
+
+TEST_F(DbgSpliceTest, DbgSpliceTest5) {
+  Dest.setHeadBit(true);
+  First.setHeadBit(false);
+  Last.setTailBit(true);
+
+  /*
+        define i16 @f(i16 %a) !dbg !6 {
+BBEntry entry:
+DPVA      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+First     %b = add i16 %a, 1, !dbg !11
+DPVB      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+Last      br label %exit, !dbg !11
+
+BBExit  exit:
+DPVConst  call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+Dest      %c = add i16 %b, 1, !dbg !11
+          ret i16 0, !dbg !11
+        }
+
+    Splice from First, not including the leading dbg.value, to Last, but NOT
+    including the trailing dbg.value because the tail bit is set. Place at head
+    of Dest, before any debug-info there. Becomes:
+
+        define i16 @f(i16 %a) !dbg !6 {
+BBEntry entry:
+DPVA      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+DPVB      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+Last      br label %exit, !dbg !11
+
+BBExit  exit:
+First     %b = add i16 %a, 1, !dbg !11
+DPVConst  call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+Dest      %c = add i16 %b, 1, !dbg !11
+          ret i16 0, !dbg !11
+        }
+
+  */
+  BBExit->splice(Dest, BBEntry, First, Last);
+  EXPECT_EQ(BInst->getParent(), BBExit);
+  EXPECT_EQ(CInst->getParent(), BBExit);
+  EXPECT_EQ(Branch->getParent(), BBEntry);
+
+  // DPVB: should be on Branch as before, remain in entry block.
+  EXPECT_TRUE(InstContainsDPValue(Branch, DPVB));
+
+  // DPVA, should have remained in entry block, falls onto Branch inst.
+  EXPECT_TRUE(InstContainsDPValue(Branch, DPVA));
+
+  // DPVConst should be behind of the moved instructions, on CInst.
+  EXPECT_TRUE(InstContainsDPValue(CInst, DPVConst));
+
+  // Order of DPVA and DPVB should be thus:
+  EXPECT_TRUE(CheckDPVOrder(Branch, {DPVA, DPVB}));
+}
+
+TEST_F(DbgSpliceTest, DbgSpliceTest6) {
+  Dest.setHeadBit(false);
+  First.setHeadBit(true);
+  Last.setTailBit(true);
+
+  /*
+        define i16 @f(i16 %a) !dbg !6 {
+BBEntry entry:
+DPVA      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+First     %b = add i16 %a, 1, !dbg !11
+DPVB      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+Last      br label %exit, !dbg !11
+
+BBExit  exit:
+DPVConst  call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+Dest      %c = add i16 %b, 1, !dbg !11
+          ret i16 0, !dbg !11
+        }
+
+    Splice from First, including the leading dbg.value, to Last, but NOT
+    including the trailing dbg.value because the tail bit is set. Place at Dest,
+    after any debug-info there. Becomes:
+
+        define i16 @f(i16 %a) !dbg !6 {
+BBEntry entry:
+DPVB      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+Last      br label %exit, !dbg !11
+
+BBExit  exit:
+DPVConst  call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+DPVA      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+First     %b = add i16 %a, 1, !dbg !11
+Dest      %c = add i16 %b, 1, !dbg !11
+          ret i16 0, !dbg !11
+        }
+
+  */
+  BBExit->splice(Dest, BBEntry, First, Last);
+  EXPECT_EQ(BInst->getParent(), BBExit);
+  EXPECT_EQ(CInst->getParent(), BBExit);
+  EXPECT_EQ(Branch->getParent(), BBEntry);
+
+  // DPVB: should be on Branch as before, remain in entry block.
+  EXPECT_TRUE(InstContainsDPValue(Branch, DPVB));
+
+  // DPVA, should have transferred to BBExit, on B inst.
+  EXPECT_TRUE(InstContainsDPValue(BInst, DPVA));
+
+  // DPVConst should be ahead of the moved instructions, on BInst.
+  EXPECT_TRUE(InstContainsDPValue(BInst, DPVConst));
+
+  // Order of DPVA and DPVConst should be thus:
+  EXPECT_TRUE(CheckDPVOrder(BInst, {DPVConst, DPVA}));
+}
+
+TEST_F(DbgSpliceTest, DbgSpliceTest7) {
+  Dest.setHeadBit(true);
+  First.setHeadBit(true);
+  Last.setTailBit(true);
+
+  /*
+        define i16 @f(i16 %a) !dbg !6 {
+BBEntry entry:
+DPVA      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+First     %b = add i16 %a, 1, !dbg !11
+DPVB      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+Last      br label %exit, !dbg !11
+
+BBExit  exit:
+DPVConst  call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+Dest      %c = add i16 %b, 1, !dbg !11
+          ret i16 0, !dbg !11
+        }
+
+    Splice from First, including the leading dbg.value, to Last, but NOT
+    including the trailing dbg.value because the tail bit is set. Place at head
+    of Dest, before any debug-info there. Becomes:
+
+        define i16 @f(i16 %a) !dbg !6 {
+BBEntry entry:
+DPVB      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+Last      br label %exit, !dbg !11
+
+BBExit  exit:
+DPVA      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+First     %b = add i16 %a, 1, !dbg !11
+DPVConst  call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+Dest      %c = add i16 %b, 1, !dbg !11
+          ret i16 0, !dbg !11
+        }
+
+  */
+  BBExit->splice(Dest, BBEntry, First, Last);
+  EXPECT_EQ(BInst->getParent(), BBExit);
+  EXPECT_EQ(CInst->getParent(), BBExit);
+  EXPECT_EQ(Branch->getParent(), BBEntry);
+
+  // DPVB: should be on Branch as before, remain in entry block.
+  EXPECT_TRUE(InstContainsDPValue(Branch, DPVB));
+
+  // DPVA, should have transferred to BBExit, on B inst.
+  EXPECT_TRUE(InstContainsDPValue(BInst, DPVA));
+
+  // DPVConst should be after of the moved instructions, on CInst.
+  EXPECT_TRUE(InstContainsDPValue(CInst, DPVConst));
+}
+
+// But wait, there's more! What if you splice a range that is empty, but
+// implicitly contains debug-info? In the dbg.value design for debug-info,
+// this would be an explicit range, but in DPValue debug-info, it isn't.
+// Check that if we try to do that, with differing head-bit values, that
+// DPValues are transferred.
+// Test with empty transfers to Dest, with head bit set and not set.
+
+TEST_F(DbgSpliceTest, DbgSpliceEmpty0) {
+  Dest.setHeadBit(false);
+  First.setHeadBit(false);
+  Last.setHeadBit(false);
+  /*
+        define i16 @f(i16 %a) !dbg !6 {
+BBEntry entry:
+DPVA      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+First     %b = add i16 %a, 1, !dbg !11
+DPVB      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+Last      br label %exit, !dbg !11
+
+BBExit  exit:
+DPVConst  call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+Dest      %c = add i16 %b, 1, !dbg !11
+          ret i16 0, !dbg !11
+        }
+
+    Splice from BBEntry.getFirstInsertionPt to First -- this implicitly is a
+    splice of DPVA, but the iterators are pointing at the same instruction. The
+    only difference is the setting of the head bit. Becomes;
+
+        define i16 @f(i16 %a) !dbg !6 {
+First     %b = add i16 %a, 1, !dbg !11
+DPVB      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+Last      br label %exit, !dbg !11
+
+BBExit  exit:
+DPVConst  call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+DPVA      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+Dest      %c = add i16 %b, 1, !dbg !11
+          ret i16 0, !dbg !11
+        }
+
+  */
+  BBExit->splice(Dest, BBEntry, BBEntry->getFirstInsertionPt(), First);
+  EXPECT_EQ(BInst->getParent(), BBEntry);
+  EXPECT_EQ(CInst->getParent(), BBExit);
+  EXPECT_EQ(Branch->getParent(), BBEntry);
+
+  // DPVB: should be on Branch as before, remain in entry block.
+  EXPECT_TRUE(InstContainsDPValue(Branch, DPVB));
+
+  // DPVA, should have transferred to BBExit, on C inst.
+  EXPECT_TRUE(InstContainsDPValue(CInst, DPVA));
+
+  // DPVConst should be ahead of the moved DPValue, on CInst.
+  EXPECT_TRUE(InstContainsDPValue(CInst, DPVConst));
+
+  // Order of DPVA and DPVConst should be thus:
+  EXPECT_TRUE(CheckDPVOrder(CInst, {DPVConst, DPVA}));
+}
+
+TEST_F(DbgSpliceTest, DbgSpliceEmpty1) {
+  Dest.setHeadBit(true);
+  First.setHeadBit(false);
+  Last.setHeadBit(false);
+  /*
+        define i16 @f(i16 %a) !dbg !6 {
+BBEntry entry:
+DPVA      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+First     %b = add i16 %a, 1, !dbg !11
+DPVB      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+Last      br label %exit, !dbg !11
+
+BBExit  exit:
+DPVConst  call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+Dest      %c = add i16 %b, 1, !dbg !11
+          ret i16 0, !dbg !11
+        }
+
+    Splice from BBEntry.getFirstInsertionPt to First -- this implicitly is a
+    splice of DPVA, but the iterators are pointing at the same instruction. The
+    only difference is the setting of the head bit. Insert at head of Dest,
+    i.e. before DPVConst. Becomes;
+
+        define i16 @f(i16 %a) !dbg !6 {
+First     %b = add i16 %a, 1, !dbg !11
+DPVB      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+Last      br label %exit, !dbg !11
+
+BBExit  exit:
+DPVA      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+DPVConst  call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+Dest      %c = add i16 %b, 1, !dbg !11
+          ret i16 0, !dbg !11
+        }
+
+  */
+  BBExit->splice(Dest, BBEntry, BBEntry->getFirstInsertionPt(), First);
+  EXPECT_EQ(BInst->getParent(), BBEntry);
+  EXPECT_EQ(CInst->getParent(), BBExit);
+  EXPECT_EQ(Branch->getParent(), BBEntry);
+
+  // DPVB: should be on Branch as before, remain in entry block.
+  EXPECT_TRUE(InstContainsDPValue(Branch, DPVB));
+
+  // DPVA, should have transferred to BBExit, on C inst.
+  EXPECT_TRUE(InstContainsDPValue(CInst, DPVA));
+
+  // DPVConst should be ahead of the moved DPValue, on CInst.
+  EXPECT_TRUE(InstContainsDPValue(CInst, DPVConst));
+
+  // Order of DPVA and DPVConst should be thus:
+  EXPECT_TRUE(CheckDPVOrder(CInst, {DPVA, DPVConst}));
+}
+
+// If we splice new instructions into a block with trailing DPValues, then
+// the trailing DPValues should get flushed back out.
+TEST(BasicBlockDbgInfoTest, DbgSpliceTrailing) {
+  LLVMContext C;
+  UseNewDbgInfoFormat = true;
+
+  std::unique_ptr<Module> M = parseIR(C, R"(
+    define i16 @f(i16 %a) !dbg !6 {
+    entry:
+      call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+      br label %exit
+
+    exit:
+      %b = add i16 %a, 1, !dbg !11
+      ret i16 0, !dbg !11
+    }
+    declare void @llvm.dbg.value(metadata, metadata, metadata) #0
+    attributes #0 = { nounwind readnone speculatable willreturn }
+
+    !llvm.dbg.cu = !{!0}
+    !llvm.module.flags = !{!5}
+
+    !0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+    !1 = !DIFile(filename: "t.ll", directory: "/")
+    !2 = !{}
+    !5 = !{i32 2, !"Debug Info Version", i32 3}
+    !6 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
+    !7 = !DISubroutineType(types: !2)
+    !8 = !{!9}
+    !9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10)
+    !10 = !DIBasicType(name: "ty16", size: 16, encoding: DW_ATE_unsigned)
+    !11 = !DILocation(line: 1, column: 1, scope: !6)
+)");
+
+  BasicBlock &Entry = M->getFunction("f")->getEntryBlock();
+  BasicBlock &Exit = *Entry.getNextNode();
+  M->convertToNewDbgValues();
+
+  // Begin by forcing entry block to have dangling DPValue.
+  Entry.getTerminator()->eraseFromParent();
+  ASSERT_NE(Entry.getTrailingDPValues(), nullptr);
+  EXPECT_TRUE(Entry.empty());
+
+  // Now transfer the entire contents of the exit block into the entry.
+  Entry.splice(Entry.end(), &Exit, Exit.begin(), Exit.end());
+
+  // The trailing DPValue should have been placed at the front of what's been
+  // spliced in.
+  Instruction *BInst = &*Entry.begin();
+  ASSERT_TRUE(BInst->DbgMarker);
+  EXPECT_EQ(BInst->DbgMarker->StoredDPValues.size(), 1u);
+
+  UseNewDbgInfoFormat = false;
+}
+
+} // End anonymous namespace.
+#endif // EXPERIMENTAL_DEBUGINFO_ITERATORS

llvm/unittests/IR/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Analysis
   AsmParser
   Core
   Support
   Passes
   TargetParser
   TransformUtils
   ScalarOpts
   )
 
 add_llvm_unittest(IRTests
   AbstractCallSiteTest.cpp
   AsmWriterTest.cpp
   AttributesTest.cpp
   BasicBlockTest.cpp
+  BasicBlockDbgInfoTest.cpp
   CFGBuilder.cpp
   ConstantRangeTest.cpp
   ConstantsTest.cpp
   DataLayoutTest.cpp
   DebugInfoTest.cpp
   DebugTypeODRUniquingTest.cpp
   DemandedBitsTest.cpp
   DominatorTreeTest.cpp