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

diff --git a/src/IceCfg.cpp b/src/IceCfg.cpp
index bc1bf63..3e32d75 100644 (file)
--- 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";
     }
   }

diff --git a/src/IceRegAlloc.cpp b/src/IceRegAlloc.cpp
index f610d96..fcfe13e 100644 (file)
--- 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);

diff --git a/src/IceTargetLoweringX8632.cpp b/src/IceTargetLoweringX8632.cpp
index c2782f6..2652e8b 100644 (file)
--- 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) {