Refactor LinearScan::scan from one huge function into smaller functions.

src/IceRegAlloc.cpp

Index: src/IceRegAlloc.cpp
diff --git a/src/IceRegAlloc.cpp b/src/IceRegAlloc.cpp
index bc0643e7fd1f89f7760ee4a6ad04e587529cc8a6..4cfcceb4bcac3d39592c8782edd2e29e20b30607 100644
--- a/src/IceRegAlloc.cpp
+++ b/src/IceRegAlloc.cpp
@@ -8,9 +8,8 @@
 //===----------------------------------------------------------------------===//
 ///
 /// \file
-/// This file implements the LinearScan class, which performs the
-/// linear-scan register allocation after liveness analysis has been
-/// performed.
+/// This file implements the LinearScan class, which performs the linear-scan

[Jim Stichnoth, 2015/08/24 15:08:30]

While you're at it, could you reformat all the other comment blocks in this file
to use the full 80 columns?  (Looks like that's already done for the .h file.)

[ascull, 2015/08/24 19:53:47]

Done.

+/// register allocation after liveness analysis has been performed.
 ///
 //===----------------------------------------------------------------------===//
 
@@ -26,16 +25,12 @@ namespace Ice {
 
 namespace {
 
-// TODO(stichnot): Statically choose the size based on the target
-// being compiled.
-constexpr size_t REGS_SIZE = 32;
-
 // Returns true if Var has any definitions within Item's live range.
-// TODO(stichnot): Consider trimming the Definitions list similar to
-// how the live ranges are trimmed, since all the overlapsDefs() tests
-// are whether some variable's definitions overlap Cur, and trimming
-// is with respect Cur.start.  Initial tests show no measurable
-// performance difference, so we'll keep the code simple for now.
+// TODO(stichnot): Consider trimming the Definitions list similar to how the
+// live ranges are trimmed, since all the overlapsDefs() tests are whether some
+// variable's definitions overlap Cur, and trimming is with respect Cur.start.
+// Initial tests show no measurable performance difference, so we'll keep the
+// code simple for now.
 bool overlapsDefs(const Cfg *Func, const Variable *Item, const Variable *Var) {
   constexpr bool UseTrimmed = true;
   VariablesMetadata *VMetadata = Func->getVMetadata();
@@ -82,6 +77,10 @@ void dumpLiveRange(const Variable *Var, const Cfg *Func) {
 
 } // end of anonymous namespace
 
+LinearScan::LinearScan(Cfg *Func)
+    : Func(Func), Ctx(Func->getContext()),
+      Verbose(BuildDefs::dump() && Func->isVerbose(IceV_LinearScan)) {}
+
 // Prepare for full register allocation of all variables.  We depend
 // on liveness analysis to have calculated live ranges.
 void LinearScan::initForGlobal() {
@@ -336,27 +335,378 @@ void LinearScan::addSpillFill(Variable *Cur, llvm::SmallBitVector RegMask) {
   Target->lowerInst(Node, FillPoint, InstFakeUse::create(Func, Preg));
 }
 
-// Implements the linear-scan algorithm.  Based on "Linear Scan
-// Register Allocation in the Context of SSA Form and Register
-// Constraints" by Hanspeter Mössenböck and Michael Pfeiffer,
-// ftp://ftp.ssw.uni-linz.ac.at/pub/Papers/Moe02.PDF .  This
-// implementation is modified to take affinity into account and allow
-// two interfering variables to share the same register in certain
-// cases.
+void LinearScan::handleActiveRangeExpiredOrInactive(Variable *Cur) {

[Jim Stichnoth, 2015/08/24 15:08:30]

const Variable *Cur

[ascull, 2015/08/24 19:53:47]

Done.

+  for (SizeT I = Active.size(); I > 0; --I) {
+    const SizeT Index = I - 1;
+    Variable *Item = Active[Index];
+    Item->trimLiveRange(Cur->getLiveRange().getStart());
+    bool Moved = false;
+    if (Item->rangeEndsBefore(Cur)) {
+      // Move Item from Active to Handled list.
+      if (Verbose) {

[Jim Stichnoth, 2015/08/24 15:08:31]

Could you change every instance of "if (Verbose)" to "if (isVerbose())"?  Then
create an isVerbose method in the .h file:

  bool isVerbose() const {
    return BuildDefs::dump() && Verbose;
  }

The idea being that all the stream code should disappear from the MINIMAL build.

Going a bit further, it looks like almost every instance of "if (Verbose)"
guards the same 4 lines of code except for the string constant, so there's
another opportunity to refactor.

[ascull, 2015/08/24 19:53:47]

The initializer for Verbose includes `BuildDefs::dump()` already and given this
is a compile time constant there doesn't seem to be a need for a function call.

I have refactored the 4 lines into `dumpLiveRangeTrace`.

+        Ostream &Str = Ctx->getStrDump();
+        Str << "Expiring     ";
+        dumpLiveRange(Item, Func);
+        Str << "\n";
+      }
+      moveItem(Active, Index, Handled);
+      Moved = true;
+    } else if (!Item->rangeOverlapsStart(Cur)) {
+      // Move Item from Active to Inactive list.
+      if (Verbose) {
+        Ostream &Str = Ctx->getStrDump();
+        Str << "Inactivating ";
+        dumpLiveRange(Item, Func);
+        Str << "\n";
+      }
+      moveItem(Active, Index, Inactive);
+      Moved = true;
+    }
+    if (Moved) {
+      // Decrement Item from RegUses[].
+      assert(Item->hasRegTmp());
+      int32_t RegNum = Item->getRegNumTmp();
+      --RegUses[RegNum];
+      assert(RegUses[RegNum] >= 0);
+    }
+  }
+}
+
+void LinearScan::handleInctiveRangeExpiredOrReactived(Variable *Cur) {

[Jim Stichnoth, 2015/08/24 15:08:31]

const Variable *Cur

[ascull, 2015/08/24 19:53:47]

Done.

+  for (SizeT I = Inactive.size(); I > 0; --I) {
+    const SizeT Index = I - 1;
+    Variable *Item = Inactive[Index];
+    Item->trimLiveRange(Cur->getLiveRange().getStart());
+    if (Item->rangeEndsBefore(Cur)) {
+      // Move Item from Inactive to Handled list.
+      if (Verbose) {
+        Ostream &Str = Ctx->getStrDump();
+        Str << "Expiring     ";
+        dumpLiveRange(Item, Func);
+        Str << "\n";
+      }
+      moveItem(Inactive, Index, Handled);
+    } else if (Item->rangeOverlapsStart(Cur)) {
+      // Move Item from Inactive to Active list.
+      if (Verbose) {
+        Ostream &Str = Ctx->getStrDump();
+        Str << "Reactivating ";
+        dumpLiveRange(Item, Func);
+        Str << "\n";
+      }
+      moveItem(Inactive, Index, Active);
+      // Increment Item in RegUses[].
+      assert(Item->hasRegTmp());
+      int32_t RegNum = Item->getRegNumTmp();
+      assert(RegUses[RegNum] >= 0);
+      ++RegUses[RegNum];
+    }
+  }
+}
+
+// Infer register preference and allowable overlap. Only form a preference when
+// the current Variable has an unambiguous "first" definition. The preference
+// is some source Variable of the defining instruction that either is assigned
+// a register that is currently free, or that is assigned a register that is
+// not free but overlap is allowed. Overlap is allowed when the Variable under
+// consideration is single-definition, and its definition is a simple
+// assignment - i.e., the register gets copied/aliased but is never modified.
+// Furthermore, overlap is only allowed when preferred Variable definition
+// instructions do not appear within the current Variable's live range.
+void LinearScan::findRegisterPreference(Variable *Cur,

[Jim Stichnoth, 2015/08/24 15:08:31]

const Variable *Cur

+                                        llvm::SmallBitVector RegMask) {
+  Prefer = nullptr;
+  PreferReg = Variable::NoRegister;
+  AllowOverlap = false;
+
+  if (FindPreference) {
+    VariablesMetadata *VMetadata = Func->getVMetadata();
+    if (const Inst *DefInst = VMetadata->getFirstDefinition(Cur)) {
+      assert(DefInst->getDest() == Cur);
+      bool IsAssign = DefInst->isSimpleAssign();
+      bool IsSingleDef = !VMetadata->isMultiDef(Cur);
+      for (SizeT i = 0; i < DefInst->getSrcSize(); ++i) {
+        // TODO(stichnot): Iterate through the actual Variables of the
+        // instruction, not just the source operands. This could capture Load
+        // instructions, including address mode optimization, for Prefer (but
+        // not for AllowOverlap).
+        if (Variable *SrcVar = llvm::dyn_cast<Variable>(DefInst->getSrc(i))) {
+          int32_t SrcReg = SrcVar->getRegNumTmp();
+          // Only consider source variables that have (so far) been assigned a
+          // register. That register must be one in the RegMask set, e.g.
+          // don't try to prefer the stack pointer as a result of the stacksave
+          // intrinsic.
+          if (SrcVar->hasRegTmp() && RegMask[SrcReg]) {
+            if (FindOverlap && !Free[SrcReg]) {
+              // Don't bother trying to enable AllowOverlap if the register is
+              // already free.
+              AllowOverlap =
+                  IsSingleDef && IsAssign && !overlapsDefs(Func, Cur, SrcVar);
+            }
+            if (AllowOverlap || Free[SrcReg]) {
+              Prefer = SrcVar;
+              PreferReg = SrcReg;
+            }
+          }
+        }
+      }
+      if (Verbose && Prefer) {
+        Ostream &Str = Ctx->getStrDump();
+        Str << "Initial Prefer=";
+        Prefer->dump(Func);
+        Str << " R=" << PreferReg << " LIVE=" << Prefer->getLiveRange()
+            << " Overlap=" << AllowOverlap << "\n";
+      }
+    }
+  }
+}
+
+// Remove registers from the Free[] list where an Inactive range overlaps with
+// the current range.
+void LinearScan::filterFreeWithInactiveRanges(Variable *Cur) {

[Jim Stichnoth, 2015/08/24 15:08:30]

const Variable *Cur

+  for (const Variable *Item : Inactive) {
+    if (Item->rangeOverlaps(Cur)) {
+      int32_t RegNum = Item->getRegNumTmp();
+      // Don't assert(Free[RegNum]) because in theory (though probably never in
+      // practice) there could be two inactive variables that were marked with
+      // AllowOverlap.
+      Free[RegNum] = false;
+      // Disable AllowOverlap if an Inactive variable, which is not Prefer,
+      // shares Prefer's register, and has a definition within Cur's live
+      // range.
+      if (AllowOverlap && Item != Prefer && RegNum == PreferReg &&
+          overlapsDefs(Func, Cur, Item)) {
+        AllowOverlap = false;
+        dumpDisableOverlap(Func, Item, "Inactive");
+      }
+    }
+  }
+}
+
+// Remove registers from the Free[] list where an Unhandled pre-colored range
+// overlaps with the current range, and set those registers to infinite weight
+// so that they aren't candidates for eviction.  Cur->rangeEndsBefore(Item) is
+// an early exit check that turns a guaranteed O(N^2) algorithm into expected
+// linear complexity.
+void LinearScan::filterFreeWithPrecoloredRanges(Variable *Cur) {

[Jim Stichnoth, 2015/08/24 15:08:30]

const Variable *Cur

+  for (Variable *Item : reverse_range(UnhandledPrecolored)) {
+    assert(Item->hasReg());
+    if (Cur->rangeEndsBefore(Item))
+      break;
+    if (Item->rangeOverlaps(Cur)) {
+      int32_t ItemReg = Item->getRegNum(); // Note: not getRegNumTmp()
+      Weights[ItemReg].setWeight(RegWeight::Inf);
+      Free[ItemReg] = false;
+      PrecoloredUnhandledMask[ItemReg] = true;
+      // Disable AllowOverlap if the preferred register is one of
+      // these pre-colored unhandled overlapping ranges.
+      if (AllowOverlap && ItemReg == PreferReg) {
+        AllowOverlap = false;
+        dumpDisableOverlap(Func, Item, "PrecoloredUnhandled");
+      }
+    }
+  }
+}
+
+void LinearScan::allocatePrecoloredRegister(Variable *Cur) {
+  int32_t RegNum = Cur->getRegNum();
+  // RegNumTmp should have already been set above.
+  assert(Cur->getRegNumTmp() == RegNum);
+  if (Verbose) {
+    Ostream &Str = Ctx->getStrDump();
+    Str << "Precoloring  ";
+    dumpLiveRange(Cur, Func);
+    Str << "\n";
+  }
+  Active.push_back(Cur);
+  assert(RegUses[RegNum] >= 0);
+  ++RegUses[RegNum];
+  assert(!UnhandledPrecolored.empty());
+  assert(UnhandledPrecolored.back() == Cur);
+  UnhandledPrecolored.pop_back();
+}
+
+void LinearScan::allocatePreferedRegister(Variable *Cur) {
+  Cur->setRegNumTmp(PreferReg);
+  if (Verbose) {
+    Ostream &Str = Ctx->getStrDump();
+    Str << "Preferring   ";
+    dumpLiveRange(Cur, Func);
+    Str << "\n";
+  }
+  assert(RegUses[PreferReg] >= 0);
+  ++RegUses[PreferReg];
+  Active.push_back(Cur);
+}
+
+void LinearScan::allocateFreeRegister(Variable *Cur) {
+  int32_t RegNum = Free.find_first();
+  Cur->setRegNumTmp(RegNum);
+  if (Verbose) {
+    Ostream &Str = Ctx->getStrDump();
+    Str << "Allocating   ";
+    dumpLiveRange(Cur, Func);
+    Str << "\n";
+  }
+  assert(RegUses[RegNum] >= 0);
+  ++RegUses[RegNum];
+  Active.push_back(Cur);
+}
+
+void LinearScan::handleNoFreeRegisters(Variable *Cur,
+                                       llvm::SmallBitVector RegMask) {
+  // Check Active ranges.
+  for (const Variable *Item : Active) {
+    assert(Item->rangeOverlaps(Cur));
+    int32_t RegNum = Item->getRegNumTmp();
+    assert(Item->hasRegTmp());
+    Weights[RegNum].addWeight(Item->getLiveRange().getWeight());
+  }
+  // Same as above, but check Inactive ranges instead of Active.
+  for (const Variable *Item : Inactive) {
+    int32_t RegNum = Item->getRegNumTmp();
+    assert(Item->hasRegTmp());
+    if (Item->rangeOverlaps(Cur))
+      Weights[RegNum].addWeight(Item->getLiveRange().getWeight());
+  }
+
+  // All the weights are now calculated.  Find the register with smallest
+  // weight.
+  int32_t MinWeightIndex = RegMask.find_first();
+  // MinWeightIndex must be valid because of the initial RegMask.any() test.
+  assert(MinWeightIndex >= 0);
+  for (SizeT i = MinWeightIndex + 1; i < Weights.size(); ++i) {
+    if (RegMask[i] && Weights[i] < Weights[MinWeightIndex])
+      MinWeightIndex = i;
+  }
+
+  if (Cur->getLiveRange().getWeight() <= Weights[MinWeightIndex]) {
+    // Cur doesn't have priority over any other live ranges, so don't allocate
+    // any register to it, and move it to the Handled state.
+    Handled.push_back(Cur);
+    if (Cur->getLiveRange().getWeight().isInf()) {
+      if (Kind == RAK_Phi)
+        addSpillFill(Cur, RegMask);
+      else
+        Func->setError("Unable to find a physical register for an "
+                       "infinite-weight live range");
+    }
+  } else {
+    // Evict all live ranges in Active that register number MinWeightIndex is
+    // assigned to.
+    for (SizeT I = Active.size(); I > 0; --I) {
+      const SizeT Index = I - 1;
+      Variable *Item = Active[Index];
+      if (Item->getRegNumTmp() == MinWeightIndex) {
+        if (Verbose) {
+          Ostream &Str = Ctx->getStrDump();
+          Str << "Evicting     ";
+          dumpLiveRange(Item, Func);
+          Str << "\n";
+        }
+        --RegUses[MinWeightIndex];
+        assert(RegUses[MinWeightIndex] >= 0);
+        Item->setRegNumTmp(Variable::NoRegister);
+        moveItem(Active, Index, Handled);
+      }
+    }
+    // Do the same for Inactive.
+    for (SizeT I = Inactive.size(); I > 0; --I) {
+      const SizeT Index = I - 1;
+      Variable *Item = Inactive[Index];
+      // Note: The Item->rangeOverlaps(Cur) clause is not part of the
+      // description of AssignMemLoc() in the original paper.  But there
+      // doesn't seem to be any need to evict an inactive live range that
+      // doesn't overlap with the live range currently being considered. It's
+      // especially bad if we would end up evicting an infinite-weight but
+      // currently-inactive live range. The most common situation for this
+      // would be a scratch register kill set for call instructions.
+      if (Item->getRegNumTmp() == MinWeightIndex && Item->rangeOverlaps(Cur)) {
+        if (Verbose) {
+          Ostream &Str = Ctx->getStrDump();
+          Str << "Evicting     ";
+          dumpLiveRange(Item, Func);
+          Str << "\n";
+        }
+        Item->setRegNumTmp(Variable::NoRegister);
+        moveItem(Inactive, Index, Handled);
+      }
+    }
+    // Assign the register to Cur.
+    Cur->setRegNumTmp(MinWeightIndex);
+    assert(RegUses[MinWeightIndex] >= 0);
+    ++RegUses[MinWeightIndex];
+    Active.push_back(Cur);
+    if (Verbose) {
+      Ostream &Str = Ctx->getStrDump();
+      Str << "Allocating   ";
+      dumpLiveRange(Cur, Func);
+      Str << "\n";
+    }
+  }
+}
+
+void LinearScan::assignFinalRegisters(llvm::SmallBitVector RegMaskFull,
+                                      llvm::SmallBitVector PreDefinedRegisters,
+                                      bool Randomized) {
+  const size_t NumRegisters = RegMaskFull.size();
+  llvm::SmallVector<int32_t, REGS_SIZE> Permutation(NumRegisters);
+  if (Randomized) {
+    // Create a random number generator for regalloc randomization. Merge
+    // function's sequence and Kind value as the Salt. Because regAlloc() is
+    // called twice under O2, the second time with RAK_Phi, we check
+    // Kind == RAK_Phi to determine the lowest-order bit to make sure the Salt
+    // is different.
+    uint64_t Salt =
+        (Func->getSequenceNumber() << 1) ^ (Kind == RAK_Phi ? 0u : 1u);
+    Func->getTarget()->makeRandomRegisterPermutation(
+        Permutation, PreDefinedRegisters | ~RegMaskFull, Salt);
+  }
+
+  // Finish up by assigning RegNumTmp->RegNum (or a random permutation thereof)

[jvoung (off chromium), 2015/08/24 15:33:01]

nit: I think this was from the original comment, but maybe make
"RegNumTmp->RegNum" look less like a class member field load? (more space ?)

[ascull, 2015/08/24 19:53:47]

Done.

+  // for each Variable.
+  for (Variable *Item : Handled) {
+    int32_t RegNum = Item->getRegNumTmp();
+    int32_t AssignedRegNum = RegNum;
+
+    if (Randomized && Item->hasRegTmp() && !Item->hasReg()) {
+      AssignedRegNum = Permutation[RegNum];
+    }
+    if (Verbose) {
+      Ostream &Str = Ctx->getStrDump();
+      if (!Item->hasRegTmp()) {
+        Str << "Not assigning ";
+        Item->dump(Func);
+        Str << "\n";
+      } else {
+        Str << (AssignedRegNum == Item->getRegNum() ? "Reassigning "
+                                                    : "Assigning ")
+            << Func->getTarget()->getRegName(AssignedRegNum, IceType_i32)
+            << "(r" << AssignedRegNum << ") to ";
+        Item->dump(Func);
+        Str << "\n";
+      }
+    }
+    Item->setRegNum(AssignedRegNum);
+  }
+}
+
+// Implements the linear-scan algorithm. Based on "Linear Scan Register
+// Allocation in the Context of SSA Form and Register Constraints" by Hanspeter
+// Mössenböck and Michael Pfeiffer,
+// ftp://ftp.ssw.uni-linz.ac.at/pub/Papers/Moe02.PDF. This implementation is
+// modified to take affinity into account and allow two interfering variables
+// to share the same register in certain cases.
 //
-// Requires running Cfg::liveness(Liveness_Intervals) in
-// preparation.  Results are assigned to Variable::RegNum for each
-// Variable.
+// Requires running Cfg::liveness(Liveness_Intervals) in preparation. Results
+// are assigned to Variable::RegNum for each Variable.
 void LinearScan::scan(const llvm::SmallBitVector &RegMaskFull,
                       bool Randomized) {
   TimerMarker T(TimerStack::TT_linearScan, Func);
   assert(RegMaskFull.any()); // Sanity check
-  GlobalContext *Ctx = Func->getContext();
-  const bool Verbose = BuildDefs::dump() && Func->isVerbose(IceV_LinearScan);
   if (Verbose)
     Ctx->lockStr();
   Func->resetCurrentNode();
-  VariablesMetadata *VMetadata = Func->getVMetadata();
   const size_t NumRegisters = RegMaskFull.size();
   llvm::SmallBitVector PreDefinedRegisters(NumRegisters);
   if (Randomized) {
@@ -369,12 +719,12 @@ void LinearScan::scan(const llvm::SmallBitVector &RegMaskFull,
   LiveRange KillsRange(Kills);
   KillsRange.untrim();
 
-  // RegUses[I] is the number of live ranges (variables) that register
-  // I is currently assigned to.  It can be greater than 1 as a result
-  // of AllowOverlap inference below.
-  llvm::SmallVector<int, REGS_SIZE> RegUses(NumRegisters);
-  // Unhandled is already set to all ranges in increasing order of
-  // start points.
+  // Reset the register use count
+  RegUses.resize(NumRegisters);
+  std::fill(RegUses.begin(), RegUses.end(), 0);
+
+  // Unhandled is already set to all ranges in increasing order of start
+  // points.
   assert(Active.empty());
   assert(Inactive.empty());
   assert(Handled.empty());
@@ -397,177 +747,27 @@ void LinearScan::scan(const llvm::SmallBitVector &RegMaskFull,
         RegMaskFull & Func->getTarget()->getRegisterSetForType(Cur->getType());
     KillsRange.trim(Cur->getLiveRange().getStart());
 
-    // Check for pre-colored ranges.  If Cur is pre-colored, it
-    // definitely gets that register.  Previously processed live
-    // ranges would have avoided that register due to it being
-    // pre-colored.  Future processed live ranges won't evict that
-    // register because the live range has infinite weight.
+    // Check for pre-colored ranges. If Cur is pre-colored, it definitely gets
+    // that register. Previously processed live ranges would have avoided that
+    // register due to it being pre-colored. Future processed live ranges won't
+    // evict that register because the live range has infinite weight.
     if (Cur->hasReg()) {
-      int32_t RegNum = Cur->getRegNum();
-      // RegNumTmp should have already been set above.
-      assert(Cur->getRegNumTmp() == RegNum);
-      if (Verbose) {
-        Ostream &Str = Ctx->getStrDump();
-        Str << "Precoloring  ";
-        dumpLiveRange(Cur, Func);
-        Str << "\n";
-      }
-      Active.push_back(Cur);
-      assert(RegUses[RegNum] >= 0);
-      ++RegUses[RegNum];
-      assert(!UnhandledPrecolored.empty());
-      assert(UnhandledPrecolored.back() == Cur);
-      UnhandledPrecolored.pop_back();
+      allocatePrecoloredRegister(Cur);
       continue;
     }
 
-    // Check for active ranges that have expired or become inactive.
-    for (SizeT I = Active.size(); I > 0; --I) {
-      const SizeT Index = I - 1;
-      Variable *Item = Active[Index];
-      Item->trimLiveRange(Cur->getLiveRange().getStart());
-      bool Moved = false;
-      if (Item->rangeEndsBefore(Cur)) {
-        // Move Item from Active to Handled list.
-        if (Verbose) {
-          Ostream &Str = Ctx->getStrDump();
-          Str << "Expiring     ";
-          dumpLiveRange(Item, Func);
-          Str << "\n";
-        }
-        moveItem(Active, Index, Handled);
-        Moved = true;
-      } else if (!Item->rangeOverlapsStart(Cur)) {
-        // Move Item from Active to Inactive list.
-        if (Verbose) {
-          Ostream &Str = Ctx->getStrDump();
-          Str << "Inactivating ";
-          dumpLiveRange(Item, Func);
-          Str << "\n";
-        }
-        moveItem(Active, Index, Inactive);
-        Moved = true;
-      }
-      if (Moved) {
-        // Decrement Item from RegUses[].
-        assert(Item->hasRegTmp());
-        int32_t RegNum = Item->getRegNumTmp();
-        --RegUses[RegNum];
-        assert(RegUses[RegNum] >= 0);
-      }
-    }
-
-    // Check for inactive ranges that have expired or reactivated.
-    for (SizeT I = Inactive.size(); I > 0; --I) {
-      const SizeT Index = I - 1;
-      Variable *Item = Inactive[Index];
-      Item->trimLiveRange(Cur->getLiveRange().getStart());
-      if (Item->rangeEndsBefore(Cur)) {
-        // Move Item from Inactive to Handled list.
-        if (Verbose) {
-          Ostream &Str = Ctx->getStrDump();
-          Str << "Expiring     ";
-          dumpLiveRange(Item, Func);
-          Str << "\n";
-        }
-        moveItem(Inactive, Index, Handled);
-      } else if (Item->rangeOverlapsStart(Cur)) {
-        // Move Item from Inactive to Active list.
-        if (Verbose) {
-          Ostream &Str = Ctx->getStrDump();
-          Str << "Reactivating ";
-          dumpLiveRange(Item, Func);
-          Str << "\n";
-        }
-        moveItem(Inactive, Index, Active);
-        // Increment Item in RegUses[].
-        assert(Item->hasRegTmp());
-        int32_t RegNum = Item->getRegNumTmp();
-        assert(RegUses[RegNum] >= 0);
-        ++RegUses[RegNum];
-      }
-    }
+    handleActiveRangeExpiredOrInactive(Cur);
+    handleInctiveRangeExpiredOrReactived(Cur);
 
     // Calculate available registers into Free[].
-    llvm::SmallBitVector Free = RegMask;
+    Free = RegMask;
     for (SizeT i = 0; i < RegMask.size(); ++i) {
       if (RegUses[i] > 0)
         Free[i] = false;
     }
 
-    // Infer register preference and allowable overlap.  Only form a
-    // preference when the current Variable has an unambiguous "first"
-    // definition.  The preference is some source Variable of the
-    // defining instruction that either is assigned a register that is
-    // currently free, or that is assigned a register that is not free
-    // but overlap is allowed.  Overlap is allowed when the Variable
-    // under consideration is single-definition, and its definition is
-    // a simple assignment - i.e., the register gets copied/aliased
-    // but is never modified.  Furthermore, overlap is only allowed
-    // when preferred Variable definition instructions do not appear
-    // within the current Variable's live range.
-    Variable *Prefer = nullptr;
-    int32_t PreferReg = Variable::NoRegister;
-    bool AllowOverlap = false;
-    if (FindPreference) {
-      if (const Inst *DefInst = VMetadata->getFirstDefinition(Cur)) {
-        assert(DefInst->getDest() == Cur);
-        bool IsAssign = DefInst->isSimpleAssign();
-        bool IsSingleDef = !VMetadata->isMultiDef(Cur);
-        for (SizeT i = 0; i < DefInst->getSrcSize(); ++i) {
-          // TODO(stichnot): Iterate through the actual Variables of the
-          // instruction, not just the source operands.  This could
-          // capture Load instructions, including address mode
-          // optimization, for Prefer (but not for AllowOverlap).
-          if (Variable *SrcVar = llvm::dyn_cast<Variable>(DefInst->getSrc(i))) {
-            int32_t SrcReg = SrcVar->getRegNumTmp();
-            // Only consider source variables that have (so far) been
-            // assigned a register.  That register must be one in the
-            // RegMask set, e.g. don't try to prefer the stack pointer
-            // as a result of the stacksave intrinsic.
-            if (SrcVar->hasRegTmp() && RegMask[SrcReg]) {
-              if (FindOverlap && !Free[SrcReg]) {
-                // Don't bother trying to enable AllowOverlap if the
-                // register is already free.
-                AllowOverlap =
-                    IsSingleDef && IsAssign && !overlapsDefs(Func, Cur, SrcVar);
-              }
-              if (AllowOverlap || Free[SrcReg]) {
-                Prefer = SrcVar;
-                PreferReg = SrcReg;
-              }
-            }
-          }
-        }
-        if (Verbose && Prefer) {
-          Ostream &Str = Ctx->getStrDump();
-          Str << "Initial Prefer=";
-          Prefer->dump(Func);
-          Str << " R=" << PreferReg << " LIVE=" << Prefer->getLiveRange()
-              << " Overlap=" << AllowOverlap << "\n";
-        }
-      }
-    }
-
-    // Remove registers from the Free[] list where an Inactive range
-    // overlaps with the current range.
-    for (const Variable *Item : Inactive) {
-      if (Item->rangeOverlaps(Cur)) {
-        int32_t RegNum = Item->getRegNumTmp();
-        // Don't assert(Free[RegNum]) because in theory (though
-        // probably never in practice) there could be two inactive
-        // variables that were marked with AllowOverlap.
-        Free[RegNum] = false;
-        // Disable AllowOverlap if an Inactive variable, which is not
-        // Prefer, shares Prefer's register, and has a definition
-        // within Cur's live range.
-        if (AllowOverlap && Item != Prefer && RegNum == PreferReg &&
-            overlapsDefs(Func, Cur, Item)) {
-          AllowOverlap = false;
-          dumpDisableOverlap(Func, Item, "Inactive");
-        }
-      }
-    }
+    findRegisterPreference(Cur, RegMask);
+    filterFreeWithInactiveRanges(Cur);
 
     // Disable AllowOverlap if an Active variable, which is not
     // Prefer, shares Prefer's register, and has a definition within
@@ -583,33 +783,13 @@ void LinearScan::scan(const llvm::SmallBitVector &RegMaskFull,
       }
     }
 
-    llvm::SmallVector<RegWeight, REGS_SIZE> Weights(RegMask.size());
-
-    // Remove registers from the Free[] list where an Unhandled
-    // pre-colored range overlaps with the current range, and set those
-    // registers to infinite weight so that they aren't candidates for
-    // eviction.  Cur->rangeEndsBefore(Item) is an early exit check
-    // that turns a guaranteed O(N^2) algorithm into expected linear
-    // complexity.
-    llvm::SmallBitVector PrecoloredUnhandledMask(RegMask.size());
-    // Note: PrecoloredUnhandledMask is only used for dumping.
-    for (Variable *Item : reverse_range(UnhandledPrecolored)) {
-      assert(Item->hasReg());
-      if (Cur->rangeEndsBefore(Item))
-        break;
-      if (Item->rangeOverlaps(Cur)) {
-        int32_t ItemReg = Item->getRegNum(); // Note: not getRegNumTmp()
-        Weights[ItemReg].setWeight(RegWeight::Inf);
-        Free[ItemReg] = false;
-        PrecoloredUnhandledMask[ItemReg] = true;
-        // Disable AllowOverlap if the preferred register is one of
-        // these pre-colored unhandled overlapping ranges.
-        if (AllowOverlap && ItemReg == PreferReg) {
-          AllowOverlap = false;
-          dumpDisableOverlap(Func, Item, "PrecoloredUnhandled");
-        }
-      }
-    }
+    Weights.resize(RegMask.size());

[Jim Stichnoth, 2015/08/24 15:08:30]

It seems to me that Weights and PrecoloredUnhandledMask could be resized once
outside the loop.  Maybe to NumRegisters?

[ascull, 2015/08/24 19:53:47]

The resize must be in the loops .size() is used by a for loop. I have put a
reserve outside of the loop so memory allocation will not happen inside the
loop.

+    std::fill(Weights.begin(), Weights.end(), RegWeight());
+
+    PrecoloredUnhandledMask.resize(RegMask.size());
+    PrecoloredUnhandledMask.reset();
+
+    filterFreeWithPrecoloredRanges(Cur);
 
     // Remove scratch registers from the Free[] list, and mark their
     // Weights[] as infinite, if KillsRange overlaps Cur's live range.
@@ -638,181 +818,30 @@ void LinearScan::scan(const llvm::SmallBitVector &RegMaskFull,
     }
 
     if (Prefer && (AllowOverlap || Free[PreferReg])) {
-      // First choice: a preferred register that is either free or is
-      // allowed to overlap with its linked variable.
-      Cur->setRegNumTmp(PreferReg);
-      if (Verbose) {
-        Ostream &Str = Ctx->getStrDump();
-        Str << "Preferring   ";
-        dumpLiveRange(Cur, Func);
-        Str << "\n";
-      }
-      assert(RegUses[PreferReg] >= 0);
-      ++RegUses[PreferReg];
-      Active.push_back(Cur);
+      // First choice: a preferred register that is either free or is allowed
+      // to overlap with its linked variable.
+      allocatePreferedRegister(Cur);
     } else if (Free.any()) {
-      // Second choice: any free register.  TODO: After explicit
-      // affinity is considered, is there a strategy better than just
-      // picking the lowest-numbered available register?
-      int32_t RegNum = Free.find_first();
-      Cur->setRegNumTmp(RegNum);
-      if (Verbose) {
-        Ostream &Str = Ctx->getStrDump();
-        Str << "Allocating   ";
-        dumpLiveRange(Cur, Func);
-        Str << "\n";
-      }
-      assert(RegUses[RegNum] >= 0);
-      ++RegUses[RegNum];
-      Active.push_back(Cur);
+      // Second choice: any free register. TODO: After explicit affinity is

[Jim Stichnoth, 2015/08/24 15:08:30]

I think this TODO should be removed.

[ascull, 2015/08/24 19:53:47]

Done.

+      // considered, is there a strategy better than just picking the
+      // lowest-numbered available register?
+      allocateFreeRegister(Cur);
     } else {
       // Fallback: there are no free registers, so we look for the
       // lowest-weight register and see if Cur has higher weight.
-      // Check Active ranges.
-      for (const Variable *Item : Active) {
-        assert(Item->rangeOverlaps(Cur));
-        int32_t RegNum = Item->getRegNumTmp();
-        assert(Item->hasRegTmp());
-        Weights[RegNum].addWeight(Item->getLiveRange().getWeight());
-      }
-      // Same as above, but check Inactive ranges instead of Active.
-      for (const Variable *Item : Inactive) {
-        int32_t RegNum = Item->getRegNumTmp();
-        assert(Item->hasRegTmp());
-        if (Item->rangeOverlaps(Cur))
-          Weights[RegNum].addWeight(Item->getLiveRange().getWeight());
-      }
-
-      // All the weights are now calculated.  Find the register with
-      // smallest weight.
-      int32_t MinWeightIndex = RegMask.find_first();
-      // MinWeightIndex must be valid because of the initial
-      // RegMask.any() test.
-      assert(MinWeightIndex >= 0);
-      for (SizeT i = MinWeightIndex + 1; i < Weights.size(); ++i) {
-        if (RegMask[i] && Weights[i] < Weights[MinWeightIndex])
-          MinWeightIndex = i;
-      }
-
-      if (Cur->getLiveRange().getWeight() <= Weights[MinWeightIndex]) {
-        // Cur doesn't have priority over any other live ranges, so
-        // don't allocate any register to it, and move it to the
-        // Handled state.
-        Handled.push_back(Cur);
-        if (Cur->getLiveRange().getWeight().isInf()) {
-          if (Kind == RAK_Phi)
-            addSpillFill(Cur, RegMask);
-          else
-            Func->setError("Unable to find a physical register for an "
-                           "infinite-weight live range");
-        }
-      } else {
-        // Evict all live ranges in Active that register number
-        // MinWeightIndex is assigned to.
-        for (SizeT I = Active.size(); I > 0; --I) {
-          const SizeT Index = I - 1;
-          Variable *Item = Active[Index];
-          if (Item->getRegNumTmp() == MinWeightIndex) {
-            if (Verbose) {
-              Ostream &Str = Ctx->getStrDump();
-              Str << "Evicting     ";
-              dumpLiveRange(Item, Func);
-              Str << "\n";
-            }
-            --RegUses[MinWeightIndex];
-            assert(RegUses[MinWeightIndex] >= 0);
-            Item->setRegNumTmp(Variable::NoRegister);
-            moveItem(Active, Index, Handled);
-          }
-        }
-        // Do the same for Inactive.
-        for (SizeT I = Inactive.size(); I > 0; --I) {
-          const SizeT Index = I - 1;
-          Variable *Item = Inactive[Index];
-          // Note: The Item->rangeOverlaps(Cur) clause is not part of the
-          // description of AssignMemLoc() in the original paper.  But
-          // there doesn't seem to be any need to evict an inactive
-          // live range that doesn't overlap with the live range
-          // currently being considered.  It's especially bad if we
-          // would end up evicting an infinite-weight but
-          // currently-inactive live range.  The most common situation
-          // for this would be a scratch register kill set for call
-          // instructions.
-          if (Item->getRegNumTmp() == MinWeightIndex &&
-              Item->rangeOverlaps(Cur)) {
-            if (Verbose) {
-              Ostream &Str = Ctx->getStrDump();
-              Str << "Evicting     ";
-              dumpLiveRange(Item, Func);
-              Str << "\n";
-            }
-            Item->setRegNumTmp(Variable::NoRegister);
-            moveItem(Inactive, Index, Handled);
-          }
-        }
-        // Assign the register to Cur.
-        Cur->setRegNumTmp(MinWeightIndex);
-        assert(RegUses[MinWeightIndex] >= 0);
-        ++RegUses[MinWeightIndex];
-        Active.push_back(Cur);
-        if (Verbose) {
-          Ostream &Str = Ctx->getStrDump();
-          Str << "Allocating   ";
-          dumpLiveRange(Cur, Func);
-          Str << "\n";
-        }
-      }
+      handleNoFreeRegisters(Cur, RegMask);
     }
     dump(Func);
   }
+
   // Move anything Active or Inactive to Handled for easier handling.
-  for (Variable *I : Active)
-    Handled.push_back(I);
+  Handled.insert(Handled.end(), Active.begin(), Active.end());
   Active.clear();
-  for (Variable *I : Inactive)
-    Handled.push_back(I);
+  Handled.insert(Handled.end(), Inactive.begin(), Inactive.end());
   Inactive.clear();
   dump(Func);
 
-  llvm::SmallVector<int32_t, REGS_SIZE> Permutation(NumRegisters);
-  if (Randomized) {
-    // Create a random number generator for regalloc randomization. Merge
-    // function's sequence and Kind value as the Salt. Because regAlloc()
-    // is called twice under O2, the second time with RAK_Phi, we check
-    // Kind == RAK_Phi to determine the lowest-order bit to make sure the
-    // Salt is different.
-    uint64_t Salt =
-        (Func->getSequenceNumber() << 1) ^ (Kind == RAK_Phi ? 0u : 1u);
-    Func->getTarget()->makeRandomRegisterPermutation(
-        Permutation, PreDefinedRegisters | ~RegMaskFull, Salt);
-  }
-
-  // Finish up by assigning RegNumTmp->RegNum (or a random permutation
-  // thereof) for each Variable.
-  for (Variable *Item : Handled) {
-    int32_t RegNum = Item->getRegNumTmp();
-    int32_t AssignedRegNum = RegNum;
-
-    if (Randomized && Item->hasRegTmp() && !Item->hasReg()) {
-      AssignedRegNum = Permutation[RegNum];
-    }
-    if (Verbose) {
-      Ostream &Str = Ctx->getStrDump();
-      if (!Item->hasRegTmp()) {
-        Str << "Not assigning ";
-        Item->dump(Func);
-        Str << "\n";
-      } else {
-        Str << (AssignedRegNum == Item->getRegNum() ? "Reassigning "
-                                                    : "Assigning ")
-            << Func->getTarget()->getRegName(AssignedRegNum, IceType_i32)
-            << "(r" << AssignedRegNum << ") to ";
-        Item->dump(Func);
-        Str << "\n";
-      }
-    }
-    Item->setRegNum(AssignedRegNum);
-  }
+  assignFinalRegisters(RegMaskFull, PreDefinedRegisters, Randomized);
 
   // TODO: Consider running register allocation one more time, with
   // infinite registers, for two reasons.  First, evicted live ranges