[INFER] Only patch ints->doubles in existing frames for slots the recompiler thinks are doubles, bug 642412.
authorBrian Hackett <bhackett1024@gmail.com>
Sat, 19 Mar 2011 09:11:56 -0700
changeset 74812 7154281e487bdec4825299b2ebee511c9aac51df
parent 74811 9ee17aa5f93892227d777ab9c37ce6aef59a81b6
child 74813 332d7b94dc35f7a5bfd4b0583c27a3a8942e1ed6
push id2
push userbsmedberg@mozilla.com
push dateFri, 19 Aug 2011 14:38:13 +0000
bugs642412
milestone2.0b13pre
[INFER] Only patch ints->doubles in existing frames for slots the recompiler thinks are doubles, bug 642412.
js/src/jit-test/tests/jaeger/recompile/patchdouble.js
js/src/methodjit/Compiler.cpp
js/src/methodjit/Compiler.h
js/src/methodjit/FrameState-inl.h
js/src/methodjit/FrameState.h
js/src/methodjit/Retcon.cpp
js/src/methodjit/Retcon.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/jaeger/recompile/patchdouble.js
@@ -0,0 +1,7 @@
+// only fix doubles for slots which the recompiled script thinks are doubles.
+function foo(x) {
+  var y = x & 0xffff;
+  y = (y * (x * 1000));
+  assertEq(y, 140735340806145000);
+}
+foo(0x7fffffff);
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -78,19 +78,20 @@ using namespace js::mjit::ic;
 #if defined(JS_METHODJIT_SPEW)
 static const char *OpcodeNames[] = {
 # define OPDEF(op,val,name,token,length,nuses,ndefs,prec,format) #name,
 # include "jsopcode.tbl"
 # undef OPDEF
 };
 #endif
 
-mjit::Compiler::Compiler(JSContext *cx, JSStackFrame *fp)
+mjit::Compiler::Compiler(JSContext *cx, JSStackFrame *fp, const Vector<PatchableFrame> *frames)
   : BaseCompiler(cx),
     fp(fp),
+    frames(frames),
     script(fp->script()),
     scopeChain(&fp->scopeChain()),
     globalObj(scopeChain->getGlobal()),
     fun(fp->isFunctionFrame() && !fp->isEvalFrame()
         ? fp->fun()
         : NULL),
     isConstructing(fp->isConstructing()),
     analysis(NULL), jumpMap(NULL), savedTraps(NULL),
