From 464aeecef8861285e04d27873fdf5ebaaab57bdd Mon Sep 17 00:00:00 2001 From: Gregory Hainaut Date: Fri, 23 Oct 2015 19:33:51 +0200 Subject: [PATCH 1/5] EE-rec: use uptr for function pointer Avoid potential issue on 64 bits port --- pcsx2/x86/BaseblockEx.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pcsx2/x86/BaseblockEx.h b/pcsx2/x86/BaseblockEx.h index a564665610..eb0eec5a4e 100644 --- a/pcsx2/x86/BaseblockEx.h +++ b/pcsx2/x86/BaseblockEx.h @@ -22,7 +22,7 @@ // addressable memory. Yay! struct BASEBLOCK { - u32 m_pFnptr; + uptr m_pFnptr; const __inline uptr GetFnptr() const { return m_pFnptr; } void __inline SetFnptr( uptr ptr ) { m_pFnptr = ptr; } From 40b5195f0eef40071771d93f9d76cd4cef51853f Mon Sep 17 00:00:00 2001 From: Gregory Hainaut Date: Fri, 23 Oct 2015 19:35:38 +0200 Subject: [PATCH 2/5] EE-rec: don't save useless variable the 2 static variables are only used to debug stack issue. It saves 2 move instructions by block --- pcsx2/x86/ix86-32/iR5900-32.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pcsx2/x86/ix86-32/iR5900-32.cpp b/pcsx2/x86/ix86-32/iR5900-32.cpp index 664f98216e..6a8f566d84 100644 --- a/pcsx2/x86/ix86-32/iR5900-32.cpp +++ b/pcsx2/x86/ix86-32/iR5900-32.cpp @@ -505,8 +505,10 @@ static DynGenFunc* _DynGen_EnterRecompiledCode() xMOV( ptr32[esp+0x08+cdecl_reserve], ebp ); xLEA( ebp, ptr32[esp+0x08+cdecl_reserve] ); - xMOV( ptr[&s_store_esp], esp ); - xMOV( ptr[&s_store_ebp], ebp ); + if (EmuConfig.Cpu.Recompiler.StackFrameChecks) { + xMOV( ptr[&s_store_esp], esp ); + xMOV( ptr[&s_store_ebp], ebp ); + } xJMP( DispatcherReg ); From ccea7645561448366156c674a96d3a4421b38999 Mon Sep 17 00:00:00 2001 From: Gregory Hainaut Date: Fri, 23 Oct 2015 19:39:39 +0200 Subject: [PATCH 3/5] EE-rec: Don't jump directly to C++ function On linux, it breaks the 16B stack alignment requirement. 2 dispatchers were added to handle the call to the function. It avoid any performance impact and remove the extra inlined asm Fix #506 --- pcsx2/x86/ix86-32/iR5900-32.cpp | 48 ++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/pcsx2/x86/ix86-32/iR5900-32.cpp b/pcsx2/x86/ix86-32/iR5900-32.cpp index 6a8f566d84..ab9590173d 100644 --- a/pcsx2/x86/ix86-32/iR5900-32.cpp +++ b/pcsx2/x86/ix86-32/iR5900-32.cpp @@ -361,6 +361,8 @@ void recCall( void (*func)() ) // ===================================================================================================== static void __fastcall recRecompile( const u32 startpc ); +static void __fastcall dyna_block_discard(u32 start,u32 sz); +static void __fastcall dyna_page_reset(u32 start,u32 sz); static u32 s_store_ebp, s_store_esp; @@ -375,6 +377,8 @@ static DynGenFunc* JITCompile = NULL; static DynGenFunc* JITCompileInBlock = NULL; static DynGenFunc* EnterRecompiledCode = NULL; static DynGenFunc* ExitRecompiledCode = NULL; +static DynGenFunc* DispatchBlockDiscard = NULL; +static DynGenFunc* DispatchPageReset = NULL; static void recEventTest() { @@ -533,6 +537,22 @@ static DynGenFunc* _DynGen_EnterRecompiledCode() return (DynGenFunc*)retval; } +static DynGenFunc* _DynGen_DispatchBlockDiscard() +{ + u8* retval = xGetPtr(); + xCALL(dyna_block_discard); + xJMP(ExitRecompiledCode); + return (DynGenFunc*)retval; +} + +static DynGenFunc* _DynGen_DispatchPageReset() +{ + u8* retval = xGetPtr(); + xCALL(dyna_page_reset); + xJMP(ExitRecompiledCode); + return (DynGenFunc*)retval; +} + static void _DynGen_Dispatchers() { // In case init gets called multiple times: @@ -549,9 +569,11 @@ static void _DynGen_Dispatchers() xCALL( recEventTest ); DispatcherReg = _DynGen_DispatcherReg(); - JITCompile = _DynGen_JITCompile(); - JITCompileInBlock = _DynGen_JITCompileInBlock(); - EnterRecompiledCode = _DynGen_EnterRecompiledCode(); + JITCompile = _DynGen_JITCompile(); + JITCompileInBlock = _DynGen_JITCompileInBlock(); + EnterRecompiledCode = _DynGen_EnterRecompiledCode(); + DispatchBlockDiscard = _DynGen_DispatchBlockDiscard(); + DispatchPageReset = _DynGen_DispatchPageReset(); HostSys::MemProtectStatic( eeRecDispatchers, PageAccess_ExecOnly() ); @@ -561,7 +583,6 @@ static void _DynGen_Dispatchers() ////////////////////////////////////////////////////////////////////////////////////////// // -static void __fastcall dyna_block_discard(u32 start,u32 sz); static __ri void ClearRecLUT(BASEBLOCK* base, int memsize) { @@ -1625,15 +1646,6 @@ void __fastcall dyna_block_discard(u32 start,u32 sz) { eeRecPerfLog.Write( Color_StrongGray, "Clearing Manual Block @ 0x%08X [size=%d]", start, sz*4); recClear(start, sz); - - // Stack trick: This function was invoked via a direct jmp, so manually pop the - // EBP/stackframe before issuing a RET, else esp/ebp will be incorrect. - -#ifdef _MSC_VER - __asm leave __asm jmp [ExitRecompiledCode] -#else - __asm__ __volatile__( "leave\n jmp *%[exitRec]\n" : : [exitRec] "m" (ExitRecompiledCode) : ); -#endif } // called when a page under manual protection has been run enough times to be a candidate @@ -1644,12 +1656,6 @@ void __fastcall dyna_page_reset(u32 start,u32 sz) recClear(start & ~0xfffUL, 0x400); manual_counter[start >> 12]++; mmap_MarkCountedRamPage( start ); - -#ifdef _MSC_VER - __asm leave __asm jmp [ExitRecompiledCode] -#else - __asm__ __volatile__( "leave\n jmp *%[exitRec]\n" : : [exitRec] "m" (ExitRecompiledCode) : ); -#endif } // Skip MPEG Game-Fix @@ -2075,7 +2081,7 @@ StartRecomp: while(stg>0) { xCMP( ptr32[PSM(lpc)], *(u32*)PSM(lpc) ); - xJNE( dyna_block_discard ); + xJNE(DispatchBlockDiscard); stg -= 4; lpc += 4; @@ -2108,7 +2114,7 @@ StartRecomp: // that the current amount of recompilation is fairly cheap). xADD(ptr16[&manual_page[inpage_ptr >> 12]], sz); - xJC( dyna_page_reset ); + xJC(DispatchPageReset); // note: clearcnt is measured per-page, not per-block! ConsoleColorScope cs( Color_Gray ); From b07621f1a1c4f1a639bb0c4faeae6d0deccb8cc1 Mon Sep 17 00:00:00 2001 From: Gregory Hainaut Date: Fri, 23 Oct 2015 19:44:42 +0200 Subject: [PATCH 4/5] cmake: drop ASAN workaround It was used to mask stack issues. It seems to be fixed with previous commits. I managed to boot a game without any crash ^^ --- cmake/BuildParameters.cmake | 3 --- 1 file changed, 3 deletions(-) diff --git a/cmake/BuildParameters.cmake b/cmake/BuildParameters.cmake index 66e1f91371..7354d112e9 100644 --- a/cmake/BuildParameters.cmake +++ b/cmake/BuildParameters.cmake @@ -264,9 +264,6 @@ endif() if (USE_ASAN) set(ASAN_FLAG "-fsanitize=address -fno-omit-frame-pointer ${DBG} -DASAN_WORKAROUND") - if(${PCSX2_TARGET_ARCHITECTURES} MATCHES "i386") - set(ASAN_FLAG "${ASAN_FLAG} -mpreferred-stack-boundary=4 -mincoming-stack-boundary=2") - endif() else() set(ASAN_FLAG "") endif() From 1f2d95db7e1ecac6f27e23f2b28790e18976790f Mon Sep 17 00:00:00 2001 From: Gregory Hainaut Date: Fri, 23 Oct 2015 22:14:57 +0200 Subject: [PATCH 5/5] cmake: restore omit frame pointer optimization Crash was related to unaligned stack in dyna_page_reset/dyna_block_discard --- pcsx2/CMakeLists.txt | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pcsx2/CMakeLists.txt b/pcsx2/CMakeLists.txt index ec5239ba0d..119b1fcf78 100644 --- a/pcsx2/CMakeLists.txt +++ b/pcsx2/CMakeLists.txt @@ -13,12 +13,6 @@ endif() set(CommonFlags - # GCC-4.6 crash pcsx2 during the binding of plugins at startup... - # Disable this optimization for the moment - # GCC-4.9 update: - # Crash when you start a game. Likely a stack corruption/alignment - -fno-omit-frame-pointer - # END GCC-4.6 -fno-strict-aliasing -Wno-parentheses -Wstrict-aliasing # Allow to track strict aliasing issue.