Improve register pinning invariants (bug 591836 part 2, r=dmandelin).
☠☠ backed out by 2952314edcc8 ☠ ☠
authorDavid Anderson <danderson@mozilla.com>
Tue, 31 Aug 2010 17:42:12 -0700
changeset 74509 8703c9e17f69803299caee68d2ef340fa0b8d5cd
parent 74508 65a5a3d552f662487049c81b211c8217faa08ab9
child 74510 33772073ce075702e021d1df22f806f0de3c0ed6
child 74563 2952314edcc83ac55c26d64c13665c5f4d9ecb34
push id2
push userbsmedberg@mozilla.com
push dateFri, 19 Aug 2011 14:38:13 +0000
reviewersdmandelin
bugs591836
milestone2.0b5pre
Improve register pinning invariants (bug 591836 part 2, r=dmandelin).
js/src/methodjit/Compiler.cpp
js/src/methodjit/FastArithmetic.cpp
js/src/methodjit/FastOps.cpp
js/src/methodjit/FrameState-inl.h
js/src/methodjit/FrameState.cpp
js/src/methodjit/FrameState.h
js/src/methodjit/ImmutableSync.cpp
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -471,17 +471,17 @@ mjit::Compiler::generateMethod()
     PC = script->code;
 
     for (;;) {
         JSOp op = JSOp(*PC);
 
         OpcodeStatus &opinfo = analysis[PC];
         frame.setInTryBlock(opinfo.inTryBlock);
         if (opinfo.nincoming || opinfo.trap) {
-            frame.forgetEverything(opinfo.stackDepth);
+            frame.syncAndForgetEverything(opinfo.stackDepth);
             opinfo.safePoint = true;
         }
         jumpMap[uint32(PC - script->code)] = masm.label();
 
         if (opinfo.trap) {
             if (!trapper.untrap(PC))
                 return Compile_Error;
             op = JSOp(*PC);
@@ -552,17 +552,17 @@ mjit::Compiler::generateMethod()
             frame.pop();
             emitReturn();
           }
           END_CASE(JSOP_RETURN)
 
           BEGIN_CASE(JSOP_GOTO)
           {
             /* :XXX: this isn't really necessary if we follow the branch. */
-            frame.forgetEverything();
+            frame.syncAndForgetEverything();
             Jump j = masm.jump();
             jumpAndTrace(j, PC + GET_JUMP_OFFSET(PC));
           }
           END_CASE(JSOP_GOTO)
 
           BEGIN_CASE(JSOP_IFEQ)
           BEGIN_CASE(JSOP_IFNE)
             jsop_ifneq(op, PC + GET_JUMP_OFFSET(PC));
@@ -655,17 +655,17 @@ mjit::Compiler::generateMethod()
                     if (!target) {
                         frame.push(Value(BooleanValue(result)));
                     } else {
                         if (fused == JSOP_IFEQ)
                             result = !result;
 
                         /* Branch is never taken, don't bother doing anything. */
                         if (result) {
-                            frame.forgetEverything();
+                            frame.syncAndForgetEverything();
                             Jump j = masm.jump();
                             jumpAndTrace(j, target);
                         }
                     }
                 } else {
                     emitStubCmpOp(stub, target, fused);
                 }
             } else {
@@ -945,33 +945,33 @@ mjit::Compiler::generateMethod()
           END_CASE(JSOP_TRUE)
 
           BEGIN_CASE(JSOP_OR)
           BEGIN_CASE(JSOP_AND)
             jsop_andor(op, PC + GET_JUMP_OFFSET(PC));
           END_CASE(JSOP_AND)
 
           BEGIN_CASE(JSOP_TABLESWITCH)
-            frame.forgetEverything();
+            frame.syncAndForgetEverything();
             masm.move(ImmPtr(PC), Registers::ArgReg1);
 
-            /* prepareStubCall() is not needed due to forgetEverything() */
+            /* prepareStubCall() is not needed due to syncAndForgetEverything() */
             stubCall(stubs::TableSwitch);
             frame.pop();
 
             masm.jump(Registers::ReturnReg);
             PC += js_GetVariableBytecodeLength(PC);
             break;
           END_CASE(JSOP_TABLESWITCH)
 
           BEGIN_CASE(JSOP_LOOKUPSWITCH)
-            frame.forgetEverything();
+            frame.syncAndForgetEverything();
             masm.move(ImmPtr(PC), Registers::ArgReg1);
 
-            /* prepareStubCall() is not needed due to forgetEverything() */
+            /* prepareStubCall() is not needed due to syncAndForgetEverything() */
             stubCall(stubs::LookupSwitch);
             frame.pop();
 
             masm.jump(Registers::ReturnReg);
             PC += js_GetVariableBytecodeLength(PC);
             break;
           END_CASE(JSOP_LOOKUPSWITCH)
 
@@ -1360,17 +1360,17 @@ mjit::Compiler::generateMethod()
             // VMFrame::fp to the correct fp for the entry point. We need to copy
             // that value here to FpReg so that FpReg also has the correct sp.
             // Otherwise, we would simply be using a stale FpReg value.
             if (analysis[PC].exceptionEntry)
                 restoreFrameRegs(masm);
 
             /* For now, don't bother doing anything for this opcode. */
             JSObject *obj = script->getObject(fullAtomIndex(PC));
-            frame.forgetEverything();
+            frame.syncAndForgetEverything();
             masm.move(ImmPtr(obj), Registers::ArgReg1);
             uint32 n = js_GetEnterBlockStackDefs(cx, script, PC);
             stubCall(stubs::EnterBlock);
             frame.enterBlock(n);
           }
           END_CASE(JSOP_ENTERBLOCK)
 
           BEGIN_CASE(JSOP_LEAVEBLOCK)
