[INFER] Handle recompilations triggered in ICs by obj->lookupProperty, bug 643272.
authorBrian Hackett <bhackett1024@gmail.com>
Tue, 22 Mar 2011 12:23:37 -0700
changeset 74832 133a01a817de620432a4c9671329e0064621c567
parent 74831 17e44b678d36742576a3af602eb0300c441e786c
child 74833 7a76d795f62cdfefb17b13548dc0437080b083a8
push id2
push userbsmedberg@mozilla.com
push dateFri, 19 Aug 2011 14:38:13 +0000
bugs643272
milestone2.0b13pre
[INFER] Handle recompilations triggered in ICs by obj->lookupProperty, bug 643272.
js/src/methodjit/PolyIC.cpp
js/src/methodjit/PolyIC.h
--- a/js/src/methodjit/PolyIC.cpp
+++ b/js/src/methodjit/PolyIC.cpp
@@ -154,17 +154,20 @@ class PICStubCompiler : public BaseCompi
 
     bool isCallOp() const {
         if (pic.kind == ic::PICInfo::CALL)
             return true;
         return !!(js_CodeSpec[pic.op].format & JOF_CALLOP);
     }
 
     LookupStatus error() {
-        disable("error");
+        /*
+         * N.B. Do not try to disable the IC, we do not want to guard on
+         * whether the IC has been recompiled when propagating errors.
+         */
         return Lookup_Error;
     }
 
     LookupStatus error(JSContext *cx) {
         return error();
     }
 
     LookupStatus disable(const char *reason) {
@@ -495,18 +498,23 @@ class SetPropCompiler : public PICStubCo
             return disable("ops lookup property hook");
         if (clasp->ops.setProperty)
             return disable("ops set property hook");
 
         jsid id = ATOM_TO_JSID(atom);
 
         JSObject *holder;
         JSProperty *prop = NULL;
+
+        /* lookupProperty can trigger recompilations. */
+        uint32 recompilations = f.jit()->recompilations;
         if (!obj->lookupProperty(cx, id, &holder, &prop))
             return error();
+        if (f.jit()->recompilations != recompilations)
+            return Lookup_Uncacheable;
 
         /* If the property exists but is on a prototype, treat as addprop. */
         if (prop && holder != obj) {
             const Shape *shape = (const Shape *) prop;
 
             if (!holder->isNative())
                 return disable("non-native holder");
 
@@ -678,29 +686,30 @@ IsCacheableProtoChain(JSObject *obj, JSO
 
 template <typename IC>
 struct GetPropertyHelper {
     // These fields are set in the constructor and describe a property lookup.
     JSContext   *cx;
     JSObject    *obj;
     JSAtom      *atom;
     IC          &ic;
+    VMFrame     &f;
 
     // These fields are set by |bind| and |lookup|. After a call to either
     // function, these are set exactly as they are in JSOP_GETPROP or JSOP_NAME.
     JSObject    *aobj;
     JSObject    *holder;
     JSProperty  *prop;
  
     // This field is set by |bind| and |lookup| only if they returned
     // Lookup_Cacheable, otherwise it is NULL.
     const Shape *shape;
 
-    GetPropertyHelper(JSContext *cx, JSObject *obj, JSAtom *atom, IC &ic)
-      : cx(cx), obj(obj), atom(atom), ic(ic), holder(NULL), prop(NULL), shape(NULL)
+    GetPropertyHelper(JSContext *cx, JSObject *obj, JSAtom *atom, IC &ic, VMFrame &f)
+      : cx(cx), obj(obj), atom(atom), ic(ic), f(f), holder(NULL), prop(NULL), shape(NULL)
     { }
 
   public:
     LookupStatus bind() {
         if (!js_FindProperty(cx, ATOM_TO_JSID(atom), &obj, &holder, &prop))
             return ic.error(cx);
         if (!prop)
             return ic.disable(cx, "lookup failed");
@@ -711,18 +720,23 @@ struct GetPropertyHelper {
         shape = (const Shape *)prop;
         return Lookup_Cacheable;
     }
 
     LookupStatus lookup() {
         JSObject *aobj = js_GetProtoIfDenseArray(obj);
         if (!aobj->isNative())
             return ic.disable(cx, "non-native");
+
+        uint32 recompilations = f.jit()->recompilations;
         if (!aobj->lookupProperty(cx, ATOM_TO_JSID(atom), &holder, &prop))
             return ic.error(cx);
+        if (f.jit()->recompilations != recompilations)
+            return Lookup_Uncacheable;
+
         if (!prop)
             return ic.disable(cx, "lookup failed");
         if (!IsCacheableProtoChain(obj, holder))
             return ic.disable(cx, "non-native holder");
         shape = (const Shape *)prop;
         return Lookup_Cacheable;
     }
 
@@ -915,17 +929,17 @@ class GetPropCompiler : public PICStubCo
     LookupStatus generateStringCallStub()
     {
         JS_ASSERT(pic.hasTypeCheck());
         JS_ASSERT(pic.kind == ic::PICInfo::CALL);
 
         if (!f.fp()->script()->compileAndGo)
             return disable("String.prototype without compile-and-go");
 
-        GetPropertyHelper<GetPropCompiler> getprop(cx, obj, atom, *this);
+        GetPropertyHelper<GetPropCompiler> getprop(cx, obj, atom, *this, f);
         LookupStatus status = getprop.lookupAndTest();
         if (status != Lookup_Cacheable)
             return status;
         if (getprop.obj != getprop.holder)
             return disable("proto walk on String.prototype");
 
         Assembler masm;
 
@@ -1170,17 +1184,17 @@ class GetPropCompiler : public PICStubCo
         if (int secondGuardOffset = getLastStubSecondShapeGuard())
             repatcher.relink(label.jumpAtOffset(secondGuardOffset), cs);
     }
 
     LookupStatus update()
     {
         JS_ASSERT(pic.hit);
 
-        GetPropertyHelper<GetPropCompiler> getprop(cx, obj, atom, *this);
+        GetPropertyHelper<GetPropCompiler> getprop(cx, obj, atom, *this, f);
         LookupStatus status = getprop.lookupAndTest();
         if (status != Lookup_Cacheable)
             return status;
 
         if (obj == getprop.holder && !pic.inlinePathPatched)
             return patchInline(getprop.holder, getprop.shape);
         
         return generateStub(getprop.holder, getprop.shape);
@@ -1252,17 +1266,17 @@ class ScopeNameCompiler : public PICStub
         return Lookup_Cacheable;
     }
 
   public:
     ScopeNameCompiler(VMFrame &f, JSScript *script, JSObject *scopeChain, ic::PICInfo &pic,
                       JSAtom *atom, VoidStubPIC stub)
       : PICStubCompiler("name", f, script, pic, JS_FUNC_TO_DATA_PTR(void *, stub)),
         scopeChain(scopeChain), atom(atom),
-        getprop(f.cx, NULL, atom, *thisFromCtor())
+        getprop(f.cx, NULL, atom, *thisFromCtor(), f)
     { }
 
     static void reset(Repatcher &repatcher, ic::PICInfo &pic)
     {
         ScopeNameLabels &labels = pic.scopeNameLabels();
 
         /* Link the inline path back to the slow path. */
         JSC::CodeLocationJump inlineJump = labels.getInlineJump(pic.fastPathStart);
@@ -1743,29 +1757,29 @@ ic::GetProp(VMFrame &f, ic::PICInfo *pic
         }
         atom = f.cx->runtime->atomState.lengthAtom;
     }
 
     JSObject *obj = ValueToObject(f.cx, &f.regs.sp[-1]);
     if (!obj)
         THROW();
 
+    bool usePropCache = pic->usePropCache;
+
     if (pic->shouldUpdate(f.cx)) {
         VoidStubPIC stub = pic->usePropCache
                            ? DisabledGetPropIC
                            : DisabledGetPropICNoCache;
         GetPropCompiler cc(f, script, obj, *pic, atom, stub);
         if (!cc.update()) {
             cc.disable("error");
             THROW();
         }
     }
 
-    bool usePropCache = pic->usePropCache;
-
     Value v;
     if (!obj->getProperty(f.cx, ATOM_TO_JSID(atom), &v))
         THROW();
 
     /*
      * Ignore undefined reads for the 'prototype' property in constructors,
      * which will be at the start of the script and are never holes due to fun_resolve.
      * Any undefined value was explicitly stored here, and is known by inference.
@@ -2151,21 +2165,21 @@ GetElementIC::purge(Repatcher &repatcher
                              FunctionPtr(JS_FUNC_TO_DATA_PTR(void *, ic::CallElement)));
         }
     }
 
     reset();
 }
 
 LookupStatus
-GetElementIC::attachGetProp(JSContext *cx, JSObject *obj, const Value &v, jsid id, Value *vp)
+GetElementIC::attachGetProp(VMFrame &f, JSContext *cx, JSObject *obj, const Value &v, jsid id, Value *vp)
 {
     JS_ASSERT(v.isString());
 
-    GetPropertyHelper<GetElementIC> getprop(cx, obj, JSID_TO_ATOM(id), *this);
+    GetPropertyHelper<GetElementIC> getprop(cx, obj, JSID_TO_ATOM(id), *this, f);
     LookupStatus status = getprop.lookupAndTest();
     if (status != Lookup_Cacheable)
         return status;
 
     Assembler masm;
 
     // Guard on the string's type and identity.
     MaybeJump atomTypeGuard;
@@ -2427,20 +2441,20 @@ GetElementIC::attachTypedArray(JSContext
     if (!obj->getProperty(cx, id, vp))
         return Lookup_Error;
 
     return Lookup_Cacheable;
 }
 #endif /* JS_POLYIC_TYPED_ARRAY */
 
 LookupStatus
-GetElementIC::update(JSContext *cx, JSObject *obj, const Value &v, jsid id, Value *vp)
+GetElementIC::update(VMFrame &f, JSContext *cx, JSObject *obj, const Value &v, jsid id, Value *vp)
 {
     if (v.isString())
-        return attachGetProp(cx, obj, v, id, vp);
+        return attachGetProp(f, cx, obj, v, id, vp);
 
 #if 0 // :FIXME: bug 643842
 //#if defined JS_POLYIC_TYPED_ARRAY
     if (js_IsTypedArray(obj))
         return attachTypedArray(cx, obj, v, id, vp);
 #endif
 
     return disable(cx, "unhandled object and key type");
@@ -2469,17 +2483,17 @@ ic::CallElement(VMFrame &f, ic::GetEleme
         id = INT_TO_JSID(idval.toInt32());
     else if (!js_InternNonIntElementId(cx, thisObj, idval, &id))
         THROW();
 
     if (ic->shouldUpdate(cx)) {
 #ifdef DEBUG
         f.regs.sp[-2] = MagicValue(JS_GENERIC_MAGIC);
 #endif
-        LookupStatus status = ic->update(cx, thisObj, idval, id, &f.regs.sp[-2]);
+        LookupStatus status = ic->update(f, cx, thisObj, idval, id, &f.regs.sp[-2]);
         if (status != Lookup_Uncacheable) {
             if (status == Lookup_Error)
                 THROW();
 
             // If the result can be cached, the value was already retrieved.
             JS_ASSERT(!f.regs.sp[-2].isMagic());
             f.regs.sp[-1].setObject(*thisObj);
             if (!JSID_IS_INT(id) && !f.script()->typeMonitorUnknown(cx, f.regs.pc))
@@ -2534,17 +2548,17 @@ ic::GetElement(VMFrame &f, ic::GetElemen
         if (!js_InternNonIntElementId(cx, obj, idval, &id))
             THROW();
     }
 
     if (ic->shouldUpdate(cx)) {
 #ifdef DEBUG
         f.regs.sp[-2] = MagicValue(JS_GENERIC_MAGIC);
 #endif
-        LookupStatus status = ic->update(cx, obj, idval, id, &f.regs.sp[-2]);
+        LookupStatus status = ic->update(f, cx, obj, idval, id, &f.regs.sp[-2]);
         if (status != Lookup_Uncacheable) {
             if (status == Lookup_Error)
                 THROW();
 
             // If the result can be cached, the value was already retrieved.
             JS_ASSERT(!f.regs.sp[-2].isMagic());
             if (!JSID_IS_INT(id) && !f.script()->typeMonitorUnknown(cx, f.regs.pc))
                 THROW();
--- a/js/src/methodjit/PolyIC.h
+++ b/js/src/methodjit/PolyIC.h
@@ -288,18 +288,18 @@ struct GetElementIC : public BasePolyIC 
     void reset() {
         BasePolyIC::reset();
         inlineTypeGuardPatched = false;
         inlineClaspGuardPatched = false;
         typeRegHasBaseShape = false;
         hasLastStringStub = false;
     }
     void purge(Repatcher &repatcher);
-    LookupStatus update(JSContext *cx, JSObject *obj, const Value &v, jsid id, Value *vp);
-    LookupStatus attachGetProp(JSContext *cx, JSObject *obj, const Value &v, jsid id,
+    LookupStatus update(VMFrame &f, JSContext *cx, JSObject *obj, const Value &v, jsid id, Value *vp);
+    LookupStatus attachGetProp(VMFrame &f, JSContext *cx, JSObject *obj, const Value &v, jsid id,
                                Value *vp);
     LookupStatus attachTypedArray(JSContext *cx, JSObject *obj, const Value &v, jsid id,
                                   Value *vp);
     LookupStatus disable(JSContext *cx, const char *reason);
     LookupStatus error(JSContext *cx);
     bool shouldUpdate(JSContext *cx);
 };