[DebugInfo] Don't repeatedly created undef DBG_VALUEs during machine-sinking

[jmorse]

Summary

This patch addresses a performance regression reported in PR43855 [0] that appeared as a result of D58386 / rGf5e1b718a6. To recap: the MachineSink pass used to rely on DBG_VALUE insts always following the instruction that defines the corresponding VReg. D58386 collected all debug users as we walked through a block so that non-adjacent DBG_VALUEs could be sunk too.

It turns out that MachineSink will (often) move instructions to the first block that post-dominates the current block, and then try to sink further. This means if we have a lot of conditionals:

foo = bar + baz;
if (a)
  b++;
if (c)          // <---- here
  d++;
someuseof(foo);

When the computation of "foo" sinks to the bottom of the code, an extra spurious DBG_VALUE $noreg would be placed at the marker. Multiply this a few times, and in [0] there are some functions where an extra 25% DBG_VALUEs appear, all $noreg. This seemingly produced some pathological behaviour elsewhere.

To fix this, rather than immediately sinking DBG_VALUEs, they get recorded into a pass structure. When sinking is complete and instructions won't be sunk any further, new DBG_VALUEs are added, avoiding lots of intermediate DBG_VALUE $noregs being created.

[0] https://bugs.llvm.org/show_bug.cgi?id=43855

[aprantl]

llvm/lib/CodeGen/MachineSink.cpp

127

SmallDenseMap here, maybe?

[bjope]

llvm/lib/CodeGen/MachineSink.cpp

381

My only comment is that you might consider breaking out the new code added here to a function/method before committing.

For example

void MachineSinking::reinsertSunkDebugDefs(MachineFunction &MF) {
  ...
}

(always nice to keep the runOnMachineFunction relatively small, outlining the overall steps performed in the pass)

[jmorse]

llvm/lib/CodeGen/MachineSink.cpp

381

Shoehorned that in, cheers!

[uabelho]

Hi,

I've started seeing a verifier complaint with this patch.
I don't have a full testcase to share since I get this on our out-of-tree
target but maybe you can make somethng out of this anyway.

In the input to MachineSink we have this in one MBB

%5:an32pairs = mv [...]
%6:an32_rn_pairs = COPY %5.hiAcc:an32pairs
%8:an32_0_7 = COPY %6:an32_rn_pairs
DBG_VALUE %8:an32_0_7 [...]

and then in the output we get this, sinked into another MBB

%6:an32_rn_pairs = COPY %5.hiAcc:an32pairs
%8:an32_0_7 = COPY %6:an32_rn_pairs
DBG_VALUE %8.hiAcc:an32_0_7 [...]

Then the verifier complains that there is no sub register hiAcc in the an32_0_7
register class, i.e. the "hiAcc" in the

DBG_VALUE %8.hiAcc:an32_0_7

shouldn't be there.

[jmorse]

Ouch, yeah, I can see exactly why that is: the sunk DBG_VALUE is copied from the old / "original" DBG_VALUE, which in the meantime has been copy-propagated. And through the copy propagation, it grows an extra piece of subregister information.

This should be straightforwards to fix, I'll have a patch up shortly, thanks for the report!

[uabelho]

In D70676#1775047, @jmorse wrote:

Ouch, yeah, I can see exactly why that is: the sunk DBG_VALUE is copied from the old / "original" DBG_VALUE, which in the meantime has been copy-propagated. And through the copy propagation, it grows an extra piece of subregister information.

This should be straightforwards to fix, I'll have a patch up shortly, thanks for the report!

Sounds good. Thanks!

[jmorse]

Misery, not today, but I still think there should be an easy fix. Please do revert this patch though if it's causing difficulties.

[jmorse]

(D71279 is a precursor, just getting the second patch up, but I continue to be stuck in the tar pit...)

[jmorse]

D71283 Should (TM) fix it, but, the x86 test that I wrote and which seemed to replicate the issue, now no longer replicates the issue. Which is unfortunate.

Would you be able to give that patch (and its parent) a test to see whether it fixes the problem you're encountering? Storing the subregister separately should entirely fix this I believe, I'm just having problems replicating it on x86.

