[LiveDebugValues] Do not insert DBG_VALUEs after a MBB terminator

[vsk]

Summary

This fixes a miscompile that happened because a DBG_VALUE interfered
with the MachineOutliner's liveness analysis.

Inserting a DBG_VALUE after a terminator breaks predicates on MBB such
as isReturnBlock(). And the resulting DBG_VALUE cannot be "live", afaict.

I plan to introduce a MachineVerifier check for this situation in a
follow up.

rdar://59859175

[paquette]

LGTM

[djtodoro]

Makes sense to me! Thanks!

llvm/lib/CodeGen/LiveDebugValues.cpp

 }
 
 /// Turn the entry value backup locations into primary locations.
 void LiveDebugValues::emitEntryValues(MachineInstr &MI,
                                       OpenRangesSet &OpenRanges,
                                       VarLocMap &VarLocIDs,
                                       TransferMap &Transfers,
                                       VarLocSet &KillSet) {
+  // Do not insert entry value locations after a terminator.
+  if (MI.isTerminator())
+    return;
+
   for (uint64_t ID : KillSet) {
     LocIndex Idx = LocIndex::fromRawInteger(ID);
     const VarLoc &VL = VarLocIDs[Idx];
     if (!VL.Var.getVariable()->isParameter())
       continue;
 
     auto DebugVar = VL.Var;
     Optional<LocIndex> EntryValBackupID =
auto ProcessVarLoc = [&MI, &OpenRanges, &Transfers, &VarLocIDs](VarLoc &VL) {
     LocIndex LocId = VarLocIDs.insert(VL);
 
     // Close this variable's previous location range.
     OpenRanges.erase(VL);
 
     // Record the new location as an open range, and a postponed transfer
     // inserting a DBG_VALUE for this location.
     OpenRanges.insert(LocId, VL);
+    assert(!MI.isTerminator() && "Cannot insert DBG_VALUE after terminator");
     TransferDebugPair MIP = {&MI, LocId};
     Transfers.push_back(MIP);
   };
 
   // End all previous ranges of VL.Var.
   OpenRanges.erase(VarLocIDs[OldVarID]);
   switch (Kind) {
   case TransferKind::TransferCopy: {
while (!Worklist.empty() || !Pending.empty()) {
     Worklist.swap(Pending);
     // At this point, pending must be empty, since it was just the empty
     // worklist
     assert(Pending.empty() && "Pending should be empty");
   }
 
   // Add any DBG_VALUE instructions created by location transfers.
   for (auto &TR : Transfers) {
+    assert(!TR.TransferInst->isTerminator() &&
+           "Cannot insert DBG_VALUE after terminator");
     MachineBasicBlock *MBB = TR.TransferInst->getParent();
     const VarLoc &VL = VarLocIDs[TR.LocationID];
     MachineInstr *MI = VL.BuildDbgValue(MF);
     MBB->insertAfterBundle(TR.TransferInst->getIterator(), MI);
   }
   Transfers.clear();
 
   // Deferred inlocs will not have had any DBG_VALUE insts created; do

llvm/test/DebugInfo/MIR/X86/dbginfo-entryvals.mir

-# RUN: llc -debug-entry-values -run-pass=livedebugvalues -march=x86-64 -o - %s | FileCheck %s
+# RUN: llc -debug-entry-values -run-pass=livedebugvalues -verify-machineinstrs -march=x86-64 -o - %s | FileCheck %s
 #
 #extern void fn2(int);
 #
 #__attribute__((noinline))
 #void
 #fn1 (int x, int y) {
 #  int u = x + y;
 #  if (x > 1)
 #    u += 1;
 #  else
 #    u += 2;
 #  int a = 7;
 #  fn2 (a);
 #  u --;
 #}
 # CHECK: DBG_VALUE $edi, $noreg, !14, !DIExpression(DW_OP_LLVM_entry_value, 1), debug-location {{.*}}
-# CHECK: DBG_VALUE $esi, $noreg, !15, !DIExpression(DW_OP_LLVM_entry_value, 1), debug-location {{.*}}
+# CHECK-NOT: DBG_VALUE $esi, $noreg, !15, !DIExpression(DW_OP_LLVM_entry_value, 1)
 
 --- |
   ; ModuleID = 'test.c'
   source_filename = "test.c"
   target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
   
   ; Function Attrs: noinline nounwind uwtable
   define dso_local void @fn1(i32 %x, i32 %y) local_unnamed_addr !dbg !9 {