Bug 742163 - Clean up JSScript::jitArityCheck{Normal,Ctor}. r=dvander.

2012-04-05 00:02:02 -07:00 · 2012-04-05 00:02:02 -07:00 · 6fb367bfab
--- a/js/src/jsinfer.cpp
+++ b/js/src/jsinfer.cpp
@ -2205,15 +2205,15 @@ TypeCompartment::addPendingRecompile(JSContext *cx, JSScript *script, jsbytecode
    RecompileInfo info;
    info.script = script;

-    if (script->jitNormal) {
+    if (script->jitHandleNormal.isValid()) {
        info.constructing = false;
-        info.chunkIndex = script->jitNormal->chunkIndex(pc);
+        info.chunkIndex = script->jitHandleNormal.getValid()->chunkIndex(pc);
        addPendingRecompile(cx, info);
    }

-    if (script->jitCtor) {
+    if (script->jitHandleCtor.isValid()) {
        info.constructing = true;
-        info.chunkIndex = script->jitCtor->chunkIndex(pc);
+        info.chunkIndex = script->jitHandleCtor.getValid()->chunkIndex(pc);
        addPendingRecompile(cx, info);
    }
 #endif
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@ -1689,7 +1689,7 @@ void
 JSScript::recompileForStepMode(FreeOp *fop)
 {
 #ifdef JS_METHODJIT
-    if (jitNormal || jitCtor) {
+    if (hasJITCode()) {
        mjit::Recompiler::clearStackReferences(fop, this);
        mjit::ReleaseScriptCode(fop, this);
    }
--- a/js/src/jsscript.h
+++ b/js/src/jsscript.h
@ -272,9 +272,13 @@ namespace JSC {
    class ExecutablePool;
 }

-#define JS_UNJITTABLE_SCRIPT (reinterpret_cast<void*>(1))
+namespace js {
+namespace mjit {
+    struct JITScript;
+    class CallCompiler;
+}
+}

-namespace js { namespace mjit { struct JITScript; } }
 #endif

 namespace js {
@ -343,18 +347,61 @@ struct JSScript : public js::gc::Cell
    static const uint32_t stepFlagMask = 0x80000000U;
    static const uint32_t stepCountMask = 0x7fffffffU;

-  /*
-   * We order fields according to their size in order to avoid wasting space
-   * for alignment.
-   */
+  public:
+    // This type wraps JITScript.  It has three possible states.
+    // - "Empty": no compilation has been attempted and there is no JITScript.
+    // - "Unjittable": compilation failed and there is no JITScript.
+    // - "Valid": compilation succeeded and there is a JITScript.
+    class JITScriptHandle
+    {
+        // CallCompiler must be a friend because it generates code that uses
+        // UNJITTABLE.
+        friend class js::mjit::CallCompiler;

-  /* Larger-than-word-sized fields. */
+        // The exact representation:
+        // - NULL means "empty".
+        // - UNJITTABLE means "unjittable".
+        // - Any other value means "valid".
+        // UNJITTABLE = 1 so that we can check that a JITScript is valid
+        // with a single |> 1| test.  It's defined outside the class because
+        // non-integral static const fields can't be defined in the class.
+        static const js::mjit::JITScript *UNJITTABLE;   // = (JITScript *)1;
+        js::mjit::JITScript *value;
+
+      public:
+        JITScriptHandle()       { value = NULL; }
+
+        bool isEmpty()          { return value == NULL; }
+        bool isUnjittable()     { return value == UNJITTABLE; }
+        bool isValid()          { return value  > UNJITTABLE; }
+
+        js::mjit::JITScript *getValid() {
+            JS_ASSERT(isValid());
+            return value;
+        }
+
+        void setEmpty()         { value = NULL; }
+        void setUnjittable()    { value = const_cast<js::mjit::JITScript *>(UNJITTABLE); }
+        void setValid(js::mjit::JITScript *jit) {
+            value = jit;
+            JS_ASSERT(isValid());
+        }
+
+        static void staticAsserts();
+    };
+
+    //
+    // We order fields according to their size in order to avoid wasting space
+    // for alignment.
+    //
+
+    // Larger-than-word-sized fields.

  public:
    js::Bindings    bindings;   /* names of top-level variables in this script
                                   (and arguments if this is a function script) */

-  /* Word-sized fields. */
+    // Word-sized fields.

  public:
    jsbytecode      *code;      /* bytecodes and their immediate operands */
@ -388,16 +435,10 @@ struct JSScript : public js::gc::Cell
    /* Persistent type information retained across GCs. */
    js::types::TypeScript *types;

+  public:
 #ifdef JS_METHODJIT
-    // Fast-cached pointers to make calls faster. These are also used to
-    // quickly test whether there is JIT code; a NULL value means no
-    // compilation has been attempted. A JS_UNJITTABLE_SCRIPT value means
-    // compilation failed. Any value is the arity-check entry point.
-    void *jitArityCheckNormal;
-    void *jitArityCheckCtor;
-
-    js::mjit::JITScript *jitNormal;   /* Extra JIT info for normal scripts */
-    js::mjit::JITScript *jitCtor;     /* Extra JIT info for constructors */
+    JITScriptHandle jitHandleNormal; // extra JIT info for normal scripts
+    JITScriptHandle jitHandleCtor;   // extra JIT info for constructors
 #endif

  private:
@ -411,7 +452,7 @@ struct JSScript : public js::gc::Cell
    void *padding_;
 #endif

-    /* 32-bit fields. */
+    // 32-bit fields.

  public:
    uint32_t        length;     /* length of code vector */
@ -424,17 +465,15 @@ struct JSScript : public js::gc::Cell
    uint32_t        natoms;     /* length of atoms array */

 #ifdef DEBUG
-    /*
-     * Unique identifier within the compartment for this script, used for
-     * printing analysis information.
-     */
+    // Unique identifier within the compartment for this script, used for
+    // printing analysis information.
    uint32_t        id_;
  private:
    uint32_t        idpad;
  public:
 #endif

-    /* 16-bit fields. */
+    // 16-bit fields.

  private:
    uint16_t        version;    /* JS version under which script was compiled */
@ -449,13 +488,11 @@ struct JSScript : public js::gc::Cell
    uint16_t        nslots;     /* vars plus maximum stack depth */
    uint16_t        staticLevel;/* static level for display maintenance */

-    /* 8-bit fields. */
+    // 8-bit fields.

-    /*
-     * Offsets to various array structures from the end of this script, or
-     * JSScript::INVALID_OFFSET if the array has length 0.
-     */
  public:
+    // Offsets to various array structures from the end of this script, or
+    // JSScript::INVALID_OFFSET if the array has length 0.
    uint8_t         constsOffset;   /* offset to the array of constants */
    uint8_t         objectsOffset;  /* offset to the array of nested function,
                                       block, scope, xml and one-time regexps
@ -467,7 +504,7 @@ struct JSScript : public js::gc::Cell
    uint8_t         closedArgsOffset; /* offset to the array of closed args */
    uint8_t         closedVarsOffset; /* offset to the array of closed vars */

-    /* 1-bit fields. */
+    // 1-bit fields.

  public:
    bool            noScriptRval:1; /* no need for result value of last
@ -514,7 +551,9 @@ struct JSScript : public js::gc::Cell
    bool            analyzedArgsUsage_:1;
    bool            needsArgsObj_:1;

-    /* End of fields.  Start methods. */
+    //
+    // End of fields.  Start methods.
+    //

    /*
     * Two successively less primitive ways to make a new JSScript.  The first
@ -601,21 +640,36 @@ struct JSScript : public js::gc::Cell
  private:
    bool makeTypes(JSContext *cx);
    bool makeAnalysis(JSContext *cx);
-  public:

 #ifdef JS_METHODJIT
-    bool hasJITCode() {
-        return jitNormal || jitCtor;
+  private:
+    // CallCompiler must be a friend because it generates code that directly
+    // accesses jitHandleNormal/jitHandleCtor, via jitHandleOffset().
+    friend class js::mjit::CallCompiler;
+
+    static size_t jitHandleOffset(bool constructing) {
+        return constructing ? offsetof(JSScript, jitHandleCtor)
+                            : offsetof(JSScript, jitHandleNormal);
    }

+  public:
+    bool hasJITCode()   { return jitHandleNormal.isValid() || jitHandleCtor.isValid(); }
+
+    JITScriptHandle *jitHandle(bool constructing) {
+        return constructing ? &jitHandleCtor : &jitHandleNormal;
+    }
+
+    js::mjit::JITScript *getJIT(bool constructing) {
+        JITScriptHandle *jith = jitHandle(constructing);
+        return jith->isValid() ? jith->getValid() : NULL;
+    }
+
+    static void ReleaseCode(js::FreeOp *fop, JITScriptHandle *jith);
+
    // These methods are implemented in MethodJIT.h.
    inline void **nativeMap(bool constructing);
    inline void *nativeCodeForPC(bool constructing, jsbytecode *pc);

-    js::mjit::JITScript *getJIT(bool constructing) {
-        return constructing ? jitCtor : jitNormal;
-    }
-
    size_t getUseCount() const  { return useCount; }
    size_t incUseCount() { return ++useCount; }
    size_t *addressOfUseCount() { return &useCount; }
@ -629,6 +683,7 @@ struct JSScript : public js::gc::Cell
    size_t sizeOfJitScripts(JSMallocSizeOfFun mallocSizeOf);
 #endif

+  public:
    js::PCCounts getPCCounts(jsbytecode *pc) {
        JS_ASSERT(size_t(pc - code) < length);
        return scriptCounts.pcCountsVector[pc - code];
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@ -143,13 +143,12 @@ mjit::Compiler::compile()
 {
    JS_ASSERT(!outerChunkRef().chunk);

-    void **checkAddr = isConstructing
-                       ? &outerScript->jitArityCheckCtor
-                       : &outerScript->jitArityCheckNormal;
-
    CompileStatus status = performCompilation();
    if (status != Compile_Okay && status != Compile_Retry) {
-        *checkAddr = JS_UNJITTABLE_SCRIPT;
+        JSScript::JITScriptHandle *jith = outerScript->jitHandle(isConstructing);
+        JSScript::ReleaseCode(cx->runtime->defaultFreeOp(), jith);
+        jith->setUnjittable();
+
        if (outerScript->function()) {
            outerScript->uninlineable = true;
            types::MarkTypeObjectFlags(cx, outerScript->function(),
@ -658,15 +657,13 @@ mjit::SetChunkLimit(uint32_t limit)
 }

 JITScript *
-MakeJITScript(JSContext *cx, JSScript *script, bool construct)
+MakeJITScript(JSContext *cx, JSScript *script)
 {
    if (!script->ensureRanAnalysis(cx, NULL))
        return NULL;

    ScriptAnalysis *analysis = script->analysis();

-    JITScript *&location = construct ? script->jitCtor : script->jitNormal;
-
    Vector<ChunkDescriptor> chunks(cx);
    Vector<CrossChunkEdge> edges(cx);

@ -895,10 +892,8 @@ MakeJITScript(JSContext *cx, JSScript *script, bool construct)
        }
    }

-    if (edges.empty()) {
-        location = jit;
+    if (edges.empty())
        return jit;
-    }

    jit->nedges = edges.length();
    CrossChunkEdge *jitEdges = jit->edges();
@ -937,7 +932,6 @@ MakeJITScript(JSContext *cx, JSScript *script, bool construct)
        edge.shimLabel = shimCode + (size_t) edge.shimLabel;
    }

-    location = jit;
    return jit;
 }

@ -949,12 +943,10 @@ mjit::CanMethodJIT(JSContext *cx, JSScript *script, jsbytecode *pc,
    if (!cx->methodJitEnabled)
        return Compile_Abort;

-    void *addr = construct ? script->jitArityCheckCtor : script->jitArityCheckNormal;
-    if (addr == JS_UNJITTABLE_SCRIPT)
+    JSScript::JITScriptHandle *jith = script->jitHandle(construct);
+    if (jith->isUnjittable())
        return Compile_Abort;

-    JITScript *jit = script->getJIT(construct);
-
    if (request == CompileRequest_Interpreter &&
        !cx->hasRunOption(JSOPTION_METHODJIT_ALWAYS) &&
        (cx->typeInferenceEnabled()
@ -971,10 +963,14 @@ mjit::CanMethodJIT(JSContext *cx, JSScript *script, jsbytecode *pc,
    if (construct && !script->nslots)
        script->nslots++;

-    if (!jit) {
-        jit = MakeJITScript(cx, script, construct);
+    JITScript *jit;
+    if (jith->isEmpty()) {
+        jit = MakeJITScript(cx, script);
        if (!jit)
            return Compile_Error;
+        jith->setValid(jit);
+    } else {
+        jit = jith->getValid();
    }
    unsigned chunkIndex = jit->chunkIndex(pc);
    ChunkDescriptor &desc = jit->chunkDescriptor(chunkIndex);
@ -1378,8 +1374,6 @@ mjit::Compiler::finishThisUp()
            jit->arityCheckEntry = stubCode.locationOf(arityLabel).executableAddress();
            jit->argsCheckEntry = stubCode.locationOf(argsCheckLabel).executableAddress();
            jit->fastEntry = fullCode.locationOf(invokeLabel).executableAddress();
-            void *&addr = isConstructing ? script->jitArityCheckCtor : script->jitArityCheckNormal;
-            addr = jit->arityCheckEntry;
        }
    }

--- a/js/src/methodjit/MethodJIT.cpp
+++ b/js/src/methodjit/MethodJIT.cpp
@ -1362,13 +1362,8 @@ JITScript::destroyChunk(FreeOp *fop, unsigned chunkIndex, bool resetUses)

        invokeEntry = NULL;
        fastEntry = NULL;
-        arityCheckEntry = NULL;
        argsCheckEntry = NULL;
-
-        if (script->jitNormal == this)
-            script->jitArityCheckNormal = NULL;
-        else
-            script->jitArityCheckCtor = NULL;
+        arityCheckEntry = NULL;

        // Fixup any ICs still referring to this chunk.
        while (!JS_CLIST_IS_EMPTY(&callers)) {
@ -1385,14 +1380,27 @@ JITScript::destroyChunk(FreeOp *fop, unsigned chunkIndex, bool resetUses)
    }
 }

+const js::mjit::JITScript *JSScript::JITScriptHandle::UNJITTABLE =
+    reinterpret_cast<js::mjit::JITScript *>(1);
+
+void
+JSScript::JITScriptHandle::staticAsserts()
+{
+    // JITScriptHandle's memory layout must match that of JITScript *.
+    JS_STATIC_ASSERT(sizeof(JSScript::JITScriptHandle) == sizeof(js::mjit::JITScript *));
+    JS_STATIC_ASSERT(JS_ALIGNMENT_OF(JSScript::JITScriptHandle) ==
+                     JS_ALIGNMENT_OF(js::mjit::JITScript *));
+    JS_STATIC_ASSERT(offsetof(JSScript::JITScriptHandle, value) == 0);
+}
+
 size_t
 JSScript::sizeOfJitScripts(JSMallocSizeOfFun mallocSizeOf)
 {
    size_t n = 0;
-    if (jitNormal)
-        n += jitNormal->sizeOfIncludingThis(mallocSizeOf); 
-    if (jitCtor)
-        n += jitCtor->sizeOfIncludingThis(mallocSizeOf); 
+    if (jitHandleNormal.isValid())
+        n += jitHandleNormal.getValid()->sizeOfIncludingThis(mallocSizeOf); 
+    if (jitHandleCtor.isValid())
+        n += jitHandleCtor.getValid()->sizeOfIncludingThis(mallocSizeOf); 
    return n;
 }

@ -1438,21 +1446,16 @@ mjit::JITChunk::sizeOfIncludingThis(JSMallocSizeOfFun mallocSizeOf)
 }

 void
-mjit::ReleaseScriptCode(FreeOp *fop, JSScript *script, bool construct)
+JSScript::ReleaseCode(FreeOp *fop, JITScriptHandle *jith)
 {
    // NB: The recompiler may call ReleaseScriptCode, in which case it
    // will get called again when the script is destroyed, so we
    // must protect against calling ReleaseScriptCode twice.

-    JITScript **pjit = construct ? &script->jitCtor : &script->jitNormal;
-    void **parity = construct ? &script->jitArityCheckCtor : &script->jitArityCheckNormal;
-
-    if (*pjit) {
-        (*pjit)->destroy(fop);
-        fop->free_(*pjit);
-        *pjit = NULL;
-        *parity = NULL;
-    }
+    JITScript *jit = jith->getValid();
+    jit->destroy(fop);
+    fop->free_(jit);
+    jith->setEmpty();
 }

 #ifdef JS_METHODJIT_PROFILE_STUBS
--- a/js/src/methodjit/MethodJIT.h
+++ b/js/src/methodjit/MethodJIT.h
@ -906,16 +906,13 @@ CompileStatus
 CanMethodJIT(JSContext *cx, JSScript *script, jsbytecode *pc,
             bool construct, CompileRequest request);

-void
-ReleaseScriptCode(FreeOp *fop, JSScript *script, bool construct);
-
 inline void
 ReleaseScriptCode(FreeOp *fop, JSScript *script)
 {
-    if (script->jitCtor)
-        mjit::ReleaseScriptCode(fop, script, true);
-    if (script->jitNormal)
-        mjit::ReleaseScriptCode(fop, script, false);
+    if (script->jitHandleCtor.isValid())
+        JSScript::ReleaseCode(fop, &script->jitHandleCtor);
+    if (script->jitHandleNormal.isValid())
+        JSScript::ReleaseCode(fop, &script->jitHandleNormal);
 }

 // Expand all stack frames inlined by the JIT within a compartment.
--- a/js/src/methodjit/MonoIC.cpp
+++ b/js/src/methodjit/MonoIC.cpp
@ -555,6 +555,9 @@ mjit::NativeStubEpilogue(VMFrame &f, Assembler &masm, NativeStubLinker::FinalJum
 * scripted native, then a small stub is generated which inlines the native
 * invocation.
 */
+namespace js {
+namespace mjit {
+
 class CallCompiler : public BaseCompiler
 {
    VMFrame &f;
@ -607,16 +610,20 @@ class CallCompiler : public BaseCompiler
        Address scriptAddr(ic.funObjReg, JSFunction::offsetOfNativeOrScript());
        masm.loadPtr(scriptAddr, t0);

-        /*
-         * Test if script->nmap is NULL - same as checking ncode, but faster
-         * here since ncode has two failure modes and we need to load out of
-         * nmap anyway.
-         */
-        size_t offset = callingNew
-                        ? offsetof(JSScript, jitArityCheckCtor)
-                        : offsetof(JSScript, jitArityCheckNormal);
+        // Test that:
+        // - script->jitHandle{Ctor,Normal}->value is neither NULL nor UNJITTABLE, and
+        // - script->jitHandle{Ctor,Normal}->value->arityCheckEntry is not NULL.
+        //
+        size_t offset = JSScript::jitHandleOffset(callingNew);
        masm.loadPtr(Address(t0, offset), t0);
-        Jump hasCode = masm.branchPtr(Assembler::Above, t0, ImmPtr(JS_UNJITTABLE_SCRIPT));
+        Jump hasNoJitCode = masm.branchPtr(Assembler::BelowOrEqual, t0,
+                                           ImmPtr(JSScript::JITScriptHandle::UNJITTABLE));
+
+        masm.loadPtr(Address(t0, offsetof(JITScript, arityCheckEntry)), t0);
+
+        Jump hasCode = masm.branchPtr(Assembler::NotEqual, t0, ImmPtr(0));
+
+        hasNoJitCode.linkTo(masm.label(), &masm);

        /*
         * Write the rejoin state to indicate this is a compilation call made
@ -1010,6 +1017,9 @@ class CallCompiler : public BaseCompiler
    }
 };

+} // namespace mjit
+} // namespace js
+
 void * JS_FASTCALL
 ic::Call(VMFrame &f, CallICInfo *ic)
 {
--- a/js/src/methodjit/Retcon.cpp
+++ b/js/src/methodjit/Retcon.cpp
@ -195,13 +195,13 @@ Recompiler::patchFrame(JSCompartment *compartment, VMFrame *f, JSScript *script)
            f->stubRejoin = 0;
        }
    } else {
-        if (script->jitCtor) {
-            JITChunk *chunk = script->jitCtor->findCodeChunk(*addr);
+        if (script->jitHandleCtor.isValid()) {
+            JITChunk *chunk = script->jitHandleCtor.getValid()->findCodeChunk(*addr);
            if (chunk)
                patchCall(chunk, fp, addr);
        }
-        if (script->jitNormal) {
-            JITChunk *chunk = script->jitNormal->findCodeChunk(*addr);
+        if (script->jitHandleNormal.isValid()) {
+            JITChunk *chunk = script->jitHandleNormal.getValid()->findCodeChunk(*addr);
            if (chunk)
                patchCall(chunk, fp, addr);
        }