[hans]

We're seeing non-determinism in Chromium builds after this commit. You can reproduce using the pre-processed source from https://bugs.chromium.org/p/chromium/issues/detail?id=1032241#c32 and running this (giant, sorry) command:

$ for x in {1..25} ; do build.release/bin/clang -cc1 -triple x86_64-unknown-linux-gnu -emit-obj -disable-llvm-verifier -discard-value-names -mrelocation-model pic -pic-level 2 -mthread-model posix -fmerge-all-constants -mframe-pointer=all -relaxed-aliasing -fmath-errno -masm-verbose -mconstructor-aliases -munwind-tables -fuse-init-array -target-cpu x86-64 -target-feature +sse2 -dwarf-column-info -debug-info-kind=limited -dwarf-version=4 -debugger-tuning=gdb -O2 -std=c++14 -fno-operator-names -ftrivial-auto-var-init=pattern -fno-rtti -fgnuc-version=4.2.1 -Qn -vectorize-loops -vectorize-slp -fcomplete-member-pointers /tmp/a.ii -o /tmp/foo_$x.o & done && wait && sha1sum /tmp/foo_*.o

This should print 25 identical sha1 hashes for the 25 object files, but some of them have different hashes.

I believe this might be due to iteration over one of the DenseMaps, which is in non-deterministic order.

[hans]

I've reverted in 49da20ddb43 until it can be fixed. Please let me know if you need help with the reproducer.

[uabelho]

In D70676#1777672, @jmorse wrote:

D71283 Should (TM) fix it, but, the x86 test that I wrote and which seemed to replicate the issue, now no longer replicates the issue. Which is unfortunate.

Would you be able to give that patch (and its parent) a test to see whether it fixes the problem you're encountering? Storing the subregister separately should entirely fix this I believe, I'm just having problems replicating it on x86.

I tried out D71279 and D71283 individually and they both seem to fix the problem I saw.
I haven't done very wide testing, but the mir-reproducer I had passes. Nice!

[jmorse]

I tried out D71279 and D71283 individually and they both seem to fix the problem I saw. I haven't done very wide testing, but the mir-reproducer I had passes. Nice!

Cool; I think the former might mask the latter, I'll land the first patch when this (D70676) is back in tree...

Hans wrote:

We're seeing non-determinism in Chromium builds after this commit

Thanks for the detailed reproducer (and strong shell-fu); I've replicated this locally now, cheers. Fascinatingly, the output difference seems to be a change in the layout of location lists, not their meaning. From llvm-dwarfdump -debug-loc:

             [0x0000000000000135, 0x000000000000014b): DW_OP_breg14 R14+400, DW_OP_stack_value

 0x00014038:
+            [0x0000000000000152, 0x0000000000000171): DW_OP_reg3 RBX
+
+0x0001406b:
             [0x0000000000000152, 0x00000000000001b1): DW_OP_reg3 RBX
             [0x0000000000000281, 0x00000000000002a0): DW_OP_reg3 RBX

-0x0001407e:
+0x000140b1:
             [0x0000000000000152, 0x00000000000001b1): DW_OP_reg3 RBX
             [0x0000000000000281, 0x00000000000002a0): DW_OP_reg3 RBX

-0x000140c4:
-            [0x0000000000000152, 0x0000000000000171): DW_OP_reg3 RBX
-
 0x000140f7:
             [0x0000000000000163, 0x0000000000000171): DW_OP_reg3 RBX

That's a location entry for RBX shifting down two location list entries. It's not clear to me why this is happening; I'll dig further.

[jmorse]

Here's a revised patch using SetVector, although it's not at the location I was expecting. As it turns out, the llvm::sort of DebugVariables I was using was actively harmful -- the operator< method of DebugVariable compares pointers, which is fine for DenseMaps, but not for ensuring a consistent order.

It took a quick look at implementing a correct operator< for DebugVariable, but it looks like metadata in general doesn't support ordering.

Without objection I'll drop this in in a couple of days, as the differences are minor. Thanks @hans for the report!

