Bug 718134 - un-union StackFrame::args (r=bhackett)
authorLuke Wagner <luke@mozilla.com>
Tue, 17 Jan 2012 16:35:07 -0800
changeset 87206 7fe6463d454786c023235a4b1da4f66d45fae4c3
parent 87205 96fe039c46856b3dea40b169e8d1ba2956872781
child 87207 3b8dd350dbab3c8f6322920a2a50704a2ce04a13
push id674
push userffxbld
push dateTue, 13 Mar 2012 21:17:50 +0000
treeherdermozilla-beta@e3c4c92dec31 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs718134
milestone12.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 718134 - un-union StackFrame::args (r=bhackett)
js/src/jsanalyze.cpp
js/src/jsanalyze.h
js/src/jsinfer.cpp
js/src/methodjit/BaseAssembler.h
js/src/methodjit/Compiler.cpp
js/src/methodjit/FastOps.cpp
js/src/methodjit/LoopState.cpp
js/src/methodjit/PolyIC.cpp
js/src/vm/Stack-inl.h
js/src/vm/Stack.cpp
js/src/vm/Stack.h
--- a/js/src/jsanalyze.cpp
+++ b/js/src/jsanalyze.cpp
@@ -1528,17 +1528,17 @@ ScriptAnalysis::insertPhi(JSContext *cx,
 
     /*
      * Filter dupes inserted into small nodes to keep things clean and avoid
      * extra type constraints, but don't bother on large phi nodes to avoid
      * quadratic behavior.
      */
     if (node->length <= 8) {
         for (unsigned i = 0; i < node->length; i++) {
-            if (v.equals(node->options[i]))
+            if (v == node->options[i])
                 return;
         }
     }
 
     if (trackUseChain(v)) {
         SSAUseChain *&uses = useChain(v);
 
         SSAUseChain *use = cx->typeLifoAlloc().new_<SSAUseChain>();
@@ -1572,17 +1572,17 @@ ScriptAnalysis::insertPhi(JSContext *cx,
 }
 
 inline void
 ScriptAnalysis::mergeValue(JSContext *cx, uint32_t offset, const SSAValue &v, SlotValue *pv)
 {
     /* Make sure that v is accounted for in the pending value or phi value at pv. */
     JS_ASSERT(v.kind() != SSAValue::EMPTY && pv->value.kind() != SSAValue::EMPTY);
 
-    if (v.equals(pv->value))
+    if (v == pv->value)
         return;
 
     if (pv->value.kind() != SSAValue::PHI || pv->value.phiOffset() < offset) {
         SSAValue ov = pv->value;
         if (makePhi(cx, pv->slot, offset, &pv->value)) {
             insertPhi(cx, pv->value, v);
             insertPhi(cx, pv->value, ov);
         }
@@ -1702,17 +1702,17 @@ ScriptAnalysis::mergeExceptionTarget(JSC
      * try block starts and overwritten before it is finished can still be
      * seen at exception handlers via exception paths.
      */
     for (unsigned i = 0; i < exceptionTargets.length(); i++) {
         Vector<SlotValue> *pending = getCode(exceptionTargets[i]).pendingValues;
 
         bool duplicate = false;
         for (unsigned i = 0; i < pending->length(); i++) {
-            if ((*pending)[i].slot == slot && (*pending)[i].value.equals(value))
+            if ((*pending)[i].slot == slot && (*pending)[i].value == value)
                 duplicate = true;
         }
 
         if (!duplicate && !pending->append(SlotValue(slot, value)))
             setOOM(cx);
     }
 }
 
--- a/js/src/jsanalyze.h
+++ b/js/src/jsanalyze.h
@@ -585,17 +585,17 @@ class SSAValue
 
     Kind kind() const {
         JS_ASSERT(u.pushed.kind == u.var.kind && u.pushed.kind == u.phi.kind);
 
         /* Use a bitmask because MSVC wants to use -1 for PHI nodes. */
         return (Kind) (u.pushed.kind & 0x3);
     }
 
-    bool equals(const SSAValue &o) const {
+    bool operator==(const SSAValue &o) const {
         return !memcmp(this, &o, sizeof(SSAValue));
     }
 
     /* Accessors for values pushed by a bytecode within this script. */
 
     uint32_t pushedOffset() const {
         JS_ASSERT(kind() == PUSHED);
         return u.pushed.offset;
--- a/js/src/jsinfer.cpp
+++ b/js/src/jsinfer.cpp
@@ -4296,17 +4296,17 @@ ScriptAnalysis::followEscapingArguments(
     /*
      * trackUseChain is false for initial values of variables, which
      * cannot hold the script's arguments object.
      */
     if (!trackUseChain(v))
         return true;
 
     for (unsigned i = 0; i < seen->length(); i++) {
-        if (v.equals((*seen)[i]))
+        if (v == (*seen)[i])
             return true;
     }
     if (!seen->append(v)) {
         cx->compartment->types.setPendingNukeTypes(cx);
         return false;
     }
 
     SSAUseChain *use = useChain(v);
--- a/js/src/methodjit/BaseAssembler.h
+++ b/js/src/methodjit/BaseAssembler.h
@@ -826,17 +826,17 @@ static const JSC::MacroAssembler::Regist
         if (key.isConstant())
             key.index_ += delta;
         else
             add32(Imm32(delta), key.reg());
     }
 
     void loadFrameActuals(JSFunction *fun, RegisterID reg) {
         /* Bias for the case where there was an arguments overflow. */
-        load32(Address(JSFrameReg, StackFrame::offsetOfArgs()), reg);
+        load32(Address(JSFrameReg, StackFrame::offsetOfNumActual()), reg);
         add32(Imm32(fun->nargs + 2), reg);
         Jump overflowArgs = branchTest32(Assembler::NonZero,
                                          Address(JSFrameReg, StackFrame::offsetOfFlags()),
                                          Imm32(StackFrame::OVERFLOW_ARGS));
         move(Imm32(fun->nargs), reg);
         overflowArgs.linkTo(label(), this);
         lshiftPtr(Imm32(3), reg);
         negPtr(reg);
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -809,27 +809,27 @@ mjit::Compiler::generatePrologue()
                 stubcc.linkExitDirect(mismatch, stubcc.masm.label());
                 OOL_STUBCALL(stubs::FunctionFramePrologue, REJOIN_FUNCTION_PROLOGUE);
                 stubcc.crossJump(stubcc.masm.jump(), masm.label());
             }
         }
 
         if (outerScript->usesArguments && !script->function()->isHeavyweight()) {
             /*
-             * Make sure that fp->args.nactual is always coherent. This may be
+             * Make sure that fp->u.nactual is always coherent. This may be
              * inspected directly by JIT code, and is not guaranteed to be
              * correct if the UNDERFLOW and OVERFLOW flags are not set.
              */
             Jump hasArgs = masm.branchTest32(Assembler::NonZero, FrameFlagsAddress(),
                                              Imm32(StackFrame::OVERRIDE_ARGS |
                                                    StackFrame::UNDERFLOW_ARGS |
                                                    StackFrame::OVERFLOW_ARGS |
                                                    StackFrame::HAS_ARGS_OBJ));
             masm.storePtr(ImmPtr((void *)(size_t) script->function()->nargs),
-                          Address(JSFrameReg, StackFrame::offsetOfArgs()));
+                          Address(JSFrameReg, StackFrame::offsetOfNumActual()));
             hasArgs.linkTo(masm.label(), &masm);
         }
 
         j.linkTo(masm.label(), &masm);
     }
 
     if (cx->typeInferenceEnabled()) {
 #ifdef DEBUG
@@ -4522,22 +4522,21 @@ mjit::Compiler::jsop_getprop(PropertyNam
                 bumpPropCounter(PC, OpcodeCounts::PROP_DEFINITE);
             if (!isObject)
                 stubcc.rejoin(Changes(1));
             return true;
         }
 
         /*
          * Check if we are accessing the 'length' of the lazy arguments for the
-         * current frame. No actual arguments object has ever been constructed
-         * for the script, so we can go straight to nactual.
+         * current frame.
          */
         if (types->isLazyArguments(cx)) {
             frame.pop();
-            frame.pushWord(Address(JSFrameReg, StackFrame::offsetOfArgs()), JSVAL_TYPE_INT32);
+            frame.pushWord(Address(JSFrameReg, StackFrame::offsetOfNumActual()), JSVAL_TYPE_INT32);
             if (script->pcCounters)
                 bumpPropCounter(PC, OpcodeCounts::PROP_DEFINITE);
             return true;
         }
     }
 
     /* If the access will definitely be fetching a particular value, nop it. */
     bool testObject;
--- a/js/src/methodjit/FastOps.cpp
+++ b/js/src/methodjit/FastOps.cpp
@@ -1900,17 +1900,17 @@ mjit::Compiler::jsop_getelem_args()
     if (!key.isConstant())
         frame.pinReg(key.reg());
 
     RegisterID dataReg = frame.allocReg();
     RegisterID typeReg = frame.allocReg();
 
     // Guard on nactual.
     if (!hoistedLength) {
-        Address nactualAddr(JSFrameReg, StackFrame::offsetOfArgs());
+        Address nactualAddr(JSFrameReg, StackFrame::offsetOfNumActual());
         MaybeJump rangeGuard;
         if (key.isConstant()) {
             JS_ASSERT(key.index() >= 0);
             rangeGuard = masm.branch32(Assembler::BelowOrEqual, nactualAddr, Imm32(key.index()));
         } else {
             rangeGuard = masm.branch32(Assembler::BelowOrEqual, nactualAddr, key.reg());
         }
         stubcc.linkExit(rangeGuard.get(), Uses(2));
--- a/js/src/methodjit/LoopState.cpp
+++ b/js/src/methodjit/LoopState.cpp
@@ -1431,17 +1431,17 @@ LoopState::restoreInvariants(jsbytecode 
             Address address = frame.addressOf(frame.getTemporary(entry.u.array.temporary));
             masm.loadFrameActuals(outerScript->function(), T0);
             masm.storePayload(T0, address);
             break;
           }
 
           case InvariantEntry::INVARIANT_ARGS_LENGTH: {
             Address address = frame.addressOf(frame.getTemporary(entry.u.array.temporary));
-            masm.load32(Address(JSFrameReg, StackFrame::offsetOfArgs()), T0);
+            masm.load32(Address(JSFrameReg, StackFrame::offsetOfNumActual()), T0);
             masm.storeValueFromComponents(ImmType(JSVAL_TYPE_INT32), T0, address);
             break;
           }
 
           case InvariantEntry::INVARIANT_PROPERTY: {
             uint32_t object = entry.u.property.objectSlot;
             Jump notObject = masm.testObject(Assembler::NotEqual, frame.addressOf(object));
             jumps->append(notObject);
--- a/js/src/methodjit/PolyIC.cpp
+++ b/js/src/methodjit/PolyIC.cpp
@@ -2434,17 +2434,17 @@ GetElementIC::attachArguments(VMFrame &f
         masm.loadValueAsComponents(frameEntry, typeReg, objReg);
     }    
     Jump done2 = masm.jump();
 
     notFormalArg.linkTo(masm.label(), &masm);
 
     masm.push(typeReg);
 
-    Address argsObject(typeReg, StackFrame::offsetOfArgs());
+    Address argsObject(typeReg, StackFrame::offsetOfArgsObj());
     masm.loadPtr(argsObject, typeReg);
 
     masm.load32(Address(typeReg, JSObject::getFixedSlotOffset(ArgumentsObject::INITIAL_LENGTH_SLOT)), 
                 typeReg); 
     masm.rshift32(Imm32(ArgumentsObject::PACKED_BITS_COUNT), typeReg); 
 
     /* This basically does fp - (numFormalArgs + numActualArgs + 2) */
 
--- a/js/src/vm/Stack-inl.h
+++ b/js/src/vm/Stack-inl.h
@@ -152,17 +152,17 @@ StackFrame::initCallFrame(JSContext *cx,
                             LOWERED_CALL_APPLY |
                             OVERFLOW_ARGS |
                             UNDERFLOW_ARGS)) == 0);
     JS_ASSERT(script == callee.script());
 
     /* Initialize stack frame members. */
     flags_ = FUNCTION | HAS_PREVPC | HAS_SCOPECHAIN | HAS_BLOCKCHAIN | flagsArg;
     exec.fun = &callee;
-    args.nactual = nactual;
+    u.nactual = nactual;
     scopeChain_ = callee.environment();
     ncode_ = NULL;
     initPrev(cx);
     blockChain_= NULL;
     JS_ASSERT(!hasBlockChain());
     JS_ASSERT(!hasHookData());
     JS_ASSERT(annotation() == NULL);
     JS_ASSERT(!hasCallObj());
@@ -218,17 +218,17 @@ StackFrame::initJitFrameCallerHalf(Stack
 /*
  * The "early prologue" refers to either the fast path or arity check path up
  * to the "late prologue".
  */
 inline void
 StackFrame::initJitFrameEarlyPrologue(JSFunction *fun, uint32_t nactual)
 {
     exec.fun = fun;
-    args.nactual = nactual;
+    u.nactual = nactual;
 }
 
 /*
  * The "late prologue" (in generatePrologue) extends from the join point of the
  * fast path and arity check to where the call object is (possibly) created.
  */
 inline bool
 StackFrame::initJitFrameLatePrologue(JSContext *cx, Value **limit)
@@ -321,56 +321,54 @@ struct CopyTo
         return true;
     }
 };
 
 inline uintN
 StackFrame::numActualArgs() const
 {
     /*
-     * args.nactual is always coherent, except for method JIT frames where the
+     * u.nactual is always coherent, except for method JIT frames where the
      * callee does not access its arguments and the number of actual arguments
      * matches the number of formal arguments. The JIT requires that all frames
      * which do not have an arguments object and use their arguments have a
-     * coherent args.nactual (even though the below code may not use it), as
+     * coherent u.nactual (even though the below code may not use it), as
      * JIT code may access the field directly.
      */
     JS_ASSERT(hasArgs());
     if (JS_UNLIKELY(flags_ & (OVERFLOW_ARGS | UNDERFLOW_ARGS)))
-        return hasArgsObj() ? argsObj().initialLength() : args.nactual;
+        return u.nactual;
     return numFormalArgs();
 }
 
 inline Value *
 StackFrame::actualArgs() const
 {
     JS_ASSERT(hasArgs());
     Value *argv = formalArgs();
-    if (JS_UNLIKELY(flags_ & OVERFLOW_ARGS)) {
-        uintN nactual = hasArgsObj() ? argsObj().initialLength() : args.nactual;
-        return argv - (2 + nactual);
-    }
+    if (JS_UNLIKELY(flags_ & OVERFLOW_ARGS))
+        return argv - (2 + u.nactual);
     return argv;
 }
 
 inline Value *
 StackFrame::actualArgsEnd() const
 {
     JS_ASSERT(hasArgs());
     if (JS_UNLIKELY(flags_ & OVERFLOW_ARGS))
         return formalArgs() - 2;
     return formalArgs() + numActualArgs();
 }
 
 inline void
 StackFrame::setArgsObj(ArgumentsObject &obj)
 {
-    JS_ASSERT_IF(hasArgsObj(), &obj == args.obj);
+    JS_ASSERT_IF(hasArgsObj(), &obj == argsObj_);
     JS_ASSERT_IF(!hasArgsObj(), numActualArgs() == obj.initialLength());
-    args.obj = &obj;
+    argsObj_ = &obj;
     flags_ |= HAS_ARGS_OBJ;
 }
 
 inline void
 StackFrame::setScopeChainNoCallObj(JSObject &obj)
 {
 #ifdef DEBUG
     JS_ASSERT(&obj != NULL);
@@ -459,20 +457,18 @@ StackFrame::functionEpilogue()
     if (maintainNestingState())
         types::NestingEpilogue(this);
 }
 
 inline void
 StackFrame::updateEpilogueFlags()
 {
     if (flags_ & (HAS_ARGS_OBJ | HAS_CALL_OBJ)) {
-        if (hasArgsObj() && !argsObj().maybeStackFrame()) {
-            args.nactual = args.obj->initialLength();
+        if (hasArgsObj() && !argsObj().maybeStackFrame())
             flags_ &= ~HAS_ARGS_OBJ;
-        }
         if (hasCallObj() && !callObj().maybeStackFrame()) {
             /*
              * For function frames, the call object may or may not have have an
              * enclosing DeclEnv object, so we use the callee's parent, since
              * it was the initial scope chain. For global (strict) eval frames,
              * there is no callee, but the call object's parent is the initial
              * scope chain.
              */
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -83,23 +83,23 @@ StackFrame::initExecuteFrame(JSScript *s
         flags_ |= (prev->flags_ & (FUNCTION | GLOBAL));
 
     Value *dstvp = (Value *)this - 2;
     dstvp[1] = thisv;
 
     if (isFunctionFrame()) {
         dstvp[0] = prev->calleev();
         exec = prev->exec;
-        args.script = script;
+        u.evalScript = script;
     } else {
         JS_ASSERT(isGlobalFrame());
         dstvp[0] = NullValue();
         exec.script = script;
 #ifdef DEBUG
-        args.script = (JSScript *)0xbad;
+        u.evalScript = (JSScript *)0xbad;
 #endif
     }
 
     scopeChain_ = &scopeChain;
     prev_ = prev;
     prevpc_ = regs ? regs->pc : (jsbytecode *)0xbad;
     prevInline_ = regs ? regs->inlined() : NULL;
     blockChain_ = NULL;
@@ -229,36 +229,17 @@ StackSegment::contains(const FrameRegs *
 bool
 StackSegment::contains(const CallArgsList *call) const
 {
     if (!call || !calls_)
         return false;
 
     /* NB: this depends on the continuity of segments in memory. */
     Value *vp = call->array();
-    bool ret = vp > slotsBegin() && vp <= calls_->array();
-
-    /*
-     * :XXX: Disabled. Including this check changes the asymptotic complexity
-     * of code which calls this function.
-     */
-#if 0
-#ifdef DEBUG
-    bool found = false;
-    for (CallArgsList *c = maybeCalls(); c->argv() > slotsBegin(); c = c->prev()) {
-        if (c == call) {
-            found = true;
-            break;
-        }
-    }
-    JS_ASSERT(found == ret);
-#endif
-#endif
-
-    return ret;
+    return vp > slotsBegin() && vp <= calls_->array();
 }
 
 StackFrame *
 StackSegment::computeNextFrame(const StackFrame *f) const
 {
     JS_ASSERT(contains(f) && f != fp());
 
     StackFrame *next = fp();
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -355,27 +355,27 @@ class StackFrame
 
   private:
     mutable uint32_t    flags_;         /* bits described by Flags */
     union {                             /* describes what code is executing in a */
         JSScript        *script;        /*   global frame */
         JSFunction      *fun;           /*   function frame, pre GetScopeChain */
     } exec;
     union {                             /* describes the arguments of a function */
-        uintN           nactual;        /*   before js_GetArgsObject */
-        ArgumentsObject *obj;           /*   after js_GetArgsObject */
-        JSScript        *script;        /* eval has no args, but needs a script */
-    } args;
+        uintN           nactual;        /*   for non-eval frames */
+        JSScript        *evalScript;    /*   the script of an eval-in-function */
+    } u;
     mutable JSObject    *scopeChain_;   /* current scope chain */
     StackFrame          *prev_;         /* previous cx->regs->fp */
     void                *ncode_;        /* return address for method JIT */
 
     /* Lazily initialized */
     Value               rval_;          /* return value of the frame */
     StaticBlockObject   *blockChain_;   /* innermost let block */
+    ArgumentsObject     *argsObj_;      /* if has HAS_ARGS_OBJ */
     jsbytecode          *prevpc_;       /* pc of previous frame*/
     JSInlinedSite       *prevInline_;   /* inlined site in previous frame */
     void                *hookData_;     /* closure returned by call hook */
     void                *annotation_;   /* perhaps remove with bug 546848 */
     JSRejoinState       rejoin_;        /* If rejoining into the interpreter
                                          * from JIT code, state at rejoin. */
 
     static void staticAsserts() {
@@ -573,23 +573,23 @@ class StackFrame
     JSInlinedSite *prevInline() {
         JS_ASSERT(flags_ & HAS_PREVPC);
         return prevInline_;
     }
 
     JSScript *script() const {
         JS_ASSERT(isScriptFrame());
         return isFunctionFrame()
-               ? isEvalFrame() ? args.script : fun()->script()
+               ? isEvalFrame() ? u.evalScript : fun()->script()
                : exec.script;
     }
 
     JSScript *functionScript() const {
         JS_ASSERT(isFunctionFrame());
-        return isEvalFrame() ? args.script : fun()->script();
+        return isEvalFrame() ? u.evalScript : fun()->script();
     }
 
     JSScript *globalScript() const {
         JS_ASSERT(isGlobalFrame());
         return exec.script;
     }
 
     JSScript *maybeScript() const {
@@ -697,17 +697,17 @@ class StackFrame
 
     bool hasArgsObj() const {
         return !!(flags_ & HAS_ARGS_OBJ);
     }
 
     ArgumentsObject &argsObj() const {
         JS_ASSERT(hasArgsObj());
         JS_ASSERT(!isEvalFrame());
-        return *args.obj;
+        return *argsObj_;
     }
 
     ArgumentsObject *maybeArgsObj() const {
         return hasArgsObj() ? &argsObj() : NULL;
     }
 
     inline void setArgsObj(ArgumentsObject &obj);
 
@@ -1113,41 +1113,36 @@ class StackFrame
     static size_t offsetOfFlags() {
         return offsetof(StackFrame, flags_);
     }
 
     static size_t offsetOfExec() {
         return offsetof(StackFrame, exec);
     }
 
-    static size_t offsetOfArgs() {
-        return offsetof(StackFrame, args);
-    }    
-
-    void *addressOfArgs() {
-        return &args;
+    static size_t offsetOfNumActual() {
+        return offsetof(StackFrame, u.nactual);
     }
 
     static size_t offsetOfScopeChain() {
         return offsetof(StackFrame, scopeChain_);
     }
 
-    JSObject **addressOfScopeChain() {
-        JS_ASSERT(flags_ & HAS_SCOPECHAIN);
-        return &scopeChain_;
-    }
-
     static size_t offsetOfPrev() {
         return offsetof(StackFrame, prev_);
     }
 
     static size_t offsetOfReturnValue() {
         return offsetof(StackFrame, rval_);
     }
 
+    static size_t offsetOfArgsObj() {
+        return offsetof(StackFrame, argsObj_);
+    }
+
     static ptrdiff_t offsetOfNcode() {
         return offsetof(StackFrame, ncode_);
     }
 
     static ptrdiff_t offsetOfCallee(JSFunction *fun) {
         JS_ASSERT(fun != NULL);
         return -(fun->nargs + 2) * sizeof(Value);
     }