From df98b251c6c587206da09185647ae2ee20faa0ff Mon Sep 17 00:00:00 2001 From: Brian Hackett Date: Fri, 20 Jan 2012 07:33:44 -0800 Subject: [PATCH] Backout 5cdf9574bede for build failures. --- js/src/frontend/BytecodeEmitter.cpp | 55 +++++++++++++++++++++--- js/src/jit-test/tests/basic/bug717249.js | 2 - js/src/jsarray.h | 22 ++++++++++ js/src/jsobj.cpp | 7 +-- js/src/jsobjinlines.h | 6 +++ js/src/methodjit/PolyIC.cpp | 18 ++------ 6 files changed, 85 insertions(+), 25 deletions(-) delete mode 100644 js/src/jit-test/tests/basic/bug717249.js diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 0a94c75442a6..77710c2dd358 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -1946,6 +1946,32 @@ EmitElemOpBase(JSContext *cx, BytecodeEmitter *bce, JSOp op) return true; } +static bool +EmitSpecialPropOp(JSContext *cx, ParseNode *pn, JSOp op, BytecodeEmitter *bce) +{ + /* + * Special case for obj.__proto__ to deoptimize away from fast paths in the + * interpreter and trace recorder, which skip dense array instances by + * going up to Array.prototype before looking up the property name. + */ + jsatomid index; + if (!bce->makeAtomIndex(pn->pn_atom, &index)) + return false; + if (!EmitIndexOp(cx, JSOP_QNAMEPART, index, bce)) + return false; + + if (op == JSOP_CALLELEM && Emit1(cx, bce, JSOP_DUP) < 0) + return false; + + if (!EmitElemOpBase(cx, bce, op)) + return false; + + if (op == JSOP_CALLELEM && Emit1(cx, bce, JSOP_SWAP) < 0) + return false; + + return true; +} + static bool EmitPropOp(JSContext *cx, ParseNode *pn, JSOp op, BytecodeEmitter *bce, JSBool callContext, JSOp *psuffix = NULL) @@ -1956,6 +1982,14 @@ EmitPropOp(JSContext *cx, ParseNode *pn, JSOp op, BytecodeEmitter *bce, JS_ASSERT(pn->isArity(PN_NAME)); pn2 = pn->maybeExpr(); + /* Special case deoptimization for __proto__. */ + if ((op == JSOP_GETPROP || op == JSOP_CALLPROP) && + pn->pn_atom == cx->runtime->atomState.protoAtom) { + if (pn2 && !EmitTree(cx, bce, pn2)) + return false; + return EmitSpecialPropOp(cx, pn, callContext ? JSOP_CALLELEM : JSOP_GETELEM, bce); + } + if (callContext) { JS_ASSERT(pn->isKind(PNK_DOT)); JS_ASSERT(op == JSOP_GETPROP); @@ -1997,8 +2031,13 @@ EmitPropOp(JSContext *cx, ParseNode *pn, JSOp op, BytecodeEmitter *bce, if (NewSrcNote2(cx, bce, SRC_PCBASE, bce->offset() - pndown->pn_offset) < 0) return false; - if (!EmitAtomOp(cx, pndot, pndot->getOp(), bce)) + /* Special case deoptimization on __proto__, as above. */ + if (pndot->isArity(PN_NAME) && pndot->pn_atom == cx->runtime->atomState.protoAtom) { + if (!EmitSpecialPropOp(cx, pndot, JSOP_GETELEM, bce)) + return false; + } else if (!EmitAtomOp(cx, pndot, pndot->getOp(), bce)) { return false; + } /* Reverse the pn_expr link again. */ pnup = pndot->pn_expr; @@ -3739,13 +3778,19 @@ EmitAssignment(JSContext *cx, BytecodeEmitter *bce, ParseNode *lhs, JSOp op, Par EMIT_UINT16_IMM_OP(lhs->isOp(JSOP_SETARG) ? JSOP_GETARG : JSOP_GETLOCAL, atomIndex); } break; - case PNK_DOT: { + case PNK_DOT: if (Emit1(cx, bce, JSOP_DUP) < 0) return false; - bool isLength = (lhs->pn_atom == cx->runtime->atomState.lengthAtom); - EMIT_INDEX_OP(isLength ? JSOP_LENGTH : JSOP_GETPROP, atomIndex); + if (lhs->pn_atom == cx->runtime->atomState.protoAtom) { + if (!EmitIndexOp(cx, JSOP_QNAMEPART, atomIndex, bce)) + return false; + if (!EmitElemOpBase(cx, bce, JSOP_GETELEM)) + return false; + } else { + bool isLength = (lhs->pn_atom == cx->runtime->atomState.lengthAtom); + EMIT_INDEX_OP(isLength ? JSOP_LENGTH : JSOP_GETPROP, atomIndex); + } break; - } case PNK_LB: case PNK_LP: #if JS_HAS_XML_SUPPORT diff --git a/js/src/jit-test/tests/basic/bug717249.js b/js/src/jit-test/tests/basic/bug717249.js deleted file mode 100644 index 48db74494f1e..000000000000 --- a/js/src/jit-test/tests/basic/bug717249.js +++ /dev/null @@ -1,2 +0,0 @@ -// |jit-test| error: TypeError -[].__proto__(); diff --git a/js/src/jsarray.h b/js/src/jsarray.h index 177ceaef2253..dc1ebfafcd78 100644 --- a/js/src/jsarray.h +++ b/js/src/jsarray.h @@ -74,6 +74,28 @@ js_IdIsIndex(jsid id, jsuint *indexp) return js::StringIsArrayIndex(JSID_TO_ATOM(id), indexp); } +/* + * Dense arrays are not native -- aobj->isNative() for a dense array aobj + * results in false, meaning aobj->map does not point to a js::Shape. + * + * But Array methods are called via aobj.sort(), e.g., and the interpreter and + * the trace recorder must consult the property cache in order to perform well. + * The cache works only for native objects. + * + * Therefore the interpreter (js_Interpret in JSOP_GETPROP and JSOP_CALLPROP) + * and js_GetPropertyHelper use this inline function to skip up one link in the + * prototype chain when obj is a dense array, in order to find a native object + * (to wit, Array.prototype) in which to probe for cached methods. + * + * Note that setting aobj.__proto__ for a dense array aobj turns aobj into a + * slow array, avoiding the neede to skip. + * + * Callers of js_GetProtoIfDenseArray must take care to use the original object + * (obj) for the |this| value of a getter, setter, or method call (bug 476447). + */ +inline JSObject * +js_GetProtoIfDenseArray(JSObject *obj); + extern JSObject * js_InitArrayClass(JSContext *cx, JSObject *obj); diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index 6a5a3cbe6d15..cea4c2edf4b3 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -5359,15 +5359,16 @@ static JS_ALWAYS_INLINE JSBool js_GetPropertyHelperInline(JSContext *cx, JSObject *obj, JSObject *receiver, jsid id, uint32_t getHow, Value *vp) { - JSObject *obj2; + JSObject *aobj, *obj2; JSProperty *prop; const Shape *shape; /* Convert string indices to integers if appropriate. */ id = js_CheckForStringIndex(id); + aobj = js_GetProtoIfDenseArray(obj); /* This call site is hot -- use the always-inlined variant of LookupPropertyWithFlags(). */ - if (!LookupPropertyWithFlagsInline(cx, obj, id, cx->resolveFlags, &obj2, &prop)) + if (!LookupPropertyWithFlagsInline(cx, aobj, id, cx->resolveFlags, &obj2, &prop)) return false; if (!prop) { @@ -5441,7 +5442,7 @@ js_GetPropertyHelperInline(JSContext *cx, JSObject *obj, JSObject *receiver, jsi shape = (Shape *) prop; if (getHow & JSGET_CACHE_RESULT) - JS_PROPERTY_CACHE(cx).fill(cx, obj, 0, obj2, shape); + JS_PROPERTY_CACHE(cx).fill(cx, aobj, 0, obj2, shape); /* This call site is hot -- use the always-inlined variant of js_NativeGet(). */ if (!js_NativeGetInline(cx, receiver, obj, obj2, shape, getHow, vp)) diff --git a/js/src/jsobjinlines.h b/js/src/jsobjinlines.h index 6b99623d1e51..f329af4c4714 100644 --- a/js/src/jsobjinlines.h +++ b/js/src/jsobjinlines.h @@ -1995,6 +1995,12 @@ js_InitClass(JSContext *cx, js::HandleObject obj, JSObject *parent_proto, JSObject **ctorp = NULL, js::gc::AllocKind ctorKind = JSFunction::FinalizeKind); +inline JSObject * +js_GetProtoIfDenseArray(JSObject *obj) +{ + return obj->isDenseArray() ? obj->getProto() : obj; +} + /* * js_PurgeScopeChain does nothing if obj is not itself a prototype or parent * scope, else it reshapes the scope and prototype chains it links. It calls diff --git a/js/src/methodjit/PolyIC.cpp b/js/src/methodjit/PolyIC.cpp index 4081da26793c..82278ae16398 100644 --- a/js/src/methodjit/PolyIC.cpp +++ b/js/src/methodjit/PolyIC.cpp @@ -761,19 +761,7 @@ struct GetPropHelper { } LookupStatus lookup() { - /* - * Skip to the prototype of dense arrays, which are non-native objects - * and otherwise uncacheable. This is not valid to do for indexed - * properties (which will not be a PropertyName), nor for __proto__, - * which needs to be filtered here. - */ - JSObject *aobj = obj; - if (obj->isDenseArray()) { - if (name == cx->runtime->atomState.protoAtom) - return ic.disable(cx, "__proto__"); - aobj = obj->getProto(); - } - + JSObject *aobj = js_GetProtoIfDenseArray(obj); if (!aobj->isNative()) return ic.disable(f, "non-native"); @@ -2616,8 +2604,8 @@ GetElementIC::update(VMFrame &f, JSObject *obj, const Value &v, jsid id, Value * /* * Only treat this as a GETPROP for non-numeric string identifiers. The * GETPROP IC assumes the id has already gone through filtering for string - * indexes in the emitter, i.e. skipping to the prototype of dense arrays - * is only valid to do when looking up non-integer identifiers. + * indexes in the emitter, i.e. js_GetProtoIfDenseArray is only valid to + * use when looking up non-integer identifiers. */ if (v.isString() && js_CheckForStringIndex(id) == id) return attachGetProp(f, obj, v, JSID_TO_ATOM(id)->asPropertyName(), vp);