From 3ae9a18f24ffa50f24d46bd3d3b4216a4628da5b Mon Sep 17 00:00:00 2001 From: Brian Hackett Date: Tue, 13 Sep 2011 15:01:46 -0700 Subject: [PATCH] [INFER] Fix code discarding for JM+TM integration, bug 685358. --- .../tests/basic/wrap-primitive-this.js | 3 +- js/src/jsinterp.cpp | 76 ++------------ js/src/jsinterp.h | 7 +- js/src/methodjit/Compiler.cpp | 2 +- js/src/methodjit/InvokeHelpers.cpp | 98 +++++++++---------- js/src/methodjit/MethodJIT.cpp | 2 +- js/src/methodjit/MethodJIT.h | 7 +- js/src/methodjit/StubCalls.h | 4 +- 8 files changed, 72 insertions(+), 127 deletions(-) diff --git a/js/src/jit-test/tests/basic/wrap-primitive-this.js b/js/src/jit-test/tests/basic/wrap-primitive-this.js index 5c9db8085c4a..dd8d4adfba01 100644 --- a/js/src/jit-test/tests/basic/wrap-primitive-this.js +++ b/js/src/jit-test/tests/basic/wrap-primitive-this.js @@ -15,9 +15,10 @@ var HOTLOOP = this.tracemonkey ? tracemonkey.HOTLOOP : 8; var a; function f(n) { - for (var i = 0; i < HOTLOOP; i++) + for (var i = 0; i < HOTLOOP; i++) { if (i == HOTLOOP - 2) a = this; + } } /* diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp index 56780927883e..9755668b760d 100644 --- a/js/src/jsinterp.cpp +++ b/js/src/jsinterp.cpp @@ -1599,10 +1599,14 @@ static inline void TypeCheckNextBytecode(JSContext *cx, JSScript *script, unsigned n, const FrameRegs ®s) { #ifdef DEBUG - if (cx->typeInferenceEnabled() && - *regs.pc != JSOP_TRAP && + if (*regs.pc != JSOP_TRAP && n == analyze::GetBytecodeLength(regs.pc)) { - TypeScript::CheckBytecode(cx, script, regs.pc, regs.sp); + if (script->hasAnalysis() && !regs.fp()->hasImacropc()) { + jsbytecode *nextpc = regs.pc + GetBytecodeLength(cx, script, regs.pc); + JS_ASSERT(regs.sp == regs.fp()->base() + script->analysis()->getCode(nextpc).stackDepth); + } + if (cx->typeInferenceEnabled()) + TypeScript::CheckBytecode(cx, script, regs.pc, regs.sp); } #endif } @@ -1828,30 +1832,12 @@ js::Interpret(JSContext *cx, StackFrame *entryFrame, InterpMode interpMode) goto error; \ JS_END_MACRO -#if defined(JS_TRACER) && defined(JS_METHODJIT) -# define LEAVE_ON_SAFE_POINT() \ - do { \ - JS_ASSERT_IF(leaveOnSafePoint, !TRACE_RECORDER(cx)); \ - JS_ASSERT_IF(leaveOnSafePoint, !TRACE_PROFILER(cx)); \ - JS_ASSERT_IF(leaveOnSafePoint, interpMode != JSINTERP_NORMAL); \ - if (leaveOnSafePoint && !regs.fp()->hasImacropc() && \ - script->maybeNativeCodeForPC(regs.fp()->isConstructing(), regs.pc)) { \ - JS_ASSERT(!TRACE_RECORDER(cx)); \ - interpReturnOK = true; \ - goto leave_on_safe_point; \ - } \ - } while (0) -#else -# define LEAVE_ON_SAFE_POINT() /* nop */ -#endif - #define BRANCH(n) \ JS_BEGIN_MACRO \ regs.pc += (n); \ op = (JSOp) *regs.pc; \ if ((n) <= 0) \ goto check_backedge; \ - LEAVE_ON_SAFE_POINT(); \ DO_OP(); \ JS_END_MACRO @@ -1887,13 +1873,6 @@ js::Interpret(JSContext *cx, StackFrame *entryFrame, InterpMode interpMode) Value *argv = regs.fp()->maybeFormalArgs(); CHECK_INTERRUPT_HANDLER(); -#if defined(JS_TRACER) && defined(JS_METHODJIT) - bool leaveOnSafePoint = (interpMode == JSINTERP_SAFEPOINT); -# define CLEAR_LEAVE_ON_TRACE_POINT() ((void) (leaveOnSafePoint = false)) -#else -# define CLEAR_LEAVE_ON_TRACE_POINT() ((void) 0) -#endif - if (!entryFrame) entryFrame = regs.fp(); @@ -2078,17 +2057,8 @@ js::Interpret(JSContext *cx, StackFrame *entryFrame, InterpMode interpMode) LoopProfile *prof = TRACE_PROFILER(cx); JS_ASSERT(!TRACE_RECORDER(cx)); LoopProfile::ProfileAction act = prof->profileOperation(cx, op); - switch (act) { - case LoopProfile::ProfComplete: - if (interpMode != JSINTERP_NORMAL) { - leaveOnSafePoint = true; - LEAVE_ON_SAFE_POINT(); - } - break; - default: - moreInterrupts = true; - break; - } + if (act != LoopProfile::ProfComplete) + moreInterrupts = true; } #endif if (TraceRecorder* tr = TRACE_RECORDER(cx)) { @@ -2096,23 +2066,6 @@ js::Interpret(JSContext *cx, StackFrame *entryFrame, InterpMode interpMode) AbortableRecordingStatus status = tr->monitorRecording(op); JS_ASSERT_IF(cx->isExceptionPending(), status == ARECORD_ERROR); - if (interpMode != JSINTERP_NORMAL) { - JS_ASSERT(interpMode == JSINTERP_RECORD || JSINTERP_SAFEPOINT); - switch (status) { - case ARECORD_IMACRO_ABORTED: - case ARECORD_ABORTED: - case ARECORD_COMPLETED: - case ARECORD_STOP: -#ifdef JS_METHODJIT - leaveOnSafePoint = true; - LEAVE_ON_SAFE_POINT(); -#endif - break; - default: - break; - } - } - switch (status) { case ARECORD_CONTINUE: moreInterrupts = true; @@ -2121,7 +2074,6 @@ js::Interpret(JSContext *cx, StackFrame *entryFrame, InterpMode interpMode) case ARECORD_IMACRO_ABORTED: atoms = rt->atomState.commonAtomsStart(); op = JSOp(*regs.pc); - CLEAR_LEAVE_ON_TRACE_POINT(); if (status == ARECORD_IMACRO) DO_OP(); /* keep interrupting for op. */ break; @@ -2166,7 +2118,7 @@ END_EMPTY_CASES BEGIN_CASE(JSOP_TRACE) BEGIN_CASE(JSOP_NOTRACE) - LEAVE_ON_SAFE_POINT(); + /* No-op */ END_CASE(JSOP_TRACE) check_backedge: @@ -2183,7 +2135,6 @@ check_backedge: JS_ASSERT(!TRACE_PROFILER(cx)); MONITOR_BRANCH_TRACEVIS; ENABLE_INTERRUPTS(); - CLEAR_LEAVE_ON_TRACE_POINT(); } JS_ASSERT_IF(cx->isExceptionPending(), r == MONITOR_ERROR); RESTORE_INTERP_VARS_CHECK_EXCEPTION(); @@ -2308,7 +2259,6 @@ BEGIN_CASE(JSOP_STOP) if (js_CodeSpec[*imacpc].format & JOF_DECOMPOSE) regs.pc += GetDecomposeLength(imacpc, js_CodeSpec[*imacpc].length); regs.fp()->clearImacropc(); - LEAVE_ON_SAFE_POINT(); atoms = script->atoms; op = JSOp(*regs.pc); DO_OP(); @@ -5409,12 +5359,6 @@ END_VARLEN_CASE BEGIN_CASE(JSOP_EXCEPTION) PUSH_COPY(cx->getPendingException()); cx->clearPendingException(); -#if defined(JS_TRACER) && defined(JS_METHODJIT) - if (interpMode == JSINTERP_PROFILE) { - leaveOnSafePoint = true; - LEAVE_ON_SAFE_POINT(); - } -#endif CHECK_BRANCH(); END_CASE(JSOP_EXCEPTION) diff --git a/js/src/jsinterp.h b/js/src/jsinterp.h index f4e013db1b25..afd132cb0e42 100644 --- a/js/src/jsinterp.h +++ b/js/src/jsinterp.h @@ -235,10 +235,9 @@ enum InterpMode { JSINTERP_NORMAL = 0, /* interpreter is running normally */ JSINTERP_RECORD = 1, /* interpreter has been started to record/run traces */ - JSINTERP_SAFEPOINT = 2, /* interpreter should leave on a method JIT safe point */ - JSINTERP_PROFILE = 3, /* interpreter should profile a loop */ - JSINTERP_REJOIN = 4, /* as normal, but the frame has already started */ - JSINTERP_SKIP_TRAP = 5 /* as REJOIN, but skip trap at first opcode */ + JSINTERP_PROFILE = 2, /* interpreter should profile a loop */ + JSINTERP_REJOIN = 3, /* as normal, but the frame has already started */ + JSINTERP_SKIP_TRAP = 4 /* as REJOIN, but skip trap at first opcode */ }; /* diff --git a/js/src/methodjit/Compiler.cpp b/js/src/methodjit/Compiler.cpp index 915febab0e46..5033afb996a6 100644 --- a/js/src/methodjit/Compiler.cpp +++ b/js/src/methodjit/Compiler.cpp @@ -6790,7 +6790,7 @@ mjit::Compiler::jumpAndTrace(Jump j, jsbytecode *target, Jump *slow, bool *tramp jsbytecode* pc = PC; PC = target; - OOL_STUBCALL(stubs::InvokeTracer, REJOIN_FINISH_FRAME); + OOL_STUBCALL(stubs::InvokeTracer, REJOIN_RUN_TRACER); PC = pc; } diff --git a/js/src/methodjit/InvokeHelpers.cpp b/js/src/methodjit/InvokeHelpers.cpp index 56713712b877..58cdde0883cf 100644 --- a/js/src/methodjit/InvokeHelpers.cpp +++ b/js/src/methodjit/InvokeHelpers.cpp @@ -177,10 +177,17 @@ top: static void InlineReturn(VMFrame &f) { + bool shiftResult = f.fp()->loweredCallOrApply(); + JS_ASSERT(f.fp() != f.entryfp); JS_ASSERT(!IsActiveWithOrBlock(f.cx, f.fp()->scopeChain(), 0)); f.cx->stack.popInlineFrame(f.regs); + if (shiftResult) { + f.regs.sp[-2] = f.regs.sp[-1]; + f.regs.sp--; + } + DebugOnly op = js_GetOpcode(f.cx, f.fp()->script(), f.regs.pc); JS_ASSERT(op == JSOP_CALL || op == JSOP_NEW || @@ -774,18 +781,6 @@ HandleErrorInExcessFrame(VMFrame &f, StackFrame *stopFp, bool searchedTopmostFra return returnOK; } -/* Returns whether the current PC has method JIT'd code. */ -static inline void * -AtSafePoint(JSContext *cx) -{ - StackFrame *fp = cx->fp(); - if (fp->hasImacropc()) - return NULL; - - JSScript *script = fp->script(); - return script->maybeNativeCodeForPC(fp->isConstructing(), cx->regs().pc); -} - /* * Interprets until either a safe point is reached that has method JIT'd * code, or the current frame tries to return. @@ -795,16 +790,12 @@ PartialInterpret(VMFrame &f) { JSContext *cx = f.cx; StackFrame *fp = cx->fp(); - -#ifdef DEBUG - JSScript *script = fp->script(); JS_ASSERT(!fp->finishedInInterpreter()); - JS_ASSERT(fp->hasImacropc() || - !script->maybeNativeCodeForPC(fp->isConstructing(), cx->regs().pc)); -#endif - JSBool ok = JS_TRUE; - ok = Interpret(cx, fp, JSINTERP_SAFEPOINT); + JS_ASSERT(!cx->compartment->jaegerCompartment()->finishingTracer); + cx->compartment->jaegerCompartment()->finishingTracer = true; + JSBool ok = Interpret(cx, fp, JSINTERP_REJOIN); + cx->compartment->jaegerCompartment()->finishingTracer = false; return ok; } @@ -916,13 +907,6 @@ EvaluateExcessFrame(VMFrame &f, StackFrame *entryFrame) if (!fp->hasImacropc() && FrameIsFinished(cx)) return HandleFinishedFrame(f, entryFrame); - if (void *ncode = AtSafePoint(cx)) { - if (!JaegerShotAtSafePoint(cx, ncode, false)) - return false; - InlineReturn(f); - return true; - } - return PartialInterpret(f); } @@ -1013,10 +997,10 @@ js::mjit::ResetTraceHint(JSScript *script, jsbytecode *pc, uint16_t index, bool } #if JS_MONOIC -void * +JSBool RunTracer(VMFrame &f, ic::TraceICInfo &ic) #else -void * +JSBool RunTracer(VMFrame &f) #endif { @@ -1026,7 +1010,14 @@ RunTracer(VMFrame &f) /* :TODO: nuke PIC? */ if (!cx->traceJitEnabled) - return NULL; + return false; + + /* + * Don't reenter the tracer while finishing frames we bailed out from, + * to avoid over-recursing. + */ + if (cx->compartment->jaegerCompartment()->finishingTracer) + return false; /* * Force initialization of the entry frame's scope chain and return value, @@ -1073,6 +1064,9 @@ RunTracer(VMFrame &f) tpa = MonitorTracePoint(f.cx, &blacklist, traceData, traceEpoch, loopCounter, hits); JS_ASSERT(!TRACE_RECORDER(cx)); + + if (tpa != TPA_Nothing) + ClearAllFrames(cx->compartment); } #if JS_MONOIC @@ -1090,11 +1084,11 @@ RunTracer(VMFrame &f) JS_ASSERT(f.fp() == cx->fp()); switch (tpa) { case TPA_Nothing: - return NULL; + return false; case TPA_Error: if (!HandleErrorInExcessFrame(f, entryFrame, f.fp()->finishedInInterpreter())) - THROWV(NULL); + THROWV(false); JS_ASSERT(!cx->fp()->hasImacropc()); break; @@ -1130,7 +1124,7 @@ RunTracer(VMFrame &f) restart: /* Step 1. Finish frames created after the entry frame. */ if (!FinishExcessFrames(f, entryFrame)) - THROWV(NULL); + THROWV(false); /* IMacros are guaranteed to have been removed by now. */ JS_ASSERT(f.fp() == entryFrame); @@ -1139,21 +1133,14 @@ RunTracer(VMFrame &f) /* Step 2. If entryFrame is done, use a special path to return to EnterMethodJIT(). */ if (FrameIsFinished(cx)) { if (!HandleFinishedFrame(f, entryFrame)) - THROWV(NULL); - void *addr = *f.returnAddressLocation(); - if (addr != JS_FUNC_TO_DATA_PTR(void *, JaegerInterpoline)) - *f.returnAddressLocation() = cx->jaegerCompartment()->forceReturnFromFastCall(); - return NULL; + THROWV(false); + return true; } - /* Step 3. If entryFrame is at a safe point, just leave. */ - if (void *ncode = AtSafePoint(cx)) - return ncode; - - /* Step 4. Do a partial interp, then restart the whole process. */ + /* Step 3. Do a partial interp, then restart the whole process. */ if (!PartialInterpret(f)) { if (!HandleErrorInExcessFrame(f, entryFrame)) - THROWV(NULL); + THROWV(false); } goto restart; @@ -1163,7 +1150,7 @@ RunTracer(VMFrame &f) #if defined JS_TRACER # if defined JS_MONOIC -void *JS_FASTCALL +JSBool JS_FASTCALL stubs::InvokeTracer(VMFrame &f, ic::TraceICInfo *ic) { return RunTracer(f, *ic); @@ -1171,7 +1158,7 @@ stubs::InvokeTracer(VMFrame &f, ic::TraceICInfo *ic) # else -void *JS_FASTCALL +JSBool JS_FASTCALL stubs::InvokeTracer(VMFrame &f) { return RunTracer(f); @@ -1572,10 +1559,21 @@ js_InternalInterpret(void *returnData, void *returnType, void *returnReg, js::VM break; } - case REJOIN_FINISH_FRAME: - /* InvokeTracer finishes the frame it is given, including the epilogue. */ - if (fp->isFunctionFrame()) - fp->markFunctionEpilogueDone(); + case REJOIN_RUN_TRACER: + if (returnReg) { + /* InvokeTracer finishes the frame it is given, including the epilogue. */ + if (fp->isFunctionFrame()) + fp->markFunctionEpilogueDone(); + if (fp != f.entryfp) { + InlineReturn(f); + cx->compartment->jaegerCompartment()->setLastUnfinished(Jaeger_Unfinished); + *f.oldregs = f.regs; + return NULL; + } else { + cx->compartment->jaegerCompartment()->setLastUnfinished(Jaeger_Returned); + return NULL; + } + } break; default: diff --git a/js/src/methodjit/MethodJIT.cpp b/js/src/methodjit/MethodJIT.cpp index 9492f5d87539..a72307a01415 100644 --- a/js/src/methodjit/MethodJIT.cpp +++ b/js/src/methodjit/MethodJIT.cpp @@ -894,7 +894,7 @@ mjit::EnterMethodJIT(JSContext *cx, StackFrame *fp, void *code, Value *stackLimi JaegerStatus status = cx->compartment->jaegerCompartment()->lastUnfinished(); if (status) { - if (partial) { + if (partial || status == Jaeger_Returned) { /* * Being called from the interpreter, which will resume execution * where the JIT left off. diff --git a/js/src/methodjit/MethodJIT.h b/js/src/methodjit/MethodJIT.h index 434ba7ad3703..b89401eb0599 100644 --- a/js/src/methodjit/MethodJIT.h +++ b/js/src/methodjit/MethodJIT.h @@ -319,8 +319,8 @@ enum RejoinState { */ REJOIN_BRANCH, - /* Calls to RunTracer which finished the given frame. */ - REJOIN_FINISH_FRAME + /* Calls to RunTracer which either finished the frame or did nothing. */ + REJOIN_RUN_TRACER }; /* Helper to watch for recompilation and frame expansion activity on a compartment. */ @@ -463,6 +463,9 @@ class JaegerCompartment { */ Vector orphanedNativeFrames; Vector orphanedNativePools; + + /* Whether frames pushed after bailing out in RunTracer are unwinding. */ + bool finishingTracer; }; /* diff --git a/js/src/methodjit/StubCalls.h b/js/src/methodjit/StubCalls.h index 41c712a24b7e..7c9e72b21d93 100644 --- a/js/src/methodjit/StubCalls.h +++ b/js/src/methodjit/StubCalls.h @@ -114,9 +114,9 @@ void UncachedNewHelper(VMFrame &f, uint32 argc, UncachedCallResult *ucr); void JS_FASTCALL CreateThis(VMFrame &f, JSObject *proto); void JS_FASTCALL Throw(VMFrame &f); #if JS_MONOIC -void * JS_FASTCALL InvokeTracer(VMFrame &f, ic::TraceICInfo *tic); +JSBool JS_FASTCALL InvokeTracer(VMFrame &f, ic::TraceICInfo *tic); #else -void * JS_FASTCALL InvokeTracer(VMFrame &f); +JSBool JS_FASTCALL InvokeTracer(VMFrame &f); #endif void * JS_FASTCALL LookupSwitch(VMFrame &f, jsbytecode *pc);