From eeef0a92bbe1ea48d354040e839a379a3dbfd35b Mon Sep 17 00:00:00 2001
From: Stenzek <stenzek@gmail.com>
Date: Sat, 10 Feb 2024 23:16:44 +0900
Subject: [PATCH] CPU: Make single step go through the "normal" execution path
That way it exits and re-enters the dynarec as expected.
---
src/core/cpu_core.cpp | 94 ++++++++++++++++++++++++-------------------
src/core/cpu_core.h | 9 +++--
src/core/system.cpp | 16 ++------
3 files changed, 63 insertions(+), 56 deletions(-)
diff --git a/src/core/cpu_core.cpp b/src/core/cpu_core.cpp
index 1e78d698e..4338816d1 100644
--- a/src/core/cpu_core.cpp
+++ b/src/core/cpu_core.cpp
@@ -1,12 +1,8 @@
-// SPDX-FileCopyrightText: 2019-2022 Connor McLaughlin <stenzek@gmail.com>
+// SPDX-FileCopyrightText: 2019-2024 Connor McLaughlin <stenzek@gmail.com>
// SPDX-License-Identifier: (GPL-3.0 OR CC-BY-NC-ND-4.0)
#include "cpu_core.h"
#include "bus.h"
-#include "common/align.h"
-#include "common/fastjmp.h"
-#include "common/file_system.h"
-#include "common/log.h"
#include "cpu_code_cache_private.h"
#include "cpu_core_private.h"
#include "cpu_disasm.h"
@@ -18,7 +14,14 @@
#include "settings.h"
#include "system.h"
#include "timing_event.h"
+
#include "util/state_wrapper.h"
+
+#include "common/align.h"
+#include "common/fastjmp.h"
+#include "common/file_system.h"
+#include "common/log.h"
+
#include <cstdio>
Log_SetChannel(CPU::Core);
@@ -45,7 +48,7 @@ static bool IsCop0ExecutionBreakpointUnmasked();
static void Cop0ExecutionBreakpointCheck();
template<MemoryAccessType type>
static void Cop0DataBreakpointCheck(VirtualMemoryAddress address);
-static bool BreakpointCheck();
+static void BreakpointCheck();
#ifdef _DEBUG
static void TracePrintInstruction();
@@ -94,8 +97,7 @@ static constexpr u32 INVALID_BREAKPOINT_PC = UINT32_C(0xFFFFFFFF);
static std::vector<Breakpoint> s_breakpoints;
static u32 s_breakpoint_counter = 1;
static u32 s_last_breakpoint_check_pc = INVALID_BREAKPOINT_PC;
-static bool s_single_step = false;
-static bool s_single_step_done = false;
+static u8 s_single_step = 0; // 0 - off, 1 - executing one, 2 - done
} // namespace CPU
bool CPU::IsTraceEnabled()
@@ -157,7 +159,7 @@ void CPU::Initialize()
s_breakpoints.clear();
s_breakpoint_counter = 1;
s_last_breakpoint_check_pc = INVALID_BREAKPOINT_PC;
- s_single_step = false;
+ s_single_step = 0;
UpdateMemoryPointers();
@@ -1932,7 +1934,7 @@ void CPU::DispatchInterrupt()
bool CPU::UpdateDebugDispatcherFlag()
{
- const bool has_any_breakpoints = !s_breakpoints.empty();
+ const bool has_any_breakpoints = !s_breakpoints.empty() || (s_single_step != 0);
// TODO: cop0 breakpoints
const auto& dcic = g_state.cop0_regs.dcic;
@@ -2120,29 +2122,18 @@ bool CPU::AddStepOutBreakpoint(u32 max_instructions_to_search)
return false;
}
-bool CPU::BreakpointCheck()
+ALWAYS_INLINE_RELEASE void CPU::BreakpointCheck()
{
const u32 pc = g_state.pc;
- // single step - we want to break out after this instruction, so set a pending exit
- // the bp check happens just before execution, so this is fine
- if (s_single_step)
- {
- if (s_single_step_done)
- ExitExecution();
- else
- s_single_step_done = true;
-
- s_last_breakpoint_check_pc = pc;
- return false;
- }
-
- if (pc == s_last_breakpoint_check_pc)
+ if (pc == s_last_breakpoint_check_pc && !s_single_step) [[unlikely]]
{
// we don't want to trigger the same breakpoint which just paused us repeatedly.
- return false;
+ return;
}
+ s_last_breakpoint_check_pc = pc;
+
u32 count = static_cast<u32>(s_breakpoints.size());
for (u32 i = 0; i < count;)
{
@@ -2186,12 +2177,31 @@ bool CPU::BreakpointCheck()
i++;
}
+ s_single_step = 0;
ExitExecution();
}
}
- s_last_breakpoint_check_pc = pc;
- return System::IsPaused();
+ // single step - we want to break out after this instruction, so set a pending exit
+ // the bp check happens just before execution, so this is fine
+ if (s_single_step != 0) [[unlikely]]
+ {
+ // not resetting this will break double single stepping, because we broke on the next instruction.
+ s_last_breakpoint_check_pc = INVALID_BREAKPOINT_PC;
+
+ if (s_single_step == 2)
+ {
+ s_single_step = 0;
+ System::PauseSystem(true);
+
+ Host::ReportFormattedDebuggerMessage("Stepped to 0x%08X.", g_state.pc);
+ UpdateDebugDispatcherFlag();
+ ExitExecution();
+ }
+
+ // if there's a bp on the instruction we're about to execute, we should've paused already
+ s_single_step = 2;
+ }
}
template<PGXPMode pgxp_mode, bool debug>
@@ -2206,12 +2216,7 @@ template<PGXPMode pgxp_mode, bool debug>
if constexpr (debug)
{
Cop0ExecutionBreakpointCheck();
-
- if (BreakpointCheck())
- {
- // continue is measurably faster than break on msvc for some reason
- continue;
- }
+ BreakpointCheck();
}
g_state.pending_ticks++;
@@ -2280,8 +2285,17 @@ void CPU::Execute()
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)
- g_state.npc = g_state.pc;
+ {
+ if (!SafeReadInstruction(g_state.pc, &g_state.next_instruction.bits)) [[unlikely]]
+ {
+ g_state.next_instruction.bits = 0;
+ Log_ErrorFmt("Failed to read current instruction from 0x{:08X}", g_state.pc);
+ }
+
+ g_state.npc = g_state.pc + sizeof(Instruction);
+ }
return;
}
@@ -2319,13 +2333,11 @@ void CPU::Execute()
}
}
-void CPU::SingleStep()
+void CPU::SetSingleStepFlag()
{
- s_single_step = true;
- s_single_step_done = false;
- if (fastjmp_set(&s_jmp_buf) == 0)
- ExecuteDebug();
- Host::ReportFormattedDebuggerMessage("Stepped to 0x%08X.", g_state.pc);
+ s_single_step = 1;
+ if (UpdateDebugDispatcherFlag())
+ System::InterruptExecution();
}
template<PGXPMode pgxp_mode>
diff --git a/src/core/cpu_core.h b/src/core/cpu_core.h
index 737317dba..fdadd6483 100644
--- a/src/core/cpu_core.h
+++ b/src/core/cpu_core.h
@@ -1,11 +1,14 @@
-// SPDX-FileCopyrightText: 2019-2022 Connor McLaughlin <stenzek@gmail.com>
+// SPDX-FileCopyrightText: 2019-2024 Connor McLaughlin <stenzek@gmail.com>
// SPDX-License-Identifier: (GPL-3.0 OR CC-BY-NC-ND-4.0)
#pragma once
-#include "common/bitfield.h"
+
#include "cpu_types.h"
#include "gte_types.h"
#include "types.h"
+
+#include "common/bitfield.h"
+
#include <array>
#include <optional>
#include <string>
@@ -127,7 +130,6 @@ void ExecutionModeChanged();
/// Executes interpreter loop.
void Execute();
-void SingleStep();
// Forces an early exit from the CPU dispatcher.
void ExitExecution();
@@ -214,6 +216,7 @@ bool RemoveBreakpoint(VirtualMemoryAddress address);
void ClearBreakpoints();
bool AddStepOverBreakpoint();
bool AddStepOutBreakpoint(u32 max_instructions_to_search = 1000);
+void SetSingleStepFlag();
extern bool TRACE_EXECUTION;
diff --git a/src/core/system.cpp b/src/core/system.cpp
index 40291448d..cf883508e 100644
--- a/src/core/system.cpp
+++ b/src/core/system.cpp
@@ -1932,19 +1932,11 @@ void System::Throttle()
void System::SingleStepCPU()
{
- s_frame_timer.Reset();
- s_system_executing = true;
+ CPU::SetSingleStepFlag();
- g_gpu->RestoreDeviceContext();
-
- CPU::SingleStep();
-
- g_gpu->FlushRender();
- SPU::GeneratePendingSamples();
-
- InvalidateDisplay();
-
- s_system_executing = false;
+ // If this gets called when the system is executing, we're not going to end up here..
+ if (IsPaused())
+ PauseSystem(false);
}
void System::IncrementInternalFrameNumber()