Hands down the hardest bug I had to debug in TM so far. Make sure to read back any registers an inner tree might have changed before writing out the typemap for the nested_exit guard, otherwise we might be pointing to old stale pre-(inner-)loop state and pick an incorrect (in this case too narrow) type. fannkuch=2.8x with this.
authorAndreas Gal <gal@mozilla.com>
Tue, 12 Aug 2008 20:18:29 -0700
changeset 18116 966c38b7fc2a3b05315f2f427dfb89dafec5c1d9
parent 18114 3d7ea778485f7d3fa75e6e2fee8a37d5b44c82df
child 18117 c0da9464cd2c230fe6e6f0facc1d9a91d5eb08ca
push id1452
push usershaver@mozilla.com
push dateFri, 22 Aug 2008 00:08:22 +0000
treeherdermozilla-central@d13bb0868596 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
milestone1.9.1a2pre
Hands down the hardest bug I had to debug in TM so far. Make sure to read back any registers an inner tree might have changed before writing out the typemap for the nested_exit guard, otherwise we might be pointing to old stale pre-(inner-)loop state and pick an incorrect (in this case too narrow) type. fannkuch=2.8x with this.
js/src/jstracer.cpp
js/src/nanojit/avmplus.h
--- a/js/src/jstracer.cpp
+++ b/js/src/jstracer.cpp
@@ -1150,29 +1150,32 @@ TraceRecorder::snapshot(ExitType exitTyp
     /* reserve space for the type map */
     unsigned ngslots = treeInfo->globalSlots.length();
     LIns* data = lir_buf_writer->skip((stackSlots + ngslots) * sizeof(uint8));
     /* setup side exit structure */
     memset(&exit, 0, sizeof(exit));
     exit.from = fragment;
     exit.calldepth = callDepth;
     exit.numGlobalSlots = ngslots;
+    exit.numStackSlots = stackSlots;
     exit.exitType = exitType;
     exit.ip_adj = fp->regs->pc - (jsbytecode*)fragment->root->ip;
     exit.sp_adj = (stackSlots - treeInfo->entryNativeStackSlots) * sizeof(double);
     exit.rp_adj = exit.calldepth * sizeof(FrameInfo);
     uint8* m = exit.typeMap = (uint8 *)data->payload();
     /* Determine the type of a store by looking at the current type of the actual value the
        interpreter is using. For numbers we have to check what kind of store we used last
        (integer or double) to figure out what the side exit show reflect in its typemap. */
     FORALL_SLOTS(cx, ngslots, treeInfo->globalSlots.data(), callDepth,
         LIns* i = get(vp);
-        *m++ = isNumber(*vp)
+        *m = isNumber(*vp)
                ? (isPromoteInt(i) ? JSVAL_INT : JSVAL_DOUBLE)
                : JSVAL_TAG(*vp);
+        JS_ASSERT((*m != JSVAL_INT) || isInt32(*vp));
+        ++m;
     );
     return &exit;
 }
 
 /* Emit a guard for condition (cond), expecting to evaluate to boolean result (expected). */
 LIns*
 TraceRecorder::guard(bool expected, LIns* cond, ExitType exitType)
 {
@@ -1318,22 +1321,23 @@ TraceRecorder::emitTreeCall(Fragment* in
         lir->insStorei(lir->ins2i(LIR_add, lirbuf->sp, sp_adj), 
                 lirbuf->state, offsetof(InterpState, sp));
     }
     /* Invoke the inner tree. */
     LIns* args[] = { lir->insImmPtr(inner), lirbuf->state }; /* reverse order */
     LIns* ret = lir->insCall(F_CallTree, args);
     /* Make a note that we now depend on that tree. */
     ((TreeInfo*)inner->vmprivate)->dependentTrees.addUnique(fragment);
+    /* Read back all registers, in case the called tree changed any of them. */
+    SideExit* exit = lr->exit;
+    import(exit->numGlobalSlots, exit->typeMap, exit->typeMap + exit->numGlobalSlots);
     /* Guard that we come out of the inner tree along the same side exit we came out when
        we called the inner tree at recording time. */
-    LIns* g = guard(true, lir->ins2(LIR_eq, ret, lir->insImmPtr(lr)), NESTED_EXIT);
+    guard(true, lir->ins2(LIR_eq, ret, lir->insImmPtr(lr)), NESTED_EXIT);
     /* Re-read all values from the native frames since the inner tree might have changed them. */
-    SideExit* exit = g->exit();
-    import(exit->numGlobalSlots, exit->typeMap, exit->typeMap + exit->numGlobalSlots);
     /* Restore state->sp to its original value (we still have it in a register). */
     if (callDepth > 0)
         lir->insStorei(lirbuf->sp, lirbuf->state, offsetof(InterpState, sp));
 }
 
 int
 nanojit::StackFilter::getTop(LInsp guard)
 {
--- a/js/src/nanojit/avmplus.h
+++ b/js/src/nanojit/avmplus.h
@@ -153,16 +153,17 @@ namespace nanojit
 	{
         intptr_t ip_adj;
         intptr_t sp_adj;
         intptr_t rp_adj;
         Fragment *target;
         Fragment *from;
         int32_t calldepth;
         uint32 numGlobalSlots;
+        uint32 numStackSlots;
         uint8 *typeMap;
         ExitType exitType;
 #if defined NJ_VERBOSE
 		uint32_t sid;
 #endif
 	};
 
 	class LIns;