Subzero: Consider all instruction variables for register preference.

src/IceRegAlloc.cpp

 //===- subzero/src/IceRegAlloc.cpp - Linear-scan implementation -----------===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
@@ skipping 475 matching lines @@
 void LinearScan::findRegisterPreference(IterationState &Iter) {
   Iter.Prefer = nullptr;
   Iter.PreferReg = Variable::NoRegister;
   Iter.AllowOverlap = false;
 
   if (FindPreference) {
     VariablesMetadata *VMetadata = Func->getVMetadata();
     if (const Inst *DefInst =
             VMetadata->getFirstDefinitionSingleBlock(Iter.Cur)) {
       assert(DefInst->getDest() == Iter.Cur);
-      bool IsAssign = DefInst->isSimpleAssign();
+      bool IsAssign = DefInst->isVarAssign();
       bool IsSingleDef = !VMetadata->isMultiDef(Iter.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() && Iter.RegMask[SrcReg]) {
-            if (FindOverlap && !Iter.Free[SrcReg]) {
-              // Don't bother trying to enable AllowOverlap if the register is
-              // already free.
-              Iter.AllowOverlap = IsSingleDef && IsAssign &&
-                                  !overlapsDefs(Func, Iter.Cur, SrcVar);
-            }
-            if (Iter.AllowOverlap || Iter.Free[SrcReg]) {
-              Iter.Prefer = SrcVar;
-              Iter.PreferReg = SrcReg;
-            }
+      FOREACH_VAR_IN_INST(SrcVar, *DefInst) {
+        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() && (Iter.RegMask & *RegAliases[SrcReg]).any()) {

[John, 2015/10/12 15:35:17]

this is super hard to read!!!

const bool SrcVarRegAliasAvailable = (set & set).any();

[Jim Stichnoth, 2015/10/12 17:27:31]

Done.  Note that I want to put the cheap hasRegTmp() test before the more
expensive bitvector operation.

[John, 2015/10/12 20:39:58]

Then you can define a static method (in an unnamed name space)

namespace {
bool SrcVarRegAliasAvaiabke(Iter, RegAliases[SrcReg]) {
}
}

+          if (FindOverlap && !Iter.Free[SrcReg]) {
+            // Don't bother trying to enable AllowOverlap if the register is
+            // already free.
+            Iter.AllowOverlap = IsSingleDef && IsAssign &&
+                                !overlapsDefs(Func, Iter.Cur, SrcVar);
+          }
+          if (Iter.AllowOverlap || Iter.Free[SrcReg]) {
+            Iter.Prefer = SrcVar;
+            Iter.PreferReg = SrcReg;
+            // Stop looking for a preference after the first valid preference is
+            // found.  One might think that we should look at all instruction
+            // variables to find the best <Prefer,AllowOverlap> combination, but
+            // note that AllowOverlap can only be true for a simple assignment
+            // statement which can have only one source operand, so it's not
+            // possible for AllowOverlap to be true beyond the first source
+            // operand.
+            break;
           }
         }
       }
       if (Verbose && Iter.Prefer) {
         Ostream &Str = Ctx->getStrDump();
         Str << "Initial Iter.Prefer=";
         Iter.Prefer->dump(Func);
         Str << " R=" << Iter.PreferReg
             << " LIVE=" << Iter.Prefer->getLiveRange()
             << " Overlap=" << Iter.AllowOverlap << "\n";
       }
     }
   }
 }
 
 // Remove registers from the Free[] list where an Inactive range overlaps with
 // the current range.
 void LinearScan::filterFreeWithInactiveRanges(IterationState &Iter) {
   for (const Variable *Item : Inactive) {
     if (!Item->rangeOverlaps(Iter.Cur))
       continue;
     const llvm::SmallBitVector &Aliases = *RegAliases[Item->getRegNumTmp()];
+    // TODO(stichnot): Do this with bitvector ops, not a loop, for efficiency.

[John, 2015/10/12 15:35:17]

this is a simple TODOne:

for (int32_t RegAlias = Aliases.find_first(); RegAlias >= 0; RegAlias =
Aliases.find_next(RegAlias)) {
...
}

change it now?

[Jim Stichnoth, 2015/10/12 17:27:31]

What I mean is, I think it can be changed to something like:

Iter.Free ~= ~Aliases;
if (Iter.AllowOverlap && Item != Iter.Prefer && Aliases[Iter.PreferReg] &&
overlapsDefs(...)) ...

I'm planning to do it as part of the x86-32 proper register aliasing
CL, so I can test that before-and-after output on spec2k is identical.

     for (int32_t RegAlias = Aliases.find_first(); RegAlias >= 0;
          RegAlias = Aliases.find_next(RegAlias)) {
       // Don't assert(Free[RegNum]) because in theory (though probably never in
       // practice) there could be two inactive variables that were marked with
       // AllowOverlap.
       Iter.Free[RegAlias] = 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 (Iter.AllowOverlap && Item != Iter.Prefer &&
@@ skipping 293 matching lines @@
     }
 
     findRegisterPreference(Iter);
     filterFreeWithInactiveRanges(Iter);
 
     // Disable AllowOverlap if an Active variable, which is not Prefer, shares
     // Prefer's register, and has a definition within Cur's live range.
     if (Iter.AllowOverlap) {
       for (const Variable *Item : Active) {
         int32_t RegNum = Item->getRegNumTmp();
+        // TODO(stichnot): Consider aliases of RegNum.  This is probably a
+        // correctness issue.

[John, 2015/10/12 15:35:17]

This is definitively a correctness issue... :(

[Jim Stichnoth, 2015/10/12 17:27:31]

Yeah, I want to address it in the context of the x86-32 proper register aliasing
CL, where I have a chance of actually testing it.

         if (Item != Iter.Prefer && RegNum == Iter.PreferReg &&
             overlapsDefs(Func, Iter.Cur, Item)) {
           Iter.AllowOverlap = false;
           dumpDisableOverlap(Func, Item, "Active");
         }
       }
     }
 
     Iter.Weights.resize(Iter.RegMask.size());
     std::fill(Iter.Weights.begin(), Iter.Weights.end(), RegWeight());
@@ skipping 105 matching lines @@
     Str << "\n";
   }
   Str << "++++++ Inactive:\n";
   for (const Variable *Item : Inactive) {
     dumpLiveRange(Item, Func);
     Str << "\n";
   }
 }
 
 } // end of namespace Ice