@@ -130,27 +131,27 @@ mjit::Compiler::Compiler(JSContext *cx, 
     applyTricks(NoApplyTricks)
 {
     /* :FIXME: bug 637856 disabling traceJit if inference is enabled */
     if (cx->typeInferenceEnabled())
         addTraceHints = false;
 }
 
 CompileStatus
-mjit::Compiler::compile(const Vector<JSStackFrame*> *frames)
+mjit::Compiler::compile()
 {
     JS_ASSERT_IF(isConstructing, !script->jitCtor);
     JS_ASSERT_IF(!isConstructing, !script->jitNormal);
 
     JITScript **jit = isConstructing ? &script->jitCtor : &script->jitNormal;
     void **checkAddr = isConstructing
                        ? &script->jitArityCheckCtor
                        : &script->jitArityCheckNormal;
 
-    CompileStatus status = performCompilation(jit, frames);
+    CompileStatus status = performCompilation(jit);
     if (status == Compile_Okay) {
         // Global scripts don't have an arity check entry. That's okay, we
         // just need a pointer so the VM can quickly decide whether this
         // method can be JIT'd or not. Global scripts cannot be IC'd, since
         // they have no functions, so there is no danger.
         *checkAddr = (*jit)->arityCheckEntry
                      ? (*jit)->arityCheckEntry
                      : (*jit)->invokeEntry;
@@ -169,17 +170,17 @@ mjit::Compiler::compile(const Vector<JSS
                 js_ReportOutOfMemory(cx);                            \
             if (!cx->compartment->types.checkPendingRecompiles(cx))  \
                 return Compile_Error;                                \
             return status_;                                          \
         }                                                            \
     JS_END_MACRO
 
 CompileStatus
-mjit::Compiler::performCompilation(JITScript **jitp, const Vector<JSStackFrame*> *frames)
+mjit::Compiler::performCompilation(JITScript **jitp)
 {
     JaegerSpew(JSpew_Scripts, "compiling script (file \"%s\") (line \"%d\") (length \"%d\")\n",
                script->filename, script->lineno, script->length);
 
     analyze::Script analysis_;
     analysis_.analyze(cx, script);
 
     this->analysis = &analysis_;
@@ -244,17 +245,17 @@ mjit::Compiler::performCompilation(JITSc
     for (uint32 i = 0; i < script->nClosedVars; i++)
         frame.setClosedVar(script->getClosedVar(i));
     for (uint32 i = 0; i < script->nClosedArgs; i++)
         frame.setClosedArg(script->getClosedArg(i));
 
     types::AutoEnterTypeInference enter(cx, true);
 
     if (cx->typeInferenceEnabled()) {
-        CompileStatus status = prepareInferenceTypes(frames);
+        CompileStatus status = prepareInferenceTypes();
         if (status != Compile_Okay) {
             if (!cx->compartment->types.checkPendingRecompiles(cx))
                 return Compile_Error;
             return status;
         }
     }
 
     CHECK_STATUS(generatePrologue());
@@ -283,109 +284,47 @@ mjit::Compiler::performCompilation(JITSc
 
 mjit::Compiler::~Compiler()
 {
     cx->free(jumpMap);
     cx->free(savedTraps);
 }
 
 CompileStatus
-mjit::Compiler::prepareInferenceTypes(const Vector<JSStackFrame*> *frames)
+mjit::Compiler::prepareInferenceTypes()
 {
     /* Analyze the script if we have not already done so. */
     if (!script->types) {
         /* Uncached eval scripts are not analyzed or compiled. */
         if (script->isUncachedEval)
             return Compile_Abort;
         types::AnalyzeScriptTypes(cx, script);
         if (!script->types)
             return Compile_Error;
     }
 
-    /*
-     * Fill in known types of arguments and locals, and patch up doubles for
-     * arguments and locals in existing frames. Any value assumed to be a double
-     * in this compilation may instead be an int in earlier compilation and stack
-     * frames. We handle this by patching up all hold stack frames to ensure that
-     * arguments, locals, and stack values we treat as doubles actually are doubles.
-     *
-     * Skip this for closed arguments and locals, which could be assigned an
-     * integer value at any time elsewhere in the VM.
-     */
+    /* Get the known types of arguments and locals. */
 
     uint32 nargs = fun ? fun->nargs : 0;
     if (!argumentTypes.reserve(nargs))
         return Compile_Error;
     for (unsigned i = 0; i < nargs; i++) {
         JSValueType type = JSVAL_TYPE_UNKNOWN;
         if (!analysis->argEscapes(i))
             type = script->argTypes(i)->getKnownTypeTag(cx, script);
         argumentTypes.append(type);
-        if (type == JSVAL_TYPE_DOUBLE && frames) {
-            for (unsigned j = 0; j < frames->length(); j++) {
-                Value *vp = (*frames)[j]->formalArgs() + i;
-                if (vp->isInt32())
-                    vp->setDouble((double)vp->toInt32());
-            }
-        }
     }
 
     if (!localTypes.reserve(script->nfixed))
         return Compile_Error;
     for (unsigned i = 0; i < script->nfixed; i++) {
         JSValueType type = JSVAL_TYPE_UNKNOWN;
         if (!analysis->localHasUseBeforeDef(i))
             type = script->localTypes(i)->getKnownTypeTag(cx, script);
         localTypes.append(type);
-        if (type == JSVAL_TYPE_DOUBLE && frames) {
-            for (unsigned j = 0; j < frames->length(); j++) {
-                Value *vp = (*frames)[j]->slots() + i;
-                if (vp->isInt32())
-                    vp->setDouble((double)vp->toInt32());
-            }
-        }
-    }
-
-    /*
-     * Patch up stack values in other frames which need to be doubles. We only
-     * keep track of the types of things pushed by a particular opcode, so can't
-     * quickly figure out what the inferred type is for the entire stack at a
-     * given point. For each active frame, work backwards to the start of the
-     * current basic block looking for known pushed doubles. We don't need to
-     * go back before the start of the basic block, as nothing is assumed about
-     * stack values at the start of basic blocks.
-     */
-    for (unsigned i = 0; frames && i < frames->length(); i++) {
-        uint32 offset = (*frames)[i]->pc(cx) - script->code;
-        uint32 stackDepth = analysis->getCode(offset).stackDepth;
-        if (!stackDepth)
-            continue;
-        JS_ASSERT(offset);
-        while (offset--) {
-            analyze::Bytecode *code = analysis->maybeCode(offset);
-            if (!code)
-                continue;
-
-            uint32 startDepth = code->stackDepth - analyze::GetUseCount(script, offset);
-            if (startDepth < stackDepth) {
-                for (unsigned j = startDepth; j < stackDepth; j++) {
-                    types::TypeSet *types = script->types->pushed(offset, j - startDepth);
-                    JSValueType type = types->getKnownTypeTag(cx, NULL);
-                    if (type == JSVAL_TYPE_DOUBLE) {
-                        Value *vp = (*frames)[i]->slots() + script->nfixed + j;
-                        if (vp->isInt32())
-                            vp->setDouble((double)vp->toInt32());
-                    }
-                }
-                stackDepth = startDepth;
-            }
-
-            if (code->jumpTarget)
-                break;
-        }
     }
 
     return Compile_Okay;
 }
 
 CompileStatus JS_NEVER_INLINE
 mjit::TryCompile(JSContext *cx, JSStackFrame *fp)
 {
@@ -399,18 +338,18 @@ mjit::TryCompile(JSContext *cx, JSStackF
     // Ensure that constructors have at least one slot.
     if (fp->isConstructing() && !fp->script()->nslots)
         fp->script()->nslots++;
 
     // If there are static overflows in the function, try recompiling it a few
     // times, using a limit to handle scripts with many static overflows.
     CompileStatus status = Compile_Overflow;
     for (unsigned i = 0; status == Compile_Overflow && i < 5; i++) {
-        Compiler cc(cx, fp);
-        status = cc.compile(NULL);
+        Compiler cc(cx, fp, NULL);
+        status = cc.compile();
     }
 
     return status;
 }
 
 bool
 mjit::Compiler::loadOldTraps(const Vector<CallSite> &sites)
 {
@@ -1093,16 +1032,23 @@ public:
 
 #define BEGIN_CASE(name)        case name:
 #define END_CASE(name)                      \
     JS_BEGIN_MACRO                          \
         PC += name##_LENGTH;                \
     JS_END_MACRO;                           \
     break;
 
+static inline void
+FixDouble(Value &val)
+{
+    if (val.isInt32())
+        val.setDouble((double)val.toInt32());
+}
+
 CompileStatus
 mjit::Compiler::generateMethod()
 {
     mjit::AutoScriptRetrapper trapper(cx, script);
     SrcNoteLineScanner scanner(script->notes());
 
     /* For join points, whether there was fallthrough from the previous opcode. */
     bool fallthrough = true;
@@ -1204,16 +1150,51 @@ mjit::Compiler::generateMethod()
                                    Assembler::stackPointerRegister);
             }
             stubcc.crossJump(stubcc.masm.jump(), masm.label());
 
             InternalCallSite site(offset, PC, CallSite::MAGIC_TRAP_ID, false, true);
             addCallSite(site);
         }
 
+        /*
+         * If we are recompiling, check for any frames on the stack at this
+         * opcode, and patch the types of any arg/local/stack slots which are
+         * integers but need to be doubles. Any value assumed to be a double in
+         * this compilation may instead be an int in the earlier compilation
+         * and stack frames. Other transitions between known types are not
+         * possible --- type sets can only grow, and if new non-double type
+         * tags become possible we will treat that slot as unknown in this
+         * compilation.
+         */
+        for (unsigned i = 0; frames && i < frames->length(); i++) {
+            if ((*frames)[i].pc != PC)
+                continue;
+            JSStackFrame *patchfp = (*frames)[i].fp;
+
+            for (unsigned j = 0; fun && j < fun->nargs; j++) {
+                FrameEntry *fe = frame.getArg(j);
+                if (fe->isType(JSVAL_TYPE_DOUBLE))
+                    FixDouble(patchfp->formalArg(j));
+            }
+
+            for (unsigned j = 0; j < script->nfixed; j++) {
+                FrameEntry *fe = frame.getLocal(j);
+                if (fe->isType(JSVAL_TYPE_DOUBLE))
+                    FixDouble(patchfp->varSlot(j));
+            }
+
+            unsigned depth = opinfo->stackDepth - analyze::GetUseCount(script, PC - script->code);
+            for (unsigned j = 0; j < depth; j++) {
+                FrameEntry *fe = frame.getStack(j);
+                if (fe->isType(JSVAL_TYPE_DOUBLE))
+                    FixDouble(patchfp->base()[j]);
+            }
+        }
+
     /**********************
      * BEGIN COMPILER OPS *
      **********************/ 
 
         jsbytecode *oldPC = PC;
 
         switch (op) {
           BEGIN_CASE(JSOP_NOP)
--- a/js/src/methodjit/Compiler.h
+++ b/js/src/methodjit/Compiler.h
@@ -48,16 +48,21 @@
 #include "BaseCompiler.h"
 #include "StubCompiler.h"
 #include "MonoIC.h"
 #include "PolyIC.h"
 
 namespace js {
 namespace mjit {
 
+struct PatchableFrame {
+    JSStackFrame *fp;
+    jsbytecode *pc;
+};
+
 class Compiler : public BaseCompiler
 {
     friend class StubCompiler;
 
     struct BranchPatch {
         BranchPatch(const Jump &j, jsbytecode *pc)
           : jump(j), pc(pc)
         { }
@@ -337,16 +342,20 @@ class Compiler : public BaseCompiler
     };
 
     struct LoopEntry {
         uint32 pcOffset;
         Label label;
     };
 
     JSStackFrame *fp;
+
+    /* Existing frames on the stack whose slots may need to be updated. */
+    const Vector<PatchableFrame> *frames;
+
     JSScript *script;
     JSObject *scopeChain;
     JSObject *globalObj;
     JSFunction *fun;
     bool isConstructing;
     analyze::Script *analysis;
     Label *jumpMap;
     bool *savedTraps;
@@ -394,41 +403,41 @@ class Compiler : public BaseCompiler
     Compiler *thisFromCtor() { return this; }
 
     friend class CompilerAllocPolicy;
   public:
     // Special atom index used to indicate that the atom is 'length'. This
     // follows interpreter usage in JSOP_LENGTH.
     enum { LengthAtomIndex = uint32(-2) };
 
-    Compiler(JSContext *cx, JSStackFrame *fp);
+    Compiler(JSContext *cx, JSStackFrame *fp, const Vector<PatchableFrame> *frames);
     ~Compiler();
 
-    CompileStatus compile(const Vector<JSStackFrame*> *frames);
+    CompileStatus compile();
 
     jsbytecode *getPC() { return PC; }
     Label getLabel() { return masm.label(); }
     bool knownJump(jsbytecode *pc);
     Label labelOf(jsbytecode *target);
     void *findCallSite(const CallSite &callSite);
     void addCallSite(const InternalCallSite &callSite);
     void addReturnSite(Label joinPoint);
     bool loadOldTraps(const Vector<CallSite> &site);
 
     bool debugMode() { return debugMode_; }
 
   private:
-    CompileStatus performCompilation(JITScript **jitp, const Vector<JSStackFrame*> *frames);
+    CompileStatus performCompilation(JITScript **jitp);
     CompileStatus generatePrologue();
     CompileStatus generateMethod();
     CompileStatus generateEpilogue();
     CompileStatus finishThisUp(JITScript **jitp);
 
     /* Analysis helpers. */
-    CompileStatus prepareInferenceTypes(const Vector<JSStackFrame*> *frames);
+    CompileStatus prepareInferenceTypes();
     void fixDoubleTypes(Uses uses);
     void restoreAnalysisTypes(uint32 stackDepth);
     JSValueType knownThisType();
     JSValueType knownArgumentType(uint32 arg);
     JSValueType knownLocalType(uint32 local);
     JSValueType knownPushedType(uint32 pushed);
     bool arrayPrototypeHasIndexedProperty();
     bool mayPushUndefined(uint32 pushed);
--- a/js/src/methodjit/FrameState-inl.h
+++ b/js/src/methodjit/FrameState-inl.h
@@ -978,16 +978,23 @@ FrameState::getOrTrack(uint32 index)
     if (!fe->isTracked()) {
         addToTracker(fe);
         fe->resetSynced();
     }
     return fe;
 }
 
 inline FrameEntry *
+FrameState::getStack(uint32 slot)
+{
+    JS_ASSERT(slot < uint32(sp - spBase));
+    return getOrTrack(uint32(&spBase[slot] - entries));
+}
+
+inline FrameEntry *
 FrameState::getLocal(uint32 slot)
 {
     JS_ASSERT(slot < script->nslots);
     return getOrTrack(uint32(&locals[slot] - entries));
 }
 
 inline FrameEntry *
 FrameState::getArg(uint32 slot)
--- a/js/src/methodjit/FrameState.h
+++ b/js/src/methodjit/FrameState.h
@@ -359,16 +359,17 @@ class FrameState
     // Pushes a copy of a slot (formal argument, local variable, or stack slot)
     // onto the operation stack.
     void pushLocal(uint32 n, JSValueType knownType);
     void pushArg(uint32 n, JSValueType knownType);
     void pushCallee();
     void pushThis();
     inline void learnThisIsObject();
 
+    inline FrameEntry *getStack(uint32 slot);
     inline FrameEntry *getLocal(uint32 slot);
     inline FrameEntry *getArg(uint32 slot);
 
     /*
      * Allocates a temporary register for a FrameEntry's type. The register
      * can be spilled or clobbered by the frame. The compiler may only operate
      * on it temporarily, and must take care not to clobber it.
      */
--- a/js/src/methodjit/Retcon.cpp
+++ b/js/src/methodjit/Retcon.cpp
@@ -248,35 +248,37 @@ Recompiler::recompile()
      *    new compiler gives us for the call sites.
      */
     Vector<PatchableAddress> normalPatches(cx);
     Vector<PatchableAddress> ctorPatches(cx);
     Vector<PatchableNative> normalNatives(cx);
     Vector<PatchableNative> ctorNatives(cx);
 
     /* Frames containing data that may need to be patched from int to double. */
-    Vector<JSStackFrame*> normalFrames(cx);
-    Vector<JSStackFrame*> ctorFrames(cx);
+    Vector<PatchableFrame> normalFrames(cx);
+    Vector<PatchableFrame> ctorFrames(cx);
 
     // Find all JIT'd stack frames to account for return addresses that will
     // need to be patched after recompilation.
     for (VMFrame *f = script->compartment->jaegerCompartment->activeFrame();
          f != NULL;
          f = f->previous) {
 
         // Scan all frames owned by this VMFrame.
         JSStackFrame *end = f->entryfp->prev();
         JSStackFrame *next = NULL;
         for (JSStackFrame *fp = f->fp(); fp != end; fp = fp->prev()) {
             if (fp->script() == script) {
                 // Remember every frame for each type of JIT'd code.
-                fp->pc(cx, next);
-                if (fp->isConstructing() && !ctorFrames.append(fp))
+                PatchableFrame frame;
+                frame.fp = fp;
+                frame.pc = fp->pc(cx, next);
+                if (fp->isConstructing() && !ctorFrames.append(frame))
                     return false;
-                if (!fp->isConstructing() && !normalFrames.append(fp))
+                if (!fp->isConstructing() && !normalFrames.append(frame))
                     return false;
             }
 
             // check for a scripted call returning into the recompiled script.
             void **addr = fp->addressOfNativeReturnAddress();
             if (script->jitCtor && script->jitCtor->isValidCode(*addr)) {
                 if (!ctorPatches.append(findPatch(script->jitCtor, addr)))
                     return false;
@@ -359,29 +361,29 @@ Recompiler::cleanup(JITScript *jit, Vect
     }
 
     *recompilations = jit->recompilations;
 
     return true;
 }
 
 bool
-Recompiler::recompile(Vector<JSStackFrame*> &frames, Vector<PatchableAddress> &patches, Vector<CallSite> &sites,
+Recompiler::recompile(Vector<PatchableFrame> &frames, Vector<PatchableAddress> &patches, Vector<CallSite> &sites,
                       Vector<PatchableNative> &natives,
                       uint32 recompilations)
 {
-    JSStackFrame *fp = frames[0];
+    JSStackFrame *fp = frames[0].fp;
 
     JaegerSpew(JSpew_Recompile, "On stack recompilation, %u patches, %u natives\n",
                patches.length(), natives.length());
 
-    Compiler c(cx, fp);
+    Compiler c(cx, fp, &frames);
     if (!c.loadOldTraps(sites))
         return false;
-    if (c.compile(&frames) != Compile_Okay)
+    if (c.compile() != Compile_Okay)
         return false;
 
     script->getJIT(fp->isConstructing())->recompilations = recompilations + 1;
 
     /* Perform the earlier scanned patches */
     for (uint32 i = 0; i < patches.length(); i++)
         applyPatch(c, patches[i]);
     for (uint32 i = 0; i < natives.length(); i++)
--- a/js/src/methodjit/Retcon.h
+++ b/js/src/methodjit/Retcon.h
@@ -105,17 +105,17 @@ public:
 private:
     JSContext *cx;
     JSScript *script;
     
     PatchableAddress findPatch(JITScript *jit, void **location);
     void applyPatch(Compiler& c, PatchableAddress& toPatch);
     PatchableNative stealNative(JITScript *jit, jsbytecode *pc);
     void patchNative(JITScript *jit, PatchableNative &native);
-    bool recompile(Vector<JSStackFrame*> &frames,
+    bool recompile(Vector<PatchableFrame> &frames,
                    Vector<PatchableAddress> &patches, Vector<CallSite> &sites,
                    Vector<PatchableNative> &natives,
                    uint32 recompilations);
 
     /* Detach jit from any IC callers and save any traps to sites. */
     bool cleanup(JITScript *jit, Vector<CallSite> *sites, uint32 *recompilations);
 };