[CodeGen] Fix assertion failure in TwoAddressInstructionPass::rescheduleMIBelowKill

[foad]

Summary

This fixes an assertion failure with -early-live-intervals when trying
to update the live intervals for a dbug instruction, which don't even
have slot indexes.

[RKSimon]

Any other comments?

llvm/lib/CodeGen/TwoAddressInstructionPass.cpp

958–959

Adjust this - since we're not actually moving the debug instr copies

[foad]

llvm/lib/CodeGen/TwoAddressInstructionPass.cpp

958–959

Not quite sure what you mean. The instructions in the range [AfterMI,End) are a mixture of copies and debug instructions. We are moving all of them, but (with this patch) only updating LiveIntervals when we move a copy. I think the comment above is still accurate. Did you have a suggestion for improving it?

[MatzeB]

Let's fix the condition then this is good to go.

llvm/lib/CodeGen/TwoAddressInstructionPass.cpp

958–959

I also needed a while to understand this comment, since it implies that there are only COPY instructions here. I think you should either tweak the comment to say that there may be debug or pseudo instructions mixed in or add a comment before the if (isDebugOrPseudoInstr().

963–964

The condition used in SlotIndexes.cpp and in skipDebugInstructionsForward (I wish it had a better name) seems to be isDebugOrPseudoInstr().

You could go for LiveIntervals::hasIndex() instead to be safe for future changes (at the cost of an extra map lookup). I'll let you make the call depending on how perf critical you judge this part.

[foad]

Use isDebugOrPseudoInstr. Update comment.

[MatzeB]

LGTM

llvm/lib/CodeGen/TwoAddressInstructionPass.cpp

bool TwoAddressInstructionPass::rescheduleMIBelowKill(
 
   // Move debug info as well.
   while (Begin != MBB->begin() && std::prev(Begin)->isDebugInstr())
     --Begin;
 
   nmi = End;
   MachineBasicBlock::iterator InsertPos = KillPos;
   if (LIS) {
-    // We have to move the copies first so that the MBB is still well-formed
-    // when calling handleMove().
+    // We have to move the copies (and any interleaved debug instructions)
+    // first so that the MBB is still well-formed when calling handleMove().
     for (MachineBasicBlock::iterator MBBI = AfterMI; MBBI != End;) {
       auto CopyMI = MBBI++;
       MBB->splice(InsertPos, MBB, CopyMI);
+      if (!CopyMI->isDebugOrPseudoInstr())
         LIS->handleMove(*CopyMI);
       InsertPos = CopyMI;
     }
     End = std::next(MachineBasicBlock::iterator(MI));
   }
 
   // Copies following MI may have been moved as well.
   MBB->splice(InsertPos, MBB, Begin, End);
   DistanceMap.erase(DI);

llvm/test/DebugInfo/MIR/X86/mlicm-hoist-post-regalloc.mir

 --- |
   ; REQUIRES: asserts
   ; RUN: llc -start-before=phi-node-elimination -stop-after=machinelicm -debug-only=machinelicm -o - %s  | FileCheck %s
+  ; RUN: llc -start-before=phi-node-elimination -stop-after=machinelicm -debug-only=machinelicm -early-live-intervals -o - %s  | FileCheck %s
   ; Ensure we execute machinelicm post register allocation.
   ; Line numbers should not be retained when loop invariant instructions are hoisted.
   ;
   ; CHECK-LABEL:  bb.0.entry:
   ; CHECK:        MOV64rm $rip, 1, $noreg, target-flags(x86-gotpcrel) @x, $noreg :: (load (s64) from got)
   ; CHECK-LABEL:  bb.1.while.body:
   ;
   target triple = "x86_64-unknown-linux-gnu"