[RegAlloc] Make tryInstructionSplit less aggressive.

The greedy register allocator tries to split a live-range around each
instruction where it is used or defined to relax the constraints on the entire
live-range (this is a last chance split before falling back to spill).
The goal is to have a big live-range that is unconstrained (i.e., that can use
the largest legal register class) and several small local live-range that carry
the constraints implied by each instruction.
E.g.,
Let csti be the constraints on operation i.

V1=
op1 V1(cst1)
op2 V1(cst2)

V1 live-range is constrained on the intersection of cst1 and cst2.

tryInstructionSplit relaxes those constraints by aggressively splitting each
def/use point:
V1=
V2 = V1
V3 = V2
op1 V3(cst1)
V4 = V2
op2 V4(cst2)

Because of how the coalescer infrastructure works, each new variable (V3, V4)
that is alive at the same time as V1 (or its copy, here V2) interfere with V1.
Thus, we end up with an uncoalescable copy for each split point.

To make tryInstructionSplit less aggressive, we check if the split point
actually relaxes the constraints on the whole live-range. If it does not, we do
not insert it.
Indeed, it will not help the global allocation problem:
- V1 will have the same constraints.
- V1 will have the same interference + possibly the newly added split variable
  VS.
- VS will produce an uncoalesceable copy if alive at the same time as V1.

<rdar://problem/15570057>


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@198369 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/CodeGen/MachineInstr.h

@@ -830,6 +830,37 @@ class MachineInstr : public ilist_node<MachineInstr> {
                         const TargetInstrInfo *TII,
                         const TargetRegisterInfo *TRI) const;
 
+  /// \brief Applies the constraints (def/use) implied by this MI on \p Reg to
+  /// the given \p CurRC.
+  /// If \p ExploreBundle is set and MI is part of a bundle, all the
+  /// instructions inside the bundle will be taken into account. In other words,
+  /// this method accumulates all the constrains of the operand of this MI and
+  /// the related bundle if MI is a bundle or inside a bundle.
+  ///
+  /// Returns the register class that statisfies both \p CurRC and the
+  /// constraints set by MI. Returns NULL if such a register class does not
+  /// exist.
+  ///
+  /// \pre CurRC must not be NULL.
+  const TargetRegisterClass *getRegClassConstraintEffectForVReg(
+      unsigned Reg, const TargetRegisterClass *CurRC,
+      const TargetInstrInfo *TII, const TargetRegisterInfo *TRI,
+      bool ExploreBundle = false) const;
+
+  /// \brief Applies the constraints (def/use) implied by the \p OpIdx operand
+  /// to the given \p CurRC.
+  ///
+  /// Returns the register class that statisfies both \p CurRC and the
+  /// constraints set by \p OpIdx MI. Returns NULL if such a register class
+  /// does not exist.
+  ///
+  /// \pre CurRC must not be NULL.
+  /// \pre The operand at \p OpIdx must be a register.
+  const TargetRegisterClass *
+  getRegClassConstraintEffect(unsigned OpIdx, const TargetRegisterClass *CurRC,
+                              const TargetInstrInfo *TII,
+                              const TargetRegisterInfo *TRI) const;
+
   /// tieOperands - Add a tie between the register operands at DefIdx and
   /// UseIdx. The tie will cause the register allocator to ensure that the two
   /// operands are assigned the same physical register.
@@ -1038,6 +1069,13 @@ class MachineInstr : public ilist_node<MachineInstr> {
   /// hasPropertyInBundle - Slow path for hasProperty when we're dealing with a
   /// bundle.
   bool hasPropertyInBundle(unsigned Mask, QueryType Type) const;
+
+  /// \brief Implements the logic of getRegClassConstraintEffectForVReg for the
+  /// this MI and the given operand index \p OpIdx.
+  /// If the related operand does not constrained Reg, this returns CurRC.
+  const TargetRegisterClass *getRegClassConstraintEffectForVRegImpl(
+      unsigned OpIdx, unsigned Reg, const TargetRegisterClass *CurRC,
+      const TargetInstrInfo *TII, const TargetRegisterInfo *TRI) const;
 };
 
 /// MachineInstrExpressionTrait - Special DenseMapInfo traits to compare

lib/CodeGen/MachineInstr.cpp

@@ -993,6 +993,54 @@ MachineInstr::getRegClassConstraint(unsigned OpIdx,
   return NULL;
 }
 
