From 3a5968ba331845539e21e408ec0fb729de4ff465 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 3 Nov 2024 20:49:20 +0100 Subject: [PATCH] Don't block the render thread while the CPU is paused. This is a prereq for imgui debuggers. --- Core/Core.cpp | 141 +++++++++--------- Core/Core.h | 14 +- Core/Debugger/WebSocket/CPUCoreSubscriber.cpp | 2 +- .../Debugger/WebSocket/SteppingSubscriber.cpp | 7 +- Core/SaveState.cpp | 1 - Core/System.cpp | 6 +- GPU/Debugger/Stepping.cpp | 4 +- Windows/Debugger/Debugger_Disasm.cpp | 2 +- 8 files changed, 88 insertions(+), 89 deletions(-) diff --git a/Core/Core.cpp b/Core/Core.cpp index bac21f14db..c93b93eee4 100644 --- a/Core/Core.cpp +++ b/Core/Core.cpp @@ -35,6 +35,7 @@ #include "Core/Core.h" #include "Core/Config.h" #include "Core/MemMap.h" +#include "Core/MIPS/MIPSDebugInterface.h" #include "Core/SaveState.h" #include "Core/System.h" #include "Core/MemFault.h" @@ -51,16 +52,33 @@ #include "Windows/InputDevice.h" #endif -static std::condition_variable m_StepCond; -static std::mutex m_hStepMutex; +// Step command to execute next +static std::mutex g_stepMutex; +struct StepCommand { + CPUStepType type; + int param; + const char *reason; + u32 relatedAddr; + bool empty() const { + return type == CPUStepType::None; + } + void clear() { + type = CPUStepType::None; + param = 0; + reason = ""; + relatedAddr = 0; + } +}; +static StepCommand g_stepCommand; + +// This is so that external threads can wait for the CPU to become inactive. static std::condition_variable m_InactiveCond; static std::mutex m_hInactiveMutex; -static int g_singleStepsPending = 0; + static int steppingCounter = 0; -static const char *steppingReason = ""; -static uint32_t steppingAddress = 0; static std::set lifecycleFuncs; static std::set stopFuncs; + static bool windowHidden = false; static bool powerSaving = false; @@ -126,14 +144,6 @@ bool Core_IsInactive() { return coreState != CORE_RUNNING && coreState != CORE_NEXTFRAME && !coreStatePending; } -static inline void Core_StateProcessed() { - if (coreStatePending) { - std::lock_guard guard(m_hInactiveMutex); - coreStatePending = false; - m_InactiveCond.notify_all(); - } -} - void Core_WaitInactive() { while (Core_IsActive() && !GPUStepping::IsStepping()) { std::unique_lock guard(m_hInactiveMutex); @@ -227,19 +237,23 @@ void Core_RunLoop(GraphicsContext *ctx) { NativeFrame(ctx); } -void Core_DoSingleStep() { - std::lock_guard guard(m_hStepMutex); - g_singleStepsPending++; - m_StepCond.notify_all(); -} - -void Core_UpdateSingleStep() { - std::lock_guard guard(m_hStepMutex); - m_StepCond.notify_all(); +bool Core_RequestSingleStep(CPUStepType type, int stepSize) { + std::lock_guard guard(g_stepMutex); + if (g_stepCommand.type != CPUStepType::None) { + ERROR_LOG(Log::CPU, "Can't submit two steps in one frame"); + return false; + } + g_stepCommand = { type, stepSize }; + return true; } // See comment in header. -void Core_PerformStep(DebugInterface *cpu, CPUStepType stepType, int stepSize) { +// Handles more advanced step types (used by the debugger). +// stepSize is to support stepping through compound instructions like fused lui+ladd (li). +// Yes, our disassembler does support those. +// Doesn't return the new address, as that's just mips->getPC(). +// Internal use. +static void Core_PerformStep(MIPSDebugInterface *cpu, CPUStepType stepType, int stepSize) { switch (stepType) { case CPUStepType::Into: { @@ -247,8 +261,8 @@ void Core_PerformStep(DebugInterface *cpu, CPUStepType stepType, int stepSize) { u32 newAddress = currentPc + stepSize; // If the current PC is on a breakpoint, the user still wants the step to happen. CBreakPoints::SetSkipFirst(currentPc); - for (int i = 0; i < (newAddress - currentPc) / 4; i++) { - Core_DoSingleStep(); + for (int i = 0; i < (int)(newAddress - currentPc) / 4; i++) { + currentMIPS->SingleStep(); } return; } @@ -316,22 +330,8 @@ void Core_PerformStep(DebugInterface *cpu, CPUStepType stepType, int stepSize) { } } -static inline int Core_WaitStepping() { - std::unique_lock guard(m_hStepMutex); - // We only wait 16ms so that we can still draw UI or react to events. - double sleepStart = time_now_d(); - if (!g_singleStepsPending && coreState == CORE_STEPPING) - m_StepCond.wait_for(guard, std::chrono::milliseconds(16)); - double sleepEnd = time_now_d(); - DisplayNotifySleep(sleepEnd - sleepStart); - - int result = g_singleStepsPending; - g_singleStepsPending = 0; - return result; -} - -void Core_ProcessStepping() { - Core_StateProcessed(); +void Core_ProcessStepping(MIPSDebugInterface *cpu) { + coreStatePending = false; // Check if there's any pending save state actions. SaveState::Process(); @@ -352,21 +352,26 @@ void Core_ProcessStepping() { } // Need to check inside the lock to avoid races. - int doSteps = Core_WaitStepping(); + std::lock_guard guard(g_stepMutex); - // We may still be stepping without singleStepPending to process a save state. - if (doSteps && coreState == CORE_STEPPING) { - Core_ResetException(); - - for (int i = 0; i < doSteps; i++) { - currentMIPS->SingleStep(); - steppingCounter++; - } - - // Update disasm dialog. - System_Notify(SystemNotification::DISASSEMBLY_AFTERSTEP); - System_Notify(SystemNotification::MEM_VIEW); + if (coreState != CORE_STEPPING || g_stepCommand.empty()) { + return; } + + Core_ResetException(); + + if (!g_stepCommand.empty()) { + Core_PerformStep(cpu, g_stepCommand.type, g_stepCommand.param); + if (g_stepCommand.type == CPUStepType::Into) { + // We're already done. The other step types will resume the CPU. + System_Notify(SystemNotification::DISASSEMBLY_AFTERSTEP); + } + g_stepCommand.clear(); + steppingCounter++; + } + + // Update disasm dialog. + System_Notify(SystemNotification::MEM_VIEW); } // Many platforms, like Android, do not call this function but handle things on their own. @@ -375,7 +380,6 @@ bool Core_Run(GraphicsContext *ctx) { System_Notify(SystemNotification::DISASSEMBLY); while (true) { if (GetUIState() != UISTATE_INGAME) { - Core_StateProcessed(); if (GetUIState() == UISTATE_EXIT) { // Not sure why we do a final frame here? NativeFrame(ctx); @@ -388,11 +392,9 @@ bool Core_Run(GraphicsContext *ctx) { switch (coreState) { case CORE_RUNNING: case CORE_STEPPING: - Core_StateProcessed(); // enter a fast runloop Core_RunLoop(ctx); if (coreState == CORE_POWERDOWN) { - Core_StateProcessed(); return true; } break; @@ -402,7 +404,6 @@ bool Core_Run(GraphicsContext *ctx) { case CORE_BOOT_ERROR: case CORE_RUNTIME_ERROR: // Exit loop!! - Core_StateProcessed(); return true; case CORE_NEXTFRAME: @@ -411,27 +412,30 @@ bool Core_Run(GraphicsContext *ctx) { } } +// Free-threaded (hm, possibly except tracing). void Core_Break(const char *reason, u32 relatedAddress) { // Stop the tracer mipsTracer.stop_tracing(); - Core_UpdateState(CORE_STEPPING); - steppingCounter++; - _assert_msg_(reason != nullptr, "No reason specified for break"); - steppingReason = reason; - steppingAddress = relatedAddress; + { + std::lock_guard lock(g_stepMutex); + steppingCounter++; + _assert_msg_(reason != nullptr, "No reason specified for break"); + + Core_UpdateState(CORE_STEPPING); + } System_Notify(SystemNotification::DEBUG_MODE_CHANGE); } +// Free-threaded (or at least should be) void Core_Resume() { // Clear the exception if we resume. Core_ResetException(); coreState = CORE_RUNNING; - coreStatePending = false; - m_StepCond.notify_all(); System_Notify(SystemNotification::DEBUG_MODE_CHANGE); } +// Should be called from the EmuThread. bool Core_NextFrame() { if (coreState == CORE_RUNNING) { coreState = CORE_NEXTFRAME; @@ -447,8 +451,11 @@ int Core_GetSteppingCounter() { SteppingReason Core_GetSteppingReason() { SteppingReason r; - r.reason = steppingReason; - r.relatedAddress = steppingAddress; + std::lock_guard lock(g_stepMutex); + if (!g_stepCommand.empty()) { + r.reason = g_stepCommand.reason; + r.relatedAddress = g_stepCommand.relatedAddr; + } return r; } diff --git a/Core/Core.h b/Core/Core.h index 306d522819..7c8886304a 100644 --- a/Core/Core.h +++ b/Core/Core.h @@ -24,7 +24,6 @@ #include "Core/CoreParameter.h" class GraphicsContext; -class DebugInterface; // called from emu thread void UpdateRunLoop(GraphicsContext *ctx); @@ -53,16 +52,9 @@ void Core_Break(const char *reason, u32 relatedAddress = 0); // void Core_Step(CPUStepType type); // CPUStepType::None not allowed void Core_Resume(); -// Handles more advanced step types (used by the debugger). -// stepSize is to support stepping through compound instructions like fused lui+ladd (li). -// Yes, our disassembler does support those. -// Doesn't return the new address, as that's just mips->getPC(). -void Core_PerformStep(DebugInterface *cpu, CPUStepType stepType, int stepSize); - -// Refactor. -void Core_DoSingleStep(); -void Core_UpdateSingleStep(); -void Core_ProcessStepping(); +// This should be called externally. +// Can fail if another step type was requested this frame. +bool Core_RequestSingleStep(CPUStepType stepType, int stepSize); bool Core_ShouldRunBehind(); bool Core_MustRunBehind(); diff --git a/Core/Debugger/WebSocket/CPUCoreSubscriber.cpp b/Core/Debugger/WebSocket/CPUCoreSubscriber.cpp index 4d32dd82fc..71d7e0bc07 100644 --- a/Core/Debugger/WebSocket/CPUCoreSubscriber.cpp +++ b/Core/Debugger/WebSocket/CPUCoreSubscriber.cpp @@ -90,7 +90,7 @@ void WebSocketCPUResume(DebuggerRequest &req) { CBreakPoints::SetSkipFirst(currentMIPS->pc); if (currentMIPS->inDelaySlot) { - Core_DoSingleStep(); + Core_RequestSingleStep(CPUStepType::Into, 1); } Core_Resume(); } diff --git a/Core/Debugger/WebSocket/SteppingSubscriber.cpp b/Core/Debugger/WebSocket/SteppingSubscriber.cpp index 468185ed8c..03693817ad 100644 --- a/Core/Debugger/WebSocket/SteppingSubscriber.cpp +++ b/Core/Debugger/WebSocket/SteppingSubscriber.cpp @@ -107,9 +107,7 @@ void WebSocketSteppingState::Into(DebuggerRequest &req) { CBreakPoints::SetSkipFirst(currentMIPS->pc); int c = GetNextInstructionCount(cpuDebug); - for (int i = 0; i < c; ++i) { - Core_DoSingleStep(); - } + Core_RequestSingleStep(CPUStepType::Into, c); } else { uint32_t breakpointAddress = cpuDebug->GetPC(); PrepareResume(); @@ -278,7 +276,8 @@ int WebSocketSteppingState::GetNextInstructionCount(DebugInterface *cpuDebug) { void WebSocketSteppingState::PrepareResume() { if (currentMIPS->inDelaySlot) { - Core_DoSingleStep(); + // Delay slot instructions are never joined, so we pass 1. + Core_RequestSingleStep(CPUStepType::Into, 1); } else { // If the current PC is on a breakpoint, the user doesn't want to do nothing. CBreakPoints::SetSkipFirst(currentMIPS->pc); diff --git a/Core/SaveState.cpp b/Core/SaveState.cpp index 6cc7ea63dd..ea83b2a243 100644 --- a/Core/SaveState.cpp +++ b/Core/SaveState.cpp @@ -419,7 +419,6 @@ namespace SaveState // Don't actually run it until next frame. // It's possible there might be a duplicate but it won't hurt us. needsProcess = true; - Core_UpdateSingleStep(); } void Load(const Path &filename, int slot, Callback callback, void *cbUserData) diff --git a/Core/System.cpp b/Core/System.cpp index 69e92f0fb2..b2cd17587b 100644 --- a/Core/System.cpp +++ b/Core/System.cpp @@ -118,6 +118,9 @@ static volatile bool pspIsIniting = false; static volatile bool pspIsQuitting = false; static volatile bool pspIsRebooting = false; +// This is called on EmuThread before RunLoop. +void Core_ProcessStepping(MIPSDebugInterface *cpu); + void ResetUIState() { globalUIState = UISTATE_MENU; } @@ -386,7 +389,6 @@ void Core_UpdateState(CoreState newState) { if ((coreState == CORE_RUNNING || coreState == CORE_NEXTFRAME) && newState != CORE_RUNNING) coreStatePending = true; coreState = newState; - Core_UpdateSingleStep(); } void Core_UpdateDebugStats(bool collectStats) { @@ -629,7 +631,7 @@ void PSP_RunLoopUntil(u64 globalticks) { if (coreState == CORE_POWERDOWN || coreState == CORE_BOOT_ERROR || coreState == CORE_RUNTIME_ERROR) { return; } else if (coreState == CORE_STEPPING) { - Core_ProcessStepping(); + Core_ProcessStepping(currentDebugMIPS); return; } diff --git a/GPU/Debugger/Stepping.cpp b/GPU/Debugger/Stepping.cpp index db6b657e73..1898bce1e8 100644 --- a/GPU/Debugger/Stepping.cpp +++ b/GPU/Debugger/Stepping.cpp @@ -74,8 +74,8 @@ static void SetPauseAction(PauseAction act, bool waitComplete = true) { pauseAction = act; pauseLock.unlock(); - if (coreState == CORE_STEPPING && act != PAUSE_CONTINUE) - Core_UpdateSingleStep(); + // if (coreState == CORE_STEPPING && act != PAUSE_CONTINUE) + // Core_UpdateSingleStep(); actionComplete = false; pauseWait.notify_all(); diff --git a/Windows/Debugger/Debugger_Disasm.cpp b/Windows/Debugger/Debugger_Disasm.cpp index 67870e9032..8935cd975e 100644 --- a/Windows/Debugger/Debugger_Disasm.cpp +++ b/Windows/Debugger/Debugger_Disasm.cpp @@ -208,7 +208,7 @@ void CDisasm::step(CPUStepType stepType) { lastTicks_ = CoreTiming::GetTicks(); u32 stepSize = ptr->getInstructionSizeAt(cpu->GetPC()); - Core_PerformStep(cpu, stepType, stepSize); + Core_RequestSingleStep(stepType, stepSize); } void CDisasm::runToLine() {