@@ -1625,25 +1625,25 @@ mjit::Compiler::emitReturn()
      * However, it's an optimization to throw it away early - the tracker
      * won't be spilled on further exits or join points.
      */
     if (fun) {
         if (fun->isHeavyweight()) {
             /* There will always be a call object. */
             prepareStubCall(Uses(0));
             stubCall(stubs::PutCallObject);
-            frame.throwaway();
+            frame.discardFrame();
         } else {
             /* if (callobj) ... */
             Jump callObj = masm.branchPtr(Assembler::NotEqual,
                                           Address(JSFrameReg, JSStackFrame::offsetCallObj()),
                                           ImmPtr(0));
             stubcc.linkExit(callObj, Uses(frame.frameDepth()));
 
-            frame.throwaway();
+            frame.discardFrame();
 
             stubcc.leave();
             stubcc.call(stubs::PutCallObject);
             Jump j = stubcc.masm.jump();
 
             /* if (arguments) ... */
             Jump argsObj = masm.branchPtr(Assembler::NotEqual,
                                           Address(JSFrameReg, JSStackFrame::offsetArgsObj()),
@@ -1771,17 +1771,19 @@ mjit::Compiler::inlineCallHelper(uint32 
         }
     }
 
     /*
      * We rely on the fact that syncAndKill() is not allowed to touch the
      * registers we've preserved.
      */
     frame.syncAndKill(Registers(Registers::AvailRegs), Uses(argc + 2));
-    frame.resetRegState();
+    frame.unpinKilledReg(data);
+    if (typeReg.isSet())
+        frame.unpinKilledReg(typeReg.reg());
 
     Label invoke = stubcc.masm.label();
 
 #ifdef JS_MONOIC
     mic.stubEntry = invoke;
     mic.dataReg = data;
 #endif
 
@@ -1992,17 +1994,17 @@ mjit::Compiler::emitStubCmpOp(BoolStub s
     frame.pop();
 
     if (!target) {
         frame.takeReg(Registers::ReturnReg);
         frame.pushTypedPayload(JSVAL_TYPE_BOOLEAN, Registers::ReturnReg);
     } else {
         JS_ASSERT(fused == JSOP_IFEQ || fused == JSOP_IFNE);
 
-        frame.forgetEverything();
+        frame.syncAndForgetEverything();
         Assembler::Condition cond = (fused == JSOP_IFEQ)
                                     ? Assembler::Zero
                                     : Assembler::NonZero;
         Jump j = masm.branchTest32(cond, Registers::ReturnReg,
                                    Registers::ReturnReg);
         jumpAndTrace(j, target);
     }
 }
@@ -3416,17 +3418,17 @@ mjit::Compiler::iterMore()
     Jump notFast = masm.branchPtr(Assembler::NotEqual, T1, ImmPtr(&js_IteratorClass));
     stubcc.linkExitForBranch(notFast);
 
     /* Get private from iter obj. */
     masm.loadFunctionPrivate(reg, T1);
 
     /* Get props_cursor, test */
     RegisterID T2 = frame.allocReg();
-    frame.forgetEverything();
+    frame.syncAndForgetEverything();
     masm.loadPtr(Address(T1, offsetof(NativeIterator, props_cursor)), T2);
     masm.loadPtr(Address(T1, offsetof(NativeIterator, props_end)), T1);
     Jump jFast = masm.branchPtr(Assembler::LessThan, T2, T1);
 
     jsbytecode *target = &PC[JSOP_MOREITER_LENGTH];
     JSOp next = JSOp(*target);
     JS_ASSERT(next == JSOP_IFNE || next == JSOP_IFNEX);
 
@@ -3653,18 +3655,17 @@ mjit::Compiler::jsop_setgname(uint32 ind
         objReg = frame.allocReg();
 
         masm.load32FromImm(&obj->objShape, objReg);
         shapeGuard = masm.branch32WithPatch(Assembler::NotEqual, objReg,
                                             Imm32(int32(JSObjectMap::INVALID_SHAPE)),
                                             mic.shape);
         masm.move(ImmPtr(obj), objReg);
     } else {
-        objReg = frame.tempRegForData(objFe);
-        frame.pinReg(objReg);
+        objReg = frame.copyDataIntoReg(objFe);
         RegisterID reg = frame.allocReg();
 
         masm.loadShape(objReg, reg);
         shapeGuard = masm.branch32WithPatch(Assembler::NotEqual, reg,
                                             Imm32(int32(JSObjectMap::INVALID_SHAPE)),
                                             mic.shape);
         frame.freeReg(reg);
     }
@@ -3748,18 +3749,17 @@ mjit::Compiler::jsop_setgname(uint32 ind
      * used. Since we only need to patch the last instruction in
      * both paths above, remember the distance between the
      * load label and after the instruction to be patched.
      */
     mic.patchValueOffset = masm.differenceBetween(mic.load, masm.label());
     JS_ASSERT(mic.patchValueOffset == masm.differenceBetween(mic.load, masm.label()));
 #endif
 
-    if (objFe->isConstant())
-        frame.freeReg(objReg);
+    frame.freeReg(objReg);
     frame.popn(2);
     if (mic.u.name.dataConst) {
         frame.push(v);
     } else {
         if (mic.u.name.typeConst)
             frame.pushTypedPayload(typeTag, dataReg);
         else
             frame.pushRegs(typeReg, dataReg);
--- a/js/src/methodjit/FastArithmetic.cpp
+++ b/js/src/methodjit/FastArithmetic.cpp
@@ -1005,17 +1005,17 @@ mjit::Compiler::jsop_relational_int(JSOp
 
         frame.pop();
         frame.pop();
 
         /*
          * Note: this resets the regster allocator, so rr and lr don't need
          * to be freed. We're not going to touch the frame.
          */
-        frame.forgetEverything();
+        frame.syncAndForgetEverything();
 
         /* Invert the test for IFEQ. */
         if (fused == JSOP_IFEQ) {
             switch (cond) {
               case Assembler::Equal:
                 cond = Assembler::NotEqual;
                 break;
               case Assembler::NotEqual:
@@ -1208,17 +1208,17 @@ mjit::Compiler::jsop_relational_double(J
         if (lhsNotNumber.isSet())
             stubcc.linkExitForBranch(lhsNotNumber.get());
         if (rhsNotNumber.isSet())
             stubcc.linkExitForBranch(rhsNotNumber.get());
         stubcc.leave();
         stubcc.call(stub);
 
         frame.popn(2);
-        frame.forgetEverything();
+        frame.syncAndForgetEverything();
 
         Jump j = masm.branchDouble(dblCond, fpLeft, fpRight);
 
         /*
          * The stub call has no need to rejoin since the state is synced.
          * Instead, we can just test the return value.
          */
         Assembler::Condition cond = (fused == JSOP_IFEQ)
@@ -1375,16 +1375,21 @@ mjit::Compiler::jsop_relational_full(JSO
         }
 
         /* Forget the world, preserving data. */
         frame.pinReg(cmpReg);
         if (reg.isSet())
             frame.pinReg(reg.reg());
         
         frame.popn(2);
+
+        frame.syncAndKillEverything();
+        frame.unpinKilledReg(cmpReg);
+        if (reg.isSet())
+            frame.unpinKilledReg(reg.reg());
         frame.forgetEverything();
         
         /* Operands could have been reordered, so use cmpOp. */
         Assembler::Condition i32Cond;
         bool ifeq = fused == JSOP_IFEQ;
         switch (cmpOp) {
           case JSOP_GT:
             i32Cond = ifeq ? Assembler::LessThanOrEqual : Assembler::GreaterThan;
--- a/js/src/methodjit/FastOps.cpp
+++ b/js/src/methodjit/FastOps.cpp
@@ -573,17 +573,17 @@ mjit::Compiler::jsop_equality(JSOp op, B
         frame.pop();
 
         /*
          * :FIXME: Easier test for undefined || null?
          * Maybe put them next to each other, subtract, do a single compare?
          */
 
         if (target) {
-            frame.forgetEverything();
+            frame.syncAndForgetEverything();
 
             if ((op == JSOP_EQ && fused == JSOP_IFNE) ||
                 (op == JSOP_NE && fused == JSOP_IFEQ)) {
                 Jump j = masm.branchPtr(Assembler::Equal, reg, ImmType(JSVAL_TYPE_UNDEFINED));
                 jumpAndTrace(j, target);
                 j = masm.branchPtr(Assembler::Equal, reg, ImmType(JSVAL_TYPE_NULL));
                 jumpAndTrace(j, target);
             } else {
@@ -812,18 +812,17 @@ mjit::Compiler::booleanJumpScript(JSOp o
 
     MaybeRegisterID type;
     MaybeRegisterID data;
 
     if (!fe->isTypeKnown() && !frame.shouldAvoidTypeRemat(fe))
         type.setReg(frame.copyTypeIntoReg(fe));
     data.setReg(frame.copyDataIntoReg(fe));
 
-    /* :FIXME: Can something more lightweight be used? */
-    frame.forgetEverything();
+    frame.syncAndForgetEverything();
 
     Assembler::Condition cond = (op == JSOP_IFNE || op == JSOP_OR)
                                 ? Assembler::NonZero
                                 : Assembler::Zero;
     Assembler::Condition ncond = (op == JSOP_IFNE || op == JSOP_OR)
                                  ? Assembler::Zero
                                  : Assembler::NonZero;
 
@@ -900,17 +899,17 @@ mjit::Compiler::jsop_ifneq(JSOp op, jsby
     if (fe->isConstant()) {
         JSBool b = js_ValueToBoolean(fe->getValue());
 
         frame.pop();
 
         if (op == JSOP_IFEQ)
             b = !b;
         if (b) {
-            frame.forgetEverything();
+            frame.syncAndForgetEverything();
             jumpAndTrace(masm.jump(), target);
         }
         return;
     }
 
     booleanJumpScript(op, target);
 }
 
@@ -920,17 +919,17 @@ mjit::Compiler::jsop_andor(JSOp op, jsby
     FrameEntry *fe = frame.peek(-1);
 
     if (fe->isConstant()) {
         JSBool b = js_ValueToBoolean(fe->getValue());
         
         /* Short-circuit. */
         if ((op == JSOP_OR && b == JS_TRUE) ||
             (op == JSOP_AND && b == JS_FALSE)) {
-            frame.forgetEverything();
+            frame.syncAndForgetEverything();
             jumpAndTrace(masm.jump(), target);
         }
 
         frame.pop();
         return;
     }
 
     booleanJumpScript(op, target);
--- a/js/src/methodjit/FrameState-inl.h
+++ b/js/src/methodjit/FrameState-inl.h
@@ -81,45 +81,53 @@ FrameState::haveSameBacking(FrameEntry *
         rhs = rhs->copyOf();
     return lhs == rhs;
 }
 
 inline JSC::MacroAssembler::RegisterID
 FrameState::allocReg()
 {
     RegisterID reg;
-    if (!freeRegs.empty())
+    if (!freeRegs.empty()) {
         reg = freeRegs.takeAnyReg();
-    else
+    } else {
         reg = evictSomeReg();
-    regstate[reg].fe = NULL;
+        regstate[reg].forget();
+    }
+
     return reg;
 }
 
 inline JSC::MacroAssembler::RegisterID
 FrameState::allocReg(uint32 mask)
 {
     RegisterID reg;
-    if (freeRegs.hasRegInMask(mask))
+    if (freeRegs.hasRegInMask(mask)) {
         reg = freeRegs.takeRegInMask(mask);
-    else
+    } else {
         reg = evictSomeReg(mask);
-    regstate[reg].fe = NULL;
+        regstate[reg].forget();
+    }
+
     return reg;
 }
 
 inline JSC::MacroAssembler::RegisterID
 FrameState::allocReg(FrameEntry *fe, RematInfo::RematType type)
 {
     RegisterID reg;
-    if (!freeRegs.empty())
+    if (!freeRegs.empty()) {
         reg = freeRegs.takeAnyReg();
-    else
+    } else {
         reg = evictSomeReg();
-    regstate[reg] = RegisterState(fe, type);
+        regstate[reg].forget();
+    }
+
+    regstate[reg].associate(fe, type);
+
     return reg;
 }
 
 inline void
 FrameState::emitLoadTypeTag(FrameEntry *fe, RegisterID reg) const
 {
     emitLoadTypeTag(this->masm, fe, reg);
 }
@@ -164,35 +172,40 @@ FrameState::pop()
         return;
 
     forgetAllRegs(fe);
 }
 
 inline void
 FrameState::freeReg(RegisterID reg)
 {
-    JS_ASSERT(regstate[reg].fe == NULL);
+    JS_ASSERT(!regstate[reg].usedBy());
+
     freeRegs.putReg(reg);
 }
 
 inline void
 FrameState::forgetReg(RegisterID reg)
 {
     /*
      * Important: Do not touch the fe here. We can peephole optimize away
      * loads and stores by re-using the contents of old FEs.
      */
-    JS_ASSERT_IF(regstate[reg].fe, !regstate[reg].fe->isCopy());
-    freeRegs.putReg(reg);
+    JS_ASSERT_IF(regstate[reg].fe(), !regstate[reg].fe()->isCopy());
+
+    if (!regstate[reg].isPinned()) {
+        regstate[reg].forget();
+        freeRegs.putReg(reg);
+    }
 }
 
 inline void
-FrameState::forgetEverything(uint32 newStackDepth)
+FrameState::syncAndForgetEverything(uint32 newStackDepth)
 {
-    forgetEverything();
+    syncAndForgetEverything();
     sp = spBase + newStackDepth;
 }
 
 inline FrameEntry *
 FrameState::rawPush()
 {
     JS_ASSERT(unsigned(sp - entries) < nargs + script->nslots);
 
@@ -231,17 +244,17 @@ FrameState::pushSynced(JSValueType type,
 {
     FrameEntry *fe = rawPush();
 
     fe->resetUnsynced();
     fe->type.sync();
     fe->data.sync();
     fe->setType(type);
     fe->data.setRegister(reg);
-    regstate[reg] = RegisterState(fe, RematInfo::DATA);
+    regstate[reg].associate(fe, RematInfo::DATA);
 }
 
 inline void
 FrameState::push(Address address)
 {
     FrameEntry *fe = rawPush();
 
     /* :XXX: X64 */
@@ -270,31 +283,31 @@ FrameState::pushRegs(RegisterID type, Re
 {
     JS_ASSERT(!freeRegs.hasReg(type) && !freeRegs.hasReg(data));
 
     FrameEntry *fe = rawPush();
 
     fe->resetUnsynced();
     fe->type.setRegister(type);
     fe->data.setRegister(data);
-    regstate[type] = RegisterState(fe, RematInfo::TYPE);
-    regstate[data] = RegisterState(fe, RematInfo::DATA);
+    regstate[type].associate(fe, RematInfo::TYPE);
+    regstate[data].associate(fe, RematInfo::DATA);
 }
 
 inline void
 FrameState::pushTypedPayload(JSValueType type, RegisterID payload)
 {
     JS_ASSERT(!freeRegs.hasReg(payload));
 
     FrameEntry *fe = rawPush();
 
     fe->resetUnsynced();
     fe->setType(type);
     fe->data.setRegister(payload);
-    regstate[payload] = RegisterState(fe, RematInfo::DATA);
+    regstate[payload].associate(fe, RematInfo::DATA);
 }
 
 inline void
 FrameState::pushNumber(MaybeRegisterID payload, bool asInt32)
 {
     JS_ASSERT_IF(payload.isSet(), !freeRegs.hasReg(payload.reg()));
 
     FrameEntry *fe = rawPush();
@@ -309,17 +322,17 @@ FrameState::pushNumber(MaybeRegisterID p
     } else {
         fe->type.setMemory();
     }
 
     fe->isNumber = true;
     if (payload.isSet()) {
         fe->data.unsync();
         fe->data.setRegister(payload.reg());
-        regstate[payload.reg()] = RegisterState(fe, RematInfo::DATA);
+        regstate[payload.reg()].associate(fe, RematInfo::DATA);
     } else {
         fe->data.setMemory();
     }
 }
 
 inline void
 FrameState::pushInt32(RegisterID payload)
 {
@@ -328,17 +341,17 @@ FrameState::pushInt32(RegisterID payload
     JS_ASSERT(!fe->isNumber);
 
     masm.storeTypeTag(ImmType(JSVAL_TYPE_INT32), addressOf(fe));
     fe->type.setMemory();
 
     fe->isNumber = true;
     fe->data.unsync();
     fe->data.setRegister(payload);
-    regstate[payload] = RegisterState(fe, RematInfo::DATA);
+    regstate[payload].associate(fe, RematInfo::DATA);
 }
 
 inline void
 FrameState::pushUntypedPayload(JSValueType type, RegisterID payload)
 {
     JS_ASSERT(!freeRegs.hasReg(payload));
 
     FrameEntry *fe = rawPush();
@@ -351,23 +364,23 @@ FrameState::pushUntypedPayload(JSValueTy
 #ifdef DEBUG
     fe->type.unsync();
 #endif
     fe->type.setMemory();
     fe->data.unsync();
     fe->setNotCopied();
     fe->setCopyOf(NULL);
     fe->data.setRegister(payload);
-    regstate[payload] = RegisterState(fe, RematInfo::DATA);
+    regstate[payload].associate(fe, RematInfo::DATA);
 }
 
 inline JSC::MacroAssembler::RegisterID
 FrameState::tempRegForType(FrameEntry *fe, RegisterID fallback)
 {
-    JS_ASSERT(regstate[fallback].fe == NULL);
+    JS_ASSERT(!regstate[fallback].fe());
     if (fe->isCopy())
         fe = fe->copyOf();
 
     JS_ASSERT(!fe->type.isConstant());
 
     if (fe->type.inRegister())
         return fe->type.reg();
 
@@ -424,25 +437,25 @@ FrameState::tempRegInMaskForData(FrameEn
 
     RegisterID reg;
     if (fe->data.inRegister()) {
         RegisterID old = fe->data.reg();
         if (Registers::maskReg(old) & mask)
             return old;
 
         /* Keep the old register pinned. */
-        regstate[old].fe = NULL;
+        regstate[old].forget();
         reg = allocReg(mask);
         masm.move(old, reg);
         freeReg(old);
     } else {
         reg = allocReg(mask);
         masm.loadPayload(addressOf(fe), reg);
     }
-    regstate[reg] = RegisterState(fe, RematInfo::DATA);
+    regstate[reg].associate(fe, RematInfo::DATA);
     fe->data.setRegister(reg);
     return reg;
 }
 
 inline JSC::MacroAssembler::RegisterID
 FrameState::tempRegForData(FrameEntry *fe, RegisterID reg, Assembler &masm) const
 {
     JS_ASSERT(!fe->data.isConstant());
@@ -618,26 +631,30 @@ FrameState::getLocal(uint32 slot)
         fe->resetSynced();
     }
     return fe;
 }
 
 inline void
 FrameState::pinReg(RegisterID reg)
 {
-    JS_ASSERT(regstate[reg].fe);
-    regstate[reg].save = regstate[reg].fe;
-    regstate[reg].fe = NULL;
+    regstate[reg].pin();
 }
 
 inline void
 FrameState::unpinReg(RegisterID reg)
 {
-    JS_ASSERT(!regstate[reg].fe);
-    regstate[reg].fe = regstate[reg].save;
+    regstate[reg].unpin();
+}
+
+inline void
+FrameState::unpinKilledReg(RegisterID reg)
+{
+    regstate[reg].unpinUnsafe();
+    freeRegs.putReg(reg);
 }
 
 inline void
 FrameState::forgetAllRegs(FrameEntry *fe)
 {
     if (fe->type.inRegister())
         forgetReg(fe->type.reg());
     if (fe->data.inRegister())
--- a/js/src/methodjit/FrameState.cpp
+++ b/js/src/methodjit/FrameState.cpp
@@ -105,29 +105,30 @@ FrameState::init(uint32 nargs)
     return true;
 }
 
 void
 FrameState::takeReg(RegisterID reg)
 {
     if (freeRegs.hasReg(reg)) {
         freeRegs.takeReg(reg);
+        JS_ASSERT(!regstate[reg].usedBy());
     } else {
-        JS_ASSERT(regstate[reg].fe);
+        JS_ASSERT(regstate[reg].fe());
         evictReg(reg);
+        regstate[reg].forget();
     }
-    regstate[reg].fe = NULL;
 }
 
 void
 FrameState::evictReg(RegisterID reg)
 {
-    FrameEntry *fe = regstate[reg].fe;
+    FrameEntry *fe = regstate[reg].fe();
 
-    if (regstate[reg].type == RematInfo::TYPE) {
+    if (regstate[reg].type() == RematInfo::TYPE) {
         if (!fe->type.synced()) {
             syncType(fe, addressOf(fe), masm);
             fe->type.sync();
         }
         fe->type.setMemory();
     } else {
         if (!fe->data.synced()) {
             syncData(fe, addressOf(fe), masm);
@@ -148,63 +149,81 @@ FrameState::evictSomeReg(uint32 mask)
     for (uint32 i = 0; i < JSC::MacroAssembler::TotalRegisters; i++) {
         RegisterID reg = RegisterID(i);
 
         /* Register is not allocatable, don't bother.  */
         if (!(Registers::maskReg(reg) & mask))
             continue;
 
         /* Register is not owned by the FrameState. */
-        FrameEntry *fe = regstate[i].fe;
+        FrameEntry *fe = regstate[i].fe();
         if (!fe)
             continue;
 
         /* Try to find a candidate... that doesn't need spilling. */
 #ifdef DEBUG
         fallbackSet = true;
 #endif
         fallback = reg;
 
-        if (regstate[i].type == RematInfo::TYPE && fe->type.synced()) {
+        if (regstate[i].type() == RematInfo::TYPE && fe->type.synced()) {
             fe->type.setMemory();
             return fallback;
         }
-        if (regstate[i].type == RematInfo::DATA && fe->data.synced()) {
+        if (regstate[i].type() == RematInfo::DATA && fe->data.synced()) {
             fe->data.setMemory();
             return fallback;
         }
     }
 
     JS_ASSERT(fallbackSet);
 
     evictReg(fallback);
     return fallback;
 }
 
 
 void
-FrameState::forgetEverything()
+FrameState::syncAndForgetEverything()
 {
     syncAndKill(Registers(Registers::AvailRegs), Uses(frameDepth()));
-
-    throwaway();
+    forgetEverything();
 }
 
-
 void
-FrameState::throwaway()
+FrameState::resetInternalState()
 {
     for (uint32 i = 0; i < tracker.nentries; i++)
         tracker[i]->untrack();
 
     tracker.reset();
     freeRegs.reset();
 }
 
 void
+FrameState::discardFrame()
+{
+    resetInternalState();
+
+    memset(regstate, 0, sizeof(regstate));
+}
+
+void
+FrameState::forgetEverything()
+{
+    resetInternalState();
+
+#ifdef DEBUG
+    for (uint32 i = 0; i < JSC::MacroAssembler::TotalRegisters; i++) {
+        JS_ASSERT(!regstate[i].usedBy());
+    }
+#endif
+}
+
+void
 FrameState::storeTo(FrameEntry *fe, Address address, bool popped)
 {
     if (fe->isConstant()) {
         masm.storeValue(fe->getValue(), address);
         return;
     }
 
     if (fe->isCopy())
@@ -265,25 +284,31 @@ FrameState::assertValidRegisterState() c
         JS_ASSERT_IF(fe->isCopy(), !fe->type.inRegister() && !fe->data.inRegister());
         JS_ASSERT_IF(fe->isCopy(), fe->copyOf() < tos);
         JS_ASSERT_IF(fe->isCopy(), fe->copyOf()->isCopied());
 
         if (fe->isCopy())
             continue;
         if (fe->type.inRegister()) {
             checkedFreeRegs.takeReg(fe->type.reg());
-            JS_ASSERT(regstate[fe->type.reg()].fe == fe);
+            JS_ASSERT(regstate[fe->type.reg()].fe() == fe);
         }
         if (fe->data.inRegister()) {
             checkedFreeRegs.takeReg(fe->data.reg());
-            JS_ASSERT(regstate[fe->data.reg()].fe == fe);
+            JS_ASSERT(regstate[fe->data.reg()].fe() == fe);
         }
     }
 
     JS_ASSERT(checkedFreeRegs == freeRegs);
+
+    for (uint32 i = 0; i < JSC::MacroAssembler::TotalRegisters; i++) {
+        JS_ASSERT(!regstate[i].isPinned());
+        JS_ASSERT_IF(regstate[i].fe(), !freeRegs.hasReg(RegisterID(i)));
+        JS_ASSERT_IF(regstate[i].fe(), regstate[i].fe()->isTracked());
+    }
 }
 #endif
 
 void
 FrameState::syncFancy(Assembler &masm, Registers avail, uint32 resumeAt,
                       FrameEntry *bottom) const
 {
     /* :TODO: can be resumeAt? */
@@ -400,44 +425,38 @@ FrameState::syncAndKill(Registers kill, 
             syncData(backing, address, masm);
             fe->data.sync();
             if (fe->isConstant() && !fe->type.synced())
                 fe->type.sync();
         }
         if (killData) {
             JS_ASSERT(backing == fe);
             JS_ASSERT(fe->data.synced());
-            if (regstate[fe->data.reg()].fe)
+            if (regstate[fe->data.reg()].fe())
                 forgetReg(fe->data.reg());
             fe->data.setMemory();
         }
         bool killType = fe->type.inRegister() && kill.hasReg(fe->type.reg());
         if (!fe->type.synced() && (killType || fe >= bottom)) {
             if (backing != fe && backing->type.inMemory())
                 tempRegForType(backing);
             syncType(backing, address, masm);
             fe->type.sync();
         }
         if (killType) {
             JS_ASSERT(backing == fe);
             JS_ASSERT(fe->type.synced());
-            if (regstate[fe->type.reg()].fe)
+            if (regstate[fe->type.reg()].fe())
                 forgetReg(fe->type.reg());
             fe->type.setMemory();
         }
     }
 }
 
 void
-FrameState::resetRegState()
-{
-    freeRegs = Registers();
-}
-
-void
 FrameState::merge(Assembler &masm, Changes changes) const
 {
     FrameEntry *tos = tosFe();
     Registers temp(Registers::TempRegs);
 
     for (uint32 i = 0; i < tracker.nentries; i++) {
         FrameEntry *fe = tracker[i];
         if (fe >= tos)
@@ -486,19 +505,19 @@ FrameState::copyDataIntoReg(FrameEntry *
         if (freeRegs.empty()) {
             if (!fe->data.synced())
                 syncData(fe, addressOf(fe), masm);
             fe->data.setMemory();
         } else {
             reg = allocReg();
             masm.move(hint, reg);
             fe->data.setRegister(reg);
-            regstate[reg] = regstate[hint];
+            regstate[reg].associate(regstate[hint].fe(), RematInfo::DATA);
         }
-        regstate[hint].fe = NULL;
+        regstate[hint].forget();
     } else {
         pinReg(reg);
         takeReg(hint);
         unpinReg(reg);
         masm.move(reg, hint);
     }
 }
 
@@ -511,17 +530,17 @@ FrameState::copyDataIntoReg(Assembler &m
         fe = fe->copyOf();
 
     if (fe->data.inRegister()) {
         RegisterID reg = fe->data.reg();
         if (freeRegs.empty()) {
             if (!fe->data.synced())
                 syncData(fe, addressOf(fe), masm);
             fe->data.setMemory();
-            regstate[reg].fe = NULL;
+            regstate[reg].forget();
         } else {
             RegisterID newReg = allocReg();
             masm.move(reg, newReg);
             reg = newReg;
         }
         return reg;
     }
 
@@ -544,17 +563,17 @@ FrameState::copyTypeIntoReg(FrameEntry *
         fe = fe->copyOf();
 
     if (fe->type.inRegister()) {
         RegisterID reg = fe->type.reg();
         if (freeRegs.empty()) {
             if (!fe->type.synced())
                 syncType(fe, addressOf(fe), masm);
             fe->type.setMemory();
-            regstate[reg].fe = NULL;
+            regstate[reg].forget();
         } else {
             RegisterID newReg = allocReg();
             masm.move(reg, newReg);
             reg = newReg;
         }
         return reg;
     }
 
@@ -631,30 +650,31 @@ FrameState::ownRegForType(FrameEntry *fe
         }
 
         if (freeRegs.empty()) {
             /* For now... just steal the register that already exists. */
             if (!backing->type.synced())
                 syncType(backing, addressOf(backing), masm);
             reg = backing->type.reg();
             backing->type.setMemory();
-            moveOwnership(reg, NULL);
+            regstate[reg].forget();
         } else {
             reg = allocReg();
             masm.move(backing->type.reg(), reg);
         }
         return reg;
     }
 
     if (fe->type.inRegister()) {
         reg = fe->type.reg();
+
         /* Remove ownership of this register. */
-        JS_ASSERT(regstate[reg].fe == fe);
-        JS_ASSERT(regstate[reg].type == RematInfo::TYPE);
-        regstate[reg].fe = NULL;
+        JS_ASSERT(regstate[reg].fe() == fe);
+        JS_ASSERT(regstate[reg].type() == RematInfo::TYPE);
+        regstate[reg].forget();
         fe->type.invalidate();
     } else {
         JS_ASSERT(fe->type.inMemory());
         reg = allocReg();
         masm.loadTypeTag(addressOf(fe), reg);
     }
     return reg;
 }
@@ -674,17 +694,17 @@ FrameState::ownRegForData(FrameEntry *fe
         }
 
         if (freeRegs.empty()) {
             /* For now... just steal the register that already exists. */
             if (!backing->data.synced())
                 syncData(backing, addressOf(backing), masm);
             reg = backing->data.reg();
             backing->data.setMemory();
-            moveOwnership(reg, NULL);
+            regstate[reg].forget();
         } else {
             reg = allocReg();
             masm.move(backing->data.reg(), reg);
         }
         return reg;
     }
 
     if (fe->isCopied()) {
@@ -694,19 +714,19 @@ FrameState::ownRegForData(FrameEntry *fe
             fe->data.invalidate();
             return copyDataIntoReg(copy);
         }
     }
     
     if (fe->data.inRegister()) {
         reg = fe->data.reg();
         /* Remove ownership of this register. */
-        JS_ASSERT(regstate[reg].fe == fe);
-        JS_ASSERT(regstate[reg].type == RematInfo::DATA);
-        regstate[reg].fe = NULL;
+        JS_ASSERT(regstate[reg].fe() == fe);
+        JS_ASSERT(regstate[reg].type() == RematInfo::DATA);
+        regstate[reg].forget();
         fe->data.invalidate();
     } else {
         JS_ASSERT(fe->data.inMemory());
         reg = allocReg();
         masm.loadPayload(addressOf(fe), reg);
     }
     return reg;
 }
@@ -835,26 +855,26 @@ FrameState::uncopy(FrameEntry *original)
          * If the copy is unsynced, and the original is in memory,
          * give the original a register. We do this below too; it's
          * okay if it's spilled.
          */
         if (original->type.inMemory() && !fe->type.synced())
             tempRegForType(original);
         fe->type.inherit(original->type);
         if (fe->type.inRegister())
-            moveOwnership(fe->type.reg(), fe);
+            regstate[fe->type.reg()].reassociate(fe);
     } else {
         JS_ASSERT(fe->isTypeKnown());
         JS_ASSERT(fe->getKnownType() == original->getKnownType());
     }
     if (original->data.inMemory() && !fe->data.synced())
         tempRegForData(original);
     fe->data.inherit(original->data);
     if (fe->data.inRegister())
-        moveOwnership(fe->data.reg(), fe);
+        regstate[fe->data.reg()].reassociate(fe);
 
     return fe;
 }
 
 void
 FrameState::storeLocal(uint32 n, bool popGuaranteed, bool typeChange)
 {
     FrameEntry *localFe = getLocal(n);
@@ -971,25 +991,25 @@ FrameState::storeLocal(uint32 n, bool po
         swapInTracker(backing, localFe);
 
     /*
      * Move the backing store down - we spill registers here, but we could be
      * smarter and re-use the type reg.
      */
     RegisterID reg = tempRegForData(backing);
     localFe->data.setRegister(reg);
-    moveOwnership(reg, localFe);
+    regstate[reg].reassociate(localFe);
 
     if (typeChange) {
         if (backing->isTypeKnown()) {
             localFe->setType(backing->getKnownType());
         } else {
             RegisterID reg = tempRegForType(backing);
             localFe->type.setRegister(reg);
-            moveOwnership(reg, localFe);
+            regstate[reg].reassociate(localFe);
         }
     } else {
         if (!wasSynced)
             masm.storeTypeTag(ImmType(backing->getKnownType()), addressOf(localFe));
         localFe->type.setMemory();
     }
 
     if (!backing->isTypeKnown())
--- a/js/src/methodjit/FrameState.h
+++ b/js/src/methodjit/FrameState.h
@@ -170,32 +170,110 @@ class FrameState
             JS_ASSERT(n < nentries);
             return entries[n];
         }
 
         FrameEntry **entries;
         uint32 nentries;
     };
 
+    /*
+     * Some RegisterState invariants.
+     *
+     *  - Both |fe| and |save| cannot be non-NULL.
+     *  - If either |fe| or |save| is non-NULL, the register is not in freeRegs.
+     *  - If both |fe| and |save| are NULL, the register is either in freeRegs,
+     *    or allocated by the Compiler (and thus not tracked as part of a Value).
+     */
     struct RegisterState {
-        RegisterState()
+        RegisterState() : fe_(NULL), save_(NULL)
         { }
 
         RegisterState(FrameEntry *fe, RematInfo::RematType type)
-          : fe(fe), type(type)
-        { }
+          : fe_(fe), save_(NULL), type_(type)
+        {
+            JS_ASSERT(!save_);
+        }
+
+        bool isPinned() const {
+            assertConsistency();
+            return !!save_;
+        }
+
+        void assertConsistency() const {
+            JS_ASSERT_IF(fe_, !save_);
+            JS_ASSERT_IF(save_, !fe_);
+        }
+
+        FrameEntry *fe() const {
+            assertConsistency();
+            return fe_;
+        }
+
+        RematInfo::RematType type() const {
+            assertConsistency();
+            return type_;
+        }
+
+        FrameEntry *usedBy() const {
+            if (fe_)
+                return fe_;
+            return save_;
+        }
+
+        void associate(FrameEntry *fe, RematInfo::RematType type) {
+            JS_ASSERT(!fe_);
+            JS_ASSERT(!save_);
 
+            fe_ = fe;
+            type_ = type;
+            JS_ASSERT(!save_);
+        }
+
+        /* Change ownership. */
+        void reassociate(FrameEntry *fe) {
+            assertConsistency();
+            JS_ASSERT(fe);
+
+            fe_ = fe;
+        }
+
+        /* Unassociate this register from the FE. */
+        void forget() {
+            JS_ASSERT(fe_);
+            fe_ = NULL;
+            JS_ASSERT(!save_);
+        }
+
+        void pin() {
+            assertConsistency();
+            save_ = fe_;
+            fe_ = NULL;
+        }
+
+        void unpin() {
+            assertConsistency();
+            fe_ = save_;
+            save_ = NULL;
+        }
+
+        void unpinUnsafe() {
+            assertConsistency();
+            save_ = NULL;
+        }
+
+      private:
         /* FrameEntry owning this register, or NULL if not owned by a frame. */
-        FrameEntry *fe;
+        FrameEntry *fe_;
 
         /* Hack - simplifies register allocation for pairs. */
-        FrameEntry *save;
+        FrameEntry *save_;
         
         /* Part of the FrameEntry that owns the FE. */
-        RematInfo::RematType type;
+        RematInfo::RematType type_;
     };
 
   public:
     FrameState(JSContext *cx, JSScript *script, Assembler &masm);
     ~FrameState();
     bool init(uint32 nargs);
 
     /*
@@ -505,38 +583,44 @@ class FrameState
     void sync(Assembler &masm, Uses uses) const;
 
     /*
      * Syncs all outstanding stores to memory and possibly kills regs in the
      * process.
      */
     void syncAndKill(Registers kill, Uses uses); 
 
-    /*
-     * Reset the register state.
-     */
-    void resetRegState();
+    /* Syncs and kills everything. */
+    void syncAndKillEverything() {
+        syncAndKill(Registers(Registers::AvailRegs), Uses(frameDepth()));
+    }
 
     /*
      * Clear all tracker entries, syncing all outstanding stores in the process.
      * The stack depth is in case some merge points' edges did not immediately
      * precede the current instruction.
      */
-    inline void forgetEverything(uint32 newStackDepth);
+    inline void syncAndForgetEverything(uint32 newStackDepth);
 
     /*
      * Same as above, except the stack depth is not changed. This is used for
      * branching opcodes.
      */
+    void syncAndForgetEverything();
+
+    /*
+     * Throw away the entire frame state, without syncing anything.
+     * This can only be called after a syncAndKill() against all registers.
+     */
     void forgetEverything();
 
     /*
-     * Throw away the entire frame state, without syncing anything.
+     * Discard the entire framestate forcefully.
      */
-    void throwaway();
+    void discardFrame();
 
     /*
      * Mark an existing slot with a type.
      */
     inline void learnType(FrameEntry *fe, JSValueType type);
 
     /*
      * Forget a type, syncing in the process.
@@ -582,27 +666,33 @@ class FrameState
     /*
      * Helper function. Tests if a slot's type is primitve. Condition should
      * be Equal or NotEqual.
      */
     inline Jump testPrimitive(Assembler::Condition cond, FrameEntry *fe);
 
     /*
      * Marks a register such that it cannot be spilled by the register
-     * allocator. Any pinned registers must be unpinned at the end of the op.
-     * Note: This function should only be used on registers tied to FEs.
+     * allocator. Any pinned registers must be unpinned at the end of the op,
+     * no matter what. In addition, pinReg() can only be used on registers
+     * which are associated with FrameEntries.
      */
     inline void pinReg(RegisterID reg);
 
     /*
      * Unpins a previously pinned register.
      */
     inline void unpinReg(RegisterID reg);
 
     /*
+     * Same as unpinReg(), but does not restore the FrameEntry.
+     */
+    inline void unpinKilledReg(RegisterID reg);
+
+    /*
      * Dups the top item on the stack.
      */
     inline void dup();
 
     /*
      * Dups the top 2 items on the stack.
      */
     inline void dup2();
@@ -677,36 +767,33 @@ class FrameState
     inline FrameEntry *getLocal(uint32 slot);
     inline void forgetAllRegs(FrameEntry *fe);
     inline void swapInTracker(FrameEntry *lhs, FrameEntry *rhs);
     inline uint32 localIndex(uint32 n);
     void pushCopyOf(uint32 index);
     void syncFancy(Assembler &masm, Registers avail, uint32 resumeAt,
                    FrameEntry *bottom) const;
     inline bool tryFastDoubleLoad(FrameEntry *fe, FPRegisterID fpReg, Assembler &masm) const;
+    void resetInternalState();
 
     /*
      * "Uncopies" the backing store of a FrameEntry that has been copied. The
      * original FrameEntry is not invalidated; this is the responsibility of
      * the caller. The caller can check isCopied() to see if the registers
      * were moved to a copy.
      *
      * Later addition: uncopy() returns the first copy found.
      */
     FrameEntry *uncopy(FrameEntry *original);
 
     FrameEntry *entryFor(uint32 index) const {
         JS_ASSERT(entries[index].isTracked());
         return &entries[index];
     }
 
-    void moveOwnership(RegisterID reg, FrameEntry *newFe) {
-        regstate[reg].fe = newFe;
-    }
-
     RegisterID evictSomeReg() {
         return evictSomeReg(Registers::AvailRegs);
     }
 
     uint32 indexOf(int32 depth) {
         return uint32((sp + depth) - entries);
     }
 
--- a/js/src/methodjit/ImmutableSync.cpp
+++ b/js/src/methodjit/ImmutableSync.cpp
@@ -87,17 +87,17 @@ ImmutableSync::allocReg()
         RegisterID reg = RegisterID(i);
         if (!(Registers::maskReg(reg) & Registers::AvailRegs))
             continue;
 
         lastResort = 0;
 
         if (!regs[i]) {
             /* If the frame does not own this register, take it! */
-            FrameEntry *fe = frame.regstate[i].fe;
+            FrameEntry *fe = frame.regstate[i].fe();
             if (!fe)
                 return reg;
 
             /*
              * The Reifier does not own this register, but the frame does.
              * This must mean that we've not yet processed this entry, and
              * that it's data has not been clobbered.
              */
@@ -110,19 +110,19 @@ ImmutableSync::allocReg()
              * That's about as good as it gets, so just break out now.
              */
             if (!fe->isCopied())
                 break;
         }
     }
 
     if (evictFromFrame != FrameState::InvalidIndex) {
-        FrameEntry *fe = frame.regstate[evictFromFrame].fe;
+        FrameEntry *fe = frame.regstate[evictFromFrame].fe();
         SyncEntry &e = entryFor(fe);
-        if (frame.regstate[evictFromFrame].type == RematInfo::TYPE) {
+        if (frame.regstate[evictFromFrame].type() == RematInfo::TYPE) {
             JS_ASSERT(!e.typeClobbered);
             e.typeSynced = true;
             e.typeClobbered = true;
             masm->storeTypeTag(fe->type.reg(), frame.addressOf(fe));
         } else {
             JS_ASSERT(!e.dataClobbered);
             e.dataSynced = true;
             e.dataClobbered = true;
@@ -267,20 +267,20 @@ ImmutableSync::syncNormal(FrameEntry *fe
             masm->storeTypeTag(ImmType(e.type), addr);
         else
             masm->storeTypeTag(ensureTypeReg(fe, e), addr);
     }
 
     if (e.hasDataReg) {
         avail.putReg(e.dataReg);
         regs[e.dataReg] = NULL;
-    } else if (!e.dataClobbered && fe->data.inRegister() && frame.regstate[fe->data.reg()].fe) {
+    } else if (!e.dataClobbered && fe->data.inRegister() && frame.regstate[fe->data.reg()].fe()) {
         avail.putReg(fe->data.reg());
     }
 
     if (e.hasTypeReg) {
         avail.putReg(e.typeReg);
         regs[e.typeReg] = NULL;
-    } else if (!e.typeClobbered && fe->type.inRegister() && frame.regstate[fe->type.reg()].fe) {
+    } else if (!e.typeClobbered && fe->type.inRegister() && frame.regstate[fe->type.reg()].fe()) {
         avail.putReg(fe->type.reg());
     }
 }