Smarter speculative demotion of numbers to integers and promotion of the trace seems to require actual doubles. If the number at entry looks like an int we make the slot an int and compile as such. If the loop-tail proves the slot to be a double, we recompile the trace. Currently such miss-speculation cannot be handled on secondary traces since we are currently unable to recompile the primary trace. Such secondary traces are blacklisted.
authorAndreas Gal <gal@mozilla.com>
Sun, 27 Jul 2008 02:15:17 -0700
changeset 17841 bf4d58f8cc7f728221884369a256f09aadbae20a
parent 17837 f38fbde81ed186b3880bc39943cf9916c6958156
child 17842 ff7fb1bfd03df2eb976db17bf1fd65e67d53ee30
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.1a1pre
Smarter speculative demotion of numbers to integers and promotion of the trace seems to require actual doubles. If the number at entry looks like an int we make the slot an int and compile as such. If the loop-tail proves the slot to be a double, we recompile the trace. Currently such miss-speculation cannot be handled on secondary traces since we are currently unable to recompile the primary trace. Such secondary traces are blacklisted.
js/src/jstracer.cpp
js/src/jstracer.h
--- a/js/src/jstracer.cpp
+++ b/js/src/jstracer.cpp
@@ -64,20 +64,16 @@
 #include "jsautooplen.h"        // generated headers last
 
 /* Number of iterations of a loop before we start tracing. */
 #define HOTLOOP 2
 
 /* Number of times we wait to exit on a side exit before we try to extend the tree. */
 #define HOTEXIT 0
 
