Bug 821735 - Cleanup/fix population and checking of register and slot information in safepoints, r=jandem.
authorBrian Hackett <bhackett1024@gmail.com>
Mon, 17 Dec 2012 15:32:03 -0700
changeset 125434 9a99043b80e7a1901776e663187b7e8bccd06e48
parent 125433 f774c609480827729a5d199504763775cdd2ac40
child 125435 52557de563d79a9582558ae51b1304325268fd95
push id2151
push userlsblakk@mozilla.com
push dateTue, 19 Feb 2013 18:06:57 +0000
treeherdermozilla-beta@4952e88741ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs821735
milestone20.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 821735 - Cleanup/fix population and checking of register and slot information in safepoints, r=jandem.
js/src/ion/LIR.cpp
js/src/ion/LIR.h
js/src/ion/RegisterAllocator.cpp
js/src/ion/RegisterAllocator.h
--- a/js/src/ion/LIR.cpp
+++ b/js/src/ion/LIR.cpp
@@ -210,26 +210,29 @@ PrintDefinition(FILE *fp, const LDefinit
         fprintf(fp, " (-)");
     }
     fprintf(fp, "]");
 }
 
 static void
 PrintUse(char *buf, size_t size, const LUse *use)
 {
-    if (use->policy() == LUse::ANY) {
-        JS_snprintf(buf, size, "v%d:*", use->virtualRegister());
-    } else if (use->policy() == LUse::REGISTER) {
+    switch (use->policy()) {
+      case LUse::REGISTER:
         JS_snprintf(buf, size, "v%d:r", use->virtualRegister());
-    } else {
+        break;
+      case LUse::FIXED:
         // Unfortunately, we don't know here whether the virtual register is a
         // float or a double. Should we steal a bit in LUse for help? For now,
         // nothing defines any fixed xmm registers.
         JS_snprintf(buf, size, "v%d:%s", use->virtualRegister(),
                     Registers::GetName(Registers::Code(use->registerCode())));
+        break;
+      default:
+        JS_snprintf(buf, size, "v%d:*", use->virtualRegister());
     }
 }
 
 #ifdef DEBUG
 const char *
 LAllocation::toString() const
 {
     // Not reentrant!
--- a/js/src/ion/LIR.h
+++ b/js/src/ion/LIR.h
@@ -933,44 +933,55 @@ class LSafepoint : public TempObject
 {
     typedef SafepointNunboxEntry NunboxEntry;
 
   public:
     typedef Vector<uint32_t, 0, IonAllocPolicy> SlotList;
     typedef Vector<NunboxEntry, 0, IonAllocPolicy> NunboxList;
 
   private:
-    // The set of registers which are live after the safepoint. This is empty
-    // for instructions marked as calls.
+    // The information in a safepoint describes the registers and gc related
+    // values that are live at the start of the associated instruction.
+
+    // The set of registers which are live at an OOL call made within the
+    // instruction. This includes any registers for inputs which are not
+    // use-at-start, any registers for temps, and any registers live after the
+    // call except outputs of the instruction.
+    //
+    // For call instructions, the live regs are empty. Call instructions may
+    // have register inputs or temporaries, which will *not* be in the live
+    // registers: if passed to the call, the values passed will be marked via
+    // MarkIonExitFrame, and no registers can be live after the instruction
+    // except its outputs.
     RegisterSet liveRegs_;
 
-    // The set of registers which contain gcthings.
+    // The subset of liveRegs which contains gcthing pointers.
     GeneralRegisterSet gcRegs_;
 
     // Offset to a position in the safepoint stream, or
     // INVALID_SAFEPOINT_OFFSET.
     uint32_t safepointOffset_;
 
     // Assembler buffer displacement to OSI point's call location.
     uint32_t osiCallPointOffset_;
 
-    // List of stack slots which have gc pointers.
+    // List of stack slots which have gcthing pointers.
     SlotList gcSlots_;
 
     // List of stack slots which have Values.
     SlotList valueSlots_;
 
 #ifdef JS_NUNBOX32
-    // List of registers which contain pieces of values.
+    // List of registers (in liveRegs) and stack slots which contain pieces of Values.
     NunboxList nunboxParts_;
 
     // Number of nunboxParts which are not completely filled in.
     uint32_t partialNunboxes_;
 #elif JS_PUNBOX64
-    // List of registers which contain values.
+    // The subset of liveRegs which have Values.
     GeneralRegisterSet valueRegs_;
 #endif
 
   public:
     LSafepoint()
       : safepointOffset_(INVALID_SAFEPOINT_OFFSET)
       , osiCallPointOffset_(0)
 #ifdef JS_NUNBOX32
@@ -991,21 +1002,22 @@ class LSafepoint : public TempObject
     }
     bool addGcSlot(uint32_t slot) {
         return gcSlots_.append(slot);
     }
     SlotList &gcSlots() {
         return gcSlots_;
     }
 
-    void addGcPointer(LAllocation alloc) {
+    bool addGcPointer(LAllocation alloc) {
+        if (alloc.isStackSlot())
+            return addGcSlot(alloc.toStackSlot()->slot());
         if (alloc.isRegister())
             addGcRegister(alloc.toRegister().gpr());
-        else if (alloc.isStackSlot())
-            addGcSlot(alloc.toStackSlot()->slot());
+        return true;
     }
 
     bool hasGcPointer(LAllocation alloc) {
         if (alloc.isRegister())
             return gcRegs().has(alloc.toRegister().gpr());
         if (alloc.isStackSlot()) {
             for (size_t i = 0; i < gcSlots_.length(); i++) {
                 if (gcSlots_[i] == alloc.toStackSlot()->slot())
--- a/js/src/ion/RegisterAllocator.cpp
+++ b/js/src/ion/RegisterAllocator.cpp
@@ -37,50 +37,46 @@ AllocationIntegrityState::record()
         BlockInfo &blockInfo = blocks[i];
         if (!blockInfo.phis.reserve(block->numPhis()))
             return false;
 
         for (size_t j = 0; j < block->numPhis(); j++) {
             blockInfo.phis.infallibleAppend(InstructionInfo());
             InstructionInfo &info = blockInfo.phis[j];
             LPhi *phi = block->getPhi(j);
-            for (size_t k = 0; k < phi->numDefs(); k++) {
-                uint32_t vreg = phi->getDef(k)->virtualRegister();
-                virtualRegisters[vreg] = phi->getDef(k);
-                if (!info.outputs.append(vreg))
-                    return false;
-            }
+            JS_ASSERT(phi->numDefs() == 1);
+            uint32_t vreg = phi->getDef(0)->virtualRegister();
+            virtualRegisters[vreg] = phi->getDef(0);
+            if (!info.outputs.append(*phi->getDef(0)))
+                return false;
             for (size_t k = 0; k < phi->numOperands(); k++) {
-                if (!info.inputs.append(phi->getOperand(k)->toUse()->virtualRegister()))
+                if (!info.inputs.append(*phi->getOperand(k)))
                     return false;
             }
         }
 
         for (LInstructionIterator iter = block->begin(); iter != block->end(); iter++) {
             LInstruction *ins = *iter;
             InstructionInfo &info = instructions[ins->id()];
 
+            for (size_t k = 0; k < ins->numTemps(); k++) {
+                uint32_t vreg = ins->getTemp(k)->virtualRegister();
+                virtualRegisters[vreg] = ins->getTemp(k);
+                if (!info.temps.append(*ins->getTemp(k)))
+                    return false;
+            }
             for (size_t k = 0; k < ins->numDefs(); k++) {
                 uint32_t vreg = ins->getDef(k)->virtualRegister();
                 virtualRegisters[vreg] = ins->getDef(k);
-                if (!info.outputs.append(vreg))
+                if (!info.outputs.append(*ins->getDef(k)))
                     return false;
-                if (ins->getDef(k)->policy() == LDefinition::MUST_REUSE_INPUT) {
-                    JS_ASSERT(k == 0);
-                    info.reusedInput = ins->getDef(k)->getReusedInput();
-                }
             }
             for (LInstruction::InputIterator alloc(*ins); alloc.more(); alloc.next()) {
-                if (alloc->isUse() && alloc->toUse()->policy() != LUse::RECOVERED_INPUT) {
-                    if (!info.inputs.append(alloc->toUse()->virtualRegister()))
-                        return false;
-                } else {
-                    if (!info.inputs.append(UINT32_MAX))
-                        return false;
-                }
+                if (!info.inputs.append(**alloc))
+                    return false;
             }
         }
     }
 
     return seen.init();
 }
 
 bool
@@ -101,31 +97,28 @@ AllocationIntegrityState::check(bool pop
 
             for (LInstruction::InputIterator alloc(*ins); alloc.more(); alloc.next())
                 JS_ASSERT(!alloc->isUse());
 
             for (size_t i = 0; i < ins->numDefs(); i++) {
                 LDefinition *def = ins->getDef(i);
                 JS_ASSERT_IF(def->policy() != LDefinition::PASSTHROUGH, !def->output()->isUse());
 
-                if (def->output()->isRegister()) {
-                    // The live regs for an instruction's safepoint should
-                    // exclude the instruction's definitions.
-                    LSafepoint *safepoint = ins->safepoint();
-                    JS_ASSERT_IF(safepoint, !safepoint->liveRegs().has(def->output()->toRegister()));
-                }
-
-                uint32_t reusedInput = instructions[ins->id()].reusedInput;
-                JS_ASSERT_IF(reusedInput != UINT32_MAX,
-                             *def->output() == *ins->getOperand(reusedInput));
+                LDefinition oldDef = instructions[ins->id()].outputs[i];
+                JS_ASSERT_IF(oldDef.policy() == LDefinition::MUST_REUSE_INPUT,
+                             *def->output() == *ins->getOperand(oldDef.getReusedInput()));
             }
 
             for (size_t i = 0; i < ins->numTemps(); i++) {
                 LDefinition *temp = ins->getTemp(i);
                 JS_ASSERT_IF(!temp->isBogusTemp(), temp->output()->isRegister());
+
+                LDefinition oldTemp = instructions[ins->id()].temps[i];
+                JS_ASSERT_IF(oldTemp.policy() == LDefinition::MUST_REUSE_INPUT,
+                             *temp->output() == *ins->getOperand(oldTemp.getReusedInput()));
             }
         }
     }
 #endif
 
     // Check that the register assignment and move groups preserve the original
     // semantics of the virtual registers. Each virtual register has a single
     // write (owing to the SSA representation), but the allocation may move the
@@ -136,22 +129,42 @@ AllocationIntegrityState::check(bool pop
     // backward through the script, along all paths to the value's virtual
     // register's definition.
     for (size_t blockIndex = 0; blockIndex < graph.numBlocks(); blockIndex++) {
         LBlock *block = graph.getBlock(blockIndex);
         for (LInstructionIterator iter = block->begin(); iter != block->end(); iter++) {
             LInstruction *ins = *iter;
             const InstructionInfo &info = instructions[ins->id()];
 
+            LSafepoint *safepoint = ins->safepoint();
+            if (safepoint) {
+                for (size_t i = 0; i < ins->numTemps(); i++) {
+                    uint32_t vreg = info.temps[i].virtualRegister();
+                    LAllocation *alloc = ins->getTemp(i)->output();
+                    if (!checkSafepointAllocation(ins, vreg, *alloc, populateSafepoints))
+                        return false;
+                }
+                JS_ASSERT_IF(ins->isCall() && !populateSafepoints,
+                             safepoint->liveRegs().empty(true) &&
+                             safepoint->liveRegs().empty(false));
+            }
+
             size_t inputIndex = 0;
             for (LInstruction::InputIterator alloc(*ins); alloc.more(); alloc.next()) {
-                uint32_t vreg = info.inputs[inputIndex++];
-                if (vreg == UINT32_MAX)
+                LAllocation oldInput = info.inputs[inputIndex++];
+                if (!oldInput.isUse())
                     continue;
 
+                uint32_t vreg = oldInput.toUse()->virtualRegister();
+
+                if (safepoint && !oldInput.toUse()->usedAtStart()) {
+                    if (!checkSafepointAllocation(ins, vreg, **alloc, populateSafepoints))
+                        return false;
+                }
+
                 // Start checking at the previous instruction, in case this
                 // instruction reuses its input register for an output.
                 LInstructionReverseIterator riter = block->rbegin(ins);
                 riter++;
                 checkIntegrity(block, *riter, vreg, **alloc, populateSafepoints);
 
                 while (!worklist.empty()) {
                     IntegrityItem item = worklist.back();
@@ -190,103 +203,48 @@ AllocationIntegrityState::checkIntegrity
         // Make sure the physical location being tracked is not clobbered by
         // another instruction, and that if the originating vreg definition is
         // found that it is writing to the tracked location.
 
         for (size_t i = 0; i < ins->numDefs(); i++) {
             LDefinition *def = ins->getDef(i);
             if (def->policy() == LDefinition::PASSTHROUGH)
                 continue;
-            if (info.outputs[i] == vreg) {
+            if (info.outputs[i].virtualRegister() == vreg) {
                 JS_ASSERT(*def->output() == alloc);
 
                 // Found the original definition, done scanning.
                 return true;
             } else {
                 JS_ASSERT(*def->output() != alloc);
             }
         }
 
         for (size_t i = 0; i < ins->numTemps(); i++) {
             LDefinition *temp = ins->getTemp(i);
             if (!temp->isBogusTemp())
                 JS_ASSERT(*temp->output() != alloc);
         }
 
-        LSafepoint *safepoint = ins->safepoint();
-        if (!safepoint)
-            continue;
-
-        if (alloc.isRegister()) {
-            AnyRegister reg = alloc.toRegister();
-            if (populateSafepoints)
-                safepoint->addLiveRegister(reg);
-            else
-                JS_ASSERT(safepoint->liveRegs().has(reg));
-        }
-
-        LDefinition::Type type = virtualRegisters[vreg]
-                                 ? virtualRegisters[vreg]->type()
-                                 : LDefinition::GENERAL;
-
-        switch (type) {
-          case LDefinition::OBJECT:
-            if (populateSafepoints) {
-                IonSpew(IonSpew_RegAlloc, "Safepoint object v%u i%u %s",
-                        vreg, ins->id(), alloc.toString());
-                safepoint->addGcPointer(alloc);
-            } else {
-                JS_ASSERT(safepoint->hasGcPointer(alloc));
-            }
-            break;
-#ifdef JS_NUNBOX32
-          // Do not assert that safepoint information for nunboxes is complete,
-          // as if a vreg for a value's components are copied in multiple places
-          // then the safepoint information may be incomplete and not reflect
-          // all copies. See SafepointWriter::writeNunboxParts.
-          case LDefinition::TYPE:
-            if (populateSafepoints) {
-                IonSpew(IonSpew_RegAlloc, "Safepoint type v%u i%u %s",
-                        vreg, ins->id(), alloc.toString());
-                safepoint->addNunboxType(vreg, alloc);
-            }
-            break;
-          case LDefinition::PAYLOAD:
-            if (populateSafepoints) {
-                IonSpew(IonSpew_RegAlloc, "Safepoint payload v%u i%u %s",
-                        vreg, ins->id(), alloc.toString());
-                safepoint->addNunboxPayload(vreg, alloc);
-            }
-            break;
-#else
-          case LDefinition::BOX:
-            if (populateSafepoints) {
-                IonSpew(IonSpew_RegAlloc, "Safepoint boxed value v%u i%u %s",
-                        vreg, ins->id(), alloc.toString());
-                safepoint->addBoxedValue(alloc);
-            } else {
-                JS_ASSERT(safepoint->hasBoxedValue(alloc));
-            }
-            break;
-#endif
-          default:
-            break;
+        if (ins->safepoint()) {
+            if (!checkSafepointAllocation(ins, vreg, alloc, populateSafepoints))
+                return false;
         }
     }
 
     // Phis are effectless, but change the vreg we are tracking. Check if there
     // is one which produced this vreg. We need to follow back through the phi
     // inputs as it is not guaranteed the register allocator filled in physical
     // allocations for the inputs and outputs of the phis.
     for (size_t i = 0; i < block->numPhis(); i++) {
         InstructionInfo &info = blocks[block->mir()->id()].phis[i];
         LPhi *phi = block->getPhi(i);
-        if (info.outputs[0] == vreg) {
+        if (info.outputs[0].virtualRegister() == vreg) {
             for (size_t j = 0; j < phi->numOperands(); j++) {
-                uint32_t newvreg = info.inputs[j];
+                uint32_t newvreg = info.inputs[j].toUse()->virtualRegister();
                 LBlock *predecessor = graph.getBlock(block->mir()->getPredecessor(j)->id());
                 if (!addPredecessor(predecessor, newvreg, alloc))
                     return false;
             }
             return true;
         }
     }
 
@@ -297,16 +255,87 @@ AllocationIntegrityState::checkIntegrity
         if (!addPredecessor(predecessor, vreg, alloc))
             return false;
     }
 
     return true;
 }
 
 bool
+AllocationIntegrityState::checkSafepointAllocation(LInstruction *ins,
+                                                   uint32_t vreg, LAllocation alloc,
+                                                   bool populateSafepoints)
+{
+    LSafepoint *safepoint = ins->safepoint();
+    JS_ASSERT(safepoint);
+
+    if (ins->isCall() && alloc.isRegister())
+        return true;
+
+    if (alloc.isRegister()) {
+        AnyRegister reg = alloc.toRegister();
+        if (populateSafepoints)
+            safepoint->addLiveRegister(reg);
+        JS_ASSERT(safepoint->liveRegs().has(reg));
+    }
+
+    LDefinition::Type type = virtualRegisters[vreg]
+                             ? virtualRegisters[vreg]->type()
+                             : LDefinition::GENERAL;
+
+    switch (type) {
+      case LDefinition::OBJECT:
+        if (populateSafepoints) {
+            IonSpew(IonSpew_RegAlloc, "Safepoint object v%u i%u %s",
+                    vreg, ins->id(), alloc.toString());
+            if (!safepoint->addGcPointer(alloc))
+                return false;
+        }
+        JS_ASSERT(safepoint->hasGcPointer(alloc));
+        break;
+#ifdef JS_NUNBOX32
+      // Do not assert that safepoint information for nunboxes is complete,
+      // as if a vreg for a value's components are copied in multiple places
+      // then the safepoint information may not reflect all copies.
+      // See SafepointWriter::writeNunboxParts.
+      case LDefinition::TYPE:
+        if (populateSafepoints) {
+            IonSpew(IonSpew_RegAlloc, "Safepoint type v%u i%u %s",
+                    vreg, ins->id(), alloc.toString());
+            if (!safepoint->addNunboxType(vreg, alloc))
+                return false;
+        }
+        break;
+      case LDefinition::PAYLOAD:
+        if (populateSafepoints) {
+            IonSpew(IonSpew_RegAlloc, "Safepoint payload v%u i%u %s",
+                    vreg, ins->id(), alloc.toString());
+            if (!safepoint->addNunboxPayload(vreg, alloc))
+                return false;
+        }
+        break;
+#else
+      case LDefinition::BOX:
+        if (populateSafepoints) {
+            IonSpew(IonSpew_RegAlloc, "Safepoint boxed value v%u i%u %s",
+                    vreg, ins->id(), alloc.toString());
+            if (!safepoint->addBoxedValue(alloc))
+                return false;
+        }
+        JS_ASSERT(safepoint->hasBoxedValue(alloc));
+        break;
+#endif
+      default:
+        break;
+    }
+
+    return true;
+}
+
+bool
 AllocationIntegrityState::addPredecessor(LBlock *block, uint32_t vreg, LAllocation alloc)
 {
     // There is no need to reanalyze if we have already seen this predecessor.
     // We share the seen allocations across analysis of each use, as there will
     // likely be common ground between different uses of the same vreg.
     IntegrityItem item;
     item.block = block;
     item.vreg = vreg;
@@ -336,19 +365,19 @@ AllocationIntegrityState::dump()
         for (size_t i = 0; i < mir->numSuccessors(); i++)
             printf(" [successor %u]", mir->getSuccessor(i)->id());
         printf("\n");
 
         for (size_t i = 0; i < block->numPhis(); i++) {
             InstructionInfo &info = blocks[blockIndex].phis[i];
             LPhi *phi = block->getPhi(i);
 
-            printf("Phi v%u <-", info.outputs[0]);
+            printf("Phi v%u <-", info.outputs[0].virtualRegister());
             for (size_t j = 0; j < phi->numOperands(); j++)
-                printf(" v%u", info.inputs[j]);
+                printf(" %s", info.inputs[j].toString());
             printf("\n");
         }
 
         for (LInstructionIterator iter = block->begin(); iter != block->end(); iter++) {
             LInstruction *ins = *iter;
             InstructionInfo &info = instructions[ins->id()];
 
             CodePosition input(ins->id(), CodePosition::INPUT);
@@ -365,30 +394,30 @@ AllocationIntegrityState::dump()
                 }
                 printf("\n");
                 continue;
             }
 
             for (size_t i = 0; i < ins->numTemps(); i++) {
                 LDefinition *temp = ins->getTemp(i);
                 if (!temp->isBogusTemp())
-                    printf(" [temp %s]", temp->output()->toString());
+                    printf(" [temp v%u %s]", info.temps[i].virtualRegister(),
+                           temp->output()->toString());
             }
 
             for (size_t i = 0; i < ins->numDefs(); i++) {
                 LDefinition *def = ins->getDef(i);
-                printf(" [def v%u %s]", info.outputs[i], def->output()->toString());
+                printf(" [def v%u %s]", info.outputs[i].virtualRegister(),
+                       def->output()->toString());
             }
 
             size_t index = 0;
             for (LInstruction::InputIterator alloc(*ins); alloc.more(); alloc.next()) {
-                uint32_t vreg = info.inputs[index++];
-                if (vreg == UINT32_MAX)
-                    continue;
-                printf(" [use v%u %s]", vreg, alloc->toString());
+                printf(" [use %s", info.inputs[index++].toString());
+                printf(" %s]", alloc->toString());
             }
 
             printf("\n");
         }
     }
 
     printf("\nIntermediate Allocations:\n\n");
 
--- a/js/src/ion/RegisterAllocator.h
+++ b/js/src/ion/RegisterAllocator.h
@@ -50,29 +50,29 @@ struct AllocationIntegrityState
 
     // For all instructions and phis in the graph, keep track of the virtual
     // registers for all inputs and outputs of the nodes. These are overwritten
     // in place during register allocation. This information is kept on the
     // side rather than in the instructions and phis themselves to avoid
     // debug-builds-only bloat in the size of the involved structures.
 
     struct InstructionInfo {
-        Vector<uint32_t, 2, SystemAllocPolicy> inputs;
-        Vector<uint32_t, 1, SystemAllocPolicy> outputs;
-        uint32_t reusedInput;
+        Vector<LAllocation, 2, SystemAllocPolicy> inputs;
+        Vector<LDefinition, 0, SystemAllocPolicy> temps;
+        Vector<LDefinition, 1, SystemAllocPolicy> outputs;
 
         InstructionInfo()
-          : reusedInput(UINT32_MAX)
-        {}
+        { }
 
         InstructionInfo(const InstructionInfo &o)
-          : reusedInput(o.reusedInput)
         {
             for (size_t i = 0; i < o.inputs.length(); i++)
                 inputs.append(o.inputs[i]);
+            for (size_t i = 0; i < o.temps.length(); i++)
+                temps.append(o.temps[i]);
             for (size_t i = 0; i < o.outputs.length(); i++)
                 outputs.append(o.outputs[i]);
         }
     };
     Vector<InstructionInfo, 0, SystemAllocPolicy> instructions;
 
     struct BlockInfo {
         Vector<InstructionInfo, 5, SystemAllocPolicy> phis;
@@ -116,16 +116,18 @@ struct AllocationIntegrityState
     Vector<IntegrityItem, 10, SystemAllocPolicy> worklist;
 
     // Set of all items that have already been processed.
     typedef HashSet<IntegrityItem, IntegrityItem, SystemAllocPolicy> IntegrityItemSet;
     IntegrityItemSet seen;
 
     bool checkIntegrity(LBlock *block, LInstruction *ins, uint32_t vreg, LAllocation alloc,
                         bool populateSafepoints);
+    bool checkSafepointAllocation(LInstruction *ins, uint32_t vreg, LAllocation alloc,
+                                  bool populateSafepoints);
     bool addPredecessor(LBlock *block, uint32_t vreg, LAllocation alloc);
 
     void dump();
 };
 
 // Represents with better-than-instruction precision a position in the
 // instruction stream.
 //