Bug 738525 - Add IC for getters backed by a JSNative. r=bhackett
authorJan de Mooij <jdemooij@mozilla.com>
Mon, 07 May 2012 21:17:48 +0200
changeset 93405 e763ef9f3d5d08b01168a6db2ddab4d6f839db89
parent 93404 1dbfff7cf0ab8fd028d32a754b9aafefc7481a95
child 93406 f63a2c79ad1b8b92dc11372a8cf55ba5f296631d
push id22634
push useremorley@mozilla.com
push dateTue, 08 May 2012 09:48:43 +0000
treeherdermozilla-central@e4f9e2eab6b1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs738525
milestone15.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 738525 - Add IC for getters backed by a JSNative. r=bhackett
js/src/jit-test/tests/jaeger/bug738525.js
js/src/methodjit/BaseAssembler.h
js/src/methodjit/PolyIC.cpp
js/src/shell/js.cpp
js/src/vm/Stack.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/jaeger/bug738525.js
@@ -0,0 +1,37 @@
+// Test IC for getters backed by a JSNative.
+function test1() {
+    for (var i = 0; i < 60; i++) {
+        assertEq(it.customNative, undefined);
+    }
+
+    var res = 0;
+    for (var i = 0; i < 60; i++) {
+        it.customNative = i;
+        res += it.customNative;
+    }
+
+    assertEq(res, 1770);
+}
+function test2() {
+    function getValue() {
+        return it.customNative;
+    }
+
+    for (var i = 0; i < 60; i++) {
+        it.customNative = i;
+        assertEq(getValue(), i);
+    }
+
+    for (var i = 0; i < 60; i++) {
+        it.customNative = null;
+        assertEq(getValue(), null);
+
+        delete it["customNativ" + "e"];
+        assertEq(getValue(), undefined);
+        assertEq(it.customNative, undefined);
+    }
+}
+if ("it" in this) {
+    test1();
+    test2();
+}
--- a/js/src/methodjit/BaseAssembler.h
+++ b/js/src/methodjit/BaseAssembler.h
@@ -562,16 +562,27 @@ static const JSC::MacroAssembler::Regist
         if (Registers::regForArg(callConvention, i, &to)) {
             move(imm, to);
             availInCall.takeRegUnchecked(to);
         } else {
             storePtr(imm, addressOfArg(i));
         }
     }
 