[aprantl]

Without objection I'll drop this in in a couple of days, as the differences are minor. Thanks @hans for the report!

Could you please clarify? Does this patch fix the non-determinism in the output? Non-deterministic output is something we strive to avoid even in the debug info.

[bjope]

Don't fully understand. This revision is closed, but latest diff is showing a new diff (not the one closed/reverted, right?). If I understand it correctly revision should be re-opened for review again, right?

[bjope]

LGTM!
(if removing the unused VarInstPair/InstsToSink)

llvm/lib/CodeGen/MachineSink.cpp

345

VarInstPair and InstsToSink are no longer used afaict.

[jmorse]

Remove some now needless variables, switch another DenseMap to MapVector. Having fluffed the explanation before, I'll reopen this momentarily with a fuller explanation,

[jmorse]

Too lazy on the earlier explanation there sorry. To elaborate: I thought I'd accounted for nondeterminism in the two loops in reinsertSunkDebugDefs, but both were a bit broken:

For the outer loop, because we're only ever placing DBG_VALUEs after pre-determined instructions like this:

inst1
<--- insert DBG_VALUE here
inst2
<--- insert DBG_VALUE here

I'd assumed that it didn't matter what order that happens in. However, on second thoughts if an instruction defines more than one vreg, groups of DBG_VALUEs associated with different vregs from an instruction could change order.

For the inner loop, I was moving elements into a vector and then sorting them, based on the "DebugVariable" classes operator<. However, it turns out that compares pointers, and more generally it seems metadata doesn't support being ordered.

For both of these problems I've now switched to MapVector and SetVector respectively; the insertion order is defined by MachineSinking iterating over basic blocks (which is stable AFAIUI), and MachineSinking::ProcessBlock walking through the block backwards, which is also stable.

In the output from hans' reproducer, packs of DBG_VALUEs were being re-ordered, but in a way that didn't change the meaning of any variable location. I don't know why that led to changes in the layout of location lists (again, the meaning was the same), but either way the re-ordering is bad.

[aprantl]

llvm/lib/CodeGen/MachineSink.cpp

132

Either

struct SunkDebugDef {
     /// Set of DBG_VALUEs to recreate.
     SinkingVarSet InstsToSink; 
   /// Location to place DBG_VALUEs.
     MachineInstr *MI;          
   };

or

struct SunkDebugDef {
     SinkingVarSet InstsToSink; ///< Set of DBG_VALUEs to recreate.
     MachineInstr *MI;          ///< Location to place DBG_VALUEs.
   };

202

I find the nomenclature very confusing.

In line 137 the comment makes it sound as if

%foo = some_insn
DBG_VALUE %foo, "x"
DBG_VALUE %foo, "y"

%foo is a DebugDef and the two DB_VALUEs are insts, but here it sounds more like the DBG_VALUEs are DebugDefs ... can this be unified?

[jmorse]

Address review comments; will drop an explanation inline.

[jmorse]

llvm/lib/CodeGen/MachineSink.cpp

132

Picked the former,

202

Yeah, that seems to have been bogus. I've switched the data structures to be "SunkVRegDef"s, which should be closer to the truth, it is the instruction that defines the vreg which is sinking. I've rewritten the comments around the data structures, and renamed this function: it's inserting DBG_VALUEs, and it's the VReg definitions that have sunk.

This means the data structures doesn't have "Debug" in their names, but it should be clear from the comments that they're purely for tracking debugging information.

[aprantl]

llvm/lib/CodeGen/MachineSink.cpp

1050

"Debug uses with no other DBG_VALUE for this variable in between can be sunk"?

[jmorse]

(Clearing out old revisions) -- it's been so long that this is unlikely to land. This was also written at the point where I was realising that to handle DBG_VALUE movement in machinesink, we basically need to re-compute SSA form each time.

llvm/lib/CodeGen/MachineSink.cpp

 // they aren't executed on paths where their results aren't needed.
 //
 // This pass is not intended to be a replacement or a complete alternative
 // for an LLVM-IR-level sinking pass. It is only designed to sink simple
 // constructs that are not exposed before lowering and instruction selection.
 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/SparseBitVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Analysis/AliasAnalysis.h"
