From 4934b377d9cf5df6f80da7caab4f2178c6cec307 Mon Sep 17 00:00:00 2001 From: Ben Cheng Date: Mon, 20 Sep 2010 22:20:31 -0700 Subject: Several fixes for JIT and self-verification under corner cases. 1) Fix the self-verification mode to handle backward chaining cell properly when a single-step instruction is in the middle of the cyclic portion of the trace. Then found issue 2 when changing the JIT threshold to 1. 2) When the code cache is full, the VM may stop making forward progress and bounces back and forth between the debug and fast intepreters as the translation request is constantly rejected. The fix is to stay in the debug interpreter until the corner case condition is cleared. Then found issue 3. 3) Under self-verification mode, the code cache reset request may get delayed indefinitely due to spurious indication that a thread is running JIT'ed code. Trivial fix - make sure the inJitCodeCache flag is cleared. (cherry-picked from dalvik-dev) Change-Id: Ic0b9952c0ae545f68f7eb2ae06a82a634ab62e9e --- vm/interp/InterpDefs.h | 25 +++++++++++++++++ vm/interp/Jit.c | 53 ++++++++++++++++++++++------------- vm/mterp/armv5te/footer.S | 4 +++ vm/mterp/out/InterpAsm-armv4t.S | 4 +++ vm/mterp/out/InterpAsm-armv5te-vfp.S | 4 +++ vm/mterp/out/InterpAsm-armv5te.S | 4 +++ vm/mterp/out/InterpAsm-armv7-a-neon.S | 4 +++ vm/mterp/out/InterpAsm-armv7-a.S | 4 +++ 8 files changed, 82 insertions(+), 20 deletions(-) diff --git a/vm/interp/InterpDefs.h b/vm/interp/InterpDefs.h index a01ee6857..4ccdee327 100644 --- a/vm/interp/InterpDefs.h +++ b/vm/interp/InterpDefs.h @@ -247,6 +247,31 @@ static inline bool dvmJitDebuggerOrProfilerActive() || gDvm.activeProfilers != 0 || gDvm.debuggerActive; } + +/* + * Hide the translations and stick with the interpreter as long as one of the + * following conditions is true. + */ +static inline bool dvmJitHideTranslation() +{ + return (gDvm.sumThreadSuspendCount != 0) || + (gDvmJit.codeCacheFull == true) || + (gDvmJit.pProfTable == NULL); +} + +/* + * The fast and debug interpreter may be doing ping-pong without making forward + * progress if the same trace building request sent upon entering the fast + * interpreter is rejected immediately by the debug interpreter. Use the + * following function to poll the rejection reasons and stay in the debug + * interpreter until they are cleared. This will guarantee forward progress + * in the extreme corner cases (eg set compiler threashold to 1). + */ +static inline bool dvmJitStayInPortableInterpreter() +{ + return dvmJitHideTranslation() || + (gDvmJit.compilerQueueLength >= gDvmJit.compilerHighWater); +} #endif #endif /*_DALVIK_INTERP_DEFS*/ diff --git a/vm/interp/Jit.c b/vm/interp/Jit.c index fda9f18e1..c800fe558 100644 --- a/vm/interp/Jit.c +++ b/vm/interp/Jit.c @@ -173,26 +173,27 @@ void* dvmSelfVerificationRestoreState(const u2* pc, const void* fp, // Special case when punting after a single instruction if (exitState == kSVSPunt && pc == shadowSpace->startPC) { shadowSpace->selfVerificationState = kSVSIdle; - } else if (exitState == kSVSBackwardBranch && pc != shadowSpace->startPC) { + } else if (exitState == kSVSBackwardBranch && pc < shadowSpace->startPC) { /* - * Consider a loop which contains the following instructions: + * Consider a trace with a backward branch: * 1: .. * 2: .. - * 3: ..(single-stepped) + * 3: .. * 4: .. - * 5: Goto 1 + * 5: Goto {1 or 2 or 3 or 4} * - * The state comparisons are conducted in two batches: 1,2 as the - * first batch and 4,5 as the second batch. If the execution in the - * JIT'ed code is resumed from 4, the start PC for self-verification - * will be set to 4. The exit point for 5 is a backward branch and - * will suggest the end PC to be 1, and will do the state comparison - * when 1 is hit for the second time in the interpreter. It is incorrect - * in this case as the work for 1,2 from the current iteration have been - * committed and the state comparison has been conducted. + * If there instruction 5 goes to 1 and there is no single-step + * instruction in the loop, pc is equal to shadowSpace->startPC and + * we will honor the backward branch condition. * - * So we alter the state from BackwardBranch to Normal and state - * comparison will be issued immediate following the goto. + * If the single-step instruction is outside the loop, then after + * resuming in the trace the startPC will be less than pc so we will + * also honor the backward branch condition. + * + * If the single-step is inside the loop, we won't hit the same endPC + * twice when the interpreter is re-executing the trace so we want to + * cancel the backward branch condition. In this case it can be + * detected as the endPC (ie pc) will be less than startPC. */ shadowSpace->selfVerificationState = kSVSNormal; } else { @@ -909,7 +910,19 @@ int dvmCheckJit(const u2* pc, Thread* self, InterpState* interpState, interpState->jitState = kJitSingleStepEnd; break; case kJitSingleStepEnd: - interpState->entryPoint = kInterpEntryResume; + /* + * Clear the inJitCodeCache flag and abandon the resume attempt if + * we cannot switch back to the translation due to corner-case + * conditions. If the flag is not cleared and the code cache is full + * we will be stuck in the debug interpreter as the code cache + * cannot be reset. + */ + if (dvmJitStayInPortableInterpreter()) { + interpState->entryPoint = kInterpEntryInstr; + self->inJitCodeCache = 0; + } else { + interpState->entryPoint = kInterpEntryResume; + } interpState->jitState = kJitDone; switchInterp = true; break; @@ -945,7 +958,8 @@ int dvmCheckJit(const u2* pc, Thread* self, InterpState* interpState, */ assert(switchInterp == false || interpState->jitState == kJitDone || interpState->jitState == kJitNot); - return switchInterp && !debugOrProfile && !stayOneMoreInst; + return switchInterp && !debugOrProfile && !stayOneMoreInst && + !dvmJitStayInPortableInterpreter(); } JitEntry *dvmFindJitEntry(const u2* pc) @@ -975,9 +989,7 @@ void* dvmJitGetCodeAddr(const u2* dPC) int idx = dvmJitHash(dPC); const u2* npc = gDvmJit.pJitEntryTable[idx].dPC; if (npc != NULL) { - bool hideTranslation = (gDvm.sumThreadSuspendCount != 0) || - (gDvmJit.codeCacheFull == true) || - (gDvmJit.pProfTable == NULL); + bool hideTranslation = dvmJitHideTranslation(); if (npc == dPC) { #if defined(WITH_JIT_TUNING) @@ -1209,7 +1221,8 @@ bool dvmJitCheckTraceRequest(Thread* self, InterpState* interpState) * the jitState is kJitDone when switchInterp is set to true. */ assert(switchInterp == false || interpState->jitState == kJitDone); - return switchInterp && !debugOrProfile; + return switchInterp && !debugOrProfile && + !dvmJitStayInPortableInterpreter(); } /* diff --git a/vm/mterp/armv5te/footer.S b/vm/mterp/armv5te/footer.S index 2ba5357dc..b69aef800 100644 --- a/vm/mterp/armv5te/footer.S +++ b/vm/mterp/armv5te/footer.S @@ -325,6 +325,10 @@ common_updateProfile: bl dvmCompilerGetInterpretTemplate cmp r0, r10 @ special case? bne jitSVShadowRunStart @ set up self verification shadow space + @ Need to clear the inJitCodeCache flag + ldr r10, [rGLUE, #offGlue_self] @ r10 <- glue->self + mov r3, #0 @ 0 means not in the JIT code cache + str r3, [r10, #offThread_inJitCodeCache] @ back to the interp land GET_INST_OPCODE(ip) GOTO_OPCODE(ip) /* no return */ diff --git a/vm/mterp/out/InterpAsm-armv4t.S b/vm/mterp/out/InterpAsm-armv4t.S index 3593a6e58..e31f96652 100644 --- a/vm/mterp/out/InterpAsm-armv4t.S +++ b/vm/mterp/out/InterpAsm-armv4t.S @@ -10173,6 +10173,10 @@ common_updateProfile: bl dvmCompilerGetInterpretTemplate cmp r0, r10 @ special case? bne jitSVShadowRunStart @ set up self verification shadow space + @ Need to clear the inJitCodeCache flag + ldr r10, [rGLUE, #offGlue_self] @ r10 <- glue->self + mov r3, #0 @ 0 means not in the JIT code cache + str r3, [r10, #offThread_inJitCodeCache] @ back to the interp land GET_INST_OPCODE(ip) GOTO_OPCODE(ip) /* no return */ diff --git a/vm/mterp/out/InterpAsm-armv5te-vfp.S b/vm/mterp/out/InterpAsm-armv5te-vfp.S index 1ae963693..84b42b20f 100644 --- a/vm/mterp/out/InterpAsm-armv5te-vfp.S +++ b/vm/mterp/out/InterpAsm-armv5te-vfp.S @@ -9711,6 +9711,10 @@ common_updateProfile: bl dvmCompilerGetInterpretTemplate cmp r0, r10 @ special case? bne jitSVShadowRunStart @ set up self verification shadow space + @ Need to clear the inJitCodeCache flag + ldr r10, [rGLUE, #offGlue_self] @ r10 <- glue->self + mov r3, #0 @ 0 means not in the JIT code cache + str r3, [r10, #offThread_inJitCodeCache] @ back to the interp land GET_INST_OPCODE(ip) GOTO_OPCODE(ip) /* no return */ diff --git a/vm/mterp/out/InterpAsm-armv5te.S b/vm/mterp/out/InterpAsm-armv5te.S index 9b083b2b2..9a3d34041 100644 --- a/vm/mterp/out/InterpAsm-armv5te.S +++ b/vm/mterp/out/InterpAsm-armv5te.S @@ -10169,6 +10169,10 @@ common_updateProfile: bl dvmCompilerGetInterpretTemplate cmp r0, r10 @ special case? bne jitSVShadowRunStart @ set up self verification shadow space + @ Need to clear the inJitCodeCache flag + ldr r10, [rGLUE, #offGlue_self] @ r10 <- glue->self + mov r3, #0 @ 0 means not in the JIT code cache + str r3, [r10, #offThread_inJitCodeCache] @ back to the interp land GET_INST_OPCODE(ip) GOTO_OPCODE(ip) /* no return */ diff --git a/vm/mterp/out/InterpAsm-armv7-a-neon.S b/vm/mterp/out/InterpAsm-armv7-a-neon.S index 173b478f1..6bd8e8d9f 100644 --- a/vm/mterp/out/InterpAsm-armv7-a-neon.S +++ b/vm/mterp/out/InterpAsm-armv7-a-neon.S @@ -9645,6 +9645,10 @@ common_updateProfile: bl dvmCompilerGetInterpretTemplate cmp r0, r10 @ special case? bne jitSVShadowRunStart @ set up self verification shadow space + @ Need to clear the inJitCodeCache flag + ldr r10, [rGLUE, #offGlue_self] @ r10 <- glue->self + mov r3, #0 @ 0 means not in the JIT code cache + str r3, [r10, #offThread_inJitCodeCache] @ back to the interp land GET_INST_OPCODE(ip) GOTO_OPCODE(ip) /* no return */ diff --git a/vm/mterp/out/InterpAsm-armv7-a.S b/vm/mterp/out/InterpAsm-armv7-a.S index 15e48a4b5..1aed05f83 100644 --- a/vm/mterp/out/InterpAsm-armv7-a.S +++ b/vm/mterp/out/InterpAsm-armv7-a.S @@ -9645,6 +9645,10 @@ common_updateProfile: bl dvmCompilerGetInterpretTemplate cmp r0, r10 @ special case? bne jitSVShadowRunStart @ set up self verification shadow space + @ Need to clear the inJitCodeCache flag + ldr r10, [rGLUE, #offGlue_self] @ r10 <- glue->self + mov r3, #0 @ 0 means not in the JIT code cache + str r3, [r10, #offThread_inJitCodeCache] @ back to the interp land GET_INST_OPCODE(ip) GOTO_OPCODE(ip) /* no return */ -- cgit v1.2.3