+    void storeArg(uint32_t i, Imm32 imm) {
+        JS_ASSERT(callIsAligned);
+        RegisterID to;
+        if (Registers::regForArg(callConvention, i, &to)) {
+            move(imm, to);
+            availInCall.takeRegUnchecked(to);
+        } else {
+            store32(imm, addressOfArg(i));
+        }
+    }
+
     // High-level call helper, given an optional function pointer and a
     // calling convention. setupABICall() must have been called beforehand,
     // as well as each numbered argument stored with storeArg().
     //
     // After callWithABI(), the call state is reset, so a new call may begin.
     Call callWithABI(void *fun, bool canThrow) {
 #ifdef JS_CPU_ARM
         // the repatcher requires that these instructions are adjacent in
--- a/js/src/methodjit/PolyIC.cpp
+++ b/js/src/methodjit/PolyIC.cpp
@@ -750,18 +750,21 @@ struct GetPropHelper {
         if (!IsCacheableProtoChain(obj, holder))
             return ic.disable(f, "non-native holder");
         shape = (const Shape *)prop;
         return Lookup_Cacheable;
     }
 
     LookupStatus testForGet() {
         if (!shape->hasDefaultGetter()) {
-            if (shape->hasGetterValue())
-                return ic.disable(f, "getter value shape");
+            if (shape->hasGetterValue()) {
+                JSObject *getterObj = shape->getterObject();
+                if (!getterObj->isFunction() || !getterObj->toFunction()->isNative())
+                    return ic.disable(f, "getter object not a native function");
+            }
             if (shape->hasSlot() && holder != obj)
                 return ic.disable(f, "slotful getter hook through prototype");
             if (!ic.canCallHook)
                 return ic.disable(f, "can't call getter hook");
             if (f.regs.inlined()) {
                 /*
                  * As with native stubs, getter hook stubs can't be
                  * generated for inline frames. Mark the inner function
@@ -1048,16 +1051,17 @@ class GetPropCompiler : public PICStubCo
         repatcher.repatch(labels.getInlineShapeData(pic.getFastShapeGuard()), obj->lastProperty());
         repatcher.patchAddressOffsetForValueLoad(labels.getValueLoad(pic.fastPathRejoin), offset);
 
         pic.inlinePathPatched = true;
 
         return Lookup_Cacheable;
     }
 
+    /* For JSPropertyOp getters. */
     void generateGetterStub(Assembler &masm, const Shape *shape, jsid userid,
                             Label start, Vector<Jump, 8> &shapeMismatches)
     {
         /*
          * Getter hook needs to be called from the stub. The state is fully
          * synced and no registers are live except the result registers.
          */
         JS_ASSERT(pic.canCallHook);
@@ -1080,29 +1084,29 @@ class GetPropCompiler : public PICStubCo
         }
 
         RegisterID t0 = tempRegs.takeAnyReg().reg();
         masm.bumpStubCount(f.script(), f.pc(), t0);
 
         /*
          * Initialize vp, which is either a slot in the object (the holder,
          * actually, which must equal the object here) or undefined.
-         * Use vp == sp (which for CALLPROP will actually be the original
-         * sp + 1), to avoid clobbering stack values.
+         * Use vp == sp to avoid clobbering stack values.
          */
         int32_t vpOffset = (char *) f.regs.sp - (char *) f.fp();
         if (shape->hasSlot()) {
             masm.loadObjProp(obj, holdObjReg, shape,
                              Registers::ClobberInCall, t0);
             masm.storeValueFromComponents(Registers::ClobberInCall, t0, Address(JSFrameReg, vpOffset));
         } else {
             masm.storeValue(UndefinedValue(), Address(JSFrameReg, vpOffset));
         }
 
-        int32_t initialFrameDepth = f.regs.sp - f.fp()->slots();
+        /* sp + 1 to avoid clobbering vp if the getter calls scripted functions. */
+        int32_t initialFrameDepth = f.regs.sp + 1 - f.fp()->slots();
         masm.setupFallibleABICall(cx->typeInferenceEnabled(), f.regs.pc, initialFrameDepth);
 
         /* Grab cx. */
 #ifdef JS_CPU_X86
         RegisterID cxReg = tempRegs.takeAnyReg().reg();
 #else
         RegisterID cxReg = Registers::ArgReg0;
 #endif
@@ -1134,16 +1138,108 @@ class GetPropCompiler : public PICStubCo
             return;
         }
 
         linker.patchJump(pic.fastPathRejoin);
 
         linkerEpilogue(linker, start, shapeMismatches);
     }
 
+    /* For getters backed by a JSNative. */
+    void generateNativeGetterStub(Assembler &masm, const Shape *shape,
+                                  Label start, Vector<Jump, 8> &shapeMismatches)
+    {
+        /*
+         * Getter hook needs to be called from the stub. The state is fully
+         * synced and no registers are live except the result registers.
+         */
+        JS_ASSERT(pic.canCallHook);
+
+        JSFunction *fun = shape->getterObject()->toFunction();
+        Native native = fun->native();
+
+        masm.storePtr(ImmPtr((void *) REJOIN_NATIVE_GETTER),
+                      FrameAddress(offsetof(VMFrame, stubRejoin)));
+
+        Registers tempRegs = Registers::tempCallRegMask();
+        if (tempRegs.hasReg(Registers::ClobberInCall))
+            tempRegs.takeReg(Registers::ClobberInCall);
+
+        /* Get a register to hold obj while we set up the rest of the frame. */
+        RegisterID holdObjReg = pic.objReg;
+        if (tempRegs.hasReg(pic.objReg)) {
+            tempRegs.takeReg(pic.objReg);
+        } else {
+            holdObjReg = tempRegs.takeAnyReg().reg();
+            masm.move(pic.objReg, holdObjReg);
+        }
+
+        RegisterID t0 = tempRegs.takeAnyReg().reg();
+        masm.bumpStubCount(f.script(), f.pc(), t0);
+
+        /*
+         * A JSNative has the following signature:
+         *
+         *   JSBool native(JSContext *cx, unsigned argc, Value *vp);
+         *
+         * Since we are calling a getter, argc is always 0. vp must point to two
+         * values, the callee and the holder. We use vp == sp to avoid clobbering
+         * stack values.
+         */
+        int32_t vpOffset = (char *) f.regs.sp - (char *) f.fp();
+
+        masm.storeValue(ObjectValue(*fun), Address(JSFrameReg, vpOffset));
+        masm.storeValueFromComponents(ImmType(JSVAL_TYPE_OBJECT), holdObjReg,
+                                      Address(JSFrameReg, vpOffset + sizeof(js::Value)));
+
+        /*
+         * sp + 2 to avoid clobbering vp[0] and vp[1] if the getter calls
+         * scripted functions.
+         */
+        int32_t initialFrameDepth = f.regs.sp + 2 - f.fp()->slots();
+        masm.setupFallibleABICall(cx->typeInferenceEnabled(), f.regs.pc, initialFrameDepth);
+
+        /* Grab cx. */
+#ifdef JS_CPU_X86
+        RegisterID cxReg = tempRegs.takeAnyReg().reg();
+#else
+        RegisterID cxReg = Registers::ArgReg0;
+#endif
+        masm.loadPtr(FrameAddress(offsetof(VMFrame, cx)), cxReg);
+
+        /* Grap vp. */
+        RegisterID vpReg = t0;
+        masm.addPtr(Imm32(vpOffset), JSFrameReg, vpReg);
+
+        masm.restoreStackBase();
+        masm.setupABICall(Registers::NormalCall, 3);
+        masm.storeArg(2, vpReg);
+        masm.storeArg(1, Imm32(0)); // argc
+        masm.storeArg(0, cxReg);
+
+        masm.callWithABI(JS_FUNC_TO_DATA_PTR(void *, native), false);
+
+        NativeStubLinker::FinalJump done;
+        if (!NativeStubEpilogue(f, masm, &done, 0, vpOffset, pic.shapeReg, pic.objReg))
+            return;
+        NativeStubLinker linker(masm, f.chunk(), f.regs.pc, done);
+        if (!linker.init(f.cx))
+            THROW();
+
+        if (!linker.verifyRange(pic.lastCodeBlock(f.chunk())) ||
+            !linker.verifyRange(f.chunk())) {
+            disable("code memory is out of range");
+            return;
+        }
+
+        linker.patchJump(pic.fastPathRejoin);
+
+        linkerEpilogue(linker, start, shapeMismatches);
+    }
+
     LookupStatus generateStub(JSObject *holder, const Shape *shape)
     {
         Vector<Jump, 8> shapeMismatches(cx);
 
         Assembler masm;
 
         Label start;
         Jump shapeGuardJump;
@@ -1196,21 +1292,24 @@ class GetPropCompiler : public PICStubCo
                 return error();
 
             pic.secondShapeGuard = masm.distanceOf(masm.label()) - masm.distanceOf(start);
         } else {
             pic.secondShapeGuard = 0;
         }
 
         if (!shape->hasDefaultGetter()) {
-            jsid userid;
-            if (!shape->getUserId(cx, &userid))
-                return error();
-
-            generateGetterStub(masm, shape, userid, start, shapeMismatches);
+            if (shape->hasGetterValue()) {
+                generateNativeGetterStub(masm, shape, start, shapeMismatches);
+            } else {
+                jsid userid;
+                if (!shape->getUserId(cx, &userid))
+                    return error();
+                generateGetterStub(masm, shape, userid, start, shapeMismatches);
+            }
             if (setStubShapeOffset)
                 pic.getPropLabels().setStubShapeJump(masm, start, stubShapeJumpLabel);
             return Lookup_Cacheable;
         }
 
         /* Load the value out of the object. */
         masm.loadObjProp(holder, holderReg, shape, pic.shapeReg, pic.objReg);
         Jump done = masm.jump();
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -3873,36 +3873,46 @@ Help(JSContext *cx, unsigned argc, jsval
 }
 
 /*
  * Define a JS object called "it".  Give it class operations that printf why
  * they're being called for tutorial purposes.
  */
 enum its_tinyid {
     ITS_COLOR, ITS_HEIGHT, ITS_WIDTH, ITS_FUNNY, ITS_ARRAY, ITS_RDONLY,
-    ITS_CUSTOM, ITS_CUSTOMRDONLY
+    ITS_CUSTOM, ITS_CUSTOMRDONLY, ITS_CUSTOMNATIVE
 };
 
 static JSBool
 its_getter(JSContext *cx, JSObject *obj, jsid id, jsval *vp);
 
 static JSBool
 its_setter(JSContext *cx, JSObject *obj, jsid id, JSBool strict, jsval *vp);
 
+static JSBool
+its_get_customNative(JSContext *cx, unsigned argc, jsval *vp);
+
+static JSBool
+its_set_customNative(JSContext *cx, unsigned argc, jsval *vp);
+
 static JSPropertySpec its_props[] = {
     {"color",           ITS_COLOR,      JSPROP_ENUMERATE,       NULL, NULL},
     {"height",          ITS_HEIGHT,     JSPROP_ENUMERATE,       NULL, NULL},
     {"width",           ITS_WIDTH,      JSPROP_ENUMERATE,       NULL, NULL},
     {"funny",           ITS_FUNNY,      JSPROP_ENUMERATE,       NULL, NULL},
     {"array",           ITS_ARRAY,      JSPROP_ENUMERATE,       NULL, NULL},
     {"rdonly",          ITS_RDONLY,     JSPROP_READONLY,        NULL, NULL},
     {"custom",          ITS_CUSTOM,     JSPROP_ENUMERATE,
                         its_getter,     its_setter},
     {"customRdOnly",    ITS_CUSTOMRDONLY, JSPROP_ENUMERATE | JSPROP_READONLY,
                         its_getter,     its_setter},
+    {"customNative",    ITS_CUSTOMNATIVE,
+                        JSPROP_ENUMERATE | JSPROP_NATIVE_ACCESSORS,
+                        (JSPropertyOp)its_get_customNative,
+                        (JSStrictPropertyOp)its_set_customNative },
     {NULL,0,0,NULL,NULL}
 };
 
 static JSBool its_noisy;    /* whether to be noisy when finalizing it */
 static JSBool its_enum_fail;/* whether to fail when enumerating it */
 
 static JSBool
 its_addProperty(JSContext *cx, JSObject *obj, jsid id, jsval *vp)
@@ -4093,16 +4103,68 @@ its_setter(JSContext *cx, JSObject *obj,
     }
 
     JS_SetPrivate(obj, (void*)val);
 
     *val = *vp;
     return JS_TRUE;
 }
 
+static JSBool
+its_get_customNative(JSContext *cx, unsigned argc, jsval *vp)
+{
+    JSObject *obj = JS_THIS_OBJECT(cx, vp);
+    if (!obj)
+        return false;
+
+    if (JS_GetClass(obj) == &its_class) {
+        jsval *val = (jsval *) JS_GetPrivate(obj);
+        *vp = val ? *val : JSVAL_VOID;
+    } else {
+        *vp = JSVAL_VOID;
+    }
+
+    return true;
+}
+
+static JSBool
+its_set_customNative(JSContext *cx, unsigned argc, jsval *vp)
+{
+    JSObject *obj = JS_THIS_OBJECT(cx, vp);
+    if (!obj)
+        return false;
+
+    if (JS_GetClass(obj) != &its_class)
+        return true;
+
+    jsval *argv = JS_ARGV(cx, vp);
+
+    jsval *val = (jsval *) JS_GetPrivate(obj);
+    if (val) {
+        *val = *argv;
+        return true;
+    }
+
+    val = new jsval;
+    if (!val) {
+        JS_ReportOutOfMemory(cx);
+        return false;
+    }
+
+    if (!JS_AddValueRoot(cx, val)) {
+        delete val;
+        return false;
+    }
+
+    JS_SetPrivate(obj, (void *)val);
+
+    *val = *argv;
+    return true;
+}
+
 JSErrorFormatString jsShell_ErrorFormatString[JSShellErr_Limit] = {
 #define MSG_DEF(name, number, count, exception, format) \
     { format, count, JSEXN_ERR } ,
 #include "jsshell.msg"
 #undef MSG_DEF
 };
 
 static const JSErrorFormatString *
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -1166,18 +1166,26 @@ StackIter::settleOnNewState()
                     state_ = IMPLICIT_NATIVE;
                     args_ = CallArgsFromVp(argc, vp);
                     return;
                 }
             }
 
             state_ = SCRIPTED;
             script_ = fp_->script();
-            JS_ASSERT_IF(op != JSOP_FUNAPPLY,
-                         sp_ >= fp_->base() && sp_ <= fp_->slots() + script_->nslots);
+
+            /*
+             * Check sp and pc. JM's getter ICs may push 2 extra values on the
+             * stack; this is okay since the methodjit reserves some extra slots
+             * for loop temporaries.
+             */
+            if (op == JSOP_GETPROP || op == JSOP_CALLPROP)
+                JS_ASSERT(sp_ >= fp_->base() && sp_ <= fp_->slots() + script_->nslots + 2);
+            else if (op != JSOP_FUNAPPLY)
+                JS_ASSERT(sp_ >= fp_->base() && sp_ <= fp_->slots() + script_->nslots);
             JS_ASSERT(pc_ >= script_->code && pc_ < script_->code + script_->length);
             return;
         }
 
         /*
          * A CallArgsList element is pushed for any call to Invoke, regardless
          * of whether the callee is a scripted function or even a callable
          * object. Thus, it is necessary to filter calleev for natives.