[MachineBasicBlock] Skip over debug instructions in computeRegisterLiveness before checking for begin/end.

[craig.topper]

Summary

If there are debug instructions before/after the stopping point,
we need to skip over them before checking for begin/end in order
to avoid having the debug instructions effect behavior.

Fixes PR43758.

[craig.topper]

Create the diff correctly. I forgot to commit the test case before running arcanist.

[yechunliang]

llvm/lib/CodeGen/MachineBasicBlock.cpp

1410–1411

Does this code duplicated with the line: 1392-1394?

for (; I != end() && N > 0; ++I) {
   if (I->isDebugInstr())
     continue;

[yechunliang]

Will it be better to use getFirstNonDebugInstr() instead of begin() on line 1475?

// Did we get to the start of the block?
-   if (I == begin()) {
+   if (I == getFirstNonDebugInstr()) {

[craig.topper]

In D69606#1726744, @yechunliang wrote:

Will it be better to use getFirstNonDebugInstr() instead of begin() on line 1475?

// Did we get to the start of the block?
-   if (I == begin()) {
+   if (I == getFirstNonDebugInstr()) {

Not sure. I guess it depends on whether its more likely that there are debug instructions at the beginning of the block or the current iterator. If we use getFirstNonDebugInstr we're starting a separate scan from the beginning of the block. The code I have here starts from the current iterator.

llvm/lib/CodeGen/MachineBasicBlock.cpp

1410–1411

But that code stops when N is 0. So we can exit the loop without being at end() and have debug instructions after right?

[efriedma]

llvm/lib/CodeGen/MachineBasicBlock.cpp

1410–1411

Yes, but it's probably better to refactor the loop to use an an early exit, or something like that.

[yechunliang]

Look at the debugging printf information, I found the begin() is the first instruction of the BasicBlock, but the end() is not the last instruction, it's something unknown or none flag.
If what I found is correct, and would say end() could not be debug instruction, and maybe we needn't consider about debug instruction impact for end() part.

llvm/lib/CodeGen/MachineBasicBlock.cpp

1410–1411

Before N--, the above code " if (I->isDebugInstr()) continue" will skip the DebugInstr. So the variable "I" could not be DebugInstr when N=0.
So when case N=0, the variable "I" should be real instruction. And Line 1409-1411 will be duplicate.

[craig.topper]

Remove the end part of the code

[efriedma]

LGTM

llvm/lib/CodeGen/MachineBasicBlock.cpp

for (; I != end() && N > 0; ++I) {
     if (Info.Read)
       return LQR_Live;
     // Register is dead if we can fully overwrite or clobber it here.
     if (Info.FullyDefined || Info.Clobbered)
       return LQR_Dead;
   }
 
   // If we reached the end, it is safe to clobber Reg at the end of a block of
   // no successor has it live in.
   if (I == end()) {
     for (MachineBasicBlock *S : successors()) {
       for (const MachineBasicBlock::RegisterMaskPair &LI : S->liveins()) {
         if (TRI->regsOverlap(LI.PhysReg, Reg))
           return LQR_Live;
       }
     }
 
     return LQR_Dead;
do {
         return LQR_Dead;
       // Register must be live if we read it.
       if (Info.Read)
         return LQR_Live;
 
     } while (I != begin() && N > 0);
   }
 
+  // If all the instructions before this in the block are debug instructions,
+  // skip over them.
+  while (I != begin() && std::prev(I)->isDebugInstr())
+    --I;
+
   // Did we get to the start of the block?
   if (I == begin()) {
     // If so, the register's state is definitely defined by the live-in state.
     for (const MachineBasicBlock::RegisterMaskPair &LI : liveins())
       if (TRI->regsOverlap(LI.PhysReg, Reg))
         return LQR_Live;
 
     return LQR_Dead;

llvm/test/CodeGen/X86/leaFixup64.mir

bb.0 (%ir-block.0):
 
     ; CHECK-LABEL: name: pr43758
     ; CHECK: liveins: $rax, $rbp
     ; CHECK: DBG_VALUE 0, $noreg
     ; CHECK: NOOP
     ; CHECK: NOOP
     ; CHECK: NOOP
     ; CHECK: NOOP
-    ; CHECK: $ebp = LEA64_32r killed $rbp, 1, killed $rax, 0, $noreg
+    ; CHECK: $ebp = ADD32rr $ebp, $eax, implicit-def $eflags, implicit $rbp, implicit $rax
     ; CHECK: NOOP
     ; CHECK: NOOP
     ; CHECK: NOOP
     ; CHECK: NOOP
     ; CHECK: RETQ $ebp
     DBG_VALUE 0, $noreg
     NOOP
     NOOP