Bug 685315 - Remove the GETGLOBAL opcode; r=dvander
authorTerrence Cole <terrence@trainedmonkeystudios.org>
Thu, 22 Sep 2011 17:35:25 +0100
changeset 77335 dfe8e0b734533c99f93cac902581f1e302b81c19
parent 77334 ee268ef7df396d3d61280d374f2169c724ac4f0b
child 77336 154c98ef39474d80d2c085e957fbcd934c6d5580
push id2067
push userbmo@edmorley.co.uk
push dateThu, 22 Sep 2011 16:36:05 +0000
treeherdermozilla-inbound@fb6480b0b03f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander
bugs685315
milestone9.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 685315 - Remove the GETGLOBAL opcode; r=dvander This was a nice and simple way to get a perf boost in the interpreter and JM, but JSOP_GETGNAME has the same information and more. TI doesn't need it, JM technically doesn't, and IM won't either. We can just do a normal property lookup during compilation.
js/src/jsemit.cpp
js/src/jsinfer.cpp
js/src/jsinterp.cpp
js/src/jsopcode.cpp
js/src/jsopcode.h
js/src/jsopcode.tbl
js/src/jsscript.h
js/src/jstracer.cpp
js/src/jsxdrapi.h
js/src/methodjit/Compiler.cpp
js/src/methodjit/Compiler.h
js/src/methodjit/InvokeHelpers.cpp
--- a/js/src/jsemit.cpp
+++ b/js/src/jsemit.cpp
@@ -2263,28 +2263,27 @@ BindNameToSlot(JSContext *cx, JSCodeGene
 
     if (dn->isGlobal()) {
         if (op == JSOP_NAME) {
             /*
              * If the definition is a defined global, not potentially aliased
              * by a local variable, and not mutating the variable, try and
              * optimize to a fast, unguarded global access.
              */
-            if (!BindKnownGlobal(cx, cg, dn, pn, atom))
-                return JS_FALSE;
             if (!pn->pn_cookie.isFree()) {
-                pn->setOp(JSOP_GETGLOBAL);
+                pn->setOp(JSOP_GETGNAME);
+                pn->pn_dflags |= PND_BOUND;
                 return JS_TRUE;
             }
         }
 
         /*
          * The locally stored cookie here should really come from |pn|, not
          * |dn|. For example, we could have a SETGNAME op's lexdef be a
-         * GETGLOBAL op, and their cookies have very different meanings. As
+         * GETGNAME op, and their cookies have very different meanings. As
          * a workaround, just make the cookie free.
          */
         cookie.makeFree();
     }
 
     if (cookie.isFree()) {
         StackFrame *caller = cg->parser->callerFrame;
         if (caller) {
@@ -2767,19 +2766,16 @@ EmitNameOp(JSContext *cx, JSCodeGenerato
     if (callContext) {
         switch (op) {
           case JSOP_NAME:
             op = JSOP_CALLNAME;
             break;
           case JSOP_GETGNAME:
             op = JSOP_CALLGNAME;
             break;
-          case JSOP_GETGLOBAL:
-            op = JSOP_CALLGLOBAL;
-            break;
           case JSOP_GETARG:
             op = JSOP_CALLARG;
             break;
           case JSOP_GETLOCAL:
             op = JSOP_CALLLOCAL;
             break;
           case JSOP_GETFCSLOT:
             op = JSOP_CALLFCSLOT;
@@ -4654,20 +4650,17 @@ EmitAssignment(JSContext *cx, JSCodeGene
                 }
             } else if (lhs->isOp(JSOP_SETNAME)) {
                 if (js_Emit1(cx, cg, JSOP_DUP) < 0)
                     return false;
                 EMIT_INDEX_OP(JSOP_GETXPROP, atomIndex);
             } else if (lhs->isOp(JSOP_SETGNAME)) {
                 if (!BindGlobal(cx, cg, lhs, lhs->pn_atom))
                     return false;
-                if (lhs->pn_cookie.isFree())
-                    EmitAtomOp(cx, lhs, JSOP_GETGNAME, cg);
-                else
-                    EMIT_UINT16_IMM_OP(JSOP_GETGLOBAL, lhs->pn_cookie.asInteger());
+                EmitAtomOp(cx, lhs, JSOP_GETGNAME, cg);
             } else {
                 EMIT_UINT16_IMM_OP(lhs->isOp(JSOP_SETARG) ? JSOP_GETARG : JSOP_GETLOCAL, atomIndex);
             }
             break;
           case TOK_DOT:
             if (js_Emit1(cx, cg, JSOP_DUP) < 0)
                 return false;
             if (lhs->pn_atom == cx->runtime->atomState.protoAtom) {
--- a/js/src/jsinfer.cpp
+++ b/js/src/jsinfer.cpp
@@ -1969,23 +1969,16 @@ TypeCompartment::newAllocationSiteTypeOb
 
 static inline jsid
 GetAtomId(JSContext *cx, JSScript *script, const jsbytecode *pc, unsigned offset)
 {
     unsigned index = js_GetIndexFromBytecode(cx, script, (jsbytecode*) pc, offset);
     return MakeTypeId(cx, ATOM_TO_JSID(script->getAtom(index)));
 }
 
-static inline jsid
-GetGlobalId(JSContext *cx, JSScript *script, const jsbytecode *pc)
-{
-    unsigned index = GET_SLOTNO(pc);
-    return MakeTypeId(cx, ATOM_TO_JSID(script->getGlobalAtom(index)));
-}
-
 static inline JSObject *
 GetScriptObject(JSContext *cx, JSScript *script, const jsbytecode *pc, unsigned offset)
 {
     unsigned index = js_GetIndexFromBytecode(cx, script, (jsbytecode*) pc, offset);
     return script->getObject(index);
 }
 
 static inline const Value &
@@ -3458,25 +3451,19 @@ ScriptAnalysis::analyzeTypesBytecode(JSC
         unsigned pickedDepth = (op == JSOP_SWAP ? 1 : pc[1]);
         /* The last popped value is the last pushed. */
         poppedTypes(pc, pickedDepth)->addSubset(cx, &pushed[pickedDepth]);
         for (unsigned i = 0; i < pickedDepth; i++)
             poppedTypes(pc, i)->addSubset(cx, &pushed[pickedDepth - 1 - i]);
         break;
       }
 
-      case JSOP_GETGLOBAL:
-      case JSOP_CALLGLOBAL:
       case JSOP_GETGNAME:
       case JSOP_CALLGNAME: {
-        jsid id;
-        if (op == JSOP_GETGLOBAL || op == JSOP_CALLGLOBAL)
-            id = GetGlobalId(cx, script, pc);
-        else
-            id = GetAtomId(cx, script, pc, 0);
+        jsid id = GetAtomId(cx, script, pc, 0);
 
         TypeSet *seen = bytecodeTypes(pc);
         seen->addSubset(cx, &pushed[0]);
 
         /*
          * Normally we rely on lazy standard class initialization to fill in
          * the types of global properties the script can access. In a few cases
          * the method JIT will bypass this, and we need to add the types direclty.
@@ -3486,17 +3473,17 @@ ScriptAnalysis::analyzeTypesBytecode(JSC
         if (id == ATOM_TO_JSID(cx->runtime->atomState.NaNAtom))
             seen->addType(cx, Type::DoubleType());
         if (id == ATOM_TO_JSID(cx->runtime->atomState.InfinityAtom))
             seen->addType(cx, Type::DoubleType());
 
         /* Handle as a property access. */
         PropertyAccess(cx, script, pc, script->global()->getType(cx), false, seen, id);
 
-        if (op == JSOP_CALLGLOBAL || op == JSOP_CALLGNAME) {
+        if (op == JSOP_CALLGNAME) {
             pushed[1].addType(cx, Type::UnknownType());
             pushed[0].addPropagateThis(cx, script, pc, Type::UnknownType());
         }
 
         if (CheckNextTest(pc))
             pushed[0].addType(cx, Type::UndefinedType());
         break;
       }
--- a/js/src/jsinterp.cpp
+++ b/js/src/jsinterp.cpp
@@ -4574,29 +4574,18 @@ BEGIN_CASE(JSOP_CALLFCSLOT)
     JS_ASSERT(index < obj->getFunctionPrivate()->script()->bindings.countUpvars());
     PUSH_COPY(obj->getFlatClosureUpvar(index));
     TypeScript::Monitor(cx, script, regs.pc, regs.sp[-1]);
     if (op == JSOP_CALLFCSLOT)
         PUSH_UNDEFINED();
 }
 END_CASE(JSOP_GETFCSLOT)
 
-BEGIN_CASE(JSOP_GETGLOBAL)
-BEGIN_CASE(JSOP_CALLGLOBAL)
-{
-    uint32 slot = GET_SLOTNO(regs.pc);
-    slot = script->getGlobalSlot(slot);
-    JSObject *obj = regs.fp()->scopeChain().getGlobal();
-    JS_ASSERT(obj->containsSlot(slot));
-    PUSH_COPY(obj->getSlot(slot));
-    TypeScript::Monitor(cx, script, regs.pc, regs.sp[-1]);
-    if (op == JSOP_CALLGLOBAL)
-        PUSH_UNDEFINED();
-}
-END_CASE(JSOP_GETGLOBAL)
+BEGIN_CASE(JSOP_UNUSED0)
+BEGIN_CASE(JSOP_UNUSED1)
 
 BEGIN_CASE(JSOP_DEFCONST)
 BEGIN_CASE(JSOP_DEFVAR)
 {
     uint32 index = GET_INDEX(regs.pc);
     JSAtom *atom = atoms[index];
 
     JSObject *obj = &regs.fp()->varObj();
--- a/js/src/jsopcode.cpp
+++ b/js/src/jsopcode.cpp
@@ -540,27 +540,16 @@ js_Disassemble1(JSContext *cx, JSScript 
         {
             JSAutoByteString bytes;
             if (!ToDisassemblySource(cx, v, &bytes))
                 return 0;
             Sprint(sp, " %s", bytes.ptr());
         }
         break;
 
-      case JOF_GLOBAL:
-        atom = script->getGlobalAtom(GET_SLOTNO(pc));
-        v = STRING_TO_JSVAL(atom);
-        {
-            JSAutoByteString bytes;
-            if (!ToDisassemblySource(cx, v, &bytes))
-                return 0;
-            Sprint(sp, " %s", bytes.ptr());
-        }
-        break;
-
       case JOF_UINT16PAIR:
         i = (jsint)GET_UINT16(pc);
         Sprint(sp, " %d", i);
         pc += UINT16_LEN;
         /* FALL THROUGH */
 
       case JOF_UINT16:
         i = (jsint)GET_UINT16(pc);
@@ -4011,21 +4000,16 @@ Decompile(SprintStack *ss, jsbytecode *p
                     todo = Sprint(&ss->sprinter, "%s[%d]", js_arguments_str, i);
                     break;
                 }
 #else
                 LOCAL_ASSERT(atom);
 #endif
                 goto do_name;
 
-              case JSOP_CALLGLOBAL:
-              case JSOP_GETGLOBAL:
-                atom = jp->script->getGlobalAtom(GET_SLOTNO(pc));
-                goto do_name;
-
               case JSOP_CALLNAME:
               case JSOP_NAME:
               case JSOP_GETGNAME:
               case JSOP_CALLGNAME:
                 LOAD_ATOM(0);
               do_name:
                 lval = "";
 #if JS_HAS_XML_SUPPORT
--- a/js/src/jsopcode.h
+++ b/js/src/jsopcode.h
@@ -91,17 +91,16 @@ typedef enum JSOp {
                                      atom index */
 #define JOF_INT32         14      /* int32 immediate operand */
 #define JOF_OBJECT        15      /* unsigned 16-bit object index */
 #define JOF_SLOTOBJECT    16      /* uint16 slot index + object index */
 #define JOF_REGEXP        17      /* unsigned 16-bit regexp index */
 #define JOF_INT8          18      /* int8 immediate operand */
 #define JOF_ATOMOBJECT    19      /* uint16 constant index + object index */
 #define JOF_UINT16PAIR    20      /* pair of uint16 immediates */
-#define JOF_GLOBAL        21      /* uint16 global array index */
 #define JOF_TYPEMASK      0x001f  /* mask for above immediate types */
 
 #define JOF_NAME          (1U<<5) /* name operation */
 #define JOF_PROP          (2U<<5) /* obj.prop operation */
 #define JOF_ELEM          (3U<<5) /* obj[index] operation */
 #define JOF_XMLNAME       (4U<<5) /* XML name: *, a::b, @a, @a::b, etc. */
 #define JOF_VARPROP       (5U<<5) /* x.prop for this, arg, var, or local x */
 #define JOF_MODEMASK      (7U<<5) /* mask for above addressing modes */
--- a/js/src/jsopcode.tbl
+++ b/js/src/jsopcode.tbl
@@ -270,18 +270,18 @@ OPDEF(JSOP_ARGDEC,   100, "argdec",     
 OPDEF(JSOP_INCLOCAL,  101,"inclocal",   NULL,         3,  0,  1, 15,  JOF_LOCAL|JOF_NAME|JOF_INC|JOF_TMPSLOT3)
 OPDEF(JSOP_DECLOCAL,  102,"declocal",   NULL,         3,  0,  1, 15,  JOF_LOCAL|JOF_NAME|JOF_DEC|JOF_TMPSLOT3)
 OPDEF(JSOP_LOCALINC,  103,"localinc",   NULL,         3,  0,  1, 15,  JOF_LOCAL|JOF_NAME|JOF_INC|JOF_POST|JOF_TMPSLOT3)
 OPDEF(JSOP_LOCALDEC,  104,"localdec",   NULL,         3,  0,  1, 15,  JOF_LOCAL|JOF_NAME|JOF_DEC|JOF_POST|JOF_TMPSLOT3)
 
 OPDEF(JSOP_IMACOP,    105,"imacop",     NULL,         1,  0,  0,  0,  JOF_BYTE)
 
 /* Static binding for globals. */
-OPDEF(JSOP_GETGLOBAL, 106,"getglobal",  NULL,         3,  0,  1, 19,  JOF_GLOBAL|JOF_NAME|JOF_TYPESET)
-OPDEF(JSOP_CALLGLOBAL,107,"callglobal", NULL,         3,  0,  2, 19,  JOF_GLOBAL|JOF_NAME|JOF_TYPESET|JOF_CALLOP)
+OPDEF(JSOP_UNUSED0,   106,"unused0",    NULL,         3,  0,  1, 19,  JOF_BYTE)
+OPDEF(JSOP_UNUSED1,   107,"unused1",    NULL,         3,  0,  2, 19,  JOF_BYTE)
 
 /* Like JSOP_FUNAPPLY but for f.call instead of f.apply. */
 OPDEF(JSOP_FUNCALL,   108,"funcall",    NULL,         3, -1,  1, 18,  JOF_UINT16|JOF_INVOKE|JOF_TYPESET)
 
 /* This opcode stores an index that is unique to the given loop. */
 OPDEF(JSOP_TRACE,     109,"trace",      NULL,         3,  0,  0,  0,  JOF_UINT16)
 
 /* ECMA-compliant assignment ops. */
--- a/js/src/jsscript.h
+++ b/js/src/jsscript.h
@@ -742,28 +742,16 @@ struct JSScript : public js::gc::Cell {
     }
 
     JSObject *getObject(size_t index) {
         JSObjectArray *arr = objects();
         JS_ASSERT(index < arr->length);
         return arr->vector[index];
     }
 
-    uint32 getGlobalSlot(size_t index) {
-        js::GlobalSlotArray *arr = globals();
-        JS_ASSERT(index < arr->length);
-        return arr->vector[index].slot;
-    }
-
-    JSAtom *getGlobalAtom(size_t index) {
-        js::GlobalSlotArray *arr = globals();
-        JS_ASSERT(index < arr->length);
-        return getAtom(arr->vector[index].atomIndex);
-    }
-
     JSVersion getVersion() const {
         return JSVersion(version);
     }
 
     inline JSFunction *getFunction(size_t index);
     inline JSFunction *getCallerFunction();
 
     inline JSObject *getRegExp(size_t index);
--- a/js/src/jstracer.cpp
+++ b/js/src/jstracer.cpp
@@ -16356,36 +16356,22 @@ TraceRecorder::record_JSOP_UNBRANDTHIS()
 
 JS_REQUIRES_STACK AbortableRecordingStatus
 TraceRecorder::record_JSOP_SHARPINIT()
 {
     return ARECORD_STOP;
 }
 
 JS_REQUIRES_STACK AbortableRecordingStatus
-TraceRecorder::record_JSOP_GETGLOBAL()
-{
-    uint32 slot = cx->fp()->script()->getGlobalSlot(GET_SLOTNO(cx->regs().pc));
-    if (!lazilyImportGlobalSlot(slot))
-         RETURN_STOP_A("lazy import of global slot failed");
-
-    stack(0, get(&globalObj->getSlotRef(slot)));
-    return ARECORD_CONTINUE;
-}
-
-JS_REQUIRES_STACK AbortableRecordingStatus
-TraceRecorder::record_JSOP_CALLGLOBAL()
-{
-    uint32 slot = cx->fp()->script()->getGlobalSlot(GET_SLOTNO(cx->regs().pc));
-    if (!lazilyImportGlobalSlot(slot))
-         RETURN_STOP_A("lazy import of global slot failed");
-
-    const Value &v = globalObj->getSlot(slot);
-    stack(0, get(&v));
-    stack(1, w.immiUndefined());
+TraceRecorder::record_JSOP_UNUSED0() {
+    return ARECORD_CONTINUE;
+}
+
+JS_REQUIRES_STACK AbortableRecordingStatus
+TraceRecorder::record_JSOP_UNUSED1() {
     return ARECORD_CONTINUE;
 }
 
 JS_REQUIRES_STACK AbortableRecordingStatus
 TraceRecorder::record_JSOP_GETGNAME()
 {
     return record_JSOP_NAME();
 }
--- a/js/src/jsxdrapi.h
+++ b/js/src/jsxdrapi.h
@@ -217,17 +217,17 @@ JS_XDRFindClassById(JSXDRState *xdr, uin
  * Bytecode version number. Increment the subtrahend whenever JS bytecode
  * changes incompatibly.
  *
  * This version number should be XDR'ed once near the front of any file or
  * larger storage unit containing XDR'ed bytecode and other data, and checked
  * before deserialization of bytecode.  If the saved version does not match
  * the current version, abort deserialization and invalidate the file.
  */
-#define JSXDR_BYTECODE_VERSION      (0xb973c0de - 94)
+#define JSXDR_BYTECODE_VERSION      (0xb973c0de - 95)
 
 /*
  * Library-private functions.
  */
 extern JSBool
 js_XDRAtom(JSXDRState *xdr, JSAtom **atomp);
 
 JS_END_EXTERN_C
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -2753,23 +2753,16 @@ mjit::Compiler::generateMethod()
             jsop_unbrand();
           END_CASE(JSOP_UNBRAND)
 
           BEGIN_CASE(JSOP_UNBRANDTHIS)
             prepareStubCall(Uses(1));
             INLINE_STUBCALL(stubs::UnbrandThis, REJOIN_FALLTHROUGH);
           END_CASE(JSOP_UNBRANDTHIS)
 
-          BEGIN_CASE(JSOP_GETGLOBAL)
-          BEGIN_CASE(JSOP_CALLGLOBAL)
-            jsop_getglobal(GET_SLOTNO(PC));
-            if (op == JSOP_CALLGLOBAL)
-                frame.push(UndefinedValue());
-          END_CASE(JSOP_GETGLOBAL)
-
           default:
            /* Sorry, this opcode isn't implemented yet. */
 #ifdef JS_METHODJIT_SPEW
             JaegerSpew(JSpew_Abort, "opcode %s not handled yet (%s line %d)\n", OpcodeNames[op],
                        script->filename, js_PCToLineNumber(cx, script, PC));
 #endif
             return Compile_Abort;
         }
@@ -2893,48 +2886,16 @@ mjit::Compiler::jumpInScript(Jump j, jsb
     if (pc < PC) {
         j.linkTo(a->jumpMap[uint32(pc - script->code)], &masm);
         return true;
     }
     return branchPatches.append(BranchPatch(j, pc, a->inlineIndex));
 }
 
 void
-mjit::Compiler::jsop_getglobal(uint32 index)
-{
-    JS_ASSERT(globalObj);
-    uint32 slot = script->getGlobalSlot(index);
-
-    JSObject *singleton = pushedSingleton(0);
-    if (singleton && !hasTypeBarriers(PC) && !globalObj->getSlot(slot).isUndefined()) {
-        frame.push(ObjectValue(*singleton));
-        return;
-    }
-
-    if (cx->typeInferenceEnabled() && globalObj->isGlobal() &&
-        !globalObj->getType(cx)->unknownProperties()) {
-        Value *value = &globalObj->getSlotRef(slot);
-        if (!value->isUndefined()) {
-            watchGlobalReallocation();
-            RegisterID reg = frame.allocReg();
-            masm.move(ImmPtr(value), reg);
-
-            BarrierState barrier = pushAddressMaybeBarrier(Address(reg), knownPushedType(0), true);
-            finishBarrier(barrier, REJOIN_GETTER, 0);
-            return;
-        }
-    }
-
-    RegisterID reg = frame.allocReg();
-    Address address = masm.objSlotRef(globalObj, reg, slot);
-    BarrierState barrier = pushAddressMaybeBarrier(address, knownPushedType(0), true);
-    finishBarrier(barrier, REJOIN_GETTER, 0);
-}
-
-void
 mjit::Compiler::emitFinalReturn(Assembler &masm)
 {
     masm.loadPtr(Address(JSFrameReg, StackFrame::offsetOfNcode()), Registers::ReturnReg);
     masm.jump(Registers::ReturnReg);
 }
 
 // Emits code to load a return value of the frame into the scripted-ABI
 // type & data register pair. If the return value is in fp->rval, then |fe|
--- a/js/src/methodjit/Compiler.h
+++ b/js/src/methodjit/Compiler.h
@@ -610,17 +610,16 @@ class Compiler : public BaseCompiler
     void truncateDoubleToInt32(FrameEntry *fe, Uses uses);
 
     /* Opcode handlers. */
     bool jumpAndTrace(Jump j, jsbytecode *target, Jump *slow = NULL, bool *trampoline = NULL);
     bool startLoop(jsbytecode *head, Jump entry, jsbytecode *entryTarget);
     bool finishLoop(jsbytecode *head);
     void jsop_bindname(JSAtom *atom, bool usePropCache);
     void jsop_setglobal(uint32 index);
-    void jsop_getglobal(uint32 index);
     void jsop_getprop_slow(JSAtom *atom, bool usePropCache = true);
     void jsop_getarg(uint32 slot);
     void jsop_setarg(uint32 slot, bool popped);
     void jsop_this();
     void emitReturn(FrameEntry *fe);
     void emitFinalReturn(Assembler &masm);
     void loadReturnValue(Assembler *masm, FrameEntry *fe);
     void emitReturnValue(Assembler *masm, FrameEntry *fe);
--- a/js/src/methodjit/InvokeHelpers.cpp
+++ b/js/src/methodjit/InvokeHelpers.cpp
@@ -1470,33 +1470,31 @@ js_InternalInterpret(void *returnData, v
       case REJOIN_GETTER:
         /*
          * Match the PC to figure out whether this property fetch is part of a
          * fused opcode which needs to be finished.
          */
         switch (op) {
           case JSOP_NAME:
           case JSOP_GETGNAME:
-          case JSOP_GETGLOBAL:
           case JSOP_GETFCSLOT:
           case JSOP_GETPROP:
           case JSOP_GETXPROP:
           case JSOP_LENGTH:
             /* Non-fused opcode, state is already correct for the next op. */
             f.regs.pc = nextpc;
             break;
 
           case JSOP_CALLGNAME:
           case JSOP_CALLNAME:
             if (!ComputeImplicitThis(cx, &fp->scopeChain(), nextsp[-2], &nextsp[-1]))
                 return js_InternalThrow(f);
             f.regs.pc = nextpc;
             break;
 
-          case JSOP_CALLGLOBAL:
           case JSOP_CALLFCSLOT:
             /* |this| is always undefined for CALLGLOBAL/CALLFCSLOT. */
             nextsp[-1].setUndefined();
             f.regs.pc = nextpc;
             break;
 
           case JSOP_CALLPROP: {
             /*