[LiveIntervals] Improve repair after convertToThreeAddress

[foad]

Summary

After TwoAddressInstructionPass calls
TargetInstrInfo::convertToThreeAddress, improve the LiveIntervals repair
to cope with convertToThreeAddress creating more than one new
instruction.

This mostly seems to benefit X86. For example in
test/CodeGen/X86/zext-trunc.ll it converts:

%4:gr32 = ADD32rr %3:gr32(tied-def 0), %2:gr32, implicit-def dead $eflags

to:

undef %6.sub_32bit:gr64 = COPY %3:gr32
undef %7.sub_32bit:gr64_nosp = COPY %2:gr32
%4:gr32 = LEA64_32r killed %6:gr64, 1, killed %7:gr64_nosp, 0, $noreg

[qcolombet]

Hey!

This mostly seems to benefit X86

What benefits are we about here?
Is it compile time?

Cheers,
-Quentin

[foad]

What benefits are we about here?
Is it compile time?

Good question. It is mostly about reviving an incomplete transition from LiveVariables to LiveIntervals, as described here (thread): https://lists.llvm.org/pipermail/llvm-dev/2013-February/059261.html

LiveIntervals have good support for tracking subregister liveness information as described here (thread): https://lists.llvm.org/pipermail/llvm-dev/2013-October/066176.html

My motivation for working on this is that I have recently run into cases where TwoAddressInstructionPass is doing a poor job because it is relying on kill flags, which only relate to a whole "superregister", instead of fine grained subregister liveness. The best way to fix this seems to be to complete the transition to LiveIntervals. It's easy to test by forcing -early-live-intervals on. I don't know what the compile time impact will be yet.

[foad]

This mostly seems to benefit X86

What benefits are we about here?
Is it compile time?

At this stage I'm just trying to fix various crashes caused by -early-live-intervals, so I just meant that this patch mostly fixes crashes in X86 test cases.

[qcolombet]

Makes sense, thanks!

llvm/lib/CodeGen/TwoAddressInstructionPass.cpp

bool TwoAddressInstructionPass::isProfitableToConv3Addr(Register RegA,
   return (ToRegA && !regsAreCompatible(FromRegB, ToRegA, TRI));
 }
 
 /// Convert the specified two-address instruction into a three address one.
 /// Return true if this transformation was successful.
 bool TwoAddressInstructionPass::convertInstTo3Addr(
     MachineBasicBlock::iterator &mi, MachineBasicBlock::iterator &nmi,
     Register RegA, Register RegB, unsigned Dist) {
+  MachineInstrSpan MIS(mi, MBB);
   MachineInstr *NewMI = TII->convertToThreeAddress(*mi, LV);
   if (!NewMI)
     return false;
 
   LLVM_DEBUG(dbgs() << "2addr: CONVERTING 2-ADDR: " << *mi);
   LLVM_DEBUG(dbgs() << "2addr:         TO 3-ADDR: " << *NewMI);
 
-  if (LIS)
-    LIS->ReplaceMachineInstrInMaps(*mi, *NewMI);
-
   // If the old instruction is debug value tracked, an update is required.
   if (auto OldInstrNum = mi->peekDebugInstrNum()) {
     // Sanity check.
     assert(mi->getNumExplicitDefs() == 1);
     assert(NewMI->getNumExplicitDefs() == 1);
 
     // Find the old and new def location.
     auto OldIt = mi->defs().begin();
     auto NewIt = NewMI->defs().begin();
     unsigned OldIdx = mi->getOperandNo(OldIt);
     unsigned NewIdx = NewMI->getOperandNo(NewIt);
 
     // Record that one def has been replaced by the other.
     unsigned NewInstrNum = NewMI->getDebugInstrNum();
     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);
+
+  SmallVector<Register> OrigRegs;
+  if (LIS && !SingleInst) {
+    for (const MachineOperand &MO : mi->operands()) {
+      if (MO.isReg())
+        OrigRegs.push_back(MO.getReg());
+    }
+  }
+
   MBB->erase(mi); // Nuke the old inst.
 
+  if (LIS && !SingleInst)
+    LIS->repairIntervalsInRange(MBB, MIS.begin(), MIS.end(), OrigRegs);
+
   DistanceMap.insert(std::make_pair(NewMI, Dist));
   mi = NewMI;
   nmi = std::next(mi);
 
   // Update source and destination register maps.
   SrcRegMap.erase(RegA);
   DstRegMap.erase(RegB);
   return true;

llvm/test/CodeGen/X86/zext-trunc.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown | FileCheck %s
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -early-live-intervals -verify-machineinstrs | FileCheck %s
 ; rdar://7570931
 
 define i64 @foo(i64 %a, i64 %b) nounwind {
 ; CHECK-LABEL: foo:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    leal (%rdi,%rsi), %eax
 ; CHECK-NEXT:    retq
   %c = add i64 %a, %b
   %d = trunc i64 %c to i32
   %e = zext i32 %d to i64
   ret i64 %e
 }