Bug 904827 - Part 2: Implement JSNative setter calls in SetPropertyIC. (r=djvj)
authorEric Faust <efaustbmo@gmail.com>
Fri, 30 Aug 2013 18:50:36 -0700
changeset 153138 6f82c0e76f2d2039a5c85b1f51f8efec767d0f32
parent 153137 587ec77a2373eb139f94141d76b6229f602ae5e4
child 153139 c1ccfd8f31bf1310797c02c0babca78d4c3c0e27
push idunknown
push userunknown
push dateunknown
reviewersdjvj
bugs904827
milestone26.0a1
Bug 904827 - Part 2: Implement JSNative setter calls in SetPropertyIC. (r=djvj)
js/src/jit/IonCaches.cpp
--- a/js/src/jit/IonCaches.cpp
+++ b/js/src/jit/IonCaches.cpp
@@ -1968,16 +1968,48 @@ SetPropertyIC::attachNativeExisting(JSCo
         masm.storeConstantOrRegister(value(), addr);
     }
 
     attacher.jumpRejoin(masm);
 
     return linkAndAttachStub(cx, masm, attacher, ion, "setting");
 }
 
+static bool
+IsCacheableSetPropCallNative(HandleObject obj, HandleObject holder, HandleShape shape)
+{
+    if (!shape || !IsCacheableProtoChain(obj, holder))
+        return false;
+
+    return shape->hasSetterValue() && shape->setterObject()->is<JSFunction>() &&
+           shape->setterObject()->as<JSFunction>().isNative();
+}
+
+static bool
+IsCacheableSetPropCallPropertyOp(HandleObject obj, HandleObject holder,
+                                 HandleShape shape)
+{
+    if (!shape)
+        return false;
+
+    if (!IsCacheableProtoChain(obj, holder))
+        return false;
+
+    if (shape->hasSlot())
+        return false;
+
+    if (shape->hasDefaultSetter())
+        return false;
+
+    if (shape->hasSetterValue())
+        return false;
+
+    return true;
+}
+
 bool
 SetPropertyIC::attachSetterCall(JSContext *cx, IonScript *ion,
                                 HandleObject obj, HandleObject holder, HandleShape shape,
                                 void *returnAddr)
 {
     MacroAssembler masm(cx);
     RepatchStubAppender attacher(*this);
 
@@ -2033,88 +2065,139 @@ SetPropertyIC::attachSetterCall(JSContex
     // so leave it alone.
     RegisterSet regSet(RegisterSet::All());
     regSet.take(AnyRegister(object()));
 
     // This is a slower stub path, and we're going to be doing a call anyway.  Don't need
     // to try so hard to not use the stack.  Scratch regs are just taken from the register
     // set not including the input, current value saved on the stack, and restored when
     // we're done with it.
+    //
+    // Be very careful not to use any of these before value() is pushed, since they
+    // might shadow.
     Register scratchReg     = regSet.takeGeneral();
     Register argJSContextReg = regSet.takeGeneral();
-    Register argObjReg       = regSet.takeGeneral();
-    Register argIdReg        = regSet.takeGeneral();
-    Register argStrictReg    = regSet.takeGeneral();
     Register argVpReg        = regSet.takeGeneral();
 
+    bool callNative = IsCacheableSetPropCallNative(obj, holder, shape);
+    JS_ASSERT_IF(!callNative, IsCacheableSetPropCallPropertyOp(obj, holder, shape));
+
     // Ensure stack is aligned.
     DebugOnly<uint32_t> initialStack = masm.framePushed();
 
-    attacher.pushStubCodePointer(masm);
-
-    StrictPropertyOp target = shape->setterOp();
-    JS_ASSERT(target);
-    // JSStrictPropertyOp: bool fn(JSContext *cx, HandleObject obj,
-    //                               HandleId id, bool strict, MutableHandleValue vp);
-
-    // Push args on stack first so we can take pointers to make handles.
-    if (value().constant())
-        masm.Push(value().value());
-    else
-        masm.Push(value().reg());
-    masm.movePtr(StackPointer, argVpReg);
-
-    masm.move32(Imm32(strict() ? 1 : 0), argStrictReg);
-
-    // push canonical jsid from shape instead of propertyname.
-    RootedId propId(cx);
-    if (!shape->getUserId(cx, &propId))
-        return false;
-    masm.Push(propId, argIdReg);
-    masm.movePtr(StackPointer, argIdReg);
-
-    masm.Push(object());
-    masm.movePtr(StackPointer, argObjReg);
-
-    masm.loadJSContext(argJSContextReg);
-
-    if (!masm.buildOOLFakeExitFrame(returnAddr))
-        return false;
-    masm.enterFakeExitFrame(ION_FRAME_OOL_PROPERTY_OP);
-
-    // Make the call.
-    masm.setupUnalignedABICall(5, scratchReg);
-    masm.passABIArg(argJSContextReg);
-    masm.passABIArg(argObjReg);
-    masm.passABIArg(argIdReg);
-    masm.passABIArg(argStrictReg);
-    masm.passABIArg(argVpReg);
-    masm.callWithABI(JS_FUNC_TO_DATA_PTR(void *, target));
+    if (callNative) {
+        JS_ASSERT(shape->hasSetterValue() && shape->setterObject() &&
+                  shape->setterObject()->is<JSFunction>());
+        JSFunction *target = &shape->setterObject()->as<JSFunction>();
+
+        JS_ASSERT(target->isNative());
+
+        Register argUintNReg = regSet.takeGeneral();
+
+        // Set up the call:
+        //  bool (*)(JSContext *, unsigned, Value *vp)
+        // vp[0] is callee/outparam
+        // vp[1] is |this|
+        // vp[2] is the value
+
+        // Build vp and move the base into argVpReg.
+        masm.Push(value());
+        masm.Push(TypedOrValueRegister(MIRType_Object, AnyRegister(object())));
+        masm.Push(ObjectValue(*target));
+        masm.movePtr(StackPointer, argVpReg);
+
+        // Preload other regs
+        masm.loadJSContext(argJSContextReg);
+        masm.move32(Imm32(1), argUintNReg);
+
+        // Push data for GC marking
+        masm.Push(argUintNReg);
+        attacher.pushStubCodePointer(masm);
+
+        if (!masm.buildOOLFakeExitFrame(returnAddr))
+            return false;
+        masm.enterFakeExitFrame(ION_FRAME_OOL_NATIVE);
+
+        // Make the call
+        masm.setupUnalignedABICall(3, scratchReg);
+        masm.passABIArg(argJSContextReg);
+        masm.passABIArg(argUintNReg);
+        masm.passABIArg(argVpReg);
+        masm.callWithABI(JS_FUNC_TO_DATA_PTR(void *, target->native()));
+    } else {
+        Register argObjReg       = regSet.takeGeneral();
+        Register argIdReg        = regSet.takeGeneral();
+        Register argStrictReg    = regSet.takeGeneral();
+
+        attacher.pushStubCodePointer(masm);
+
+        StrictPropertyOp target = shape->setterOp();
+        JS_ASSERT(target);
+        // JSStrictPropertyOp: bool fn(JSContext *cx, HandleObject obj,
+        //                               HandleId id, bool strict, MutableHandleValue vp);
+
+        // Push args on stack first so we can take pointers to make handles.
+        if (value().constant())
+            masm.Push(value().value());
+        else
+            masm.Push(value().reg());
+        masm.movePtr(StackPointer, argVpReg);
+
+        masm.move32(Imm32(strict() ? 1 : 0), argStrictReg);
+
+        // push canonical jsid from shape instead of propertyname.
+        RootedId propId(cx);
+        if (!shape->getUserId(cx, &propId))
+            return false;
+        masm.Push(propId, argIdReg);
+        masm.movePtr(StackPointer, argIdReg);
+
+        masm.Push(object());
+        masm.movePtr(StackPointer, argObjReg);
+
+        masm.loadJSContext(argJSContextReg);
+
+        if (!masm.buildOOLFakeExitFrame(returnAddr))
+            return false;
+        masm.enterFakeExitFrame(ION_FRAME_OOL_PROPERTY_OP);
+
+        // Make the call.
+        masm.setupUnalignedABICall(5, scratchReg);
+        masm.passABIArg(argJSContextReg);
+        masm.passABIArg(argObjReg);
+        masm.passABIArg(argIdReg);
+        masm.passABIArg(argStrictReg);
+        masm.passABIArg(argVpReg);
+        masm.callWithABI(JS_FUNC_TO_DATA_PTR(void *, target));
+    }
 
     // Test for failure.
     masm.branchIfFalseBool(ReturnReg, masm.exceptionLabel());
 
     // The next instruction is removing the footer of the exit frame, so there
     // is no need for leaveFakeExitFrame.
 
     // Move the StackPointer back to its original location, unwinding the exit frame.
-    masm.adjustStack(IonOOLPropertyOpExitFrameLayout::Size());
+    if (callNative)
+        masm.adjustStack(IonOOLNativeExitFrameLayout::Size(1));
+    else
+        masm.adjustStack(IonOOLPropertyOpExitFrameLayout::Size());
     JS_ASSERT(masm.framePushed() == initialStack);
 
     // restoreLive()
     masm.PopRegsInMask(liveRegs_);
 
     // Rejoin jump.
     attacher.jumpRejoin(masm);
 
     // Jump to next stub.
     masm.bind(&failure);
     attacher.jumpNextStub(masm);
 
-    return linkAndAttachStub(cx, masm, attacher, ion, "calling");
+    return linkAndAttachStub(cx, masm, attacher, ion, "setter call");
 }
 
 bool
 SetPropertyIC::attachNativeAdding(JSContext *cx, IonScript *ion, JSObject *obj,
                                   HandleShape oldShape, HandleShape newShape,
                                   HandleShape propShape)
 {
     MacroAssembler masm(cx);
@@ -2211,43 +2294,16 @@ IsPropertySetInlineable(JSContext *cx, H
         return false;
 
     pshape.set(shape);
 
     return true;
 }
 
 static bool
-IsPropertySetterCallInlineable(JSContext *cx, HandleObject obj, HandleObject holder,
-                               HandleShape shape)
-{
-    if (!shape)
-        return false;
-
-    if (!holder->isNative())
-        return false;
-
-    if (shape->hasSlot())
-        return false;
-
-    if (shape->hasDefaultSetter())
-        return false;
-
-    if (!shape->writable())
-        return false;
-
-    // We only handle propertyOps for now, so fail if we have SetterValue
-    // (which implies JSNative setter).
-    if (shape->hasSetterValue())
-        return false;
-
-    return true;
-}
-
-static bool
 IsPropertyAddInlineable(JSContext *cx, HandleObject obj, HandleId id, uint32_t oldSlots,
                         MutableHandleShape pShape)
 {
     // This is not a Add, the property exists.
     if (pShape.get())
         return false;
 
     RootedShape shape(cx, obj->nativeLookup(cx, id));
@@ -2320,17 +2376,19 @@ SetPropertyIC::update(JSContext *cx, siz
             if (!cache.attachNativeExisting(cx, ion, obj, shape))
                 return false;
             addedSetterStub = true;
         } else {
             RootedObject holder(cx);
             if (!JSObject::lookupProperty(cx, obj, name, &holder, &shape))
                 return false;
 
-            if (IsPropertySetterCallInlineable(cx, obj, holder, shape)) {
+            if (IsCacheableSetPropCallPropertyOp(obj, holder, shape) ||
+                IsCacheableSetPropCallNative(obj, holder, shape))
+            {
                 if (!cache.attachSetterCall(cx, ion, obj, holder, shape, returnAddr))
                     return false;
                 addedSetterStub = true;
             }
         }
     }
 
     uint32_t oldSlots = obj->numDynamicSlots();