From d73a0b5f829b18993c239aeb63a43fc0706ab2ea Mon Sep 17 00:00:00 2001 From: Brian Hackett Date: Sun, 3 Oct 2010 08:21:38 -0700 Subject: [PATCH] Lazify fp->scopeChain, JM call path cleanup. bug 593882, r=lw,dvander. --- js/src/jsinterp.h | 55 +++++++++++-------------- js/src/jsinterpinlines.h | 24 ++++------- js/src/jstracer.cpp | 1 - js/src/methodjit/BytecodeAnalyzer.cpp | 12 ++++++ js/src/methodjit/BytecodeAnalyzer.h | 6 ++- js/src/methodjit/Compiler.cpp | 28 ++++++++----- js/src/methodjit/InlineFrameAssembler.h | 2 - js/src/methodjit/InvokeHelpers.cpp | 13 +++++- js/src/methodjit/MethodJIT.cpp | 8 ++-- js/src/methodjit/StubCalls.cpp | 4 +- js/src/methodjit/TrampolineCompiler.cpp | 2 +- 11 files changed, 86 insertions(+), 69 deletions(-) diff --git a/js/src/jsinterp.h b/js/src/jsinterp.h index 690cf876b0e..04708a494bd 100644 --- a/js/src/jsinterp.h +++ b/js/src/jsinterp.h @@ -91,23 +91,14 @@ enum JSFrameFlags JSFRAME_UNDERFLOW_ARGS = 0x4000, /* numActualArgs < numFormalArgs */ /* Lazy frame initialization */ - JSFRAME_HAS_IMACRO_PC = 0x8000, /* frame has imacpc value available */ - JSFRAME_HAS_CALL_OBJ = 0x10000, /* frame has a callobj reachable from scopeChain_ */ - JSFRAME_HAS_ARGS_OBJ = 0x20000, /* frame has an argsobj in JSStackFrame::args */ - JSFRAME_HAS_HOOK_DATA = 0x40000, /* frame has hookData_ set */ - JSFRAME_HAS_ANNOTATION = 0x80000, /* frame has annotation_ set */ - - /* - * Whether the prevpc_ value is valid. If not set, the ncode_ value is - * valid and prevpc_ can be recovered using it. - */ - JSFRAME_HAS_PREVPC = 0x100000, - - /* - * For use by compiled functions, at function exit indicates whether rval_ - * has been assigned to. Otherwise the return value is carried in registers. - */ - JSFRAME_RVAL_ASSIGNED = 0x200000 + JSFRAME_HAS_IMACRO_PC = 0x8000, /* frame has imacpc value available */ + JSFRAME_HAS_CALL_OBJ = 0x10000, /* frame has a callobj reachable from scopeChain_ */ + JSFRAME_HAS_ARGS_OBJ = 0x20000, /* frame has an argsobj in JSStackFrame::args */ + JSFRAME_HAS_HOOK_DATA = 0x40000, /* frame has hookData_ set */ + JSFRAME_HAS_ANNOTATION = 0x80000, /* frame has annotation_ set */ + JSFRAME_HAS_RVAL = 0x100000, /* frame has rval_ set */ + JSFRAME_HAS_SCOPECHAIN = 0x200000, /* frame has scopeChain_ set */ + JSFRAME_HAS_PREVPC = 0x400000 /* frame has prevpc_ set */ }; /* @@ -117,7 +108,7 @@ enum JSFrameFlags struct JSStackFrame { private: - uint32 flags_; /* bits described by JSFrameFlags */ + mutable uint32 flags_; /* bits described by JSFrameFlags */ union { /* describes what code is executing in a */ JSScript *script; /* global frame */ JSFunction *fun; /* function frame, pre GetScopeChain */ @@ -127,7 +118,7 @@ struct JSStackFrame JSObject *obj; /* post GetArgumentsObject */ JSScript *script; /* eval has no args, but needs a script */ } args; - JSObject *scopeChain_; /* current scope chain */ + mutable JSObject *scopeChain_; /* current scope chain */ JSStackFrame *prev_; /* previous cx->regs->fp */ void *ncode_; /* return address for method JIT */ @@ -199,8 +190,7 @@ struct JSStackFrame uint32 nactual, uint32 flags); /* Called by method-jit stubs and serve as a specification for jit-code. */ - inline void initCallFrameCallerHalf(JSContext *cx, JSObject &scopeChain, - uint32 nactual, uint32 flags); + inline void initCallFrameCallerHalf(JSContext *cx, uint32 nactual, uint32 flags); inline void initCallFrameEarlyPrologue(JSFunction *fun, void *ncode); inline void initCallFrameLatePrologue(); @@ -497,6 +487,11 @@ struct JSStackFrame */ JSObject &scopeChain() const { + JS_ASSERT_IF(!(flags_ & JSFRAME_HAS_SCOPECHAIN), isFunctionFrame()); + if (!(flags_ & JSFRAME_HAS_SCOPECHAIN)) { + scopeChain_ = callee().getParent(); + flags_ |= JSFRAME_HAS_SCOPECHAIN; + } return *scopeChain_; } @@ -576,24 +571,23 @@ struct JSStackFrame /* Return value */ const js::Value& returnValue() { + if (!(flags_ & JSFRAME_HAS_RVAL)) + rval_.setUndefined(); return rval_; } + void markReturnValue() { + flags_ |= JSFRAME_HAS_RVAL; + } + void setReturnValue(const js::Value &v) { rval_ = v; + markReturnValue(); } void clearReturnValue() { rval_.setUndefined(); - } - - js::Value* addressReturnValue() { - return &rval_; - } - - void setAssignedReturnValue(const js::Value &v) { - flags_ |= JSFRAME_RVAL_ASSIGNED; - setReturnValue(v); + markReturnValue(); } /* Native-code return address */ @@ -740,6 +734,7 @@ struct JSStackFrame } JSObject **addressOfScopeChain() { + JS_ASSERT(flags_ & JSFRAME_HAS_SCOPECHAIN); return &scopeChain_; } diff --git a/js/src/jsinterpinlines.h b/js/src/jsinterpinlines.h index 87eb5dd9162..87d5f3d87ad 100644 --- a/js/src/jsinterpinlines.h +++ b/js/src/jsinterpinlines.h @@ -50,22 +50,20 @@ JSStackFrame::initCallFrame(JSContext *cx, JSObject &callee, JSFunction *fun, JS_ASSERT(fun == callee.getFunctionPrivate()); /* Initialize stack frame members. */ - flags_ = JSFRAME_FUNCTION | JSFRAME_HAS_PREVPC | flagsArg; + flags_ = JSFRAME_FUNCTION | JSFRAME_HAS_PREVPC | JSFRAME_HAS_SCOPECHAIN | flagsArg; exec.fun = fun; args.nactual = nactual; /* only need to write if over/under-flow */ scopeChain_ = callee.getParent(); /* prevpc_, prev_ initialized by push*Frame */ JS_ASSERT(!hasImacropc()); JS_ASSERT(!hasHookData()); - rval_.setUndefined(); JS_ASSERT(annotation() == NULL); JS_ASSERT(!hasCallObj()); } inline void -JSStackFrame::initCallFrameCallerHalf(JSContext *cx, JSObject &scopeChain, - uint32 nactual, uint32 flagsArg) +JSStackFrame::initCallFrameCallerHalf(JSContext *cx, uint32 nactual, uint32 flagsArg) { JS_ASSERT((flagsArg & ~(JSFRAME_CONSTRUCTING | JSFRAME_FUNCTION | @@ -76,7 +74,6 @@ JSStackFrame::initCallFrameCallerHalf(JSContext *cx, JSObject &scopeChain, /* Initialize the caller half of the stack frame members. */ flags_ = JSFRAME_FUNCTION | flagsArg; args.nactual = nactual; /* only need to write if over/under-flow */ - scopeChain_ = &scopeChain; prev_ = regs->fp; JS_ASSERT(!hasImacropc()); JS_ASSERT(!hasHookData()); @@ -104,8 +101,6 @@ JSStackFrame::initCallFrameEarlyPrologue(JSFunction *fun, void *ncode) inline void JSStackFrame::initCallFrameLatePrologue() { - rval_.setUndefined(); - SetValueRangeToUndefined(slots(), script()->nfixed); } @@ -128,7 +123,7 @@ JSStackFrame::initEvalFrame(JSScript *script, JSStackFrame *prev, dstvp[0].toObject().isFunction()); /* Initialize stack frame members. */ - flags_ = flagsArg | JSFRAME_HAS_PREVPC | + flags_ = flagsArg | JSFRAME_HAS_PREVPC | JSFRAME_HAS_SCOPECHAIN | (prev->flags_ & (JSFRAME_FUNCTION | JSFRAME_GLOBAL | JSFRAME_HAS_CALL_OBJ)); @@ -144,7 +139,6 @@ JSStackFrame::initEvalFrame(JSScript *script, JSStackFrame *prev, setPrev(prev, prevpc); JS_ASSERT(!hasImacropc()); JS_ASSERT(!hasHookData()); - rval_.setUndefined(); setAnnotation(prev->annotation()); } @@ -159,7 +153,7 @@ JSStackFrame::initGlobalFrame(JSScript *script, JSObject &chain, uint32 flagsArg vp[1].setUndefined(); /* Set after frame pushed using thisObject */ /* Initialize stack frame members. */ - flags_ = flagsArg | JSFRAME_GLOBAL | JSFRAME_HAS_PREVPC; + flags_ = flagsArg | JSFRAME_GLOBAL | JSFRAME_HAS_PREVPC | JSFRAME_HAS_SCOPECHAIN; exec.script = script; args.script = (JSScript *)0xbad; scopeChain_ = &chain; @@ -167,7 +161,6 @@ JSStackFrame::initGlobalFrame(JSScript *script, JSObject &chain, uint32 flagsArg prev_ = NULL; JS_ASSERT(!hasImacropc()); JS_ASSERT(!hasHookData()); - rval_.setUndefined(); JS_ASSERT(annotation() == NULL); } @@ -175,7 +168,7 @@ inline void JSStackFrame::initDummyFrame(JSContext *cx, JSObject &chain) { js::PodZero(this); - flags_ = JSFRAME_DUMMY | JSFRAME_HAS_PREVPC; + flags_ = JSFRAME_DUMMY | JSFRAME_HAS_PREVPC | JSFRAME_HAS_SCOPECHAIN; setPrev(cx->regs); chain.isGlobal(); setScopeChainNoCallObj(chain); @@ -335,12 +328,13 @@ JSStackFrame::setScopeChainNoCallObj(JSObject &obj) #ifdef DEBUG JS_ASSERT(&obj != NULL); JSObject *callObjBefore = maybeCallObj(); - if (!hasCallObj() && scopeChain_ != sInvalidScopeChain) { - for (JSObject *pobj = scopeChain_; pobj; pobj = pobj->getParent()) + if (!hasCallObj() && &scopeChain() != sInvalidScopeChain) { + for (JSObject *pobj = &scopeChain(); pobj; pobj = pobj->getParent()) JS_ASSERT_IF(pobj->isCall(), pobj->getPrivate() != this); } #endif scopeChain_ = &obj; + flags_ |= JSFRAME_HAS_SCOPECHAIN; JS_ASSERT(callObjBefore == maybeCallObj()); } @@ -350,7 +344,7 @@ JSStackFrame::setScopeChainAndCallObj(JSObject &obj) JS_ASSERT(&obj != NULL); JS_ASSERT(!hasCallObj() && obj.isCall() && obj.getPrivate() == this); scopeChain_ = &obj; - flags_ |= JSFRAME_HAS_CALL_OBJ; + flags_ |= JSFRAME_HAS_SCOPECHAIN | JSFRAME_HAS_CALL_OBJ; } inline void diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index 6962e5cef1c..81cdb7e1521 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -5739,7 +5739,6 @@ SynthesizeFrame(JSContext* cx, const FrameInfo& fi, JSObject* callee) ? JSFRAME_CONSTRUCTING | JSFRAME_CONSTRUCTING : 0; - /* Get pointer to new/frame/slots, prepare arguments. */ StackSpace &stack = cx->stack(); JSStackFrame *newfp = stack.getInlineFrame(cx, regs->sp, argc, newfun, diff --git a/js/src/methodjit/BytecodeAnalyzer.cpp b/js/src/methodjit/BytecodeAnalyzer.cpp index d5586ae243e..3332911ef75 100644 --- a/js/src/methodjit/BytecodeAnalyzer.cpp +++ b/js/src/methodjit/BytecodeAnalyzer.cpp @@ -116,6 +116,18 @@ BytecodeAnalyzer::analyze(uint32 index) usesRval = true; break; + case JSOP_NAME: + case JSOP_CALLNAME: + case JSOP_BINDNAME: + case JSOP_SETNAME: + case JSOP_DELNAME: + case JSOP_INCNAME: + case JSOP_DECNAME: + case JSOP_NAMEINC: + case JSOP_NAMEDEC: + usesScope = true; + break; + case JSOP_DEFAULT: case JSOP_GOTO: offs = (pc + JSOP_GOTO_LENGTH) - script->code; diff --git a/js/src/methodjit/BytecodeAnalyzer.h b/js/src/methodjit/BytecodeAnalyzer.h index fa672a24090..911219fe03e 100644 --- a/js/src/methodjit/BytecodeAnalyzer.h +++ b/js/src/methodjit/BytecodeAnalyzer.h @@ -68,11 +68,14 @@ namespace js /* Whether there are POPV/SETRVAL bytecodes which can write to the frame's rval. */ bool usesRval; + /* Whether there are NAME bytecodes which can access the frame's scope chain. */ + bool usesScope; + public: BytecodeAnalyzer(JSContext *cx, JSScript *script) : cx(cx), script(script), ops(NULL), doList(ContextAllocPolicy(cx)), - usesRval(false) + usesRval(false), usesScope(false) { } ~BytecodeAnalyzer(); @@ -83,6 +86,7 @@ namespace js public: bool usesReturnValue() const { return usesRval; } + bool usesScopeChain() const { return usesScope; } inline const OpcodeStatus & operator [](uint32 offs) const { JS_ASSERT(offs < script->length); diff --git a/js/src/methodjit/Compiler.cpp b/js/src/methodjit/Compiler.cpp index 4cbd3370227..83f70832be2 100644 --- a/js/src/methodjit/Compiler.cpp +++ b/js/src/methodjit/Compiler.cpp @@ -250,13 +250,7 @@ mjit::Compiler::generatePrologue() stubcc.crossJump(stubcc.masm.jump(), masm.label()); } - /* Fill in the members that initCallFrameLatePrologue does. */ - masm.storeValue(UndefinedValue(), Address(JSFrameReg, JSStackFrame::offsetOfReturnValue())); - - /* Set cx->fp */ - masm.loadPtr(FrameAddress(offsetof(VMFrame, cx)), Registers::ReturnReg); - - /* Set locals to undefined. */ + /* Set locals to undefined, as in initCallFrameLatePrologue */ for (uint32 i = 0; i < script->nfixed; i++) { Address local(JSFrameReg, sizeof(JSStackFrame) + i * sizeof(Value)); masm.storeValue(UndefinedValue(), local); @@ -269,6 +263,21 @@ mjit::Compiler::generatePrologue() } j.linkTo(masm.label(), &masm); + + if (analysis.usesScopeChain() && !fun->isHeavyweight()) { + /* + * Load the scope chain into the frame if necessary. The scope chain + * is always set for global and eval frames, and will have been set by + * GetCallObject for heavyweight function frames. + */ + RegisterID t0 = Registers::ReturnReg; + Jump hasScope = masm.branchTest32(Assembler::NonZero, + FrameFlagsAddress(), Imm32(JSFRAME_HAS_SCOPECHAIN)); + masm.loadPayload(Address(JSFrameReg, JSStackFrame::offsetOfCallee(fun)), t0); + masm.loadPtr(Address(t0, offsetof(JSObject, parent)), t0); + masm.storePtr(t0, Address(JSFrameReg, JSStackFrame::offsetOfScopeChain())); + hasScope.linkTo(masm.label(), &masm); + } } return Compile_Okay; @@ -647,7 +656,7 @@ mjit::Compiler::generateMethod() { RegisterID reg = frame.allocReg(); masm.load32(FrameFlagsAddress(), reg); - masm.or32(Imm32(JSFRAME_RVAL_ASSIGNED), reg); + masm.or32(Imm32(JSFRAME_HAS_RVAL), reg); masm.store32(reg, FrameFlagsAddress()); frame.freeReg(reg); @@ -1782,8 +1791,7 @@ mjit::Compiler::loadReturnValue(Assembler &masm) masm.loadValueAsComponents(UndefinedValue(), JSReturnReg_Type, JSReturnReg_Data); if (analysis.usesReturnValue()) { Jump rvalClear = masm.branchTest32(Assembler::Zero, - FrameFlagsAddress(), - Imm32(JSFRAME_RVAL_ASSIGNED)); + FrameFlagsAddress(), Imm32(JSFRAME_HAS_RVAL)); Address rvalAddress(JSFrameReg, JSStackFrame::offsetOfReturnValue()); masm.loadValueAsComponents(rvalAddress, JSReturnReg_Type, JSReturnReg_Data); diff --git a/js/src/methodjit/InlineFrameAssembler.h b/js/src/methodjit/InlineFrameAssembler.h index 833d3a40d68..23d91923a9b 100644 --- a/js/src/methodjit/InlineFrameAssembler.h +++ b/js/src/methodjit/InlineFrameAssembler.h @@ -114,8 +114,6 @@ class InlineFrameAssembler { AdjustedFrame adj(sizeof(JSStackFrame) + frameDepth * sizeof(Value)); masm.store32(Imm32(JSFRAME_FUNCTION | flags), adj.addrOf(JSStackFrame::offsetOfFlags())); - masm.loadPtr(Address(funObjReg, offsetof(JSObject, parent)), t0); - masm.storePtr(t0, adj.addrOf(JSStackFrame::offsetOfScopeChain())); masm.storePtr(JSFrameReg, adj.addrOf(JSStackFrame::offsetOfPrev())); DataLabelPtr ncodePatch = diff --git a/js/src/methodjit/InvokeHelpers.cpp b/js/src/methodjit/InvokeHelpers.cpp index b24a9da0ecd..a7e3eadf9b3 100644 --- a/js/src/methodjit/InvokeHelpers.cpp +++ b/js/src/methodjit/InvokeHelpers.cpp @@ -311,7 +311,6 @@ stubs::FixupArity(VMFrame &f, uint32 nactual) * early prologue. */ uint32 flags = oldfp->isConstructingFlag(); - JSObject &scopeChain = oldfp->scopeChain(); JSFunction *fun = oldfp->fun(); void *ncode = oldfp->nativeReturnAddress(); @@ -327,7 +326,7 @@ stubs::FixupArity(VMFrame &f, uint32 nactual) THROWV(NULL); /* Reset the part of the stack frame set by the caller. */ - newfp->initCallFrameCallerHalf(cx, scopeChain, nactual, flags); + newfp->initCallFrameCallerHalf(cx, nactual, flags); /* Reset the part of the stack frame set by the prologue up to now. */ newfp->initCallFrameEarlyPrologue(fun, ncode); @@ -822,6 +821,16 @@ RunTracer(VMFrame &f) if (!cx->traceJitEnabled) return NULL; + /* + * Force initialization of the entry frame's scope chain and return value, + * if necessary. The tracer can query the scope chain without needing to + * check the HAS_SCOPECHAIN flag, and the frame is guaranteed to have the + * correct return value stored if we trace/interpret through to the end + * of the frame. + */ + entryFrame->scopeChain(); + entryFrame->returnValue(); + bool blacklist; uintN inlineCallCount = 0; tpa = MonitorTracePoint(f.cx, inlineCallCount, blacklist); diff --git a/js/src/methodjit/MethodJIT.cpp b/js/src/methodjit/MethodJIT.cpp index cd7143851f1..191c9c54960 100644 --- a/js/src/methodjit/MethodJIT.cpp +++ b/js/src/methodjit/MethodJIT.cpp @@ -738,10 +738,6 @@ EnterMethodJIT(JSContext *cx, JSStackFrame *fp, void *code) prof.start(); #endif -#ifdef DEBUG - JSStackFrame *checkFp = fp; -#endif - Value *stackLimit = cx->stack().getStackLimit(cx); if (!stackLimit) return false; @@ -752,8 +748,10 @@ EnterMethodJIT(JSContext *cx, JSStackFrame *fp, void *code) JSBool ok = JaegerTrampoline(cx, fp, code, stackLimit); cx->setCurrentRegs(oldRegs); + JS_ASSERT(fp == cx->fp()); - JS_ASSERT(checkFp == cx->fp()); + /* The trampoline wrote the return value but did not set the HAS_RVAL flag. */ + fp->markReturnValue(); #ifdef JS_METHODJIT_SPEW prof.stop(); diff --git a/js/src/methodjit/StubCalls.cpp b/js/src/methodjit/StubCalls.cpp index 70e405ce2ab..5850c36e46a 100644 --- a/js/src/methodjit/StubCalls.cpp +++ b/js/src/methodjit/StubCalls.cpp @@ -1338,7 +1338,7 @@ stubs::Debugger(VMFrame &f, jsbytecode *pc) case JSTRAP_RETURN: f.cx->throwing = JS_FALSE; - f.cx->fp()->setAssignedReturnValue(rval); + f.cx->fp()->setReturnValue(rval); #if (defined(JS_NO_FASTCALL) && defined(JS_CPU_X86)) || defined(_WIN64) *f.returnAddressLocation() = JS_FUNC_TO_DATA_PTR(void *, JS_METHODJIT_DATA(f.cx).trampolines.forceReturnFast); @@ -1378,7 +1378,7 @@ stubs::Trap(VMFrame &f, jsbytecode *pc) case JSTRAP_RETURN: f.cx->throwing = JS_FALSE; - f.cx->fp()->setAssignedReturnValue(rval); + f.cx->fp()->setReturnValue(rval); #if (defined(JS_NO_FASTCALL) && defined(JS_CPU_X86)) || defined(_WIN64) *f.returnAddressLocation() = JS_FUNC_TO_DATA_PTR(void *, JS_METHODJIT_DATA(f.cx).trampolines.forceReturnFast); diff --git a/js/src/methodjit/TrampolineCompiler.cpp b/js/src/methodjit/TrampolineCompiler.cpp index bffa17e9fb5..efca7be919c 100644 --- a/js/src/methodjit/TrampolineCompiler.cpp +++ b/js/src/methodjit/TrampolineCompiler.cpp @@ -126,7 +126,7 @@ TrampolineCompiler::generateForceReturn(Assembler &masm) /* Store any known return value */ masm.loadValueAsComponents(UndefinedValue(), JSReturnReg_Type, JSReturnReg_Data); Jump rvalClear = masm.branchTest32(Assembler::Zero, - FrameFlagsAddress(), Imm32(JSFRAME_RVAL_ASSIGNED)); + FrameFlagsAddress(), Imm32(JSFRAME_HAS_RVAL)); Address rvalAddress(JSFrameReg, JSStackFrame::offsetOfReturnValue()); masm.loadValueAsComponents(rvalAddress, JSReturnReg_Type, JSReturnReg_Data); rvalClear.linkTo(masm.label(), &masm);