From 85547d94beb3e5c01b1b5dda2235344e2ab22dc1 Mon Sep 17 00:00:00 2001 From: Fiora Date: Fri, 26 Sep 2014 13:45:24 -0700 Subject: [PATCH] JIT: properly remove FIFO write addresses when code is invalidated Fixes a bug caused by interaction with carry optimizations; might fix other issues too. --- Source/Core/Common/BreakPoints.cpp | 8 ++++---- .../PowerPC/Interpreter/Interpreter_LoadStore.cpp | 6 +++--- Source/Core/Core/PowerPC/Jit64/Jit.cpp | 2 +- Source/Core/Core/PowerPC/JitCommon/JitCache.cpp | 12 +++++++++++- Source/Core/Core/PowerPC/JitCommon/JitCache.h | 2 +- Source/Core/Core/PowerPC/JitInterface.cpp | 6 +++--- Source/Core/Core/PowerPC/JitInterface.h | 3 ++- Source/Core/Core/PowerPC/PPCCache.cpp | 2 +- Source/Core/DolphinWX/Debugger/CodeWindow.cpp | 2 +- 9 files changed, 27 insertions(+), 16 deletions(-) diff --git a/Source/Core/Common/BreakPoints.cpp b/Source/Core/Common/BreakPoints.cpp index d23e1fa0e2..db0cedf0bb 100644 --- a/Source/Core/Common/BreakPoints.cpp +++ b/Source/Core/Common/BreakPoints.cpp @@ -66,7 +66,7 @@ void BreakPoints::Add(const TBreakPoint& bp) { m_BreakPoints.push_back(bp); if (jit) - jit->GetBlockCache()->InvalidateICache(bp.iAddress, 4); + jit->GetBlockCache()->InvalidateICache(bp.iAddress, 4, true); } } @@ -82,7 +82,7 @@ void BreakPoints::Add(u32 em_address, bool temp) m_BreakPoints.push_back(pt); if (jit) - jit->GetBlockCache()->InvalidateICache(em_address, 4); + jit->GetBlockCache()->InvalidateICache(em_address, 4, true); } } @@ -94,7 +94,7 @@ void BreakPoints::Remove(u32 em_address) { m_BreakPoints.erase(i); if (jit) - jit->GetBlockCache()->InvalidateICache(em_address, 4); + jit->GetBlockCache()->InvalidateICache(em_address, 4, true); return; } } @@ -106,7 +106,7 @@ void BreakPoints::Clear() { for (const TBreakPoint& bp : m_BreakPoints) { - jit->GetBlockCache()->InvalidateICache(bp.iAddress, 4); + jit->GetBlockCache()->InvalidateICache(bp.iAddress, 4, true); } } diff --git a/Source/Core/Core/PowerPC/Interpreter/Interpreter_LoadStore.cpp b/Source/Core/Core/PowerPC/Interpreter/Interpreter_LoadStore.cpp index 86d42e3239..5d247e93d5 100644 --- a/Source/Core/Core/PowerPC/Interpreter/Interpreter_LoadStore.cpp +++ b/Source/Core/Core/PowerPC/Interpreter/Interpreter_LoadStore.cpp @@ -327,7 +327,7 @@ void Interpreter::dcbf(UGeckoInstruction _inst) NPC = PC + 12; }*/ u32 address = Helper_Get_EA_X(_inst); - JitInterface::InvalidateICache(address & ~0x1f, 32); + JitInterface::InvalidateICache(address & ~0x1f, 32, false); } void Interpreter::dcbi(UGeckoInstruction _inst) @@ -335,7 +335,7 @@ void Interpreter::dcbi(UGeckoInstruction _inst) // Removes a block from data cache. Since we don't emulate the data cache, we don't need to do anything to the data cache // However, we invalidate the jit block cache on dcbi u32 address = Helper_Get_EA_X(_inst); - JitInterface::InvalidateICache(address & ~0x1f, 32); + JitInterface::InvalidateICache(address & ~0x1f, 32, false); // The following detects a situation where the game is writing to the dcache at the address being DMA'd. As we do not // have dcache emulation, invalid data is being DMA'd causing audio glitches. The following code detects this and @@ -359,7 +359,7 @@ void Interpreter::dcbst(UGeckoInstruction _inst) // Cache line flush. Since we don't emulate the data cache, we don't need to do anything. // Invalidate the jit block cache on dcbst in case new code has been loaded via the data cache u32 address = Helper_Get_EA_X(_inst); - JitInterface::InvalidateICache(address & ~0x1f, 32); + JitInterface::InvalidateICache(address & ~0x1f, 32, false); } void Interpreter::dcbt(UGeckoInstruction _inst) diff --git a/Source/Core/Core/PowerPC/Jit64/Jit.cpp b/Source/Core/Core/PowerPC/Jit64/Jit.cpp index 050151a611..45e2bf0922 100644 --- a/Source/Core/Core/PowerPC/Jit64/Jit.cpp +++ b/Source/Core/Core/PowerPC/Jit64/Jit.cpp @@ -159,7 +159,7 @@ bool Jit64::HandleFault(uintptr_t access_address, SContext* ctx) // CALLs, but we can't yet. Fake the downcount so we're forced to the // dispatcher (no block linking), and clear the cache so we're sent to // Jit. Yeah, it's kind of gross. - GetBlockCache()->InvalidateICache(0, 0xffffffff); + GetBlockCache()->InvalidateICache(0, 0xffffffff, true); CoreTiming::ForceExceptionCheck(0); m_clear_cache_asap = true; diff --git a/Source/Core/Core/PowerPC/JitCommon/JitCache.cpp b/Source/Core/Core/PowerPC/JitCommon/JitCache.cpp index d8fc87f449..c577ddaf3d 100644 --- a/Source/Core/Core/PowerPC/JitCommon/JitCache.cpp +++ b/Source/Core/Core/PowerPC/JitCommon/JitCache.cpp @@ -327,7 +327,7 @@ using namespace Gen; WriteDestroyBlock(b.checkedEntry, b.originalAddress); } - void JitBaseBlockCache::InvalidateICache(u32 address, const u32 length) + void JitBaseBlockCache::InvalidateICache(u32 address, const u32 length, bool forced) { // Convert the logical address to a physical address for the block map u32 pAddr = address & 0x1FFFFFFF; @@ -358,6 +358,16 @@ using namespace Gen; { block_map.erase(it1, it2); } + + // If the code was actually modified, we need to clear the relevant entries from the + // FIFO write address cache, so we don't end up with FIFO checks in places they shouldn't + // be (this can clobber flags, and thus break any optimization that relies on flags + // being in the right place between instructions). + if (!forced) + { + for (u32 i = address; i < address + length; i += 4) + jit->js.fifoWriteAddresses.erase(i); + } } } diff --git a/Source/Core/Core/PowerPC/JitCommon/JitCache.h b/Source/Core/Core/PowerPC/JitCommon/JitCache.h index 0d711dcfa2..9cc25b85bc 100644 --- a/Source/Core/Core/PowerPC/JitCommon/JitCache.h +++ b/Source/Core/Core/PowerPC/JitCommon/JitCache.h @@ -161,7 +161,7 @@ public: CompiledCode GetCompiledCodeFromBlock(int block_num); // DOES NOT WORK CORRECTLY WITH INLINING - void InvalidateICache(u32 address, const u32 length); + void InvalidateICache(u32 address, const u32 length, bool forced); void DestroyBlock(int block_num, bool invalidate); }; diff --git a/Source/Core/Core/PowerPC/JitInterface.cpp b/Source/Core/Core/PowerPC/JitInterface.cpp index a8986f7251..6a598a736a 100644 --- a/Source/Core/Core/PowerPC/JitInterface.cpp +++ b/Source/Core/Core/PowerPC/JitInterface.cpp @@ -210,10 +210,10 @@ namespace JitInterface jit->GetBlockCache()->Clear(); } - void InvalidateICache(u32 address, u32 size) + void InvalidateICache(u32 address, u32 size, bool forced) { if (jit) - jit->GetBlockCache()->InvalidateICache(address, size); + jit->GetBlockCache()->InvalidateICache(address, size, forced); } u32 ReadOpcodeJIT(u32 _Address) @@ -263,7 +263,7 @@ namespace JitInterface exception_addresses->insert(PC); // Invalidate the JIT block so that it gets recompiled with the external exception check included. - jit->GetBlockCache()->InvalidateICache(PC, 4); + jit->GetBlockCache()->InvalidateICache(PC, 4, true); } } } diff --git a/Source/Core/Core/PowerPC/JitInterface.h b/Source/Core/Core/PowerPC/JitInterface.h index 53b2b38312..3ed62edd40 100644 --- a/Source/Core/Core/PowerPC/JitInterface.h +++ b/Source/Core/Core/PowerPC/JitInterface.h @@ -36,7 +36,8 @@ namespace JitInterface void ClearSafe(); - void InvalidateICache(u32 address, u32 size); + // If "forced" is true, a recompile is being requested on code that hasn't been modified. + void InvalidateICache(u32 address, u32 size, bool forced); void CompileExceptionCheck(int type); diff --git a/Source/Core/Core/PowerPC/PPCCache.cpp b/Source/Core/Core/PowerPC/PPCCache.cpp index 93693dd7b3..7e96e64a45 100644 --- a/Source/Core/Core/PowerPC/PPCCache.cpp +++ b/Source/Core/Core/PowerPC/PPCCache.cpp @@ -95,7 +95,7 @@ namespace PowerPC lookup_table[((tags[set][i] << 7) | set) & 0xfffff] = 0xff; } valid[set] = 0; - JitInterface::InvalidateICache(addr & ~0x1f, 32); + JitInterface::InvalidateICache(addr & ~0x1f, 32, false); } u32 InstructionCache::ReadInstruction(u32 addr) diff --git a/Source/Core/DolphinWX/Debugger/CodeWindow.cpp b/Source/Core/DolphinWX/Debugger/CodeWindow.cpp index 194e72b11c..5abbfc6bc8 100644 --- a/Source/Core/DolphinWX/Debugger/CodeWindow.cpp +++ b/Source/Core/DolphinWX/Debugger/CodeWindow.cpp @@ -288,7 +288,7 @@ void CCodeWindow::SingleStep() { if (CCPU::IsStepping()) { - JitInterface::InvalidateICache(PC, 4); + JitInterface::InvalidateICache(PC, 4, true); CCPU::StepOpcode(&sync_event); wxThread::Sleep(20); // need a short wait here