[LiveIntervals] Fix repairOldRegInRange for simple def cases

[foad]

Summary

The fix applied in D23303 "LiveIntervalAnalysis: fix a crash in repairOldRegInRange"
was over-zealous. It would bail out when the end of the range to be
repaired was in the middle of the first segment of the live range of
Reg, which was always the case when the range contained a single def of
Reg.

This patch fixes it as suggested by Matthias Braun in post-commit review
on the original patch, and tests it by adding -early-live-intervals to
a selection of existing lit tests that now pass.

(Note that D23303 was originally applied to fix a crash in
SILoadStoreOptimizer, but that is now moot since D23814 updated
SILoadStoreOptimizer to run before scheduling so it no longer has to
update live intervals.)

[arsenm]

LGTM. I don't follow how the test changes stress this though

[foad]

I don't follow how the test changes stress this though

-early-live-intervals means that LiveIntervals is available to TwoAddressInstructionPass, which uses repairIntervalsInRange to update it. Without this patch we hit various assertion and/or verification failures because it doesn't get updated correctly.

[arsenm]

In D110238#3015149, @foad wrote:

I don't follow how the test changes stress this though

-early-live-intervals means that LiveIntervals is available to TwoAddressInstructionPass, which uses repairIntervalsInRange to update it. Without this patch we hit various assertion and/or verification failures because it doesn't get updated correctly.

Can you add a dedicated MIR pass that just runs TwoAddressInstructionPass then?

[foad]

Add a MIR test case.

[foad]

This is failing with expensive checks: https://lab.llvm.org/buildbot/#/builders/16/builds/17100

I'vv revert if I can't fix it in the next few minutes.

llvm/lib/CodeGen/LiveIntervals.cpp

 
 void LiveIntervals::repairOldRegInRange(const MachineBasicBlock::iterator Begin,
                                         const MachineBasicBlock::iterator End,
                                         const SlotIndex EndIdx, LiveRange &LR,
                                         const Register Reg,
                                         LaneBitmask LaneMask) {
   LiveInterval::iterator LII = LR.find(EndIdx);
   SlotIndex lastUseIdx;
-  if (LII == LR.begin()) {
-    // This happens when the function is called for a subregister that only
-    // occurs _after_ the range that is to be repaired.
-    return;
-  }
-  if (LII != LR.end() && LII->start < EndIdx)
+  if (LII != LR.end() && LII->start < EndIdx) {
     lastUseIdx = LII->end;
-  else
+  } else if (LII == LR.begin()) {
+    // We may not have a liverange at all if this is a subregister untouched
+    // between \p Begin and \p End.
+  } else {
     --LII;
+  }
 
   for (MachineBasicBlock::iterator I = End; I != Begin;) {
     --I;
     MachineInstr &MI = *I;
     if (MI.isDebugOrPseudoInstr())
       continue;
 
     SlotIndex instrIdx = getInstructionIndex(MI);

llvm/test/CodeGen/AMDGPU/twoaddr-regsequence.mir

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=amdgcn-amd-amdhsa -run-pass=liveintervals,twoaddressinstruction,simple-register-coalescing -verify-machineinstrs -o - %s | FileCheck %s
+
+# Check that LiveIntervals are correctly updated when eliminating REG_SEQUENCE.
+---
+name: f
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $vgpr0, $vgpr1
+
+    ; CHECK-LABEL: name: f
+    ; CHECK: liveins: $vgpr0, $vgpr1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: undef %2.sub0:vreg_64 = COPY $vgpr0
+    ; CHECK-NEXT: %2.sub1:vreg_64 = COPY $vgpr1
+    ; CHECK-NEXT: $vgpr2_vgpr3 = COPY %2
+    ; CHECK-NEXT: S_NOP 0, implicit $vgpr2_vgpr3
+    %0:vgpr_32 = COPY $vgpr0
+    %1:vgpr_32 = COPY $vgpr1
+    %35:vreg_64 = REG_SEQUENCE %0, %subreg.sub0, %1, %subreg.sub1
+    $vgpr2_vgpr3 = COPY %35
+    S_NOP 0, implicit $vgpr2_vgpr3
+...

llvm/test/CodeGen/ARM/signext-inreg.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=armv8 | FileCheck %s
+; RUN: llc < %s -mtriple=armv8 -early-live-intervals -verify-machineinstrs | FileCheck %s
 define <4 x i32> @test(<4 x i32> %m) {
 ; CHECK-LABEL: test:
 ; CHECK:       @ %bb.0: @ %entry
 ; CHECK-NEXT:    vmov d17, r2, r3
 ; CHECK-NEXT:    vmov d16, r0, r1
 ; CHECK-NEXT:    vshl.i32 q8, q8, #24
 ; CHECK-NEXT:    vshr.s32 q8, q8, #24
 ; CHECK-NEXT:    vmov r0, r1, d16
 ; CHECK-NEXT:    vmov r2, r3, d17
 ; CHECK-NEXT:    bx lr
 entry:
   %shl = shl <4 x i32> %m, <i32 24, i32 24, i32 24, i32 24>
   %shr = ashr exact <4 x i32> %shl, <i32 24, i32 24, i32 24, i32 24>
   ret <4 x i32> %shr
 }

llvm/test/CodeGen/X86/mul-shift-reassoc.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=i686-- | FileCheck %s
+; RUN: llc < %s -mtriple=i686-- -early-live-intervals -verify-machineinstrs | FileCheck %s
 
 define i32 @test(i32 %X, i32 %Y) {
   ; Push the shl through the mul to allow an LEA to be formed, instead
         ; of using a shift and add separately.
 ; CHECK-LABEL: test:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; CHECK-NEXT:    movl {{[0-9]+}}(%esp), %ecx