class MachineSinking : public MachineFunctionPass {
     /// Record of DBG_VALUE uses of vregs in a block, so that we can identify
     /// debug instructions to sink.
     DenseMap<unsigned, TinyPtrVector<SeenDbgUser>> SeenDbgUsers;
 
     /// Record of debug variables that have had their locations set in the
     /// current block.
     DenseSet<DebugVariable> SeenDbgVars;
 
+    /// A set of DBG_VALUEs, indexed by their DebugVariable. Refers to the
+    /// original / non-sunk DBG_VALUE, which should be cloned to the final
+    /// sunk location.
+    using SinkingVarSet = DenseMap<DebugVariable, MachineInstr *>;
+
+    /// Record of variable locations to re-create after the sinking of a
+    /// vreg definition completes.
+    struct SunkDebugDef {
+      SinkingVarSet InstsToSink; /// Set of DBG_VALUEs to recreate.
+      MachineInstr *MI;          /// Location to place DBG_VALUEs.
+    };
+
+    /// Map sunk vreg to the final destination of the vreg def, and the
+    /// DBG_VALUEs that were attached to it. We need only create one new
+    /// DBG_VALUE even if the defining instruction sinks through multiple
+    /// blocks.
+    DenseMap<unsigned, SunkDebugDef> SunkDebugDefs;
+
   public:
     static char ID; // Pass identification
 
     MachineSinking() : MachineFunctionPass(ID) {
       initializeMachineSinkingPass(*PassRegistry::getPassRegistry());
     }
 
     bool runOnMachineFunction(MachineFunction &MF) override;
private:
     /// longer be dominated by the COPY, and will eventually be dropped.
     /// This is easily rectified by forwarding the non-dominated debug uses
     /// to the copy source.
     void SalvageUnsunkDebugUsersOfCopy(MachineInstr &,
                                        MachineBasicBlock *TargetBlock);
     bool AllUsesDominatedByBlock(unsigned Reg, MachineBasicBlock *MBB,
                                  MachineBasicBlock *DefMBB,
                                  bool &BreakPHIEdge, bool &LocalUse) const;
     MachineBasicBlock *FindSuccToSinkTo(MachineInstr &MI, MachineBasicBlock *MBB,
                bool &BreakPHIEdge, AllSuccsCache &AllSuccessors);
     bool isProfitableToSinkTo(unsigned Reg, MachineInstr &MI,
                               MachineBasicBlock *MBB,
                               MachineBasicBlock *SuccToSinkTo,
                               AllSuccsCache &AllSuccessors);
 
