[DebugInfo] Don't insert DEBUG_VALUE after terminators

[smaksimovic]

Summary

r314974 introduced insertion of DEBUG_VALUEs after
each redefinition of debug value register in the slot index range.

In case the instruction redefining the debug value register
was a terminator, machine verifier would complain since it
enforces the rule of no non-terminator instructions
following the first terminator.

[Ka-Ka]

The change LGTM, but I'm not sure if I'm allowed to set "ready to land". I would prefer if Adrian Prantl or Reid Kleckner also reviewed this change (both now added as reviewers).

[Ka-Ka]

Ping.
As I said before the change LGTM, but I would prefer if someone else also reviewed this change.

[wolfgangp]

Change LGTM with minor comment on possible readability improvement.

lib/CodeGen/LiveDebugVariables.cpp

1103 ↗

(On Diff #132405)

The change makes perfect sense. From a readability point of view, I'd perhaps prefer to express this concept in the while condition, or else have a separate if statement for it, i.e. either

while (I != MBB->end() && !I->isTerminator())

or

while (I != MBB->end()) {

if (I->isTerminator())
  break;
if (!LIS.....

[wolfgangp]

LGTM.

llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp

findNextInsertLocation(MachineBasicBlock *MBB,
                        SlotIndex StopIdx, MachineOperand &LocMO,
                        LiveIntervals &LIS,
                        const TargetRegisterInfo &TRI) {
   if (!LocMO.isReg())
     return MBB->instr_end();
   unsigned Reg = LocMO.getReg();
 
   // Find the next instruction in the MBB that define the register Reg.
-  while (I != MBB->end()) {
+  while (I != MBB->end() && !I->isTerminator()) {
     if (!LIS.isNotInMIMap(*I) &&
         SlotIndex::isEarlierEqualInstr(StopIdx, LIS.getInstructionIndex(*I)))
       break;
     if (I->definesRegister(Reg, &TRI))
       // The insert location is directly after the instruction/bundle.
       return std::next(I);
     ++I;
   }

llvm/trunk/test/CodeGen/Mips/pr34975.ll

-; RUN: llc -mtriple=mips64-unknown-freebsd -target-abi n64 -relocation-model pic -o /dev/null %s -O2
+; RUN: llc -mtriple=mips64-unknown-freebsd -target-abi n64 -relocation-model pic -verify-machineinstrs -o /dev/null %s -O2
 
 ; Test that the presence of debug information does not cause the branch folder
 ; to rewrite branches to have negative basic block ids, which would cause the
 ; long branch pass to crash.
 
 @c = external global i32, align 4
 
 define void @e() !dbg !19 {

llvm/trunk/test/CodeGen/Mips/pr35071.ll

-; RUN: llc -mtriple mips64-unknown-freebsd12.0 -relocation-model pic -mcpu=mips4 -target-abi n64 -O2 -o - %s
+; RUN: llc -mtriple mips64-unknown-freebsd12.0 -relocation-model pic -mcpu=mips4 -target-abi n64 -O2 -verify-machineinstrs -o - %s
 
 ; Test that the long branch pass does not crash due to the control flow
 ; optimizer producing malformed basic block operands due to the backend
 ; failing to handle debug information around branch instructions.
 
 define void @f() !dbg !5 {
 entry:
   %cmp = icmp eq i32 undef, 0, !dbg !16