Subzero: Fix a regalloc bug involving too-aggressive AllowRegisterOverlap.

See the BUG description for more details. In short, the register allocator was inappropriately honoring AllowRegisterOverlap even when the variable's live range overlaps with an Unhandled variable precolored to the preferred register. Also changes legalize() logic to recognize when a variable is guaranteed to ultimately have a physical register due to infinite weight, and not create a new temporary in those cases. Finally, dumps RegisterPreference and AllowRegisterOverlap info for Variables for improved diagnostics. BUG= https://code.google.com/p/nativeclient/issues/detail?id=3897 R=jvoung@chromium.org Review URL: https://codereview.chromium.org/380363002

Subzero: Fix a regalloc bug involving too-aggressive AllowRegisterOverlap.
ca662e9d · Jim Stichnoth · 240e0f8a · ca662e9d · ca662e9d · ca662e9d
Commit ca662e9d authored Jul 10, 2014 by Jim Stichnoth
Hide whitespace changes
Inline Side-by-side

Showing with 20 additions and 4 deletions

IceCfg.cpp src/IceCfg.cpp +7 -0

IceRegAlloc.cpp src/IceRegAlloc.cpp +8 -2

IceTargetLoweringX8632.cpp src/IceTargetLoweringX8632.cpp +5 -2

No files found.
--- a/src/IceCfg.cpp
+++ b/src/IceCfg.cpp
@@ -345,6 +345,13 @@ void Cfg::dump(const IceString &Message) {
          << " multiblock=" << Var->isMultiblockLife() << " "
          << " weight=" << Var->getWeight() << " ";
      Var->dump(this);
+      if (Variable *Pref = Var->getPreferredRegister()) {
+        Str << " pref=";
+        Pref->dump(this);
+        if (Var->getRegisterOverlap())
+          Str << ",overlap";
+        Str << " ";
+      }
      Str << " LIVE=" << Var->getLiveRange() << "\n";
    }
  }

--- a/src/IceRegAlloc.cpp
+++ b/src/IceRegAlloc.cpp
@@ -205,12 +205,14 @@ void LinearScan::scan(const llvm::SmallBitVector &RegMaskFull) {
    // overlaps with the current range and is precolored.
    // Cur.endsBefore(*I) is an early exit check that turns a
    // guaranteed O(N^2) algorithm into expected linear complexity.
+    llvm::SmallBitVector PrecoloredUnhandled(RegMask.size());
    for (OrderedRanges::const_iterator I = Unhandled.begin(),
                                       E = Unhandled.end();
         I != E && !Cur.endsBefore(*I); ++I) {
      LiveRangeWrapper Item = *I;
      if (Item.Var->hasReg() && Item.overlaps(Cur)) {
        Free[Item.Var->getRegNum()] = false; // Note: getRegNum not getRegNumTmp
+        PrecoloredUnhandled[Item.Var->getRegNum()] = true;
      }
    }

@@ -219,7 +221,8 @@ void LinearScan::scan(const llvm::SmallBitVector &RegMaskFull) {
      for (SizeT i = 0; i < RegMask.size(); ++i) {
        if (RegMask[i]) {
          Str << Func->getTarget()->getRegName(i, IceType_i32)
-              << "(U=" << RegUses[i] << ",F=" << Free[i] << ") ";
+              << "(U=" << RegUses[i] << ",F=" << Free[i]
+              << ",P=" << PrecoloredUnhandled[i] << ") ";
        }
      }
      Str << "\n";
@@ -228,8 +231,11 @@ void LinearScan::scan(const llvm::SmallBitVector &RegMaskFull) {
    Variable *Prefer = Cur.Var->getPreferredRegister();
    int32_t PreferReg = Prefer && Prefer->hasRegTmp() ? Prefer->getRegNumTmp()
                                                      : Variable::NoRegister;
+    bool AllowedToOverlap = Cur.Var->getRegisterOverlap() &&
+                            PreferReg != Variable::NoRegister &&
+                            !PrecoloredUnhandled[PreferReg];
    if (PreferReg != Variable::NoRegister &&
-        (Cur.Var->getRegisterOverlap() || Free[PreferReg])) {
+        (AllowedToOverlap || Free[PreferReg])) {
      // First choice: a preferred register that is either free or is
      // allowed to overlap with its linked variable.
      Cur.Var->setRegNumTmp(PreferReg);

--- a/src/IceTargetLoweringX8632.cpp
+++ b/src/IceTargetLoweringX8632.cpp
@@ -2615,9 +2615,12 @@ Operand *TargetX8632::legalize(Operand *From, LegalMask Allowed,
  }
  if (Variable *Var = llvm::dyn_cast<Variable>(From)) {
    // We need a new physical register for the operand if:
-    //   Mem is not allowed and Var->getRegNum() is unknown, or
+    //   Mem is not allowed and Var isn't guaranteed a physical
+    //   register, or
    //   RegNum is required and Var->getRegNum() doesn't match.
-    if ((!(Allowed & Legal_Mem) && !Var->hasReg()) ||
+    bool WillHaveRegister =
+        (Var->hasReg() || Var->getWeight() == RegWeight::Inf);
+    if ((!(Allowed & Legal_Mem) && !WillHaveRegister) ||
        (RegNum != Variable::NoRegister && RegNum != Var->getRegNum())) {
      Variable *Reg = copyToReg(From, RegNum);
      if (RegNum == Variable::NoRegister) {