Bug 905091 part 1 - Fix Ion regalloc to not insert movegroups between an instruction and its OsiPoint. r=bhackett
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 15 Aug 2013 16:14:41 +0200
changeset 142771 a655e261366727fbf69725c401e9ec23de923acc
parent 142770 67d3e9da1b4b6b9f389e2efe710257701ad2d963
child 142772 9c90bda449925de88dfb3840d34168fdf6c48d55
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersbhackett
bugs905091
milestone26.0a1
Bug 905091 part 1 - Fix Ion regalloc to not insert movegroups between an instruction and its OsiPoint. r=bhackett
js/src/jit/BacktrackingAllocator.cpp
js/src/jit/BacktrackingAllocator.h
js/src/jit/LinearScan.cpp
js/src/jit/RegisterAllocator.h
--- a/js/src/jit/BacktrackingAllocator.cpp
+++ b/js/src/jit/BacktrackingAllocator.cpp
@@ -1325,33 +1325,16 @@ BacktrackingAllocator::computePriority(c
     size_t priority = 0;
     for (size_t j = 0; j < group->registers.length(); j++) {
         uint32_t vreg = group->registers[j];
         priority += computePriority(vregs[vreg].getInterval(0));
     }
     return priority;
 }
 
-CodePosition
-BacktrackingAllocator::minimalDefEnd(LInstruction *ins)
-{
-    // Compute the shortest interval that captures vregs defined by ins.
-    // Watch for instructions that are followed by an OSI point and/or Nop.
-    // If moves are introduced between the instruction and the OSI point then
-    // safepoint information for the instruction may be incorrect. This is
-    // pretty disgusting and should be fixed somewhere else in the compiler.
-    while (true) {
-        LInstruction *next = insData[outputOf(ins).next()].ins();
-        if (!next->isNop() && !next->isOsiPoint())
-            break;
-        ins = next;
-    }
-    return outputOf(ins);
-}
-
 bool
 BacktrackingAllocator::minimalDef(const LiveInterval *interval, LInstruction *ins)
 {
     // Whether interval is a minimal interval capturing a definition at ins.
     return (interval->end() <= minimalDefEnd(ins).next()) &&
         (interval->start() == inputOf(ins) || interval->start() == outputOf(ins));
 }
 
--- a/js/src/jit/BacktrackingAllocator.h
+++ b/js/src/jit/BacktrackingAllocator.h
@@ -207,17 +207,16 @@ class BacktrackingAllocator : public Liv
     bool populateSafepoints();
 
     void dumpRegisterGroups();
     void dumpLiveness();
     void dumpAllocations();
 
     struct PrintLiveIntervalRange;
 
-    CodePosition minimalDefEnd(LInstruction *ins);
     bool minimalDef(const LiveInterval *interval, LInstruction *ins);
     bool minimalUse(const LiveInterval *interval, LInstruction *ins);
     bool minimalInterval(const LiveInterval *interval, bool *pfixed = NULL);
 
     // Heuristic methods.
 
     size_t computePriority(const LiveInterval *interval);
     size_t computeSpillWeight(const LiveInterval *interval);
--- a/js/src/jit/LinearScan.cpp
+++ b/js/src/jit/LinearScan.cpp
@@ -347,17 +347,33 @@ LinearScanAllocator::reifyAllocations()
         if (interval->index() == 0)
         {
             LDefinition *def = reg->def();
             LAllocation *spillFrom;
 
             if (def->policy() == LDefinition::PRESET && def->output()->isRegister()) {
                 AnyRegister fixedReg = def->output()->toRegister();
                 LiveInterval *from = fixedIntervals[fixedReg.code()];
-                if (!moveAfter(outputOf(reg->ins()), from, interval))
+
+                // Insert the move after any OsiPoint or Nop instructions
+                // following this one. See minimalDefEnd for more info.
+                CodePosition defEnd = minimalDefEnd(reg->ins());
+
+                // If we just skipped an OsiPoint, and it uses this vreg, it
+                // should use the fixed register instead.
+                for (UsePositionIterator usePos(interval->usesBegin());
+                     usePos != interval->usesEnd();
+                     usePos++)
+                {
+                    if (usePos->pos > defEnd)
+                        break;
+                    *static_cast<LAllocation *>(usePos->use) = LAllocation(fixedReg);
+                }
+
+                if (!moveAfter(defEnd, from, interval))
                     return false;
                 spillFrom = from->getAllocation();
             } else {
                 if (def->policy() == LDefinition::MUST_REUSE_INPUT) {
                     LAllocation *alloc = reg->ins()->getOperand(def->getReusedInput());
                     LAllocation *origAlloc = LAllocation::New(*alloc);
 
                     JS_ASSERT(!alloc->isUse());
--- a/js/src/jit/RegisterAllocator.h
+++ b/js/src/jit/RegisterAllocator.h
@@ -347,16 +347,32 @@ class RegisterAllocator
         size_t i = 0;
         for (; i < graph.numNonCallSafepoints(); i++) {
             const LInstruction *ins = graph.getNonCallSafepoint(i);
             if (from <= inputOf(ins))
                 break;
         }
         return i;
     }
+
+    CodePosition minimalDefEnd(LInstruction *ins) {
+        // Compute the shortest interval that captures vregs defined by ins.
+        // Watch for instructions that are followed by an OSI point and/or Nop.
+        // If moves are introduced between the instruction and the OSI point then
+        // safepoint information for the instruction may be incorrect. This is
+        // pretty disgusting and should be fixed somewhere else in the compiler.
+        while (true) {
+            LInstruction *next = insData[outputOf(ins).next()].ins();
+            if (!next->isNop() && !next->isOsiPoint())
+                break;
+            ins = next;
+        }
+
+        return outputOf(ins);
+    }
 };
 
 static inline AnyRegister
 GetFixedRegister(LDefinition *def, const LUse *use)
 {
     return def->type() == LDefinition::DOUBLE
            ? AnyRegister(FloatRegister::FromCode(use->registerCode()))
            : AnyRegister(Register::FromCode(use->registerCode()));