Major and winning overhaul to for-in codegen (mad props to Andreas for advice).
authorBrendan Eich <brendan@mozilla.org>
Wed, 13 Aug 2008 14:02:35 -0700
changeset 18136 b7199324e019798953cd243e7c5c73677eddcf1d
parent 18135 7dd001982f576277d739c45206b3f84b9232c074
child 18137 ad0aef3a1a4256a4a1e58bfd03b811c127a45544
child 18139 b42fa8f98b1a41db47662cef159594dcd4ad270f
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
milestone1.9.1a2pre
Major and winning overhaul to for-in codegen (mad props to Andreas for advice).
js/src/builtins.tbl
js/src/jsbuiltins.cpp
js/src/jstracer.cpp
js/src/jstracer.h
--- a/js/src/builtins.tbl
+++ b/js/src/builtins.tbl
@@ -63,17 +63,18 @@ BUILTIN4(String_p_replace_str,  LO, LO, 
 BUILTIN5(String_p_replace_str3, LO, LO, LO, LO, LO, LO, JSString*, JSContext*, JSString*, JSString*, JSString*, JSString*, 1, 1)
 BUILTIN1(Math_random,           LO,     F,      jsdouble,  JSRuntime*, 1, 1)
 BUILTIN2(EqualStrings,          LO,     LO, LO, bool,      JSString*, JSString*, 1, 1)
 BUILTIN2(CompareStrings,        LO,     LO, LO, bool,      JSString*, JSString*, 1, 1)
 BUILTIN2(StringToNumber,        LO,     LO, F,  jsdouble,  JSContext*, JSString*, 1, 1)
 BUILTIN2(StringToInt32,         LO,     LO, LO, jsint,     JSContext*, JSString*, 1, 1)
 BUILTIN3(Any_getelem,           LO, LO, LO, LO, jsval,     JSContext*, JSObject*, JSString*, 1, 1)
 BUILTIN4(Any_setelem,           LO, LO, LO, LO, LO, bool,  JSContext*, JSObject*, JSString*, jsval, 1, 1)
-BUILTIN2(ValueToEnumerator,     LO,     LO, LO, JSObject*, JSContext*, jsval, 1, 1)
+BUILTIN3(FastValueToIterator,   LO, LO, LO, LO, JSObject*, JSContext*, jsuint, jsval, 1, 1)
+BUILTIN2(FastCallIteratorNext,  LO,     LO, LO, JSObject*, JSContext*, JSObject*, 1, 1)
 BUILTIN2(CloseIterator,         LO,     LO, LO, bool,      JSContext*, jsval, 1, 1)
 BUILTIN2(CallTree,              LO, LO, LO,     nanojit::GuardRecord*, avmplus::InterpState*, nanojit::Fragment*, 0, 0)
 BUILTIN2(FastNewObject,         LO,     LO, LO, JSObject*, JSContext*, JSObject*, 1, 1)
 BUILTIN3(AddProperty,           LO, LO, LO, LO, bool,      JSContext*, JSObject*, JSScopeProperty*, 1, 1)
 BUILTIN3(CallGetter,            LO, LO, LO, LO, jsval,     JSContext*, JSObject*, JSScopeProperty*, 1, 1)
 BUILTIN2(TypeOfObject,          LO,     LO, LO, JSString*, JSContext*, JSObject*, 1, 1)
 BUILTIN2(TypeOfBoolean,         LO,     LO, LO, JSString*, JSContext*, jsint, 1, 1)
 BUILTIN2(NumberToString,        LO,     F,  LO, JSString*, JSContext*, jsdouble, 1, 1)
--- a/js/src/jsbuiltins.cpp
+++ b/js/src/jsbuiltins.cpp
@@ -362,23 +362,32 @@ bool FASTCALL
 js_Any_setelem(JSContext* cx, JSObject* obj, JSString* idstr, jsval v)
 {
     if (!JSSTRING_IS_FLAT(idstr) && !js_UndependString(cx, idstr))
         return false;
     return OBJ_SET_PROPERTY(cx, obj, ATOM_TO_JSID(STRING_TO_JSVAL(idstr)), &v);
 }
 
 JSObject* FASTCALL
-js_ValueToEnumerator(JSContext* cx, jsval v)
+js_FastValueToIterator(JSContext* cx, jsuint flags, jsval v)
 {
-    if (!js_ValueToIterator(cx, JSITER_ENUMERATE, &v))
+    if (!js_ValueToIterator(cx, flags, &v))
         return NULL;
     return JSVAL_TO_OBJECT(v);
 }
 
+jsval FASTCALL
+js_FastCallIteratorNext(JSContext* cx, JSObject* iterobj)
+{
+    jsval v;
+    if (!js_CallIteratorNext(cx, iterobj, &v))
+        return JSVAL_ERROR_COOKIE;
+    return v;
+}
+
 GuardRecord* FASTCALL
 js_CallTree(InterpState* state, Fragment* f)
 {
     /* current we can't deal with inner trees that have globals so report an error */
     JS_ASSERT(!((TreeInfo*)f->vmprivate)->globalSlots.length());
     union { NIns *code; GuardRecord* (FASTCALL *func)(InterpState*, Fragment*); } u;
     u.code = f->code();
     return u.func(state, NULL);
--- a/js/src/jstracer.cpp
+++ b/js/src/jstracer.cpp
@@ -1832,20 +1832,22 @@ js_LoopEdge(JSContext* cx, jsbytecode* o
     /* if this exits the loop, resume interpretation */
     return false;
 }
 
 void
 js_AbortRecording(JSContext* cx, jsbytecode* abortpc, const char* reason)
 {
     AUDIT(recorderAborted);
-    debug_only(if (!abortpc) abortpc = cx->fp->regs->pc;
-               printf("Abort recording (line %d, pc %d): %s.\n",
-                      js_PCToLineNumber(cx, cx->fp->script, abortpc),
-                      abortpc - cx->fp->script->code, reason);)
+    if (cx->fp) {
+        debug_only(if (!abortpc) abortpc = cx->fp->regs->pc;
+                   printf("Abort recording (line %d, pc %d): %s.\n",
+                          js_PCToLineNumber(cx, cx->fp->script, abortpc),
+                          abortpc - cx->fp->script->code, reason);)
+    }
     JS_ASSERT(JS_TRACE_MONITOR(cx).recorder != NULL);
     Fragment* f = JS_TRACE_MONITOR(cx).recorder->getFragment();
     f->blacklist();
     js_DeleteRecorder(cx);
     if (f->root == f)
         js_TrashTree(cx, f);
 }
 
@@ -2517,48 +2519,48 @@ TraceRecorder::box_jsval(jsval v, LIns*&
 
 bool
 TraceRecorder::unbox_jsval(jsval v, LIns*& v_ins)
 {
     if (isNumber(v)) {
         // JSVAL_IS_NUMBER(v)
         guard(false,
               lir->ins_eq0(lir->ins2(LIR_or,
-                                     lir->ins2(LIR_and, v_ins, lir->insImmPtr((void*)JSVAL_INT)),
+                                     lir->ins2(LIR_and, v_ins, INS_CONSTPTR(JSVAL_INT)),
                                      lir->ins2i(LIR_eq,
                                                 lir->ins2(LIR_and, v_ins,
-                                                          lir->insImmPtr((void*)JSVAL_TAGMASK)),
+                                                          INS_CONSTPTR(JSVAL_TAGMASK)),
                                                 JSVAL_DOUBLE))),
               MISMATCH_EXIT);
         v_ins = lir->insCall(F_UnboxDouble, &v_ins);
         return true;
     }
     switch (JSVAL_TAG(v)) {
       case JSVAL_BOOLEAN:
         guard(true,
               lir->ins2i(LIR_eq,
-                         lir->ins2(LIR_and, v_ins, lir->insImmPtr((void*)JSVAL_TAGMASK)),
+                         lir->ins2(LIR_and, v_ins, INS_CONSTPTR(JSVAL_TAGMASK)),
                          JSVAL_BOOLEAN),
               MISMATCH_EXIT);
          v_ins = lir->ins2i(LIR_ush, v_ins, JSVAL_TAGBITS);
          return true;
        case JSVAL_OBJECT:
         guard(true,
               lir->ins2i(LIR_eq,
-                         lir->ins2(LIR_and, v_ins, lir->insImmPtr((void*)JSVAL_TAGMASK)),
+                         lir->ins2(LIR_and, v_ins, INS_CONSTPTR(JSVAL_TAGMASK)),
                          JSVAL_OBJECT),
               MISMATCH_EXIT);
         return true;
       case JSVAL_STRING:
         guard(true,
               lir->ins2i(LIR_eq,
-                        lir->ins2(LIR_and, v_ins, lir->insImmPtr((void*)JSVAL_TAGMASK)),
+                        lir->ins2(LIR_and, v_ins, INS_CONSTPTR(JSVAL_TAGMASK)),
                         JSVAL_STRING),
               MISMATCH_EXIT);
-        v_ins = lir->ins2(LIR_and, v_ins, lir->insImmPtr((void*)~JSVAL_TAGMASK));
+        v_ins = lir->ins2(LIR_and, v_ins, INS_CONSTPTR(~JSVAL_TAGMASK));
         return true;
     }
     return false;
 }
 
 bool
 TraceRecorder::getThis(LIns*& this_ins)
 {
@@ -2626,20 +2628,26 @@ TraceRecorder::clearFrameSlotsFromCache(
 {
     /* Clear out all slots of this frame in the nativeFrameTracker. Different locations on the
        VM stack might map to different locations on the native stack depending on the
        number of arguments (i.e.) of the next call, so we have to make sure we map
        those in to the cache with the right offsets. */
     JSStackFrame* fp = cx->fp;
     jsval* vp;
     jsval* vpstop;
-    for (vp = &fp->argv[-1], vpstop = &fp->argv[JS_MAX(fp->fun->nargs,fp->argc)]; vp < vpstop; ++vp)
-        nativeFrameTracker.set(vp, (LIns*)0);
-    for (vp = &fp->slots[0], vpstop = &fp->slots[fp->script->nslots]; vp < vpstop; ++vp)
-        nativeFrameTracker.set(vp, (LIns*)0);
+    if (fp->callee) {
+        vp = &fp->argv[-1];
+        vpstop = &fp->argv[JS_MAX(fp->fun->nargs,fp->argc)];
+        while (vp < vpstop)
+            nativeFrameTracker.set(vp++, (LIns*)0);
+    }
+    vp = &fp->slots[0];
+    vpstop = &fp->slots[fp->script->nslots];
+    while (vp < vpstop)
+        nativeFrameTracker.set(vp++, (LIns*)0);
 }
 
 bool
 TraceRecorder::record_EnterFrame()
 {
     if (++callDepth >= MAX_CALLDEPTH)
         ABORT_TRACE("exceeded maximum call depth");
     JSStackFrame* fp = cx->fp;
@@ -3336,17 +3344,17 @@ TraceRecorder::record_JSOP_CALLNAME()
 
 bool
 TraceRecorder::guardShapelessCallee(jsval& callee)
 {
     if (!VALUE_IS_FUNCTION(cx, callee))
         ABORT_TRACE("shapeless callee is not a function");
 
     guard(true,
-          addName(lir->ins2(LIR_eq, get(&callee), lir->insImmPtr((void*) JSVAL_TO_OBJECT(callee))),
+          addName(lir->ins2(LIR_eq, get(&callee), INS_CONSTPTR(JSVAL_TO_OBJECT(callee))),
                   "guard(shapeless callee)"),
           MISMATCH_EXIT);
     return true;
 }
 
 bool
 TraceRecorder::interpretedFunctionCall(jsval& fval, JSFunction* fun, uintN argc)
 {
@@ -3866,17 +3874,17 @@ TraceRecorder::record_JSOP_LOOKUPSWITCH(
         jsdouble d = asNumber(v);
         jsdpun u;
         u.d = d;
         guard(true,
               addName(lir->ins2(LIR_feq, get(&v), lir->insImmq(u.u64)),
                       "guard(lookupswitch numeric)"),
               BRANCH_EXIT);
     } else if (JSVAL_IS_STRING(v)) {
-        LIns* args[] = { get(&v), lir->insImmPtr((void*) JSVAL_TO_STRING(v)) };
+        LIns* args[] = { get(&v), INS_CONSTPTR(JSVAL_TO_STRING(v)) };
         guard(true,
               addName(lir->ins_eq0(lir->ins_eq0(lir->insCall(F_EqualStrings, args))),
                       "guard(lookupswitch string)"),
               BRANCH_EXIT);
     } else if (JSVAL_IS_BOOLEAN(v)) {
         guard(true,
               addName(lir->ins2(LIR_eq, get(&v), lir->insImm(JSVAL_TO_BOOLEAN(v))),
                       "guard(lookupswitch boolean)"),
@@ -4071,161 +4079,67 @@ bool
 TraceRecorder::record_JSOP_LOCALDEC()
 {
     return inc(varval(GET_SLOTNO(cx->fp->regs->pc)), -1, false);
 }
 
 bool
 TraceRecorder::record_JSOP_ITER()
 {
-    uintN flags = cx->fp->regs->pc[1];
-    if (flags & ~JSITER_ENUMERATE)
-        ABORT_TRACE("for-each-in or destructuring JSOP_ITER not traced");
-
     jsval& v = stackval(-1);
     if (!JSVAL_IS_PRIMITIVE(v)) {
-        LIns* args[] = { get(&v), cx_ins };
-        LIns* v_ins = lir->insCall(F_ValueToEnumerator, args);
+        jsuint flags = cx->fp->regs->pc[1];
+        LIns* args[] = { get(&v), INS_CONST(flags), cx_ins };
+        LIns* v_ins = lir->insCall(F_FastValueToIterator, args);
         guard(false, lir->ins_eq0(v_ins), MISMATCH_EXIT);
         set(&v, v_ins);
         return true;
     }
 
     ABORT_TRACE("for-in on a primitive value");
 }
 
 bool
-TraceRecorder::forInProlog(JSObject*& iterobj, LIns*& iterobj_ins)
-{
-    jsval& iterval = stackval(-1);
-    JS_ASSERT(!JSVAL_IS_PRIMITIVE(iterval));
-    iterobj = JSVAL_TO_OBJECT(iterval);
-
-    iterobj_ins = get(&iterval);
-    if (guardClass(iterobj, iterobj_ins, &js_IteratorClass)) {
-        // Check flags in case we did not record the JSOP_ITER (it comes before the for-in loop).
-        uintN flags = JSVAL_TO_INT(iterobj->fslots[JSSLOT_ITER_FLAGS]);
-        if (flags & ~JSITER_ENUMERATE)
-            ABORT_TRACE("for-each-in or destructuring JSOP_ITER not traced");
-
-        guard(true,
-              addName(lir->ins_eq0(lir->ins2(LIR_and,
-                                             lir->insLoadi(iterobj_ins,
-                                                           offsetof(JSObject, fslots) +
-                                                           JSSLOT_ITER_FLAGS * sizeof(jsval)),
-                                             INS_CONST(~JSITER_ENUMERATE))),
-                      "guard(iter flags is JSITER_ENUMERATE)"),
-              MISMATCH_EXIT);
-
-        JSObject* obj = STOBJ_GET_PARENT(iterobj);
-        LIns* obj_ins = stobj_get_fslot(iterobj_ins, JSSLOT_PARENT);
-        LIns* map_ins = lir->insLoadi(obj_ins, offsetof(JSObject, map));
-        LIns* ops_ins;
-        if (!map_is_native(obj->map, map_ins, ops_ins))
+TraceRecorder::forInLoop(jsval* vp)
+{
+    jsval& iterobj_val = stackval(-1);
+    if (!JSVAL_IS_PRIMITIVE(iterobj_val)) {
+        LIns* args[] = { get(&iterobj_val), cx_ins };
+        LIns* v_ins = lir->insCall(F_FastCallIteratorNext, args);
+        guard(false, lir->ins2(LIR_eq, v_ins, INS_CONSTPTR(JSVAL_ERROR_COOKIE)), OOM_EXIT);
+
+        LIns* flag_ins = lir->ins_eq0(lir->ins2(LIR_eq, v_ins, INS_CONSTPTR(JSVAL_HOLE)));
+        LIns* iter_ins = get(vp); 
+        if (!box_jsval(JSVAL_STRING, iter_ins))
             return false;
-
-        LIns* n = lir->insLoadi(ops_ins, offsetof(JSObjectOps, enumerate));
-        if (obj->map->ops->enumerate == js_ObjectOps.enumerate) {
-            guard(true,
-                  addName(lir->ins2(LIR_eq, n, lir->insImmPtr((void*)js_ObjectOps.enumerate)),
-                          "guard(native-enumerate)"),
-                  MISMATCH_EXIT);
-            return true;
-        }
+        iter_ins = lir->ins_choose(flag_ins, v_ins, iter_ins, true);
+        if (!unbox_jsval(JSVAL_STRING, iter_ins))
+            return false;
+        set(vp, iter_ins);
+        stack(0, flag_ins);
+        return true;
     }
-    return false;
-}
-
-bool
-TraceRecorder::forInLoop(LIns*& id_ins)
-{
-    JSObject* iterobj;
-    LIns* iterobj_ins;
-    if (!forInProlog(iterobj, iterobj_ins))
-        return false;
-
-    jsval stateval = iterobj->fslots[JSSLOT_ITER_STATE];
-    LIns* stateval_ins = stobj_get_fslot(iterobj_ins, JSSLOT_ITER_STATE);
-
-    // If a guarded loop termination condition is false while recording, stack
-    // unboxed false and return so the immediately subsequent JSOP_IFEQ exits
-    // the loop.
-    int flag = 0;
-    id_ins = NULL;
-
-    guard(false, addName(lir->ins_eq0(stateval_ins), "guard(non-null iter state"), MISMATCH_EXIT);
-    if (stateval == JSVAL_NULL)
-        goto done;
-    guard(false,
-          addName(lir->ins2(LIR_eq, stateval_ins, lir->insImmPtr((void*) JSVAL_ZERO)),
-                  "guard(non-empty iter state)"),
-          MISMATCH_EXIT);
-    if (stateval == JSVAL_ZERO)
-        goto done;
-
-    // Don't initialize to avoid goto over init warnings/errors.
-    LIns* state_ins; 
-    LIns* cursor_ins;
-
-    state_ins = lir->ins2(LIR_and, stateval_ins, lir->insImmPtr((void*) ~jsval(3)));
-    cursor_ins = lir->insLoadi(state_ins, offsetof(JSNativeEnumerator, cursor));
-    guard(false, addName(lir->ins_eq0(cursor_ins), "guard(ne->cursor != 0)"), MISMATCH_EXIT);
-
-    JSNativeEnumerator* ne;
-
-    // Stack an unboxed true to make JSOP_IFEQ loop continue, even if ne is
-    // exhausted. Either we'll end up in the interpreter finding no enumerable
-    // prototype properties, or we will re-enter this trace having gone up the
-    // prototype chain one level.
-    flag = 1;
-    ne = (JSNativeEnumerator*) (stateval & ~jsval(3));
-    if (ne->cursor == 0)
-        goto done;
-
-    cursor_ins = lir->ins2i(LIR_sub, cursor_ins, 1);
-    lir->insStorei(cursor_ins, state_ins, offsetof(JSNativeEnumerator, cursor));
-
-    // Don't initialize to avoid goto over init warnings/errors.
-    LIns* ids_ins;
-    LIns* id_addr_ins;
-
-    ids_ins = lir->ins2i(LIR_add, state_ins, offsetof(JSNativeEnumerator, ids));
-    id_addr_ins = lir->ins2(LIR_add, ids_ins,
-                            lir->ins2i(LIR_lsh, cursor_ins, (sizeof(jsid) == 4) ? 2 : 3));
-
-
-    id_ins = lir->insLoadi(id_addr_ins, 0);
-done:
-    stack(0, lir->insImm(flag));
-    return true;
+
+    ABORT_TRACE("for-in on a primitive value");
 }
 
 bool
 TraceRecorder::record_JSOP_ENDITER()
 {
     LIns* args[] = { stack(-1), cx_ins };
     LIns* ok_ins = lir->insCall(F_CloseIterator, args);
     guard(false, lir->ins_eq0(ok_ins), MISMATCH_EXIT);
     return true;
 }
 
 bool
 TraceRecorder::record_JSOP_FORNAME()
 {
-    LIns* id_ins; 
-    if (!forInLoop(id_ins))
-        return false;
-    if (!id_ins)
-        return true;
-
     jsval* vp;
-    if (!name(vp))
-        return false;
-    set(vp, id_ins);
-    return true;
+    return name(vp) && forInLoop(vp);
 }
 
 bool
 TraceRecorder::record_JSOP_FORPROP()
 {
     return false;
 }
 
@@ -4233,33 +4147,23 @@ bool
 TraceRecorder::record_JSOP_FORELEM()
 {
     return false;
 }
 
 bool
 TraceRecorder::record_JSOP_FORARG()
 {
-    LIns* id_ins; 
-    if (!forInLoop(id_ins))
-        return false;
-    if (id_ins)
-        arg(GET_ARGNO(cx->fp->regs->pc), id_ins);
-    return true;
+    return forInLoop(&argval(GET_ARGNO(cx->fp->regs->pc)));
 }
 
 bool
 TraceRecorder::record_JSOP_FORLOCAL()
 {
-    LIns* id_ins; 
-    if (!forInLoop(id_ins))
-        return false;
-    if (id_ins)
-        var(GET_SLOTNO(cx->fp->regs->pc), id_ins);
-    return true;
+    return forInLoop(&varval(GET_SLOTNO(cx->fp->regs->pc)));
 }
 
 bool
 TraceRecorder::record_JSOP_FORCONST()
 {
     return false;
 }
 
--- a/js/src/jstracer.h
+++ b/js/src/jstracer.h
@@ -295,18 +295,17 @@ class TraceRecorder {
     bool unbox_jsval(jsval v, nanojit::LIns*& v_ins);
     bool guardClass(JSObject* obj, nanojit::LIns* obj_ins, JSClass* clasp);
     bool guardDenseArray(JSObject* obj, nanojit::LIns* obj_ins);
     bool guardDenseArrayIndex(JSObject* obj, jsint idx, nanojit::LIns* obj_ins,
                               nanojit::LIns* dslots_ins, nanojit::LIns* idx_ins);
     void clearFrameSlotsFromCache();
     bool guardShapelessCallee(jsval& callee);
     bool interpretedFunctionCall(jsval& fval, JSFunction* fun, uintN argc);
-    bool forInProlog(JSObject*& iterobj, nanojit::LIns*& iterobj_ins);
-    bool forInLoop(nanojit::LIns*& id_ins);
+    bool forInLoop(jsval* vp);
 
 public:
     TraceRecorder(JSContext* cx, nanojit::GuardRecord*, nanojit::Fragment*, 
             unsigned ngslots, uint8* globalTypeMap, uint8* stackTypeMap);
     ~TraceRecorder();
 
     nanojit::SideExit* snapshot(nanojit::ExitType exitType);
     nanojit::Fragment* getFragment() const { return fragment; }