From 7ca3f3c999e37917348403f1fec25f6d934726f1 Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Tue, 21 May 2013 16:09:01 +0200 Subject: [PATCH] Bug 873155 - Remove StackFrame argument duplication. r=luke --- js/src/ion/BaselineFrame.cpp | 5 +- js/src/ion/BaselineFrame.h | 9 +-- js/src/ion/BaselineIC.cpp | 4 +- js/src/ion/BaselineJIT.cpp | 26 +-------- js/src/ion/Ion.cpp | 26 +-------- js/src/vm/ArgumentsObject.cpp | 20 ++----- js/src/vm/Stack-inl.h | 102 +++++++--------------------------- js/src/vm/Stack.cpp | 2 +- js/src/vm/Stack.h | 68 +++++++++-------------- 9 files changed, 62 insertions(+), 200 deletions(-) diff --git a/js/src/ion/BaselineFrame.cpp b/js/src/ion/BaselineFrame.cpp index 85c207c60e3..c0fa105e974 100644 --- a/js/src/ion/BaselineFrame.cpp +++ b/js/src/ion/BaselineFrame.cpp @@ -27,8 +27,7 @@ BaselineFrame::trace(JSTracer *trc) // Mark actual and formal args. if (isNonEvalFunctionFrame()) { unsigned numArgs = js::Max(numActualArgs(), numFormalArgs()); - JS_ASSERT(actuals() == formals()); - gc::MarkValueRootRange(trc, numArgs, actuals(), "baseline-args"); + gc::MarkValueRootRange(trc, numArgs, argv(), "baseline-args"); } // Mark scope chain. @@ -62,7 +61,7 @@ BaselineFrame::copyRawFrameSlots(AutoValueVector *vec) const if (!vec->resize(nformals + nfixed)) return false; - mozilla::PodCopy(vec->begin(), formals(), nformals); + mozilla::PodCopy(vec->begin(), argv(), nformals); for (unsigned i = 0; i < nfixed; i++) (*vec)[nformals + i] = *valueSlot(i); return true; diff --git a/js/src/ion/BaselineFrame.h b/js/src/ion/BaselineFrame.h index 85fdb925e54..3a10069331d 100644 --- a/js/src/ion/BaselineFrame.h +++ b/js/src/ion/BaselineFrame.h @@ -158,14 +158,14 @@ class BaselineFrame JS_ASSERT(i < numFormalArgs()); JS_ASSERT_IF(checkAliasing, !script()->argsObjAliasesFormals()); JS_ASSERT_IF(checkAliasing, !script()->formalIsAliased(i)); - return formals()[i]; + return argv()[i]; } Value &unaliasedActual(unsigned i, MaybeCheckAliasing checkAliasing) const { JS_ASSERT(i < numActualArgs()); JS_ASSERT_IF(checkAliasing, !script()->argsObjAliasesFormals()); JS_ASSERT_IF(checkAliasing && i < numFormalArgs(), !script()->formalIsAliased(i)); - return actuals()[i]; + return argv()[i]; } Value &unaliasedLocal(unsigned i, MaybeCheckAliasing checkAliasing = CHECK_ALIASING) const { @@ -188,14 +188,11 @@ class BaselineFrame BaselineFrame::Size() + offsetOfThis()); } - Value *formals() const { + Value *argv() const { return (Value *)(reinterpret_cast(this) + BaselineFrame::Size() + offsetOfArg(0)); } - Value *actuals() const { - return formals(); - } bool copyRawFrameSlots(AutoValueVector *vec) const; diff --git a/js/src/ion/BaselineIC.cpp b/js/src/ion/BaselineIC.cpp index 60dcb084585..16edc74233d 100644 --- a/js/src/ion/BaselineIC.cpp +++ b/js/src/ion/BaselineIC.cpp @@ -785,7 +785,7 @@ PrepareOsrTempData(JSContext *cx, ICUseCount_Fallback *stub, BaselineFrame *fram // // Copy formal args and thisv. - memcpy(stackFrameStart, frame->formals() - 1, (numFormalArgs + 1) * sizeof(Value)); + memcpy(stackFrameStart, frame->argv() - 1, (numFormalArgs + 1) * sizeof(Value)); // Initialize ScopeChain, Exec, and Flags fields in StackFrame struct. uint8_t *stackFrame = info->stackFrame; @@ -8019,7 +8019,7 @@ DoCreateRestParameter(JSContext *cx, BaselineFrame *frame, ICRest_Fallback *stub unsigned numActuals = frame->numActualArgs(); unsigned numRest = numActuals > numFormals ? numActuals - numFormals : 0; - JSObject *obj = NewDenseCopiedArray(cx, numRest, frame->actuals() + numFormals, NULL); + JSObject *obj = NewDenseCopiedArray(cx, numRest, frame->argv() + numFormals, NULL); if (!obj) return false; obj->setType(type); diff --git a/js/src/ion/BaselineJIT.cpp b/js/src/ion/BaselineJIT.cpp index b475cedc03e..a729741e613 100644 --- a/js/src/ion/BaselineJIT.cpp +++ b/js/src/ion/BaselineJIT.cpp @@ -89,34 +89,14 @@ EnterBaseline(JSContext *cx, StackFrame *fp, void *jitcode, bool osr) // arguments and the number of formal arguments. It accounts for |this|. int maxArgc = 0; Value *maxArgv = NULL; - int numActualArgs = 0; + unsigned numActualArgs = 0; RootedValue thisv(cx); void *calleeToken; if (fp->isNonEvalFunctionFrame()) { - // CountArgSlot include |this| and the |scopeChain|, and maybe |argumentsObj| - // Want to keep including this, but remove the scopeChain and any argumentsObj. - maxArgc = CountArgSlots(fp->script(), fp->fun()) - StartArgSlot(fp->script(), fp->fun()); - maxArgv = fp->formals() - 1; // -1 = include |this| - - // Formal arguments are the argument corresponding to the function - // definition and actual arguments are corresponding to the call-site - // arguments. numActualArgs = fp->numActualArgs(); - - // We do not need to handle underflow because formal arguments are pad - // with |undefined| values but we need to distinguish between the - if (fp->hasOverflowArgs()) { - int formalArgc = maxArgc; - Value *formalArgv = maxArgv; - maxArgc = numActualArgs + 1; // +1 = include |this| - maxArgv = fp->actuals() - 1; // -1 = include |this| - - // The beginning of the actual args is not updated, so we just copy - // the formal args into the actual args to get a linear vector which - // can be copied by generateEnterJit. - memcpy(maxArgv, formalArgv, formalArgc * sizeof(Value)); - } + maxArgc = Max(numActualArgs, fp->numFormalArgs()) + 1; // +1 = include |this| + maxArgv = fp->argv() - 1; // -1 = include |this| calleeToken = CalleeToToken(&fp->callee()); } else { // For eval function frames, set the callee token to the enclosing function. diff --git a/js/src/ion/Ion.cpp b/js/src/ion/Ion.cpp index fed1dd07793..68ef551e9ad 100644 --- a/js/src/ion/Ion.cpp +++ b/js/src/ion/Ion.cpp @@ -1951,34 +1951,14 @@ EnterIon(JSContext *cx, StackFrame *fp, void *jitcode) // arguments and the number of formal arguments. It accounts for |this|. int maxArgc = 0; Value *maxArgv = NULL; - int numActualArgs = 0; + unsigned numActualArgs = 0; RootedValue thisv(cx); void *calleeToken; if (fp->isFunctionFrame()) { - // CountArgSlot include |this| and the |scopeChain| and maybe |argumentsObj|. - // Keep |this|, but discard the others. - maxArgc = CountArgSlots(fp->script(), fp->fun()) - StartArgSlot(fp->script(), fp->fun()); - maxArgv = fp->formals() - 1; // -1 = include |this| - - // Formal arguments are the argument corresponding to the function - // definition and actual arguments are corresponding to the call-site - // arguments. numActualArgs = fp->numActualArgs(); - - // We do not need to handle underflow because formal arguments are pad - // with |undefined| values but we need to distinguish between the - if (fp->hasOverflowArgs()) { - int formalArgc = maxArgc; - Value *formalArgv = maxArgv; - maxArgc = numActualArgs + 1; // +1 = include |this| - maxArgv = fp->actuals() - 1; // -1 = include |this| - - // The beginning of the actual args is not updated, so we just copy - // the formal args into the actual args to get a linear vector which - // can be copied by generateEnterJit. - memcpy(maxArgv, formalArgv, formalArgc * sizeof(Value)); - } + maxArgc = Max(numActualArgs, fp->numFormalArgs()) + 1; // +1 = include |this| + maxArgv = fp->argv() - 1; // -1 = include |this| calleeToken = CalleeToToken(&fp->callee()); } else { calleeToken = CalleeToToken(fp->script()); diff --git a/js/src/vm/ArgumentsObject.cpp b/js/src/vm/ArgumentsObject.cpp index f3372aad646..2500d953f14 100644 --- a/js/src/vm/ArgumentsObject.cpp +++ b/js/src/vm/ArgumentsObject.cpp @@ -30,25 +30,13 @@ CopyStackFrameArguments(const AbstractFramePtr frame, HeapValue *dst, unsigned t { JS_ASSERT_IF(frame.isStackFrame(), !frame.asStackFrame()->runningInIon()); - unsigned numActuals = frame.numActualArgs(); - unsigned numFormals = frame.callee()->nargs; - JS_ASSERT(numActuals <= totalArgs); - JS_ASSERT(numFormals <= totalArgs); - JS_ASSERT(Max(numActuals, numFormals) == totalArgs); + JS_ASSERT(Max(frame.numActualArgs(), frame.numFormalArgs()) == totalArgs); - /* Copy formal arguments. */ - Value *src = frame.formals(); - Value *end = src + numFormals; + /* Copy arguments. */ + Value *src = frame.argv(); + Value *end = src + totalArgs; while (src != end) (dst++)->init(*src++); - - /* Copy actual argument which are not contignous. */ - if (numFormals < numActuals) { - src = frame.actuals() + numFormals; - end = src + (numActuals - numFormals); - while (src != end) - (dst++)->init(*src++); - } } /* static */ void diff --git a/js/src/vm/Stack-inl.h b/js/src/vm/Stack-inl.h index 384e5ec159a..ff024bcc9e8 100644 --- a/js/src/vm/Stack-inl.h +++ b/js/src/vm/Stack-inl.h @@ -99,9 +99,7 @@ inline void StackFrame::initCallFrame(JSContext *cx, JSFunction &callee, JSScript *script, uint32_t nactual, StackFrame::Flags flagsArg) { - JS_ASSERT((flagsArg & ~(CONSTRUCTING | - OVERFLOW_ARGS | - UNDERFLOW_ARGS)) == 0); + JS_ASSERT((flagsArg & ~CONSTRUCTING) == 0); JS_ASSERT(callee.nonLazyScript() == script); /* Initialize stack frame members. */ @@ -129,7 +127,7 @@ StackFrame::createRestParameter(JSContext *cx) JS_ASSERT(fun()->hasRest()); unsigned nformal = fun()->nargs - 1, nactual = numActualArgs(); unsigned nrest = (nactual > nformal) ? nactual - nformal : 0; - return NewDenseCopiedArray(cx, nrest, actuals() + nformal, NULL); + return NewDenseCopiedArray(cx, nrest, argv() + nformal, NULL); } inline Value & @@ -155,7 +153,7 @@ StackFrame::unaliasedFormal(unsigned i, MaybeCheckAliasing checkAliasing) JS_ASSERT(i < numFormalArgs()); JS_ASSERT_IF(checkAliasing, !script()->argsObjAliasesFormals()); JS_ASSERT_IF(checkAliasing, !script()->formalIsAliased(i)); - return formals()[i]; + return argv()[i]; } inline Value & @@ -164,7 +162,7 @@ StackFrame::unaliasedActual(unsigned i, MaybeCheckAliasing checkAliasing) JS_ASSERT(i < numActualArgs()); JS_ASSERT_IF(checkAliasing, !script()->argsObjAliasesFormals()); JS_ASSERT_IF(checkAliasing && i < numFormalArgs(), !script()->formalIsAliased(i)); - return i < numFormalArgs() ? formals()[i] : actuals()[i]; + return argv()[i]; } template @@ -174,25 +172,9 @@ StackFrame::forEachUnaliasedActual(Op op) JS_ASSERT(!script()->funHasAnyAliasedFormal); JS_ASSERT(!script()->needsArgsObj()); - unsigned nformal = numFormalArgs(); - unsigned nactual = numActualArgs(); - - const Value *formalsEnd = (const Value *)this; - const Value *formals = formalsEnd - nformal; - - if (nactual <= nformal) { - const Value *actualsEnd = formals + nactual; - for (const Value *p = formals; p < actualsEnd; ++p) - op(*p); - } else { - for (const Value *p = formals; p < formalsEnd; ++p) - op(*p); - - const Value *actualsEnd = formals - 2; - const Value *actuals = actualsEnd - nactual; - for (const Value *p = actuals + nformal; p < actualsEnd; ++p) - op(*p); - } + const Value *argsEnd = argv() + numActualArgs(); + for (const Value *p = argv(); p < argsEnd; ++p) + op(*p); } struct CopyTo @@ -209,30 +191,6 @@ struct CopyToHeap void operator()(const Value &src) { dst->init(src); ++dst; } }; -inline unsigned -StackFrame::numFormalArgs() const -{ - JS_ASSERT(hasArgs()); - return fun()->nargs; -} - -inline unsigned -StackFrame::numActualArgs() const -{ - /* - * 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 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 u.nactual; - return numFormalArgs(); -} - inline ArgumentsObject & StackFrame::argsObj() const { @@ -315,31 +273,19 @@ ContextStack::getCallFrame(JSContext *cx, MaybeReportError report, const CallArg unsigned nvals = VALUES_PER_STACK_FRAME + script->nslots; - /* Maintain layout invariant: &formals[0] == ((Value *)fp) - nformal. */ - - if (args.length() == nformal) { + if (args.length() >= nformal) { if (!space().ensureSpace(cx, report, firstUnused, nvals)) return NULL; return reinterpret_cast(firstUnused); } - if (args.length() < nformal) { - *flags = StackFrame::Flags(*flags | StackFrame::UNDERFLOW_ARGS); - unsigned nmissing = nformal - args.length(); - if (!space().ensureSpace(cx, report, firstUnused, nmissing + nvals)) - return NULL; - SetValueRangeToUndefined(firstUnused, nmissing); - return reinterpret_cast(firstUnused + nmissing); - } - - *flags = StackFrame::Flags(*flags | StackFrame::OVERFLOW_ARGS); - unsigned ncopy = 2 + nformal; - if (!space().ensureSpace(cx, report, firstUnused, ncopy + nvals)) + /* Pad any missing arguments with |undefined|. */ + JS_ASSERT(args.length() < nformal); + unsigned nmissing = nformal - args.length(); + if (!space().ensureSpace(cx, report, firstUnused, nmissing + nvals)) return NULL; - Value *dst = firstUnused; - Value *src = args.base(); - mozilla::PodCopy(dst, src, ncopy); - return reinterpret_cast(firstUnused + ncopy); + SetValueRangeToUndefined(firstUnused, nmissing); + return reinterpret_cast(firstUnused + nmissing); } JS_ALWAYS_INLINE bool @@ -386,7 +332,7 @@ ContextStack::popInlineFrame(FrameRegs ®s) JS_ASSERT(®s == &seg_->regs()); StackFrame *fp = regs.fp(); - Value *newsp = fp->actuals() - 1; + Value *newsp = fp->argv() - 1; JS_ASSERT(newsp >= fp->prev()->base()); newsp[-1] = fp->returnValue(); @@ -815,27 +761,17 @@ AbstractFramePtr::isStrictEvalFrame() const } inline Value * -AbstractFramePtr::formals() const +AbstractFramePtr::argv() const { if (isStackFrame()) - return asStackFrame()->formals(); + return asStackFrame()->argv(); #ifdef JS_ION - return asBaselineFrame()->formals(); -#else - JS_NOT_REACHED("Invalid frame"); -#endif -} -inline Value * -AbstractFramePtr::actuals() const -{ - if (isStackFrame()) - return asStackFrame()->actuals(); -#ifdef JS_ION - return asBaselineFrame()->actuals(); + return asBaselineFrame()->argv(); #else JS_NOT_REACHED("Invalid frame"); #endif } + inline bool AbstractFramePtr::hasArgsObj() const { diff --git a/js/src/vm/Stack.cpp b/js/src/vm/Stack.cpp index e38bc1b726b..4ed0e8fa88f 100644 --- a/js/src/vm/Stack.cpp +++ b/js/src/vm/Stack.cpp @@ -221,7 +221,7 @@ StackFrame::copyRawFrameSlots(AutoValueVector *vec) { if (!vec->resize(numFormalArgs() + script()->nfixed)) return false; - PodCopy(vec->begin(), formals(), numFormalArgs()); + PodCopy(vec->begin(), argv(), numFormalArgs()); PodCopy(vec->begin() + numFormalArgs(), slots(), script()->nfixed); return true; } diff --git a/js/src/vm/Stack.h b/js/src/vm/Stack.h index 8dc5e54ef7b..4e70b4f0e42 100644 --- a/js/src/vm/Stack.h +++ b/js/src/vm/Stack.h @@ -212,8 +212,7 @@ class AbstractFramePtr inline unsigned numActualArgs() const; inline unsigned numFormalArgs() const; - inline Value *formals() const; - inline Value *actuals() const; + inline Value *argv() const; inline bool hasArgsObj() const; inline ArgumentsObject &argsObj() const; @@ -287,33 +286,29 @@ class StackFrame YIELDING = 0x40, /* Interpret dispatched JSOP_YIELD */ FINISHED_IN_INTERP = 0x80, /* set if frame finished in Interpret() */ - /* Function arguments */ - OVERFLOW_ARGS = 0x100, /* numActualArgs > numFormalArgs */ - UNDERFLOW_ARGS = 0x200, /* numActualArgs < numFormalArgs */ - /* Function prologue state */ - HAS_CALL_OBJ = 0x400, /* CallObject created for heavyweight fun */ - HAS_ARGS_OBJ = 0x800, /* ArgumentsObject created for needsArgsObj script */ + HAS_CALL_OBJ = 0x100, /* CallObject created for heavyweight fun */ + HAS_ARGS_OBJ = 0x200, /* ArgumentsObject created for needsArgsObj script */ /* Lazy frame initialization */ - HAS_HOOK_DATA = 0x1000, /* frame has hookData_ set */ - HAS_RVAL = 0x2000, /* frame has rval_ set */ - HAS_SCOPECHAIN = 0x4000, /* frame has scopeChain_ set */ - HAS_PREVPC = 0x8000, /* frame has prevpc_ and prevInline_ set */ - HAS_BLOCKCHAIN = 0x10000, /* frame has blockChain_ set */ + HAS_HOOK_DATA = 0x400, /* frame has hookData_ set */ + HAS_RVAL = 0x800, /* frame has rval_ set */ + HAS_SCOPECHAIN = 0x1000, /* frame has scopeChain_ set */ + HAS_PREVPC = 0x2000, /* frame has prevpc_ and prevInline_ set */ + HAS_BLOCKCHAIN = 0x4000, /* frame has blockChain_ set */ /* Debugger state */ - PREV_UP_TO_DATE = 0x20000, /* see DebugScopes::updateLiveScopes */ + PREV_UP_TO_DATE = 0x8000, /* see DebugScopes::updateLiveScopes */ /* Used in tracking calls and profiling (see vm/SPSProfiler.cpp) */ - HAS_PUSHED_SPS_FRAME = 0x40000, /* SPS was notified of enty */ + HAS_PUSHED_SPS_FRAME = 0x10000, /* SPS was notified of enty */ /* Ion frame state */ - RUNNING_IN_ION = 0x80000, /* frame is running in Ion */ - CALLING_INTO_ION = 0x100000, /* frame is calling into Ion */ + RUNNING_IN_ION = 0x20000, /* frame is running in Ion */ + CALLING_INTO_ION = 0x40000, /* frame is calling into Ion */ /* Miscellaneous state. */ - USE_NEW_TYPE = 0x200000 /* Use new type for constructed |this| object. */ + USE_NEW_TYPE = 0x80000 /* Use new type for constructed |this| object. */ }; private: @@ -356,9 +351,7 @@ class StackFrame public: Value *slots() const { return (Value *)(this + 1); } Value *base() const { return slots() + script()->nfixed; } - Value *formals() const { return (Value *)this - fun()->nargs; } - Value *actuals() const { return formals() - (flags_ & OVERFLOW_ARGS ? 2 + u.nactual : 0); } - unsigned nactual() const { return u.nactual; } + Value *argv() const { return (Value *)this - Max(numActualArgs(), numFormalArgs()); } private: friend class FrameRegs; @@ -506,15 +499,8 @@ class StackFrame * * Only non-eval function frames have arguments. The arguments pushed by * the caller are the 'actual' arguments. The declared arguments of the - * callee are the 'formal' arguments. When the caller passes less or equal - * actual arguments, the actual and formal arguments are the same array - * (but with different extents). When the caller passes too many arguments, - * the formal subset of the actual arguments is copied onto the top of the - * stack. This allows the engine to maintain a jit-time constant offset of - * arguments from the frame pointer. Since the formal subset of the actual - * arguments is potentially on the stack twice, it is important for all - * reads/writes to refer to the same canonical memory location. This is - * abstracted by the unaliased{Formal,Actual} methods. + * callee are the 'formal' arguments. When the caller passes less actual + * arguments, missing formal arguments are padded with |undefined|. * * When a local/formal variable is "aliased" (accessed by nested closures, * dynamic scope operations, or 'arguments), the canonical location for @@ -536,8 +522,8 @@ class StackFrame bool copyRawFrameSlots(AutoValueVector *v); - inline unsigned numFormalArgs() const; - inline unsigned numActualArgs() const; + unsigned numFormalArgs() const { JS_ASSERT(hasArgs()); return fun()->nargs; } + unsigned numActualArgs() const { JS_ASSERT(hasArgs()); return u.nactual; } inline Value &canonicalActualArg(unsigned i) const; template @@ -706,18 +692,18 @@ class StackFrame JS_ASSERT(isFunctionFrame()); if (isEvalFrame()) return ((Value *)this)[-1]; - return formals()[-1]; + return argv()[-1]; } JSObject &constructorThis() const { JS_ASSERT(hasArgs()); - return formals()[-1].toObject(); + return argv()[-1].toObject(); } Value &thisValue() const { if (flags_ & (EVAL | GLOBAL)) return ((Value *)this)[-1]; - return formals()[-1]; + return argv()[-1]; } /* @@ -742,7 +728,7 @@ class StackFrame const Value &maybeCalleev() const { Value &calleev = flags_ & (EVAL | GLOBAL) ? ((Value *)this)[-2] - : formals()[-2]; + : argv()[-2]; JS_ASSERT(calleev.isObjectOrNull()); return calleev; } @@ -751,11 +737,11 @@ class StackFrame JS_ASSERT(isFunctionFrame()); if (isEvalFrame()) return ((Value *)this)[-2]; - return formals()[-2]; + return argv()[-2]; } CallReceiver callReceiver() const { - return CallReceiverFromArgv(formals()); + return CallReceiverFromArgv(argv()); } /* @@ -850,7 +836,7 @@ class StackFrame Value *generatorArgsSnapshotBegin() const { JS_ASSERT(isGeneratorFrame()); - return actuals() - 2; + return argv() - 2; } Value *generatorArgsSnapshotEnd() const { @@ -947,10 +933,6 @@ class StackFrame flags_ |= PREV_UP_TO_DATE; } - bool hasOverflowArgs() const { - return !!(flags_ & OVERFLOW_ARGS); - } - bool isYielding() { return !!(flags_ & YIELDING); }