Bug 708303 - Use pinReg/unpinReg more in write barriers (r=dmandelin)
authorBill McCloskey <wmccloskey@mozilla.com>
Mon, 16 Jan 2012 13:52:57 -0800
changeset 85823 60eb0da71cdb391b45cfb1ddca7c01e6c6c09501
parent 85822 e1c5a79f27cf84114d4e11cd8b37428c8416c85d
child 85824 97deecdd51f1c0f309b2f30295f32796547927c6
push id805
push userakeybl@mozilla.com
push dateWed, 01 Feb 2012 18:17:35 +0000
treeherdermozilla-aurora@6fb3bf232436 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdmandelin
bugs708303
milestone12.0a1
Bug 708303 - Use pinReg/unpinReg more in write barriers (r=dmandelin)
js/src/methodjit/Compiler.cpp
js/src/methodjit/FastOps.cpp
js/src/methodjit/ImmutableSync.cpp
js/src/methodjit/MachineRegs.h
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -5025,17 +5025,17 @@ mjit::Compiler::jsop_setprop(PropertyNam
     /*
      * If this is a SETNAME to a variable of a non-reentrant outer function,
      * set the variable's slot directly for the active call object.
      */
     if (cx->typeInferenceEnabled() && js_CodeSpec[*PC].format & JOF_NAME) {
         ScriptAnalysis::NameAccess access =
             analysis->resolveNameAccess(cx, ATOM_TO_JSID(name), true);
         if (access.nesting) {
-            /* Use a SavedReg so it isn't clobbered by the stub call. */
+            /* Use a SavedReg so it isn't clobbered by sync or the stub call. */
             RegisterID nameReg = frame.allocReg(Registers::SavedRegs).reg();
             Address address = frame.loadNameAddress(access, nameReg);
 
 #ifdef JSGC_INCREMENTAL_MJ
             /* Write barrier. */
             if (cx->compartment->needsBarrier()) {
                 stubcc.linkExit(masm.jump(), Uses(0));
                 stubcc.leave();
@@ -5072,28 +5072,28 @@ mjit::Compiler::jsop_setprop(PropertyNam
             types->addFreeze(cx);
             uint32_t slot = propertyTypes->definiteSlot();
             RegisterID reg = frame.tempRegForData(lhs);
             bool isObject = lhs->isTypeKnown();
             MaybeJump notObject;
             if (!isObject)
                 notObject = frame.testObject(Assembler::NotEqual, lhs);
 #ifdef JSGC_INCREMENTAL_MJ
-            frame.pinReg(reg);
             if (cx->compartment->needsBarrier() && propertyTypes->needsBarrier(cx)) {
                 /* Write barrier. */
+                frame.pinReg(reg);
                 Jump j = masm.testGCThing(Address(reg, JSObject::getFixedSlotOffset(slot)));
                 stubcc.linkExit(j, Uses(0));
                 stubcc.leave();
                 stubcc.masm.addPtr(Imm32(JSObject::getFixedSlotOffset(slot)),
                                    reg, Registers::ArgReg1);
                 OOL_STUBCALL(stubs::GCThingWriteBarrier, REJOIN_NONE);
                 stubcc.rejoin(Changes(0));
+                frame.unpinReg(reg);
             }
-            frame.unpinReg(reg);
 #endif
             if (!isObject) {
                 stubcc.linkExit(notObject.get(), Uses(2));
                 stubcc.leave();
                 stubcc.masm.move(ImmPtr(name), Registers::ArgReg1);
                 OOL_STUBCALL(STRICT_VARIANT(stubs::SetName), REJOIN_FALLTHROUGH);
             }
             frame.storeTo(rhs, Address(reg, JSObject::getFixedSlotOffset(slot)), popGuaranteed);
--- a/js/src/methodjit/FastOps.cpp
+++ b/js/src/methodjit/FastOps.cpp
@@ -1166,27 +1166,36 @@ mjit::Compiler::jsop_setelem_dense()
      */
     types::TypeSet *types = frame.extra(obj).types;
     if (cx->compartment->needsBarrier() && (!types || types->propertyNeedsBarrier(cx, JSID_VOID))) {
         Label barrierStart = stubcc.masm.label();
         stubcc.linkExitDirect(masm.jump(), barrierStart);
 
         /*
          * The sync call below can potentially clobber key.reg() and slotsReg.
-         * So we save and restore them. Additionally, the WriteBarrier stub can
-         * clobber both registers. The rejoin call will restore key.reg() but
-         * not slotsReg. So we restore it again after the stub call.
+         * We pin key.reg() to avoid it being clobbered. If |hoisted| is true,
+         * we can also pin slotsReg. If not, then slotsReg is owned by the
+         * compiler and we save in manually to VMFrame::scratch.
+         *
+         * Additionally, the WriteBarrier stub can clobber both registers. The
+         * rejoin call will restore key.reg() but not slotsReg. So we save
+         * slotsReg in the frame and restore it after the stub call.
          */
         stubcc.masm.storePtr(slotsReg, FrameAddress(offsetof(VMFrame, scratch)));
+        if (hoisted)
+            frame.pinReg(slotsReg);
         if (!key.isConstant())
-            stubcc.masm.push(key.reg());
+            frame.pinReg(key.reg());
         frame.sync(stubcc.masm, Uses(3));
         if (!key.isConstant())
-            stubcc.masm.pop(key.reg());
-        stubcc.masm.loadPtr(FrameAddress(offsetof(VMFrame, scratch)), slotsReg);
+            frame.unpinReg(key.reg());
+        if (hoisted)
+            frame.unpinReg(slotsReg);
+        else
+            stubcc.masm.loadPtr(FrameAddress(offsetof(VMFrame, scratch)), slotsReg);
 
         if (key.isConstant())
             stubcc.masm.lea(Address(slotsReg, key.index() * sizeof(Value)), Registers::ArgReg1);
         else
             stubcc.masm.lea(BaseIndex(slotsReg, key.reg(), masm.JSVAL_SCALE), Registers::ArgReg1);
         OOL_STUBCALL(stubs::WriteBarrier, REJOIN_NONE);
         stubcc.masm.loadPtr(FrameAddress(offsetof(VMFrame, scratch)), slotsReg);
         stubcc.rejoin(Changes(0));
--- a/js/src/methodjit/ImmutableSync.cpp
+++ b/js/src/methodjit/ImmutableSync.cpp
@@ -43,17 +43,17 @@
 #include "FrameState.h"
 #include "FrameState-inl.h"
 #include "ImmutableSync.h"
 
 using namespace js;
 using namespace js::mjit;
 
 ImmutableSync::ImmutableSync()
-  : cx(NULL), entries(NULL), frame(NULL), avail(Registers::AvailRegs), generation(0)
+  : cx(NULL), entries(NULL), frame(NULL), avail(Registers::TempRegs), generation(0)
 {
 }
 
 ImmutableSync::~ImmutableSync()
 {
     if (cx)
         cx->free_(entries);
 }
@@ -66,17 +66,17 @@ ImmutableSync::init(JSContext *cx, const
 
     entries = (SyncEntry *)cx->calloc_(sizeof(SyncEntry) * nentries);
     return !!entries;
 }
 
 void
 ImmutableSync::reset(Assembler *masm, Registers avail, FrameEntry *top, FrameEntry *bottom)
 {
-    this->avail = avail;
+    this->avail = avail & Registers::TempRegs;
     this->masm = masm;
     this->top = top;
     this->bottom = bottom;
     this->generation++;
     memset(regs, 0, sizeof(regs));
 }
 
 inline JSC::MacroAssembler::RegisterID
@@ -86,17 +86,17 @@ ImmutableSync::doAllocReg()
         return avail.takeAnyReg().reg();
 
     uint32_t lastResort = FrameState::InvalidIndex;
     uint32_t evictFromFrame = FrameState::InvalidIndex;
 
     /* Find something to evict. */
     for (uint32_t i = 0; i < Registers::TotalRegisters; i++) {
         RegisterID reg = RegisterID(i);
-        if (!(Registers::maskReg(reg) & Registers::AvailRegs))
+        if (!(Registers::maskReg(reg) & Registers::TempRegs))
             continue;
 
         if (frame->regstate(reg).isPinned())
             continue;
 
         lastResort = i;
 
         if (!regs[i]) {
@@ -146,23 +146,24 @@ ImmutableSync::doAllocReg()
     return reg;
 }
 
 JSC::MacroAssembler::RegisterID
 ImmutableSync::allocReg()
 {
     RegisterID reg = doAllocReg();
     JS_ASSERT(!frame->regstate(reg).isPinned());
+    JS_ASSERT(!Registers::isSaved(reg));
     return reg;
 }
 
 void
 ImmutableSync::freeReg(JSC::MacroAssembler::RegisterID reg)
 {
-    if (!frame->regstate(reg).isPinned())
+    if (!frame->regstate(reg).isPinned() && !Registers::isSaved(reg))
         avail.putReg(reg);
 }
 
 inline ImmutableSync::SyncEntry &
 ImmutableSync::entryFor(FrameEntry *fe)
 {
     JS_ASSERT(fe <= top || frame->isTemporary(fe));
     SyncEntry &e = entries[fe - frame->entries];
--- a/js/src/methodjit/MachineRegs.h
+++ b/js/src/methodjit/MachineRegs.h
@@ -568,16 +568,20 @@ struct Registers {
     void takeRegUnchecked(AnyRegisterID reg) {
         freeMask &= ~(1 << reg.reg_);
     }
 
     bool operator ==(const Registers &other) {
         return freeMask == other.freeMask;
     }
 
+    Registers operator &(const Registers &other) {
+        return Registers(freeMask & other.freeMask);
+    }
+
     uint32_t freeMask;
 };
 
 static const JSC::MacroAssembler::RegisterID JSFrameReg = Registers::JSFrameReg;
 
 AnyRegisterID
 AnyRegisterID::fromRaw(unsigned reg_)
 {