+const TargetRegisterClass *MachineInstr::getRegClassConstraintEffectForVReg(
+    unsigned Reg, const TargetRegisterClass *CurRC, const TargetInstrInfo *TII,
+    const TargetRegisterInfo *TRI, bool ExploreBundle) const {
+  // Check every operands inside the bundle if we have
+  // been asked to.
+  if (ExploreBundle)
+    for (ConstMIBundleOperands OpndIt(this); OpndIt.isValid() && CurRC;
+         ++OpndIt)
+      CurRC = OpndIt->getParent()->getRegClassConstraintEffectForVRegImpl(
+          OpndIt.getOperandNo(), Reg, CurRC, TII, TRI);
+  else
+    // Otherwise, just check the current operands.
+    for (ConstMIOperands OpndIt(this); OpndIt.isValid() && CurRC; ++OpndIt)
+      CurRC = getRegClassConstraintEffectForVRegImpl(OpndIt.getOperandNo(), Reg,
+                                                     CurRC, TII, TRI);
+  return CurRC;
+}
+
+const TargetRegisterClass *MachineInstr::getRegClassConstraintEffectForVRegImpl(
+    unsigned OpIdx, unsigned Reg, const TargetRegisterClass *CurRC,
+    const TargetInstrInfo *TII, const TargetRegisterInfo *TRI) const {
+  assert(CurRC && "Invalid initial register class");
+  // Check if Reg is constrained by some of its use/def from MI.
+  const MachineOperand &MO = getOperand(OpIdx);
+  if (!MO.isReg() || MO.getReg() != Reg)
+    return CurRC;
+  // If yes, accumulate the constraints through the operand.
+  return getRegClassConstraintEffect(OpIdx, CurRC, TII, TRI);
+}
+
+const TargetRegisterClass *MachineInstr::getRegClassConstraintEffect(
+    unsigned OpIdx, const TargetRegisterClass *CurRC,
+    const TargetInstrInfo *TII, const TargetRegisterInfo *TRI) const {
+  const TargetRegisterClass *OpRC = getRegClassConstraint(OpIdx, TII, TRI);
+  const MachineOperand &MO = getOperand(OpIdx);
+  assert(MO.isReg() &&
+         "Cannot get register constraints for non-register operand");
+  assert(CurRC && "Invalid initial register class");
+  if (unsigned SubIdx = MO.getSubReg()) {
+    if (OpRC)
+      CurRC = TRI->getMatchingSuperRegClass(CurRC, OpRC, SubIdx);
+    else
+      CurRC = TRI->getSubClassWithSubReg(CurRC, SubIdx);
+  } else if (OpRC)
+    CurRC = TRI->getCommonSubClass(CurRC, OpRC);
+  return CurRC;
+}
+
 /// Return the number of instructions inside the MI bundle, not counting the
 /// header instruction.
 unsigned MachineInstr::getBundleSize() const {

lib/CodeGen/MachineRegisterInfo.cpp

@@ -79,17 +79,9 @@ MachineRegisterInfo::recomputeRegClass(unsigned Reg, const TargetMachine &TM) {
   // Accumulate constraints from all uses.
   for (reg_nodbg_iterator I = reg_nodbg_begin(Reg), E = reg_nodbg_end(); I != E;
        ++I) {
-    const TargetRegisterClass *OpRC =
-      I->getRegClassConstraint(I.getOperandNo(), TII,
-                               getTargetRegisterInfo());
-    if (unsigned SubIdx = I.getOperand().getSubReg()) {
-      if (OpRC)
-        NewRC = getTargetRegisterInfo()->getMatchingSuperRegClass(NewRC, OpRC,
-                                                                  SubIdx);
-      else
-        NewRC = getTargetRegisterInfo()->getSubClassWithSubReg(NewRC, SubIdx);
-    } else if (OpRC)
-      NewRC = getTargetRegisterInfo()->getCommonSubClass(NewRC, OpRC);
+    // Apply the effect of the given operand to NewRC.
+    NewRC = I->getRegClassConstraintEffect(I.getOperandNo(), NewRC, TII,
+                                           getTargetRegisterInfo());
     if (!NewRC || NewRC == OldRC)
       return false;
   }

lib/CodeGen/RegAllocGreedy.cpp

@@ -35,6 +35,7 @@
 #include "llvm/CodeGen/MachineLoopInfo.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/RegAllocRegistry.h"
+#include "llvm/CodeGen/RegisterClassInfo.h"
 #include "llvm/CodeGen/VirtRegMap.h"
 #include "llvm/PassAnalysisSupport.h"
 #include "llvm/Support/CommandLine.h"
@@ -70,6 +71,11 @@ class RAGreedy : public MachineFunctionPass,
   // context
   MachineFunction *MF;
 
+  // Shortcuts to some useful interface.
+  const TargetInstrInfo *TII;
+  const TargetRegisterInfo *TRI;
+  RegisterClassInfo RCI;
+
   // analyses
   SlotIndexes *Indexes;
   MachineBlockFrequencyInfo *MBFI;
@@ -1368,6 +1374,22 @@ unsigned RAGreedy::tryBlockSplit(LiveInterval &VirtReg, AllocationOrder &Order,
 //                         Per-Instruction Splitting
 //===----------------------------------------------------------------------===//
 
+/// Get the number of allocatable registers that match the constraints of \p Reg
+/// on \p MI and that are also in \p SuperRC.
+static unsigned getNumAllocatableRegsForConstraints(
+    const MachineInstr *MI, unsigned Reg, const TargetRegisterClass *SuperRC,
+    const TargetInstrInfo *TII, const TargetRegisterInfo *TRI,
+    const RegisterClassInfo &RCI) {
+  assert(SuperRC && "Invalid register class");
+
+  const TargetRegisterClass *ConstrainedRC =
+      MI->getRegClassConstraintEffectForVReg(Reg, SuperRC, TII, TRI,
+                                             /* ExploreBundle */ true);
+  if (!ConstrainedRC)
+    return 0;
+  return RCI.getNumAllocatableRegs(ConstrainedRC);
+}
+
 /// tryInstructionSplit - Split a live range around individual instructions.
 /// This is normally not worthwhile since the spiller is doing essentially the
 /// same thing. However, when the live range is in a constrained register
@@ -1378,8 +1400,9 @@ unsigned RAGreedy::tryBlockSplit(LiveInterval &VirtReg, AllocationOrder &Order,
 unsigned
 RAGreedy::tryInstructionSplit(LiveInterval &VirtReg, AllocationOrder &Order,
                               SmallVectorImpl<unsigned> &NewVRegs) {
+  const TargetRegisterClass *CurRC = MRI->getRegClass(VirtReg.reg);
   // There is no point to this if there are no larger sub-classes.
-  if (!RegClassInfo.isProperSubClass(MRI->getRegClass(VirtReg.reg)))
+  if (!RegClassInfo.isProperSubClass(CurRC))
     return 0;
 
   // Always enable split spill mode, since we're effectively spilling to a
@@ -1393,10 +1416,18 @@ RAGreedy::tryInstructionSplit(LiveInterval &VirtReg, AllocationOrder &Order,
 
   DEBUG(dbgs() << "Split around " << Uses.size() << " individual instrs.\n");
 
-  // Split around every non-copy instruction.
+  const TargetRegisterClass *SuperRC = TRI->getLargestLegalSuperClass(CurRC);
+  unsigned SuperRCNumAllocatableRegs = RCI.getNumAllocatableRegs(SuperRC);
+  // Split around every non-copy instruction if this split will relax
+  // the constraints on the virtual register.
+  // Otherwise, splitting just inserts uncoalescable copies that do not help
+  // the allocation.
   for (unsigned i = 0; i != Uses.size(); ++i) {
     if (const MachineInstr *MI = Indexes->getInstructionFromIndex(Uses[i]))
-      if (MI->isFullCopy()) {
+      if (MI->isFullCopy() ||
+          SuperRCNumAllocatableRegs ==
+              getNumAllocatableRegsForConstraints(MI, VirtReg.reg, SuperRC, TII,
+                                                  TRI, RCI)) {
         DEBUG(dbgs() << "    skip:\t" << Uses[i] << '\t' << *MI);
         continue;
       }
@@ -1843,6 +1874,9 @@ bool RAGreedy::runOnMachineFunction(MachineFunction &mf) {
                << "********** Function: " << mf.getName() << '\n');
 
   MF = &mf;
+  TRI = MF->getTarget().getRegisterInfo();
+  TII = MF->getTarget().getInstrInfo();
+  RCI.runOnMachineFunction(mf);
   if (VerifyEnabled)
     MF->verify(this, "Before greedy register allocator");
 

test/CodeGen/X86/ins_split_regalloc.ll

@@ -0,0 +1,33 @@
+; RUN: llc -O1 -regalloc=greedy -mtriple=x86_64-apple-macosx -march x86-64  < %s -o - | FileCheck %s
+; Check that last chance split (RAGreedy::tryInstructonSplit) just split
+; when this is beneficial, otherwise we end up with uncoalesced copies.
+; <rdar://problem/15570057> 
+
+target datalayout = "e-i64:64-f80:128-s:64-n8:16:32:64-S128"
+
+@f = external constant void (i32)*
+
+; CHECK-LABEL: test:
+; Get the address of f in the GOT.
+; CHECK: movq _f@{{[^,]+}}, [[F_ENTRY_ADDR:%[a-z0-9]+]]
+; Read the actual address of f.
+; CHECK: movq ([[F_ENTRY_ADDR]]), [[F_ADDR:%[a-z0-9]+]]
+; Check that we do not have useless split points before each call.
+; CHECK-NOT: movq
+; CHECK: callq *[[F_ADDR]]
+; Check that we do not have useless split points before each call.
+; CHECK-NOT: movq
+; CHECK: callq *[[F_ADDR]]
+; Last call is a tail call, thus the address of the function cannot use
+; a callee saved register.
+; CHECK: movq [[F_ADDR]], [[F_ADDR_TC:%[a-z0-9]+]]
+; CHECK: popq [[F_ADDR]]
+; CHECK: jmpq *[[F_ADDR_TC]]
+define void @test(i32 %a, i32 %b, i32 %c) {
+entry:
+  %fct_f = load void (i32)** @f, align 8
+  tail call void %fct_f(i32 %a)
+  tail call void %fct_f(i32 %b)
+  tail call void %fct_f(i32 %c)
+  ret void
+}