-/* Maximum number of guards after which we no longer try to demote loop variables.
-   0=off */
-#define DEMOTE_THRESHOLD 32
-
 #ifdef DEBUG
 #define ABORT_TRACE(msg)   do { fprintf(stdout, "abort: %d: %s\n", __LINE__, msg); return false; } while(0)
 #else
 #define ABORT_TRACE(msg)   return false
 #endif
 
 #ifdef DEBUG
 static struct {
@@ -166,27 +162,16 @@ void
 Tracker::set(const void* v, LIns* i)
 {
     struct Tracker::Page* p = findPage(v);
     if (!p)
         p = addPage(v);
     p->map[(jsuword(v) & 0xfff) >> 2] = i;
 }
 
-/*
- * Return the coerced type of a value. If it's a number, we always return JSVAL_DOUBLE, no matter
- * whether it's represented as an int or as a double.
- */
-static inline int getCoercedType(jsval v)
-{
-    if (JSVAL_IS_INT(v))
-        return JSVAL_DOUBLE;
-    return JSVAL_TAG(v);
-}
-
 static inline bool isNumber(jsval v)
 {
     return JSVAL_IS_INT(v) || JSVAL_IS_DOUBLE(v);
 }
 
 static inline jsdouble asNumber(jsval v)
 {
     JS_ASSERT(isNumber(v));
@@ -686,19 +671,18 @@ TraceRecorder::trackNativeStackUse(unsig
 {
     if (slots > treeInfo->maxNativeStackSlots)
         treeInfo->maxNativeStackSlots = slots;
 }
 
 /* Unbox a jsval into a slot. Slots are wide enough to hold double values
    directly (instead of storing a pointer to them). */
 static bool
-unbox_jsval(jsval v, uint8 t, double* slot)
-{
-    jsuint type = TYPEMAP_GET_TYPE(t);
+unbox_jsval(jsval v, uint8 type, double* slot)
+{
     if (type == TYPEMAP_TYPE_ANY) {
         debug_only(printf("any ");)
         return true;
     }
     if (type == JSVAL_INT) {
         jsint i;
         if (JSVAL_IS_INT(v))
             *(jsint*)slot = JSVAL_TO_INT(v);
@@ -748,19 +732,18 @@ unbox_jsval(jsval v, uint8 t, double* sl
     }
     return true;
 }
 
 /* Box a value from the native stack back into the jsval format. Integers
    that are too large to fit into a jsval are automatically boxed into
    heap-allocated doubles. */
 static bool
-box_jsval(JSContext* cx, jsval& v, uint8 t, double* slot)
-{
-    jsuint type = TYPEMAP_GET_TYPE(t);
+box_jsval(JSContext* cx, jsval& v, uint8 type, double* slot)
+{
     if (type == TYPEMAP_TYPE_ANY) {
         debug_only(printf("any ");)
         return true;
     }
     jsint i;
     jsdouble d;
     switch (type) {
       case JSVAL_BOOLEAN:
@@ -786,17 +769,17 @@ box_jsval(JSContext* cx, jsval& v, uint8
         /* Its safe to trigger the GC here since we rooted all strings/objects and all the
            doubles we already processed. */
         return js_NewDoubleInRootedValue(cx, d, &v);
       case JSVAL_STRING:
         v = STRING_TO_JSVAL(*(JSString**)slot);
         debug_only(printf("string<%p> ", *(JSString**)slot);)
         break;
       default:
-        JS_ASSERT(t == JSVAL_OBJECT);
+        JS_ASSERT(type == JSVAL_OBJECT);
         v = OBJECT_TO_JSVAL(*(JSObject**)slot);
         debug_only(printf("object<%p:%s> ", JSVAL_TO_OBJECT(v),
                             JSVAL_IS_NULL(v)
                             ? "null"
                             : STOBJ_GET_CLASS(JSVAL_TO_OBJECT(v))->name);)
         break;
     }
     return true;
@@ -867,29 +850,29 @@ box(JSContext* cx, unsigned ngslots, uin
     return true;
 }
 
 /* Emit load instructions onto the trace that read the initial stack state. */
 void
 TraceRecorder::import(LIns* base, ptrdiff_t offset, jsval* p, uint8& t,
         const char *prefix, int index, jsuword *localNames)
 {
-    JS_ASSERT(TYPEMAP_GET_TYPE(t) != TYPEMAP_TYPE_ANY);
+    JS_ASSERT(t != TYPEMAP_TYPE_ANY);
     LIns* ins;
-    if (TYPEMAP_GET_TYPE(t) == JSVAL_INT) { /* demoted */
+    if (t == JSVAL_INT) { /* demoted */
         JS_ASSERT(isInt32(*p));
         /* Ok, we have a valid demotion attempt pending, so insert an integer
            read and promote it to double since all arithmetic operations expect
            to see doubles on entry. The first op to use this slot will emit a
            f2i cast which will cancel out the i2f we insert here. */
         ins = lir->insLoadi(base, offset);
         nativeFrameTracker.set(p, ins);
         ins = lir->ins1(LIR_i2f, ins);
     } else {
-        JS_ASSERT(isNumber(*p) == (TYPEMAP_GET_TYPE(t) == JSVAL_DOUBLE));
+        JS_ASSERT(isNumber(*p) == (t == JSVAL_DOUBLE));
         ins = lir->insLoad(t == JSVAL_DOUBLE ? LIR_ldq : LIR_ld, base, offset);
         nativeFrameTracker.set(p, ins);
     }
     tracker.set(p, ins);
 #ifdef DEBUG
     char name[64];
     JS_ASSERT(strlen(prefix) < 10);
     if (!strcmp(prefix, "argv")) {
@@ -1024,108 +1007,74 @@ TraceRecorder::guard(bool expected, LIns
                          snapshot());
 }
 
 bool
 TraceRecorder::checkType(jsval& v, uint8& t)
 {
     if (t == TYPEMAP_TYPE_ANY) /* ignore unused slots */
         return true;
-    if (isNumber(v)) {
-        /* Initially we start out all numbers as JSVAL_DOUBLE in the type map. If we still
-           see a number in v, its a valid trace but we might want to ask to demote the
-           slot if we know or suspect that its integer. */
+    if (t == JSVAL_INT) { /* initially all whole numbers cause the slot to be demoted */
+        if (!isNumber(v))
+            return false; /* not a number? type mismatch */
         LIns* i = get(&v);
-        if (TYPEMAP_GET_TYPE(t) == JSVAL_DOUBLE) {
-            /* BUG: We can't specialize once the primary trace has been compiled since
-               we would have to recompile it. */
-            if (fragment->root != fragment)
-                return true;
-            /* Don't type specialize really long traces (we count the number of guards in them). */
-            if (DEMOTE_THRESHOLD > 0 && guardCount > DEMOTE_THRESHOLD)
-                return true;
-            if (isInt32(v) && !TYPEMAP_GET_FLAG(t, TYPEMAP_FLAG_DONT_DEMOTE)) {
-                /* If the value associated with v via the tracker comes from a i2f operation,
-                   we can be sure it will always be an int. If we see INCVAR, we similarly
-                   speculate that the result will be int, even though this is not
-                   guaranteed and this might cause the entry map to mismatch and thus
-                   the trace never to be entered. */
-                if (i->isop(LIR_i2f) ||
-                        (i->isop(LIR_fadd) && i->oprnd2()->isconstq() &&
-                                fabs(i->oprnd2()->constvalf()) == 1.0)) {
-#ifdef DEBUG
-                    ptrdiff_t offset;
-                    printf("demoting type of an entry slot #%d, triggering re-compilation\n",
-                            ((offset = nativeGlobalOffset(&v)) == -1)
-                             ? nativeStackOffset(&v)
-                             : offset);
-#endif
-                    JS_ASSERT(!TYPEMAP_GET_FLAG(t, TYPEMAP_FLAG_DEMOTE) ||
-                            TYPEMAP_GET_FLAG(t, TYPEMAP_FLAG_DONT_DEMOTE));
-                    TYPEMAP_SET_FLAG(t, TYPEMAP_FLAG_DEMOTE);
-                    TYPEMAP_SET_TYPE(t, JSVAL_INT);
-                    AUDIT(slotDemoted);
-                    recompileFlag = true;
-                    return true; /* keep going */
-                }
-            }
-            return true;
-        }
-        /* Looks like we are compiling an integer slot. The recorder always casts to doubles
-           after each integer operation, or emits an operation that produces a double right
-           away. If we started with an integer, we must arrive here pointing at a i2f cast.
-           If not, than demoting the slot didn't work out. Flag the slot to be not
-           demoted again. */
-        JS_ASSERT(TYPEMAP_GET_TYPE(t) == JSVAL_INT &&
-                TYPEMAP_GET_FLAG(t, TYPEMAP_FLAG_DEMOTE) &&
-                !TYPEMAP_GET_FLAG(t, TYPEMAP_FLAG_DONT_DEMOTE));
-        if (!i->isop(LIR_i2f)) {
-            AUDIT(slotPromoted);
+        if (!isInt32(v) || (!i->isop(LIR_i2f) && 
+                !(i->isop(LIR_fadd) && i->oprnd2()->isconstq() && 
+                        fabs(i->oprnd2()->constvalf()) == 1.0))) {
 #ifdef DEBUG
             ptrdiff_t offset;
-            printf("demoting type of a slot #%d failed, locking it and re-compiling\n",
-                    ((offset = nativeGlobalOffset(&v)) == -1)
+            printf("int slot is !isInt32, slot #%d, triggering re-compilation\n",
+                   ((offset = nativeGlobalOffset(&v)) == -1)
                      ? nativeStackOffset(&v)
                      : offset);
+            
 #endif
-            TYPEMAP_SET_FLAG(t, TYPEMAP_FLAG_DONT_DEMOTE);
-            TYPEMAP_SET_TYPE(t, JSVAL_DOUBLE);
+            AUDIT(slotPromoted);
+            if (fragment->root == fragment) /* BUG! can't fix miss-speculation without
+                                               recompiling root fragment as well */
+                t = JSVAL_DOUBLE; /* next time make this slot a double */
             recompileFlag = true;
-            return true; /* keep going, recompileFlag will trigger error when we are done with
-                            all the slots */
-
+            return true; /* keep checking types, but request re-compilation */
         }
-        JS_ASSERT(isInt32(v));
-        /* Looks like we got the final LIR_i2f as we expected. Overwrite the value in that
+        /* looks good, slot is an int32, the last instruction should be i2f */
+        JS_ASSERT(i->isop(LIR_i2f));
+        /* we got the final LIR_i2f as we expected. Overwrite the value in that
            slot with the argument of i2f since we want the integer store to flow along
            the loop edge, not the casted value. */
         set(&v, i->oprnd1());
         return true;
     }
+    if (t == JSVAL_DOUBLE) 
+        return isNumber(v);
     /* for non-number types we expect a precise match of the type */
 #ifdef DEBUG
-    if (JSVAL_TAG(v) != TYPEMAP_GET_TYPE(t)) {
+    if (JSVAL_TAG(v) != t) {
         printf("Type mismatch: val %c, map %c ", "OID?S?B"[JSVAL_TAG(v)],
                "OID?S?B"[t]);
     }
 #endif
-    return JSVAL_TAG(v) == TYPEMAP_GET_TYPE(t);
+    return JSVAL_TAG(v) == t;
 }
 
 /* Make sure that the current values in the given stack frame and all stack frames
    up and including entryFrame are type-compatible with the entry map. */
 bool
 TraceRecorder::verifyTypeStability(uint8* map)
 {
     uint8* m = map;
     FORALL_SLOTS(cx, treeInfo->ngslots, treeInfo->gslots, callDepth,
         if (!checkType(*vp, *m))
             return false;
         ++m
     );
+    /* BUG: We can't recompile if this is not the root fragment. */
+    if (recompileFlag && fragment->root != fragment) {
+        fragment->blacklist();
+        return false;
+    }
     return !recompileFlag;
 }
 
 void
 TraceRecorder::closeLoop(Fragmento* fragmento)
 {
     if (!verifyTypeStability(treeInfo->typeMap)) {
         AUDIT(unstableLoopVariable);
@@ -1295,17 +1244,16 @@ js_AttemptToExtendTree(JSContext* cx, Gu
     Fragment* c;
     if (!(c = lr->target)) {
         c = JS_TRACE_MONITOR(cx).fragmento->createBranch(lr, lr->exit);
         c->spawnedFrom = lr->guard;
         c->parent = f;
         lr->exit->target = c;
         lr->target = c;
         c->root = f;
-        c->calldepth = lr->calldepth;
     }
 
     if (++c->hits() >= HOTEXIT) {
         /* start tracing secondary trace from this point */
         c->lirbuf = f->lirbuf;
         return js_StartRecorder(cx, lr, c, lr->guard->exit()->typeMap);
     }
     return false;
@@ -1385,17 +1333,17 @@ js_LoopEdge(JSContext* cx, jsbytecode* o
             }
 
             /* capture the entry type map if we don't have one yet (or we threw it away) */
             if (!ti->typeMap) {
                 ti->typeMap = (uint8*)malloc((ti->entryNativeStackSlots + ti->ngslots) * sizeof(uint8));
                 uint8* m = ti->typeMap;
                 /* remember the coerced type of each active slot in the type map */
                 FORALL_SLOTS(cx, ti->ngslots, ti->gslots, 0/*callDepth*/,
-                    *m++ = getCoercedType(*vp)
+                    *m++ = isInt32(*vp) ? JSVAL_INT : JSVAL_TAG(*vp);
                 );
             }
             /* recording primary trace */
             return js_StartRecorder(cx, NULL, f, ti->typeMap);
         }
         return false;
     }
 
--- a/js/src/jstracer.h
+++ b/js/src/jstracer.h
@@ -110,26 +110,18 @@ public:
     uint32                  globalShape;
     unsigned                ngslots;
     uint8                  *typeMap;
     uint16                 *gslots;
 };
 
 extern struct nanojit::CallInfo builtins[];
 
-#define TYPEMAP_GET_TYPE(x)         ((x) & JSVAL_TAGMASK)
-#define TYPEMAP_SET_TYPE(x, t)      (x = (x & 0xf0) | t)
-#define TYPEMAP_GET_FLAG(x, flag)   ((x) & flag)
-#define TYPEMAP_SET_FLAG(x, flag)   do { (x) |= flag; } while (0)
-
 #define TYPEMAP_TYPE_ANY            7
 
-#define TYPEMAP_FLAG_DEMOTE 0x10 /* try to record as int */
-#define TYPEMAP_FLAG_DONT_DEMOTE 0x20 /* do not try to record as int */
-
 class TraceRecorder {
     JSContext*              cx;
     JSObject*               globalObj;
     Tracker                 tracker;
     Tracker                 nativeFrameTracker;
     char*                   entryTypeMap;
     struct JSFrameRegs*     entryRegs;
     unsigned                callDepth;