From 292f7e316d04016222e6ea4e3781deccfceb3ce5 Mon Sep 17 00:00:00 2001 From: Andreas Gal Date: Mon, 31 Aug 2009 16:35:50 -0700 Subject: [PATCH] Don't restore FP twice when exiting a fragment (513787, r=rreitmai). --- js/src/nanojit/Assembler.cpp | 7 +++---- js/src/nanojit/Assembler.h | 1 + js/src/nanojit/NativeARM.cpp | 20 +++++++++----------- js/src/nanojit/NativePPC.cpp | 9 +++++---- js/src/nanojit/NativeSparc.cpp | 11 +++++++---- js/src/nanojit/NativeX64.cpp | 11 +++++++---- js/src/nanojit/Nativei386.cpp | 25 ++++++++++++++++--------- 7 files changed, 48 insertions(+), 36 deletions(-) diff --git a/js/src/nanojit/Assembler.cpp b/js/src/nanojit/Assembler.cpp index bb9cf4700a9..35dc2e1c9e6 100644 --- a/js/src/nanojit/Assembler.cpp +++ b/js/src/nanojit/Assembler.cpp @@ -105,6 +105,7 @@ namespace nanojit verbose_only( _logc = logc; ) verbose_only( _outputCache = 0; ) verbose_only( outlineEOL[0] = '\0'; ) + verbose_only( outputAddr = false; ) reset(); } @@ -538,7 +539,6 @@ namespace nanojit swapptrs(); _inExit = true; - //verbose_only( verbose_outputf(" LIR_xend swapptrs, _nIns is now %08X(%08X), _nExitIns is now %08X(%08X)",_nIns, *_nIns,_nExitIns,*_nExitIns) ); debug_only( _sv_fpuStkDepth = _fpuStkDepth; _fpuStkDepth = 0; ) nFragExit(guard); @@ -612,11 +612,10 @@ namespace nanojit _stats.codeExitStart = _nExitIns-1; #endif /* PERFM */ - _epilogue = genEpilogue(); _branchStateMap = branchStateMap; + _epilogue = NULL; - verbose_only( outputAddr=true; ) - verbose_only( asm_output("[epilogue]"); ) + nBeginAssembly(); } void Assembler::assemble(Fragment* frag) diff --git a/js/src/nanojit/Assembler.h b/js/src/nanojit/Assembler.h index 6d2292c63b4..6710f6e30c7 100644 --- a/js/src/nanojit/Assembler.h +++ b/js/src/nanojit/Assembler.h @@ -301,6 +301,7 @@ namespace nanojit // platform specific implementation (see NativeXXX.cpp file) void nInit(AvmCore *); + void nBeginAssembly(); Register nRegisterAllocFromSet(RegisterMask set); void nRegisterResetAll(RegAlloc& a); static void nPatchBranch(NIns* branch, NIns* location); diff --git a/js/src/nanojit/NativeARM.cpp b/js/src/nanojit/NativeARM.cpp index 2744cc01686..6b5e728e288 100644 --- a/js/src/nanojit/NativeARM.cpp +++ b/js/src/nanojit/NativeARM.cpp @@ -505,7 +505,10 @@ Assembler::nFragExit(LInsp guard) // The target doesn't exit yet, so emit a jump to the epilogue. If the // target is created later on, the jump will be patched. - GuardRecord * gr = guard->record(); + GuardRecord *gr = guard->record(); + + if (!_epilogue) + _epilogue = genEpilogue(); // Jump to the epilogue. This may get patched later, but JMP_far always // emits two instructions even when only one is required, so patching @@ -549,13 +552,6 @@ Assembler::genEpilogue() POP_mask(savingMask); // regs - // Pop the stack frame. - // As far as I can tell, the generated code doesn't use the stack between - // popping the stack frame in nFragExit and getting here and so this MOV - // should be redundant. However, removing this seems to break some regular - // expression stuff. - MOV(SP,FP); - // nFragExit loads the guard record pointer into R2, but we need it in R0 // so it must be moved here. MOV(R0,R2); // return GuardRecord* @@ -2360,9 +2356,11 @@ Assembler::asm_int(LInsp ins) void Assembler::asm_ret(LIns *ins) { - if (_nIns != _epilogue) { - B(_epilogue); - } + genEpilogue(); + + // Pop the stack frame. + MOV(SP,FP); + assignSavedRegs(); LIns *value = ins->oprnd1(); if (ins->isop(LIR_ret)) { diff --git a/js/src/nanojit/NativePPC.cpp b/js/src/nanojit/NativePPC.cpp index 12f72e8b39d..bc142d6ee71 100644 --- a/js/src/nanojit/NativePPC.cpp +++ b/js/src/nanojit/NativePPC.cpp @@ -118,7 +118,6 @@ namespace nanojit } NIns* Assembler::genEpilogue() { - max_param_size = 0; BLR(); MTLR(R0); LP(R0, lr_offset, SP); @@ -563,9 +562,7 @@ namespace nanojit } void Assembler::asm_ret(LIns *ins) { - UNLESS_PEDANTIC( if (_nIns != _epilogue) ) { - br(_epilogue, 0); - } + genEpilogue(); assignSavedParams(); LIns *value = ins->oprnd1(); Register r = ins->isop(LIR_ret) ? R3 : F1; @@ -1179,6 +1176,10 @@ namespace nanojit void Assembler::nInit(AvmCore*) { } + void Assembler::nBeginAssembly() { + max_param_size = 0; + } + void Assembler::nativePageSetup() { if (!_nIns) { codeAlloc(); diff --git a/js/src/nanojit/NativeSparc.cpp b/js/src/nanojit/NativeSparc.cpp index dddc8c1a460..2ac917365ca 100644 --- a/js/src/nanojit/NativeSparc.cpp +++ b/js/src/nanojit/NativeSparc.cpp @@ -76,6 +76,9 @@ namespace nanojit has_cmov = true; } + void Assembler::nBeginAssembly() { + } + NIns* Assembler::genPrologue() { /** @@ -126,7 +129,9 @@ namespace nanojit } else { - // target doesn't exit yet. emit jump to epilog, and set up to patch later. + // Target doesn't exit yet. Emit jump to epilog, and set up to patch later. + if (!_epilogue) + _epilogue = genEpilogue(); lr = guard->record(); JMP_long((intptr_t)_epilogue); lr->jmp = _nIns; @@ -1042,9 +1047,7 @@ namespace nanojit void Assembler::asm_ret(LInsp ins) { - if (_nIns != _epilogue) { - JMP(_epilogue); - } + genEpilogue(); assignSavedRegs(); LIns *val = ins->oprnd1(); if (ins->isop(LIR_ret)) { diff --git a/js/src/nanojit/NativeX64.cpp b/js/src/nanojit/NativeX64.cpp index 8857383b256..af366dbaebe 100644 --- a/js/src/nanojit/NativeX64.cpp +++ b/js/src/nanojit/NativeX64.cpp @@ -898,7 +898,7 @@ namespace nanojit } void Assembler::asm_ret(LIns *ins) { - JMP(_epilogue); + genEpilogue(); assignSavedRegs(); LIns *value = ins->oprnd1(); Register r = ins->isop(LIR_ret) ? RAX : XMM0; @@ -1169,7 +1169,6 @@ namespace nanojit // mov rsp, rbp // pop rbp // ret - max_stk_used = 0; emit(X64_ret); emitr(X64_popr, RBP); MR(RSP, RBP); @@ -1234,8 +1233,12 @@ namespace nanojit TODO(nFragExit); } - void Assembler::nInit(AvmCore*) - {} + void Assembler::nInit(AvmCore*) { + } + + void Assembler::nBeginAssembly() { + max_stk_used = 0; + } void Assembler::underrunProtect(ptrdiff_t bytes) { NanoAssertMsg(bytes<=LARGEST_UNDERRUN_PROT, "constant LARGEST_UNDERRUN_PROT is too small"); diff --git a/js/src/nanojit/Nativei386.cpp b/js/src/nanojit/Nativei386.cpp index cf736fd11d3..c171f6ef9f2 100644 --- a/js/src/nanojit/Nativei386.cpp +++ b/js/src/nanojit/Nativei386.cpp @@ -83,6 +83,9 @@ namespace nanojit OSDep::getDate(); } + void Assembler::nBeginAssembly() { + } + NIns* Assembler::genPrologue() { /** @@ -119,12 +122,15 @@ namespace nanojit Fragment *frag = exit->target; GuardRecord *lr = 0; bool destKnown = (frag && frag->fragEntry); + // Generate jump to epilog and initialize lr. // If the guard is LIR_xtbl, use a jump table with epilog in every entry if (guard->isop(LIR_xtbl)) { lr = guard->record(); Register r = EDX; SwitchInfo* si = guard->record()->exit->switchInfo; + if (!_epilogue) + _epilogue = genEpilogue(); emitJumpTable(si, _epilogue); JMP_indirect(r); LEAmi4(r, si->table, r); @@ -133,14 +139,16 @@ namespace nanojit if (destKnown && !trees) { JMP(frag->fragEntry); lr = 0; - } else { // target doesn't exist. Use 0 jump offset and patch later + } else { // Target doesn't exist. Jump to an epilogue for now. This can be patched later. + if (!_epilogue) + _epilogue = genEpilogue(); lr = guard->record(); JMP_long(_epilogue); lr->jmp = _nIns; } } - // first restore ESP from EBP, undoing SUBi(SP,amt) from genPrologue + // Restore ESP from EBP, undoing SUBi(SP,amt) in the prologue MR(SP,FP); // return value is GuardRecord* @@ -150,9 +158,8 @@ namespace nanojit NIns *Assembler::genEpilogue() { RET(); - POPr(FP); // Restore caller's FP. - MR(SP,FP); // pop the stack frame + return _nIns; } @@ -1670,9 +1677,11 @@ namespace nanojit void Assembler::asm_ret(LInsp ins) { - if (_nIns != _epilogue) { - JMP(_epilogue); - } + genEpilogue(); + + // Restore ESP from EBP, undoing SUBi(SP,amt) in the prologue + MR(SP,FP); + assignSavedRegs(); LIns *val = ins->oprnd1(); if (ins->isop(LIR_ret)) { @@ -1688,7 +1697,5 @@ namespace nanojit TODO(asm_promote); } - - #endif /* FEATURE_NANOJIT */ }