[TwoAddressInstruction] Fix handling of operands tied to early-clobbers

[foad]

Summary

When updating live intervals (with -early-live-intervals), fix handling
of live ranges that were previously tied to an early-clobber def but no
longer are.

[MatzeB]

I can't shake the feeling that we would be better of having the convertToThreeAddress callback fix the liveness information like it is doing with LV, rather than guessing around after the fact with repairIntervalsInRange... Not sure what the original intention here was and if its fair to ask for that big refactoring now...

llvm/lib/CodeGen/TwoAddressInstructionPass.cpp

708–718

You fix the early-clobber -> register case, but I guess register->early clobber could happen too, given that we don't really know what instruction is involved here.

[MatzeB]

llvm/test/CodeGen/RISCV/rvv/vwadd.w-rv64.ll

3

Could we create a separate .mir test instead?

[foad]

In D113123#3115810, @MatzeB wrote:

I can't shake the feeling that we would be better of having the convertToThreeAddress callback fix the liveness information like it is doing with LV, rather than guessing around after the fact with repairIntervalsInRange... Not sure what the original intention here was and if its fair to ask for that big refactoring now...

I agree. I have fixed a few problems that I encountered in repairIntervalsInRange, but I am not sure if it will ever be 100% reliable.

I will try passing LIS into convertToThreeAddress instead.

[foad]

In D113123#3116272, @foad wrote:

In D113123#3115810, @MatzeB wrote:

I can't shake the feeling that we would be better of having the convertToThreeAddress callback fix the liveness information like it is doing with LV, rather than guessing around after the fact with repairIntervalsInRange... Not sure what the original intention here was and if its fair to ask for that big refactoring now...

I agree. I have fixed a few problems that I encountered in repairIntervalsInRange, but I am not sure if it will ever be 100% reliable.

Actually it is used more than I thought:

• in MachineBasicBlock::SplitCriticalEdge
• in PPCTLSDynamicCall::processBlock
• after TargetInstrInfo::convertToThreeAddress
• after TargetInstrInfo::unfoldMemoryOperand
• in TwoAddressInstructionPass::eliminateRegSequence

[foad]

Superseded by D113493.

llvm/lib/CodeGen/TwoAddressInstructionPass.cpp

if (auto OldInstrNum = mi->peekDebugInstrNum()) {
     MF->makeDebugValueSubstitution(std::make_pair(OldInstrNum, OldIdx),
                                    std::make_pair(NewInstrNum, NewIdx));
   }
 
   // If convertToThreeAddress created a single new instruction, assume it has
   // exactly the same effect on liveness as the old instruction. This is much
   // more efficient than calling repairIntervalsInRange.
   bool SingleInst = std::next(MIS.begin(), 2) == MIS.end();
-  if (LIS && SingleInst)
-    LIS->ReplaceMachineInstrInMaps(*mi, *NewMI);
+  if (LIS && SingleInst) {
+    SlotIndex Idx = LIS->ReplaceMachineInstrInMaps(*mi, *NewMI);
+
+    // RegB was tied to RegA. If RegA is early-clobber then the live range of
+    // RegB would have ended at an early-clobber slot, but now they are not tied
+    // we need to update it to the normal register slot.
+    LiveInterval &LI = LIS->getInterval(RegB);
+    LiveRange::Segment *S = LI.getSegmentContaining(Idx);
+    if (S->end == Idx.getRegSlot(true))
+      S->end = Idx.getRegSlot();
+  }
 
   SmallVector<Register> OrigRegs;
   if (LIS && !SingleInst) {
     for (const MachineOperand &MO : mi->operands()) {
       if (MO.isReg())
         OrigRegs.push_back(MO.getReg());
     }
 

llvm/test/CodeGen/RISCV/rvv/vwadd.w-rv32.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=riscv32 -mattr=+experimental-v -verify-machineinstrs \
-; RUN:   < %s | FileCheck %s
+; RUN: llc -mtriple=riscv32 -mattr=+experimental-v -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -mtriple=riscv32 -mattr=+experimental-v -verify-machineinstrs -early-live-intervals < %s | FileCheck %s
 declare <vscale x 1 x i16> @llvm.riscv.vwadd.w.nxv1i16.nxv1i8(
   <vscale x 1 x i16>,
   <vscale x 1 x i8>,
   i32);
 
 define <vscale x 1 x i16> @intrinsic_vwadd.w_wv_nxv1i16_nxv1i16_nxv1i8(<vscale x 1 x i16> %0, <vscale x 1 x i8> %1, i32 %2) nounwind {
 ; CHECK-LABEL: intrinsic_vwadd.w_wv_nxv1i16_nxv1i16_nxv1i8:
 ; CHECK:       # %bb.0: # %entry

llvm/test/CodeGen/RISCV/rvv/vwadd.w-rv64.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=riscv64 -mattr=+experimental-v -verify-machineinstrs \
-; RUN:   < %s | FileCheck %s
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-v -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-v -verify-machineinstrs -early-live-intervals < %s | FileCheck %s
 declare <vscale x 1 x i16> @llvm.riscv.vwadd.w.nxv1i16.nxv1i8(
   <vscale x 1 x i16>,
   <vscale x 1 x i8>,
   i64);
 
 define <vscale x 1 x i16> @intrinsic_vwadd.w_wv_nxv1i16_nxv1i16_nxv1i8(<vscale x 1 x i16> %0, <vscale x 1 x i8> %1, i64 %2) nounwind {
 ; CHECK-LABEL: intrinsic_vwadd.w_wv_nxv1i16_nxv1i16_nxv1i8:
 ; CHECK:       # %bb.0: # %entry