     bool PerformTrivialForwardCoalescing(MachineInstr &MI,
                                          MachineBasicBlock *MBB);
bool MachineSinking::runOnMachineFunction(MachineFunction &MF) {
   TII = MF.getSubtarget().getInstrInfo();
   TRI = MF.getSubtarget().getRegisterInfo();
   MRI = &MF.getRegInfo();
   DT = &getAnalysis<MachineDominatorTree>();
   PDT = &getAnalysis<MachinePostDominatorTree>();
   LI = &getAnalysis<MachineLoopInfo>();
   MBFI = UseBlockFreqInfo ? &getAnalysis<MachineBlockFrequencyInfo>() : nullptr;
   MBPI = &getAnalysis<MachineBranchProbabilityInfo>();
   AA = &getAnalysis<AAResultsWrapperPass>().getAAResults();
 
   bool EverMadeChange = false;
 
   while (true) {
     bool MadeChange = false;
 
     // Process all basic blocks.
     CEBCandidates.clear();
while (true) {
     EverMadeChange = true;
   }
 
   // Now clear any kill flags for recorded registers.
   for (auto I : RegsToClearKillFlags)
     MRI->clearKillFlags(I);
   RegsToClearKillFlags.clear();
 
+  // Re-insert any DBG_VALUEs sunk
+  for (auto &Iterator : SunkDebugDefs) {
+    SunkDebugDef Def = Iterator.second;
+    unsigned VReg = Iterator.first;
+
+    MachineBasicBlock::iterator DestLoc = Def.MI->getIterator();
+    MachineBasicBlock *MBB = Def.MI->getParent();
+
+    // Place DBG_VALUEs immediately after the sunk instruction.
+    ++DestLoc;
+
+    // Collect the DBG_VALUEs being sunk and put them in a deterministic order.
+    using VarInstPair = std::pair<DebugVariable, MachineInstr *>;
+    SmallVector<VarInstPair, 16> InstsToSink;
+    for (auto &SunkLoc : Def.InstsToSink)
+      InstsToSink.push_back(SunkLoc);
+    llvm::sort(InstsToSink);
+
+    for (auto &SunkLoc : InstsToSink) {
+      MachineInstr *NewDbgMI = MF.CloneMachineInstr(SunkLoc.second);
+      NewDbgMI->getOperand(0).setReg(VReg);
+      MBB->insert(DestLoc, NewDbgMI);
+    }
+  }
+
+  SunkDebugDefs.clear();
+
   return EverMadeChange;
 }
 
 bool MachineSinking::ProcessBlock(MachineBasicBlock &MBB) {
   // Can't sink anything out of a block that has less than two successors.
   if (MBB.succ_size() <= 1 || MBB.empty()) return false;
 
   // Don't bother sinking code out of unreachable blocks. In addition to being
bool MachineSinking::SinkInstruction(MachineInstr &MI, bool &SawStore,
   }
 
   // Determine where to insert into. Skip phi nodes.
   MachineBasicBlock::iterator InsertPos = SuccToSinkTo->begin();
   while (InsertPos != SuccToSinkTo->end() && InsertPos->isPHI())
     ++InsertPos;
 
   // Collect debug users of any vreg that this inst defines.
-  SmallVector<MachineInstr *, 4> DbgUsersToSink;
   for (auto &MO : MI.operands()) {
     if (!MO.isReg() || !MO.isDef() || !MO.getReg().isVirtual())
       continue;
+
+    // If no DBG_VALUE uses this reg, skip further analysis.
     if (!SeenDbgUsers.count(MO.getReg()))
       continue;
 
-    // Sink any users that don't pass any other DBG_VALUEs for this variable.
+    SunkDebugDef &SunkDef = SunkDebugDefs[MO.getReg()];
+    SunkDef.MI = &MI;
+
+    // Record that these DBG_VALUEs refer to this sinking vreg, so that they
+    // can be re-specified after sinking completes. Try copy-propagating
+    // the original location too.
     auto &Users = SeenDbgUsers[MO.getReg()];
     for (auto &User : Users) {
       MachineInstr *DbgMI = User.getPointer();
-      if (User.getInt()) {
-        // This DBG_VALUE would re-order assignments. If we can't copy-propagate
-        // it, it can't be recovered. Set it undef.
-        if (!attemptDebugCopyProp(MI, *DbgMI))
-          DbgMI->getOperand(0).setReg(0);
-      } else {
-        DbgUsersToSink.push_back(DbgMI);
-      }
+      DebugVariable Var(DbgMI->getDebugVariable(), DbgMI->getDebugExpression(),
+                        DbgMI->getDebugLoc()->getInlinedAt());
+      // If we can't copy-propagate the original DBG_VALUE, mark it undef, as
+      // its operand will not be available.
+
+      // Debug users that don't pass any other DBG_VALUEs for this variable
+      // can be sunk.
+      if (!User.getInt())
+        SunkDef.InstsToSink.insert({Var, DbgMI});
     }
   }
 
   // After sinking, some debug users may not be dominated any more. If possible,
   // copy-propagate their operands. As it's expensive, don't do this if there's
   // no debuginfo in the program.
   if (MI.getMF()->getFunction().getSubprogram() && MI.isCopy())
     SalvageUnsunkDebugUsersOfCopy(MI, SuccToSinkTo);
 
+  SmallVector<MachineInstr *, 4> DbgUsersToSink; // Deliberately empty
   performSink(MI, *SuccToSinkTo, InsertPos, DbgUsersToSink);
 
   // Conservatively, clear any kill flags, since it's possible that they are no
   // longer correct.
   // Note that we have to clear the kill flags for any register this instruction
   // uses as we may sink over another instruction which currently kills the
   // used registers.
   for (MachineOperand &MO : MI.operands()) {

llvm/test/DebugInfo/MIR/X86/machinesink.mir

 # RUN: llc -mtriple=x86_64-unknown-unknown -run-pass=machine-sink -o - %s | FileCheck %s
 # In the first test, check various things when we sink machine instructions:
 #   a) DBG_VALUEs should sink with defs
 #   b) Undefs should be left behind
 #   c) DBG_VALUEs not immediately following the defining inst should sink too
 #   d) If we generate debug-use-before-defs through sinking, and can copy
 #      propagate to a different value, we should do that.
 #
 # Test two checks DBG_VALUEs do not sink past a DBG_VALUE in the same block
 # that redefines the variable location, while three checks that copy-propagation
 # happens in the same scenario.
+#
+# Test four: if we sink a DBG_VALUE via an intermediate block, no spurious
+# DBG_VALUE $noreg's should appear along the way.
 --- |
   target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
   target triple = "x86_64-unknown-linux-gnu"
 
   @x = common local_unnamed_addr global i32 0, align 4, !dbg !0
 
   ; Function Attrs: noreturn nounwind uwtable
   define void @Process(i32* nocapture readonly %p) local_unnamed_addr !dbg !9 {
--- |
   entry:
     br label %block1
   block1:
     br label %exit
   exit:
     ret void
   }
 
+  define void @test4(i32* nocapture readonly %p) local_unnamed_addr !dbg !301 {
+  ; Stripped
+  entry:
+    br label %block1
+  block1:
+    br label %block2
+  block2:
+    br label %exit
+  combined:
+    br label %branch1
+  branch1:
+    br label %exit
+  exit:
+    ret void
+  }
+
   ; Function Attrs: nounwind readnone
   declare void @llvm.dbg.value(metadata, i64, metadata, metadata)
 
   !llvm.dbg.cu = !{!1}
   !llvm.module.flags = !{!6, !7}
   !llvm.ident = !{!8}
 
   !0 = !DIGlobalVariableExpression(var: !DIGlobalVariable(name: "x", scope: !1, file: !2, line: 1, type: !5, isLocal: false, isDefinition: true), expr: !DIExpression())
--- |
   !101 = distinct !DISubprogram(name: "test2", scope: !2, file: !2, line: 2, type: !10, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: true, unit: !1, retainedNodes: !102)
   !102 = !{!103}
   !103 = !DILocalVariable(name: "q", arg: 1, scope: !101, file: !2, line: 2, type: !12)
   !104 = !DILocation(line: 1, column: 1, scope: !101)
   !201 = distinct !DISubprogram(name: "test2", scope: !2, file: !2, line: 2, type: !10, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: true, unit: !1, retainedNodes: !202)
   !202 = !{!203}
   !203 = !DILocalVariable(name: "r", arg: 1, scope: !201, file: !2, line: 2, type: !12)
   !204 = !DILocation(line: 1, column: 1, scope: !201)
+  !301 = distinct !DISubprogram(name: "test2", scope: !2, file: !2, line: 2, type: !10, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: true, unit: !1, retainedNodes: !302)
+  !302 = !{!303}
+  !303 = !DILocalVariable(name: "s", arg: 1, scope: !301, file: !2, line: 2, type: !12)
+  !304 = !DILocation(line: 1, column: 1, scope: !301)
 
   ; CHECK: [[VARNUM:![0-9]+]] = !DILocalVariable(name: "p",
   ; CHECK: [[VAR2NUM:![0-9]+]] = !DILocalVariable(name: "q",
   ; CHECK: [[VAR3NUM:![0-9]+]] = !DILocalVariable(name: "r",
+  ; CHECK: [[VAR4NUM:![0-9]+]] = !DILocalVariable(name: "s",
 
 ...
 ---
 name:            Process
 tracksRegLiveness: true
 liveins:
   - { reg: '$rdi', virtual-reg: '%2' }
   - { reg: '$rsi', virtual-reg: '%2' }
bb.1.block1:
     ; CHECK: JMP_1 %bb.2
     %1:gr64 = ADD64ri8 %5, 4, implicit-def dead $eflags
     JMP_1 %bb.2.exit
 
   bb.2.exit:
     $rax = MOV64rr %2
     RET 0
 ...
+---
+name:            test4
+tracksRegLiveness: true
+liveins:
+  - { reg: '$rdi', virtual-reg: '%2' }
+  - { reg: '$rsi', virtual-reg: '%2' }
+body:             |
+  bb.0.entry:
+    successors: %bb.1.block1, %bb.2.block2
+    liveins: $rdi, $esi
+
+    ; The DBG_VALUE here should sink through several blocks, but not leave any
+    ; additional DBG_VALUE $noregs on its way.
+    ; CHECK-LABEL: bb.0.entry:
+    ; CHECK:       [[TEST4VREG:%[0-9]+]]:gr64 = COPY $rdi
+    ; CHECK-NEXT:  CMP32ri $esi, 0
+    ; CHECK-NEXT:  DBG_VALUE $noreg, $noreg, [[VAR4NUM]]
+    ; CHECK-NEXT:  JCC_1 %bb.1, 4
+    ; CHECK-NEXT:  JMP_1
+
+    %2:gr64 = COPY $rdi
+    %5:gr64 = ADD64ri8 %2, 1, implicit-def dead $eflags
+    CMP32ri $esi, 0, implicit-def $eflags
+    DBG_VALUE %5, $noreg, !303, !17, debug-location !304
+    JCC_1 %bb.1.block1, 4, implicit $eflags
+    JMP_1 %bb.2.block2
+
+  bb.1.block1:
+    successors: %bb.3.combined
+    liveins: $esi
+
+    ; CHECK-LABEL: bb.1.block1
+    ; CHECK-NOT:   DBG_VALUE
+    ; CHECK:       JMP_1 %bb.3
+
+    JMP_1 %bb.3.combined
+
+  bb.2.block2:
+    successors: %bb.3.combined
+    liveins: $esi
+
+    ; CHECK-LABEL: bb.2.block2
+    ; CHECK-NOT:   DBG_VALUE
+    ; CHECK:       JMP_1 %bb.3
+
+    JMP_1 %bb.3.combined
+
+  bb.3.combined:
+    successors: %bb.4.branch1, %bb.5.exit
+    liveins: $esi
+
+    ; CHECK-LABEL: bb.3.combined
+    ; CHECK-NOT:   DBG_VALUE
+    ; CHECK:       JCC_1 %bb.4, 4, implicit $eflags
+    ; CHECK-NEXT:  JMP_1 %bb.5
+
+    CMP32ri $esi, 1, implicit-def $eflags
+    JCC_1 %bb.4.branch1, 4, implicit $eflags
+    JMP_1 %bb.5.exit
+
+  bb.4.branch1:
+    successors: %bb.5.exit
+
+    ; This block should receive the sunk copy and DBG_VALUE.
+    ; CHECK-LABEL: bb.4.branch1:
+    ; CHECK-NOT:   DBG_VALUE
+    ; CHECK:       [[TEST4VREG2:%[0-9]+]]:gr64 = ADD64ri8 [[TEST4VREG]], 1
+    ; CHECK-NEXT:  DBG_VALUE [[TEST4VREG2]], $noreg, [[VAR4NUM]]
+    ; CHECK-NEXT:  ADD64ri8
+    ; CHECK:       JMP_1 %bb.5
+
+    %1:gr64 = ADD64ri8 %5, 4, implicit-def dead $eflags
+    JMP_1 %bb.5.exit
+
+  bb.5.exit:
+    ; CHECK-LABEL: bb.5.exit
+    ; CHECK-NOT:   DBG_VALUE
+    ; CHECK:       RET 0
+
+    $rax = MOV64rr %2
+    RET 0
+...