From 0407f939fca233572bdbba400b3dbccc3c2b8058 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Tue, 23 Jul 2024 17:27:08 +1000 Subject: [PATCH] CPU: Fix loading recompiler-saved states with interpreter --- src/core/cpu_core.cpp | 147 ++++++++++++--------- src/core/cpu_core.h | 5 +- src/core/cpu_recompiler_code_generator.cpp | 2 +- src/core/imgui_overlays.cpp | 9 +- src/core/save_state_version.h | 2 +- src/core/system.cpp | 5 - 6 files changed, 97 insertions(+), 73 deletions(-) diff --git a/src/core/cpu_core.cpp b/src/core/cpu_core.cpp index 9f439aa29..a54bfd83d 100644 --- a/src/core/cpu_core.cpp +++ b/src/core/cpu_core.cpp @@ -27,6 +27,7 @@ Log_SetChannel(CPU::Core); namespace CPU { +static bool ShouldUseInterpreter(); static void SetPC(u32 new_pc); static void UpdateLoadDelay(); static void Branch(u32 target); @@ -66,7 +67,7 @@ static void LogInstruction(u32 bits, u32 pc, bool regs); static void HandleWriteSyscall(); static void HandlePutcSyscall(); static void HandlePutsSyscall(); -static void ExecuteDebug(); +[[noreturn]] static void ExecuteInterpreter(); template static void ExecuteInstruction(); @@ -161,7 +162,8 @@ void CPU::Initialize() // From nocash spec. g_state.cop0_regs.PRID = UINT32_C(0x00000002); - g_state.use_debug_dispatcher = false; + g_state.using_debug_dispatcher = false; + g_state.using_interpreter = ShouldUseInterpreter(); for (BreakpointList& bps : s_breakpoints) bps.clear(); s_breakpoint_counter = 1; @@ -200,6 +202,7 @@ void CPU::Reset() ClearICache(); UpdateMemoryPointers(); + UpdateDebugDispatcherFlag(); GTE::Reset(); @@ -276,8 +279,20 @@ bool CPU::DoState(StateWrapper& sw) sw.Do(&g_state.icache_data); } + bool using_interpreter = g_state.using_interpreter; + sw.DoEx(&using_interpreter, 67, g_state.using_interpreter); + if (sw.IsReading()) { + // Since the recompilers do not use npc/next_instruction, and the icache emulation doesn't actually fill the data, + // only the tags, if we save state with the recompiler, then load state with the interpreter, we're most likely + // going to crash. Clear both in the case that we are switching. + if (using_interpreter != g_state.using_interpreter) + { + WARNING_LOG("Current execution mode does not match save state. Resetting icache state."); + ExecutionModeChanged(); + } + UpdateMemoryPointers(); g_state.gte_completion_tick = 0; } @@ -285,6 +300,12 @@ bool CPU::DoState(StateWrapper& sw) return !sw.HasError(); } +ALWAYS_INLINE_RELEASE bool CPU::ShouldUseInterpreter() +{ + // Currently, any breakpoints require the interpreter. + return (g_settings.cpu_execution_mode == CPUExecutionMode::Interpreter || g_state.using_debug_dispatcher); +} + ALWAYS_INLINE_RELEASE void CPU::SetPC(u32 new_pc) { DebugAssert(Common::IsAlignedPow2(new_pc, 4)); @@ -1983,11 +2004,16 @@ bool CPU::UpdateDebugDispatcherFlag() const bool use_debug_dispatcher = has_any_breakpoints || has_cop0_breakpoints || s_trace_to_log || (g_settings.cpu_execution_mode == CPUExecutionMode::Interpreter && g_settings.bios_tty_logging); - if (use_debug_dispatcher == g_state.use_debug_dispatcher) + if (use_debug_dispatcher == g_state.using_debug_dispatcher) return false; DEV_LOG("{} debug dispatcher", use_debug_dispatcher ? "Now using" : "No longer using"); - g_state.use_debug_dispatcher = use_debug_dispatcher; + g_state.using_debug_dispatcher = use_debug_dispatcher; + + // Switching to interpreter? + if (g_state.using_interpreter != ShouldUseInterpreter()) + ExecutionModeChanged(); + return true; } @@ -2350,74 +2376,47 @@ template } } -void CPU::ExecuteDebug() +void CPU::ExecuteInterpreter() { - if (g_settings.gpu_pgxp_enable) + if (g_state.using_debug_dispatcher) { - if (g_settings.gpu_pgxp_cpu) - ExecuteImpl(); + if (g_settings.gpu_pgxp_enable) + { + if (g_settings.gpu_pgxp_cpu) + ExecuteImpl(); + else + ExecuteImpl(); + } else - ExecuteImpl(); + { + ExecuteImpl(); + } } else { - ExecuteImpl(); + if (g_settings.gpu_pgxp_enable) + { + if (g_settings.gpu_pgxp_cpu) + ExecuteImpl(); + else + ExecuteImpl(); + } + else + { + ExecuteImpl(); + } } } void CPU::Execute() { - const CPUExecutionMode exec_mode = g_settings.cpu_execution_mode; - const bool use_debug_dispatcher = g_state.use_debug_dispatcher; if (fastjmp_set(&s_jmp_buf) != 0) - { - // Before we return, set npc to pc so that we can switch from recs to int. - // We'll also need to fetch the next instruction to execute. - if (exec_mode != CPUExecutionMode::Interpreter && !use_debug_dispatcher) - { - if (!SafeReadInstruction(g_state.pc, &g_state.next_instruction.bits)) [[unlikely]] - { - g_state.next_instruction.bits = 0; - ERROR_LOG("Failed to read current instruction from 0x{:08X}", g_state.pc); - } - - g_state.npc = g_state.pc + sizeof(Instruction); - } - return; - } - if (use_debug_dispatcher) - { - ExecuteDebug(); - return; - } - - switch (exec_mode) - { - case CPUExecutionMode::Recompiler: - case CPUExecutionMode::CachedInterpreter: - case CPUExecutionMode::NewRec: - CodeCache::Execute(); - break; - - case CPUExecutionMode::Interpreter: - default: - { - if (g_settings.gpu_pgxp_enable) - { - if (g_settings.gpu_pgxp_cpu) - ExecuteImpl(); - else - ExecuteImpl(); - } - else - { - ExecuteImpl(); - } - } - break; - } + if (g_state.using_interpreter) + ExecuteInterpreter(); + else + CodeCache::Execute(); } void CPU::SetSingleStepFlag() @@ -2572,9 +2571,37 @@ void CPU::UpdateMemoryPointers() void CPU::ExecutionModeChanged() { + const bool prev_interpreter = g_state.using_interpreter; + + UpdateDebugDispatcherFlag(); + + // Clear out bus errors in case only memory exceptions are toggled on. g_state.bus_error = false; - if (UpdateDebugDispatcherFlag()) - System::InterruptExecution(); + + // Have to clear out the icache too, only the tags are valid in the recs. + ClearICache(); + + // Switching to interpreter? + g_state.using_interpreter = ShouldUseInterpreter(); + if (g_state.using_interpreter != prev_interpreter && !prev_interpreter) + { + // Before we return, set npc to pc so that we can switch from recs to int. + // We'll also need to fetch the next instruction to execute. + if (!SafeReadInstruction(g_state.pc, &g_state.next_instruction.bits)) [[unlikely]] + { + g_state.next_instruction.bits = 0; + ERROR_LOG("Failed to read current instruction from 0x{:08X}", g_state.pc); + } + + g_state.npc = g_state.pc + sizeof(Instruction); + } + + // Wipe out code cache when switching back to recompiler. + if (!g_state.using_interpreter && prev_interpreter) + CPU::CodeCache::Reset(); + + UpdateDebugDispatcherFlag(); + System::InterruptExecution(); } template diff --git a/src/core/cpu_core.h b/src/core/cpu_core.h index b4d539f4b..90362b0e2 100644 --- a/src/core/cpu_core.h +++ b/src/core/cpu_core.h @@ -110,8 +110,9 @@ struct State // GTE registers are stored here so we can access them on ARM with a single instruction GTE::Regs gte_regs = {}; - // 4 bytes of padding here on x64 - bool use_debug_dispatcher = false; + // 2 bytes of padding here on x64 + bool using_interpreter = false; + bool using_debug_dispatcher = false; void* fastmem_base = nullptr; void** memory_handlers = nullptr; diff --git a/src/core/cpu_recompiler_code_generator.cpp b/src/core/cpu_recompiler_code_generator.cpp index f9cc1b6f7..fc05b7a0d 100644 --- a/src/core/cpu_recompiler_code_generator.cpp +++ b/src/core/cpu_recompiler_code_generator.cpp @@ -2843,7 +2843,7 @@ bool CodeGenerator::Compile_cop0(Instruction instruction, const CodeCache::Instr // update dispatcher flag, if enabled, exit block EmitFunctionCall(nullptr, &UpdateDebugDispatcherFlag); - EmitLoadCPUStructField(dcic_value.GetHostRegister(), RegSize_8, OFFSETOF(State, use_debug_dispatcher)); + EmitLoadCPUStructField(dcic_value.GetHostRegister(), RegSize_8, OFFSETOF(State, using_debug_dispatcher)); EmitBranchIfBitClear(dcic_value.GetHostRegister(), RegSize_8, 0, ¬_enabled); m_register_cache.UninhibitAllocation(); diff --git a/src/core/imgui_overlays.cpp b/src/core/imgui_overlays.cpp index dcf1a5b9a..fabfd260a 100644 --- a/src/core/imgui_overlays.cpp +++ b/src/core/imgui_overlays.cpp @@ -6,6 +6,7 @@ #include "imgui_overlays.h" #include "cdrom.h" #include "controller.h" +#include "cpu_core_private.h" #include "dma.h" #include "fullscreen_ui.h" #include "gpu.h" @@ -320,9 +321,9 @@ void ImGuiManager::DrawPerformanceOverlay() System::GetMaximumFrameTime()); DRAW_LINE(fixed_font, text, IM_COL32(255, 255, 255, 255)); - if (g_settings.cpu_overclock_active || - (g_settings.cpu_execution_mode != CPUExecutionMode::Recompiler || g_settings.cpu_recompiler_icache || - g_settings.cpu_recompiler_memory_exceptions)) + if (g_settings.cpu_overclock_active || CPU::g_state.using_interpreter || + g_settings.cpu_execution_mode != CPUExecutionMode::Recompiler || g_settings.cpu_recompiler_icache || + g_settings.cpu_recompiler_memory_exceptions) { first = true; text.assign("CPU["); @@ -331,7 +332,7 @@ void ImGuiManager::DrawPerformanceOverlay() text.append_format("{}", g_settings.GetCPUOverclockPercent()); first = false; } - if (g_settings.cpu_execution_mode == CPUExecutionMode::Interpreter) + if (CPU::g_state.using_interpreter) { text.append_format("{}{}", first ? "" : "/", "I"); first = false; diff --git a/src/core/save_state_version.h b/src/core/save_state_version.h index 70776d45a..0275e5d51 100644 --- a/src/core/save_state_version.h +++ b/src/core/save_state_version.h @@ -5,7 +5,7 @@ #include "types.h" static constexpr u32 SAVE_STATE_MAGIC = 0x43435544; -static constexpr u32 SAVE_STATE_VERSION = 66; +static constexpr u32 SAVE_STATE_VERSION = 67; static constexpr u32 SAVE_STATE_MINIMUM_VERSION = 42; static_assert(SAVE_STATE_VERSION >= SAVE_STATE_MINIMUM_VERSION); diff --git a/src/core/system.cpp b/src/core/system.cpp index 34625cd7e..33a0b5281 100644 --- a/src/core/system.cpp +++ b/src/core/system.cpp @@ -4012,7 +4012,6 @@ void System::CheckForSettingsChanges(const Settings& old_settings) CPU::CodeCache::Shutdown(); if (g_settings.cpu_execution_mode != CPUExecutionMode::Interpreter) CPU::CodeCache::Initialize(); - CPU::ClearICache(); } if (CPU::CodeCache::IsUsingAnyRecompiler() && @@ -4025,10 +4024,6 @@ void System::CheckForSettingsChanges(const Settings& old_settings) TRANSLATE_STR("OSDMessage", "Recompiler options changed, flushing all blocks."), Host::OSD_INFO_DURATION); CPU::ExecutionModeChanged(); - CPU::CodeCache::Reset(); - - if (g_settings.cpu_recompiler_icache != old_settings.cpu_recompiler_icache) - CPU::ClearICache(); } else if (g_settings.cpu_execution_mode == CPUExecutionMode::Interpreter && g_settings.bios_tty_logging != old_settings.bios_tty_logging)